diff --git a/compiler/rustc_lint/src/context.rs b/compiler/rustc_lint/src/context.rs index b6bf45dfbcfbf..8a0f50e0219b2 100644 --- a/compiler/rustc_lint/src/context.rs +++ b/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, 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 diff --git a/compiler/rustc_lint/src/lints.rs b/compiler/rustc_lint/src/lints.rs index 3d17dfbc45198..eebb9339e246d 100644 --- a/compiler/rustc_lint/src/lints.rs +++ b/compiler/rustc_lint/src/lints.rs @@ -1348,6 +1348,8 @@ pub(crate) struct NonUpperCaseGlobal<'a> { pub name: &'a str, #[subdiagnostic] pub sub: NonUpperCaseGlobalSub, + #[subdiagnostic] + pub usages: Vec, } #[derive(Subdiagnostic)] @@ -1357,14 +1359,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)] diff --git a/compiler/rustc_lint/src/nonstandard_style.rs b/compiler/rustc_lint/src/nonstandard_style.rs index 1b60466a589d2..a42a6076fc341 100644 --- a/compiler/rustc_lint/src/nonstandard_style.rs +++ b/compiler/rustc_lint/src/nonstandard_style.rs @@ -1,9 +1,12 @@ use rustc_abi::ExternAbi; use rustc_attr_data_structures::{AttributeKind, ReprAttr}; 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}; @@ -489,22 +492,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, 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, + } + + 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 } + }); } } } @@ -516,10 +579,20 @@ impl<'tcx> LateLintPass<'tcx> for NonUpperCaseGlobals { hir::ItemKind::Static(_, ident, ..) if !ast::attr::contains_name(attrs, sym::no_mangle) => { - 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, + ); } _ => {} } @@ -527,7 +600,7 @@ impl<'tcx> LateLintPass<'tcx> for NonUpperCaseGlobals { 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); } } @@ -535,7 +608,7 @@ impl<'tcx> LateLintPass<'tcx> for NonUpperCaseGlobals { 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); } } @@ -551,6 +624,7 @@ impl<'tcx> LateLintPass<'tcx> for NonUpperCaseGlobals { NonUpperCaseGlobals::check_upper_case( cx, "constant in pattern", + None, &segment.ident, ); } @@ -560,7 +634,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", ¶m.name.ident()); + NonUpperCaseGlobals::check_upper_case( + cx, + "const parameter", + Some(param.def_id), + ¶m.name.ident(), + ); } } } diff --git a/tests/ui/lint/lint-non-uppercase-usages.fixed b/tests/ui/lint/lint-non-uppercase-usages.fixed new file mode 100644 index 0000000000000..231991dcae08c --- /dev/null +++ b/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 +// + +//@ 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 = unreachable!(); + //~^ WARN constant `fooFOO` should have an upper case name + //~| SUGGESTION FOO_FOO +} + +fn foo() { + //~^ 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 +} diff --git a/tests/ui/lint/lint-non-uppercase-usages.rs b/tests/ui/lint/lint-non-uppercase-usages.rs new file mode 100644 index 0000000000000..9cdf5e47003d5 --- /dev/null +++ b/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 +// + +//@ 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 = unreachable!(); + //~^ WARN constant `fooFOO` should have an upper case name + //~| SUGGESTION FOO_FOO +} + +fn foo() { + //~^ 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 +} diff --git a/tests/ui/lint/lint-non-uppercase-usages.stderr b/tests/ui/lint/lint-non-uppercase-usages.stderr new file mode 100644 index 0000000000000..7c7e573a88edc --- /dev/null +++ b/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 = unreachable!(); + | ^^^^^^ + | +help: convert the identifier to upper case + | +LL - static fooFOO: Cell = unreachable!(); +LL + static FOO_FOO: Cell = unreachable!(); + | + +warning: const parameter `foo` should have an upper case name + --> $DIR/lint-non-uppercase-usages.rs:29:14 + | +LL | fn foo() { + | ^^^ + | +help: convert the identifier to upper case (notice the capitalization difference) + | +LL - fn foo() { +LL + fn foo() { + | + +warning: 3 warnings emitted +