From 42bb66add31d196bb8d0f0bfb79a00dfd2cad55b Mon Sep 17 00:00:00 2001 From: Urgau Date: Tue, 17 Jun 2025 23:27:06 +0200 Subject: [PATCH 1/6] Also emit suggestions for usages in the `non_upper_case_globals` lint --- compiler/rustc_lint/src/lints.rs | 2 + compiler/rustc_lint/src/nonstandard_style.rs | 77 ++++++++++++++++--- tests/ui/lint/lint-non-uppercase-usages.fixed | 39 ++++++++++ tests/ui/lint/lint-non-uppercase-usages.rs | 39 ++++++++++ .../ui/lint/lint-non-uppercase-usages.stderr | 64 +++++++++++++++ 5 files changed, 210 insertions(+), 11 deletions(-) create mode 100644 tests/ui/lint/lint-non-uppercase-usages.fixed create mode 100644 tests/ui/lint/lint-non-uppercase-usages.rs create mode 100644 tests/ui/lint/lint-non-uppercase-usages.stderr diff --git a/compiler/rustc_lint/src/lints.rs b/compiler/rustc_lint/src/lints.rs index 3d17dfbc45198..d157bf6986ce9 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)] diff --git a/compiler/rustc_lint/src/nonstandard_style.rs b/compiler/rustc_lint/src/nonstandard_style.rs index 1b60466a589d2..a5b3eb3f0ffb5 100644 --- a/compiler/rustc_lint/src/nonstandard_style.rs +++ b/compiler/rustc_lint/src/nonstandard_style.rs @@ -2,8 +2,9 @@ use rustc_abi::ExternAbi; use rustc_attr_data_structures::{AttributeKind, ReprAttr}; use rustc_attr_parsing::AttributeParser; use rustc_hir::def::{DefKind, Res}; -use rustc_hir::intravisit::FnKind; +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}; @@ -489,21 +490,59 @@ 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(); + // 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 } + let sub = |span| { + if *name != uc { + NonUpperCaseGlobalSub::Suggestion { span, replace: uc.clone() } + } else { + NonUpperCaseGlobalSub::Label { span } + } + }; + + struct UsageCollector<'a, 'tcx> { + cx: &'tcx LateContext<'a>, + did: LocalDefId, + 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 { + for seg in path.segments { + if seg.res.opt_def_id() == Some(self.did.to_def_id()) { + self.collected.push(seg.ident.span); + } + } + } + } + + let usages = if let Some(did) = did { + let mut usage_collector = UsageCollector { cx, did, collected: Vec::new() }; + cx.tcx.hir_walk_toplevel_module(&mut usage_collector); + usage_collector.collected.into_iter().map(|span| sub(span)).collect() } else { - NonUpperCaseGlobalSub::Label { span: ident.span } + vec![] }; + cx.emit_span_lint( NON_UPPER_CASE_GLOBALS, ident.span, - NonUpperCaseGlobal { sort, name, sub }, + NonUpperCaseGlobal { sort, name, sub: sub(ident.span), usages }, ); } } @@ -516,10 +555,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 +576,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 +584,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 +600,7 @@ impl<'tcx> LateLintPass<'tcx> for NonUpperCaseGlobals { NonUpperCaseGlobals::check_upper_case( cx, "constant in pattern", + None, &segment.ident, ); } @@ -560,7 +610,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..048a936ff2730 --- /dev/null +++ b/tests/ui/lint/lint-non-uppercase-usages.fixed @@ -0,0 +1,39 @@ +// 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 + +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..b5b9ffac6bce0 --- /dev/null +++ b/tests/ui/lint/lint-non-uppercase-usages.rs @@ -0,0 +1,39 @@ +// 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 + +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..fa47b4ba6a8f6 --- /dev/null +++ b/tests/ui/lint/lint-non-uppercase-usages.stderr @@ -0,0 +1,64 @@ +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; + | +help: convert the identifier to upper case + | +LL - const LOL: u32 = my_static + 0; +LL + const LOL: u32 = MY_STATIC + 0; + | +help: convert the identifier to upper case + | +LL - let _a = crate::my_static; +LL + let _a = crate::MY_STATIC; + | + +warning: constant `fooFOO` should have an upper case name + --> $DIR/lint-non-uppercase-usages.rs:19:12 + | +LL | static fooFOO: Cell = unreachable!(); + | ^^^^^^ + | +help: convert the identifier to upper case + | +LL - static fooFOO: Cell = unreachable!(); +LL + static FOO_FOO: Cell = unreachable!(); + | +help: convert the identifier to upper case + | +LL - fooFOO.set(9); +LL + FOO_FOO.set(9); + | +help: convert the identifier to upper case + | +LL - println!("{}", fooFOO.get()); +LL + println!("{}", FOO_FOO.get()); + | + +warning: const parameter `foo` should have an upper case name + --> $DIR/lint-non-uppercase-usages.rs:24:14 + | +LL | fn foo() { + | ^^^ + | +help: convert the identifier to upper case (notice the capitalization difference) + | +LL - fn foo() { +LL + fn foo() { + | +help: convert the identifier to upper case (notice the capitalization difference) + | +LL - let _a = foo + 1; +LL + let _a = FOO + 1; + | + +warning: 3 warnings emitted + From 4df9f2f8412db164e787233d1fc56d2988f255c8 Mon Sep 17 00:00:00 2001 From: Urgau Date: Wed, 18 Jun 2025 19:15:10 +0200 Subject: [PATCH 2/6] Emit the usages suggestions as tool-only suggestions --- compiler/rustc_lint/src/lints.rs | 15 ++++++++++- compiler/rustc_lint/src/nonstandard_style.rs | 24 ++++++++++-------- .../ui/lint/lint-non-uppercase-usages.stderr | 25 ------------------- 3 files changed, 28 insertions(+), 36 deletions(-) diff --git a/compiler/rustc_lint/src/lints.rs b/compiler/rustc_lint/src/lints.rs index d157bf6986ce9..a20050a10b488 100644 --- a/compiler/rustc_lint/src/lints.rs +++ b/compiler/rustc_lint/src/lints.rs @@ -1349,7 +1349,7 @@ pub(crate) struct NonUpperCaseGlobal<'a> { #[subdiagnostic] pub sub: NonUpperCaseGlobalSub, #[subdiagnostic] - pub usages: Vec, + pub usages: Vec, } #[derive(Subdiagnostic)] @@ -1367,6 +1367,19 @@ pub(crate) enum NonUpperCaseGlobalSub { }, } +#[derive(Subdiagnostic)] +#[suggestion( + lint_suggestion, + code = "{replace}", + applicability = "maybe-incorrect", + 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 a5b3eb3f0ffb5..e92e8e0638292 100644 --- a/compiler/rustc_lint/src/nonstandard_style.rs +++ b/compiler/rustc_lint/src/nonstandard_style.rs @@ -14,7 +14,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}; @@ -497,12 +497,10 @@ impl NonUpperCaseGlobals { // We cannot provide meaningful suggestions // if the characters are in the category of "Lowercase Letter". - let sub = |span| { - if *name != uc { - NonUpperCaseGlobalSub::Suggestion { span, replace: uc.clone() } - } else { - NonUpperCaseGlobalSub::Label { span } - } + let sub = if *name != uc { + NonUpperCaseGlobalSub::Suggestion { span: ident.span, replace: uc.clone() } + } else { + NonUpperCaseGlobalSub::Label { span: ident.span } }; struct UsageCollector<'a, 'tcx> { @@ -531,10 +529,16 @@ impl NonUpperCaseGlobals { } } - let usages = if let Some(did) = did { + let usages = if let Some(did) = did + && *name != uc + { let mut usage_collector = UsageCollector { cx, did, collected: Vec::new() }; cx.tcx.hir_walk_toplevel_module(&mut usage_collector); - usage_collector.collected.into_iter().map(|span| sub(span)).collect() + usage_collector + .collected + .into_iter() + .map(|span| NonUpperCaseGlobalSubTool { span, replace: uc.clone() }) + .collect() } else { vec![] }; @@ -542,7 +546,7 @@ impl NonUpperCaseGlobals { cx.emit_span_lint( NON_UPPER_CASE_GLOBALS, ident.span, - NonUpperCaseGlobal { sort, name, sub: sub(ident.span), usages }, + NonUpperCaseGlobal { sort, name, sub, usages }, ); } } diff --git a/tests/ui/lint/lint-non-uppercase-usages.stderr b/tests/ui/lint/lint-non-uppercase-usages.stderr index fa47b4ba6a8f6..34a0a5b0ca68d 100644 --- a/tests/ui/lint/lint-non-uppercase-usages.stderr +++ b/tests/ui/lint/lint-non-uppercase-usages.stderr @@ -10,16 +10,6 @@ help: convert the identifier to upper case LL - const my_static: u32 = 0; LL + const MY_STATIC: u32 = 0; | -help: convert the identifier to upper case - | -LL - const LOL: u32 = my_static + 0; -LL + const LOL: u32 = MY_STATIC + 0; - | -help: convert the identifier to upper case - | -LL - let _a = crate::my_static; -LL + let _a = crate::MY_STATIC; - | warning: constant `fooFOO` should have an upper case name --> $DIR/lint-non-uppercase-usages.rs:19:12 @@ -32,16 +22,6 @@ help: convert the identifier to upper case LL - static fooFOO: Cell = unreachable!(); LL + static FOO_FOO: Cell = unreachable!(); | -help: convert the identifier to upper case - | -LL - fooFOO.set(9); -LL + FOO_FOO.set(9); - | -help: convert the identifier to upper case - | -LL - println!("{}", fooFOO.get()); -LL + println!("{}", FOO_FOO.get()); - | warning: const parameter `foo` should have an upper case name --> $DIR/lint-non-uppercase-usages.rs:24:14 @@ -54,11 +34,6 @@ help: convert the identifier to upper case (notice the capitalization difference LL - fn foo() { LL + fn foo() { | -help: convert the identifier to upper case (notice the capitalization difference) - | -LL - let _a = foo + 1; -LL + let _a = FOO + 1; - | warning: 3 warnings emitted From 09d0a739f7dc4d84ae7baeac03a7e0d94671f3cb Mon Sep 17 00:00:00 2001 From: Urgau Date: Fri, 20 Jun 2025 18:59:34 +0200 Subject: [PATCH 3/6] Switch `non_upper_case_globals` suggestions to being machine-applicable --- compiler/rustc_lint/src/lints.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_lint/src/lints.rs b/compiler/rustc_lint/src/lints.rs index a20050a10b488..0ef7b3c14acdc 100644 --- a/compiler/rustc_lint/src/lints.rs +++ b/compiler/rustc_lint/src/lints.rs @@ -1359,7 +1359,7 @@ pub(crate) enum NonUpperCaseGlobalSub { #[primary_span] span: Span, }, - #[suggestion(lint_suggestion, code = "{replace}", applicability = "maybe-incorrect")] + #[suggestion(lint_suggestion, code = "{replace}", applicability = "machine-applicable")] Suggestion { #[primary_span] span: Span, @@ -1371,7 +1371,7 @@ pub(crate) enum NonUpperCaseGlobalSub { #[suggestion( lint_suggestion, code = "{replace}", - applicability = "maybe-incorrect", + applicability = "machine-applicable", style = "tool-only" )] pub(crate) struct NonUpperCaseGlobalSubTool { From 8b882651330b08bb5c99be5d10d92d21422dad7c Mon Sep 17 00:00:00 2001 From: Urgau Date: Sat, 21 Jun 2025 11:39:28 +0200 Subject: [PATCH 4/6] Lazily collect `NonUpperCaseGlobalSubTool` diagnostics --- compiler/rustc_lint/src/nonstandard_style.rs | 38 ++++++++++---------- 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/compiler/rustc_lint/src/nonstandard_style.rs b/compiler/rustc_lint/src/nonstandard_style.rs index e92e8e0638292..0cb3dac248232 100644 --- a/compiler/rustc_lint/src/nonstandard_style.rs +++ b/compiler/rustc_lint/src/nonstandard_style.rs @@ -1,6 +1,7 @@ use rustc_abi::ExternAbi; use rustc_attr_data_structures::{AttributeKind, ReprAttr}; use rustc_attr_parsing::AttributeParser; +use rustc_errors::LintDiagnostic; use rustc_hir::def::{DefKind, Res}; use rustc_hir::intravisit::{FnKind, Visitor}; use rustc_hir::{AttrArgs, AttrItem, Attribute, GenericParamKind, PatExprKind, PatKind}; @@ -529,25 +530,26 @@ impl NonUpperCaseGlobals { } } - let usages = if let Some(did) = did - && *name != uc - { - let mut usage_collector = UsageCollector { cx, did, 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![] - }; + #[allow(rustc::diagnostic_outside_of_impl)] + cx.opt_span_lint(NON_UPPER_CASE_GLOBALS, ident.span.into(), |diag| { + // 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 let Some(did) = did + && *name != uc + { + let mut usage_collector = UsageCollector { cx, did, 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![] + }; - cx.emit_span_lint( - NON_UPPER_CASE_GLOBALS, - ident.span, - NonUpperCaseGlobal { sort, name, sub, usages }, - ); + NonUpperCaseGlobal { sort, name, sub, usages }.decorate_lint(diag) + }); } } } From 1b5ec3fa1d81c1e469e5bc1c29fa2bc0452697c3 Mon Sep 17 00:00:00 2001 From: Urgau Date: Sat, 21 Jun 2025 13:40:05 +0200 Subject: [PATCH 5/6] Add `emit_span_lint_lazy` to lazily create `LintDiagnostic` structs --- compiler/rustc_lint/src/context.rs | 14 ++++++++++++++ compiler/rustc_lint/src/nonstandard_style.rs | 6 ++---- 2 files changed, 16 insertions(+), 4 deletions(-) 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/nonstandard_style.rs b/compiler/rustc_lint/src/nonstandard_style.rs index 0cb3dac248232..c325b7a95c9b1 100644 --- a/compiler/rustc_lint/src/nonstandard_style.rs +++ b/compiler/rustc_lint/src/nonstandard_style.rs @@ -1,7 +1,6 @@ use rustc_abi::ExternAbi; use rustc_attr_data_structures::{AttributeKind, ReprAttr}; use rustc_attr_parsing::AttributeParser; -use rustc_errors::LintDiagnostic; use rustc_hir::def::{DefKind, Res}; use rustc_hir::intravisit::{FnKind, Visitor}; use rustc_hir::{AttrArgs, AttrItem, Attribute, GenericParamKind, PatExprKind, PatKind}; @@ -530,8 +529,7 @@ impl NonUpperCaseGlobals { } } - #[allow(rustc::diagnostic_outside_of_impl)] - cx.opt_span_lint(NON_UPPER_CASE_GLOBALS, ident.span.into(), |diag| { + 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 let Some(did) = did @@ -548,7 +546,7 @@ impl NonUpperCaseGlobals { vec![] }; - NonUpperCaseGlobal { sort, name, sub, usages }.decorate_lint(diag) + NonUpperCaseGlobal { sort, name, sub, usages } }); } } From 6ffd0e6c235f9a28f724ce21bda7a9525a4dac03 Mon Sep 17 00:00:00 2001 From: Urgau Date: Sat, 21 Jun 2025 22:07:36 +0200 Subject: [PATCH 6/6] Address review comments --- compiler/rustc_lint/src/lints.rs | 4 ++- compiler/rustc_lint/src/nonstandard_style.rs | 36 ++++++++++++++----- tests/ui/lint/lint-non-uppercase-usages.fixed | 5 +++ tests/ui/lint/lint-non-uppercase-usages.rs | 5 +++ .../ui/lint/lint-non-uppercase-usages.stderr | 4 +-- 5 files changed, 43 insertions(+), 11 deletions(-) diff --git a/compiler/rustc_lint/src/lints.rs b/compiler/rustc_lint/src/lints.rs index 0ef7b3c14acdc..eebb9339e246d 100644 --- a/compiler/rustc_lint/src/lints.rs +++ b/compiler/rustc_lint/src/lints.rs @@ -1359,10 +1359,12 @@ pub(crate) enum NonUpperCaseGlobalSub { #[primary_span] span: Span, }, - #[suggestion(lint_suggestion, code = "{replace}", applicability = "machine-applicable")] + #[suggestion(lint_suggestion, code = "{replace}")] Suggestion { #[primary_span] span: Span, + #[applicability] + applicability: Applicability, replace: String, }, } diff --git a/compiler/rustc_lint/src/nonstandard_style.rs b/compiler/rustc_lint/src/nonstandard_style.rs index c325b7a95c9b1..a42a6076fc341 100644 --- a/compiler/rustc_lint/src/nonstandard_style.rs +++ b/compiler/rustc_lint/src/nonstandard_style.rs @@ -1,7 +1,9 @@ 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::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; @@ -495,17 +497,33 @@ impl NonUpperCaseGlobals { 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.clone() } + NonUpperCaseGlobalSub::Suggestion { + span: ident.span, + replace: uc.clone(), + applicability: if can_change_usages { + Applicability::MachineApplicable + } else { + Applicability::MaybeIncorrect + }, + } } else { NonUpperCaseGlobalSub::Label { span: ident.span } }; struct UsageCollector<'a, 'tcx> { cx: &'tcx LateContext<'a>, - did: LocalDefId, + did: DefId, collected: Vec, } @@ -521,10 +539,10 @@ impl NonUpperCaseGlobals { path: &rustc_hir::Path<'v>, _id: rustc_hir::HirId, ) -> Self::Result { - for seg in path.segments { - if seg.res.opt_def_id() == Some(self.did.to_def_id()) { - self.collected.push(seg.ident.span); - } + if let Some(final_seg) = path.segments.last() + && final_seg.res.opt_def_id() == Some(self.did) + { + self.collected.push(final_seg.ident.span); } } } @@ -532,10 +550,12 @@ impl NonUpperCaseGlobals { 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 let Some(did) = did + let usages = if can_change_usages && *name != uc + && let Some(did) = did { - let mut usage_collector = UsageCollector { cx, did, collected: Vec::new() }; + 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 diff --git a/tests/ui/lint/lint-non-uppercase-usages.fixed b/tests/ui/lint/lint-non-uppercase-usages.fixed index 048a936ff2730..231991dcae08c 100644 --- a/tests/ui/lint/lint-non-uppercase-usages.fixed +++ b/tests/ui/lint/lint-non-uppercase-usages.fixed @@ -15,6 +15,11 @@ const MY_STATIC: u32 = 0; 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 diff --git a/tests/ui/lint/lint-non-uppercase-usages.rs b/tests/ui/lint/lint-non-uppercase-usages.rs index b5b9ffac6bce0..9cdf5e47003d5 100644 --- a/tests/ui/lint/lint-non-uppercase-usages.rs +++ b/tests/ui/lint/lint-non-uppercase-usages.rs @@ -15,6 +15,11 @@ const my_static: u32 = 0; 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 diff --git a/tests/ui/lint/lint-non-uppercase-usages.stderr b/tests/ui/lint/lint-non-uppercase-usages.stderr index 34a0a5b0ca68d..7c7e573a88edc 100644 --- a/tests/ui/lint/lint-non-uppercase-usages.stderr +++ b/tests/ui/lint/lint-non-uppercase-usages.stderr @@ -12,7 +12,7 @@ LL + const MY_STATIC: u32 = 0; | warning: constant `fooFOO` should have an upper case name - --> $DIR/lint-non-uppercase-usages.rs:19:12 + --> $DIR/lint-non-uppercase-usages.rs:24:12 | LL | static fooFOO: Cell = unreachable!(); | ^^^^^^ @@ -24,7 +24,7 @@ LL + static FOO_FOO: Cell = unreachable!(); | warning: const parameter `foo` should have an upper case name - --> $DIR/lint-non-uppercase-usages.rs:24:14 + --> $DIR/lint-non-uppercase-usages.rs:29:14 | LL | fn foo() { | ^^^