Skip to content

Commit eb4860c

Browse files
borsbors
committed
Auto merge of rust-lang#78864 - Mark-Simulacrum:warn-on-forbids, r=pnkfelix
Use true previous lint level when detecting overriden forbids Previously, cap-lints was ignored when checking the previous forbid level, which meant that it was a hard error to do so. This is different from the normal behavior of lints, which are silenced by cap-lints; if the forbid would not take effect regardless, there is not much point in complaining about the fact that we are reducing its level. It might be considered a bug that even `--cap-lints deny` would suffice to silence the error on overriding forbid, depending on if one cares about failing the build or precisely forbid being set. But setting cap-lints to deny is quite odd and not really done in practice, so we don't try to handle it specially. This also unifies the code paths for nested and same-level scopes. However, the special case for CLI lint flags is left in place (introduced by rust-lang#70918) to fix the regression noted in rust-lang#70819. That means that CLI flags do not lint on forbid being overridden by a non-forbid level. It is unclear whether this is a bug or a desirable feature, but it is certainly inconsistent. CLI flags are a sufficiently different "type" of place though that this is deemed out of scope for this commit. r? `@pnkfelix` perhaps? cc rust-lang#77713 -- not marking as "Fixes" because of the lack of proper unused attribute handling in this PR
2 parents 18aa5ee + 64efcbe commit eb4860c

18 files changed

+126
-234
lines changed

‎compiler/rustc_lint/src/levels.rs

