diff --git a/src/cargo/ops/fix.rs b/src/cargo/ops/fix.rs index 24dd8b63500..df354b36e4b 100644 --- a/src/cargo/ops/fix.rs +++ b/src/cargo/ops/fix.rs @@ -285,7 +285,7 @@ fn migrate_manifests(ws: &Workspace<'_>, pkgs: &[&Package]) -> CargoResult<()> { fixes += rename_dep_fields_2024(workspace, "dependencies"); } - fixes += add_feature_for_unused_deps(pkg, root); + fixes += add_feature_for_unused_deps(pkg, root, ws.gctx()); fixes += rename_table(root, "project", "package"); if let Some(target) = root.get_mut("lib").and_then(|t| t.as_table_like_mut()) { fixes += rename_target_fields_2024(target); @@ -435,7 +435,11 @@ fn rename_table(parent: &mut dyn toml_edit::TableLike, old: &str, new: &str) -> 1 } -fn add_feature_for_unused_deps(pkg: &Package, parent: &mut dyn toml_edit::TableLike) -> usize { +fn add_feature_for_unused_deps( + pkg: &Package, + parent: &mut dyn toml_edit::TableLike, + gctx: &GlobalContext, +) -> usize { let manifest = pkg.manifest(); let activated_opt_deps = manifest @@ -456,18 +460,48 @@ fn add_feature_for_unused_deps(pkg: &Package, parent: &mut dyn toml_edit::TableL 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()) { - fixes += 1; if let Some(features) = parent .entry("features") .or_insert(toml_edit::table()) .as_table_like_mut() { - features.insert( - dep_name_in_toml.as_str(), - toml_edit::Item::Value(toml_edit::Value::Array(toml_edit::Array::from_iter( - &[format!("dep:{}", dep_name_in_toml)], - ))), - ); + let activate_dep = format!("dep:{dep_name_in_toml}"); + let strong_dep_feature_prefix = format!("{dep_name_in_toml}/"); + features + .entry(dep_name_in_toml.as_str()) + .or_insert_with(|| { + fixes += 1; + toml_edit::Item::Value(toml_edit::Value::Array( + toml_edit::Array::from_iter([&activate_dep]), + )) + }); + // Ensure `dep:dep_name` is present for `dep_name/feature_name` since `dep:` is the + // only way to guarantee an optional dependency is available for use. + // + // The way we avoid implicitly creating features in Edition2024 is we remove the + // dependency from `resolved_toml` if there is no `dep:` syntax as that is the only + // syntax that suppresses the creation of the implicit feature. + for (feature_name, activations) in features.iter_mut() { + let Some(activations) = activations.as_array_mut() else { + let _ = gctx.shell().warn(format_args!("skipping fix of feature `{feature_name}` in package `{}`: unsupported feature schema", pkg.name())); + continue; + }; + if activations + .iter() + .any(|a| a.as_str().map(|a| a == activate_dep).unwrap_or(false)) + { + continue; + } + let Some(activate_dep_pos) = activations.iter().position(|a| { + a.as_str() + .map(|a| a.starts_with(&strong_dep_feature_prefix)) + .unwrap_or(false) + }) else { + continue; + }; + fixes += 1; + activations.insert(activate_dep_pos, &activate_dep); + } } } } diff --git a/tests/testsuite/fix.rs b/tests/testsuite/fix.rs index 1a34996449b..a86acb80ea8 100644 --- a/tests/testsuite/fix.rs +++ b/tests/testsuite/fix.rs @@ -1297,9 +1297,9 @@ fn fix_to_broken_code() { .with_stderr_contains("[WARNING] failed to automatically apply fixes [..]") .run(); - assert_eq!( + assert_e2e().eq( p.read_file("bar/src/lib.rs"), - "pub fn foo() { let x = 3; let _ = x; }" + str!["pub fn foo() { let x = 3; let _ = x; }"], ); } @@ -1320,7 +1320,10 @@ fn fix_with_common() { p.cargo("fix --edition --allow-no-vcs").run(); - assert_eq!(p.read_file("tests/common/mod.rs"), "pub fn r#try() {}"); + assert_e2e().eq( + p.read_file("tests/common/mod.rs"), + str!["pub fn r#try() {}"], + ); } #[cargo_test] @@ -2312,9 +2315,10 @@ edition = "2021" ", ) .run(); - assert_eq!( + assert_e2e().eq( p.read_file("Cargo.toml"), - r#" + str![[r#" + cargo-features = ["edition2024"] # Before project @@ -2323,7 +2327,8 @@ cargo-features = ["edition2024"] name = "foo" edition = "2021" # After project table -"# + +"#]], ); } @@ -2365,9 +2370,10 @@ edition = "2021" ", ) .run(); - assert_eq!( + assert_e2e().eq( p.read_file("Cargo.toml"), - r#" + str![[r#" + cargo-features = ["edition2024"] # Before package @@ -2376,7 +2382,8 @@ cargo-features = ["edition2024"] name = "foo" edition = "2021" # After project table -"# + +"#]], ); } @@ -2469,9 +2476,10 @@ a = {path = "a", default_features = false} ", ) .run(); - assert_eq!( + assert_e2e().eq( p.read_file("Cargo.toml"), - r#" + str![[r#" + cargo-features = ["edition2024"] [workspace.dependencies] @@ -2519,14 +2527,15 @@ a = {path = "a", default-features = false} # After build_dependencies line a = {path = "a", default-features = false} # After build_dependencies table -"#, + +"#]], ); } #[cargo_test] fn add_feature_for_unused_dep() { - Package::new("bar", "0.1.0").publish(); - Package::new("baz", "0.1.0").publish(); + Package::new("regular-dep", "0.1.0").publish(); + Package::new("build-dep", "0.1.0").publish(); Package::new("target-dep", "0.1.0").publish(); let p = project() .file( @@ -2538,10 +2547,10 @@ version = "0.1.0" edition = "2021" [dependencies] -bar = { version = "0.1.0", optional = true } +regular-dep = { version = "0.1.0", optional = true } [build-dependencies] -baz = { version = "0.1.0", optional = true } +build-dep = { version = "0.1.0", optional = true } [target.'cfg(target_os = "linux")'.dependencies] target-dep = { version = "0.1.0", optional = true } @@ -2564,36 +2573,36 @@ target-dep = { version = "0.1.0", optional = true } ", ) .run(); - assert_eq!( + assert_e2e().eq( p.read_file("Cargo.toml"), - r#" + str![[r#" + [package] name = "foo" version = "0.1.0" edition = "2021" [dependencies] -bar = { version = "0.1.0", optional = true } +regular-dep = { version = "0.1.0", optional = true } [build-dependencies] -baz = { version = "0.1.0", optional = true } +build-dep = { version = "0.1.0", optional = true } [target.'cfg(target_os = "linux")'.dependencies] target-dep = { version = "0.1.0", optional = true } [features] -bar = ["dep:bar"] -baz = ["dep:baz"] +regular-dep = ["dep:regular-dep"] +build-dep = ["dep:build-dep"] target-dep = ["dep:target-dep"] -"# + +"#]], ); } #[cargo_test] fn add_feature_for_unused_dep_existing_table() { - Package::new("bar", "0.1.0").publish(); - Package::new("baz", "0.1.0").publish(); - Package::new("target-dep", "0.1.0").publish(); + Package::new("dep", "0.1.0").publish(); let p = project() .file( "Cargo.toml", @@ -2604,16 +2613,10 @@ version = "0.1.0" edition = "2021" [dependencies] -bar = { version = "0.1.0", optional = true } - -[build-dependencies] -baz = { version = "0.1.0", optional = true } - -[target.'cfg(target_os = "linux")'.dependencies] -target-dep = { version = "0.1.0", optional = true } +dep = { version = "0.1.0", optional = true } [features] -target-dep = ["dep:target-dep"] +existing = [] "#, ) .file("src/lib.rs", "") @@ -2624,37 +2627,117 @@ target-dep = ["dep:target-dep"] .with_stderr( "\ [MIGRATING] Cargo.toml from 2021 edition to 2024 -[FIXED] Cargo.toml (2 fixes) +[FIXED] Cargo.toml (1 fix) [UPDATING] `dummy-registry` index -[LOCKING] 4 packages to latest compatible versions +[LOCKING] 2 packages to latest compatible versions [CHECKING] foo v0.1.0 ([CWD]) [MIGRATING] src/lib.rs from 2021 edition to 2024 [FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [..]s ", ) .run(); - assert_eq!( + assert_e2e().eq( p.read_file("Cargo.toml"), - r#" + str![[r#" + [package] name = "foo" version = "0.1.0" edition = "2021" [dependencies] -bar = { version = "0.1.0", optional = true } +dep = { version = "0.1.0", optional = true } -[build-dependencies] -baz = { version = "0.1.0", optional = true } +[features] +existing = [] +dep = ["dep:dep"] -[target.'cfg(target_os = "linux")'.dependencies] -target-dep = { version = "0.1.0", optional = true } +"#]], + ); +} + +#[cargo_test] +fn activate_dep_for_dep_feature() { + Package::new("dep-feature", "0.1.0") + .feature("a", &[]) + .feature("b", &[]) + .publish(); + Package::new("dep-and-dep-feature", "0.1.0") + .feature("a", &[]) + .feature("b", &[]) + .publish(); + Package::new("renamed-feature", "0.1.0") + .feature("a", &[]) + .feature("b", &[]) + .publish(); + Package::new("unrelated-feature", "0.1.0") + .feature("a", &[]) + .feature("b", &[]) + .publish(); + let p = project() + .file( + "Cargo.toml", + r#" +[package] +name = "foo" +version = "0.1.0" +edition = "2021" + +[dependencies] +dep-feature = { version = "0.1.0", optional = true } +dep-and-dep-feature = { version = "0.1.0", optional = true } +renamed-feature = { version = "0.1.0", optional = true } +unrelated-feature = { version = "0.1.0", optional = true } [features] -target-dep = ["dep:target-dep"] -bar = ["dep:bar"] -baz = ["dep:baz"] -"# +dep-feature = ["dep-feature/a", "dep-feature/b"] +dep-and-dep-feature = ["dep:dep-and-dep-feature", "dep-and-dep-feature/a", "dep-and-dep-feature/b"] +renamed = ["renamed-feature/a", "renamed-feature/b"] +unrelated-feature = [] +unrelated-dep-feature = ["unrelated-feature/a", "unrelated-feature/b"] +"#, + ) + .file("src/lib.rs", "") + .build(); + + p.cargo("fix --edition --allow-no-vcs") + .masquerade_as_nightly_cargo(&["edition2024"]) + .with_stderr( + "\ +[MIGRATING] Cargo.toml from 2021 edition to 2024 +[FIXED] Cargo.toml (4 fixes) +[UPDATING] `dummy-registry` index +[LOCKING] 5 packages to latest compatible versions +[CHECKING] foo v0.1.0 ([CWD]) +[MIGRATING] src/lib.rs from 2021 edition to 2024 +[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [..]s +", + ) + .run(); + assert_e2e().eq( + p.read_file("Cargo.toml"), + str![[r#" + +[package] +name = "foo" +version = "0.1.0" +edition = "2021" + +[dependencies] +dep-feature = { version = "0.1.0", optional = true } +dep-and-dep-feature = { version = "0.1.0", optional = true } +renamed-feature = { version = "0.1.0", optional = true } +unrelated-feature = { version = "0.1.0", optional = true } + +[features] +dep-feature = [ "dep:dep-feature","dep-feature/a", "dep-feature/b"] +dep-and-dep-feature = ["dep:dep-and-dep-feature", "dep-and-dep-feature/a", "dep-and-dep-feature/b"] +renamed = [ "dep:renamed-feature","renamed-feature/a", "renamed-feature/b"] +unrelated-feature = [] +unrelated-dep-feature = [ "dep:unrelated-feature","unrelated-feature/a", "unrelated-feature/b"] +renamed-feature = ["dep:renamed-feature"] + +"#]], ); } @@ -2777,11 +2860,12 @@ dep_df_false = { version = "0.1.0", default-features = false } ) .run(); - assert_eq!(p.read_file("pkg_default/Cargo.toml"), pkg_default); - assert_eq!(p.read_file("pkg_df_true/Cargo.toml"), pkg_df_true); - assert_eq!( + assert_e2e().eq(p.read_file("pkg_default/Cargo.toml"), pkg_default); + assert_e2e().eq(p.read_file("pkg_df_true/Cargo.toml"), pkg_df_true); + assert_e2e().eq( p.read_file("pkg_df_false/Cargo.toml"), - r#" + str![[r#" + [package] name = "pkg_df_false" version = "0.1.0" @@ -2801,6 +2885,7 @@ dep_df_false = { workspace = true, default-features = false } dep_simple = { workspace = true} dep_df_true = { workspace = true} dep_df_false = { workspace = true, default-features = false } -"# + +"#]], ); }