Skip to content

fix: Suggest similar looking feature names when feature is missing #15454

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

Merged
merged 6 commits into from
Apr 25, 2025
Merged

fix: Suggest similar looking feature names when feature is missing #15454

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
a1c4644
test(features): Clarify test names
epage Apr 24, 2025
a38d44c
test(features): Show typoed feature behavior
epage Apr 24, 2025
93c734b
fix(features): When feature activates a typo, suggest alt
epage Apr 24, 2025
3fd3656
fix(resolver): Have errors follow rustc's casing convention
epage Apr 25, 2025
f616aaf
fix(resolver): Make features singular; we only track one
epage Apr 25, 2025
08ed8de
fix(resolver): Suggest similar looking features
epage Apr 25, 2025
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
23 changes: 11 additions & 12 deletions src/cargo/core/resolver/dep_cache.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -524,15 +524,13 @@ impl RequirementError {
"feature",
);
ActivateError::Fatal(anyhow::format_err!(
"Package `{}` does not have the feature `{}`{}",
"package `{}` does not have the feature `{}`{}",
summary.package_id(),
feat,
closest
))
}
Some(p) => {
ActivateError::Conflict(p, ConflictReason::MissingFeatures(feat))
}
Some(p) => ActivateError::Conflict(p, ConflictReason::MissingFeature(feat)),
};
}
if deps.iter().any(|dep| dep.is_optional()) {
Expand All @@ -555,9 +553,11 @@ impl RequirementError {
}
ActivateError::Fatal(anyhow::format_err!(
"\
Package `{}` does not have feature `{}`. It has an optional dependency \
with that name, but that dependency uses the \"dep:\" \
syntax in the features table, so it does not have an implicit feature with that name.{}",
package `{}` does not have feature `{}`

help: an optional dependency \
with that name exists, but the `features` table includes it with the \"dep:\" \
syntax so it does not have an implicit feature with that name{}",
summary.package_id(),
feat,
suggestion
Expand All @@ -571,8 +571,9 @@ syntax in the features table, so it does not have an implicit feature with that
} else {
match parent {
None => ActivateError::Fatal(anyhow::format_err!(
"Package `{}` does not have feature `{}`. It has a required dependency \
with that name, but only optional dependencies can be used as features.",
"package `{}` does not have feature `{}`

help: a depednency with that name exists but it is required dependency and only optional dependencies can be used as features.",
summary.package_id(),
feat,
)),
Expand All @@ -592,9 +593,7 @@ syntax in the features table, so it does not have an implicit feature with that
)),
// This code path currently isn't used, since `foo/bar`
// and `dep:` syntax is not allowed in a dependency.
Some(p) => {
ActivateError::Conflict(p, ConflictReason::MissingFeatures(dep_name))
}
Some(p) => ActivateError::Conflict(p, ConflictReason::MissingFeature(dep_name)),
}
}
RequirementError::Cycle(feat) => ActivateError::Fatal(anyhow::format_err!(
Expand Down
46 changes: 27 additions & 19 deletions src/cargo/core/resolver/errors.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use std::task::Poll;
use crate::core::{Dependency, PackageId, Registry, Summary};
use crate::sources::source::QueryKind;
use crate::sources::IndexSummary;
use crate::util::edit_distance::edit_distance;
use crate::util::edit_distance::{closest, edit_distance};
use crate::util::errors::CargoResult;
use crate::util::{GlobalContext, OptVersionReq, VersionExt};
use anyhow::Error;
Expand Down Expand Up @@ -137,7 +137,7 @@ pub(super) fn activation_error(
has_semver = true;
}
ConflictReason::Links(link) => {
msg.push_str("\n\nthe package `");
msg.push_str("\n\npackage `");
msg.push_str(&*dep.package_name());
msg.push_str("` links to the native library `");
msg.push_str(link);
Expand All @@ -150,46 +150,54 @@ pub(super) fn activation_error(
msg.push_str(link);
msg.push_str("\"` value. For more information, see https://doc.rust-lang.org/cargo/reference/resolver.html#links.");
}
ConflictReason::MissingFeatures(features) => {
msg.push_str("\n\nthe package `");
ConflictReason::MissingFeature(feature) => {
msg.push_str("\n\npackage `");
msg.push_str(&*p.name());
msg.push_str("` depends on `");
msg.push_str(&*dep.package_name());
msg.push_str("`, with features: `");
msg.push_str(features);
msg.push_str("` with feature `");
msg.push_str(feature);
msg.push_str("` but `");
msg.push_str(&*dep.package_name());
msg.push_str("` does not have these features.\n");
msg.push_str("` does not have that feature.\n");
let latest = candidates.last().expect("in the non-empty branch");
if let Some(closest) = closest(feature, latest.features().keys(), |k| k) {
msg.push_str(" package `");
msg.push_str(&*dep.package_name());
msg.push_str("` does have feature `");
msg.push_str(closest);
msg.push_str("`\n");
}
// p == parent so the full path is redundant.
}
ConflictReason::RequiredDependencyAsFeature(features) => {
msg.push_str("\n\nthe package `");
ConflictReason::RequiredDependencyAsFeature(feature) => {
msg.push_str("\n\npackage `");
msg.push_str(&*p.name());
msg.push_str("` depends on `");
msg.push_str(&*dep.package_name());
msg.push_str("`, with features: `");
msg.push_str(features);
msg.push_str("` with feature `");
msg.push_str(feature);
msg.push_str("` but `");
msg.push_str(&*dep.package_name());
msg.push_str("` does not have these features.\n");
msg.push_str("` does not have that feature.\n");
msg.push_str(
" It has a required dependency with that name, \
" A required dependency with that name exists, \
but only optional dependencies can be used as features.\n",
);
// p == parent so the full path is redundant.
}
ConflictReason::NonImplicitDependencyAsFeature(features) => {
msg.push_str("\n\nthe package `");
ConflictReason::NonImplicitDependencyAsFeature(feature) => {
msg.push_str("\n\npackage `");
msg.push_str(&*p.name());
msg.push_str("` depends on `");
msg.push_str(&*dep.package_name());
msg.push_str("`, with features: `");
msg.push_str(features);
msg.push_str("` with feature `");
msg.push_str(feature);
msg.push_str("` but `");
msg.push_str(&*dep.package_name());
msg.push_str("` does not have these features.\n");
msg.push_str("` does not have that feature.\n");
msg.push_str(
" It has an optional dependency with that name, \
" An optional dependency with that name exists, \
but that dependency uses the \"dep:\" \
syntax in the features table, so it does not have an \
implicit feature with that name.\n",
Expand Down
8 changes: 4 additions & 4 deletions src/cargo/core/resolver/types.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -339,10 +339,10 @@ pub enum ConflictReason {
/// we're only allowed one per dependency graph.
Links(InternedString),

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

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

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

pub fn is_required_dependency_as_features(&self) -> bool {
Expand Down
4 changes: 3 additions & 1 deletion src/cargo/core/summary.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use crate::core::{Dependency, PackageId, SourceId};
use crate::util::closest_msg;
use crate::util::interning::InternedString;
use crate::util::CargoResult;
use anyhow::bail;
Expand Down Expand Up @@ -241,9 +242,10 @@ fn build_feature_map(
Feature(f) => {
if !features.contains_key(f) {
if !is_any_dep {
let closest = closest_msg(f, features.keys(), |k| k, "feature");
bail!(
"feature `{feature}` includes `{fv}` which is neither a dependency \
nor another feature"
nor another feature{closest}"
);
}
if is_optional_dep {
Expand Down
6 changes: 3 additions & 3 deletions tests/testsuite/build_script.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1032,7 +1032,7 @@ fn links_duplicates() {
... required by package `foo v0.5.0 ([ROOT]/foo)`
versions that meet the requirements `*` are: 0.5.0

the package `a-sys` links to the native library `a`, but it conflicts with a previous package which links to `a` as well:
package `a-sys` links to the native library `a`, but it conflicts with a previous package which links to `a` as well:
package `foo v0.5.0 ([ROOT]/foo)`
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.

Expand Down Expand Up @@ -1159,7 +1159,7 @@ fn links_duplicates_deep_dependency() {
... which satisfies path dependency `a` of package `foo v0.5.0 ([ROOT]/foo)`
versions that meet the requirements `*` are: 0.5.0

the package `a-sys` links to the native library `a`, but it conflicts with a previous package which links to `a` as well:
package `a-sys` links to the native library `a`, but it conflicts with a previous package which links to `a` as well:
package `foo v0.5.0 ([ROOT]/foo)`
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.

Expand Down Expand Up @@ -4767,7 +4767,7 @@ fn links_duplicates_with_cycle() {
... required by package `foo v0.5.0 ([ROOT]/foo)`
versions that meet the requirements `*` are: 0.5.0

the package `a` links to the native library `a`, but it conflicts with a previous package which links to `a` as well:
package `a` links to the native library `a`, but it conflicts with a previous package which links to `a` as well:
package `foo v0.5.0 ([ROOT]/foo)`
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.

Expand Down
Loading