improve non_upper_case_globals diagnostics

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

13 files changed, 89 insertions, 103 deletions

diff --git a/src/librustc_lint/nonstandard_style.rs b/src/librustc_lint/nonstandard_style.rs
index fa6665d1ae7..d2fdaab9315 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 cf842dddeb3..a19d0458edd 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 9b3d84b3dd5..d30b95843f3 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 641e50a5fc9..b92acecb83e 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 97e3ce6fd37..176f246ad76 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
index e78ae40da2f..8ca5af21630 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
index c79da6e7fb8..c2e159eec1b 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 00000000000..d95510ccd2d
--- /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
index c851b27bac0..7b0d9396077 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
index 43feb03761b..2185d5a0ab4 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 9424bef3dfb..5bd1430328b 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 9c3dbb2fdea..f5bba5f145d 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 1ddb831bf9d..00000000000
--- 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
-