Rollup merge of rust-lang#122416 - Zalathar:levels, r=petrochenkov

Various style improvements to `rustc_lint::levels`

While reading this file, I noticed a few opportunities to make things a little nicer:

- Replace some nested if-let with let-chains
- Tweak a match pattern to allow shorthand struct syntax
- Fuse an `is_empty` check with getting the last element
- Merge some common code that emits `MalformedAttribute` and continues
- Format `"{tool}::{name}"` in a way that's consistent with other match arms
- Replace if-let-else-panic with let-else
- Use early-exit to flatten a method body

Some of these changes cause indentation churn, so ignoring whitespace is recommended.

Author: matthiaskrgr

compiler/rustc_lint/src/levels.rs

@@ -103,11 +103,12 @@ impl LintLevelSets {
         mut idx: LintStackIndex,
         aux: Option<&FxIndexMap<LintId, LevelAndSource>>,
     ) -> (Option<Level>, LintLevelSource) {
-        if let Some(specs) = aux {
-            if let Some(&(level, src)) = specs.get(&id) {
-                return (Some(level), src);
-            }
+        if let Some(specs) = aux
+            && let Some(&(level, src)) = specs.get(&id)
+        {
+            return (Some(level), src);
         }
+
         loop {
             let LintSet { ref specs, parent } = self.list[idx];
             if let Some(&(level, src)) = specs.get(&id) {
@@ -177,7 +178,7 @@ fn shallow_lint_levels_on(tcx: TyCtxt<'_>, owner: hir::OwnerId) -> ShallowLintLe
         // There is only something to do if there are attributes at all.
         [] => {}
         // Most of the time, there is only one attribute. Avoid fetching HIR in that case.
-        [(local_id, _)] => levels.add_id(HirId { owner, local_id: *local_id }),
+        &[(local_id, _)] => levels.add_id(HirId { owner, local_id }),
         // Otherwise, we need to visit the attributes in source code order, so we fetch HIR and do
         // a standard visit.
         // FIXME(#102522) Just iterate on attrs once that iteration order matches HIR's.
@@ -643,63 +644,61 @@ impl<'s, P: LintLevelsProvider> LintLevelsBuilder<'s, P> {
         //
         // This means that this only errors if we're truly lowering the lint
         // level from forbid.
-        if self.lint_added_lints && level != Level::Forbid {
-            if let Level::Forbid = old_level {
-                // Backwards compatibility check:
-                //
-                // We used to not consider `forbid(lint_group)`
-                // as preventing `allow(lint)` for some lint `lint` in
-                // `lint_group`. For now, issue a future-compatibility
-                // warning for this case.
-                let id_name = id.lint.name_lower();
-                let fcw_warning = match old_src {
-                    LintLevelSource::Default => false,
-                    LintLevelSource::Node { name, .. } => self.store.is_lint_group(name),
-                    LintLevelSource::CommandLine(symbol, _) => self.store.is_lint_group(symbol),
-                };
-                debug!(
-                    "fcw_warning={:?}, specs.get(&id) = {:?}, old_src={:?}, id_name={:?}",
-                    fcw_warning,
-                    self.current_specs(),
-                    old_src,
-                    id_name
-                );
-                let sub = match old_src {
-                    LintLevelSource::Default => {
-                        OverruledAttributeSub::DefaultSource { id: id.to_string() }
-                    }
-                    LintLevelSource::Node { span, reason, .. } => {
-                        OverruledAttributeSub::NodeSource { span, reason }
-                    }
-                    LintLevelSource::CommandLine(_, _) => OverruledAttributeSub::CommandLineSource,
-                };
-                if !fcw_warning {
-                    self.sess.dcx().emit_err(OverruledAttribute {
-                        span: src.span(),
+        if self.lint_added_lints && level != Level::Forbid && old_level == Level::Forbid {
+            // Backwards compatibility check:
+            //
+            // We used to not consider `forbid(lint_group)`
+            // as preventing `allow(lint)` for some lint `lint` in
+            // `lint_group`. For now, issue a future-compatibility
+            // warning for this case.
+            let id_name = id.lint.name_lower();
+            let fcw_warning = match old_src {
+                LintLevelSource::Default => false,
+                LintLevelSource::Node { name, .. } => self.store.is_lint_group(name),
+                LintLevelSource::CommandLine(symbol, _) => self.store.is_lint_group(symbol),
+            };
+            debug!(
+                "fcw_warning={:?}, specs.get(&id) = {:?}, old_src={:?}, id_name={:?}",
+                fcw_warning,
+                self.current_specs(),
+                old_src,
+                id_name
+            );
+            let sub = match old_src {
+                LintLevelSource::Default => {
+                    OverruledAttributeSub::DefaultSource { id: id.to_string() }
+                }
+                LintLevelSource::Node { span, reason, .. } => {
+                    OverruledAttributeSub::NodeSource { span, reason }
+                }
+                LintLevelSource::CommandLine(_, _) => OverruledAttributeSub::CommandLineSource,
+            };
+            if !fcw_warning {
+                self.sess.dcx().emit_err(OverruledAttribute {
+                    span: src.span(),
+                    overruled: src.span(),
+                    lint_level: level.as_str(),
+                    lint_source: src.name(),
+                    sub,
+                });
+            } else {
+                self.emit_span_lint(
+                    FORBIDDEN_LINT_GROUPS,
+                    src.span().into(),
+                    OverruledAttributeLint {
                         overruled: src.span(),
                         lint_level: level.as_str(),
                         lint_source: src.name(),
                         sub,
-                    });
-                } else {
-                    self.emit_span_lint(
-                        FORBIDDEN_LINT_GROUPS,
-                        src.span().into(),
-                        OverruledAttributeLint {
-                            overruled: src.span(),
-                            lint_level: level.as_str(),
-                            lint_source: src.name(),
-                            sub,
-                        },
-                    );
-                }
+                    },
+                );
+            }
 
-                // Retain the forbid lint level, unless we are
-                // issuing a FCW. In the FCW case, we want to
-                // respect the new setting.
-                if !fcw_warning {
-                    return;
-                }
+            // Retain the forbid lint level, unless we are
+            // issuing a FCW. In the FCW case, we want to
+            // respect the new setting.
+            if !fcw_warning {
+                return;
             }
         }
 
@@ -770,15 +769,15 @@ impl<'s, P: LintLevelsProvider> LintLevelsBuilder<'s, P> {
 
             let Some(mut metas) = attr.meta_item_list() else { continue };
 
-            if metas.is_empty() {
+            // Check whether `metas` is empty, and get its last element.
+            let Some(tail_li) = metas.last() else {
                 // This emits the unused_attributes lint for `#[level()]`
                 continue;
-            }
+            };
 
             // Before processing the lint names, look for a reason (RFC 2383)
             // at the end.
             let mut reason = None;
-            let tail_li = &metas[metas.len() - 1];
             if let Some(item) = tail_li.meta_item() {
                 match item.kind {
                     ast::MetaItemKind::Word => {} // actual lint names handled later
@@ -834,21 +833,16 @@ impl<'s, P: LintLevelsProvider> LintLevelsBuilder<'s, P> {
                 let meta_item = match li {
                     ast::NestedMetaItem::MetaItem(meta_item) if meta_item.is_word() => meta_item,
                     _ => {
-                        if let Some(item) = li.meta_item() {
-                            if let ast::MetaItemKind::NameValue(_) = item.kind {
-                                if item.path == sym::reason {
-                                    sess.dcx().emit_err(MalformedAttribute {
-                                        span: sp,
-                                        sub: MalformedAttributeSub::ReasonMustComeLast(sp),
-                                    });
-                                    continue;
-                                }
-                            }
-                        }
-                        sess.dcx().emit_err(MalformedAttribute {
-                            span: sp,
-                            sub: MalformedAttributeSub::BadAttributeArgument(sp),
-                        });
+                        let sub = if let Some(item) = li.meta_item()
+                            && let ast::MetaItemKind::NameValue(_) = item.kind
+                            && item.path == sym::reason
+                        {
+                            MalformedAttributeSub::ReasonMustComeLast(sp)
+                        } else {
+                            MalformedAttributeSub::BadAttributeArgument(sp)
+                        };
+
+                        sess.dcx().emit_err(MalformedAttribute { span: sp, sub });
                         continue;
                     }
                 };
@@ -987,11 +981,7 @@ impl<'s, P: LintLevelsProvider> LintLevelsBuilder<'s, P> {
                     }
 
                     CheckLintNameResult::NoLint(suggestion) => {
-                        let name = if let Some(tool_ident) = tool_ident {
-                            format!("{}::{}", tool_ident.name, name)
-                        } else {
-                            name.to_string()
-                        };
+                        let name = tool_ident.map(|tool| format!("{tool}::{name}")).unwrap_or(name);
                         let suggestion = suggestion.map(|(replace, from_rustc)| {
                             UnknownLintSuggestion::WithSpan { suggestion: sp, replace, from_rustc }
                         });
@@ -1005,27 +995,24 @@ impl<'s, P: LintLevelsProvider> LintLevelsBuilder<'s, P> {
                 if let CheckLintNameResult::Renamed(new_name) = lint_result {
                     // Ignore any errors or warnings that happen because the new name is inaccurate
                     // NOTE: `new_name` already includes the tool name, so we don't have to add it again.
-                    if let CheckLintNameResult::Ok(ids) =
+                    let CheckLintNameResult::Ok(ids) =
                         self.store.check_lint_name(&new_name, None, self.registered_tools)
-                    {
-                        let src = LintLevelSource::Node {
-                            name: Symbol::intern(&new_name),
-                            span: sp,
-                            reason,
-                        };
-                        for &id in ids {
-                            if self.check_gated_lint(id, attr.span, false) {
-                                self.insert_spec(id, (level, src));
-                            }
-                        }
-                        if let Level::Expect(expect_id) = level {
-                            self.provider.push_expectation(
-                                expect_id,
-                                LintExpectation::new(reason, sp, false, tool_name),
-                            );
-                        }
-                    } else {
+                    else {
                         panic!("renamed lint does not exist: {new_name}");
+                    };
+
+                    let src =
+                        LintLevelSource::Node { name: Symbol::intern(&new_name), span: sp, reason };
+                    for &id in ids {
+                        if self.check_gated_lint(id, attr.span, false) {
+                            self.insert_spec(id, (level, src));
+                        }
+                    }
+                    if let Level::Expect(expect_id) = level {
+                        self.provider.push_expectation(
+                            expect_id,
+                            LintExpectation::new(reason, sp, false, tool_name),
+                        );
                     }
                 }
             }
@@ -1058,38 +1045,44 @@ impl<'s, P: LintLevelsProvider> LintLevelsBuilder<'s, P> {
     /// Returns `true` if the lint's feature is enabled.
     #[track_caller]
     fn check_gated_lint(&self, lint_id: LintId, span: Span, lint_from_cli: bool) -> bool {
-        if let Some(feature) = lint_id.lint.feature_gate {
-            if !self.features.active(feature) {
-                if self.lint_added_lints {
-                    let lint = builtin::UNKNOWN_LINTS;
-                    let (level, src) = self.lint_level(builtin::UNKNOWN_LINTS);
-                    // FIXME: make this translatable
-                    #[allow(rustc::diagnostic_outside_of_impl)]
-                    #[allow(rustc::untranslatable_diagnostic)]
-                    lint_level(
-                        self.sess,
+        let feature = if let Some(feature) = lint_id.lint.feature_gate
+            && !self.features.active(feature)
+        {
+            // Lint is behind a feature that is not enabled; eventually return false.
+            feature
+        } else {
+            // Lint is ungated or its feature is enabled; exit early.
+            return true;
+        };
+
+        if self.lint_added_lints {
+            let lint = builtin::UNKNOWN_LINTS;
+            let (level, src) = self.lint_level(builtin::UNKNOWN_LINTS);
+            // FIXME: make this translatable
+            #[allow(rustc::diagnostic_outside_of_impl)]
+            #[allow(rustc::untranslatable_diagnostic)]
+            lint_level(
+                self.sess,
+                lint,
+                level,
+                src,
+                Some(span.into()),
+                fluent::lint_unknown_gated_lint,
+                |lint| {
+                    lint.arg("name", lint_id.lint.name_lower());
+                    lint.note(fluent::lint_note);
+                    rustc_session::parse::add_feature_diagnostics_for_issue(
                         lint,
-                        level,
-                        src,
-                        Some(span.into()),
-                        fluent::lint_unknown_gated_lint,
-                        |lint| {
-                            lint.arg("name", lint_id.lint.name_lower());
-                            lint.note(fluent::lint_note);
-                            rustc_session::parse::add_feature_diagnostics_for_issue(
-                                lint,
-                                &self.sess,
-                                feature,
-                                GateIssue::Language,
-                                lint_from_cli,
-                            );
-                        },
+                        &self.sess,
+                        feature,
+                        GateIssue::Language,
+                        lint_from_cli,
                     );
-                }
-                return false;
-            }
+                },
+            );
         }
-        true
+
+        false
     }
 
     /// Find the lint level for a lint.