Skip to content

Commit e379970

Browse files
euclioeuclio
committed
improve non_upper_case_globals diagnostics
Use a structured suggestion and tighten the span to just the identifier.
1 parent 7c0d145 commit e379970

13 files changed

+89
-103
lines changed

‎src/librustc_lint/nonstandard_style.rs

Lines changed: 27 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -356,21 +356,21 @@ declare_lint! {
356356
pub struct NonUpperCaseGlobals;
357357

358358
impl NonUpperCaseGlobals {
359-
fn check_upper_case(cx: &LateContext, sort: &str, name: ast::Name, span: Span) {
360-
if name.as_str().chars().any(|c| c.is_lowercase()) {
361-
let uc = NonSnakeCase::to_snake_case(&name.as_str()).to_uppercase();
362-
if name != &*uc {
363-
cx.span_lint(NON_UPPER_CASE_GLOBALS,
364-
span,
365-
&format!("{} `{}` should have an upper case name such as `{}`",
366-
sort,
367-
name,
368-
uc));
369-
} else {
370-
cx.span_lint(NON_UPPER_CASE_GLOBALS,
371-
span,
372-
&format!("{} `{}` should have an upper case name", sort, name));
373-
}
359+
fn check_upper_case(cx: &LateContext, sort: &str, ident: &Ident) {
360+
let name = &ident.name.as_str();
361+
362+
if name.chars().any(|c| c.is_lowercase()) {
363+
let uc = NonSnakeCase::to_snake_case(&name).to_uppercase();
364+
365+
let msg = format!("{} `{}` should have an upper case name", sort, name);
366+
cx.struct_span_lint(NON_UPPER_CASE_GLOBALS, ident.span, &msg)
367+
.span_suggestion_with_applicability(
368+
ident.span,
369+
"convert the identifier to upper case",
370+
uc,
371+
Applicability::MaybeIncorrect,
372+
)
373+
.emit();
374374
}
375375
}
376376
}
@@ -384,38 +384,25 @@ impl LintPass for NonUpperCaseGlobals {
384384
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NonUpperCaseGlobals {
385385
fn check_item(&mut self, cx: &LateContext, it: &hir::Item) {
386386
match it.node {
387-
hir::ItemKind::Static(..) => {
388-
if attr::find_by_name(&it.attrs, "no_mangle").is_some() {
389-
return;
390-
}
391-
NonUpperCaseGlobals::check_upper_case(cx, "static variable", it.ident.name,
392-
it.span);
387+
hir::ItemKind::Static(..) if !attr::contains_name(&it.attrs, "no_mangle") => {
388+
NonUpperCaseGlobals::check_upper_case(cx, "static variable", &it.ident);
393389
}
394390
hir::ItemKind::Const(..) => {
395-
NonUpperCaseGlobals::check_upper_case(cx, "constant", it.ident.name,
396-
it.span);
391+
NonUpperCaseGlobals::check_upper_case(cx, "constant", &it.ident);
397392
}
398393
_ => {}
399394
}
400395
}
401396

402397
fn check_trait_item(&mut self, cx: &LateContext, ti: &hir::TraitItem) {
403-
match ti.node {
404-
hir::TraitItemKind::Const(..) => {
405-
NonUpperCaseGlobals::check_upper_case(cx, "associated constant",
406-
ti.ident.name, ti.span);
407-
}
408-
_ => {}
398+
if let hir::TraitItemKind::Const(..) = ti.node {
399+
NonUpperCaseGlobals::check_upper_case(cx, "associated constant", &ti.ident);
409400
}
410401
}
411402

412403
fn check_impl_item(&mut self, cx: &LateContext, ii: &hir::ImplItem) {
413-
match ii.node {
414-
hir::ImplItemKind::Const(..) => {
415-
NonUpperCaseGlobals::check_upper_case(cx, "associated constant",
416-
ii.ident.name, ii.span);
417-
}
418-
_ => {}
404+
if let hir::ImplItemKind::Const(..) = ii.node {
405+
NonUpperCaseGlobals::check_upper_case(cx, "associated constant", &ii.ident);
419406
}
420407
}
421408

@@ -424,10 +411,11 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NonUpperCaseGlobals {
424411
if let PatKind::Path(hir::QPath::Resolved(None, ref path)) = p.node {
425412
if let Def::Const(..) = path.def {
426413
if path.segments.len() == 1 {
427-
NonUpperCaseGlobals::check_upper_case(cx,
428-
"constant in pattern",
429-
path.segments[0].ident.name,
430-
path.span);
414+
NonUpperCaseGlobals::check_upper_case(
415+
cx,
416+
"constant in pattern",
417+
&path.segments[0].ident
418+
);
431419
}
432420
}
433421
}

‎src/libsyntax_ext/test.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -124,14 +124,14 @@ pub fn expand_test_or_bench(
124124
])
125125
};
126126

127-
let mut test_const = cx.item(sp, item.ident.gensym(),
127+
let mut test_const = cx.item(sp, ast::Ident::new(item.ident.name.gensymed(), sp),
128128
vec![
129129
// #[cfg(test)]
130130
cx.attribute(attr_sp, cx.meta_list(attr_sp, Symbol::intern("cfg"), vec![
131131
cx.meta_list_item_word(attr_sp, Symbol::intern("test"))
132132
])),
133133
// #[rustc_test_marker]
134-
cx.attribute(attr_sp, cx.meta_word(attr_sp, Symbol::intern("rustc_test_marker")))
134+
cx.attribute(attr_sp, cx.meta_word(attr_sp, Symbol::intern("rustc_test_marker"))),
135135
],
136136
// const $ident: test::TestDescAndFn =
137137
ast::ItemKind::Const(cx.ty(sp, ast::TyKind::Path(None, test_path("TestDescAndFn"))),

‎src/test/ui/issues/issue-17718-const-naming.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
#![warn(unused)]
2-
#[deny(warnings)]
2+
#![deny(warnings)]
33

44
const foo: isize = 3;
5-
//~^ ERROR: should have an upper case name such as
5+
//~^ ERROR: should have an upper case name
66
//~^^ ERROR: constant item is never used
77

88
fn main() {}

‎src/test/ui/issues/issue-17718-const-naming.stderr

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,23 +5,23 @@ LL | const foo: isize = 3;
55
| ^^^^^^^^^^^^^^^^^^^^^
66
|
77
note: lint level defined here
8-
--> $DIR/issue-17718-const-naming.rs:2:8
8+
--> $DIR/issue-17718-const-naming.rs:2:9
99
|
10-
LL | #[deny(warnings)]
11-
| ^^^^^^^^
10+
LL | #![deny(warnings)]
11+
| ^^^^^^^^
1212
= note: #[deny(dead_code)] implied by #[deny(warnings)]
1313

14-
error: constant `foo` should have an upper case name such as `FOO`
15-
--> $DIR/issue-17718-const-naming.rs:4:1
14+
error: constant `foo` should have an upper case name
15+
--> $DIR/issue-17718-const-naming.rs:4:7
1616
|
1717
LL | const foo: isize = 3;
18-
| ^^^^^^^^^^^^^^^^^^^^^
18+
| ^^^ help: convert the identifier to upper case: `FOO`
1919
|
2020
note: lint level defined here
21-
--> $DIR/issue-17718-const-naming.rs:2:8
21+
--> $DIR/issue-17718-const-naming.rs:2:9
2222
|
23-
LL | #[deny(warnings)]
24-
| ^^^^^^^^
23+
LL | #![deny(warnings)]
24+
| ^^^^^^^^
2525
= note: #[deny(non_upper_case_globals)] implied by #[deny(warnings)]
2626

2727
error: aborting due to 2 previous errors

‎src/test/ui/lint/lint-group-nonstandard-style.stderr

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,11 +37,11 @@ LL | #[forbid(nonstandard_style)]
3737
| ^^^^^^^^^^^^^^^^^
3838
= note: #[forbid(non_snake_case)] implied by #[forbid(nonstandard_style)]
3939

40-
error: static variable `bad` should have an upper case name such as `BAD`
41-
--> $DIR/lint-group-nonstandard-style.rs:14:9
40+
error: static variable `bad` should have an upper case name
41+
--> $DIR/lint-group-nonstandard-style.rs:14:16
4242
|
4343
LL | static bad: isize = 1; //~ ERROR should have an upper
44-
| ^^^^^^^^^^^^^^^^^^^^^^
44+
| ^^^ help: convert the identifier to upper case: `BAD`
4545
|
4646
note: lint level defined here
4747
--> $DIR/lint-group-nonstandard-style.rs:10:14

‎src/test/run-pass/binding/match-static-const-rename.rs renamed to ‎src/test/ui/lint/lint-lowercase-static-const-pattern-rename.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,12 @@
1-
// run-pass
1+
// compile-pass
22
// Issue #7526: lowercase static constants in patterns look like bindings
33

4-
// This is similar to compile-fail/match-static-const-lc, except it
4+
// This is similar to lint-lowercase-static-const-pattern.rs, except it
55
// shows the expected usual workaround (choosing a different name for
66
// the static definition) and also demonstrates that one can work
77
// around this problem locally by renaming the constant in the `use`
88
// form to an uppercase identifier that placates the lint.
99

10-
1110
#![deny(non_upper_case_globals)]
1211

1312
pub const A : isize = 97;

‎src/test/ui/match/match-static-const-lc.rs renamed to ‎src/test/ui/lint/lint-lowercase-static-const-pattern.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ pub const a : isize = 97;
99
fn f() {
1010
let r = match (0,0) {
1111
(0, a) => 0,
12-
//~^ ERROR constant in pattern `a` should have an upper case name such as `A`
12+
//~^ ERROR constant in pattern `a` should have an upper case name
1313
(x, y) => 1 + x + y,
1414
};
1515
assert_eq!(r, 1);
@@ -24,7 +24,7 @@ fn g() {
2424
use self::m::aha;
2525
let r = match (0,0) {
2626
(0, aha) => 0,
27-
//~^ ERROR constant in pattern `aha` should have an upper case name such as `AHA`
27+
//~^ ERROR constant in pattern `aha` should have an upper case name
2828
(x, y) => 1 + x + y,
2929
};
3030
assert_eq!(r, 1);
@@ -38,7 +38,7 @@ fn h() {
3838
use self::n::OKAY as not_okay;
3939
let r = match (0,0) {
4040
(0, not_okay) => 0,
41-
//~^ ERROR constant in pattern `not_okay` should have an upper case name such as `NOT_OKAY`
41+
//~^ ERROR constant in pattern `not_okay` should have an upper case name
4242
(x, y) => 1 + x + y,
4343
};
4444
assert_eq!(r, 1);

‎src/test/ui/lint/lint-lowercase-static-const-pattern.stderr

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
error: constant in pattern `a` should have an upper case name
2+
--> $DIR/lint-lowercase-static-const-pattern.rs:11:13
3+
|
4+
LL | (0, a) => 0,
5+
| ^ help: convert the identifier to upper case: `A`
6+
|
7+
note: lint level defined here
8+
--> $DIR/lint-lowercase-static-const-pattern.rs:4:9
9+
|
10+
LL | #![deny(non_upper_case_globals)]
11+
| ^^^^^^^^^^^^^^^^^^^^^^
12+
13+
error: constant in pattern `aha` should have an upper case name
14+
--> $DIR/lint-lowercase-static-const-pattern.rs:26:13
15+
|
16+
LL | (0, aha) => 0,
17+
| ^^^ help: convert the identifier to upper case: `AHA`
18+
19+
error: constant in pattern `not_okay` should have an upper case name
20+
--> $DIR/lint-lowercase-static-const-pattern.rs:40:13
21+
|
22+
LL | (0, not_okay) => 0,
23+
| ^^^^^^^^ help: convert the identifier to upper case: `NOT_OKAY`
24+
25+
error: aborting due to 3 previous errors
26+

‎src/test/ui/associated-const/associated-const-upper-case-lint.rs renamed to ‎src/test/ui/lint/lint-non-uppercase-associated-const.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,6 @@ struct Foo;
66
impl Foo {
77
const not_upper: bool = true;
88
}
9-
//~^^ ERROR associated constant `not_upper` should have an upper case name such as `NOT_UPPER`
9+
//~^^ ERROR associated constant `not_upper` should have an upper case name
1010

1111
fn main() {}

‎src/test/ui/associated-const/associated-const-upper-case-lint.stderr renamed to ‎src/test/ui/lint/lint-non-uppercase-associated-const.stderr

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
1-
error: associated constant `not_upper` should have an upper case name such as `NOT_UPPER`
2-
--> $DIR/associated-const-upper-case-lint.rs:7:5
1+
error: associated constant `not_upper` should have an upper case name
2+
--> $DIR/lint-non-uppercase-associated-const.rs:7:11
33
|
44
LL | const not_upper: bool = true;
5-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
5+
| ^^^^^^^^^ help: convert the identifier to upper case: `NOT_UPPER`
66
|
77
note: lint level defined here
8-
--> $DIR/associated-const-upper-case-lint.rs:1:9
8+
--> $DIR/lint-non-uppercase-associated-const.rs:1:9
99
|
1010
LL | #![deny(non_upper_case_globals)]
1111
| ^^^^^^^^^^^^^^^^^^^^^^

‎src/test/ui/lint/lint-non-uppercase-statics.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,9 @@
11
#![forbid(non_upper_case_globals)]
22
#![allow(dead_code)]
33

4-
static foo: isize = 1; //~ ERROR static variable `foo` should have an upper case name such as `FOO`
4+
static foo: isize = 1; //~ ERROR static variable `foo` should have an upper case name
55

6-
static mut bar: isize = 1;
7-
//~^ ERROR static variable `bar` should have an upper case name such as `BAR`
6+
static mut bar: isize = 1; //~ ERROR static variable `bar` should have an upper case name
87

98
#[no_mangle]
109
pub static extern_foo: isize = 1; // OK, because #[no_mangle] supersedes the warning

‎src/test/ui/lint/lint-non-uppercase-statics.stderr

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,20 @@
1-
error: static variable `foo` should have an upper case name such as `FOO`
2-
--> $DIR/lint-non-uppercase-statics.rs:4:1
1+
error: static variable `foo` should have an upper case name
2+
--> $DIR/lint-non-uppercase-statics.rs:4:8
33
|
4-
LL | static foo: isize = 1; //~ ERROR static variable `foo` should have an upper case name such as `FOO`
5-
| ^^^^^^^^^^^^^^^^^^^^^^
4+
LL | static foo: isize = 1; //~ ERROR static variable `foo` should have an upper case name
5+
| ^^^ help: convert the identifier to upper case: `FOO`
66
|
77
note: lint level defined here
88
--> $DIR/lint-non-uppercase-statics.rs:1:11
99
|
1010
LL | #![forbid(non_upper_case_globals)]
1111
| ^^^^^^^^^^^^^^^^^^^^^^
1212

13-
error: static variable `bar` should have an upper case name such as `BAR`
14-
--> $DIR/lint-non-uppercase-statics.rs:6:1
13+
error: static variable `bar` should have an upper case name
14+
--> $DIR/lint-non-uppercase-statics.rs:6:12
1515
|
16-
LL | static mut bar: isize = 1;
17-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
16+
LL | static mut bar: isize = 1; //~ ERROR static variable `bar` should have an upper case name
17+
| ^^^ help: convert the identifier to upper case: `BAR`
1818

1919
error: aborting due to 2 previous errors
2020

‎src/test/ui/match/match-static-const-lc.stderr

Lines changed: 0 additions & 26 deletions
This file was deleted.

0 commit comments

Comments
 (0)