Skip to content

Commit 83a3964

Browse files
committed
Auto merge of #14018 - epage:dep_name, r=ehuss
fix(fix): Address problems with implicit -> explicit feature migration ### What does this PR try to resolve? Within the scope of `cargo fix` there are two problems - We wipe out existing feature activations if it has the same name as an optional dependency - The `Cargo.toml` isn't parseable because the unused optional dependency won't "exist" if just `dep_name/feature_name` is used Fixes #14010 ### How should we test and review this PR? As for the unused optional dependency not "existing" error, - #14015 is for improving the message for weak dep features - #14016 is for re-evaluating how we handle this for strong dep features Depending on what solution we go with for #14016, we might want to revisit the second migration within this PR. This is one reason I made the commit separate (in addition to just making it clearer whats happening as this gets into some finer details of features). ### Additional information
2 parents f911dfd + 21c0928 commit 83a3964

File tree

2 files changed

+180
-61
lines changed

2 files changed

+180
-61
lines changed

src/cargo/ops/fix.rs

+43-9
Original file line numberDiff line numberDiff line change
@@ -285,7 +285,7 @@ fn migrate_manifests(ws: &Workspace<'_>, pkgs: &[&Package]) -> CargoResult<()> {
285285
fixes += rename_dep_fields_2024(workspace, "dependencies");
286286
}
287287

288-
fixes += add_feature_for_unused_deps(pkg, root);
288+
fixes += add_feature_for_unused_deps(pkg, root, ws.gctx());
289289
fixes += rename_table(root, "project", "package");
290290
if let Some(target) = root.get_mut("lib").and_then(|t| t.as_table_like_mut()) {
291291
fixes += rename_target_fields_2024(target);
@@ -435,7 +435,11 @@ fn rename_table(parent: &mut dyn toml_edit::TableLike, old: &str, new: &str) ->
435435
1
436436
}
437437

438-
fn add_feature_for_unused_deps(pkg: &Package, parent: &mut dyn toml_edit::TableLike) -> usize {
438+
fn add_feature_for_unused_deps(
439+
pkg: &Package,
440+
parent: &mut dyn toml_edit::TableLike,
441+
gctx: &GlobalContext,
442+
) -> usize {
439443
let manifest = pkg.manifest();
440444

441445
let activated_opt_deps = manifest
@@ -456,18 +460,48 @@ fn add_feature_for_unused_deps(pkg: &Package, parent: &mut dyn toml_edit::TableL
456460
for dep in manifest.dependencies() {
457461
let dep_name_in_toml = dep.name_in_toml();
458462
if dep.is_optional() && !activated_opt_deps.contains(dep_name_in_toml.as_str()) {
459-
fixes += 1;
460463
if let Some(features) = parent
461464
.entry("features")
462465
.or_insert(toml_edit::table())
463466
.as_table_like_mut()
464467
{
465-
features.insert(
466-
dep_name_in_toml.as_str(),
467-
toml_edit::Item::Value(toml_edit::Value::Array(toml_edit::Array::from_iter(
468-
&[format!("dep:{}", dep_name_in_toml)],
469-
))),
470-
);
468+
let activate_dep = format!("dep:{dep_name_in_toml}");
469+
let strong_dep_feature_prefix = format!("{dep_name_in_toml}/");
470+
features
471+
.entry(dep_name_in_toml.as_str())
472+
.or_insert_with(|| {
473+
fixes += 1;
474+
toml_edit::Item::Value(toml_edit::Value::Array(
475+
toml_edit::Array::from_iter([&activate_dep]),
476+
))
477+
});
478+
// Ensure `dep:dep_name` is present for `dep_name/feature_name` since `dep:` is the
479+
// only way to guarantee an optional dependency is available for use.
480+
//
481+
// The way we avoid implicitly creating features in Edition2024 is we remove the
482+
// dependency from `resolved_toml` if there is no `dep:` syntax as that is the only
483+
// syntax that suppresses the creation of the implicit feature.
484+
for (feature_name, activations) in features.iter_mut() {
485+
let Some(activations) = activations.as_array_mut() else {
486+
let _ = gctx.shell().warn(format_args!("skipping fix of feature `{feature_name}` in package `{}`: unsupported feature schema", pkg.name()));
487+
continue;
488+
};
489+
if activations
490+
.iter()
491+
.any(|a| a.as_str().map(|a| a == activate_dep).unwrap_or(false))
492+
{
493+
continue;
494+
}
495+
let Some(activate_dep_pos) = activations.iter().position(|a| {
496+
a.as_str()
497+
.map(|a| a.starts_with(&strong_dep_feature_prefix))
498+
.unwrap_or(false)
499+
}) else {
500+
continue;
501+
};
502+
fixes += 1;
503+
activations.insert(activate_dep_pos, &activate_dep);
504+
}
471505
}
472506
}
473507
}

0 commit comments

Comments
 (0)