Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(toml): Remove underscore field support in 2024 #13804

Merged
merged 6 commits into from
Apr 26, 2024
Merged

fix(toml): Remove underscore field support in 2024 #13804

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
b81f94a
refactor(toml): Move underscore validation to resolving
epage Apr 24, 2024
aecb40b
fix(toml): Remove underscore field support in 2024
epage Apr 25, 2024
bcf032e
test(fix): Show underscore field migration
epage Apr 25, 2024
a71b8fe
feat(fix): Migrate underscore toml fields
epage Apr 25, 2024
57b4ab9
refactor(toml): Consolidate field name conversion
epage Apr 26, 2024
8ab7683
refactor(fix): Be consistent in fix tracking
epage Apr 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 72 additions & 5 deletions src/cargo/ops/fix.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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");
Expand All @@ -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 {
Expand Down
98 changes: 54 additions & 44 deletions src/cargo/util/toml/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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,
Expand All @@ -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(),
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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>) {
Expand Down
37 changes: 23 additions & 14 deletions src/cargo/util/toml/targets.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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,
);
)
}
Loading