Repository: rust-lang/cargo (rust)

Issue:
2024 edition migration of `default_features` doesn't work for virtual workspace inherited dependencies
### Problem

A virtual workspace with a workspace dependency that uses an underscore for `default_features` doesn't get migrated to 2024. After updating to 2024, this results in an error.

It does print a warning, though. However, I believe it is intended that the auto-fix should apply to this case to (it supports [workspaces](https://github.com/rust-lang/cargo/blob/ef5edffb89b8c3212ae1061619e5c2309cd8da98/src/cargo/ops/fix.rs#L282-L289), just not a virtual one).

### Steps

Root workspace:

```toml
[workspace]
members = ["a"]

[workspace.dependencies]
empty-library = {version="*", default_features=false}
```

and member "a":

```toml
[package]
name = "a"
version = "0.1.0"
edition = "2021"

[dependencies]
empty-library.workspace = true
```

and run `cargo fix --edition`.

### Possible Solution(s)

I think `cargo fix --edition` should also update these dependencies.


### Notes

_No response_

### Version

```text
cargo 1.85.0-nightly (4c39aaff6 2024-11-25)
release: 1.85.0-nightly
commit-hash: 4c39aaff66862cc0da52fe529aa1990bb8bb9a22
commit-date: 2024-11-25
host: aarch64-unknown-linux-gnu
libgit2: 1.8.1 (sys:0.19.0 vendored)
libcurl: 8.9.0-DEV (sys:0.4.74+curl-8.9.0 vendored ssl:OpenSSL/1.1.1w)
ssl: OpenSSL 1.1.1w  11 Sep 2023
os: Debian 12.0.0 [64-bit]
```

Pull request:
fix(fix): Migrate workspace dependencies
### What does this PR try to resolve?

Technically, the edition doesn't affect the workspace. We could adjust this as we're inheriting to not be a problem.

But it feels weird to keep this around in newer editions. We could frame this as around the what the package is, including inheritance. The nice thing is that default-features works for all versions that inheritance works so we're not forcing the users hand with multiple editions in a workspace.

Fixes #14886


### How should we test and review this PR?


### Additional information



Patch:
diff --git a/src/cargo/ops/fix.rs b/src/cargo/ops/fix.rs
index 6e4c718fc..1530e77f4 100644
--- a/src/cargo/ops/fix.rs
+++ b/src/cargo/ops/fix.rs
@@ -252,6 +252,59 @@ fn check_version_control(gctx: &GlobalContext, opts: &FixOptions) -> CargoResult
 }
 
 fn migrate_manifests(ws: &Workspace<'_>, pkgs: &[&Package]) -> CargoResult<()> {
+    // HACK: Duplicate workspace migration logic between virtual manifests and real manifests to
+    // reduce multiple Migrating messages being reported for the same file to the user
+    if matches!(ws.root_maybe(), MaybePackage::Virtual(_)) {
+        // Warning: workspaces do not have an edition so this should only include changes needed by
+        // packages that preserve the behavior of the workspace on all editions
+        let highest_edition = pkgs
+            .iter()
+            .map(|p| p.manifest().edition())
+            .max()
+            .unwrap_or_default();
+        let prepare_for_edition = highest_edition.saturating_next();
+        if highest_edition == prepare_for_edition
+            || (!prepare_for_edition.is_stable() && !ws.gctx().nightly_features_allowed)
+        {
+            //
+        } else {
+            let mut manifest_mut = LocalManifest::try_new(ws.root_manifest())?;
+            let document = &mut manifest_mut.data;
+            let mut fixes = 0;
+
+            if Edition::Edition2024 <= prepare_for_edition {
+                let root = document.as_table_mut();
+
+                if let Some(workspace) = root
+                    .get_mut("workspace")
+                    .and_then(|t| t.as_table_like_mut())
+                {
+                    // strictly speaking, the edition doesn't apply to this table but it should be safe
+                    // enough
+                    fixes += rename_dep_fields_2024(workspace, "dependencies");
+                }
+            }
+
+            if 0 < fixes {
+                // HACK: As workspace migration is a special case, only report it if something
+                // happened
+                let file = ws.root_manifest();
+                let file = file.strip_prefix(ws.root()).unwrap_or(file);
+                let file = file.display();
+                ws.gctx().shell().status(
+                    "Migrating",
+                    format!("{file} from {highest_edition} edition to {prepare_for_edition}"),
+                )?;
+
+                let verb = if fixes == 1 { "fix" } else { "fixes" };
+                let msg = format!("{file} ({fixes} {verb})");
+                ws.gctx().shell().status("Fixed", msg)?;
+
+                manifest_mut.write()?;
+            }
+        }
+    }
+
     for pkg in pkgs {
         let existing_edition = pkg.manifest().edition();
         let prepare_for_edition = existing_edition.saturating_next();
@@ -268,15 +321,15 @@ fn migrate_manifests(ws: &Workspace<'_>, pkgs: &[&Package]) -> CargoResult<()> {
             format!("{file} from {existing_edition} edition to {prepare_for_edition}"),
         )?;
 
+        let mut manifest_mut = LocalManifest::try_new(pkg.manifest_path())?;
+        let document = &mut manifest_mut.data;
+        let mut fixes = 0;
+
         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 manifest_mut = LocalManifest::try_new(pkg.manifest_path())?;
-            let document = &mut manifest_mut.data;
-            let mut fixes = 0;
-
             let root = document.as_table_mut();
 
             if let Some(workspace) = root
@@ -331,14 +384,14 @@ fn migrate_manifests(ws: &Workspace<'_>, pkgs: &[&Package]) -> CargoResult<()> {
                     ws_original_toml,
                 );
             }
+        }
 
-            if 0 < fixes {
-                let verb = if fixes == 1 { "fix" } else { "fixes" };
-                let msg = format!("{file} ({fixes} {verb})");
-                ws.gctx().shell().status("Fixed", msg)?;
+        if 0 < fixes {
+            let verb = if fixes == 1 { "fix" } else { "fixes" };
+            let msg = format!("{file} ({fixes} {verb})");
+            ws.gctx().shell().status("Fixed", msg)?;
 
-                manifest_mut.write()?;
-            }
+            manifest_mut.write()?;
         }
     }
 


