Rollup merge of #142645 - Urgau:usage-non_upper_case_globals, r=fmease

Also emit suggestions for usages in the `non_upper_case_globals` lint

This PR adds suggestions for all the usages of the renamed item in the warning of the  `non_upper_case_globals` lint.

Fixes #124061

Author: matthiaskrgr

compiler/rustc_lint/src/context.rs

@@ -524,6 +524,20 @@ pub trait LintContext {
         });
     }
 
+    /// Emit a lint at `span` from a lazily-constructed lint struct (some type that implements
+    /// `LintDiagnostic`, typically generated by `#[derive(LintDiagnostic)]`).
+    fn emit_span_lint_lazy<S: Into<MultiSpan>, L: for<'a> LintDiagnostic<'a, ()>>(
+        &self,
+        lint: &'static Lint,
+        span: S,
+        decorator: impl FnOnce() -> L,
+    ) {
+        self.opt_span_lint(lint, Some(span), |lint| {
+            let decorator = decorator();
+            decorator.decorate_lint(lint);
+        });
+    }
+
     /// Emit a lint at the appropriate level, with an associated span.
     ///
     /// [`lint_level`]: rustc_middle::lint::lint_level#decorate-signature

compiler/rustc_lint/src/lints.rs

@@ -1353,6 +1353,8 @@ pub(crate) struct NonUpperCaseGlobal<'a> {
     pub name: &'a str,
     #[subdiagnostic]
     pub sub: NonUpperCaseGlobalSub,
+    #[subdiagnostic]
+    pub usages: Vec<NonUpperCaseGlobalSubTool>,
 }
 
 #[derive(Subdiagnostic)]
@@ -1362,14 +1364,29 @@ pub(crate) enum NonUpperCaseGlobalSub {
         #[primary_span]
         span: Span,
     },
-    #[suggestion(lint_suggestion, code = "{replace}", applicability = "maybe-incorrect")]
+    #[suggestion(lint_suggestion, code = "{replace}")]
     Suggestion {
         #[primary_span]
         span: Span,
+        #[applicability]
+        applicability: Applicability,
         replace: String,
     },
 }
 
+#[derive(Subdiagnostic)]
+#[suggestion(
+    lint_suggestion,
+    code = "{replace}",
+    applicability = "machine-applicable",
+    style = "tool-only"
+)]
+pub(crate) struct NonUpperCaseGlobalSubTool {
+    #[primary_span]
+    pub(crate) span: Span,
+    pub(crate) replace: String,
+}
+
 // noop_method_call.rs
 #[derive(LintDiagnostic)]
 #[diag(lint_noop_method_call)]

compiler/rustc_lint/src/nonstandard_style.rs

@@ -1,9 +1,12 @@
 use rustc_abi::ExternAbi;
 use rustc_attr_data_structures::{AttributeKind, ReprAttr, find_attr};
 use rustc_attr_parsing::AttributeParser;
+use rustc_errors::Applicability;
 use rustc_hir::def::{DefKind, Res};
-use rustc_hir::intravisit::FnKind;
+use rustc_hir::def_id::DefId;
+use rustc_hir::intravisit::{FnKind, Visitor};
 use rustc_hir::{AttrArgs, AttrItem, Attribute, GenericParamKind, PatExprKind, PatKind};
+use rustc_middle::hir::nested_filter::All;
 use rustc_middle::ty;
 use rustc_session::config::CrateType;
 use rustc_session::{declare_lint, declare_lint_pass};
@@ -13,7 +16,7 @@ use {rustc_ast as ast, rustc_hir as hir};
 
 use crate::lints::{
     NonCamelCaseType, NonCamelCaseTypeSub, NonSnakeCaseDiag, NonSnakeCaseDiagSub,
-    NonUpperCaseGlobal, NonUpperCaseGlobalSub,
+    NonUpperCaseGlobal, NonUpperCaseGlobalSub, NonUpperCaseGlobalSubTool,
 };
 use crate::{EarlyContext, EarlyLintPass, LateContext, LateLintPass, LintContext};
 
