Auto merge of #13804 - epage:underscore, r=weihanglo

fix(toml): Remove underscore field support in 2024

### What does this PR try to resolve?

This is part of the 2024 Edition and is part of rust-lang/rust#123754 and #13629

### How should we test and review this PR?

### Additional information

Author: bors

src/cargo/ops/fix.rs

@@ -254,9 +254,42 @@ fn migrate_manifests(ws: &Workspace<'_>, pkgs: &[&Package]) -> CargoResult<()> {
             let mut fixes = 0;
 
             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");
+            }
+
             fixes += add_feature_for_unused_deps(pkg, root);
-            if rename_table(root, "project", "package") {
-                fixes += 1;
+            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);
+            }
+            fixes += rename_array_of_target_fields_2024(root, "bin");
+            fixes += rename_array_of_target_fields_2024(root, "example");
+            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 += rename_table(root, "dev_dependencies", "dev-dependencies");
+            fixes += rename_dep_fields_2024(root, "dev-dependencies");
+            fixes += rename_table(root, "build_dependencies", "build-dependencies");
+            fixes += rename_dep_fields_2024(root, "build-dependencies");
+            for target in root
+                .get_mut("target")
+                .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())
+            {
+                fixes += rename_dep_fields_2024(target, "dependencies");
+                fixes += rename_table(target, "dev_dependencies", "dev-dependencies");
+                fixes += rename_dep_fields_2024(target, "dev-dependencies");
+                fixes += rename_table(target, "build_dependencies", "build-dependencies");
+                fixes += rename_dep_fields_2024(target, "build-dependencies");
             }
 
             if 0 < fixes {
@@ -274,9 +307,43 @@ fn migrate_manifests(ws: &Workspace<'_>, pkgs: &[&Package]) -> CargoResult<()> {
     Ok(())
 }
 
-fn rename_table(parent: &mut dyn toml_edit::TableLike, old: &str, new: &str) -> bool {
+fn rename_dep_fields_2024(parent: &mut dyn toml_edit::TableLike, dep_kind: &str) -> usize {
+    let mut fixes = 0;
+    for 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())
+    {
+        fixes += rename_table(target, "default_features", "default-features");
+    }
+    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
+        .get_mut(kind)
+        .and_then(|t| t.as_array_of_tables_mut())
+        .iter_mut()
+        .flat_map(|t| t.iter_mut())
+    {
+        fixes += rename_target_fields_2024(target);
+    }
+    fixes
+}
+
+fn rename_target_fields_2024(target: &mut dyn toml_edit::TableLike) -> usize {
+    let mut fixes = 0;
+    fixes += rename_table(target, "crate_type", "crate-type");
+    fixes += rename_table(target, "proc_macro", "proc-macro");
+    fixes
+}
+
+fn rename_table(parent: &mut dyn toml_edit::TableLike, old: &str, new: &str) -> usize {
     let Some(old_key) = parent.key(old).cloned() else {
-        return false;
+        return 0;
     };
 
     let project = parent.remove(old).expect("returned early");
@@ -286,7 +353,7 @@ fn rename_table(parent: &mut dyn toml_edit::TableLike, old: &str, new: &str) ->
         *new_key.dotted_decor_mut() = old_key.dotted_decor().clone();
         *new_key.leaf_decor_mut() = old_key.leaf_decor().clone();
     }
-    true
+    1
 }
 
 fn add_feature_for_unused_deps(pkg: &Package, parent: &mut dyn toml_edit::TableLike) -> usize {

src/cargo/util/toml/mod.rs

@@ -306,6 +306,8 @@ fn resolve_toml(
     };
 
     if let Some(original_package) = original_toml.package() {
+        let package_name = &original_package.name;
+
         let resolved_package =
             resolve_package_toml(original_package, features, package_root, &inherit)?;
         let edition = resolved_package
@@ -341,6 +343,15 @@ fn resolve_toml(
             package_root,
             warnings,
         )?;
+        deprecated_underscore(
+            &original_toml.dev_dependencies2,
+            &original_toml.dev_dependencies,
+            "dev-dependencies",
+            package_name,
+            "package",
+            edition,
+            warnings,
+        )?;
         resolved_toml.dev_dependencies = resolve_dependencies(
             gctx,
             edition,
@@ -352,6 +363,15 @@ fn resolve_toml(
             package_root,
             warnings,
         )?;
+        deprecated_underscore(
+            &original_toml.build_dependencies2,
+            &original_toml.build_dependencies,
+            "build-dependencies",
+            package_name,
+            "package",
+            edition,
+            warnings,
+        )?;
         resolved_toml.build_dependencies = resolve_dependencies(
             gctx,
             edition,
@@ -376,6 +396,15 @@ fn resolve_toml(
                 package_root,
                 warnings,
             )?;
+            deprecated_underscore(
+                &platform.dev_dependencies2,
+                &platform.dev_dependencies,
+                "dev-dependencies",
+                name,
+                "platform target",
+                edition,
+                warnings,
+            )?;
             let resolved_dev_dependencies = resolve_dependencies(
                 gctx,
                 edition,
@@ -387,6 +416,15 @@ fn resolve_toml(
                 package_root,
                 warnings,
             )?;
+            deprecated_underscore(
+                &platform.build_dependencies2,
+                &platform.build_dependencies,
+                "build-dependencies",
+                name,
+                "platform target",
+                edition,
+                warnings,
+            )?;
             let resolved_build_dependencies = resolve_dependencies(
                 gctx,
                 edition,
@@ -617,6 +655,15 @@ fn resolve_dependencies<'a>(
         let mut resolved =
             dependency_inherit_with(v.clone(), name_in_toml, inherit, package_root, warnings)?;
         if let manifest::TomlDependency::Detailed(ref mut d) = resolved {
+            deprecated_underscore(
+                &d.default_features2,
+                &d.default_features,
+                "default-features",
+                name_in_toml,
+                "dependency",
+                edition,
+                warnings,
+            )?;
             if d.public.is_some() {
                 let public_feature = features.require(Feature::public_dependency());
                 let with_public_feature = public_feature.is_ok();
@@ -1146,28 +1193,12 @@ fn to_real_manifest(
     }
 
     validate_dependencies(original_toml.dependencies.as_ref(), None, None, warnings)?;
-    deprecated_underscore(
-        &original_toml.dev_dependencies2,
-        &original_toml.dev_dependencies,
-        "dev-dependencies",
-        package_name,
-        "package",
-        warnings,
-    );
     validate_dependencies(
         original_toml.dev_dependencies(),
         None,
         Some(DepKind::Development),
         warnings,
     )?;
-    deprecated_underscore(
-        &original_toml.build_dependencies2,
-        &original_toml.build_dependencies,
-        "build-dependencies",
-        package_name,
-        "package",
-        warnings,
-    );
     validate_dependencies(
         original_toml.build_dependencies(),
         None,
@@ -1184,28 +1215,12 @@ fn to_real_manifest(
             None,
             warnings,
         )?;
-        deprecated_underscore(
-            &platform.build_dependencies2,
-            &platform.build_dependencies,
-            "build-dependencies",
-            name,
-            "platform target",
-            warnings,
-        );
         validate_dependencies(
             platform.build_dependencies(),
             platform_kind.as_ref(),
             Some(DepKind::Build),
             warnings,
         )?;
-        deprecated_underscore(
-            &platform.dev_dependencies2,
-            &platform.dev_dependencies,
-            "dev-dependencies",
-            name,
-            "platform target",
-            warnings,
-        );
         validate_dependencies(
             platform.dev_dependencies(),
             platform_kind.as_ref(),
@@ -1811,14 +1826,6 @@ fn detailed_dep_to_dependency<P: ResolveToPath + Clone>(
 
     let version = orig.version.as_deref();
     let mut dep = Dependency::parse(pkg_name, version, new_source_id)?;
-    deprecated_underscore(
-        &orig.default_features2,
-        &orig.default_features,
-        "default-features",
-        name_in_toml,
-        "dependency",
-        manifest_ctx.warnings,
-    );
     dep.set_features(orig.features.iter().flatten())
         .set_default_features(orig.default_features().unwrap_or(true))
         .set_optional(orig.optional.unwrap_or(false))
@@ -2321,19 +2328,22 @@ fn deprecated_underscore<T>(
     new_path: &str,
     name: &str,
     kind: &str,
+    edition: Edition,
     warnings: &mut Vec<String>,
-) {
-    if old.is_some() && new.is_some() {
-        let old_path = new_path.replace("-", "_");
+) -> CargoResult<()> {
+    let old_path = new_path.replace("-", "_");
+    if old.is_some() && Edition::Edition2024 <= edition {
+        anyhow::bail!("`{old_path}` is unsupported as of the 2024 edition; instead use `{new_path}`\n(in the `{name}` {kind})");
+    } else if old.is_some() && new.is_some() {
         warnings.push(format!(
             "`{old_path}` is redundant with `{new_path}`, preferring `{new_path}` in the `{name}` {kind}"
         ))
     } else if old.is_some() {
-        let old_path = new_path.replace("-", "_");
         warnings.push(format!(
             "`{old_path}` is deprecated in favor of `{new_path}` and will not work in the 2024 edition\n(in the `{name}` {kind})"
         ))
     }
+    Ok(())
 }
 
 fn warn_on_unused(unused: &BTreeSet<String>, warnings: &mut Vec<String>) {

src/cargo/util/toml/targets.rs

@@ -97,12 +97,7 @@ pub(super) fn to_targets(
         warnings,
         errors,
     )?;
-    targets.extend(to_example_targets(
-        &toml_examples,
-        package_root,
-        edition,
-        warnings,
-    )?);
+    targets.extend(to_example_targets(&toml_examples, package_root, edition)?);
 
     let toml_tests = resolve_tests(
         resolved_toml.test.as_ref(),
@@ -183,6 +178,10 @@ fn resolve_lib(
     // Check early to improve error messages
     validate_lib_name(&lib, warnings)?;
 
+    // Checking the original lib
+    validate_proc_macro(&lib, "library", edition, warnings)?;
+    validate_crate_types(&lib, "library", edition, warnings)?;
+
     if lib.path.is_none() {
         if let Some(inferred) = inferred {
             lib.path = Some(PathValue(inferred));
@@ -218,8 +217,6 @@ fn to_lib_target(
     let Some(lib) = resolved_lib else {
         return Ok(None);
     };
-    validate_proc_macro(lib, "library", warnings);
-    validate_crate_types(lib, "library", warnings);
 
     let path = lib.path.as_ref().expect("previously resolved");
     let path = package_root.join(&path.0);
@@ -442,14 +439,12 @@ fn to_example_targets(
     targets: &[TomlExampleTarget],
     package_root: &Path,
     edition: Edition,
-    warnings: &mut Vec<String>,
 ) -> CargoResult<Vec<Target>> {
     validate_unique_names(&targets, "example")?;
 
     let mut result = Vec::new();
     for toml in targets {
         let path = package_root.join(&toml.path.as_ref().expect("previously resolved").0);
-        validate_crate_types(&toml, "example", warnings);
         let crate_types = match toml.crate_types() {
             Some(kinds) => kinds.iter().map(|s| s.into()).collect(),
             None => Vec::new(),
@@ -637,6 +632,8 @@ fn resolve_targets_with_legacy_path(
 
     for target in &toml_targets {
         validate_target_name(target, target_kind_human, target_kind, warnings)?;
+        validate_proc_macro(target, target_kind_human, edition, warnings)?;
+        validate_crate_types(target, target_kind_human, edition, warnings)?;
     }
 
     let mut result = Vec::new();
@@ -1101,24 +1098,36 @@ fn name_or_panic(target: &TomlTarget) -> &str {
         .unwrap_or_else(|| panic!("target name is required"))
 }
 
-fn validate_proc_macro(target: &TomlTarget, kind: &str, warnings: &mut Vec<String>) {
+fn validate_proc_macro(
+    target: &TomlTarget,
+    kind: &str,
+    edition: Edition,
+    warnings: &mut Vec<String>,
+) -> CargoResult<()> {
     deprecated_underscore(
         &target.proc_macro2,
         &target.proc_macro,
         "proc-macro",
         name_or_panic(target),
         format!("{kind} target").as_str(),
+        edition,
         warnings,
-    );
+    )
 }
 
-fn validate_crate_types(target: &TomlTarget, kind: &str, warnings: &mut Vec<String>) {
+fn validate_crate_types(
+    target: &TomlTarget,
+    kind: &str,
+    edition: Edition,
+    warnings: &mut Vec<String>,
+) -> CargoResult<()> {
     deprecated_underscore(
         &target.crate_type2,
         &target.crate_type,
         "crate-type",
         name_or_panic(target),
         format!("{kind} target").as_str(),
+        edition,
         warnings,
-    );
+    )
 }