feat: Avoid creating implicit features on edition 2024

Author: Muscraft

src/cargo/util/lints.rs

@@ -1,3 +1,4 @@
+use crate::core::dependency::DepKind;
 use crate::core::FeatureValue::Dep;
 use crate::core::{Edition, FeatureValue, Package};
 use crate::util::interning::InternedString;
@@ -254,71 +255,96 @@ pub fn unused_dependencies(
 ) -> CargoResult<()> {
     let edition = pkg.manifest().edition();
     // Unused optional dependencies can only exist on edition 2024+
-    if edition <= Edition::Edition2021 {
+    if edition < Edition::Edition2024 {
         return Ok(());
     }
 
     let lint_level = UNUSED_OPTIONAL_DEPENDENCY.level(lints, edition);
     if lint_level == LintLevel::Allow {
         return Ok(());
     }
-
+    let mut emitted_source = None;
     let manifest = pkg.manifest();
-    let activated_opt_deps = manifest
-        .resolved_toml()
-        .features()
-        .map(|map| {
-            map.values()
-                .flatten()
-                .filter_map(|f| match FeatureValue::new(InternedString::new(f)) {
-                    Dep { dep_name } => Some(dep_name.as_str()),
-                    _ => None,
-                })
-                .collect::<HashSet<_>>()
-        })
-        .unwrap_or_default();
+    let original_toml = manifest.original_toml();
+    // Unused dependencies were stripped from the manifest, leaving only the used ones
+    let used_dependencies = manifest
+        .dependencies()
+        .into_iter()
+        .map(|d| d.name_in_toml().to_string())
+        .collect::<HashSet<String>>();
+    let mut orig_deps = vec![
+        (
+            original_toml.dependencies.as_ref(),
+            vec![DepKind::Normal.kind_table()],
+        ),
+        (
+            original_toml.dev_dependencies.as_ref(),
+            vec![DepKind::Development.kind_table()],
+        ),
+        (
+            original_toml.build_dependencies.as_ref(),
+            vec![DepKind::Build.kind_table()],
+        ),
+    ];
+    for (name, platform) in original_toml.target.iter().flatten() {
+        orig_deps.push((
+            platform.dependencies.as_ref(),
+            vec!["target", name, DepKind::Normal.kind_table()],
+        ));
+        orig_deps.push((
+            platform.dev_dependencies.as_ref(),
+            vec!["target", name, DepKind::Development.kind_table()],
+        ));
+        orig_deps.push((
+            platform.build_dependencies.as_ref(),
+            vec!["target", name, DepKind::Normal.kind_table()],
+        ));
+    }
+    for (deps, toml_path) in orig_deps {
+        if let Some(deps) = deps {
+            for name in deps.keys() {
+                if !used_dependencies.contains(name.as_str()) {
+                    if lint_level == LintLevel::Forbid || lint_level == LintLevel::Deny {
+                        *error_count += 1;
+                    }
+                    let toml_path = toml_path
+                        .iter()
+                        .map(|s| *s)
+                        .chain(std::iter::once(name.as_str()))
+                        .collect::<Vec<_>>();
+                    let level = lint_level.to_diagnostic_level();
+                    let manifest_path = rel_cwd_manifest_path(path, gctx);
 
-    let mut emitted_source = None;
-    for dep in manifest.dependencies() {
-        let dep_name_in_toml = dep.name_in_toml();
-        if !dep.is_optional() || activated_opt_deps.contains(dep_name_in_toml.as_str()) {
-            continue;
-        }
-        if lint_level == LintLevel::Forbid || lint_level == LintLevel::Deny {
-            *error_count += 1;
-        }
-        let mut toml_path = vec![dep.kind().kind_table(), dep_name_in_toml.as_str()];
-        let platform = dep.platform().map(|p| p.to_string());
-        if let Some(platform) = platform.as_ref() {
-            toml_path.insert(0, platform);
-            toml_path.insert(0, "target");
-        }
-        let level = lint_level.to_diagnostic_level();
-        let manifest_path = rel_cwd_manifest_path(path, gctx);
-        let mut message = level.title(UNUSED_OPTIONAL_DEPENDENCY.desc).snippet(
-            Snippet::source(manifest.contents())
-                .origin(&manifest_path)
-                .annotation(level.span(get_span(manifest.document(), &toml_path, false).unwrap()))
-                .fold(true),
-        );
-        if emitted_source.is_none() {
-            emitted_source = Some(format!(
-                "`cargo::{}` is set to `{lint_level}`",
-                UNUSED_OPTIONAL_DEPENDENCY.name
-            ));
-            message = message.footer(Level::Note.title(emitted_source.as_ref().unwrap()));
+                    let mut message = level.title(UNUSED_OPTIONAL_DEPENDENCY.desc).snippet(
+                        Snippet::source(manifest.contents())
+                            .origin(&manifest_path)
+                            .annotation(level.span(
+                                get_span(manifest.document(), toml_path.as_slice(), false).unwrap(),
+                            ))
+                            .fold(true),
+                    );
+                    if emitted_source.is_none() {
+                        emitted_source = Some(format!(
+                            "`cargo::{}` is set to `{lint_level}`",
+                            UNUSED_OPTIONAL_DEPENDENCY.name
+                        ));
+                        message =
+                            message.footer(Level::Note.title(emitted_source.as_ref().unwrap()));
+                    }
+                    let help = format!(
+                        "remove the dependency or activate it in a feature with `dep:{name}`"
+                    );
+                    message = message.footer(Level::Help.title(&help));
+                    let renderer = Renderer::styled().term_width(
+                        gctx.shell()
+                            .err_width()
+                            .diagnostic_terminal_width()
+                            .unwrap_or(annotate_snippets::renderer::DEFAULT_TERM_WIDTH),
+                    );
+                    writeln!(gctx.shell().err(), "{}", renderer.render(message))?;
+                }
+            }
         }
-        let help = format!(
-            "remove the dependency or activate it in a feature with `dep:{dep_name_in_toml}`"
-        );
-        message = message.footer(Level::Help.title(&help));
-        let renderer = Renderer::styled().term_width(
-            gctx.shell()
-                .err_width()
-                .diagnostic_terminal_width()
-                .unwrap_or(annotate_snippets::renderer::DEFAULT_TERM_WIDTH),
-        );
-        writeln!(gctx.shell().err(), "{}", renderer.render(message))?;
     }
     Ok(())
 }

src/cargo/util/toml/mod.rs

@@ -1,5 +1,5 @@
 use annotate_snippets::{Level, Renderer, Snippet};
-use std::collections::{BTreeMap, BTreeSet, HashMap};
+use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet};
 use std::ffi::OsStr;
 use std::path::{Path, PathBuf};
 use std::rc::Rc;
@@ -20,6 +20,7 @@ use crate::core::compiler::{CompileKind, CompileTarget};
 use crate::core::dependency::{Artifact, ArtifactTarget, DepKind};
 use crate::core::manifest::{ManifestMetadata, TargetSourcePath};
 use crate::core::resolver::ResolveBehavior;
+use crate::core::FeatureValue::Dep;
 use crate::core::{find_workspace_root, resolve_relative_path, CliUnstable, FeatureValue};
 use crate::core::{Dependency, Manifest, Package, PackageId, Summary, Target};
 use crate::core::{Edition, EitherManifest, Feature, Features, VirtualManifest, Workspace};
@@ -300,30 +301,56 @@ fn resolve_toml(
 
     if let Some(original_package) = original_toml.package() {
         let resolved_package = resolve_package_toml(original_package, package_root, &inherit)?;
+        let edition = resolved_package
+            .resolved_edition()
+            .expect("previously resolved")
+            .map_or(Edition::default(), |e| {
+                Edition::from_str(&e).unwrap_or_default()
+            });
         resolved_toml.package = Some(resolved_package);
 
+        let activated_opt_deps = resolved_toml
+            .features
+            .as_ref()
+            .map(|map| {
+                map.values()
+                    .flatten()
+                    .filter_map(|f| match FeatureValue::new(InternedString::new(f)) {
+                        Dep { dep_name } => Some(dep_name.as_str()),
+                        _ => None,
+                    })
+                    .collect::<HashSet<_>>()
+            })
+            .unwrap_or_default();
+
         resolved_toml.dependencies = resolve_dependencies(
             gctx,
+            edition,
             &features,
             original_toml.dependencies.as_ref(),
+            &activated_opt_deps,
             None,
             &inherit,
             package_root,
             warnings,
         )?;
         resolved_toml.dev_dependencies = resolve_dependencies(
             gctx,
+            edition,
             &features,
             original_toml.dev_dependencies(),
+            &activated_opt_deps,
             Some(DepKind::Development),
             &inherit,
             package_root,
             warnings,
         )?;
         resolved_toml.build_dependencies = resolve_dependencies(
             gctx,
+            edition,
             &features,
             original_toml.build_dependencies(),
+            &activated_opt_deps,
             Some(DepKind::Build),
             &inherit,
             package_root,
@@ -333,26 +360,32 @@ fn resolve_toml(
         for (name, platform) in original_toml.target.iter().flatten() {
             let resolved_dependencies = resolve_dependencies(
                 gctx,
+                edition,
                 &features,
                 platform.dependencies.as_ref(),
+                &activated_opt_deps,
                 None,
                 &inherit,
                 package_root,
                 warnings,
             )?;
             let resolved_dev_dependencies = resolve_dependencies(
                 gctx,
+                edition,
                 &features,
                 platform.dev_dependencies(),
+                &activated_opt_deps,
                 Some(DepKind::Development),
                 &inherit,
                 package_root,
                 warnings,
             )?;
             let resolved_build_dependencies = resolve_dependencies(
                 gctx,
+                edition,
                 &features,
                 platform.build_dependencies(),
+                &activated_opt_deps,
                 Some(DepKind::Build),
                 &inherit,
                 package_root,
@@ -561,8 +594,10 @@ fn default_readme_from_package_root(package_root: &Path) -> Option<String> {
 #[tracing::instrument(skip_all)]
 fn resolve_dependencies<'a>(
     gctx: &GlobalContext,
+    edition: Edition,
     features: &Features,
     orig_deps: Option<&BTreeMap<manifest::PackageName, manifest::InheritableDependency>>,
+    activated_opt_deps: &HashSet<&str>,
     kind: Option<DepKind>,
     inherit: &dyn Fn() -> CargoResult<&'a InheritableFields>,
     package_root: &Path,
@@ -610,10 +645,18 @@ fn resolve_dependencies<'a>(
             }
         }
 
-        deps.insert(
-            name_in_toml.clone(),
-            manifest::InheritableDependency::Value(resolved.clone()),
-        );
+        // if the dependency is not optional, it is always used
+        // if the dependency is optional and activated, it is used
+        // if the dependency is optional and not activated, it is not used
+        let is_dep_activated =
+            !resolved.is_optional() || activated_opt_deps.contains(name_in_toml.as_str());
+        // If the edition is less than 2024, we don't need to check for unused optional dependencies
+        if edition < Edition::Edition2024 || is_dep_activated {
+            deps.insert(
+                name_in_toml.clone(),
+                manifest::InheritableDependency::Value(resolved.clone()),
+            );
+        }
     }
     Ok(Some(deps))
 }

tests/testsuite/lints/implicit_features/edition_2024/stderr.term.svg

@@ -1,3 +1,4 @@
+use cargo_test_support::compare::assert_match_exact;
 use cargo_test_support::prelude::*;
 use cargo_test_support::registry::Package;
 use cargo_test_support::str;
@@ -39,4 +40,16 @@ target-dep = { version = "0.1.0", optional = true }
         .success()
         .stdout_matches(str![""])
         .stderr_matches(file!["stderr.term.svg"]);
+
+    let expected_lockfile = r#"# This file is automatically @generated by Cargo.
+# It is not intended for manual editing.
+version = 3
+
+[[package]]
+name = "foo"
+version = "0.1.0"
+"#;
+
+    let lock = p.read_lockfile();
+    assert_match_exact(expected_lockfile, &lock);
 }

tests/testsuite/lints/unused_optional_dependencies/edition_2024/mod.rs

@@ -1,2 +1,3 @@
 mod edition_2021;
 mod edition_2024;
+mod renamed_deps;

tests/testsuite/lints/unused_optional_dependencies/edition_2024/stderr.term.svg

@@ -0,0 +1,42 @@
+use cargo_test_support::prelude::*;
+use cargo_test_support::registry::Package;
+use cargo_test_support::str;
+use cargo_test_support::{file, project};
+
+#[cargo_test(nightly, reason = "edition2024 is not stable")]
+fn case() {
+    Package::new("bar", "0.1.0").publish();
+    Package::new("bar", "0.2.0").publish();
+    Package::new("target-dep", "0.1.0").publish();
+    let p = project()
+        .file(
+            "Cargo.toml",
+            r#"
+cargo-features = ["edition2024"]
+[package]
+name = "foo"
+version = "0.1.0"
+edition = "2024"
+
+[dependencies]
+bar = { version = "0.1.0", optional = true }
+
+[build-dependencies]
+baz = { version = "0.2.0", package = "bar", optional = true }
+
+[target.'cfg(target_os = "linux")'.dependencies]
+target-dep = { version = "0.1.0", optional = true }
+"#,
+        )
+        .file("src/lib.rs", "")
+        .build();
+
+    snapbox::cmd::Command::cargo_ui()
+        .masquerade_as_nightly_cargo(&["edition2024"])
+        .current_dir(p.root())
+        .arg("check")
+        .assert()
+        .success()
+        .stdout_matches(str![""])
+        .stderr_matches(file!["stderr.term.svg"]);
+}