Skip to content

Commit 64efcbe

Browse files
Mark-SimulacrumMark-Simulacrum
committed
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 #70918) to fix the regression noted in #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.
1 parent cf9cf7c commit 64efcbe

18 files changed

+126
-234
lines changed

‎compiler/rustc_lint/src/levels.rs

+21-49
Original file line numberDiff line numberDiff line change
@@ -106,18 +106,32 @@ impl<'s> LintLevelsBuilder<'s> {
106106
id: LintId,
107107
(level, src): LevelSource,
108108
) {
109-
if let Some((old_level, old_src)) = specs.get(&id) {
110-
if old_level == &Level::Forbid && level != Level::Forbid {
109+
// Setting to a non-forbid level is an error if the lint previously had
110+
// a forbid level. Note that this is not necessarily true even with a
111+
// `#[forbid(..)]` attribute present, as that is overriden by `--cap-lints`.
112+
//
113+
// This means that this only errors if we're truly lowering the lint
114+
// level from forbid.
115+
if level != Level::Forbid {
116+
if let (Level::Forbid, old_src) =
117+
self.sets.get_lint_level(id.lint, self.cur, Some(&specs), &self.sess)
118+
{
111119
let mut diag_builder = struct_span_err!(
112120
self.sess,
113121
src.span(),
114122
E0453,
115-
"{}({}) incompatible with previous forbid in same scope",
123+
"{}({}) incompatible with previous forbid",
116124
level.as_str(),
117125
src.name(),
118126
);
119-
match *old_src {
120-
LintSource::Default => {}
127+
diag_builder.span_label(src.span(), "overruled by previous forbid");
128+
match old_src {
129+
LintSource::Default => {
130+
diag_builder.note(&format!(
131+
"`forbid` lint level is the default for {}",
132+
id.to_string()
133+
));
134+
}
121135
LintSource::Node(_, forbid_source_span, reason) => {
122136
diag_builder.span_label(forbid_source_span, "`forbid` level set here");
123137
if let Some(rationale) = reason {
@@ -129,6 +143,8 @@ impl<'s> LintLevelsBuilder<'s> {
129143
}
130144
}
131145
diag_builder.emit();
146+
147+
// Retain the forbid lint level
132148
return;
133149
}
134150
}
@@ -412,50 +428,6 @@ impl<'s> LintLevelsBuilder<'s> {
412428
}
413429
}
414430

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