improve non_upper_case_globals diagnostics

Use a structured suggestion and tighten the span to just the identifier.

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) {
+    fn check_upper_case(cx: &LateContext, sort: &str, ident: &Ident) {
-        if name.as_str().chars().any(|c| c.is_lowercase()) {
+        let name = &ident.name.as_str();
-            let uc = NonSnakeCase::to_snake_case(&name.as_str()).to_uppercase();
+
-            if name != &*uc {
+        if name.chars().any(|c| c.is_lowercase()) {
-                cx.span_lint(NON_UPPER_CASE_GLOBALS,
+            let uc = NonSnakeCase::to_snake_case(&name).to_uppercase();
-                             span,
+
-                             &format!("{} `{}` should have an upper case name such as `{}`",
+            let msg = format!("{} `{}` should have an upper case name", sort, name);
-                                      sort,
+            cx.struct_span_lint(NON_UPPER_CASE_GLOBALS, ident.span, &msg)
-                                      name,
+                .span_suggestion_with_applicability(
-                                      uc));
+                    ident.span,
-            } else {
+                    "convert the identifier to upper case",
-                cx.span_lint(NON_UPPER_CASE_GLOBALS,
+                    uc,
-                             span,
+                    Applicability::MaybeIncorrect,
-                             &format!("{} `{}` should have an upper case name", sort, name));
+                )
-            }
+                .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(..) => {
+            hir::ItemKind::Static(..) if !attr::contains_name(&it.attrs, "no_mangle") => {
-                if attr::find_by_name(&it.attrs, "no_mangle").is_some() {
+                NonUpperCaseGlobals::check_upper_case(cx, "static variable", &it.ident);
-                    return;
-                }
-                NonUpperCaseGlobals::check_upper_case(cx, "static variable", it.ident.name,
-                                                      it.span);
             }
             hir::ItemKind::Const(..) => {
-                NonUpperCaseGlobals::check_upper_case(cx, "constant", it.ident.name,
+                NonUpperCaseGlobals::check_upper_case(cx, "constant", &it.ident);
-                                                      it.span);
             }
             _ => {}
         }
     }
 
     fn check_trait_item(&mut self, cx: &LateContext, ti: &hir::TraitItem) {
-        match ti.node {
+        if let hir::TraitItemKind::Const(..) = ti.node {
-            hir::TraitItemKind::Const(..) => {
+            NonUpperCaseGlobals::check_upper_case(cx, "associated constant", &ti.ident);
-                NonUpperCaseGlobals::check_upper_case(cx, "associated constant",
-                                                      ti.ident.name, ti.span);
-            }
-            _ => {}
         }
     }
 
     fn check_impl_item(&mut self, cx: &LateContext, ii: &hir::ImplItem) {
-        match ii.node {
+        if let hir::ImplItemKind::Const(..) = ii.node {
-            hir::ImplItemKind::Const(..) => {
+            NonUpperCaseGlobals::check_upper_case(cx, "associated constant", &ii.ident);
-                NonUpperCaseGlobals::check_upper_case(cx, "associated constant",
-                                                      ii.ident.name, ii.span);
-            }
-            _ => {}
         }
     }
 
@@ -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,
+                    NonUpperCaseGlobals::check_upper_case(
-                                                          "constant in pattern",
+                        cx,
-                                                          path.segments[0].ident.name,
+                        "constant in pattern",
-                                                          path.span);
+                        &path.segments[0].ident
+                    );
                 }
             }
         }

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"))),

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() {}

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`
+error: constant `foo` should have an upper case name
-  --> $DIR/issue-17718-const-naming.rs:4:1
+  --> $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

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`
+error: static variable `bad` should have an upper case name
-  --> $DIR/lint-group-nonstandard-style.rs:14:9
+  --> $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

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;

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

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
+

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() {}

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`
+error: associated constant `not_upper` should have an upper case name
-  --> $DIR/associated-const-upper-case-lint.rs:7:5
+  --> $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)]
    |         ^^^^^^^^^^^^^^^^^^^^^^

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;
+static mut bar: isize = 1; //~ ERROR static variable `bar` should have an upper case name
-        //~^ ERROR static variable `bar` should have an upper case name such as `BAR`
 
 #[no_mangle]
 pub static extern_foo: isize = 1; // OK, because #[no_mangle] supersedes the warning

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`
+error: static variable `foo` should have an upper case name
-  --> $DIR/lint-non-uppercase-statics.rs:4:1
+  --> $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`
+error: static variable `bar` should have an upper case name
-  --> $DIR/lint-non-uppercase-statics.rs:6:1
+  --> $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
 

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

@@ -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
-