Prevent forbid from being ignored if overriden at the same level.
That is, this changes `#[forbid(foo)] #[allow(foo)]` from allowing foo to forbidding foo.
This commit is contained in:
Felix S. Klock II
Mark Rousskov
parent
b4e77d21bc
commit
afa2a67545
6 changed files with 144 additions and 18 deletions
|
|
@ -10,6 +10,7 @@ use rustc_hir as hir;
|
|||
use rustc_hir::def_id::{CrateNum, LOCAL_CRATE};
|
||||
use rustc_hir::{intravisit, HirId};
|
||||
use rustc_middle::hir::map::Map;
|
||||
use rustc_middle::lint::LevelSource;
|
||||
use rustc_middle::lint::LintDiagnosticBuilder;
|
||||
use rustc_middle::lint::{struct_lint_level, LintLevelMap, LintLevelSets, LintSet, LintSource};
|
||||
use rustc_middle::ty::query::Providers;
|
||||
|
|
@ -95,6 +96,44 @@ impl<'s> LintLevelsBuilder<'s> {
|
|||
self.sets.list.push(LintSet::CommandLine { specs });
|
||||
}
|
||||
|
||||
/// Attempts to insert the `id` to `level_src` map entry. If unsuccessful
|
||||
/// (e.g. if a forbid was already inserted on the same scope), then emits a
|
||||
/// diagnostic with no change to `specs`.
|
||||
fn insert_spec(
|
||||
&mut self,
|
||||
specs: &mut FxHashMap<LintId, LevelSource>,
|
||||
id: LintId,
|
||||
(level, src): LevelSource,
|
||||
) {
|
||||
if let Some((old_level, old_src)) = specs.get(&id) {
|
||||
if old_level == &Level::Forbid && level != Level::Forbid {
|
||||
let mut diag_builder = struct_span_err!(
|
||||
self.sess,
|
||||
src.span(),
|
||||
E0453,
|
||||
"{}({}) incompatible with previous forbid in same scope",
|
||||
level.as_str(),
|
||||
src.name(),
|
||||
);
|
||||
match *old_src {
|
||||
LintSource::Default => {}
|
||||
LintSource::Node(_, forbid_source_span, reason) => {
|
||||
diag_builder.span_label(forbid_source_span, "`forbid` level set here");
|
||||
if let Some(rationale) = reason {
|
||||
diag_builder.note(&rationale.as_str());
|
||||
}
|
||||
}
|
||||
LintSource::CommandLine(_) => {
|
||||
diag_builder.note("`forbid` lint level was set on command line");
|
||||
}
|
||||
}
|
||||
diag_builder.emit();
|
||||
return;
|
||||
}
|
||||
}
|
||||
specs.insert(id, (level, src));
|
||||
}
|
||||
|
||||
/// Pushes a list of AST lint attributes onto this context.
|
||||
///
|
||||
/// This function will return a `BuilderPush` object which should be passed
|
||||
|
|
@ -109,7 +148,7 @@ impl<'s> LintLevelsBuilder<'s> {
|
|||
/// `#[allow]`
|
||||
///
|
||||
/// Don't forget to call `pop`!
|
||||
pub fn push(
|
||||
pub(crate) fn push(
|
||||
&mut self,
|
||||
attrs: &[ast::Attribute],
|
||||
store: &LintStore,
|
||||
|
|
@ -221,7 +260,7 @@ impl<'s> LintLevelsBuilder<'s> {
|
|||
let src = LintSource::Node(name, li.span(), reason);
|
||||
for &id in ids {
|
||||
self.check_gated_lint(id, attr.span);
|
||||
specs.insert(id, (level, src));
|
||||
self.insert_spec(&mut specs, id, (level, src));
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -235,7 +274,7 @@ impl<'s> LintLevelsBuilder<'s> {
|
|||
reason,
|
||||
);
|
||||
for id in ids {
|
||||
specs.insert(*id, (level, src));
|
||||
self.insert_spec(&mut specs, *id, (level, src));
|
||||
}
|
||||
}
|
||||
Err((Some(ids), new_lint_name)) => {
|
||||
|
|
@ -272,7 +311,7 @@ impl<'s> LintLevelsBuilder<'s> {
|
|||
reason,
|
||||
);
|
||||
for id in ids {
|
||||
specs.insert(*id, (level, src));
|
||||
self.insert_spec(&mut specs, *id, (level, src));
|
||||
}
|
||||
}
|
||||
Err((None, _)) => {
|
||||
|
|
|
|||
|
|
@ -9,7 +9,7 @@ use rustc_session::lint::{builtin, Level, Lint, LintId};
|
|||
use rustc_session::{DiagnosticMessageId, Session};
|
||||
use rustc_span::hygiene::MacroKind;
|
||||
use rustc_span::source_map::{DesugaringKind, ExpnKind, MultiSpan};
|
||||
use rustc_span::{Span, Symbol};
|
||||
use rustc_span::{symbol, Span, Symbol, DUMMY_SP};
|
||||
|
||||
/// How a lint level was set.
|
||||
#[derive(Clone, Copy, PartialEq, Eq, HashStable)]
|
||||
|
|
@ -25,6 +25,24 @@ pub enum LintSource {
|
|||
CommandLine(Symbol),
|
||||
}
|
||||
|
||||
impl LintSource {
|
||||
pub fn name(&self) -> Symbol {
|
||||
match *self {
|
||||
LintSource::Default => symbol::kw::Default,
|
||||
LintSource::Node(name, _, _) => name,
|
||||
LintSource::CommandLine(name) => name,
|
||||
}
|
||||
}
|
||||
|
||||
pub fn span(&self) -> Span {
|
||||
match *self {
|
||||
LintSource::Default => DUMMY_SP,
|
||||
LintSource::Node(_, span, _) => span,
|
||||
LintSource::CommandLine(_) => DUMMY_SP,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
pub type LevelSource = (Level, LintSource);
|
||||
|
||||
pub struct LintLevelSets {
|
||||
|
|
|
|||
|
|
@ -0,0 +1,49 @@
|
|||
// This test is checking that you cannot override a `forbid` by adding in other
|
||||
// attributes later in the same scope. (We already ensure that you cannot
|
||||
// override it in nested scopes).
|
||||
|
||||
// If you turn off deduplicate diagnostics (which rustc turns on by default but
|
||||
// compiletest turns off when it runs ui tests), then the errors are
|
||||
// (unfortunately) repeated here because the checking is done as we read in the
|
||||
// errors, and curretly that happens two or three different times, depending on
|
||||
// compiler flags.
|
||||
//
|
||||
// I decided avoiding the redundant output was not worth the time in engineering
|
||||
// effort for bug like this, which 1. end users are unlikely to run into in the
|
||||
// first place, and 2. they won't see the redundant output anyway.
|
||||
|
||||
// compile-flags: -Z deduplicate-diagnostics=yes
|
||||
|
||||
fn forbid_first(num: i32) -> i32 {
|
||||
#![forbid(unused)]
|
||||
#![deny(unused)]
|
||||
//~^ ERROR: deny(unused) incompatible with previous forbid in same scope [E0453]
|
||||
#![warn(unused)]
|
||||
//~^ ERROR: warn(unused) incompatible with previous forbid in same scope [E0453]
|
||||
#![allow(unused)]
|
||||
//~^ ERROR: allow(unused) incompatible with previous forbid in same scope [E0453]
|
||||
|
||||
num * num
|
||||
}
|
||||
|
||||
fn forbid_last(num: i32) -> i32 {
|
||||
#![deny(unused)]
|
||||
#![warn(unused)]
|
||||
#![allow(unused)]
|
||||
#![forbid(unused)]
|
||||
|
||||
num * num
|
||||
}
|
||||
|
||||
fn forbid_multiple(num: i32) -> i32 {
|
||||
#![forbid(unused)]
|
||||
#![forbid(unused)]
|
||||
|
||||
num * num
|
||||
}
|
||||
|
||||
fn main() {
|
||||
forbid_first(10);
|
||||
forbid_last(10);
|
||||
forbid_multiple(10);
|
||||
}
|
||||
|
|
@ -0,0 +1,29 @@
|
|||
error[E0453]: deny(unused) incompatible with previous forbid in same scope
|
||||
--> $DIR/issue-70819-dont-override-forbid-in-same-scope.rs:19:13
|
||||
|
|
||||
LL | #![forbid(unused)]
|
||||
| ------ `forbid` level set here
|
||||
LL | #![deny(unused)]
|
||||
| ^^^^^^
|
||||
|
||||
error[E0453]: warn(unused) incompatible with previous forbid in same scope
|
||||
--> $DIR/issue-70819-dont-override-forbid-in-same-scope.rs:21:13
|
||||
|
|
||||
LL | #![forbid(unused)]
|
||||
| ------ `forbid` level set here
|
||||
...
|
||||
LL | #![warn(unused)]
|
||||
| ^^^^^^
|
||||
|
||||
error[E0453]: allow(unused) incompatible with previous forbid in same scope
|
||||
--> $DIR/issue-70819-dont-override-forbid-in-same-scope.rs:23:14
|
||||
|
|
||||
LL | #![forbid(unused)]
|
||||
| ------ `forbid` level set here
|
||||
...
|
||||
LL | #![allow(unused)]
|
||||
| ^^^^^^
|
||||
|
||||
error: aborting due to 3 previous errors
|
||||
|
||||
For more information about this error, try `rustc --explain E0453`.
|
||||
|
|
@ -3,7 +3,6 @@
|
|||
// Test that the whole restriction group is not enabled
|
||||
#![warn(clippy::restriction)]
|
||||
#![deny(clippy::restriction)]
|
||||
#![forbid(clippy::restriction)]
|
||||
#![allow(clippy::missing_docs_in_private_items, clippy::panic, clippy::unreachable)]
|
||||
|
||||
#[inline(always)]
|
||||
|
|
|
|||
|
|
@ -1,5 +1,5 @@
|
|||
error: you have declared `#[inline(always)]` on `test_attr_lint`. This is usually a bad idea
|
||||
--> $DIR/attrs.rs:9:1
|
||||
--> $DIR/attrs.rs:8:1
|
||||
|
|
||||
LL | #[inline(always)]
|
||||
| ^^^^^^^^^^^^^^^^^
|
||||
|
|
@ -7,7 +7,7 @@ LL | #[inline(always)]
|
|||
= note: `-D clippy::inline-always` implied by `-D warnings`
|
||||
|
||||
error: the since field must contain a semver-compliant version
|
||||
--> $DIR/attrs.rs:29:14
|
||||
--> $DIR/attrs.rs:28:14
|
||||
|
|
||||
LL | #[deprecated(since = "forever")]
|
||||
| ^^^^^^^^^^^^^^^^^
|
||||
|
|
@ -15,7 +15,7 @@ LL | #[deprecated(since = "forever")]
|
|||
= note: `-D clippy::deprecated-semver` implied by `-D warnings`
|
||||
|
||||
error: the since field must contain a semver-compliant version
|
||||
--> $DIR/attrs.rs:32:14
|
||||
--> $DIR/attrs.rs:31:14
|
||||
|
|
||||
LL | #[deprecated(since = "1")]
|
||||
| ^^^^^^^^^^^
|
||||
|
|
@ -37,13 +37,5 @@ LL | #![deny(clippy::restriction)]
|
|||
|
|
||||
= help: try enabling only the lints you really need
|
||||
|
||||
error: restriction lints are not meant to be all enabled
|
||||
--> $DIR/attrs.rs:6:11
|
||||
|
|
||||
LL | #![forbid(clippy::restriction)]
|
||||
| ^^^^^^^^^^^^^^^^^^^
|
||||
|
|
||||
= help: try enabling only the lints you really need
|
||||
|
||||
error: aborting due to 6 previous errors
|
||||
error: aborting due to 5 previous errors
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue