Skip to content

Commit 63dd59d

Browse files
Auto merge of #142645 - Urgau:usage-non_upper_case_globals, r=<try>
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
2 parents a30f178 + 6ffd0e6 commit 63dd59d

File tree

6 files changed

+252
-15
lines changed

6 files changed

+252
-15
lines changed

compiler/rustc_lint/src/context.rs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -524,6 +524,20 @@ pub trait LintContext {
524524
});
525525
}
526526

527+
/// Emit a lint at `span` from a lazily-constructed lint struct (some type that implements
528+
/// `LintDiagnostic`, typically generated by `#[derive(LintDiagnostic)]`).
529+
fn emit_span_lint_lazy<S: Into<MultiSpan>, L: for<'a> LintDiagnostic<'a, ()>>(
530+
&self,
531+
lint: &'static Lint,
532+
span: S,
533+
decorator: impl FnOnce() -> L,
534+
) {
535+
self.opt_span_lint(lint, Some(span), |lint| {
536+
let decorator = decorator();
537+
decorator.decorate_lint(lint);
538+
});
539+
}
540+
527541
/// Emit a lint at the appropriate level, with an associated span.
528542
///
529543
/// [`lint_level`]: rustc_middle::lint::lint_level#decorate-signature

compiler/rustc_lint/src/lints.rs

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1353,6 +1353,8 @@ pub(crate) struct NonUpperCaseGlobal<'a> {
13531353
pub name: &'a str,
13541354
#[subdiagnostic]
13551355
pub sub: NonUpperCaseGlobalSub,
1356+
#[subdiagnostic]
1357+
pub usages: Vec<NonUpperCaseGlobalSubTool>,
13561358
}
13571359

13581360
#[derive(Subdiagnostic)]
@@ -1362,14 +1364,29 @@ pub(crate) enum NonUpperCaseGlobalSub {
13621364
#[primary_span]
13631365
span: Span,
13641366
},
1365-
#[suggestion(lint_suggestion, code = "{replace}", applicability = "maybe-incorrect")]
1367+
#[suggestion(lint_suggestion, code = "{replace}")]
13661368
Suggestion {
13671369
#[primary_span]
13681370
span: Span,
1371+
#[applicability]
1372+
applicability: Applicability,
13691373
replace: String,
13701374
},
13711375
}
13721376

1377+
#[derive(Subdiagnostic)]
1378+
#[suggestion(
1379+
lint_suggestion,
1380+
code = "{replace}",
1381+
applicability = "machine-applicable",
1382+
style = "tool-only"
1383+
)]
1384+
pub(crate) struct NonUpperCaseGlobalSubTool {
1385+
#[primary_span]
1386+
pub(crate) span: Span,
1387+
pub(crate) replace: String,
1388+
}
1389+
13731390
// noop_method_call.rs
13741391
#[derive(LintDiagnostic)]
13751392
#[diag(lint_noop_method_call)]

compiler/rustc_lint/src/nonstandard_style.rs

Lines changed: 93 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,12 @@
11
use rustc_abi::ExternAbi;
22
use rustc_attr_data_structures::{AttributeKind, ReprAttr};
33
use rustc_attr_parsing::AttributeParser;
4+
use rustc_errors::Applicability;
45
use rustc_hir::def::{DefKind, Res};
5-
use rustc_hir::intravisit::FnKind;
6+
use rustc_hir::def_id::DefId;
7+
use rustc_hir::intravisit::{FnKind, Visitor};
68
use rustc_hir::{AttrArgs, AttrItem, Attribute, GenericParamKind, PatExprKind, PatKind};
9+
use rustc_middle::hir::nested_filter::All;
710
use rustc_middle::ty;
811
use rustc_session::config::CrateType;
912
use rustc_session::{declare_lint, declare_lint_pass};
@@ -13,7 +16,7 @@ use {rustc_ast as ast, rustc_hir as hir};
1316

