Skip to content

Commit fb3906d

Browse files
epageepage
authored
fix: Suggest similar looking feature names when feature is missing (#15454)
### What does this PR try to resolve? I recently depended on a package with `preserve-order` rather than `preserve_order` and the error message didn't help me with the problem so I figure I'd fix that. I also found other improvements along the way - Suggest an alternative feature when a feature includes a missing feature - Suggest an alternative feature when a dependency includes a missing feature - Lower case error messages - Re-frame prescriptive information as help - Change plural "features" error messages to singular "feature" as they can only ever have one (knowing an the `MissingFeature` string only has one feature in it was important for doing a `closest` match on the feature). ### How should we test and review this PR? ### Additional information
2 parents 872b92e + 08ed8de commit fb3906d

File tree

9 files changed

+172
-62
lines changed

9 files changed

+172
-62
lines changed

‎src/cargo/core/resolver/dep_cache.rs‎

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -524,15 +524,13 @@ impl RequirementError {
524524
"feature",
525525
);
526526
ActivateError::Fatal(anyhow::format_err!(
527-
"Package `{}` does not have the feature `{}`{}",
527+
"package `{}` does not have the feature `{}`{}",
528528
summary.package_id(),
529529
feat,
530530
closest
531531
))
532532
}
533-
Some(p) => {
534-
ActivateError::Conflict(p, ConflictReason::MissingFeatures(feat))
535-
}
533+
Some(p) => ActivateError::Conflict(p, ConflictReason::MissingFeature(feat)),
536534
};
537535
}
538536
if deps.iter().any(|dep| dep.is_optional()) {
@@ -555,9 +553,11 @@ impl RequirementError {
555553
}
556554
ActivateError::Fatal(anyhow::format_err!(
557555
"\
558-
Package `{}` does not have feature `{}`. It has an optional dependency \
559-
with that name, but that dependency uses the \"dep:\" \
560-
syntax in the features table, so it does not have an implicit feature with that name.{}",
556+
package `{}` does not have feature `{}`
557+
558+
help: an optional dependency \
559+
with that name exists, but the `features` table includes it with the \"dep:\" \
560+
syntax so it does not have an implicit feature with that name{}",
561561
summary.package_id(),
562562
feat,
563563
suggestion
@@ -571,8 +571,9 @@ syntax in the features table, so it does not have an implicit feature with that
571571
} else {
572572
match parent {
573573
None => ActivateError::Fatal(anyhow::format_err!(
574-
"Package `{}` does not have feature `{}`. It has a required dependency \
575-
with that name, but only optional dependencies can be used as features.",
574+
"package `{}` does not have feature `{}`
575+
576+
help: a depednency with that name exists but it is required dependency and only optional dependencies can be used as features.",
576577
summary.package_id(),
577578
feat,
578579
)),
@@ -592,9 +593,7 @@ syntax in the features table, so it does not have an implicit feature with that
592593
)),
593594
// This code path currently isn't used, since `foo/bar`
594595
// and `dep:` syntax is not allowed in a dependency.
595-
Some(p) => {
596-
ActivateError::Conflict(p, ConflictReason::MissingFeatures(dep_name))
597-
}
596+
Some(p) => ActivateError::Conflict(p, ConflictReason::MissingFeature(dep_name)),
598597
}
599598
}
600599
RequirementError::Cycle(feat) => ActivateError::Fatal(anyhow::format_err!(

‎src/cargo/core/resolver/errors.rs‎

Lines changed: 27 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use std::task::Poll;
55
use crate::core::{Dependency, PackageId, Registry, Summary};
66
use crate::sources::source::QueryKind;
77
use crate::sources::IndexSummary;
8-
use crate::util::edit_distance::edit_distance;
8+
use crate::util::edit_distance::{closest, edit_distance};
99
use crate::util::errors::CargoResult;
1010
use crate::util::{GlobalContext, OptVersionReq, VersionExt};
1111
use anyhow::Error;
@@ -137,7 +137,7 @@ pub(super) fn activation_error(
137137
has_semver = true;
138138
}
139139
ConflictReason::Links(link) => {
140-
msg.push_str("\n\nthe package `");
140+
msg.push_str("\n\npackage `");
141141
msg.push_str(&*dep.package_name());
142142
msg.push_str("` links to the native library `");
143143
msg.push_str(link);
@@ -150,46 +150,54 @@ pub(super) fn activation_error(
150150
msg.push_str(link);
151151
msg.push_str("\"` value. For more information, see https://doc.rust-lang.org/cargo/reference/resolver.html#links.");
152152
}
153-
ConflictReason::MissingFeatures(features) => {
154-
msg.push_str("\n\nthe package `");
153+
ConflictReason::MissingFeature(feature) => {
154+
msg.push_str("\n\npackage `");
155155
msg.push_str(&*p.name());
156156
msg.push_str("` depends on `");
157157
msg.push_str(&*dep.package_name());
158-
msg.push_str("`, with features: `");
159-
msg.push_str(features);
158+
msg.push_str("` with feature `");
159+
msg.push_str(feature);
160160
msg.push_str("` but `");
161161
msg.push_str(&*dep.package_name());
162-
msg.push_str("` does not have these features.\n");
162+
msg.push_str("` does not have that feature.\n");
163+
let latest = candidates.last().expect("in the non-empty branch");
164+
if let Some(closest) = closest(feature, latest.features().keys(), |k| k) {
165+
msg.push_str(" package `");
166+
msg.push_str(&*dep.package_name());
167+
msg.push_str("` does have feature `");
168+
msg.push_str(closest);
169+
msg.push_str("`\n");
170+
}
163171
// p == parent so the full path is redundant.
164172
}
165-
ConflictReason::RequiredDependencyAsFeature(features) => {
166-
msg.push_str("\n\nthe package `");
173+
ConflictReason::RequiredDependencyAsFeature(feature) => {
174+
msg.push_str("\n\npackage `");
167175
msg.push_str(&*p.name());
168176
msg.push_str("` depends on `");
169177
msg.push_str(&*dep.package_name());
170-
msg.push_str("`, with features: `");
171-
msg.push_str(features);
178+
msg.push_str("` with feature `");
179+
msg.push_str(feature);
172180
msg.push_str("` but `");
173181
msg.push_str(&*dep.package_name());
174-
msg.push_str("` does not have these features.\n");
182+
msg.push_str("` does not have that feature.\n");
175183
msg.push_str(
176-
" It has a required dependency with that name, \
184+
" A required dependency with that name exists, \
177185
but only optional dependencies can be used as features.\n",
178186
);
179187
// p == parent so the full path is redundant.
180188
}
181-
ConflictReason::NonImplicitDependencyAsFeature(features) => {
182-
msg.push_str("\n\nthe package `");
189+
ConflictReason::NonImplicitDependencyAsFeature(feature) => {
190+
msg.push_str("\n\npackage `");
183191
msg.push_str(&*p.name());
184192
msg.push_str("` depends on `");
185193
msg.push_str(&*dep.package_name());
186-
msg.push_str("`, with features: `");
187-
msg.push_str(features);
194+
msg.push_str("` with feature `");
195+
msg.push_str(feature);
188196
msg.push_str("` but `");
189197
msg.push_str(&*dep.package_name());
190-
msg.push_str("` does not have these features.\n");
198+
msg.push_str("` does not have that feature.\n");
191199
msg.push_str(
192-
" It has an optional dependency with that name, \
200+
" An optional dependency with that name exists, \
193201
but that dependency uses the \"dep:\" \
194202
syntax in the features table, so it does not have an \
195203
implicit feature with that name.\n",

‎src/cargo/core/resolver/types.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -339,10 +339,10 @@ pub enum ConflictReason {
339339
/// we're only allowed one per dependency graph.
340340
Links(InternedString),
341341

342-
/// A dependency listed features that weren't actually available on the
342+
/// A dependency listed a feature that wasn't actually available on the
343343
/// candidate. For example we tried to activate feature `foo` but the
344344
/// candidate we're activating didn't actually have the feature `foo`.
345-
MissingFeatures(InternedString),
345+
MissingFeature(InternedString),
346346

347347
/// A dependency listed a feature that ended up being a required dependency.
348348
/// For example we tried to activate feature `foo` but the
@@ -360,8 +360,8 @@ impl ConflictReason {
360360
matches!(self, ConflictReason::Links(_))
361361
}
362362

363-
pub fn is_missing_features(&self) -> bool {
364-
matches!(self, ConflictReason::MissingFeatures(_))
363+
pub fn is_missing_feature(&self) -> bool {
364+
matches!(self, ConflictReason::MissingFeature(_))
365365
}
366366

367367
pub fn is_required_dependency_as_features(&self) -> bool {

‎src/cargo/core/summary.rs‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
use crate::core::{Dependency, PackageId, SourceId};
2+
use crate::util::closest_msg;
23
use crate::util::interning::InternedString;
34
use crate::util::CargoResult;
45
use anyhow::bail;
@@ -241,9 +242,10 @@ fn build_feature_map(
241242
Feature(f) => {
242243
if !features.contains_key(f) {
243244
if !is_any_dep {
245+
let closest = closest_msg(f, features.keys(), |k| k, "feature");
244246
bail!(
245247
"feature `{feature}` includes `{fv}` which is neither a dependency \
246-
nor another feature"
248+
nor another feature{closest}"
247249
);
248250
}
249251
if is_optional_dep {

‎tests/testsuite/build_script.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1032,7 +1032,7 @@ fn links_duplicates() {
10321032
... required by package `foo v0.5.0 ([ROOT]/foo)`
10331033
versions that meet the requirements `*` are: 0.5.0
10341034
1035-
the package `a-sys` links to the native library `a`, but it conflicts with a previous package which links to `a` as well:
1035+
package `a-sys` links to the native library `a`, but it conflicts with a previous package which links to `a` as well:
10361036
package `foo v0.5.0 ([ROOT]/foo)`
10371037
Only one package in the dependency graph may specify the same links value. This helps ensure that only one copy of a native library is linked in the final binary. Try to adjust your dependencies so that only one package uses the `links = "a"` value. For more information, see https://doc.rust-lang.org/cargo/reference/resolver.html#links.
10381038
@@ -1159,7 +1159,7 @@ fn links_duplicates_deep_dependency() {
11591159
... which satisfies path dependency `a` of package `foo v0.5.0 ([ROOT]/foo)`
11601160
versions that meet the requirements `*` are: 0.5.0
11611161
1162-
the package `a-sys` links to the native library `a`, but it conflicts with a previous package which links to `a` as well:
1162+
package `a-sys` links to the native library `a`, but it conflicts with a previous package which links to `a` as well:
11631163
package `foo v0.5.0 ([ROOT]/foo)`
11641164
Only one package in the dependency graph may specify the same links value. This helps ensure that only one copy of a native library is linked in the final binary. Try to adjust your dependencies so that only one package uses the `links = "a"` value. For more information, see https://doc.rust-lang.org/cargo/reference/resolver.html#links.
11651165
@@ -4767,7 +4767,7 @@ fn links_duplicates_with_cycle() {
47674767
... required by package `foo v0.5.0 ([ROOT]/foo)`
47684768
versions that meet the requirements `*` are: 0.5.0
47694769
4770-
the package `a` links to the native library `a`, but it conflicts with a previous package which links to `a` as well:
4770+
package `a` links to the native library `a`, but it conflicts with a previous package which links to `a` as well:
47714771
package `foo v0.5.0 ([ROOT]/foo)`
47724772
Only one package in the dependency graph may specify the same links value. This helps ensure that only one copy of a native library is linked in the final binary. Try to adjust your dependencies so that only one package uses the `links = "a"` value. For more information, see https://doc.rust-lang.org/cargo/reference/resolver.html#links.
47734773

0 commit comments

Comments
 (0)