From 738cdeca8bb6cdf78cb3631563f3d2111e453ef1 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 1 May 2024 11:11:11 -0500 Subject: [PATCH 1/7] refactor(toml): Clarify what the ws default features function does --- src/cargo/util/toml/mod.rs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index fc45afe4111..1ba2529cbbc 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -970,7 +970,11 @@ fn inner_dependency_inherit_with<'a>( package_root: &Path, warnings: &mut Vec, ) -> CargoResult { - fn default_features_msg(label: &str, ws_def_feat: Option, warnings: &mut Vec) { + fn deprecated_ws_default_features( + label: &str, + ws_def_feat: Option, + warnings: &mut Vec, + ) { let ws_def_feat = match ws_def_feat { Some(true) => "true", Some(false) => "false", @@ -1014,12 +1018,12 @@ fn inner_dependency_inherit_with<'a>( // workspace: default-features = true should ignore member // default-features (Some(false), Some(true)) => { - default_features_msg(name, Some(true), warnings); + deprecated_ws_default_features(name, Some(true), warnings); } // member: default-features = false and // workspace: dep = "1.0" should ignore member default-features (Some(false), None) => { - default_features_msg(name, None, warnings); + deprecated_ws_default_features(name, None, warnings); } _ => {} } From 7f2c7061f11fc18f2c6bc6f7b7ee3fdcd377df37 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 1 May 2024 11:11:34 -0500 Subject: [PATCH 2/7] refactor(toml): Move the ws default features function out --- src/cargo/util/toml/mod.rs | 33 +++++++++++++++++---------------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 1ba2529cbbc..69d82d0c27f 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -970,22 +970,6 @@ fn inner_dependency_inherit_with<'a>( package_root: &Path, warnings: &mut Vec, ) -> CargoResult { - fn deprecated_ws_default_features( - label: &str, - ws_def_feat: Option, - warnings: &mut Vec, - ) { - let ws_def_feat = match ws_def_feat { - Some(true) => "true", - Some(false) => "false", - None => "not specified", - }; - warnings.push(format!( - "`default-features` is ignored for {label}, since `default-features` was \ - {ws_def_feat} for `workspace.dependencies.{label}`, \ - this could become a hard error in the future" - )) - } inherit()?.get_dependency(name, package_root).map(|ws_dep| { let mut merged_dep = match ws_dep { manifest::TomlDependency::Simple(ws_version) => manifest::TomlDetailedDependency { @@ -1044,6 +1028,23 @@ fn inner_dependency_inherit_with<'a>( }) } +fn deprecated_ws_default_features( + label: &str, + ws_def_feat: Option, + warnings: &mut Vec, +) { + let ws_def_feat = match ws_def_feat { + Some(true) => "true", + Some(false) => "false", + None => "not specified", + }; + warnings.push(format!( + "`default-features` is ignored for {label}, since `default-features` was \ + {ws_def_feat} for `workspace.dependencies.{label}`, \ + this could become a hard error in the future" + )) +} + #[tracing::instrument(skip_all)] fn to_real_manifest( contents: String, From 656107a1735aab69f4d8832c5231f39a965bf9b8 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 1 May 2024 11:28:38 -0500 Subject: [PATCH 3/7] refactor(toml): Allow adding errors to ws dep inheritance --- src/cargo/util/toml/mod.rs | 107 ++++++++++++++++++------------------- 1 file changed, 53 insertions(+), 54 deletions(-) diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 69d82d0c27f..6de90bcb51f 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -970,62 +970,61 @@ fn inner_dependency_inherit_with<'a>( package_root: &Path, warnings: &mut Vec, ) -> CargoResult { - inherit()?.get_dependency(name, package_root).map(|ws_dep| { - let mut merged_dep = match ws_dep { - manifest::TomlDependency::Simple(ws_version) => manifest::TomlDetailedDependency { - version: Some(ws_version), - ..Default::default() - }, - manifest::TomlDependency::Detailed(ws_dep) => ws_dep.clone(), - }; - let manifest::TomlInheritedDependency { - workspace: _, + let ws_dep = inherit()?.get_dependency(name, package_root)?; + let mut merged_dep = match ws_dep { + manifest::TomlDependency::Simple(ws_version) => manifest::TomlDetailedDependency { + version: Some(ws_version), + ..Default::default() + }, + manifest::TomlDependency::Detailed(ws_dep) => ws_dep.clone(), + }; + let manifest::TomlInheritedDependency { + workspace: _, - features, - optional, - default_features, - default_features2, - public, - - _unused_keys: _, - } = &pkg_dep; - let default_features = default_features.or(*default_features2); - - match (default_features, merged_dep.default_features()) { - // member: default-features = true and - // workspace: default-features = false should turn on - // default-features - (Some(true), Some(false)) => { - merged_dep.default_features = Some(true); - } - // member: default-features = false and - // workspace: default-features = true should ignore member - // default-features - (Some(false), Some(true)) => { - deprecated_ws_default_features(name, Some(true), warnings); - } - // member: default-features = false and - // workspace: dep = "1.0" should ignore member default-features - (Some(false), None) => { - deprecated_ws_default_features(name, None, warnings); - } - _ => {} + features, + optional, + default_features, + default_features2, + public, + + _unused_keys: _, + } = &pkg_dep; + let default_features = default_features.or(*default_features2); + + match (default_features, merged_dep.default_features()) { + // member: default-features = true and + // workspace: default-features = false should turn on + // default-features + (Some(true), Some(false)) => { + merged_dep.default_features = Some(true); } - merged_dep.features = match (merged_dep.features.clone(), features.clone()) { - (Some(dep_feat), Some(inherit_feat)) => Some( - dep_feat - .into_iter() - .chain(inherit_feat) - .collect::>(), - ), - (Some(dep_fet), None) => Some(dep_fet), - (None, Some(inherit_feat)) => Some(inherit_feat), - (None, None) => None, - }; - merged_dep.optional = *optional; - merged_dep.public = *public; - manifest::TomlDependency::Detailed(merged_dep) - }) + // member: default-features = false and + // workspace: default-features = true should ignore member + // default-features + (Some(false), Some(true)) => { + deprecated_ws_default_features(name, Some(true), warnings); + } + // member: default-features = false and + // workspace: dep = "1.0" should ignore member default-features + (Some(false), None) => { + deprecated_ws_default_features(name, None, warnings); + } + _ => {} + } + merged_dep.features = match (merged_dep.features.clone(), features.clone()) { + (Some(dep_feat), Some(inherit_feat)) => Some( + dep_feat + .into_iter() + .chain(inherit_feat) + .collect::>(), + ), + (Some(dep_fet), None) => Some(dep_fet), + (None, Some(inherit_feat)) => Some(inherit_feat), + (None, None) => None, + }; + merged_dep.optional = *optional; + merged_dep.public = *public; + Ok(manifest::TomlDependency::Detailed(merged_dep)) } fn deprecated_ws_default_features( From 2e686d46f37d030a6c8a44e3ae76670fe92b91d7 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 1 May 2024 11:26:24 -0500 Subject: [PATCH 4/7] test(toml): Show default features inheritance on 2024 Edition --- .../testsuite/inheritable_workspace_fields.rs | 106 ++++++++++++++++++ 1 file changed, 106 insertions(+) diff --git a/tests/testsuite/inheritable_workspace_fields.rs b/tests/testsuite/inheritable_workspace_fields.rs index a91b7abb6f9..e701451f062 100644 --- a/tests/testsuite/inheritable_workspace_fields.rs +++ b/tests/testsuite/inheritable_workspace_fields.rs @@ -1518,6 +1518,59 @@ true for `workspace.dependencies.dep`, this could become a hard error in the fut .run(); } +#[cargo_test(nightly, reason = "edition2024 is not stable")] +fn warn_inherit_def_feat_true_member_def_feat_false_2024_edition() { + Package::new("dep", "0.1.0") + .feature("default", &["fancy_dep"]) + .add_dep(Dependency::new("fancy_dep", "0.2").optional(true)) + .file("src/lib.rs", "") + .publish(); + + Package::new("fancy_dep", "0.2.4").publish(); + + let p = project() + .file( + "Cargo.toml", + r#" + cargo-features = ["edition2024"] + + [package] + name = "bar" + version = "0.2.0" + edition = "2024" + authors = [] + [dependencies] + dep = { workspace = true, default-features = false } + + [workspace] + members = [] + [workspace.dependencies] + dep = { version = "0.1.0", default-features = true } + "#, + ) + .file("src/main.rs", "fn main() {}") + .build(); + + p.cargo("check") + .masquerade_as_nightly_cargo(&["edition2024"]) + .with_stderr( + "\ +[WARNING] [CWD]/Cargo.toml: `default-features` is ignored for dep, since `default-features` was \ +true for `workspace.dependencies.dep`, this could become a hard error in the future +[UPDATING] `dummy-registry` index +[LOCKING] 3 packages to latest Rust [..] compatible versions +[DOWNLOADING] crates ... +[DOWNLOADED] fancy_dep v0.2.4 ([..]) +[DOWNLOADED] dep v0.1.0 ([..]) +[CHECKING] fancy_dep v0.2.4 +[CHECKING] dep v0.1.0 +[CHECKING] bar v0.2.0 ([CWD]) +[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [..] +", + ) + .run(); +} + #[cargo_test] fn warn_inherit_simple_member_def_feat_false() { Package::new("dep", "0.1.0") @@ -1568,6 +1621,59 @@ not specified for `workspace.dependencies.dep`, this could become a hard error i .run(); } +#[cargo_test(nightly, reason = "edition2024 is not stable")] +fn warn_inherit_simple_member_def_feat_false_2024_edition() { + Package::new("dep", "0.1.0") + .feature("default", &["fancy_dep"]) + .add_dep(Dependency::new("fancy_dep", "0.2").optional(true)) + .file("src/lib.rs", "") + .publish(); + + Package::new("fancy_dep", "0.2.4").publish(); + + let p = project() + .file( + "Cargo.toml", + r#" + cargo-features = ["edition2024"] + + [package] + name = "bar" + version = "0.2.0" + edition = "2024" + authors = [] + [dependencies] + dep = { workspace = true, default-features = false } + + [workspace] + members = [] + [workspace.dependencies] + dep = "0.1.0" + "#, + ) + .file("src/main.rs", "fn main() {}") + .build(); + + p.cargo("check") + .masquerade_as_nightly_cargo(&["edition2024"]) + .with_stderr( + "\ +[WARNING] [CWD]/Cargo.toml: `default-features` is ignored for dep, since `default-features` was \ +not specified for `workspace.dependencies.dep`, this could become a hard error in the future +[UPDATING] `dummy-registry` index +[LOCKING] 3 packages to latest compatible versions +[DOWNLOADING] crates ... +[DOWNLOADED] fancy_dep v0.2.4 ([..]) +[DOWNLOADED] dep v0.1.0 ([..]) +[CHECKING] fancy_dep v0.2.4 +[CHECKING] dep v0.1.0 +[CHECKING] bar v0.2.0 ([CWD]) +[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [..] +", + ) + .run(); +} + #[cargo_test] fn inherit_def_feat_false_member_def_feat_true() { Package::new("dep", "0.1.0") From 5965c151790471ae7829f6982029df910af1e06a Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 1 May 2024 11:35:31 -0500 Subject: [PATCH 5/7] fix(toml): Disallow ignored default-features when inheriting --- src/cargo/util/toml/mod.rs | 32 +++++++++++----- .../testsuite/inheritable_workspace_fields.rs | 38 ++++++++----------- 2 files changed, 39 insertions(+), 31 deletions(-) diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 6de90bcb51f..a03e8ba5254 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -692,8 +692,14 @@ fn resolve_dependencies<'a>( let mut deps = BTreeMap::new(); for (name_in_toml, v) in dependencies.iter() { - let mut resolved = - dependency_inherit_with(v.clone(), name_in_toml, inherit, package_root, warnings)?; + let mut resolved = dependency_inherit_with( + v.clone(), + name_in_toml, + inherit, + package_root, + edition, + warnings, + )?; if let manifest::TomlDependency::Detailed(ref mut d) = resolved { deprecated_underscore( &d.default_features2, @@ -949,12 +955,13 @@ fn dependency_inherit_with<'a>( name: &str, inherit: &dyn Fn() -> CargoResult<&'a InheritableFields>, package_root: &Path, + edition: Edition, warnings: &mut Vec, ) -> CargoResult { match dependency { manifest::InheritableDependency::Value(value) => Ok(value), manifest::InheritableDependency::Inherit(w) => { - inner_dependency_inherit_with(w, name, inherit, package_root, warnings).with_context(|| { + inner_dependency_inherit_with(w, name, inherit, package_root, edition, warnings).with_context(|| { format!( "error inheriting `{name}` from workspace root manifest's `workspace.dependencies.{name}`", ) @@ -968,6 +975,7 @@ fn inner_dependency_inherit_with<'a>( name: &str, inherit: &dyn Fn() -> CargoResult<&'a InheritableFields>, package_root: &Path, + edition: Edition, warnings: &mut Vec, ) -> CargoResult { let ws_dep = inherit()?.get_dependency(name, package_root)?; @@ -1002,12 +1010,12 @@ fn inner_dependency_inherit_with<'a>( // workspace: default-features = true should ignore member // default-features (Some(false), Some(true)) => { - deprecated_ws_default_features(name, Some(true), warnings); + deprecated_ws_default_features(name, Some(true), edition, warnings)?; } // member: default-features = false and // workspace: dep = "1.0" should ignore member default-features (Some(false), None) => { - deprecated_ws_default_features(name, None, warnings); + deprecated_ws_default_features(name, None, edition, warnings)?; } _ => {} } @@ -1030,18 +1038,24 @@ fn inner_dependency_inherit_with<'a>( fn deprecated_ws_default_features( label: &str, ws_def_feat: Option, + edition: Edition, warnings: &mut Vec, -) { +) -> CargoResult<()> { let ws_def_feat = match ws_def_feat { Some(true) => "true", Some(false) => "false", None => "not specified", }; - warnings.push(format!( - "`default-features` is ignored for {label}, since `default-features` was \ + if Edition::Edition2024 <= edition { + anyhow::bail!("`default-features = false` cannot override workspace's `default-features`"); + } else { + warnings.push(format!( + "`default-features` is ignored for {label}, since `default-features` was \ {ws_def_feat} for `workspace.dependencies.{label}`, \ this could become a hard error in the future" - )) + )); + } + Ok(()) } #[tracing::instrument(skip_all)] diff --git a/tests/testsuite/inheritable_workspace_fields.rs b/tests/testsuite/inheritable_workspace_fields.rs index e701451f062..9b6c3fbcf8e 100644 --- a/tests/testsuite/inheritable_workspace_fields.rs +++ b/tests/testsuite/inheritable_workspace_fields.rs @@ -1553,19 +1553,16 @@ fn warn_inherit_def_feat_true_member_def_feat_false_2024_edition() { p.cargo("check") .masquerade_as_nightly_cargo(&["edition2024"]) + .with_status(101) .with_stderr( "\ -[WARNING] [CWD]/Cargo.toml: `default-features` is ignored for dep, since `default-features` was \ -true for `workspace.dependencies.dep`, this could become a hard error in the future -[UPDATING] `dummy-registry` index -[LOCKING] 3 packages to latest Rust [..] compatible versions -[DOWNLOADING] crates ... -[DOWNLOADED] fancy_dep v0.2.4 ([..]) -[DOWNLOADED] dep v0.1.0 ([..]) -[CHECKING] fancy_dep v0.2.4 -[CHECKING] dep v0.1.0 -[CHECKING] bar v0.2.0 ([CWD]) -[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [..] +[ERROR] failed to parse manifest at `[CWD]/Cargo.toml` + +Caused by: + error inheriting `dep` from workspace root manifest's `workspace.dependencies.dep` + +Caused by: + `default-features = false` cannot override workspace's `default-features` ", ) .run(); @@ -1656,19 +1653,16 @@ fn warn_inherit_simple_member_def_feat_false_2024_edition() { p.cargo("check") .masquerade_as_nightly_cargo(&["edition2024"]) + .with_status(101) .with_stderr( "\ -[WARNING] [CWD]/Cargo.toml: `default-features` is ignored for dep, since `default-features` was \ -not specified for `workspace.dependencies.dep`, this could become a hard error in the future -[UPDATING] `dummy-registry` index -[LOCKING] 3 packages to latest compatible versions -[DOWNLOADING] crates ... -[DOWNLOADED] fancy_dep v0.2.4 ([..]) -[DOWNLOADED] dep v0.1.0 ([..]) -[CHECKING] fancy_dep v0.2.4 -[CHECKING] dep v0.1.0 -[CHECKING] bar v0.2.0 ([CWD]) -[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [..] +[ERROR] failed to parse manifest at `[CWD]/Cargo.toml` + +Caused by: + error inheriting `dep` from workspace root manifest's `workspace.dependencies.dep` + +Caused by: + `default-features = false` cannot override workspace's `default-features` ", ) .run(); From 10012ce9cfb382c8658e9dca43eb14cbdde7398e Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 1 May 2024 12:30:57 -0500 Subject: [PATCH 6/7] test(fix): Show default-features migration --- tests/testsuite/fix.rs | 152 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 152 insertions(+) diff --git a/tests/testsuite/fix.rs b/tests/testsuite/fix.rs index 019ce2ee176..675ccae9d1b 100644 --- a/tests/testsuite/fix.rs +++ b/tests/testsuite/fix.rs @@ -2654,3 +2654,155 @@ baz = ["dep:baz"] "# ); } + +#[cargo_test] +fn remove_ignored_default_features() { + Package::new("dep_simple", "0.1.0").publish(); + Package::new("dep_df_true", "0.1.0").publish(); + Package::new("dep_df_false", "0.1.0").publish(); + + let pkg_default = r#" +[package] +name = "pkg_default" +version = "0.1.0" +edition = "2021" + +[dependencies] +dep_simple = { workspace = true } +dep_df_true = { workspace = true } +dep_df_false = { workspace = true } + +[build-dependencies] +dep_simple = { workspace = true } +dep_df_true = { workspace = true } +dep_df_false = { workspace = true } + +[target.'cfg(target_os = "linux")'.dependencies] +dep_simple = { workspace = true } +dep_df_true = { workspace = true } +dep_df_false = { workspace = true } +"#; + let pkg_df_true = r#" +[package] +name = "pkg_df_true" +version = "0.1.0" +edition = "2021" + +[dependencies] +dep_simple = { workspace = true, default-features = true } +dep_df_true = { workspace = true, default-features = true } +dep_df_false = { workspace = true, default-features = true } + +[build-dependencies] +dep_simple = { workspace = true, default-features = true } +dep_df_true = { workspace = true, default-features = true } +dep_df_false = { workspace = true, default-features = true } + +[target.'cfg(target_os = "linux")'.dependencies] +dep_simple = { workspace = true, default-features = true } +dep_df_true = { workspace = true, default-features = true } +dep_df_false = { workspace = true, default-features = true } +"#; + let pkg_df_false = r#" +[package] +name = "pkg_df_false" +version = "0.1.0" +edition = "2021" + +[dependencies] +dep_simple = { workspace = true, default-features = false } +dep_df_true = { workspace = true, default-features = false } +dep_df_false = { workspace = true, default-features = false } + +[build-dependencies] +dep_simple = { workspace = true, default-features = false } +dep_df_true = { workspace = true, default-features = false } +dep_df_false = { workspace = true, default-features = false } + +[target.'cfg(target_os = "linux")'.dependencies] +dep_simple = { workspace = true, default-features = false } +dep_df_true = { workspace = true, default-features = false } +dep_df_false = { workspace = true, default-features = false } +"#; + let p = project() + .file( + "Cargo.toml", + r#" +[workspace] +members = ["pkg_default", "pkg_df_true", "pkg_df_false"] +resolver = "2" + +[workspace.dependencies] +dep_simple = "0.1.0" +dep_df_true = { version = "0.1.0", default-features = true } +dep_df_false = { version = "0.1.0", default-features = false } +"#, + ) + .file("pkg_default/Cargo.toml", pkg_default) + .file("pkg_default/src/lib.rs", "") + .file("pkg_df_true/Cargo.toml", pkg_df_true) + .file("pkg_df_true/src/lib.rs", "") + .file("pkg_df_false/Cargo.toml", pkg_df_false) + .file("pkg_df_false/src/lib.rs", "") + .build(); + + p.cargo("fix --all --edition --allow-no-vcs") + .masquerade_as_nightly_cargo(&["edition2024"]) + .with_stderr_unordered( + "\ +[MIGRATING] pkg_default/Cargo.toml from 2021 edition to 2024 +[MIGRATING] pkg_df_true/Cargo.toml from 2021 edition to 2024 +[MIGRATING] pkg_df_false/Cargo.toml from 2021 edition to 2024 +[WARNING] [CWD]/pkg_df_false/Cargo.toml: `default-features` is ignored for dep_df_true, since `default-features` was true for `workspace.dependencies.dep_df_true`, this could become a hard error in the future +[WARNING] [CWD]/pkg_df_false/Cargo.toml: `default-features` is ignored for dep_simple, since `default-features` was not specified for `workspace.dependencies.dep_simple`, this could become a hard error in the future +[WARNING] [CWD]/pkg_df_false/Cargo.toml: `default-features` is ignored for dep_df_true, since `default-features` was true for `workspace.dependencies.dep_df_true`, this could become a hard error in the future +[WARNING] [CWD]/pkg_df_false/Cargo.toml: `default-features` is ignored for dep_simple, since `default-features` was not specified for `workspace.dependencies.dep_simple`, this could become a hard error in the future +[WARNING] [CWD]/pkg_df_false/Cargo.toml: `default-features` is ignored for dep_df_true, since `default-features` was true for `workspace.dependencies.dep_df_true`, this could become a hard error in the future +[WARNING] [CWD]/pkg_df_false/Cargo.toml: `default-features` is ignored for dep_simple, since `default-features` was not specified for `workspace.dependencies.dep_simple`, this could become a hard error in the future +[UPDATING] `dummy-registry` index +[LOCKING] 6 packages to latest compatible versions +[DOWNLOADING] crates ... +[DOWNLOADED] dep_simple v0.1.0 (registry `dummy-registry`) +[DOWNLOADED] dep_df_true v0.1.0 (registry `dummy-registry`) +[DOWNLOADED] dep_df_false v0.1.0 (registry `dummy-registry`) +[CHECKING] dep_df_true v0.1.0 +[CHECKING] dep_df_false v0.1.0 +[CHECKING] dep_simple v0.1.0 +[CHECKING] pkg_df_true v0.1.0 ([CWD]/pkg_df_true) +[CHECKING] pkg_df_false v0.1.0 ([CWD]/pkg_df_false) +[CHECKING] pkg_default v0.1.0 ([CWD]/pkg_default) +[MIGRATING] pkg_df_false/src/lib.rs from 2021 edition to 2024 +[MIGRATING] pkg_df_true/src/lib.rs from 2021 edition to 2024 +[MIGRATING] pkg_default/src/lib.rs from 2021 edition to 2024 +[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [..]s +", + ) + .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!( + p.read_file("pkg_df_false/Cargo.toml"), + r#" +[package] +name = "pkg_df_false" +version = "0.1.0" +edition = "2021" + +[dependencies] +dep_simple = { workspace = true, default-features = false } +dep_df_true = { workspace = true, default-features = false } +dep_df_false = { workspace = true, default-features = false } + +[build-dependencies] +dep_simple = { workspace = true, default-features = false } +dep_df_true = { workspace = true, default-features = false } +dep_df_false = { workspace = true, default-features = false } + +[target.'cfg(target_os = "linux")'.dependencies] +dep_simple = { workspace = true, default-features = false } +dep_df_true = { workspace = true, default-features = false } +dep_df_false = { workspace = true, default-features = false } +"# + ); +} From 627b1d14709a21291b6ccf5149d00d0de82af88d Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 1 May 2024 12:43:47 -0500 Subject: [PATCH 7/7] feat(fix): Migrate inherited deps with ignored default-features --- crates/cargo-util-schemas/src/manifest/mod.rs | 7 +++ src/cargo/ops/fix.rs | 63 +++++++++++++++++++ tests/testsuite/fix.rs | 19 +++--- 3 files changed, 77 insertions(+), 12 deletions(-) diff --git a/crates/cargo-util-schemas/src/manifest/mod.rs b/crates/cargo-util-schemas/src/manifest/mod.rs index 80f474a6c65..375d0de451a 100644 --- a/crates/cargo-util-schemas/src/manifest/mod.rs +++ b/crates/cargo-util-schemas/src/manifest/mod.rs @@ -683,6 +683,13 @@ impl TomlDependency { } } + pub fn default_features(&self) -> Option { + match self { + TomlDependency::Detailed(d) => d.default_features(), + TomlDependency::Simple(..) => None, + } + } + pub fn unused_keys(&self) -> Vec { match self { TomlDependency::Simple(_) => vec![], diff --git a/src/cargo/ops/fix.rs b/src/cargo/ops/fix.rs index a42614968ca..67738ea6fb7 100644 --- a/src/cargo/ops/fix.rs +++ b/src/cargo/ops/fix.rs @@ -45,6 +45,7 @@ use std::{env, fs, str}; use anyhow::{bail, Context as _}; use cargo_util::{exit_status_to_string, is_simple_exit_code, paths, ProcessBuilder}; +use cargo_util_schemas::manifest::TomlManifest; use rustfix::diagnostics::Diagnostic; use rustfix::CodeFix; use semver::Version; @@ -265,6 +266,10 @@ fn migrate_manifests(ws: &Workspace<'_>, pkgs: &[&Package]) -> CargoResult<()> { format!("{file} from {existing_edition} edition to {prepare_for_edition}"), )?; + let ws_original_toml = match ws.root_maybe() { + MaybePackage::Package(package) => package.manifest().original_toml(), + MaybePackage::Virtual(manifest) => manifest.original_toml(), + }; if Edition::Edition2024 <= prepare_for_edition { let mut document = pkg.manifest().document().clone().into_mut(); let mut fixes = 0; @@ -290,10 +295,15 @@ fn migrate_manifests(ws: &Workspace<'_>, pkgs: &[&Package]) -> CargoResult<()> { fixes += rename_array_of_target_fields_2024(root, "test"); fixes += rename_array_of_target_fields_2024(root, "bench"); fixes += rename_dep_fields_2024(root, "dependencies"); + fixes += remove_ignored_default_features_2024(root, "dependencies", ws_original_toml); fixes += rename_table(root, "dev_dependencies", "dev-dependencies"); fixes += rename_dep_fields_2024(root, "dev-dependencies"); + fixes += + remove_ignored_default_features_2024(root, "dev-dependencies", ws_original_toml); fixes += rename_table(root, "build_dependencies", "build-dependencies"); fixes += rename_dep_fields_2024(root, "build-dependencies"); + fixes += + remove_ignored_default_features_2024(root, "build-dependencies", ws_original_toml); for target in root .get_mut("target") .and_then(|t| t.as_table_like_mut()) @@ -302,10 +312,22 @@ fn migrate_manifests(ws: &Workspace<'_>, pkgs: &[&Package]) -> CargoResult<()> { .filter_map(|(_k, t)| t.as_table_like_mut()) { fixes += rename_dep_fields_2024(target, "dependencies"); + fixes += + remove_ignored_default_features_2024(target, "dependencies", ws_original_toml); fixes += rename_table(target, "dev_dependencies", "dev-dependencies"); fixes += rename_dep_fields_2024(target, "dev-dependencies"); + fixes += remove_ignored_default_features_2024( + target, + "dev-dependencies", + ws_original_toml, + ); fixes += rename_table(target, "build_dependencies", "build-dependencies"); fixes += rename_dep_fields_2024(target, "build-dependencies"); + fixes += remove_ignored_default_features_2024( + target, + "build-dependencies", + ws_original_toml, + ); } if 0 < fixes { @@ -337,6 +359,47 @@ fn rename_dep_fields_2024(parent: &mut dyn toml_edit::TableLike, dep_kind: &str) fixes } +fn remove_ignored_default_features_2024( + parent: &mut dyn toml_edit::TableLike, + dep_kind: &str, + ws_original_toml: &TomlManifest, +) -> usize { + let mut fixes = 0; + for (name_in_toml, target) in parent + .get_mut(dep_kind) + .and_then(|t| t.as_table_like_mut()) + .iter_mut() + .flat_map(|t| t.iter_mut()) + .filter_map(|(k, t)| t.as_table_like_mut().map(|t| (k, t))) + { + let name_in_toml: &str = &name_in_toml; + let ws_deps = ws_original_toml + .workspace + .as_ref() + .and_then(|ws| ws.dependencies.as_ref()); + if let Some(ws_dep) = ws_deps.and_then(|ws_deps| ws_deps.get(name_in_toml)) { + if ws_dep.default_features() == Some(false) { + continue; + } + } + if target + .get("workspace") + .and_then(|i| i.as_value()) + .and_then(|i| i.as_bool()) + == Some(true) + && target + .get("default-features") + .and_then(|i| i.as_value()) + .and_then(|i| i.as_bool()) + == Some(false) + { + target.remove("default-features"); + fixes += 1; + } + } + fixes +} + fn rename_array_of_target_fields_2024(root: &mut dyn toml_edit::TableLike, kind: &str) -> usize { let mut fixes = 0; for target in root diff --git a/tests/testsuite/fix.rs b/tests/testsuite/fix.rs index 675ccae9d1b..e76a2d1a3f0 100644 --- a/tests/testsuite/fix.rs +++ b/tests/testsuite/fix.rs @@ -2753,12 +2753,7 @@ dep_df_false = { version = "0.1.0", default-features = false } [MIGRATING] pkg_default/Cargo.toml from 2021 edition to 2024 [MIGRATING] pkg_df_true/Cargo.toml from 2021 edition to 2024 [MIGRATING] pkg_df_false/Cargo.toml from 2021 edition to 2024 -[WARNING] [CWD]/pkg_df_false/Cargo.toml: `default-features` is ignored for dep_df_true, since `default-features` was true for `workspace.dependencies.dep_df_true`, this could become a hard error in the future -[WARNING] [CWD]/pkg_df_false/Cargo.toml: `default-features` is ignored for dep_simple, since `default-features` was not specified for `workspace.dependencies.dep_simple`, this could become a hard error in the future -[WARNING] [CWD]/pkg_df_false/Cargo.toml: `default-features` is ignored for dep_df_true, since `default-features` was true for `workspace.dependencies.dep_df_true`, this could become a hard error in the future -[WARNING] [CWD]/pkg_df_false/Cargo.toml: `default-features` is ignored for dep_simple, since `default-features` was not specified for `workspace.dependencies.dep_simple`, this could become a hard error in the future -[WARNING] [CWD]/pkg_df_false/Cargo.toml: `default-features` is ignored for dep_df_true, since `default-features` was true for `workspace.dependencies.dep_df_true`, this could become a hard error in the future -[WARNING] [CWD]/pkg_df_false/Cargo.toml: `default-features` is ignored for dep_simple, since `default-features` was not specified for `workspace.dependencies.dep_simple`, this could become a hard error in the future +[FIXED] pkg_df_false/Cargo.toml (6 fixes) [UPDATING] `dummy-registry` index [LOCKING] 6 packages to latest compatible versions [DOWNLOADING] crates ... @@ -2790,18 +2785,18 @@ version = "0.1.0" edition = "2021" [dependencies] -dep_simple = { workspace = true, default-features = false } -dep_df_true = { workspace = true, default-features = false } +dep_simple = { workspace = true} +dep_df_true = { workspace = true} dep_df_false = { workspace = true, default-features = false } [build-dependencies] -dep_simple = { workspace = true, default-features = false } -dep_df_true = { workspace = true, default-features = false } +dep_simple = { workspace = true} +dep_df_true = { workspace = true} dep_df_false = { workspace = true, default-features = false } [target.'cfg(target_os = "linux")'.dependencies] -dep_simple = { workspace = true, default-features = false } -dep_df_true = { workspace = true, default-features = false } +dep_simple = { workspace = true} +dep_df_true = { workspace = true} dep_df_false = { workspace = true, default-features = false } "# );