@@ -493,22 +496,82 @@ declare_lint! {
 declare_lint_pass!(NonUpperCaseGlobals => [NON_UPPER_CASE_GLOBALS]);
 
 impl NonUpperCaseGlobals {
-    fn check_upper_case(cx: &LateContext<'_>, sort: &str, ident: &Ident) {
+    fn check_upper_case(cx: &LateContext<'_>, sort: &str, did: Option<LocalDefId>, ident: &Ident) {
         let name = ident.name.as_str();
         if name.chars().any(|c| c.is_lowercase()) {
             let uc = NonSnakeCase::to_snake_case(name).to_uppercase();
+
+            // If the item is exported, suggesting changing it's name would be breaking-change
+            // and could break users without a "nice" applicable fix, so let's avoid it.
+            let can_change_usages = if let Some(did) = did {
+                !cx.tcx.effective_visibilities(()).is_exported(did)
+            } else {
+                false
+            };
+
             // We cannot provide meaningful suggestions
             // if the characters are in the category of "Lowercase Letter".
             let sub = if *name != uc {
-                NonUpperCaseGlobalSub::Suggestion { span: ident.span, replace: uc }
+                NonUpperCaseGlobalSub::Suggestion {
+                    span: ident.span,
+                    replace: uc.clone(),
+                    applicability: if can_change_usages {
+                        Applicability::MachineApplicable
+                    } else {
+                        Applicability::MaybeIncorrect
+                    },
+                }
             } else {
                 NonUpperCaseGlobalSub::Label { span: ident.span }
             };
-            cx.emit_span_lint(
-                NON_UPPER_CASE_GLOBALS,
-                ident.span,
-                NonUpperCaseGlobal { sort, name, sub },
-            );
+
+            struct UsageCollector<'a, 'tcx> {
+                cx: &'tcx LateContext<'a>,
+                did: DefId,
+                collected: Vec<Span>,
+            }
+
+            impl<'v, 'tcx> Visitor<'v> for UsageCollector<'v, 'tcx> {
+                type NestedFilter = All;
+
+                fn maybe_tcx(&mut self) -> Self::MaybeTyCtxt {
+                    self.cx.tcx
+                }
+
+                fn visit_path(
+                    &mut self,
+                    path: &rustc_hir::Path<'v>,
+                    _id: rustc_hir::HirId,
+                ) -> Self::Result {
+                    if let Some(final_seg) = path.segments.last()
+                        && final_seg.res.opt_def_id() == Some(self.did)
+                    {
+                        self.collected.push(final_seg.ident.span);
+                    }
+                }
+            }
+
+            cx.emit_span_lint_lazy(NON_UPPER_CASE_GLOBALS, ident.span, || {
+                // Compute usages lazily as it can expansive and useless when the lint is allowed.
+                // cf. https://github.com/rust-lang/rust/pull/142645#issuecomment-2993024625
+                let usages = if can_change_usages
+                    && *name != uc
+                    && let Some(did) = did
+                {
+                    let mut usage_collector =
+                        UsageCollector { cx, did: did.to_def_id(), collected: Vec::new() };
+                    cx.tcx.hir_walk_toplevel_module(&mut usage_collector);
+                    usage_collector
+                        .collected
+                        .into_iter()
+                        .map(|span| NonUpperCaseGlobalSubTool { span, replace: uc.clone() })
+                        .collect()
+                } else {
+                    vec![]
+                };
+
+                NonUpperCaseGlobal { sort, name, sub, usages }
+            });
         }
     }
 }
@@ -520,26 +583,36 @@ impl<'tcx> LateLintPass<'tcx> for NonUpperCaseGlobals {
             hir::ItemKind::Static(_, ident, ..)
                 if !find_attr!(attrs, AttributeKind::NoMangle(..)) =>
             {
-                NonUpperCaseGlobals::check_upper_case(cx, "static variable", &ident);
+                NonUpperCaseGlobals::check_upper_case(
+                    cx,
+                    "static variable",
+                    Some(it.owner_id.def_id),
+                    &ident,
+                );
             }
             hir::ItemKind::Const(ident, ..) => {
-                NonUpperCaseGlobals::check_upper_case(cx, "constant", &ident);
+                NonUpperCaseGlobals::check_upper_case(
+                    cx,
+                    "constant",
+                    Some(it.owner_id.def_id),
+                    &ident,
+                );
             }
             _ => {}
         }
     }
 
     fn check_trait_item(&mut self, cx: &LateContext<'_>, ti: &hir::TraitItem<'_>) {
         if let hir::TraitItemKind::Const(..) = ti.kind {
-            NonUpperCaseGlobals::check_upper_case(cx, "associated constant", &ti.ident);
+            NonUpperCaseGlobals::check_upper_case(cx, "associated constant", None, &ti.ident);
         }
     }
 
     fn check_impl_item(&mut self, cx: &LateContext<'_>, ii: &hir::ImplItem<'_>) {
         if let hir::ImplItemKind::Const(..) = ii.kind
             && !assoc_item_in_trait_impl(cx, ii)
         {
-            NonUpperCaseGlobals::check_upper_case(cx, "associated constant", &ii.ident);
+            NonUpperCaseGlobals::check_upper_case(cx, "associated constant", None, &ii.ident);
         }
     }
 
@@ -555,6 +628,7 @@ impl<'tcx> LateLintPass<'tcx> for NonUpperCaseGlobals {
                     NonUpperCaseGlobals::check_upper_case(
                         cx,
                         "constant in pattern",
+                        None,
                         &segment.ident,
                     );
                 }
