Streamline Level::from_{attr,symbol}

These methods return `Option<(Self, Option<LintExpectationId>)>`. But
all the call sites except one don't look at the
`Option<LintExpectationId>`.

This commit simplifies these methods to not return the
`Option<LintExpectationId>`. This means they no longer need to be passed
a closure to compute an `AttrId` (which is usually discarded anyway).
The commit also renames `from_attr` as `from_opt_symbol`, because it
takes an `Option<Symbol>`, not an `Attribute`.

These changes simplify all the call sites that don't need the
`Option<LintExpectationId>`, and also the one call site that does (in
`LintLevelsBuilder::add`): that call site no longer needs to do an
awkward destructuring, and can instead build the appropriate
`LintExpectationId` directly.

Author: nnethercote

compiler/rustc_lint/src/levels.rs

@@ -9,7 +9,6 @@ use rustc_hir as hir;
 use rustc_hir::HirId;
 use rustc_hir::intravisit::{self, Visitor};
 use rustc_index::IndexVec;
-use rustc_middle::bug;
 use rustc_middle::hir::nested_filter;
 use rustc_middle::lint::{
     LevelSpec, LintExpectation, LintLevelSource, ShallowLintLevelMap, emit_lint_base,
@@ -659,25 +658,23 @@ impl<'s, P: LintLevelsProvider> LintLevelsBuilder<'s, P> {
                 continue;
             }
 
-            let (level, lint_id) = match Level::from_attr(attr.name(), || attr.id()) {
+            let (level, lint_id) = match Level::from_opt_symbol(attr.name()) {
                 None => continue,
-                // This is the only lint level with a `LintExpectationId` that can be created from
-                // an attribute.
-                Some((Level::Expect, Some(unstable_id))) if let Some(hir_id) = source_hir_id => {
-                    let LintExpectationId::Unstable { lint_index: None, attr_id: _ } = unstable_id
-                    else {
-                        bug!("stable id Level::from_attr")
-                    };
-
-                    let stable_id = LintExpectationId::Stable {
-                        hir_id,
-                        attr_index: attr_index.try_into().unwrap(),
-                        lint_index: None,
+                // `Expect` is the only lint level with a `LintExpectationId` that can be created
+                // from an attribute.
+                Some(Level::Expect) => {
+                    let id = if let Some(hir_id) = source_hir_id {
+                        LintExpectationId::Stable {
+                            hir_id,
+                            attr_index: attr_index.try_into().unwrap(),
+                            lint_index: None,
+                        }
+                    } else {
+                        LintExpectationId::Unstable { attr_id: attr.id(), lint_index: None }
                     };
-
-                    (Level::Expect, Some(stable_id))
+                    (Level::Expect, Some(id))
                 }
-                Some((level, id)) => (level, id),
+                Some(level) => (level, None),
             };
 
             let Some(mut metas) = attr.meta_item_list() else { continue };

compiler/rustc_lint_defs/src/lib.rs

@@ -215,34 +215,19 @@ impl Level {
         }
     }
 
-    /// Converts an `Attribute` to a level.
-    pub fn from_attr(
-        attr_name: Option<Symbol>,
-        attr_id: impl Fn() -> AttrId,
-    ) -> Option<(Self, Option<LintExpectationId>)> {
-        attr_name.and_then(|name| Self::from_symbol(name, || Some(attr_id())))
+    /// Converts an `Option<Symbol>` to a level.
+    pub fn from_opt_symbol(s: Option<Symbol>) -> Option<Self> {
+        s.and_then(Self::from_symbol)
     }
 
     /// Converts a `Symbol` to a level.
-    pub fn from_symbol(
-        s: Symbol,
-        id: impl FnOnce() -> Option<AttrId>,
-    ) -> Option<(Self, Option<LintExpectationId>)> {
+    pub fn from_symbol(s: Symbol) -> Option<Self> {
         match s {
-            sym::allow => Some((Level::Allow, None)),
-            sym::expect => {
-                if let Some(attr_id) = id() {
-                    Some((
-                        Level::Expect,
-                        Some(LintExpectationId::Unstable { attr_id, lint_index: None }),
-                    ))
-                } else {
-                    None
-                }
-            }
-            sym::warn => Some((Level::Warn, None)),
-            sym::deny => Some((Level::Deny, None)),
-            sym::forbid => Some((Level::Forbid, None)),
+            sym::allow => Some(Level::Allow),
+            sym::expect => Some(Level::Expect),
+            sym::warn => Some(Level::Warn),
+            sym::deny => Some(Level::Deny),
+            sym::forbid => Some(Level::Forbid),
             _ => None,
         }
     }

compiler/rustc_mir_build/src/builder/scope.rs

@@ -1302,7 +1302,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                 .tcx
                 .hir_attrs(id)
                 .iter()
-                .any(|attr| Level::from_attr(attr.name(), || attr.id()).is_some())
+                .any(|attr| Level::from_opt_symbol(attr.name()).is_some())
             {
                 // This is a rare case. It's for a node path that doesn't reach the root due to an
                 // intervening lint level attribute. This result doesn't get cached.

src/tools/clippy/clippy_lints/src/attrs/mod.rs

@@ -583,7 +583,7 @@ impl EarlyLintPass for PostExpansionEarlyAttributes {
             if matches!(name, sym::allow | sym::expect) && self.msrv.meets(msrvs::LINT_REASONS_STABILIZATION) {
                 allow_attributes_without_reason::check(cx, name, items, attr);
             }
-            if is_lint_level(name, attr.id) {
+            if is_lint_level(name) {
                 blanket_clippy_restriction_lints::check(cx, name, items);
             }
             if items.is_empty() || !attr.has_name(sym::deprecated) {

src/tools/clippy/clippy_lints/src/attrs/unnecessary_clippy_cfg.rs

@@ -15,7 +15,7 @@ pub(super) fn check(
 ) {
     if cfg_attr.has_name(sym::clippy)
         && let Some(ident) = behind_cfg_attr.ident()
-        && Level::from_symbol(ident.name, || Some(attr.id)).is_some()
+        && Level::from_symbol(ident.name).is_some()
         && let Some(items) = behind_cfg_attr.meta_item_list()
     {
         let nb_items = items.len();

src/tools/clippy/clippy_lints/src/attrs/useless_attribute.rs

@@ -15,7 +15,7 @@ pub(super) fn check(cx: &EarlyContext<'_>, item: &Item, attrs: &[Attribute]) {
             return;
         }
         if let Some(lint_list) = &attr.meta_item_list()
-            && attr.name().is_some_and(|name| is_lint_level(name, attr.id))
+            && attr.name().is_some_and(|name| is_lint_level(name))
         {
             for lint in lint_list {
                 match item.kind {

src/tools/clippy/clippy_lints/src/attrs/utils.rs

@@ -1,5 +1,5 @@
 use clippy_utils::macros::{is_panic, macro_backtrace};
-use rustc_ast::{AttrId, MetaItemInner};
+use rustc_ast::MetaItemInner;
 use rustc_hir::{
     Block, Expr, ExprKind, ImplItem, ImplItemKind, Item, ItemKind, StmtKind, TraitFn, TraitItem, TraitItemKind,
 };
@@ -16,8 +16,8 @@ pub(super) fn is_word(nmi: &MetaItemInner, expected: Symbol) -> bool {
     }
 }
 
-pub(super) fn is_lint_level(symbol: Symbol, attr_id: AttrId) -> bool {
-    Level::from_symbol(symbol, || Some(attr_id)).is_some()
+pub(super) fn is_lint_level(symbol: Symbol) -> bool {
+    Level::from_symbol(symbol).is_some()
 }
 
 pub(super) fn is_relevant_item(cx: &LateContext<'_>, item: &Item<'_>) -> bool {

src/tools/clippy/clippy_lints/src/collapsible_if.rs

@@ -238,7 +238,7 @@ impl CollapsibleIf {
             },
 
             [attr]
-                if matches!(Level::from_attr(attr.name(), || attr.id()), Some((Level::Expect, _)))
+                if matches!(Level::from_opt_symbol(attr.name()), Some(Level::Expect))
                     && let Some(metas) = attr.meta_item_list()
                     && let Some(MetaItemInner::MetaItem(meta_item)) = metas.first()
                     && let [tool, lint_name] = meta_item.path.segments.as_slice()

src/tools/clippy/clippy_lints/src/returns/needless_return.rs

@@ -181,7 +181,7 @@ fn check_final_expr<'tcx>(
             match cx.tcx.hir_attrs(expr.hir_id) {
                 [] => {},
                 [attr] => {
-                    if matches!(Level::from_attr(attr.name(), || attr.id()), Some((Level::Expect, _)))
+                    if matches!(Level::from_opt_symbol(attr.name()), Some(Level::Expect))
                         && let metas = attr.meta_item_list()
                         && let Some(lst) = metas
                         && let [MetaItemInner::MetaItem(meta_item), ..] = lst.as_slice()