Skip to content

Commit cdbd9b7

Browse files
committed
Auto merge of #7950 - aleksator:4854_non_existent_features_error, r=ehuss
Check manifest for requiring nonexistent features Fixes #4854: Examples requiring a nonexistent feature should be an error Thanks @lukaslueg with his #4874 for the inspiration!
2 parents 8e89670 + 19a9579 commit cdbd9b7

File tree

3 files changed

+126
-9
lines changed

3 files changed

+126
-9
lines changed

src/cargo/ops/cargo_compile.rs

Lines changed: 72 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ use crate::core::compiler::{DefaultExecutor, Executor, UnitInterner};
3434
use crate::core::profiles::{Profiles, UnitFor};
3535
use crate::core::resolver::features::{self, FeaturesFor};
3636
use crate::core::resolver::{HasDevUnits, Resolve, ResolveOpts};
37-
use crate::core::{Package, PackageSet, Target};
37+
use crate::core::{FeatureValue, Package, PackageSet, Shell, Summary, Target};
3838
use crate::core::{PackageId, PackageIdSpec, TargetKind, Workspace};
3939
use crate::ops;
4040
use crate::ops::resolve::WorkspaceResolve;
@@ -417,6 +417,7 @@ pub fn create_bcx<'a, 'cfg>(
417417
&build_config.requested_kinds,
418418
build_config.mode,
419419
&resolve,
420+
&workspace_resolve,
420421
&resolved_features,
421422
&pkg_set,
422423
&profiles,
@@ -701,6 +702,7 @@ fn generate_targets(
701702
requested_kinds: &[CompileKind],
702703
mode: CompileMode,
703704
resolve: &Resolve,
705+
workspace_resolve: &Option<Resolve>,
704706
resolved_features: &features::ResolvedFeatures,
705707
package_set: &PackageSet<'_>,
706708
profiles: &Profiles,
@@ -929,6 +931,13 @@ fn generate_targets(
929931
{
930932
let unavailable_features = match target.required_features() {
931933
Some(rf) => {
934+
warn_on_missing_features(
935+
workspace_resolve,
936+
rf,
937+
pkg.summary(),
938+
&mut config.shell(),
939+
)?;
940+
932941
let features = features_map.entry(pkg).or_insert_with(|| {
933942
resolve_all_features(resolve, resolved_features, package_set, pkg.package_id())
934943
});
@@ -958,6 +967,68 @@ fn generate_targets(
958967
Ok(units.into_iter().collect())
959968
}
960969

970+
fn warn_on_missing_features(
971+
resolve: &Option<Resolve>,
972+
required_features: &[String],
973+
summary: &Summary,
974+
shell: &mut Shell,
975+
) -> CargoResult<()> {
976+
let resolve = match resolve {
977+
None => return Ok(()),
978+
Some(resolve) => resolve,
979+
};
980+
981+
for feature in required_features {
982+
match FeatureValue::new(feature.into(), summary) {
983+
// No need to do anything here, since the feature must exist to be parsed as such
984+
FeatureValue::Feature(_) => {}
985+
// Possibly mislabeled feature that was not found
986+
FeatureValue::Crate(krate) => {
987+
if !summary
988+
.dependencies()
989+
.iter()
990+
.any(|dep| dep.name_in_toml() == krate && dep.is_optional())
991+
{
992+
shell.warn(format!(
993+
"feature `{}` is not present in [features] section.",
994+
krate
995+
))?;
996+
}
997+
}
998+
// Handling of dependent_crate/dependent_crate_feature syntax
999+
FeatureValue::CrateFeature(krate, feature) => {
1000+
match resolve
1001+
.deps(summary.package_id())
1002+
.find(|(_dep_id, deps)| deps.iter().any(|dep| dep.name_in_toml() == krate))
1003+
{
1004+
Some((dep_id, _deps)) => {
1005+
let dep_summary = resolve.summary(dep_id);
1006+
if !dep_summary.features().contains_key(&feature)
1007+
&& !dep_summary
1008+
.dependencies()
1009+
.iter()
1010+
.any(|dep| dep.name_in_toml() == feature && dep.is_optional())
1011+
{
1012+
shell.warn(format!(
1013+
"feature `{}` does not exist in package `{}`.",
1014+
feature, dep_id
1015+
))?;
1016+
}
1017+
}
1018+
None => {
1019+
shell.warn(format!(
1020+
"dependency `{}` specified in required-features as `{}/{}` \
1021+
does not exist.",
1022+
krate, krate, feature
1023+
))?;
1024+
}
1025+
}
1026+
}
1027+
}
1028+
}
1029+
Ok(())
1030+
}
1031+
9611032
/// Gets all of the features enabled for a package, plus its dependencies'
9621033
/// features.
9631034
///

tests/testsuite/bench.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -595,6 +595,9 @@ fn bench_autodiscover_2015() {
595595
version = "0.0.1"
596596
authors = []
597597
edition = "2015"
598+
599+
[features]
600+
magic = []
598601
599602
[[bench]]
600603
name = "bench_magic"

tests/testsuite/features.rs

Lines changed: 51 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -291,7 +291,7 @@ fn invalid9() {
291291
.build();
292292

293293
p.cargo("build --features bar")
294-
.with_stderr(
294+
.with_stderr(
295295
"\
296296
error: Package `foo v0.0.1 ([..])` does not have feature `bar`. It has a required dependency with that name, but only optional dependencies can be used as features.
297297
",
@@ -1514,14 +1514,14 @@ fn namespaced_shadowed_dep() {
15141514
.build();
15151515

15161516
p.cargo("build").masquerade_as_nightly_cargo().with_status(101).with_stderr(
1517-
"\
1517+
"\
15181518
[ERROR] failed to parse manifest at `[..]`
15191519
15201520
Caused by:
15211521
Feature `baz` includes the optional dependency of the same name, but this is left implicit in the features included by this feature.
15221522
Consider adding `crate:baz` to this feature's requirements.
15231523
",
1524-
)
1524+
)
15251525
.run();
15261526
}
15271527

@@ -1550,15 +1550,15 @@ fn namespaced_shadowed_non_optional() {
15501550
.build();
15511551

15521552
p.cargo("build").masquerade_as_nightly_cargo().with_status(101).with_stderr(
1553-
"\
1553+
"\
15541554
[ERROR] failed to parse manifest at `[..]`
15551555
15561556
Caused by:
15571557
Feature `baz` includes the dependency of the same name, but this is left implicit in the features included by this feature.
15581558
Additionally, the dependency must be marked as optional to be included in the feature definition.
15591559
Consider adding `crate:baz` to this feature's requirements and marking the dependency as `optional = true`
15601560
",
1561-
)
1561+
)
15621562
.run();
15631563
}
15641564

@@ -1587,15 +1587,14 @@ fn namespaced_implicit_non_optional() {
15871587
.build();
15881588

15891589
p.cargo("build").masquerade_as_nightly_cargo().with_status(101).with_stderr(
1590-
"\
1590+
"\
15911591
[ERROR] failed to parse manifest at `[..]`
15921592
15931593
Caused by:
15941594
Feature `bar` includes `baz` which is not defined as a feature.
15951595
A non-optional dependency of the same name is defined; consider adding `optional = true` to its definition
15961596
",
1597-
).run(
1598-
);
1597+
).run();
15991598
}
16001599

16011600
#[cargo_test]
@@ -2190,3 +2189,47 @@ fn registry_summary_order_doesnt_matter() {
21902189
.with_stdout("it works")
21912190
.run();
21922191
}
2192+
2193+
#[cargo_test]
2194+
fn nonexistent_required_features() {
2195+
Package::new("required_dependency", "0.1.0")
2196+
.feature("simple", &[])
2197+
.publish();
2198+
Package::new("optional_dependency", "0.2.0")
2199+
.feature("optional", &[])
2200+
.publish();
2201+
let p = project()
2202+
.file(
2203+
"Cargo.toml",
2204+
r#"
2205+
[project]
2206+
name = "foo"
2207+
version = "0.1.0"
2208+
[features]
2209+
existing = []
2210+
fancy = ["optional_dependency"]
2211+
[dependencies]
2212+
required_dependency = { version = "0.1", optional = false}
2213+
optional_dependency = { version = "0.2", optional = true}
2214+
[[example]]
2215+
name = "ololo"
2216+
required-features = ["not_present",
2217+
"existing",
2218+
"fancy",
2219+
"required_dependency/not_existing",
2220+
"required_dependency/simple",
2221+
"optional_dependency/optional",
2222+
"not_specified_dependency/some_feature"]"#,
2223+
)
2224+
.file("src/main.rs", "fn main() {}")
2225+
.file("examples/ololo.rs", "fn main() {}")
2226+
.build();
2227+
2228+
p.cargo("build --examples")
2229+
.with_stderr_contains(
2230+
r#"warning: feature `not_present` is not present in [features] section.
2231+
warning: feature `not_existing` does not exist in package `required_dependency v0.1.0`.
2232+
warning: dependency `not_specified_dependency` specified in required-features as `not_specified_dependency/some_feature` does not exist."#,
2233+
)
2234+
.run();
2235+
}

0 commit comments

Comments
 (0)