1417
use crate::lints::{
1518
NonCamelCaseType, NonCamelCaseTypeSub, NonSnakeCaseDiag, NonSnakeCaseDiagSub,
16-
NonUpperCaseGlobal, NonUpperCaseGlobalSub,
19+
NonUpperCaseGlobal, NonUpperCaseGlobalSub, NonUpperCaseGlobalSubTool,
1720
};
1821
use crate::{EarlyContext, EarlyLintPass, LateContext, LateLintPass, LintContext};
1922

@@ -489,22 +492,82 @@ declare_lint! {
489492
declare_lint_pass!(NonUpperCaseGlobals => [NON_UPPER_CASE_GLOBALS]);
490493

491494
impl NonUpperCaseGlobals {
492-
fn check_upper_case(cx: &LateContext<'_>, sort: &str, ident: &Ident) {
495+
fn check_upper_case(cx: &LateContext<'_>, sort: &str, did: Option<LocalDefId>, ident: &Ident) {
493496
let name = ident.name.as_str();
494497
if name.chars().any(|c| c.is_lowercase()) {
495498
let uc = NonSnakeCase::to_snake_case(name).to_uppercase();
499+
500+
// If the item is exported, suggesting changing it's name would be breaking-change
501+
// and could break users without a "nice" applicable fix, so let's avoid it.
502+
let can_change_usages = if let Some(did) = did {
503+
!cx.tcx.effective_visibilities(()).is_exported(did)
504+
} else {
505+
false
506+
};
507+
496508
// We cannot provide meaningful suggestions
497509
// if the characters are in the category of "Lowercase Letter".
498510
let sub = if *name != uc {
499-
NonUpperCaseGlobalSub::Suggestion { span: ident.span, replace: uc }
511+
NonUpperCaseGlobalSub::Suggestion {
512+
span: ident.span,
513+
replace: uc.clone(),
514+
applicability: if can_change_usages {
515+
Applicability::MachineApplicable
516+
} else {
517+
Applicability::MaybeIncorrect
518+
},
519+
}
500520
} else {
501521
NonUpperCaseGlobalSub::Label { span: ident.span }
502522
};
503-
cx.emit_span_lint(
504-
NON_UPPER_CASE_GLOBALS,
505-
ident.span,
506-
NonUpperCaseGlobal { sort, name, sub },
507-
);
523+
524+
struct UsageCollector<'a, 'tcx> {
525+
cx: &'tcx LateContext<'a>,
526+
did: DefId,
527+
collected: Vec<Span>,
528+
}
529+
530+
impl<'v, 'tcx> Visitor<'v> for UsageCollector<'v, 'tcx> {
531+
type NestedFilter = All;
532+
533+
fn maybe_tcx(&mut self) -> Self::MaybeTyCtxt {
534+
self.cx.tcx
535+
}
536+
537+
fn visit_path(
538+
&mut self,
539+
path: &rustc_hir::Path<'v>,
540+
_id: rustc_hir::HirId,
541+
) -> Self::Result {
542+
if let Some(final_seg) = path.segments.last()
543+
&& final_seg.res.opt_def_id() == Some(self.did)
544+
{
545+
self.collected.push(final_seg.ident.span);
546+
}
547+
}
548+
}
549+
550+
cx.emit_span_lint_lazy(NON_UPPER_CASE_GLOBALS, ident.span, || {
551+
// Compute usages lazily as it can expansive and useless when the lint is allowed.
552+
// cf. https://github.com/rust-lang/rust/pull/142645#issuecomment-2993024625
553+
let usages = if can_change_usages
554+
&& *name != uc
555+
&& let Some(did) = did
556+
{
557+
let mut usage_collector =
558+
UsageCollector { cx, did: did.to_def_id(), collected: Vec::new() };
559+
cx.tcx.hir_walk_toplevel_module(&mut usage_collector);
560+
usage_collector
561+
.collected
562+
.into_iter()
563+
.map(|span| NonUpperCaseGlobalSubTool { span, replace: uc.clone() })
564+
.collect()
565+
} else {
566+
vec![]
567+
};
568+
569+
NonUpperCaseGlobal { sort, name, sub, usages }
570+
});
508571
}
509572
}
510573
}
@@ -516,26 +579,36 @@ impl<'tcx> LateLintPass<'tcx> for NonUpperCaseGlobals {
516579
hir::ItemKind::Static(_, ident, ..)
517580
if !ast::attr::contains_name(attrs, sym::no_mangle) =>
518581
{
519-
NonUpperCaseGlobals::check_upper_case(cx, "static variable", &ident);
582+
NonUpperCaseGlobals::check_upper_case(
583+
cx,
584+
"static variable",
585+
Some(it.owner_id.def_id),
586+
&ident,
587+
);
520588
}
521589
hir::ItemKind::Const(ident, ..) => {
522-
NonUpperCaseGlobals::check_upper_case(cx, "constant", &ident);
590+
NonUpperCaseGlobals::check_upper_case(
591+
cx,
592+
"constant",
593+
Some(it.owner_id.def_id),
594+
&ident,
595+
);
523596
}
524597
_ => {}
525598
}
526599
}
527600