@@ -564,7 +638,12 @@ impl<'tcx> LateLintPass<'tcx> for NonUpperCaseGlobals {
 
     fn check_generic_param(&mut self, cx: &LateContext<'_>, param: &hir::GenericParam<'_>) {
         if let GenericParamKind::Const { .. } = param.kind {
-            NonUpperCaseGlobals::check_upper_case(cx, "const parameter", &param.name.ident());
+            NonUpperCaseGlobals::check_upper_case(
+                cx,
+                "const parameter",
+                Some(param.def_id),
+                &param.name.ident(),
+            );
         }
     }
 }

tests/ui/lint/lint-non-uppercase-usages.fixed

@@ -0,0 +1,44 @@
+// Checks that the `non_upper_case_globals` emits suggestions for usages as well
+// <https://github.com/rust-lang/rust/issues/124061>
+
+//@ check-pass
+//@ run-rustfix
+
+#![allow(dead_code)]
+
+use std::cell::Cell;
+
+const MY_STATIC: u32 = 0;
+//~^ WARN constant `my_static` should have an upper case name
+//~| SUGGESTION MY_STATIC
+
+const LOL: u32 = MY_STATIC + 0;
+//~^ SUGGESTION MY_STATIC
+
+mod my_mod {
+    const INSIDE_MOD: u32 = super::MY_STATIC + 0;
+    //~^ SUGGESTION MY_STATIC
+}
+
+thread_local! {
+    static FOO_FOO: Cell<usize> = unreachable!();
+    //~^ WARN constant `fooFOO` should have an upper case name
+    //~| SUGGESTION FOO_FOO
+}
+
+fn foo<const FOO: u32>() {
+    //~^ WARN const parameter `foo` should have an upper case name
+    //~| SUGGESTION FOO
+    let _a = FOO + 1;
+    //~^ SUGGESTION FOO
+}
+
+fn main() {
+    let _a = crate::MY_STATIC;
+    //~^ SUGGESTION MY_STATIC
+
+    FOO_FOO.set(9);
+    //~^ SUGGESTION FOO_FOO
+    println!("{}", FOO_FOO.get());
+    //~^ SUGGESTION FOO_FOO
+}

tests/ui/lint/lint-non-uppercase-usages.rs

@@ -0,0 +1,44 @@
+// Checks that the `non_upper_case_globals` emits suggestions for usages as well
+// <https://github.com/rust-lang/rust/issues/124061>
+
+//@ check-pass
+//@ run-rustfix
+
+#![allow(dead_code)]
+
+use std::cell::Cell;
+
+const my_static: u32 = 0;
+//~^ WARN constant `my_static` should have an upper case name
+//~| SUGGESTION MY_STATIC
+
+const LOL: u32 = my_static + 0;
+//~^ SUGGESTION MY_STATIC
+
+mod my_mod {
+    const INSIDE_MOD: u32 = super::my_static + 0;
+    //~^ SUGGESTION MY_STATIC
+}
+
+thread_local! {
+    static fooFOO: Cell<usize> = unreachable!();
+    //~^ WARN constant `fooFOO` should have an upper case name
+    //~| SUGGESTION FOO_FOO
+}
+
+fn foo<const foo: u32>() {
+    //~^ WARN const parameter `foo` should have an upper case name
+    //~| SUGGESTION FOO
+    let _a = foo + 1;
+    //~^ SUGGESTION FOO
+}
+
+fn main() {
+    let _a = crate::my_static;
+    //~^ SUGGESTION MY_STATIC
+
+    fooFOO.set(9);
+    //~^ SUGGESTION FOO_FOO
+    println!("{}", fooFOO.get());
+    //~^ SUGGESTION FOO_FOO
+}

tests/ui/lint/lint-non-uppercase-usages.stderr

@@ -0,0 +1,39 @@
+warning: constant `my_static` should have an upper case name
+  --> $DIR/lint-non-uppercase-usages.rs:11:7
+   |
+LL | const my_static: u32 = 0;
+   |       ^^^^^^^^^
+   |
+   = note: `#[warn(non_upper_case_globals)]` on by default
+help: convert the identifier to upper case
+   |
+LL - const my_static: u32 = 0;
+LL + const MY_STATIC: u32 = 0;
+   |
+
+warning: constant `fooFOO` should have an upper case name
+  --> $DIR/lint-non-uppercase-usages.rs:24:12
+   |
+LL |     static fooFOO: Cell<usize> = unreachable!();
+   |            ^^^^^^
+   |
+help: convert the identifier to upper case
+   |
+LL -     static fooFOO: Cell<usize> = unreachable!();
+LL +     static FOO_FOO: Cell<usize> = unreachable!();
+   |
+
+warning: const parameter `foo` should have an upper case name
+  --> $DIR/lint-non-uppercase-usages.rs:29:14
+   |
+LL | fn foo<const foo: u32>() {
+   |              ^^^
+   |
+help: convert the identifier to upper case (notice the capitalization difference)
+   |
+LL - fn foo<const foo: u32>() {
+LL + fn foo<const FOO: u32>() {
+   |
+
+warning: 3 warnings emitted
+