about summary refs log tree commit diff
path: root/compiler/rustc_lint/src
diff options
context:
space:
mode:
authorMark Rousskov <mark.simulacrum@gmail.com>2020-11-07 18:14:38 -0500
committerMark Rousskov <mark.simulacrum@gmail.com>2020-11-14 15:56:07 -0500
commit64efcbe0e9e0e2dc02960031e6df4fe7450782ca (patch)
tree77db9d6b4940a2c4e2b83f395580a7739e0e9505 /compiler/rustc_lint/src
parentcf9cf7c923eb01146971429044f216a3ca905e06 (diff)
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.
Diffstat (limited to 'compiler/rustc_lint/src')
-rw-r--r--compiler/rustc_lint/src/levels.rs70
1 files changed, 21 insertions, 49 deletions
diff --git a/compiler/rustc_lint/src/levels.rs b/compiler/rustc_lint/src/levels.rs
index aca28988364..d2a5875cb2f 100644
--- a/compiler/rustc_lint/src/levels.rs
+++ b/compiler/rustc_lint/src/levels.rs
@@ -106,18 +106,32 @@ impl<'s> LintLevelsBuilder<'s> {
         id: LintId,
         (level, src): LevelSource,
     ) {
-        if let Some((old_level, old_src)) = specs.get(&id) {
-            if old_level == &Level::Forbid && level != Level::Forbid {
+        // Setting to a non-forbid level is an error if the lint previously had
+        // a forbid level. Note that this is not necessarily true even with a
+        // `#[forbid(..)]` attribute present, as that is overriden by `--cap-lints`.
+        //
+        // This means that this only errors if we're truly lowering the lint
+        // level from forbid.
+        if level != Level::Forbid {
+            if let (Level::Forbid, old_src) =
+                self.sets.get_lint_level(id.lint, self.cur, Some(&specs), &self.sess)
+            {
                 let mut diag_builder = struct_span_err!(
                     self.sess,
                     src.span(),
                     E0453,
-                    "{}({}) incompatible with previous forbid in same scope",
+                    "{}({}) incompatible with previous forbid",
                     level.as_str(),
                     src.name(),
                 );
-                match *old_src {
-                    LintSource::Default => {}
+                diag_builder.span_label(src.span(), "overruled by previous forbid");
+                match old_src {
+                    LintSource::Default => {
+                        diag_builder.note(&format!(
+                            "`forbid` lint level is the default for {}",
+                            id.to_string()
+                        ));
+                    }
                     LintSource::Node(_, forbid_source_span, reason) => {
                         diag_builder.span_label(forbid_source_span, "`forbid` level set here");
                         if let Some(rationale) = reason {
@@ -129,6 +143,8 @@ impl<'s> LintLevelsBuilder<'s> {
                     }
                 }
                 diag_builder.emit();
+
+                // Retain the forbid lint level
                 return;
             }
         }
@@ -412,50 +428,6 @@ impl<'s> LintLevelsBuilder<'s> {
             }
         }
 
-        for (id, &(level, ref src)) in specs.iter() {
-            if level == Level::Forbid {
-                continue;
-            }
-            let forbid_src = match self.sets.get_lint_id_level(*id, self.cur, None) {
-                (Some(Level::Forbid), src) => src,
-                _ => continue,
-            };
-            let forbidden_lint_name = match forbid_src {
-                LintSource::Default => id.to_string(),
-                LintSource::Node(name, _, _) => name.to_string(),
-                LintSource::CommandLine(name, _) => name.to_string(),
-            };
-            let (lint_attr_name, lint_attr_span) = match *src {
-                LintSource::Node(name, span, _) => (name, span),
-                _ => continue,
-            };
-            let mut diag_builder = struct_span_err!(
-                self.sess,
-                lint_attr_span,
-                E0453,
-                "{}({}) overruled by outer forbid({})",
-                level.as_str(),
-                lint_attr_name,
-                forbidden_lint_name
-            );
-            diag_builder.span_label(lint_attr_span, "overruled by previous forbid");
-            match forbid_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();
-            // don't set a separate error for every lint in the group
-            break;
-        }
-
         let prev = self.cur;
         if !specs.is_empty() {
             self.cur = self.sets.list.len() as u32;