From f3965fef5ea675a5b709a397d06ff37c271deebd Mon Sep 17 00:00:00 2001
From: Charlie Marsh <charlie.r.marsh@gmail.com>
Date: Mon, 20 May 2024 09:25:31 -0400
Subject: [PATCH] Use filename trait for `WheelWire` conversion (#3651)

## Summary

The main motivation here is that the `.filename()` method that we
implement on `Url` will do URL decoding for the last segment, which we
were missing here.

The errors are a bit awkward, because in
`crates/uv-resolver/src/lock.rs`, we wrap in `failed to extract filename
from URL: {url}`, so in theory we want the underlying errors to _omit_
the URL? But sometimes they use `#[error(transparent)]`?
---
 crates/distribution-types/src/error.rs |  7 +++++--
 crates/distribution-types/src/lib.rs   | 10 ++++++----
 crates/uv-installer/src/plan.rs        |  2 +-
 crates/uv-resolver/src/lock.rs         | 15 +++++++--------
 4 files changed, 19 insertions(+), 15 deletions(-)

diff --git a/crates/distribution-types/src/error.rs b/crates/distribution-types/src/error.rs
index 1e2b2a193..80a2206b2 100644
--- a/crates/distribution-types/src/error.rs
+++ b/crates/distribution-types/src/error.rs
@@ -14,8 +14,11 @@ pub enum Error {
     #[error(transparent)]
     WheelFilename(#[from] distribution_filename::WheelFilenameError),
 
-    #[error("Unable to extract filename from URL: {0}")]
-    UrlFilename(Url),
+    #[error("Unable to extract file path from URL: {0}")]
+    MissingFilePath(Url),
+
+    #[error("Could not extract path segments from URL: {0}")]
+    MissingPathSegments(Url),
 
     #[error("Distribution not found at: {0}")]
     NotFound(Url),
diff --git a/crates/distribution-types/src/lib.rs b/crates/distribution-types/src/lib.rs
index 4739b6cac..cf072ff6c 100644
--- a/crates/distribution-types/src/lib.rs
+++ b/crates/distribution-types/src/lib.rs
@@ -709,13 +709,15 @@ impl RemoteSource for File {
 impl RemoteSource for Url {
     fn filename(&self) -> Result<Cow<'_, str>, Error> {
         // Identify the last segment of the URL as the filename.
-        let filename = self
+        let path_segments = self
             .path_segments()
-            .and_then(Iterator::last)
-            .ok_or_else(|| Error::UrlFilename(self.clone()))?;
+            .ok_or_else(|| Error::MissingPathSegments(self.clone()))?;
+
+        // This is guaranteed by the contract of `Url::path_segments`.
+        let last = path_segments.last().expect("path segments is non-empty");
 
         // Decode the filename, which may be percent-encoded.
-        let filename = urlencoding::decode(filename)?;
+        let filename = urlencoding::decode(last)?;
 
         Ok(filename)
     }
diff --git a/crates/uv-installer/src/plan.rs b/crates/uv-installer/src/plan.rs
index 02aa0274d..b6dd5b9c0 100644
--- a/crates/uv-installer/src/plan.rs
+++ b/crates/uv-installer/src/plan.rs
@@ -336,7 +336,7 @@ impl<'a> Planner<'a> {
                     // Store the canonicalized path, which also serves to validate that it exists.
                     let path = match url
                         .to_file_path()
-                        .map_err(|()| Error::UrlFilename(url.to_url()))?
+                        .map_err(|()| Error::MissingFilePath(url.to_url()))?
                         .canonicalize()
                     {
                         Ok(path) => path,
diff --git a/crates/uv-resolver/src/lock.rs b/crates/uv-resolver/src/lock.rs
index 5587f8a12..3a1791160 100644
--- a/crates/uv-resolver/src/lock.rs
+++ b/crates/uv-resolver/src/lock.rs
@@ -1087,15 +1087,14 @@ impl TryFrom<WheelWire> for Wheel {
     type Error = String;
 
     fn try_from(wire: WheelWire) -> Result<Wheel, String> {
-        let path_segments = wire
-            .url
-            .path_segments()
-            .ok_or_else(|| format!("could not extract path from URL `{}`", wire.url))?;
-        // This is guaranteed by the contract of Url::path_segments.
-        let last = path_segments.last().expect("path segments is non-empty");
-        let filename = last
+        // Extract the filename segment from the URL.
+        let filename = wire.url.filename().map_err(|err| err.to_string())?;
+
+        // Parse the filename as a wheel filename.
+        let filename = filename
             .parse::<WheelFilename>()
-            .map_err(|err| format!("failed to parse `{last}` as wheel filename: {err}"))?;
+            .map_err(|err| format!("failed to parse `{filename}` as wheel filename: {err}"))?;
+
         Ok(Wheel {
             url: wire.url,
             hash: wire.hash,