Skip to content

Commit 46ba901

Browse files
committed
Auto merge of #9549 - Bryysen:master, r=ehuss
Warn if an "all" target is specified, but we don't match anything If a combination of --bins, --benches, --examples, --tests flags have been specified, but we didn't match on anything after resolving the unit-list, we emit a warning to make it clear that cargo didn't do anything and that the code is unchecked. This is my first PR and there are a couple things that I'm unsure about * The integration test covers only one case (ideally it should cover every combination of the above mentioned flags the user can pass). I figured since the warning function is so simple, it'd best not to clog the testsuite with unnecessary `p.cargo().runs()` and whatnot. If I should make the test more comprehensive I can do that, it's also very easy to write unit tests so i can do that as well if needed. * I figure we don't actually have to check the `--all-targets`, but i'm doing so for consistency. I also didn't check for the `--lib` flag at all because (I'm assuming) if the user passes `--lib` and there are no libraries, we error. Edit: I notice (among other things) we sometimes silently skip certain units that have incompatible feature flags (see [here](https://github.com/rust-lang/cargo/blob/ed0c8c6d66e36fafbce4f78907a110838797ae39/src/cargo/ops/cargo_compile.rs#L1140)) so maybe we should be checking the `--lib` flag after all, in the event that a library was silently skipped and we no-opped 🤔 And thanks to `@ehuss` for taking the time to answer my questions and helping me through the contribution process, much appreciated Closes #9536
2 parents 66686fd + da1c2f3 commit 46ba901

File tree

2 files changed

+81
-0
lines changed

2 files changed

+81
-0
lines changed

src/cargo/ops/cargo_compile.rs

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1140,11 +1140,63 @@ fn generate_targets(
11401140
// else, silently skip target.
11411141
}
11421142
let mut units: Vec<_> = units.into_iter().collect();
1143+
unmatched_target_filters(&units, &filter, &mut ws.config().shell())?;
1144+
11431145
// Keep the roots in a consistent order, which helps with checking test output.
11441146
units.sort_unstable();
11451147
Ok(units)
11461148
}
11471149

1150+
/// Checks if the unit list is empty and the user has passed any combination of
1151+
/// --tests, --examples, --benches or --bins, and we didn't match on any targets.
1152+
/// We want to emit a warning to make sure the user knows that this run is a no-op,
1153+
/// and their code remains unchecked despite cargo not returning any errors
1154+
fn unmatched_target_filters(
1155+
units: &[Unit],
1156+
filter: &CompileFilter,
1157+
shell: &mut Shell,
1158+
) -> CargoResult<()> {
1159+
if let CompileFilter::Only {
1160+
all_targets,
1161+
lib: _,
1162+
ref bins,
1163+
ref examples,
1164+
ref tests,
1165+
ref benches,
1166+
} = *filter
1167+
{
1168+
if units.is_empty() {
1169+
let mut filters = String::new();
1170+
let mut miss_count = 0;
1171+
1172+
let mut append = |t: &FilterRule, s| {
1173+
if let FilterRule::All = *t {
1174+
miss_count += 1;
1175+
filters.push_str(s);
1176+
}
1177+
};
1178+
1179+
if all_targets {
1180+
filters.push_str(" `all-targets`");
1181+
} else {
1182+
append(bins, " `bins`,");
1183+
append(tests, " `tests`,");
1184+
append(examples, " `examples`,");
1185+
append(benches, " `benches`,");
1186+
filters.pop();
1187+
}
1188+
1189+
return shell.warn(format!(
1190+
"Target {}{} specified, but no targets matched. This is a no-op",
1191+
if miss_count > 1 { "filters" } else { "filter" },
1192+
filters,
1193+
));
1194+
}
1195+
}
1196+
1197+
Ok(())
1198+
}
1199+
11481200
/// Warns if a target's required-features references a feature that doesn't exist.
11491201
///
11501202
/// This is a warning because historically this was not validated, and it

tests/testsuite/cargo_targets.rs

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,35 @@
22
33
use cargo_test_support::project;
44

5+
#[cargo_test]
6+
fn warn_unmatched_target_filters() {
7+
let p = project()
8+
.file(
9+
"Cargo.toml",
10+
r#"
11+
[package]
12+
name = "foo"
13+
version = "0.1.0"
14+
15+
[lib]
16+
test = false
17+
bench = false
18+
"#,
19+
)
20+
.file("src/lib.rs", r#"fn main() {}"#)
21+
.build();
22+
23+
p.cargo("check --tests --bins --examples --benches")
24+
.with_stderr(
25+
"\
26+
[WARNING] Target filters `bins`, `tests`, `examples`, `benches` specified, \
27+
but no targets matched. This is a no-op
28+
[FINISHED][..]
29+
",
30+
)
31+
.run();
32+
}
33+
534
#[cargo_test]
635
fn reserved_windows_target_name() {
736
let p = project()

0 commit comments

Comments
 (0)