+21-49
Original file line numberDiff line numberDiff line change
@@ -108,18 +108,32 @@ impl<'s> LintLevelsBuilder<'s> {
108108
id: LintId,
109109
(level, src): LevelSource,
110110
) {
111-
if let Some((old_level, old_src)) = specs.get(&id) {
112-
if old_level == &Level::Forbid && level != Level::Forbid {
111+
// Setting to a non-forbid level is an error if the lint previously had
112+
// a forbid level. Note that this is not necessarily true even with a
113+
// `#[forbid(..)]` attribute present, as that is overriden by `--cap-lints`.
114+
//
115+
// This means that this only errors if we're truly lowering the lint
116+
// level from forbid.
117+
if level != Level::Forbid {
118+
if let (Level::Forbid, old_src) =
119+
self.sets.get_lint_level(id.lint, self.cur, Some(&specs), &self.sess)
120+
{
113121
let mut diag_builder = struct_span_err!(
114122
self.sess,
115123
src.span(),
116124
E0453,
117-
"{}({}) incompatible with previous forbid in same scope",
125+
"{}({}) incompatible with previous forbid",
118126
level.as_str(),
119127
src.name(),
120128
);
121-
match *old_src {
122-
LintSource::Default => {}
129+
diag_builder.span_label(src.span(), "overruled by previous forbid");
130+
match old_src {
131+
LintSource::Default => {
132+
diag_builder.note(&format!(
133+
"`forbid` lint level is the default for {}",
134+
id.to_string()
135+
));
136+
}
123137
LintSource::Node(_, forbid_source_span, reason) => {
124138
diag_builder.span_label(forbid_source_span, "`forbid` level set here");
125139
if let Some(rationale) = reason {
@@ -131,6 +145,8 @@ impl<'s> LintLevelsBuilder<'s> {
131145
}
132146
}
133147
diag_builder.emit();
148+
149+
// Retain the forbid lint level
134150
return;
135151
}
136152
}
@@ -414,50 +430,6 @@ impl<'s> LintLevelsBuilder<'s> {
414430
}
415431
}
416432

417-
for (id, &(level, ref src)) in specs.iter() {
418-
if level == Level::Forbid {
419-
continue;
420-
}
421-
let forbid_src = match self.sets.get_lint_id_level(*id, self.cur, None) {
422-
(Some(Level::Forbid), src) => src,
423-
_ => continue,
424-
};
425-
let forbidden_lint_name = match forbid_src {
426-
LintSource::Default => id.to_string(),
427-
LintSource::Node(name, _, _) => name.to_string(),
428-
LintSource::CommandLine(name, _) => name.to_string(),
429-
};
430-
let (lint_attr_name, lint_attr_span) = match *src {
431-
LintSource::Node(name, span, _) => (name, span),
432-
_ => continue,
433-
};
434-
let mut diag_builder = struct_span_err!(
435-
self.sess,
436-
lint_attr_span,
437-
E0453,
438-
"{}({}) overruled by outer forbid({})",
439-
level.as_str(),
440-
lint_attr_name,
441-
forbidden_lint_name
442-
);
443-
diag_builder.span_label(lint_attr_span, "overruled by previous forbid");
444-
match forbid_src {
445-
LintSource::Default => {}
446-
LintSource::Node(_, forbid_source_span, reason) => {
447-
diag_builder.span_label(forbid_source_span, "`forbid` level set here");
448-
if let Some(rationale) = reason {
449-
diag_builder.note(&rationale.as_str());
450-
}
451-
}
452-
LintSource::CommandLine(_, _) => {
453-
diag_builder.note("`forbid` lint level was set on command line");
454-
}
455-
}
456-
diag_builder.emit();
457-
// don't set a separate error for every lint in the group
458-
break;
459-
}
460-
461433
let prev = self.cur;
462434
if !specs.is_empty() {
463435
self.cur = self.sets.list.len() as u32;

‎src/test/ui-fulldeps/lint-plugin-forbid-attrs.rs

+4-4
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,12 @@
66
//~^ WARN use of deprecated attribute `plugin`
77
#![forbid(test_lint)]
88

9-
fn lintme() { } //~ ERROR item is named 'lintme'
9+
fn lintme() {} //~ ERROR item is named 'lintme'
1010

1111
#[allow(test_lint)]
12-
//~^ ERROR allow(test_lint) overruled by outer forbid(test_lint)
13-
//~| ERROR allow(test_lint) overruled by outer forbid(test_lint)
14-
//~| ERROR allow(test_lint) overruled by outer forbid(test_lint)
12+
//~^ ERROR allow(test_lint) incompatible
13+
//~| ERROR allow(test_lint) incompatible
14+
//~| ERROR allow(test_lint) incompatible
1515
pub fn main() {
1616
lintme();
1717
}

‎src/test/ui-fulldeps/lint-plugin-forbid-attrs.stderr

+5-5
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
error[E0453]: allow(test_lint) overruled by outer forbid(test_lint)
1+
error[E0453]: allow(test_lint) incompatible with previous forbid
22
--> $DIR/lint-plugin-forbid-attrs.rs:11:9
33
|
44
LL | #![forbid(test_lint)]
@@ -7,7 +7,7 @@ LL | #![forbid(test_lint)]
77
LL | #[allow(test_lint)]
88
| ^^^^^^^^^ overruled by previous forbid
99

10-
error[E0453]: allow(test_lint) overruled by outer forbid(test_lint)
10+
error[E0453]: allow(test_lint) incompatible with previous forbid
1111
--> $DIR/lint-plugin-forbid-attrs.rs:11:9
1212
|
1313
LL | #![forbid(test_lint)]
@@ -27,16 +27,16 @@ LL | #![plugin(lint_plugin_test)]
2727
error: item is named 'lintme'
2828
--> $DIR/lint-plugin-forbid-attrs.rs:9:1
2929
|
30-
LL | fn lintme() { }
31-
| ^^^^^^^^^^^^^^^
30+
LL | fn lintme() {}
31+
| ^^^^^^^^^^^^^^
3232
|
3333
note: the lint level is defined here
3434
--> $DIR/lint-plugin-forbid-attrs.rs:7:11
3535
|
3636
LL | #![forbid(test_lint)]
3737
| ^^^^^^^^^
3838

39-
error[E0453]: allow(test_lint) overruled by outer forbid(test_lint)
39+
error[E0453]: allow(test_lint) incompatible with previous forbid
4040
--> $DIR/lint-plugin-forbid-attrs.rs:11:9
4141
|
4242
LL | #![forbid(test_lint)]

‎src/test/ui-fulldeps/lint-plugin-forbid-cmdline.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,9 @@
77
//~^ WARN use of deprecated attribute `plugin`
88
fn lintme() { } //~ ERROR item is named 'lintme'
99

10-
#[allow(test_lint)] //~ ERROR allow(test_lint) overruled by outer forbid(test_lint)
11-
//~| ERROR allow(test_lint) overruled by outer forbid(test_lint)
12-
//~| ERROR allow(test_lint) overruled by outer forbid(test_lint)
10+
#[allow(test_lint)] //~ ERROR allow(test_lint) incompatible
11+
//~| ERROR allow(test_lint) incompatible
12+
//~| ERROR allow(test_lint)
1313
pub fn main() {
1414
lintme();
1515
}

‎src/test/ui-fulldeps/lint-plugin-forbid-cmdline.stderr

+3-3
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
1-
error[E0453]: allow(test_lint) overruled by outer forbid(test_lint)
1+
error[E0453]: allow(test_lint) incompatible with previous forbid
22
--> $DIR/lint-plugin-forbid-cmdline.rs:10:9
33
|
44
LL | #[allow(test_lint)]
55
| ^^^^^^^^^ overruled by previous forbid
66
|
77
= note: `forbid` lint level was set on command line
88

9-
error[E0453]: allow(test_lint) overruled by outer forbid(test_lint)
9+
error[E0453]: allow(test_lint) incompatible with previous forbid
1010
--> $DIR/lint-plugin-forbid-cmdline.rs:10:9
1111
|
1212
LL | #[allow(test_lint)]
@@ -30,7 +30,7 @@ LL | fn lintme() { }
3030
|
3131
= note: requested on the command line with `-F test-lint`
3232

33-
error[E0453]: allow(test_lint) overruled by outer forbid(test_lint)
33+
error[E0453]: allow(test_lint) incompatible with previous forbid
3434
--> $DIR/lint-plugin-forbid-cmdline.rs:10:9
3535
|
3636
LL | #[allow(test_lint)]

‎src/test/ui/error-codes/E0453.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
#![forbid(non_snake_case)]
22

33
#[allow(non_snake_case)]
4-
//~^ ERROR allow(non_snake_case) overruled by outer forbid(non_snake_case)
5-
//~| ERROR allow(non_snake_case) overruled by outer forbid(non_snake_case)
6-
//~| ERROR allow(non_snake_case) overruled by outer forbid(non_snake_case)
4+
//~^ ERROR allow(non_snake_case) incompatible
5+
//~| ERROR allow(non_snake_case) incompatible
6+
//~| ERROR allow(non_snake_case) incompatible
77
fn main() {
88
}

‎src/test/ui/error-codes/E0453.stderr

+3-3
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
error[E0453]: allow(non_snake_case) overruled by outer forbid(non_snake_case)
1+
error[E0453]: allow(non_snake_case) incompatible with previous forbid
22
--> $DIR/E0453.rs:3:9
33
|
44
LL | #![forbid(non_snake_case)]
@@ -7,7 +7,7 @@ LL |
77
LL | #[allow(non_snake_case)]
88
| ^^^^^^^^^^^^^^ overruled by previous forbid
99

10-
error[E0453]: allow(non_snake_case) overruled by outer forbid(non_snake_case)
10+
error[E0453]: allow(non_snake_case) incompatible with previous forbid
1111
--> $DIR/E0453.rs:3:9
1212
|
1313
LL | #![forbid(non_snake_case)]
@@ -16,7 +16,7 @@ LL |
1616
LL | #[allow(non_snake_case)]
1717
| ^^^^^^^^^^^^^^ overruled by previous forbid
1818

19-
error[E0453]: allow(non_snake_case) overruled by outer forbid(non_snake_case)
19+
error[E0453]: allow(non_snake_case) incompatible with previous forbid
2020
--> $DIR/E0453.rs:3:9
2121
|
2222
LL | #![forbid(non_snake_case)]

‎src/test/ui/lint/forbid-error-capped.rs

+15
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
// check-pass
2+
// compile-args: --cap-lints=warn -Fwarnings
3+
4+
// This checks that the forbid attribute checking is ignored when the forbidden
5+
// lint is capped.
6+
7+
#![forbid(warnings)]
8+
#![allow(unused)]
9+
10+
#[allow(unused)]
11+
mod bar {
12+
fn bar() {}
13+
}
14+
15+
fn main() {}

‎src/test/ui/lint/issue-70819-dont-override-forbid-in-same-scope.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,11 @@
1717
fn forbid_first(num: i32) -> i32 {
1818
#![forbid(unused)]
1919
#![deny(unused)]
20-
//~^ ERROR: deny(unused) incompatible with previous forbid in same scope [E0453]
20+
//~^ ERROR: deny(unused) incompatible with previous forbid
2121
#![warn(unused)]
22-
//~^ ERROR: warn(unused) incompatible with previous forbid in same scope [E0453]
22+
//~^ ERROR: warn(unused) incompatible with previous forbid
2323
#![allow(unused)]
24-
//~^ ERROR: allow(unused) incompatible with previous forbid in same scope [E0453]
24+
//~^ ERROR: allow(unused) incompatible with previous forbid
2525

2626
num * num
2727
}

‎src/test/ui/lint/issue-70819-dont-override-forbid-in-same-scope.stderr

+6-6
Original file line numberDiff line numberDiff line change
@@ -1,28 +1,28 @@
1-
error[E0453]: deny(unused) incompatible with previous forbid in same scope
1+
error[E0453]: deny(unused) incompatible with previous forbid
22
--> $DIR/issue-70819-dont-override-forbid-in-same-scope.rs:19:13
33
|
44
LL | #![forbid(unused)]
55
| ------ `forbid` level set here
66
LL | #![deny(unused)]
7-
| ^^^^^^
7+
| ^^^^^^ overruled by previous forbid
88

9-
error[E0453]: warn(unused) incompatible with previous forbid in same scope
9+
error[E0453]: warn(unused) incompatible with previous forbid
1010
--> $DIR/issue-70819-dont-override-forbid-in-same-scope.rs:21:13
1111
|
1212
LL | #![forbid(unused)]
1313
| ------ `forbid` level set here
1414
...
1515
LL | #![warn(unused)]
16-
| ^^^^^^
16+
| ^^^^^^ overruled by previous forbid
1717

18-
error[E0453]: allow(unused) incompatible with previous forbid in same scope
18+
error[E0453]: allow(unused) incompatible with previous forbid
1919
--> $DIR/issue-70819-dont-override-forbid-in-same-scope.rs:23:14
2020
|
2121
LL | #![forbid(unused)]
2222
| ------ `forbid` level set here
2323
...
2424
LL | #![allow(unused)]
25-
| ^^^^^^
25+
| ^^^^^^ overruled by previous forbid
2626

2727
error: aborting due to 3 previous errors
2828

‎src/test/ui/lint/lint-forbid-attr.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
#![forbid(deprecated)]
22

33
#[allow(deprecated)]
4-
//~^ ERROR allow(deprecated) overruled by outer forbid(deprecated)
5-
//~| ERROR allow(deprecated) overruled by outer forbid(deprecated)
6-
//~| ERROR allow(deprecated) overruled by outer forbid(deprecated)
4+
//~^ ERROR allow(deprecated) incompatible
5+
//~| ERROR allow(deprecated) incompatible
6+
//~| ERROR allow(deprecated) incompatible
77
fn main() {
88
}

‎src/test/ui/lint/lint-forbid-attr.stderr

+3-3
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
error[E0453]: allow(deprecated) overruled by outer forbid(deprecated)
1+
error[E0453]: allow(deprecated) incompatible with previous forbid
22
--> $DIR/lint-forbid-attr.rs:3:9
33
|
44
LL | #![forbid(deprecated)]
@@ -7,7 +7,7 @@ LL |
77
LL | #[allow(deprecated)]
88
| ^^^^^^^^^^ overruled by previous forbid
99

10-
error[E0453]: allow(deprecated) overruled by outer forbid(deprecated)
10+
error[E0453]: allow(deprecated) incompatible with previous forbid
1111
--> $DIR/lint-forbid-attr.rs:3:9
1212
|
1313
LL | #![forbid(deprecated)]
@@ -16,7 +16,7 @@ LL |
1616
LL | #[allow(deprecated)]
1717
| ^^^^^^^^^^ overruled by previous forbid
1818

19-
error[E0453]: allow(deprecated) overruled by outer forbid(deprecated)
19+
error[E0453]: allow(deprecated) incompatible with previous forbid
2020
--> $DIR/lint-forbid-attr.rs:3:9
2121
|
2222
LL | #![forbid(deprecated)]

‎src/test/ui/lint/lint-forbid-cmdline.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// compile-flags: -F deprecated
22

3-
#[allow(deprecated)] //~ ERROR allow(deprecated) overruled by outer forbid(deprecated)
4-
//~| ERROR allow(deprecated) overruled by outer forbid(deprecated)
5-
//~| ERROR allow(deprecated) overruled by outer forbid(deprecated)
3+
#[allow(deprecated)] //~ ERROR allow(deprecated) incompatible
4+
//~| ERROR allow(deprecated) incompatible
5+
//~| ERROR allow(deprecated) incompatible
66
fn main() {
77
}

‎src/test/ui/lint/lint-forbid-cmdline.stderr

+3-3
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,20 @@
1-
error[E0453]: allow(deprecated) overruled by outer forbid(deprecated)
1+
error[E0453]: allow(deprecated) incompatible with previous forbid
22
--> $DIR/lint-forbid-cmdline.rs:3:9
33
|
44
LL | #[allow(deprecated)]
55
| ^^^^^^^^^^ overruled by previous forbid
66
|
77
= note: `forbid` lint level was set on command line
88

9-
error[E0453]: allow(deprecated) overruled by outer forbid(deprecated)
9+
error[E0453]: allow(deprecated) incompatible with previous forbid
1010
--> $DIR/lint-forbid-cmdline.rs:3:9
1111
|
1212
LL | #[allow(deprecated)]
1313
| ^^^^^^^^^^ overruled by previous forbid
1414
|
1515
= note: `forbid` lint level was set on command line
1616

17-
error[E0453]: allow(deprecated) overruled by outer forbid(deprecated)
17+
error[E0453]: allow(deprecated) incompatible with previous forbid
1818
--> $DIR/lint-forbid-cmdline.rs:3:9
1919
|
2020
LL | #[allow(deprecated)]

‎src/test/ui/lint/outer-forbid.rs

+13-9
Original file line numberDiff line numberDiff line change
@@ -4,21 +4,25 @@
44
// subsequent allowance of a lint group containing it (here, `nonstandard_style`). See
55
// Issue #42873.
66

7+
// If you turn off deduplicate diagnostics (which rustc turns on by default but
8+
// compiletest turns off when it runs ui tests), then the errors are
9+
// (unfortunately) repeated here because the checking is done as we read in the
10+
// errors, and currently that happens two or three different times, depending on
11+
// compiler flags.
12+
//
13+
// The test is much cleaner if we deduplicate, though.
14+
15+
// compile-flags: -Z deduplicate-diagnostics=yes
16+
717
#![forbid(unused, non_snake_case)]
818

9-
#[allow(unused_variables)] //~ ERROR overruled
10-
//~| ERROR overruled
11-
//~| ERROR overruled
19+
#[allow(unused_variables)] //~ ERROR incompatible with previous
1220
fn foo() {}
1321

14-
#[allow(unused)] //~ ERROR overruled
15-
//~| ERROR overruled
16-
//~| ERROR overruled
22+
#[allow(unused)] //~ ERROR incompatible with previous
1723
fn bar() {}
1824

19-
#[allow(nonstandard_style)] //~ ERROR overruled
20-
//~| ERROR overruled
21-
//~| ERROR overruled
25+
#[allow(nonstandard_style)] //~ ERROR incompatible with previous
2226
fn main() {
2327
println!("hello forbidden world")
2428
}

0 commit comments

Comments
 (0)