From e8b28cf87e959ef9d62810c4f6932ad786bbc116 Mon Sep 17 00:00:00 2001 From: Kornel Date: Wed, 21 Aug 2024 13:50:54 +0100 Subject: [PATCH] More helpful missing feature error message --- src/cargo/core/workspace.rs | 90 +++++++++++++++++++++++------ tests/testsuite/package_features.rs | 28 +++++++-- 2 files changed, 96 insertions(+), 22 deletions(-) diff --git a/src/cargo/core/workspace.rs b/src/cargo/core/workspace.rs index 591612341fc..e187b7ca326 100644 --- a/src/cargo/core/workspace.rs +++ b/src/cargo/core/workspace.rs @@ -1363,12 +1363,12 @@ impl<'gctx> Workspace<'gctx> { } } - fn report_unknown_features_error( + fn missing_feature_spelling_suggestions( &self, - specs: &[PackageIdSpec], + selected_members: &[&Package], cli_features: &CliFeatures, found_features: &BTreeSet, - ) -> CargoResult<()> { + ) -> Vec { // Keeps track of which features were contained in summary of `member` to suggest similar features in errors let mut summary_features: Vec = Default::default(); @@ -1387,10 +1387,7 @@ impl<'gctx> Workspace<'gctx> { let mut optional_dependency_names_per_member: BTreeMap<&Package, BTreeSet> = Default::default(); - for member in self - .members() - .filter(|m| specs.iter().any(|spec| spec.matches(m.package_id()))) - { + for &member in selected_members { // Only include features this member defines. let summary = member.summary(); @@ -1428,7 +1425,7 @@ impl<'gctx> Workspace<'gctx> { edit_distance(a.as_str(), b.as_str(), 3).is_some() }; - let suggestions: Vec<_> = cli_features + cli_features .features .difference(found_features) .map(|feature| match feature { @@ -1522,8 +1519,15 @@ impl<'gctx> Workspace<'gctx> { }) .sorted() .take(5) - .collect(); + .collect() + } + fn report_unknown_features_error( + &self, + specs: &[PackageIdSpec], + cli_features: &CliFeatures, + found_features: &BTreeSet, + ) -> CargoResult<()> { let unknown: Vec<_> = cli_features .features .difference(found_features) @@ -1531,18 +1535,70 @@ impl<'gctx> Workspace<'gctx> { .sorted() .collect(); - if suggestions.is_empty() { - bail!( - "none of the selected packages contains these features: {}", + let (selected_members, unselected_members): (Vec<_>, Vec<_>) = self + .members() + .partition(|member| specs.iter().any(|spec| spec.matches(member.package_id()))); + + let missing_packages_with_the_features = unselected_members + .into_iter() + .filter(|member| { + unknown + .iter() + .any(|feature| member.summary().features().contains_key(&**feature)) + }) + .map(|m| m.name()) + .collect_vec(); + + let these_features = if unknown.len() == 1 { + "this feature" + } else { + "these features" + }; + let mut msg = if let [singular] = &selected_members[..] { + format!( + "the package '{}' does not contain {these_features}: {}", + singular.name(), unknown.join(", ") - ); + ) } else { - bail!( - "none of the selected packages contains these features: {}, did you mean: {}?", - unknown.join(", "), - suggestions.join(", ") + let names = selected_members.iter().map(|m| m.name()).join(", "); + format!("none of the selected packages contains {these_features}: {}\nselected packages: {names}", unknown.join(", ")) + }; + + use std::fmt::Write; + if !missing_packages_with_the_features.is_empty() { + write!( + &mut msg, + "\nhelp: package{} with the missing feature{}: {}", + if missing_packages_with_the_features.len() != 1 { + "s" + } else { + "" + }, + if unknown.len() != 1 { "s" } else { "" }, + missing_packages_with_the_features.join(", ") + )?; + } else { + let suggestions = self.missing_feature_spelling_suggestions( + &selected_members, + cli_features, + found_features, ); + if !suggestions.is_empty() { + write!( + &mut msg, + "\nhelp: there {}: {}", + if suggestions.len() == 1 { + "is a similarly named feature" + } else { + "are similarly named features" + }, + suggestions.join(", ") + )?; + } } + + bail!("{msg}") } /// New command-line feature selection behavior with resolver = "2" or the diff --git a/tests/testsuite/package_features.rs b/tests/testsuite/package_features.rs index bf20ae63b3c..9d37de2c68c 100644 --- a/tests/testsuite/package_features.rs +++ b/tests/testsuite/package_features.rs @@ -75,7 +75,9 @@ fn virtual_no_default_features() { p.cargo("check --features foo") .with_status(101) .with_stderr_data(str![[r#" -[ERROR] none of the selected packages contains these features: foo, did you mean: f1? +[ERROR] none of the selected packages contains this feature: foo +selected packages: a, b +[HELP] there is a similarly named feature: f1 "#]]) .run(); @@ -83,7 +85,9 @@ fn virtual_no_default_features() { p.cargo("check --features a/dep1,b/f1,b/f2,f2") .with_status(101) .with_stderr_data(str![[r#" -[ERROR] none of the selected packages contains these features: b/f2, f2, did you mean: f1? +[ERROR] none of the selected packages contains these features: b/f2, f2 +selected packages: a, b +[HELP] there is a similarly named feature: f1 "#]]) .run(); @@ -91,7 +95,9 @@ fn virtual_no_default_features() { p.cargo("check --features a/dep,b/f1,b/f2,f2") .with_status(101) .with_stderr_data(str![[r#" -[ERROR] none of the selected packages contains these features: a/dep, b/f2, f2, did you mean: a/dep1, f1? +[ERROR] none of the selected packages contains these features: a/dep, b/f2, f2 +selected packages: a, b +[HELP] there are similarly named features: a/dep1, f1 "#]]) .run(); @@ -99,7 +105,18 @@ fn virtual_no_default_features() { p.cargo("check --features a/dep,a/dep1") .with_status(101) .with_stderr_data(str![[r#" -[ERROR] none of the selected packages contains these features: a/dep, did you mean: b/f1? +[ERROR] none of the selected packages contains this feature: a/dep +selected packages: a, b +[HELP] there is a similarly named feature: b/f1 + +"#]]) + .run(); + + p.cargo("check -p b --features=dep1") + .with_status(101) + .with_stderr_data(str![[r#" +[ERROR] the package 'b' does not contain this feature: dep1 +[HELP] package with the missing feature: a "#]]) .run(); @@ -126,7 +143,8 @@ fn virtual_typo_member_feature() { .cargo("check --features a/deny-warning") .with_status(101) .with_stderr_data(str![[r#" -[ERROR] none of the selected packages contains these features: a/deny-warning, did you mean: a/deny-warnings? +[ERROR] the package 'a' does not contain this feature: a/deny-warning +[HELP] there is a similarly named feature: a/deny-warnings "#]]) .run();