528601
fn check_trait_item(&mut self, cx: &LateContext<'_>, ti: &hir::TraitItem<'_>) {
529602
if let hir::TraitItemKind::Const(..) = ti.kind {
530-
NonUpperCaseGlobals::check_upper_case(cx, "associated constant", &ti.ident);
603+
NonUpperCaseGlobals::check_upper_case(cx, "associated constant", None, &ti.ident);
531604
}
532605
}
533606

534607
fn check_impl_item(&mut self, cx: &LateContext<'_>, ii: &hir::ImplItem<'_>) {
535608
if let hir::ImplItemKind::Const(..) = ii.kind
536609
&& !assoc_item_in_trait_impl(cx, ii)
537610
{
538-
NonUpperCaseGlobals::check_upper_case(cx, "associated constant", &ii.ident);
611+
NonUpperCaseGlobals::check_upper_case(cx, "associated constant", None, &ii.ident);
539612
}
540613
}
541614

@@ -551,6 +624,7 @@ impl<'tcx> LateLintPass<'tcx> for NonUpperCaseGlobals {
551624
NonUpperCaseGlobals::check_upper_case(
552625
cx,
553626
"constant in pattern",
627+
None,
554628
&segment.ident,
555629
);
556630
}
@@ -560,7 +634,12 @@ impl<'tcx> LateLintPass<'tcx> for NonUpperCaseGlobals {
560634

561635
fn check_generic_param(&mut self, cx: &LateContext<'_>, param: &hir::GenericParam<'_>) {
562636
if let GenericParamKind::Const { .. } = param.kind {
563-
NonUpperCaseGlobals::check_upper_case(cx, "const parameter", &param.name.ident());
637+
NonUpperCaseGlobals::check_upper_case(
638+
cx,
639+
"const parameter",
640+
Some(param.def_id),
641+
&param.name.ident(),
642+
);
564643
}
565644
}
566645
}
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
// Checks that the `non_upper_case_globals` emits suggestions for usages as well
2+
// <https://github.com/rust-lang/rust/issues/124061>
3+
4+
//@ check-pass
5+
//@ run-rustfix
6+
7+
#![allow(dead_code)]
8+
9+
use std::cell::Cell;
10+
11+
const MY_STATIC: u32 = 0;
12+
//~^ WARN constant `my_static` should have an upper case name
13+
//~| SUGGESTION MY_STATIC
14+
15+
const LOL: u32 = MY_STATIC + 0;
16+
//~^ SUGGESTION MY_STATIC
17+
18+
mod my_mod {
19+
const INSIDE_MOD: u32 = super::MY_STATIC + 0;
20+
//~^ SUGGESTION MY_STATIC
21+
}
22+
23+
thread_local! {
24+
static FOO_FOO: Cell<usize> = unreachable!();
25+
//~^ WARN constant `fooFOO` should have an upper case name
26+
//~| SUGGESTION FOO_FOO
27+
}
28+
29+
fn foo<const FOO: u32>() {
30+
//~^ WARN const parameter `foo` should have an upper case name
31+
//~| SUGGESTION FOO
32+
let _a = FOO + 1;
33+
//~^ SUGGESTION FOO
34+
}
35+
36+
fn main() {
37+
let _a = crate::MY_STATIC;
38+
//~^ SUGGESTION MY_STATIC
39+
40+
FOO_FOO.set(9);
41+
//~^ SUGGESTION FOO_FOO
42+
println!("{}", FOO_FOO.get());
43+
//~^ SUGGESTION FOO_FOO
44+
}
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
// Checks that the `non_upper_case_globals` emits suggestions for usages as well
2+
// <https://github.com/rust-lang/rust/issues/124061>
3+
4+
//@ check-pass
5+
//@ run-rustfix
6+
7+
#![allow(dead_code)]
8+
9+
use std::cell::Cell;
10+
11+
const my_static: u32 = 0;
12+
//~^ WARN constant `my_static` should have an upper case name
13+
//~| SUGGESTION MY_STATIC
14+
15+
const LOL: u32 = my_static + 0;
16+
//~^ SUGGESTION MY_STATIC
17+
18+
mod my_mod {
19+
const INSIDE_MOD: u32 = super::my_static + 0;
20+
//~^ SUGGESTION MY_STATIC
21+
}
22+
23+
thread_local! {
24+
static fooFOO: Cell<usize> = unreachable!();
25+
//~^ WARN constant `fooFOO` should have an upper case name
26+
//~| SUGGESTION FOO_FOO
27+
}
28+
29+
fn foo<const foo: u32>() {
30+
//~^ WARN const parameter `foo` should have an upper case name
31+
//~| SUGGESTION FOO
32+
let _a = foo + 1;
33+
//~^ SUGGESTION FOO
34+
}
35+
36+
fn main() {
37+
let _a = crate::my_static;
38+
//~^ SUGGESTION MY_STATIC
39+
40+
fooFOO.set(9);
41+
//~^ SUGGESTION FOO_FOO
42+
println!("{}", fooFOO.get());
43+
//~^ SUGGESTION FOO_FOO
44+
}
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
warning: constant `my_static` should have an upper case name
2+
--> $DIR/lint-non-uppercase-usages.rs:11:7
3+
|
4+
LL | const my_static: u32 = 0;
5+
| ^^^^^^^^^
6+
|
7+
= note: `#[warn(non_upper_case_globals)]` on by default
8+
help: convert the identifier to upper case
9+
|
10+
LL - const my_static: u32 = 0;
11+
LL + const MY_STATIC: u32 = 0;
12+
|
13+
14+
warning: constant `fooFOO` should have an upper case name
15+
--> $DIR/lint-non-uppercase-usages.rs:24:12
16+
|
17+
LL | static fooFOO: Cell<usize> = unreachable!();
18+
| ^^^^^^
19+
|
20+
help: convert the identifier to upper case
21+
|
22+
LL - static fooFOO: Cell<usize> = unreachable!();
23+
LL + static FOO_FOO: Cell<usize> = unreachable!();
24+
|
25+
26+
warning: const parameter `foo` should have an upper case name
27+
--> $DIR/lint-non-uppercase-usages.rs:29:14
28+
|
29+
LL | fn foo<const foo: u32>() {
30+
| ^^^
31+
|
32+
help: convert the identifier to upper case (notice the capitalization difference)
33+
|
34+
LL - fn foo<const foo: u32>() {
35+
LL + fn foo<const FOO: u32>() {
36+
|
37+
38+
warning: 3 warnings emitted
39+

0 commit comments

Comments
 (0)