Auto merge of #125057 - nnethercote:rustc_lint-cleanups, r=cjgillot

`rustc_lint` cleanups

Minor improvements I found while looking through this code.

r? `@cjgillot`

Author: bors

compiler/rustc_lint/src/builtin.rs

@@ -1494,8 +1494,9 @@ impl<'tcx> LateLintPass<'tcx> for TypeAliasBounds {
 
         let ty = cx.tcx.type_of(item.owner_id).skip_binder();
         if ty.has_inherent_projections() {
-            // Bounds of type aliases that contain opaque types or inherent projections are respected.
-            // E.g: `type X = impl Trait;`, `type X = (impl Trait, Y);`, `type X = Type::Inherent;`.
+            // Bounds of type aliases that contain opaque types or inherent projections are
+            // respected. E.g: `type X = impl Trait;`, `type X = (impl Trait, Y);`, `type X =
+            // Type::Inherent;`.
             return;
         }
 
@@ -2224,7 +2225,8 @@ impl<'tcx> LateLintPass<'tcx> for ExplicitOutlivesRequirements {
                         hir_generics.span.shrink_to_hi().to(where_span)
                     };
 
-                // Due to macro expansions, the `full_where_span` might not actually contain all predicates.
+                // Due to macro expansions, the `full_where_span` might not actually contain all
+                // predicates.
                 if where_lint_spans.iter().all(|&sp| full_where_span.contains(sp)) {
                     lint_spans.push(full_where_span);
                 } else {
@@ -2601,7 +2603,8 @@ impl<'tcx> LateLintPass<'tcx> for InvalidValue {
                     };
                     // So we have at least one potentially inhabited variant. Might we have two?
                     let Some(second_variant) = potential_variants.next() else {
-                        // There is only one potentially inhabited variant. So we can recursively check that variant!
+                        // There is only one potentially inhabited variant. So we can recursively
+                        // check that variant!
                         return variant_find_init_error(
                             cx,
                             ty,
@@ -2611,10 +2614,10 @@ impl<'tcx> LateLintPass<'tcx> for InvalidValue {
                             init,
                         );
                     };
-                    // So we have at least two potentially inhabited variants.
-                    // If we can prove that we have at least two *definitely* inhabited variants,
-                    // then we have a tag and hence leaving this uninit is definitely disallowed.
-                    // (Leaving it zeroed could be okay, depending on which variant is encoded as zero tag.)
+                    // So we have at least two potentially inhabited variants. If we can prove that
+                    // we have at least two *definitely* inhabited variants, then we have a tag and
+                    // hence leaving this uninit is definitely disallowed. (Leaving it zeroed could
+                    // be okay, depending on which variant is encoded as zero tag.)
                     if init == InitKind::Uninit {
                         let definitely_inhabited = (first_variant.1 as usize)
                             + (second_variant.1 as usize)
@@ -2825,7 +2828,8 @@ impl<'tcx> LateLintPass<'tcx> for NamedAsmLabels {
 
                 let mut found_labels = Vec::new();
 
-                // A semicolon might not actually be specified as a separator for all targets, but it seems like LLVM accepts it always
+                // A semicolon might not actually be specified as a separator for all targets, but
+                // it seems like LLVM accepts it always.
                 let statements = template_str.split(|c| matches!(c, '\n' | ';'));
                 for statement in statements {
                     // If there's a comment, trim it from the statement
@@ -2838,7 +2842,8 @@ impl<'tcx> LateLintPass<'tcx> for NamedAsmLabels {
                         let mut chars = possible_label.chars();
 
                         let Some(start) = chars.next() else {
-                            // Empty string means a leading ':' in this section, which is not a label.
+                            // Empty string means a leading ':' in this section, which is not a
+                            // label.
                             break 'label_loop;
                         };
 
@@ -2855,12 +2860,15 @@ impl<'tcx> LateLintPass<'tcx> for NamedAsmLabels {
 
                         // Labels continue with ASCII alphanumeric characters, _, or $
                         for c in chars {
-                            // Inside a template format arg, any character is permitted for the puproses of label detection
-                            // because we assume that it can be replaced with some other valid label string later.
-                            // `options(raw)` asm blocks cannot have format args, so they are excluded from this special case.
+                            // Inside a template format arg, any character is permitted for the
+                            // puproses of label detection because we assume that it can be
+                            // replaced with some other valid label string later. `options(raw)`
+                            // asm blocks cannot have format args, so they are excluded from this
+                            // special case.
                             if !raw && in_bracket {
                                 if c == '{' {
-                                    // Nested brackets are not allowed in format args, this cannot be a label.
+                                    // Nested brackets are not allowed in format args, this cannot
+                                    // be a label.
                                     break 'label_loop;
                                 }
 
@@ -2873,7 +2881,8 @@ impl<'tcx> LateLintPass<'tcx> for NamedAsmLabels {
                                 in_bracket = true;
                             } else {
                                 if !(c.is_ascii_alphanumeric() || matches!(c, '_' | '$')) {
-                                    // The potential label had an invalid character inside it, it cannot be a label.
+                                    // The potential label had an invalid character inside it, it
+                                    // cannot be a label.
                                     break 'label_loop;
                                 }
                             }
@@ -2892,7 +2901,8 @@ impl<'tcx> LateLintPass<'tcx> for NamedAsmLabels {
                         .into_iter()
                         .filter_map(|label| find_label_span(label))
                         .collect::<Vec<Span>>();
-                    // If there were labels but we couldn't find a span, combine the warnings and use the template span
+                    // If there were labels but we couldn't find a span, combine the warnings and
+                    // use the template span.
                     let target_spans: MultiSpan =
                         if spans.len() > 0 { spans.into() } else { (*template_span).into() };
 

compiler/rustc_lint/src/context.rs

@@ -94,7 +94,8 @@ enum TargetLint {
 
     /// A lint name that should give no warnings and have no effect.
     ///
-    /// This is used by rustc to avoid warning about old rustdoc lints before rustdoc registers them as tool lints.
+    /// This is used by rustc to avoid warning about old rustdoc lints before rustdoc registers
+    /// them as tool lints.
     Ignored,
 }
 
@@ -126,12 +127,16 @@ pub enum CheckLintNameResult<'a> {
     Renamed(String),
     /// The lint has been removed due to the given reason.
     Removed(String),
-    /// The lint is from a tool. If the Option is None, then either
-    /// the lint does not exist in the tool or the code was not
-    /// compiled with the tool and therefore the lint was never
-    /// added to the `LintStore`. Otherwise the `LintId` will be
-    /// returned as if it where a rustc lint.
-    Tool(Result<&'a [LintId], (Option<&'a [LintId]>, String)>),
+
+    /// The lint is from a tool. The `LintId` will be returned as if it were a
+    /// rustc lint. The `Option<String>` indicates if the lint has been
+    /// renamed.
+    Tool(&'a [LintId], Option<String>),
+
+    /// The lint is from a tool. Either the lint does not exist in the tool or
+    /// the code was not compiled with the tool and therefore the lint was
+    /// never added to the `LintStore`.
+    MissingTool,
 }
 
 impl LintStore {
@@ -384,14 +389,14 @@ impl LintStore {
                         } else {
                             // 2. The tool isn't currently running, so no lints will be registered.
                             // To avoid giving a false positive, ignore all unknown lints.
-                            CheckLintNameResult::Tool(Err((None, String::new())))
+                            CheckLintNameResult::MissingTool
                         };
                     }
                     Some(LintGroup { lint_ids, .. }) => {
-                        return CheckLintNameResult::Tool(Ok(lint_ids));
+                        return CheckLintNameResult::Tool(lint_ids, None);
                     }
                 },
-                Some(Id(id)) => return CheckLintNameResult::Tool(Ok(slice::from_ref(id))),
+                Some(Id(id)) => return CheckLintNameResult::Tool(slice::from_ref(id), None),
                 // If the lint was registered as removed or renamed by the lint tool, we don't need
                 // to treat tool_lints and rustc lints different and can use the code below.
                 _ => {}
@@ -411,7 +416,7 @@ impl LintStore {
                         return if *silent {
                             CheckLintNameResult::Ok(lint_ids)
                         } else {
-                            CheckLintNameResult::Tool(Err((Some(lint_ids), (*name).to_string())))
+                            CheckLintNameResult::Tool(lint_ids, Some((*name).to_string()))
                         };
                     }
                     CheckLintNameResult::Ok(lint_ids)
@@ -472,18 +477,17 @@ impl LintStore {
                     // Reaching this would be weird, but let's cover this case anyway
                     if let Some(LintAlias { name, silent }) = depr {
                         let LintGroup { lint_ids, .. } = self.lint_groups.get(name).unwrap();
-                        return if *silent {
-                            CheckLintNameResult::Tool(Err((Some(lint_ids), complete_name)))
+                        if *silent {
+                            CheckLintNameResult::Tool(lint_ids, Some(complete_name))
                         } else {
-                            CheckLintNameResult::Tool(Err((Some(lint_ids), (*name).to_string())))
-                        };
+                            CheckLintNameResult::Tool(lint_ids, Some((*name).to_string()))
+                        }
+                    } else {
+                        CheckLintNameResult::Tool(lint_ids, Some(complete_name))
                     }
-                    CheckLintNameResult::Tool(Err((Some(lint_ids), complete_name)))
                 }
             },
-            Some(Id(id)) => {
-                CheckLintNameResult::Tool(Err((Some(slice::from_ref(id)), complete_name)))
-            }
+            Some(Id(id)) => CheckLintNameResult::Tool(slice::from_ref(id), Some(complete_name)),
             Some(other) => {
                 debug!("got renamed lint {:?}", other);
                 CheckLintNameResult::NoLint(None)

compiler/rustc_lint/src/errors.rs

@@ -16,7 +16,7 @@ pub struct OverruledAttribute<'a> {
     #[subdiagnostic]
     pub sub: OverruledAttributeSub,
 }
-//
+
 pub enum OverruledAttributeSub {
     DefaultSource { id: String },
     NodeSource { span: Span, reason: Option<Symbol> },

compiler/rustc_lint/src/internal.rs

@@ -18,11 +18,11 @@ use rustc_span::Span;
 use tracing::debug;
 
 declare_tool_lint! {
-    /// The `default_hash_type` lint detects use of [`std::collections::HashMap`]/[`std::collections::HashSet`],
-    /// suggesting the use of `FxHashMap`/`FxHashSet`.
+    /// The `default_hash_type` lint detects use of [`std::collections::HashMap`] and
+    /// [`std::collections::HashSet`], suggesting the use of `FxHashMap`/`FxHashSet`.
     ///
-    /// This can help as `FxHasher` can perform better than the default hasher. DOS protection is not
-    /// required as input is assumed to be trusted.
+    /// This can help as `FxHasher` can perform better than the default hasher. DOS protection is
+    /// not required as input is assumed to be trusted.
     pub rustc::DEFAULT_HASH_TYPES,
     Allow,
     "forbid HashMap and HashSet and suggest the FxHash* variants",
@@ -35,7 +35,7 @@ impl LateLintPass<'_> for DefaultHashTypes {
     fn check_path(&mut self, cx: &LateContext<'_>, path: &Path<'_>, hir_id: HirId) {
         let Res::Def(rustc_hir::def::DefKind::Struct, def_id) = path.res else { return };
         if matches!(cx.tcx.hir_node(hir_id), Node::Item(Item { kind: ItemKind::Use(..), .. })) {
-            // don't lint imports, only actual usages
+            // Don't lint imports, only actual usages.
             return;
         }
         let preferred = match cx.tcx.get_diagnostic_name(def_id) {
@@ -75,8 +75,8 @@ declare_tool_lint! {
     /// potential query instability, such as iterating over a `HashMap`.
     ///
     /// Due to the [incremental compilation](https://rustc-dev-guide.rust-lang.org/queries/incremental-compilation.html) model,
-    /// queries must return deterministic, stable results. `HashMap` iteration order can change between compilations,
-    /// and will introduce instability if query results expose the order.
+    /// queries must return deterministic, stable results. `HashMap` iteration order can change
+    /// between compilations, and will introduce instability if query results expose the order.
     pub rustc::POTENTIAL_QUERY_INSTABILITY,
     Allow,
     "require explicit opt-in when using potentially unstable methods or functions",

compiler/rustc_lint/src/let_underscore.rs

@@ -113,11 +113,11 @@ impl<'tcx> LateLintPass<'tcx> for LetUnderscore {
 
         let mut top_level = true;
 
-        // We recursively walk through all patterns, so that we can catch cases where the lock is nested in a pattern.
-        // For the basic `let_underscore_drop` lint, we only look at the top level, since there are many legitimate reasons
-        // to bind a sub-pattern to an `_`, if we're only interested in the rest.
-        // But with locks, we prefer having the chance of "false positives" over missing cases, since the effects can be
-        // quite catastrophic.
+        // We recursively walk through all patterns, so that we can catch cases where the lock is
+        // nested in a pattern. For the basic `let_underscore_drop` lint, we only look at the top
+        // level, since there are many legitimate reasons to bind a sub-pattern to an `_`, if we're
+        // only interested in the rest. But with locks, we prefer having the chance of "false
+        // positives" over missing cases, since the effects can be quite catastrophic.
         local.pat.walk_always(|pat| {
             let is_top_level = top_level;
             top_level = false;