From e37997005601e838aa221dc5be087e827df57264 Mon Sep 17 00:00:00 2001 From: Andy Russell Date: Fri, 4 Jan 2019 15:00:15 -0500 Subject: [PATCH] improve non_upper_case_globals diagnostics Use a structured suggestion and tighten the span to just the identifier. --- src/librustc_lint/nonstandard_style.rs | 66 ++++++++----------- src/libsyntax_ext/test.rs | 4 +- .../ui/issues/issue-17718-const-naming.rs | 4 +- .../ui/issues/issue-17718-const-naming.stderr | 18 ++--- .../lint/lint-group-nonstandard-style.stderr | 6 +- ...-lowercase-static-const-pattern-rename.rs} | 5 +- .../lint-lowercase-static-const-pattern.rs} | 6 +- ...lint-lowercase-static-const-pattern.stderr | 26 ++++++++ .../lint-non-uppercase-associated-const.rs} | 2 +- ...int-non-uppercase-associated-const.stderr} | 8 +-- .../ui/lint/lint-non-uppercase-statics.rs | 5 +- .../ui/lint/lint-non-uppercase-statics.stderr | 16 ++--- .../ui/match/match-static-const-lc.stderr | 26 -------- 13 files changed, 89 insertions(+), 103 deletions(-) rename src/test/{run-pass/binding/match-static-const-rename.rs => ui/lint/lint-lowercase-static-const-pattern-rename.rs} (93%) rename src/test/ui/{match/match-static-const-lc.rs => lint/lint-lowercase-static-const-pattern.rs} (92%) create mode 100644 src/test/ui/lint/lint-lowercase-static-const-pattern.stderr rename src/test/ui/{associated-const/associated-const-upper-case-lint.rs => lint/lint-non-uppercase-associated-const.rs} (86%) rename src/test/ui/{associated-const/associated-const-upper-case-lint.stderr => lint/lint-non-uppercase-associated-const.stderr} (57%) delete mode 100644 src/test/ui/match/match-static-const-lc.stderr diff --git a/src/librustc_lint/nonstandard_style.rs b/src/librustc_lint/nonstandard_style.rs index fa6665d1ae73..d2fdaab93151 100644 --- a/src/librustc_lint/nonstandard_style.rs +++ b/src/librustc_lint/nonstandard_style.rs @@ -356,21 +356,21 @@ declare_lint! { pub struct NonUpperCaseGlobals; impl NonUpperCaseGlobals { - fn check_upper_case(cx: &LateContext, sort: &str, name: ast::Name, span: Span) { - if name.as_str().chars().any(|c| c.is_lowercase()) { - let uc = NonSnakeCase::to_snake_case(&name.as_str()).to_uppercase(); - if name != &*uc { - cx.span_lint(NON_UPPER_CASE_GLOBALS, - span, - &format!("{} `{}` should have an upper case name such as `{}`", - sort, - name, - uc)); - } else { - cx.span_lint(NON_UPPER_CASE_GLOBALS, - span, - &format!("{} `{}` should have an upper case name", sort, name)); - } + fn check_upper_case(cx: &LateContext, sort: &str, 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(); + + let msg = format!("{} `{}` should have an upper case name", sort, name); + cx.struct_span_lint(NON_UPPER_CASE_GLOBALS, ident.span, &msg) + .span_suggestion_with_applicability( + ident.span, + "convert the identifier to upper case", + uc, + Applicability::MaybeIncorrect, + ) + .emit(); } } } @@ -384,38 +384,25 @@ impl LintPass for NonUpperCaseGlobals { impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NonUpperCaseGlobals { fn check_item(&mut self, cx: &LateContext, it: &hir::Item) { match it.node { - hir::ItemKind::Static(..) => { - if attr::find_by_name(&it.attrs, "no_mangle").is_some() { - return; - } - NonUpperCaseGlobals::check_upper_case(cx, "static variable", it.ident.name, - it.span); + hir::ItemKind::Static(..) if !attr::contains_name(&it.attrs, "no_mangle") => { + NonUpperCaseGlobals::check_upper_case(cx, "static variable", &it.ident); } hir::ItemKind::Const(..) => { - NonUpperCaseGlobals::check_upper_case(cx, "constant", it.ident.name, - it.span); + NonUpperCaseGlobals::check_upper_case(cx, "constant", &it.ident); } _ => {} } } fn check_trait_item(&mut self, cx: &LateContext, ti: &hir::TraitItem) { - match ti.node { - hir::TraitItemKind::Const(..) => { - NonUpperCaseGlobals::check_upper_case(cx, "associated constant", - ti.ident.name, ti.span); - } - _ => {} + if let hir::TraitItemKind::Const(..) = ti.node { + NonUpperCaseGlobals::check_upper_case(cx, "associated constant", &ti.ident); } } fn check_impl_item(&mut self, cx: &LateContext, ii: &hir::ImplItem) { - match ii.node { - hir::ImplItemKind::Const(..) => { - NonUpperCaseGlobals::check_upper_case(cx, "associated constant", - ii.ident.name, ii.span); - } - _ => {} + if let hir::ImplItemKind::Const(..) = ii.node { + NonUpperCaseGlobals::check_upper_case(cx, "associated constant", &ii.ident); } } @@ -424,10 +411,11 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NonUpperCaseGlobals { if let PatKind::Path(hir::QPath::Resolved(None, ref path)) = p.node { if let Def::Const(..) = path.def { if path.segments.len() == 1 { - NonUpperCaseGlobals::check_upper_case(cx, - "constant in pattern", - path.segments[0].ident.name, - path.span); + NonUpperCaseGlobals::check_upper_case( + cx, + "constant in pattern", + &path.segments[0].ident + ); } } } diff --git a/src/libsyntax_ext/test.rs b/src/libsyntax_ext/test.rs index cf842dddeb3d..a19d0458edd8 100644 --- a/src/libsyntax_ext/test.rs +++ b/src/libsyntax_ext/test.rs @@ -124,14 +124,14 @@ pub fn expand_test_or_bench( ]) }; - let mut test_const = cx.item(sp, item.ident.gensym(), + let mut test_const = cx.item(sp, ast::Ident::new(item.ident.name.gensymed(), sp), vec![ // #[cfg(test)] cx.attribute(attr_sp, cx.meta_list(attr_sp, Symbol::intern("cfg"), vec![ cx.meta_list_item_word(attr_sp, Symbol::intern("test")) ])), // #[rustc_test_marker] - cx.attribute(attr_sp, cx.meta_word(attr_sp, Symbol::intern("rustc_test_marker"))) + cx.attribute(attr_sp, cx.meta_word(attr_sp, Symbol::intern("rustc_test_marker"))), ], // const $ident: test::TestDescAndFn = ast::ItemKind::Const(cx.ty(sp, ast::TyKind::Path(None, test_path("TestDescAndFn"))), diff --git a/src/test/ui/issues/issue-17718-const-naming.rs b/src/test/ui/issues/issue-17718-const-naming.rs index 9b3d84b3dd51..d30b95843f30 100644 --- a/src/test/ui/issues/issue-17718-const-naming.rs +++ b/src/test/ui/issues/issue-17718-const-naming.rs @@ -1,8 +1,8 @@ #![warn(unused)] -#[deny(warnings)] +#![deny(warnings)] const foo: isize = 3; -//~^ ERROR: should have an upper case name such as +//~^ ERROR: should have an upper case name //~^^ ERROR: constant item is never used fn main() {} diff --git a/src/test/ui/issues/issue-17718-const-naming.stderr b/src/test/ui/issues/issue-17718-const-naming.stderr index 641e50a5fc9b..b92acecb83ec 100644 --- a/src/test/ui/issues/issue-17718-const-naming.stderr +++ b/src/test/ui/issues/issue-17718-const-naming.stderr @@ -5,23 +5,23 @@ LL | const foo: isize = 3; | ^^^^^^^^^^^^^^^^^^^^^ | note: lint level defined here - --> $DIR/issue-17718-const-naming.rs:2:8 + --> $DIR/issue-17718-const-naming.rs:2:9 | -LL | #[deny(warnings)] - | ^^^^^^^^ +LL | #![deny(warnings)] + | ^^^^^^^^ = note: #[deny(dead_code)] implied by #[deny(warnings)] -error: constant `foo` should have an upper case name such as `FOO` - --> $DIR/issue-17718-const-naming.rs:4:1 +error: constant `foo` should have an upper case name + --> $DIR/issue-17718-const-naming.rs:4:7 | LL | const foo: isize = 3; - | ^^^^^^^^^^^^^^^^^^^^^ + | ^^^ help: convert the identifier to upper case: `FOO` | note: lint level defined here - --> $DIR/issue-17718-const-naming.rs:2:8 + --> $DIR/issue-17718-const-naming.rs:2:9 | -LL | #[deny(warnings)] - | ^^^^^^^^ +LL | #![deny(warnings)] + | ^^^^^^^^ = note: #[deny(non_upper_case_globals)] implied by #[deny(warnings)] error: aborting due to 2 previous errors diff --git a/src/test/ui/lint/lint-group-nonstandard-style.stderr b/src/test/ui/lint/lint-group-nonstandard-style.stderr index 97e3ce6fd371..176f246ad763 100644 --- a/src/test/ui/lint/lint-group-nonstandard-style.stderr +++ b/src/test/ui/lint/lint-group-nonstandard-style.stderr @@ -37,11 +37,11 @@ LL | #[forbid(nonstandard_style)] | ^^^^^^^^^^^^^^^^^ = note: #[forbid(non_snake_case)] implied by #[forbid(nonstandard_style)] -error: static variable `bad` should have an upper case name such as `BAD` - --> $DIR/lint-group-nonstandard-style.rs:14:9 +error: static variable `bad` should have an upper case name + --> $DIR/lint-group-nonstandard-style.rs:14:16 | LL | static bad: isize = 1; //~ ERROR should have an upper - | ^^^^^^^^^^^^^^^^^^^^^^ + | ^^^ help: convert the identifier to upper case: `BAD` | note: lint level defined here --> $DIR/lint-group-nonstandard-style.rs:10:14 diff --git a/src/test/run-pass/binding/match-static-const-rename.rs b/src/test/ui/lint/lint-lowercase-static-const-pattern-rename.rs similarity index 93% rename from src/test/run-pass/binding/match-static-const-rename.rs rename to src/test/ui/lint/lint-lowercase-static-const-pattern-rename.rs index e78ae40da2fa..8ca5af21630a 100644 --- a/src/test/run-pass/binding/match-static-const-rename.rs +++ b/src/test/ui/lint/lint-lowercase-static-const-pattern-rename.rs @@ -1,13 +1,12 @@ -// run-pass +// compile-pass // Issue #7526: lowercase static constants in patterns look like bindings -// This is similar to compile-fail/match-static-const-lc, except it +// This is similar to lint-lowercase-static-const-pattern.rs, except it // shows the expected usual workaround (choosing a different name for // the static definition) and also demonstrates that one can work // around this problem locally by renaming the constant in the `use` // form to an uppercase identifier that placates the lint. - #![deny(non_upper_case_globals)] pub const A : isize = 97; diff --git a/src/test/ui/match/match-static-const-lc.rs b/src/test/ui/lint/lint-lowercase-static-const-pattern.rs similarity index 92% rename from src/test/ui/match/match-static-const-lc.rs rename to src/test/ui/lint/lint-lowercase-static-const-pattern.rs index c79da6e7fb8a..c2e159eec1ba 100644 --- a/src/test/ui/match/match-static-const-lc.rs +++ b/src/test/ui/lint/lint-lowercase-static-const-pattern.rs @@ -9,7 +9,7 @@ pub const a : isize = 97; fn f() { let r = match (0,0) { (0, a) => 0, - //~^ ERROR constant in pattern `a` should have an upper case name such as `A` + //~^ ERROR constant in pattern `a` should have an upper case name (x, y) => 1 + x + y, }; assert_eq!(r, 1); @@ -24,7 +24,7 @@ fn g() { use self::m::aha; let r = match (0,0) { (0, aha) => 0, - //~^ ERROR constant in pattern `aha` should have an upper case name such as `AHA` + //~^ ERROR constant in pattern `aha` should have an upper case name (x, y) => 1 + x + y, }; assert_eq!(r, 1); @@ -38,7 +38,7 @@ fn h() { use self::n::OKAY as not_okay; let r = match (0,0) { (0, not_okay) => 0, -//~^ ERROR constant in pattern `not_okay` should have an upper case name such as `NOT_OKAY` +//~^ ERROR constant in pattern `not_okay` should have an upper case name (x, y) => 1 + x + y, }; assert_eq!(r, 1); diff --git a/src/test/ui/lint/lint-lowercase-static-const-pattern.stderr b/src/test/ui/lint/lint-lowercase-static-const-pattern.stderr new file mode 100644 index 000000000000..d95510ccd2d2 --- /dev/null +++ b/src/test/ui/lint/lint-lowercase-static-const-pattern.stderr @@ -0,0 +1,26 @@ +error: constant in pattern `a` should have an upper case name + --> $DIR/lint-lowercase-static-const-pattern.rs:11:13 + | +LL | (0, a) => 0, + | ^ help: convert the identifier to upper case: `A` + | +note: lint level defined here + --> $DIR/lint-lowercase-static-const-pattern.rs:4:9 + | +LL | #![deny(non_upper_case_globals)] + | ^^^^^^^^^^^^^^^^^^^^^^ + +error: constant in pattern `aha` should have an upper case name + --> $DIR/lint-lowercase-static-const-pattern.rs:26:13 + | +LL | (0, aha) => 0, + | ^^^ help: convert the identifier to upper case: `AHA` + +error: constant in pattern `not_okay` should have an upper case name + --> $DIR/lint-lowercase-static-const-pattern.rs:40:13 + | +LL | (0, not_okay) => 0, + | ^^^^^^^^ help: convert the identifier to upper case: `NOT_OKAY` + +error: aborting due to 3 previous errors + diff --git a/src/test/ui/associated-const/associated-const-upper-case-lint.rs b/src/test/ui/lint/lint-non-uppercase-associated-const.rs similarity index 86% rename from src/test/ui/associated-const/associated-const-upper-case-lint.rs rename to src/test/ui/lint/lint-non-uppercase-associated-const.rs index c851b27bac04..7b0d9396077d 100644 --- a/src/test/ui/associated-const/associated-const-upper-case-lint.rs +++ b/src/test/ui/lint/lint-non-uppercase-associated-const.rs @@ -6,6 +6,6 @@ struct Foo; impl Foo { const not_upper: bool = true; } -//~^^ ERROR associated constant `not_upper` should have an upper case name such as `NOT_UPPER` +//~^^ ERROR associated constant `not_upper` should have an upper case name fn main() {} diff --git a/src/test/ui/associated-const/associated-const-upper-case-lint.stderr b/src/test/ui/lint/lint-non-uppercase-associated-const.stderr similarity index 57% rename from src/test/ui/associated-const/associated-const-upper-case-lint.stderr rename to src/test/ui/lint/lint-non-uppercase-associated-const.stderr index 43feb03761bd..2185d5a0ab48 100644 --- a/src/test/ui/associated-const/associated-const-upper-case-lint.stderr +++ b/src/test/ui/lint/lint-non-uppercase-associated-const.stderr @@ -1,11 +1,11 @@ -error: associated constant `not_upper` should have an upper case name such as `NOT_UPPER` - --> $DIR/associated-const-upper-case-lint.rs:7:5 +error: associated constant `not_upper` should have an upper case name + --> $DIR/lint-non-uppercase-associated-const.rs:7:11 | LL | const not_upper: bool = true; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^ help: convert the identifier to upper case: `NOT_UPPER` | note: lint level defined here - --> $DIR/associated-const-upper-case-lint.rs:1:9 + --> $DIR/lint-non-uppercase-associated-const.rs:1:9 | LL | #![deny(non_upper_case_globals)] | ^^^^^^^^^^^^^^^^^^^^^^ diff --git a/src/test/ui/lint/lint-non-uppercase-statics.rs b/src/test/ui/lint/lint-non-uppercase-statics.rs index 9424bef3dfbf..5bd1430328b4 100644 --- a/src/test/ui/lint/lint-non-uppercase-statics.rs +++ b/src/test/ui/lint/lint-non-uppercase-statics.rs @@ -1,10 +1,9 @@ #![forbid(non_upper_case_globals)] #![allow(dead_code)] -static foo: isize = 1; //~ ERROR static variable `foo` should have an upper case name such as `FOO` +static foo: isize = 1; //~ ERROR static variable `foo` should have an upper case name -static mut bar: isize = 1; - //~^ ERROR static variable `bar` should have an upper case name such as `BAR` +static mut bar: isize = 1; //~ ERROR static variable `bar` should have an upper case name #[no_mangle] pub static extern_foo: isize = 1; // OK, because #[no_mangle] supersedes the warning diff --git a/src/test/ui/lint/lint-non-uppercase-statics.stderr b/src/test/ui/lint/lint-non-uppercase-statics.stderr index 9c3dbb2fdeae..f5bba5f145de 100644 --- a/src/test/ui/lint/lint-non-uppercase-statics.stderr +++ b/src/test/ui/lint/lint-non-uppercase-statics.stderr @@ -1,8 +1,8 @@ -error: static variable `foo` should have an upper case name such as `FOO` - --> $DIR/lint-non-uppercase-statics.rs:4:1 +error: static variable `foo` should have an upper case name + --> $DIR/lint-non-uppercase-statics.rs:4:8 | -LL | static foo: isize = 1; //~ ERROR static variable `foo` should have an upper case name such as `FOO` - | ^^^^^^^^^^^^^^^^^^^^^^ +LL | static foo: isize = 1; //~ ERROR static variable `foo` should have an upper case name + | ^^^ help: convert the identifier to upper case: `FOO` | note: lint level defined here --> $DIR/lint-non-uppercase-statics.rs:1:11 @@ -10,11 +10,11 @@ note: lint level defined here LL | #![forbid(non_upper_case_globals)] | ^^^^^^^^^^^^^^^^^^^^^^ -error: static variable `bar` should have an upper case name such as `BAR` - --> $DIR/lint-non-uppercase-statics.rs:6:1 +error: static variable `bar` should have an upper case name + --> $DIR/lint-non-uppercase-statics.rs:6:12 | -LL | static mut bar: isize = 1; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | static mut bar: isize = 1; //~ ERROR static variable `bar` should have an upper case name + | ^^^ help: convert the identifier to upper case: `BAR` error: aborting due to 2 previous errors diff --git a/src/test/ui/match/match-static-const-lc.stderr b/src/test/ui/match/match-static-const-lc.stderr deleted file mode 100644 index 1ddb831bf9de..000000000000 --- a/src/test/ui/match/match-static-const-lc.stderr +++ /dev/null @@ -1,26 +0,0 @@ -error: constant in pattern `a` should have an upper case name such as `A` - --> $DIR/match-static-const-lc.rs:11:13 - | -LL | (0, a) => 0, - | ^ - | -note: lint level defined here - --> $DIR/match-static-const-lc.rs:4:9 - | -LL | #![deny(non_upper_case_globals)] - | ^^^^^^^^^^^^^^^^^^^^^^ - -error: constant in pattern `aha` should have an upper case name such as `AHA` - --> $DIR/match-static-const-lc.rs:26:13 - | -LL | (0, aha) => 0, - | ^^^ - -error: constant in pattern `not_okay` should have an upper case name such as `NOT_OKAY` - --> $DIR/match-static-const-lc.rs:40:13 - | -LL | (0, not_okay) => 0, - | ^^^^^^^^ - -error: aborting due to 3 previous errors -