Auto merge of #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 #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.

r? `@pnkfelix` perhaps?

cc #77713 -- not marking as "Fixes" because of the lack of proper unused attribute handling in this PR

1 files changed, 21 insertions, 49 deletions

diff --git a/compiler/rustc_lint/src/levels.rs b/compiler/rustc_lint/src/levels.rs
index 02da85d25d5..3e22eba15aa 100644
--- a/compiler/rustc_lint/src/levels.rs
+++ b/compiler/rustc_lint/src/levels.rs
@@ -108,18 +108,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 {
@@ -131,6 +145,8 @@ impl<'s> LintLevelsBuilder<'s> {
                     }
                 }
                 diag_builder.emit();
+
+                // Retain the forbid lint level
                 return;
             }
         }
@@ -414,50 +430,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;