Skip to content

Also emit suggestions for usages in the non_upper_case_globals lint #142645

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions compiler/rustc_lint/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
19 changes: 18 additions & 1 deletion compiler/rustc_lint/src/lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1348,6 +1348,8 @@ pub(crate) struct NonUpperCaseGlobal<'a> {
pub name: &'a str,
#[subdiagnostic]
pub sub: NonUpperCaseGlobalSub,
#[subdiagnostic]
pub usages: Vec<NonUpperCaseGlobalSubTool>,
}

#[derive(Subdiagnostic)]
Expand All @@ -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)]
Expand Down
98 changes: 84 additions & 14 deletions compiler/rustc_lint/src/nonstandard_style.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand All @@ -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};

Expand Down Expand Up @@ -489,22 +492,73 @@ 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();

// 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 did.is_some() {
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 {
for seg in path.segments {
Copy link
Member

@fmease fmease Jun 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't it be sufficient to just look at the Res of the last segment or just at path.res? STATIC::something and CONST::something are always an error and will likely always be one.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could but I don't think that would change much as we would still (I think) iterate over the segments to find the span. It's also (no longer) an expensive check anyway.

if seg.res.opt_def_id() == Some(self.did) {
self.collected.push(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 let Some(did) = did
&& *name != uc
{
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 }
});
}
}
}
Expand All @@ -516,26 +570,36 @@ 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,
);
}
_ => {}
}
}

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);
}
}

Expand All @@ -551,6 +615,7 @@ impl<'tcx> LateLintPass<'tcx> for NonUpperCaseGlobals {
NonUpperCaseGlobals::check_upper_case(
cx,
"constant in pattern",
None,
&segment.ident,
);
}
Expand All @@ -560,7 +625,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(),
);
}
}
}
Expand Down
44 changes: 44 additions & 0 deletions tests/ui/lint/lint-non-uppercase-usages.fixed
Original file line number Diff line number Diff line change
@@ -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
}
44 changes: 44 additions & 0 deletions tests/ui/lint/lint-non-uppercase-usages.rs
Original file line number Diff line number Diff line change
@@ -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
}
39 changes: 39 additions & 0 deletions tests/ui/lint/lint-non-uppercase-usages.stderr
Original file line number Diff line number Diff line change
@@ -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

Loading