Also emit suggestions for usages in the `non_upper_case_globals` lint #142645

Urgau · 2025-06-17T21:28:50Z

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

rustbot · 2025-06-17T21:28:55Z

rustbot has assigned @fee1-dead.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

compiler-errors · 2025-06-17T21:37:38Z

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

+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;


This seems pretty noisy. Could you convert this to a multipart suggestion?

you could perhaps tweak the help: to say something like "convert the identifier and its usages to upper case"

Sure, converted a multipart suggestion.

EDIT: Didn't see your second message, will fix that tomorrow.

fmease · 2025-06-17T22:20:36Z

compiler/rustc_lint/src/lints.rs

I wonder if the original issue author meant "I ran cargo fix / cargo clippy --fix" when they wrote "I applied the fixes suggested by clippy and rustc.".

I would probably mark the suggestions for usage sites as tool_only, so they don't spam the terminal but still get applied automatically by rustfix.

cc @PartiallyUntyped

It's true that it doesn't bring much to show all of them and it can quickly become overwhelming. What do you think @compiler-errors ?

Pushed a version with the suggestions as tool only, so they won't clutter the users terminal.

fmease · 2025-06-20T16:31:56Z

compiler/rustc_lint/src/lints.rs

+#[suggestion(
+    lint_suggestion,
+    code = "{replace}",
+    applicability = "maybe-incorrect",


Does cargo fix actually apply maybe-incorrect suggestions? Or does it only apply machine-applicable ones? If cargo fix doesn't actually end up auto-fixing these, I'm not sure if it's worth having them at all 🤔

Follow-up question: Why is it actually maybe-incorrect? Because of the existence of macros?

I shameslly copied from the main suggestion NonUpperCaseGlobalSub which also has maybe-incorrect.

That's a very good point, cargo fix doesn't seems apply "maybe-incorrect" suggestions. (we are being tricked by compiletest/rustfix here)

Doing some archeology until #57387 doesn't reveal much why it's "maybe-incorrect" but if I had to guess I would say it's because it would lead to cargo fix throwing a error that there now would be errors due to the other ones not being renamed ... but since we are now renaming the other ones I think it's fine to put it at "machine-applicable".

fmease · 2025-06-20T17:51:05Z

This PR currently collects use sites eagerly, i.e., before the indirect call to lint_level which ultimately decides if the lint should be emitted or not. I wonder if this is noticeable perf-wise.

If this ends up perf heavy, it should be possible to turn this into a LintDiagnostic that computes these tool-only suggs lazily.

@bors2 try @rust-timer queue

rust-bors · 2025-06-20T17:51:09Z

⌛ Trying commit 09d0a73 with merge 4bb20cc…

To cancel the try build, run the command @bors2 try cancel.

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

rust-bors · 2025-06-20T20:20:14Z

☀️ Try build successful (CI)
Build commit: 4bb20cc (4bb20cc8be9c984fb2d6235080e1f44f349e6b9f, parent: 9c4ff566babe632af5e30281a822d1ae9972873b)

rust-timer · 2025-06-20T22:07:48Z

Finished benchmarking commit (4bb20cc): comparison URL.

Overall result: ❌ regressions - please read the text below

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please do so in sufficient writing along with @rustbot label: +perf-regression-triaged. If not, please fix the regressions and do another perf run. If its results are neutral or positive, the label will be automatically removed.

@bors rollup=never
@rustbot label: -S-waiting-on-perf +perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

	mean	range	count
Regressions ❌ (primary)	41.4%	[33.7%, 55.9%]	9
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	41.4%	[33.7%, 55.9%]	9

Max RSS (memory usage)

Results (primary 1.2%, secondary -4.2%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	3.1%	[3.1%, 3.1%]	1
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-0.7%	[-0.7%, -0.7%]	1
Improvements ✅ (secondary)	-4.2%	[-6.0%, -2.3%]	2
All ❌✅ (primary)	1.2%	[-0.7%, 3.1%]	2

Cycles

Results (primary 30.3%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	34.0%	[23.9%, 47.1%]	9
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-2.6%	[-2.6%, -2.6%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	30.3%	[-2.6%, 47.1%]	10

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 692.606s -> 715.256s (3.27%)
Artifact size: 372.01 MiB -> 372.00 MiB (-0.00%)

rustbot assigned fee1-dead Jun 17, 2025

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jun 17, 2025

compiler-errors requested changes Jun 17, 2025

View reviewed changes

compiler-errors assigned compiler-errors and unassigned fee1-dead Jun 17, 2025

Also emit suggestions for usages in the non_upper_case_globals lint

42bb66a

Urgau force-pushed the usage-non_upper_case_globals branch from 2589fe0 to 17f95ca Compare June 17, 2025 22:07

fmease reviewed Jun 17, 2025

View reviewed changes

Emit the usages suggestions as tool-only suggestions

4df9f2f

Urgau force-pushed the usage-non_upper_case_globals branch from 17f95ca to 4df9f2f Compare June 20, 2025 16:16

fmease self-assigned this Jun 20, 2025

fmease reviewed Jun 20, 2025

View reviewed changes

Switch non_upper_case_globals suggestions to being machine-applicable

09d0a73

This comment has been minimized.

Sign in to view

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jun 20, 2025

This comment has been minimized.

Sign in to view

rustbot added perf-regression Performance regression. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Jun 20, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Also emit suggestions for usages in the `non_upper_case_globals` lint #142645

Also emit suggestions for usages in the `non_upper_case_globals` lint #142645

Urgau commented Jun 17, 2025

Uh oh!

rustbot commented Jun 17, 2025

Uh oh!

compiler-errors Jun 17, 2025

Uh oh!

compiler-errors Jun 17, 2025

Uh oh!

Urgau Jun 17, 2025 •

edited

Loading

Uh oh!

fmease Jun 17, 2025 •

edited

Loading

Uh oh!

Urgau Jun 18, 2025

Uh oh!

Urgau Jun 20, 2025

Uh oh!

fmease Jun 20, 2025 •

edited

Loading

Uh oh!

Urgau Jun 20, 2025

Uh oh!

fmease commented Jun 20, 2025

Uh oh!

This comment has been minimized.

rust-bors bot commented Jun 20, 2025

Uh oh!

rust-bors bot commented Jun 20, 2025

Uh oh!

This comment has been minimized.

rust-timer commented Jun 20, 2025

Uh oh!

Uh oh!

Also emit suggestions for usages in the non_upper_case_globals lint #142645

Are you sure you want to change the base?

Also emit suggestions for usages in the non_upper_case_globals lint #142645

Conversation

Urgau commented Jun 17, 2025

Uh oh!

rustbot commented Jun 17, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Urgau Jun 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fmease Jun 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fmease Jun 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fmease commented Jun 20, 2025

Uh oh!

This comment has been minimized.

rust-bors bot commented Jun 20, 2025

Uh oh!

rust-bors bot commented Jun 20, 2025

Uh oh!

This comment has been minimized.

rust-timer commented Jun 20, 2025

Overall result: ❌ regressions - please read the text below

Uh oh!

Uh oh!

Also emit suggestions for usages in the `non_upper_case_globals` lint #142645

Also emit suggestions for usages in the `non_upper_case_globals` lint #142645

Urgau Jun 17, 2025 •

edited

Loading

fmease Jun 17, 2025 •

edited

Loading

fmease Jun 20, 2025 •

edited

Loading