From fbb00f701c5678d4ad397c9405cd271dffe36fc9 Mon Sep 17 00:00:00 2001
From: konsti <konstin@mailbox.org>
Date: Thu, 18 Jul 2024 05:44:53 +0200
Subject: [PATCH] Warn about unconstrained direct deps in lowest resolution
 (#5142)

Warn when there is a direct dependency without a lower bound and
`--resolution lowest` is set.

---------

Co-authored-by: Zanie Blue <contact@zanie.dev>
---
 crates/uv-resolver/src/resolver/mod.rs | 20 ++++++++++++++++++++
 crates/uv/tests/pip_compile.rs         |  6 +++++-
 2 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/crates/uv-resolver/src/resolver/mod.rs b/crates/uv-resolver/src/resolver/mod.rs
index 89346ad89..7371e7b0d 100644
--- a/crates/uv-resolver/src/resolver/mod.rs
+++ b/crates/uv-resolver/src/resolver/mod.rs
@@ -38,6 +38,7 @@ use uv_distribution::{ArchiveMetadata, DistributionDatabase};
 use uv_git::GitResolver;
 use uv_normalize::{ExtraName, GroupName, PackageName};
 use uv_types::{BuildContext, HashStrategy, InstalledPackagesProvider};
+use uv_warnings::warn_user_once;
 
 use crate::candidate_selector::{CandidateDist, CandidateSelector};
 use crate::dependency_provider::UvDependencyProvider;
@@ -53,6 +54,7 @@ use crate::pubgrub::{
 };
 use crate::python_requirement::PythonRequirement;
 use crate::resolution::ResolutionGraph;
+use crate::resolution_mode::ResolutionStrategy;
 pub(crate) use crate::resolver::availability::{
     IncompletePackage, ResolverVersion, UnavailablePackage, UnavailableReason, UnavailableVersion,
 };
@@ -512,6 +514,7 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
                                 &self.urls,
                                 dependencies.clone(),
                                 &self.git,
+                                self.selector.resolution_strategy(),
                             )?;
                             // Emit a request to fetch the metadata for each registry package.
                             for dependency in &dependencies {
@@ -585,6 +588,7 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
                                     &self.urls,
                                     fork.dependencies.clone(),
                                     &self.git,
+                                    self.selector.resolution_strategy(),
                                 )?;
                                 // Emit a request to fetch the metadata for each registry package.
                                 for dependency in &fork.dependencies {
@@ -2031,6 +2035,7 @@ impl ForkState {
         urls: &Urls,
         dependencies: Vec<PubGrubDependency>,
         git: &GitResolver,
+        resolution_strategy: &ResolutionStrategy,
     ) -> Result<(), ResolveError> {
         for dependency in &dependencies {
             let PubGrubDependency {
@@ -2040,6 +2045,7 @@ impl ForkState {
                 local,
             } = dependency;
 
+            let mut has_url = false;
             if let Some(name) = package.name() {
                 // From the [`Requirement`] to [`PubGrubDependency`] conversion, we get a URL if the
                 // requirement was a URL requirement. `Urls` applies canonicalization to this and
@@ -2047,6 +2053,7 @@ impl ForkState {
                 // conflicts using [`ForkUrl`].
                 if let Some(url) = urls.get_url(name, url.as_ref(), git)? {
                     self.fork_urls.insert(name, url, &self.markers)?;
+                    has_url = true;
                 };
 
                 // `PubGrubDependency` also gives us a local version if specified by the user.
@@ -2062,6 +2069,19 @@ impl ForkState {
             } else {
                 // A dependency from the root package or requirements.txt.
                 debug!("Adding direct dependency: {package}{version}");
+
+                // Warn the user if the direct dependency lacks a lower bound in lowest resolution.
+                let missing_lower_bound = version
+                    .bounding_range()
+                    .map(|(lowest, _highest)| lowest == Bound::Unbounded)
+                    .unwrap_or(true);
+                let strategy_lowest = matches!(
+                    resolution_strategy,
+                    ResolutionStrategy::Lowest | ResolutionStrategy::LowestDirect(..)
+                );
+                if !has_url && missing_lower_bound && strategy_lowest {
+                    warn_user_once!("The direct dependency `{package}` is unpinned. Consider setting a lower bound when using `--resolution-strategy lowest` to avoid using outdated versions.");
+                }
             }
 
             // Update the package priorities.
diff --git a/crates/uv/tests/pip_compile.rs b/crates/uv/tests/pip_compile.rs
index f0478c2ba..c86818da1 100644
--- a/crates/uv/tests/pip_compile.rs
+++ b/crates/uv/tests/pip_compile.rs
@@ -9520,12 +9520,15 @@ fn compile_index_url_unsafe_highest() -> Result<()> {
 /// In this case, anyio 3.5.0 is hosted on the "extra" index, but older versions are available on
 /// the "primary" index. We should prefer the older version from the "primary" index, despite the
 /// "extra" index being the preferred index.
+///
+/// We also test here that a warning is raised for missing lower bounds on direct dependencies with
+/// `--resolution lowest`.
 #[test]
 fn compile_index_url_unsafe_lowest() -> Result<()> {
     let context = TestContext::new("3.12");
 
     let requirements_in = context.temp_dir.child("requirements.in");
-    requirements_in.write_str("anyio")?;
+    requirements_in.write_str("anyio<100")?;
 
     uv_snapshot!(context.pip_compile()
         .arg("--resolution")
@@ -9547,6 +9550,7 @@ fn compile_index_url_unsafe_lowest() -> Result<()> {
         # via -r requirements.in
 
     ----- stderr -----
+    warning: The direct dependency `anyio` is unpinned. Consider setting a lower bound when using `--resolution-strategy lowest` to avoid using outdated versions.
     Resolved 1 package in [TIME]
     "###
     );