Enforce correctness of self-dependencies (#9705)

## Summary

As far as I can tell, this was added in
#319, but it seems _incorrect_ to
ignore these.

Closes #9693.

Author: charliermarsh

crates/uv-resolver/src/pubgrub/dependencies.rs

@@ -2,7 +2,6 @@ use std::iter;
 
 use either::Either;
 use pubgrub::Ranges;
-use tracing::warn;
 
 use uv_normalize::{ExtraName, GroupName, PackageName};
 use uv_pep440::{Version, VersionSpecifiers};
@@ -91,42 +90,23 @@ impl PubGrubDependency {
 
         // Add the package, plus any extra variants.
         iter.map(|(extra, group)| PubGrubRequirement::from_requirement(requirement, extra, group))
-            .filter_map(move |requirement| {
+            .map(move |requirement| {
                 let PubGrubRequirement {
                     package,
                     version,
                     url,
                 } = requirement;
                 match &*package {
-                    PubGrubPackageInner::Package { name, .. } => {
-                        // Detect self-dependencies.
-                        if dev.is_none() {
-                            if source_name.is_some_and(|source_name| source_name == name) {
-                                warn!("{name} has a dependency on itself");
-                                return None;
-                            }
-                        }
-
-                        Some(PubGrubDependency {
-                            package: package.clone(),
-                            version: version.clone(),
-                            url,
-                        })
-                    }
-                    PubGrubPackageInner::Marker { name, .. } => {
-                        // Detect self-dependencies.
-                        if dev.is_none() {
-                            if source_name.is_some_and(|source_name| source_name == name) {
-                                return None;
-                            }
-                        }
-
-                        Some(PubGrubDependency {
-                            package: package.clone(),
-                            version: version.clone(),
-                            url,
-                        })
-                    }
+                    PubGrubPackageInner::Package { .. } => PubGrubDependency {
+                        package: package.clone(),
+                        version: version.clone(),
+                        url,
+                    },
+                    PubGrubPackageInner::Marker { .. } => PubGrubDependency {
+                        package: package.clone(),
+                        version: version.clone(),
+                        url,
+                    },
                     PubGrubPackageInner::Extra { name, .. } => {
                         // Detect self-dependencies.
                         if dev.is_none() {
@@ -135,11 +115,11 @@ impl PubGrubDependency {
                                 "extras not flattened for {name}"
                             );
                         }
-                        Some(PubGrubDependency {
+                        PubGrubDependency {
                             package: package.clone(),
                             version: version.clone(),
                             url,
-                        })
+                        }
                     }
                     PubGrubPackageInner::Dev { name, .. } => {
                         // Detect self-dependencies.
@@ -149,11 +129,11 @@ impl PubGrubDependency {
                                 "group not flattened for {name}"
                             );
                         }
-                        Some(PubGrubDependency {
+                        PubGrubDependency {
                             package: package.clone(),
                             version: version.clone(),
                             url,
-                        })
+                        }
                     }
                     PubGrubPackageInner::Root(_) => unreachable!("root package in dependencies"),
                     PubGrubPackageInner::Python(_) => {

crates/uv-resolver/src/pubgrub/report.rs

@@ -133,6 +133,16 @@ impl ReportFormatter<PubGrubPackage, Range<Version>, UnavailableReason>
             External::FromDependencyOf(package, package_set, dependency, dependency_set) => {
                 let package_set = self.simplify_set(package_set, package);
                 let dependency_set = self.simplify_set(dependency_set, dependency);
+
+                if package == dependency {
+                    if let Some(member) = self.format_workspace_member(package) {
+                        return format!(
+                            "{member} depends on itself at an incompatible version ({})",
+                            PackageRange::dependency(package, &dependency_set, None)
+                        );
+                    }
+                }
+
                 if let Some(root) = self.format_root_requires(package) {
                     return format!(
                         "{root} {}",
@@ -407,6 +417,24 @@ impl PubGrubReportFormatter<'_> {
         }
     }
 
+    /// Return whether the given package is the root package.
+    fn is_root(package: &PubGrubPackage) -> bool {
+        matches!(&**package, PubGrubPackageInner::Root(_))
+    }
+
+    /// Return whether the given package is a workspace member.
+    fn is_single_project_workspace_member(&self, package: &PubGrubPackage) -> bool {
+        match &**package {
+            // TODO(zanieb): Improve handling of dev and extra for single-project workspaces
+            PubGrubPackageInner::Package {
+                name, extra, dev, ..
+            } if self.workspace_members.contains(name) => {
+                self.is_single_project_workspace() && extra.is_none() && dev.is_none()
+            }
+            _ => false,
+        }
+    }
+
     /// Create a [`PackageRange::compatibility`] display with this formatter attached.
     fn compatible_range<'a>(
         &'a self,
@@ -467,6 +495,18 @@ impl PubGrubReportFormatter<'_> {
                         .and(dependency2.package, &dependency_set2),
                 )
             }
+            (.., External::FromDependencyOf(package, _, dependency, _))
+                if Self::is_root(package)
+                    && self.is_single_project_workspace_member(dependency) =>
+            {
+                self.format_external(external1)
+            }
+            (External::FromDependencyOf(package, _, dependency, _), ..)
+                if Self::is_root(package)
+                    && self.is_single_project_workspace_member(dependency) =>
+            {
+                self.format_external(external2)
+            }
             _ => {
                 let external1 = self.format_external(external1);
                 let external2 = self.format_external(external2);
@@ -570,6 +610,16 @@ impl PubGrubReportFormatter<'_> {
                             workspace: self.is_workspace() && !self.is_single_project_workspace(),
                         });
                     }
+
+                    if package_name == dependency_name
+                        && (dependency.extra().is_none() || package.extra() == dependency.extra())
+                        && (dependency.dev().is_none() || dependency.dev() == package.dev())
+                        && workspace_members.contains(package_name)
+                    {
+                        output_hints.insert(PubGrubHint::DependsOnItself {
+                            package: package.clone(),
+                        });
+                    }
                 }
                 // Check for no versions due to `Requires-Python`.
                 if matches!(
@@ -899,6 +949,8 @@ pub(crate) enum PubGrubHint {
         dependency: PubGrubPackage,
         workspace: bool,
     },
+    /// A package depends on itself at an incompatible version.
+    DependsOnItself { package: PubGrubPackage },
     /// A package was available on an index, but not at the correct version, and at least one
     /// subsequent index was not queried. As such, a compatible version may be available on an
     /// one of the remaining indexes.
@@ -963,6 +1015,9 @@ enum PubGrubHintCore {
         dependency: PubGrubPackage,
         workspace: bool,
     },
+    DependsOnItself {
+        package: PubGrubPackage,
+    },
     UncheckedIndex {
         package: PubGrubPackage,
     },
@@ -1027,6 +1082,7 @@ impl From<PubGrubHint> for PubGrubHintCore {
                 dependency,
                 workspace,
             },
+            PubGrubHint::DependsOnItself { package } => Self::DependsOnItself { package },
             PubGrubHint::UncheckedIndex { package, .. } => Self::UncheckedIndex { package },
             PubGrubHint::UnauthorizedIndex { index } => Self::UnauthorizedIndex { index },
             PubGrubHint::ForbiddenIndex { index } => Self::ForbiddenIndex { index },
@@ -1269,6 +1325,15 @@ impl std::fmt::Display for PubGrubHint {
                     dependency.cyan(),
                 )
             }
+            Self::DependsOnItself { package } => {
+                write!(
+                    f,
+                    "{}{} The package `{}` depends on itself. This is likely a mistake. Consider removing the dependency.",
+                    "hint".bold().cyan(),
+                    ":".bold(),
+                    package.cyan(),
+                )
+            }
             Self::UncheckedIndex {
                 package,
                 range,

crates/uv-resolver/src/resolver/mod.rs

@@ -2453,8 +2453,24 @@ impl ForkState {
                         extra: ref dependency_extra,
                         dev: ref dependency_dev,
                         marker: ref dependency_marker,
-                        ..
                     } => {
+                        debug_assert!(
+                            dependency_extra.is_none(),
+                            "Packages should depend on an extra proxy"
+                        );
+                        debug_assert!(
+                            dependency_dev.is_none(),
+                            "Packages should depend on a group proxy"
+                        );
+
+                        // Ignore self-dependencies (e.g., `tensorflow-macos` depends on `tensorflow-macos`),
+                        // but allow groups to depend on other groups, or on the package itself.
+                        if self_dev.is_none() {
+                            if self_name == Some(dependency_name) {
+                                continue;
+                            }
+                        }
+
                         let to_url = self.fork_urls.get(dependency_name);
                         let to_index = self.fork_indexes.get(dependency_name);
                         let edge = ResolutionDependencyEdge {
@@ -2478,8 +2494,15 @@ impl ForkState {
                     PubGrubPackageInner::Marker {
                         name: ref dependency_name,
                         marker: ref dependency_marker,
-                        ..
                     } => {
+                        // Ignore self-dependencies (e.g., `tensorflow-macos` depends on `tensorflow-macos`),
+                        // but allow groups to depend on other groups, or on the package itself.
+                        if self_dev.is_none() {
+                            if self_name == Some(dependency_name) {
+                                continue;
+                            }
+                        }
+
                         let to_url = self.fork_urls.get(dependency_name);
                         let to_index = self.fork_indexes.get(dependency_name);
                         let edge = ResolutionDependencyEdge {
@@ -2504,8 +2527,14 @@ impl ForkState {
                         name: ref dependency_name,
                         extra: ref dependency_extra,
                         marker: ref dependency_marker,
-                        ..
                     } => {
+                        if self_dev.is_none() {
+                            debug_assert!(
+                                self_name != Some(dependency_name),
+                                "Extras should be flattened"
+                            );
+                        }
+
                         // Insert an edge from the dependent package to the extra package.
                         let to_url = self.fork_urls.get(dependency_name);
                         let to_index = self.fork_indexes.get(dependency_name);
@@ -2551,8 +2580,12 @@ impl ForkState {
                         name: ref dependency_name,
                         dev: ref dependency_dev,
                         marker: ref dependency_marker,
-                        ..
                     } => {
+                        debug_assert!(
+                            self_name != Some(dependency_name),
+                            "Groups should be flattened"
+                        );
+
                         // Add an edge from the dependent package to the dev package, but _not_ the
                         // base package.
                         let to_url = self.fork_urls.get(dependency_name);