Refactor distribution types to return Result (#470)

## Summary

A variety of small refactors to the distribution types crate to (1)
return `Result` if we find an invalid wheel, rather than treating it as
a source distribution with a `.whl` suffix, and (2) DRY up some repeated
code around URLs.

crates/puffin-resolver/src/error.rs

@@ -50,6 +50,9 @@ pub enum ResolveError {
     #[error("Package `{0}` attempted to resolve via URL: {1}. URL dependencies must be expressed as direct requirements or constraints. Consider adding `{0} @ {1}` to your dependencies or constraints file.")]
     DisallowedUrl(PackageName, Url),
 
+    #[error(transparent)]
+    DistributionType(#[from] distribution_types::Error),
+
     #[error("Failed to fetch wheel metadata from: {filename}")]
     RegistryBuiltDist {
         filename: String,

crates/puffin-resolver/src/finder.rs

@@ -87,7 +87,7 @@ impl<'a> DistFinder<'a> {
                 }
                 Some(VersionOrUrl::Url(url)) => {
                     let package_name = requirement.name.clone();
-                    let package = Dist::from_url(package_name.clone(), url.clone());
+                    let package = Dist::from_url(package_name.clone(), url.clone())?;
                     resolution.insert(package_name, package);
                 }
             }

crates/puffin-resolver/src/resolution.rs

@@ -1,22 +1,23 @@
 use std::hash::BuildHasherDefault;
 
+use anyhow::Result;
 use colored::Colorize;
+use distribution_types::{BuiltDist, Dist, Metadata, SourceDist};
 use fxhash::FxHashMap;
+use pep440_rs::{Version, VersionSpecifier, VersionSpecifiers};
+use pep508_rs::{Requirement, VersionOrUrl};
 use petgraph::visit::EdgeRef;
 use petgraph::Direction;
 use pubgrub::range::Range;
 use pubgrub::solver::{Kind, State};
 use pubgrub::type_aliases::SelectedDependencies;
+use puffin_normalize::PackageName;
+use pypi_types::{File, IndexUrl};
 use url::Url;
 use waitmap::WaitMap;
 
-use distribution_types::{BuiltDist, Dist, Metadata, SourceDist};
-use pep440_rs::{Version, VersionSpecifier, VersionSpecifiers};
-use pep508_rs::{Requirement, VersionOrUrl};
-use puffin_normalize::PackageName;
-use pypi_types::{File, IndexUrl};
-
 use crate::pubgrub::{PubGrubPackage, PubGrubPriority, PubGrubVersion};
+use crate::ResolveError;
 
 /// A set of packages pinned at specific versions.
 #[derive(Debug, Default)]
@@ -61,7 +62,7 @@ impl Graph {
         pins: &FxHashMap<PackageName, FxHashMap<Version, (IndexUrl, File)>>,
         redirects: &WaitMap<Url, Url>,
         state: &State<PubGrubPackage, Range<PubGrubVersion>, PubGrubPriority>,
-    ) -> Self {
+    ) -> Result<Self, ResolveError> {
         // TODO(charlie): petgraph is a really heavy and unnecessary dependency here. We should
         // write our own graph, given that our requirements are so simple.
         let mut graph = petgraph::graph::Graph::with_capacity(selection.len(), selection.len());
@@ -88,7 +89,7 @@ impl Graph {
                     let url = redirects
                         .get(url)
                         .map_or_else(|| url.clone(), |url| url.value().clone());
-                    let pinned_package = Dist::from_url(package_name.clone(), url);
+                    let pinned_package = Dist::from_url(package_name.clone(), url)?;
 
                     let index = graph.add_node(pinned_package);
                     inverse.insert(package_name, index);
@@ -124,7 +125,7 @@ impl Graph {
             }
         }
 
-        Self(graph)
+        Ok(Self(graph))
     }
 
     /// Return the number of packages in the graph.

crates/puffin-resolver/src/resolver.rs

@@ -21,7 +21,7 @@ use distribution_filename::WheelFilename;
 use distribution_types::{BuiltDist, Dist, Identifier, Metadata, SourceDist, VersionOrUrl};
 use pep508_rs::{MarkerEnvironment, Requirement};
 use platform_tags::Tags;
-use puffin_cache::metadata::WheelMetadataCache;
+
 use puffin_cache::CanonicalUrl;
 use puffin_client::RegistryClient;
 use puffin_distribution::Fetcher;
@@ -173,12 +173,7 @@ impl<'a, Context: BuildContext + Send + Sync> Resolver<'a, Context> {
                     })
             else {
                 let selection = state.partial_solution.extract_solution();
-                return Ok(Graph::from_state(
-                    &selection,
-                    &pins,
-                    &self.index.redirects,
-                    &state,
-                ));
+                return Graph::from_state(&selection, &pins, &self.index.redirects, &state);
             };
             next = highest_priority_pkg;
 
@@ -301,7 +296,7 @@ impl<'a, Context: BuildContext + Send + Sync> Resolver<'a, Context> {
             PubGrubPackage::Package(package_name, _extra, Some(url)) => {
                 // Emit a request to fetch the metadata for this distribution.
                 if in_flight.insert_url(url) {
-                    let distribution = Dist::from_url(package_name.clone(), url.clone());
+                    let distribution = Dist::from_url(package_name.clone(), url.clone())?;
                     priorities.add(distribution.name().clone());
                     request_sink.unbounded_send(Request::Dist(distribution))?;
                 }
@@ -606,32 +601,24 @@ impl<'a, Context: BuildContext + Send + Sync> Resolver<'a, Context> {
                     .await
             }
 
-            // Fetch wheel metadata.
-            Request::Dist(Dist::Built(distribution)) => {
-                let metadata = match &distribution {
-                    BuiltDist::Registry(wheel) => {
-                        self.client
-                            .wheel_metadata(wheel.index.clone(), wheel.file.clone())
-                            .await?
-                    }
-                    BuiltDist::DirectUrl(wheel) => {
-                        self.client
-                            .wheel_metadata_no_pep658(
-                                &wheel.filename,
-                                &wheel.url,
-                                WheelMetadataCache::Url,
-                            )
-                            .await?
-                    }
-                };
+            // Fetch registry-based wheel metadata.
+            Request::Dist(Dist::Built(BuiltDist::Registry(wheel))) => {
+                let metadata = self
+                    .client
+                    .wheel_metadata(wheel.index.clone(), wheel.file.clone())
+                    .await?;
 
-                if metadata.name != *distribution.name() {
+                if metadata.name != *wheel.name() {
                     return Err(ResolveError::NameMismatch {
                         metadata: metadata.name,
-                        given: distribution.name().clone(),
+                        given: wheel.name().clone(),
                     });
                 }
-                Ok(Response::Dist(Dist::Built(distribution), metadata, None))
+                Ok(Response::Dist(
+                    Dist::Built(BuiltDist::Registry(wheel)),
+                    metadata,
+                    None,
+                ))
             }
 
             // Fetch distribution metadata.