Merge 72af7aac00 into eee4ea2f5a

fix(cargo-lints): Don't always inherit workspace lints

When working on changes for #13805, I noticed that we always passed the contents of `[workspace.lints.cargo]` into the currently implemented lints,  even if `[lints]` was not specified or did not contain `workspace = true`. This PR makes it so we only pass in the workspace cargo lints if `[lints]` contains `workspace = true`.

You can verify this change by looking at the first commit, where I added a test showing the current behavior, and looking at the second commit and seeing the test output no longer shows a warning about specifying `im-a-teapot`.

src/cargo/core/workspace.rs

@@ -1147,11 +1147,26 @@ impl<'gctx> Workspace<'gctx> {
     }
 
     pub fn emit_warnings(&self) -> CargoResult<()> {
+        let ws_lints = self
+            .root_maybe()
+            .workspace_config()
+            .inheritable()
+            .and_then(|i| i.lints().ok())
+            .unwrap_or_default();
+
+        let ws_cargo_lints = ws_lints
+            .get("cargo")
+            .cloned()
+            .unwrap_or_default()
+            .into_iter()
+            .map(|(k, v)| (k.replace('-', "_"), v))
+            .collect();
+
         for (path, maybe_pkg) in &self.packages.packages {
             let path = path.join("Cargo.toml");
             if let MaybePackage::Package(pkg) = maybe_pkg {
                 if self.gctx.cli_unstable().cargo_lints {
-                    self.emit_lints(pkg, &path)?
+                    self.emit_lints(pkg, &path, &ws_cargo_lints)?
                 }
             }
             let warnings = match maybe_pkg {
@@ -1179,22 +1194,12 @@ impl<'gctx> Workspace<'gctx> {
         Ok(())
     }
 
-    pub fn emit_lints(&self, pkg: &Package, path: &Path) -> CargoResult<()> {
-        let ws_lints = self
-            .root_maybe()
-            .workspace_config()
-            .inheritable()
-            .and_then(|i| i.lints().ok())
-            .unwrap_or_default();
-
-        let ws_cargo_lints = ws_lints
-            .get("cargo")
-            .cloned()
-            .unwrap_or_default()
-            .into_iter()
-            .map(|(k, v)| (k.replace('-', "_"), v))
-            .collect();
-
+    pub fn emit_lints(
+        &self,
+        pkg: &Package,
+        path: &Path,
+        ws_cargo_lints: &manifest::TomlToolLints,
+    ) -> CargoResult<()> {
         let mut error_count = 0;
         let toml_lints = pkg
             .manifest()
@@ -1212,11 +1217,21 @@ impl<'gctx> Workspace<'gctx> {
             .map(|(name, lint)| (name.replace('-', "_"), lint))
             .collect();
 
+        // We should only be using workspace lints if the `[lints]` table is
+        // present in the manifest, and `workspace` is set to `true`
+        let ws_cargo_lints = pkg
+            .manifest()
+            .resolved_toml()
+            .lints
+            .as_ref()
+            .is_some_and(|l| l.workspace)
+            .then(|| ws_cargo_lints);
+
         check_im_a_teapot(
             pkg,
             &path,
             &normalized_lints,
-            &ws_cargo_lints,
+            ws_cargo_lints,
             &mut error_count,
             self.gctx,
         )?;
@@ -1224,7 +1239,7 @@ impl<'gctx> Workspace<'gctx> {
             pkg,
             &path,
             &normalized_lints,
-            &ws_cargo_lints,
+            ws_cargo_lints,
             &mut error_count,
             self.gctx,
         )?;
@@ -1232,7 +1247,7 @@ impl<'gctx> Workspace<'gctx> {
             pkg,
             &path,
             &normalized_lints,
-            &ws_cargo_lints,
+            ws_cargo_lints,
             &mut error_count,
             self.gctx,
         )?;

src/cargo/ops/cargo_add/mod.rs

@@ -364,24 +364,28 @@ fn resolve_dependency(
     };
     selected_dep = populate_dependency(selected_dep, arg);
 
-    let old_dep = get_existing_dependency(manifest, selected_dep.toml_key(), section)?;
-    let mut dependency = if let Some(mut old_dep) = old_dep.clone() {
-        if old_dep.name != selected_dep.name {
-            // Assuming most existing keys are not relevant when the package changes
-            if selected_dep.optional.is_none() {
-                selected_dep.optional = old_dep.optional;
+    let get_dependency = |mut selected_dep: Dependency| {
+        let old_dep = get_existing_dependency(manifest, selected_dep.toml_key(), section)?;
+        let dependency = if let Some(mut old_dep) = old_dep.clone() {
+            if old_dep.name != selected_dep.name {
+                // Assuming most existing keys are not relevant when the package changes
+                if selected_dep.optional.is_none() {
+                    selected_dep.optional = old_dep.optional;
+                }
+                selected_dep
+            } else {
+                if selected_dep.source().is_some() {
+                    // Overwrite with `crate_spec`
+                    old_dep.source = selected_dep.source;
+                }
+                populate_dependency(old_dep, arg)
             }
-            selected_dep
         } else {
-            if selected_dep.source().is_some() {
-                // Overwrite with `crate_spec`
-                old_dep.source = selected_dep.source;
-            }
-            populate_dependency(old_dep, arg)
-        }
-    } else {
-        selected_dep
+            selected_dep
+        };
+        anyhow::Ok((old_dep, dependency))
     };
+    let (mut old_dep, mut dependency) = get_dependency(selected_dep)?;
 
     if dependency.source().is_none() {
         // Checking for a workspace dependency happens first since a member could be specified
@@ -403,14 +407,23 @@ fn resolve_dependency(
             let latest =
                 get_latest_dependency(spec, &dependency, honor_rust_version, gctx, registry)?;
 
+            dependency = dependency.set_source(latest.source.expect("latest always has a source"));
+
             if dependency.name != latest.name {
                 gctx.shell().warn(format!(
                     "translating `{}` to `{}`",
                     dependency.name, latest.name,
                 ))?;
                 dependency.name = latest.name; // Normalize the name
+                (old_dep, dependency) = get_dependency(dependency.clone())?;
+
+                // Re-check workspace dependencies table with normalized name
+                if let Some(_dep) =
+                    find_workspace_dep(dependency.toml_key(), ws.root_manifest()).ok()
+                {
+                    dependency = dependency.set_source(WorkspaceSource::new());
+                }
             }
-            dependency = dependency.set_source(latest.source.expect("latest always has a source"));
         }
     }
 

src/cargo/util/lints.rs

@@ -88,7 +88,7 @@ impl Lint {
     pub fn level(
         &self,
         pkg_lints: &TomlToolLints,
-        ws_lints: &TomlToolLints,
+        ws_lints: Option<&TomlToolLints>,
         edition: Edition,
     ) -> (LintLevel, LintLevelReason) {
         self.groups
@@ -188,7 +188,7 @@ fn level_priority(
     default_level: LintLevel,
     edition_lint_opts: Option<(Edition, LintLevel)>,
     pkg_lints: &TomlToolLints,
-    ws_lints: &TomlToolLints,
+    ws_lints: Option<&TomlToolLints>,
     edition: Edition,
 ) -> (LintLevel, LintLevelReason, i8) {
     let (unspecified_level, reason) = if let Some(level) = edition_lint_opts
@@ -211,7 +211,7 @@ fn level_priority(
             LintLevelReason::Package,
             defined_level.priority(),
         )
-    } else if let Some(defined_level) = ws_lints.get(name) {
+    } else if let Some(defined_level) = ws_lints.and_then(|l| l.get(name)) {
         (
             defined_level.level().into(),
             LintLevelReason::Workspace,
@@ -234,7 +234,7 @@ pub fn check_im_a_teapot(
     pkg: &Package,
     path: &Path,
     pkg_lints: &TomlToolLints,
-    ws_lints: &TomlToolLints,
+    ws_lints: Option<&TomlToolLints>,
     error_count: &mut usize,
     gctx: &GlobalContext,
 ) -> CargoResult<()> {
@@ -306,7 +306,7 @@ pub fn check_implicit_features(
     pkg: &Package,
     path: &Path,
     pkg_lints: &TomlToolLints,
-    ws_lints: &TomlToolLints,
+    ws_lints: Option<&TomlToolLints>,
     error_count: &mut usize,
     gctx: &GlobalContext,
 ) -> CargoResult<()> {
@@ -390,7 +390,7 @@ pub fn unused_dependencies(
     pkg: &Package,
     path: &Path,
     pkg_lints: &TomlToolLints,
-    ws_lints: &TomlToolLints,
+    ws_lints: Option<&TomlToolLints>,
     error_count: &mut usize,
     gctx: &GlobalContext,
 ) -> CargoResult<()> {

src/cargo/util/network/sleep.rs

@@ -90,13 +90,15 @@ impl<T> SleepTracker<T> {
 #[test]
 fn returns_in_order() {
     let mut s = SleepTracker::new();
-    s.push(3, 3);
+    s.push(30_000, 30_000);
     s.push(1, 1);
-    s.push(6, 6);
-    s.push(5, 5);
-    s.push(2, 2);
-    s.push(10000, 10000);
-    assert_eq!(s.len(), 6);
-    std::thread::sleep(Duration::from_millis(100));
-    assert_eq!(s.to_retry(), &[1, 2, 3, 5, 6]);
+    assert_eq!(s.len(), 2);
+    std::thread::sleep(Duration::from_millis(2));
+    assert_eq!(s.to_retry(), &[1]);
+    assert!(s.to_retry().is_empty());
+    let next = s.time_to_next().expect("should be next");
+    assert!(
+        next < Duration::from_millis(30_000),
+        "{next:?} should be less than 30s"
+    );
 }

tests/testsuite/cargo_add/detect_workspace_inherit_fuzzy/in/Cargo.toml

@@ -0,0 +1,5 @@
+[workspace]
+members = ["primary"]
+
+[workspace.dependencies]
+fuzzy_dependency = "1.0.0"

tests/testsuite/cargo_add/detect_workspace_inherit_fuzzy/in/primary/Cargo.toml

@@ -0,0 +1,4 @@
+[package]
+name = "bar"
+version = "0.0.0"
+edition = "2015"

tests/testsuite/cargo_add/detect_workspace_inherit_fuzzy/mod.rs

@@ -0,0 +1,29 @@
+use cargo_test_support::compare::assert_ui;
+use cargo_test_support::current_dir;
+use cargo_test_support::file;
+use cargo_test_support::prelude::*;
+use cargo_test_support::registry::Package;
+use cargo_test_support::str;
+use cargo_test_support::Project;
+
+#[cargo_test]
+fn case() {
+    cargo_test_support::registry::init();
+
+    Package::new("fuzzy_dependency", "1.0.0").publish();
+
+    let project = Project::from_template(current_dir!().join("in"));
+    let project_root = project.root();
+    let cwd = &project_root;
+
+    snapbox::cmd::Command::cargo_ui()
+        .arg("add")
+        .args(["fuzzy-dependency", "-p", "bar"])
+        .current_dir(cwd)
+        .assert()
+        .success()
+        .stdout_matches(str![""])
+        .stderr_matches(file!["stderr.term.svg"]);
+
+    assert_ui().subset_matches(current_dir!().join("out"), &project_root);
+}

tests/testsuite/cargo_add/detect_workspace_inherit_fuzzy/out/Cargo.toml

@@ -0,0 +1,5 @@
+[workspace]
+members = ["primary"]
+
+[workspace.dependencies]
+fuzzy_dependency = "1.0.0"

tests/testsuite/cargo_add/detect_workspace_inherit_fuzzy/out/primary/Cargo.toml

@@ -0,0 +1,7 @@
+[package]
+name = "bar"
+version = "0.0.0"
+edition = "2015"
+
+[dependencies]
+fuzzy_dependency.workspace = true

tests/testsuite/cargo_add/detect_workspace_inherit_fuzzy/stderr.term.svg

@@ -0,0 +1,34 @@
+<svg width="740px" height="110px" xmlns="http://www.w3.org/2000/svg">
+  <style>
+    .fg { fill: #AAAAAA }
+    .bg { background: #000000 }
+    .fg-green { fill: #00AA00 }
+    .fg-yellow { fill: #AA5500 }
+    .container {
+      padding: 0 10px;
+      line-height: 18px;
+    }
+    .bold { font-weight: bold; }
+    tspan {
+      font: 14px SFMono-Regular, Consolas, Liberation Mono, Menlo, monospace;
+      white-space: pre;
+      line-height: 18px;
+    }
+  </style>
+
+  <rect width="100%" height="100%" y="0" rx="4.5" class="bg" />
+
+  <text xml:space="preserve" class="container fg">
+    <tspan x="10px" y="28px"><tspan class="fg-green bold">    Updating</tspan><tspan> `dummy-registry` index</tspan>
+</tspan>
+    <tspan x="10px" y="46px"><tspan class="fg-yellow bold">warning</tspan><tspan class="bold">:</tspan><tspan> translating `fuzzy-dependency` to `fuzzy_dependency`</tspan>
+</tspan>
+    <tspan x="10px" y="64px"><tspan class="fg-green bold">      Adding</tspan><tspan> fuzzy_dependency (workspace) to dependencies</tspan>
+</tspan>
+    <tspan x="10px" y="82px"><tspan class="fg-green bold">     Locking</tspan><tspan> 2 packages to latest compatible versions</tspan>
+</tspan>
+    <tspan x="10px" y="100px">
+</tspan>
+  </text>
+
+</svg>

tests/testsuite/cargo_add/features_fuzzy/in/Cargo.toml

@@ -0,0 +1,9 @@
+[workspace]
+
+[package]
+name = "cargo-list-test-fixture"
+version = "0.0.0"
+edition = "2015"
+
+[dependencies]
+your_face = { version = "99999.0.0", features = ["eyes"] }

tests/testsuite/cargo_add/features_fuzzy/mod.rs

@@ -0,0 +1,32 @@
+use cargo_test_support::compare::assert_ui;
+use cargo_test_support::current_dir;
+use cargo_test_support::file;
+use cargo_test_support::prelude::*;
+use cargo_test_support::str;
+use cargo_test_support::Project;
+
+#[cargo_test]
+fn case() {
+    cargo_test_support::registry::init();
+    cargo_test_support::registry::Package::new("your_face", "99999.0.0+my-package")
+        .feature("nose", &[])
+        .feature("mouth", &[])
+        .feature("eyes", &[])
+        .feature("ears", &[])
+        .publish();
+
+    let project = Project::from_template(current_dir!().join("in"));
+    let project_root = project.root();
+    let cwd = &project_root;
+
+    snapbox::cmd::Command::cargo_ui()
+        .arg("add")
+        .arg_line("your-face")
+        .current_dir(cwd)
+        .assert()
+        .success()
+        .stdout_matches(str![""])
+        .stderr_matches(file!["stderr.term.svg"]);
+
+    assert_ui().subset_matches(current_dir!().join("out"), &project_root);
+}

tests/testsuite/cargo_add/features_fuzzy/out/Cargo.toml

@@ -0,0 +1,9 @@
+[workspace]
+
+[package]
+name = "cargo-list-test-fixture"
+version = "0.0.0"
+edition = "2015"
+
+[dependencies]
+your_face = { version = "99999.0.0", features = ["eyes"] }

tests/testsuite/cargo_add/features_fuzzy/stderr.term.svg

@@ -0,0 +1,45 @@
+<svg width="740px" height="200px" xmlns="http://www.w3.org/2000/svg">
+  <style>
+    .fg { fill: #AAAAAA }
+    .bg { background: #000000 }
+    .fg-green { fill: #00AA00 }
+    .fg-red { fill: #AA0000 }
+    .fg-yellow { fill: #AA5500 }
+    .container {
+      padding: 0 10px;
+      line-height: 18px;
+    }
+    .bold { font-weight: bold; }
+    tspan {
+      font: 14px SFMono-Regular, Consolas, Liberation Mono, Menlo, monospace;
+      white-space: pre;
+      line-height: 18px;
+    }
+  </style>
+
+  <rect width="100%" height="100%" y="0" rx="4.5" class="bg" />
+
+  <text xml:space="preserve" class="container fg">
+    <tspan x="10px" y="28px"><tspan class="fg-green bold">    Updating</tspan><tspan> `dummy-registry` index</tspan>
+</tspan>
+    <tspan x="10px" y="46px"><tspan class="fg-yellow bold">warning</tspan><tspan class="bold">:</tspan><tspan> translating `your-face` to `your_face`</tspan>
+</tspan>
+    <tspan x="10px" y="64px"><tspan class="fg-green bold">      Adding</tspan><tspan> your_face v99999.0.0 to dependencies</tspan>
+</tspan>
+    <tspan x="10px" y="82px"><tspan>             Features:</tspan>
+</tspan>
+    <tspan x="10px" y="100px"><tspan>             </tspan><tspan class="fg-green bold">+</tspan><tspan> eyes</tspan>
+</tspan>
+    <tspan x="10px" y="118px"><tspan>             </tspan><tspan class="fg-red bold">-</tspan><tspan> ears</tspan>
+</tspan>
+    <tspan x="10px" y="136px"><tspan>             </tspan><tspan class="fg-red bold">-</tspan><tspan> mouth</tspan>
+</tspan>
+    <tspan x="10px" y="154px"><tspan>             </tspan><tspan class="fg-red bold">-</tspan><tspan> nose</tspan>
+</tspan>
+    <tspan x="10px" y="172px"><tspan class="fg-green bold">     Locking</tspan><tspan> 2 packages to latest compatible versions</tspan>
+</tspan>
+    <tspan x="10px" y="190px">
+</tspan>
+  </text>
+
+</svg>

tests/testsuite/cargo_add/mod.rs

@@ -13,6 +13,7 @@ mod deprecated_default_features;
 mod deprecated_section;
 mod detect_workspace_inherit;
 mod detect_workspace_inherit_features;
+mod detect_workspace_inherit_fuzzy;
 mod detect_workspace_inherit_optional;
 mod detect_workspace_inherit_public;
 mod dev;
@@ -24,6 +25,7 @@ mod features;
 mod features_activated_over_limit;
 mod features_deactivated_over_limit;
 mod features_empty;
+mod features_fuzzy;
 mod features_multiple_occurrences;
 mod features_preserve;
 mod features_spaced_values;

tests/testsuite/lints_table.rs

@@ -982,3 +982,43 @@ error: `im_a_teapot` is specified
         )
         .run();
 }
+
+#[cargo_test]
+fn dont_always_inherit_workspace_lints() {
+    let p = project()
+        .file(
+            "Cargo.toml",
+            r#"
+[workspace]
+members = ["foo"]
+
+[workspace.lints.cargo]
+im-a-teapot = "warn"
+"#,
+        )
+        .file(
+            "foo/Cargo.toml",
+            r#"
+cargo-features = ["test-dummy-unstable"]
+
+[package]
+name = "foo"
+version = "0.0.1"
+edition = "2015"
+authors = []
+im-a-teapot = true
+"#,
+        )
+        .file("foo/src/lib.rs", "")
+        .build();
+
+    p.cargo("check -Zcargo-lints")
+        .masquerade_as_nightly_cargo(&["cargo-lints"])
+        .with_stderr(
+            "\
+[CHECKING] foo v0.0.1 ([CWD]/foo)
+[FINISHED] [..]
+",
+        )
+        .run();
+}