Test patch:
diff --git a/tests/testsuite/fix.rs b/tests/testsuite/fix.rs
index 685f66480..f475fe5ba 100644
--- a/tests/testsuite/fix.rs
+++ b/tests/testsuite/fix.rs
@@ -2643,6 +2643,81 @@ a = {path = "a", default-features = false}
     );
 }
 
+#[cargo_test]
+fn migrate_rename_underscore_fields_in_virtual_manifest() {
+    let p = project()
+        .file(
+            "Cargo.toml",
+            r#"
+[workspace]
+members = ["foo"]
+resolver = "2"
+
+[workspace.dependencies]
+# Before default_features
+a = {path = "a", default_features = false}  # After default_features value
+# After default_features line
+"#,
+        )
+        .file(
+            "foo/Cargo.toml",
+            r#"
+[package]
+name = "foo"
+edition = "2021"
+"#,
+        )
+        .file("foo/src/lib.rs", "")
+        .file(
+            "a/Cargo.toml",
+            r#"
+                [package]
+                name = "a"
+                version = "0.0.1"
+                edition = "2015"
+            "#,
+        )
+        .file("a/src/lib.rs", "")
+        .build();
+
+    p.cargo("fix --edition --allow-no-vcs")
+        .with_stderr_data(str![[r#"
+[MIGRATING] Cargo.toml from 2021 edition to 2024
+[FIXED] Cargo.toml (1 fix)
+[MIGRATING] foo/Cargo.toml from 2021 edition to 2024
+[CHECKING] foo v0.0.0 ([ROOT]/foo/foo)
+[MIGRATING] foo/src/lib.rs from 2021 edition to 2024
+[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
+
+"#]])
+        .run();
+    assert_e2e().eq(
+        p.read_file("Cargo.toml"),
+        str![[r#"
+
+[workspace]
+members = ["foo"]
+resolver = "2"
+
+[workspace.dependencies]
+# Before default_features
+a = {path = "a", default-features = false}  # After default_features value
+# After default_features line
+
+"#]],
+    );
+    assert_e2e().eq(
+        p.read_file("foo/Cargo.toml"),
+        str![[r#"
+
+[package]
+name = "foo"
+edition = "2021"
+
+"#]],
+    );
+}
+
 #[cargo_test]
 fn remove_ignored_default_features() {
     Package::new("dep_simple", "0.1.0").publish();