Auto merge of #45232 - zackmdavis:moar_lint_suggestions, r=estebank

code suggestions for non-shorthand field pattern, no-mangle lints

continuing in the spirit of #44942

![moar_lint_suggestions](https://user-images.githubusercontent.com/1076988/31485011-3b20cc80-aee7-11e7-993d-81267ab77732.png)

r? @estebank

6 files changed, 141 insertions, 36 deletions

diff --git a/src/librustc_lint/builtin.rs b/src/librustc_lint/builtin.rs
index 7f331418d42..bc2a1f08441 100644
--- a/src/librustc_lint/builtin.rs
+++ b/src/librustc_lint/builtin.rs
@@ -44,7 +44,7 @@ use std::collections::HashSet;
 use syntax::ast;
 use syntax::attr;
 use syntax::feature_gate::{AttributeGate, AttributeType, Stability, deprecated_attributes};
-use syntax_pos::{Span, SyntaxContext};
+use syntax_pos::{BytePos, Span, SyntaxContext};
 use syntax::symbol::keywords;
 
 use rustc::hir::{self, PatKind};
@@ -153,7 +153,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for BoxPointers {
 declare_lint! {
     NON_SHORTHAND_FIELD_PATTERNS,
     Warn,
-    "using `Struct { x: x }` instead of `Struct { x }`"
+    "using `Struct { x: x }` instead of `Struct { x }` in a pattern"
 }
 
 #[derive(Copy, Clone)]
@@ -174,11 +174,15 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NonShorthandFieldPatterns {
                 }
                 if let PatKind::Binding(_, _, ident, None) = fieldpat.node.pat.node {
                     if ident.node == fieldpat.node.name {
-                        cx.span_lint(NON_SHORTHAND_FIELD_PATTERNS,
+                        let mut err = cx.struct_span_lint(NON_SHORTHAND_FIELD_PATTERNS,
                                      fieldpat.span,
-                                     &format!("the `{}:` in this pattern is redundant and can \
-                                              be removed",
-                                              ident.node))
+                                     &format!("the `{}:` in this pattern is redundant",
+                                              ident.node));
+                        let subspan = cx.tcx.sess.codemap().span_through_char(fieldpat.span, ':');
+                        err.span_suggestion_short(subspan,
+                                                  "remove this",
+                                                  format!("{}", ident.node));
+                        err.emit();
                     }
                 }
             }
@@ -894,7 +898,6 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnconditionalRecursion {
             let mut db = cx.struct_span_lint(UNCONDITIONAL_RECURSION,
                                              sp,
                                              "function cannot return without recurring");
-            // FIXME #19668: these could be span_lint_note's instead of this manual guard.
             // offer some help to the programmer.
             for call in &self_call_spans {
                 db.span_note(*call, "recursive call site");
@@ -1130,35 +1133,55 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for InvalidNoMangleItems {
     fn check_item(&mut self, cx: &LateContext, it: &hir::Item) {
         match it.node {
             hir::ItemFn(.., ref generics, _) => {
-                if attr::contains_name(&it.attrs, "no_mangle") &&
-                   !attr::contains_name(&it.attrs, "linkage") {
+                if let Some(no_mangle_attr) = attr::find_by_name(&it.attrs, "no_mangle") {
+                    if attr::contains_name(&it.attrs, "linkage") {
+                        return;
+                    }
                     if !cx.access_levels.is_reachable(it.id) {
-                        let msg = format!("function {} is marked #[no_mangle], but not exported",
-                                          it.name);
-                        cx.span_lint(PRIVATE_NO_MANGLE_FNS, it.span, &msg);
+                        let msg = "function is marked #[no_mangle], but not exported";
+                        let mut err = cx.struct_span_lint(PRIVATE_NO_MANGLE_FNS, it.span, msg);
+                        let insertion_span = it.span.with_hi(it.span.lo());
+                        err.span_suggestion(insertion_span,
+                                            "try making it public",
+                                            "pub ".to_owned());
+                        err.emit();
                     }
                     if generics.is_type_parameterized() {
-                        cx.span_lint(NO_MANGLE_GENERIC_ITEMS,
-                                     it.span,
-                                     "functions generic over types must be mangled");
+                        let mut err = cx.struct_span_lint(NO_MANGLE_GENERIC_ITEMS,
+                                                          it.span,
+                                                          "functions generic over \
+                                                           types must be mangled");
+                        err.span_suggestion_short(no_mangle_attr.span,
+                                                  "remove this attribute",
+                                                  "".to_owned());
+                        err.emit();
                     }
                 }
             }
             hir::ItemStatic(..) => {
                 if attr::contains_name(&it.attrs, "no_mangle") &&
                    !cx.access_levels.is_reachable(it.id) {
-                    let msg = format!("static {} is marked #[no_mangle], but not exported",
-                                      it.name);
-                    cx.span_lint(PRIVATE_NO_MANGLE_STATICS, it.span, &msg);
+                       let msg = "static is marked #[no_mangle], but not exported";
+                       let mut err = cx.struct_span_lint(PRIVATE_NO_MANGLE_STATICS, it.span, msg);
+                       let insertion_span = it.span.with_hi(it.span.lo());
+                       err.span_suggestion(insertion_span,
+                                           "try making it public",
+                                           "pub ".to_owned());
+                       err.emit();
                 }
             }
             hir::ItemConst(..) => {
                 if attr::contains_name(&it.attrs, "no_mangle") {
                     // Const items do not refer to a particular location in memory, and therefore
                     // don't have anything to attach a symbol to
-                    let msg = "const items should never be #[no_mangle], consider instead using \
-                               `pub static`";
-                    cx.span_lint(NO_MANGLE_CONST_ITEMS, it.span, msg);
+                    let msg = "const items should never be #[no_mangle]";
+                    let mut err = cx.struct_span_lint(NO_MANGLE_CONST_ITEMS, it.span, msg);
+                    // `const` is 5 chars
+                    let const_span = it.span.with_hi(BytePos(it.span.lo().0 + 5));
+                    err.span_suggestion(const_span,
+                                        "try a static value",
+                                        "pub static".to_owned());
+                    err.emit();
                 }
             }
             _ => {}
diff --git a/src/libsyntax/codemap.rs b/src/libsyntax/codemap.rs
index efaa5e5e3da..dd46903bb88 100644
--- a/src/libsyntax/codemap.rs
+++ b/src/libsyntax/codemap.rs
@@ -471,6 +471,17 @@ impl CodeMap {
         }
     }
 
+    /// Given a `Span`, try to get a shorter span ending just after the first
+    /// occurrence of `char` `c`.
+    pub fn span_through_char(&self, sp: Span, c: char) -> Span {
+        if let Ok(snippet) = self.span_to_snippet(sp) {
+            if let Some(offset) = snippet.find(c) {
+                return sp.with_hi(BytePos(sp.lo().0 + (offset + c.len_utf8()) as u32));
+            }
+        }
+        sp
+    }
+
     pub fn def_span(&self, sp: Span) -> Span {
         self.span_until_char(sp, '{')
     }
diff --git a/src/test/compile-fail/feature-gate/issue-43106-gating-of-builtin-attrs.rs b/src/test/compile-fail/feature-gate/issue-43106-gating-of-builtin-attrs.rs
index 06b87206669..ab2fe02bb14 100644
--- a/src/test/compile-fail/feature-gate/issue-43106-gating-of-builtin-attrs.rs
+++ b/src/test/compile-fail/feature-gate/issue-43106-gating-of-builtin-attrs.rs
@@ -424,7 +424,7 @@ mod no_mangle {
     mod inner { #![no_mangle="3500"] }
 
     #[no_mangle = "3500"] fn f() { }
-    //~^ WARN function f is marked #[no_mangle], but not exported
+    //~^ WARN function is marked #[no_mangle], but not exported
 
     #[no_mangle = "3500"] struct S;
 
diff --git a/src/test/compile-fail/lint-unexported-no-mangle.rs b/src/test/compile-fail/lint-unexported-no-mangle.rs
index 216fcf93535..cd64dfa7a47 100644
--- a/src/test/compile-fail/lint-unexported-no-mangle.rs
+++ b/src/test/compile-fail/lint-unexported-no-mangle.rs
@@ -10,9 +10,8 @@
 
 // compile-flags:-F private_no_mangle_fns -F no_mangle_const_items -F private_no_mangle_statics
 
-// FIXME(#19495) no_mangle'ing main ICE's.
 #[no_mangle]
-fn foo() { //~ ERROR function foo is marked #[no_mangle], but not exported
+fn foo() { //~ ERROR function is marked #[no_mangle], but not exported
 }
 
 #[allow(dead_code)]
@@ -31,7 +30,7 @@ pub static BAR: u64 = 1;
 
 #[allow(dead_code)]
 #[no_mangle]
-static PRIVATE_BAR: u64 = 1; //~ ERROR static PRIVATE_BAR is marked #[no_mangle], but not exported
+static PRIVATE_BAR: u64 = 1; //~ ERROR static is marked #[no_mangle], but not exported
 
 
 fn main() {
diff --git a/src/test/ui/lint/suggestions.rs b/src/test/ui/lint/suggestions.rs
index 874124a7d36..e078056ab5e 100644
--- a/src/test/ui/lint/suggestions.rs
+++ b/src/test/ui/lint/suggestions.rs
@@ -11,10 +11,27 @@
 #![warn(unused_mut)] // UI tests pass `-A unused`—see Issue #43896
 #![feature(no_debug)]
 
+#[no_mangle] static SHENZHOU: usize = 1; // should suggest `pub`
+#[no_mangle] const DISCOVERY: usize = 1; // should suggest `pub static` rather than `const`
+
+#[no_mangle] // should suggest removal (generics can't be no-mangle)
+pub fn defiant<T>(_t: T) {}
+
+#[no_mangle]
+fn rio_grande() {} // should suggest `pub`
+
+struct Equinox {
+    warp_factor: f32,
+}
+
 #[no_debug] // should suggest removal of deprecated attribute
 fn main() {
     while true { // should suggest `loop`
         let mut a = (1); // should suggest no `mut`, no parens
+        let d = Equinox { warp_factor: 9.975 };
+        match d {
+            Equinox { warp_factor: warp_factor } => {} // should suggest shorthand
+        }
         println!("{}", a);
     }
 }
diff --git a/src/test/ui/lint/suggestions.stderr b/src/test/ui/lint/suggestions.stderr
index 9a69c52e6cf..7a498b56413 100644
--- a/src/test/ui/lint/suggestions.stderr
+++ b/src/test/ui/lint/suggestions.stderr
@@ -1,23 +1,23 @@
 warning: unnecessary parentheses around assigned value
-  --> $DIR/suggestions.rs:17:21
+  --> $DIR/suggestions.rs:30:21
    |
-17 |         let mut a = (1); // should suggest no `mut`, no parens
+30 |         let mut a = (1); // should suggest no `mut`, no parens
    |                     ^^^ help: remove these parentheses
    |
    = note: #[warn(unused_parens)] on by default
 
 warning: use of deprecated attribute `no_debug`: the `#[no_debug]` attribute was an experimental feature that has been deprecated due to lack of demand. See https://github.com/rust-lang/rust/issues/29721
-  --> $DIR/suggestions.rs:14:1
+  --> $DIR/suggestions.rs:27:1
    |
-14 | #[no_debug] // should suggest removal of deprecated attribute
+27 | #[no_debug] // should suggest removal of deprecated attribute
    | ^^^^^^^^^^^ help: remove this attribute
    |
    = note: #[warn(deprecated)] on by default
 
 warning: variable does not need to be mutable
-  --> $DIR/suggestions.rs:17:13
+  --> $DIR/suggestions.rs:30:13
    |
-17 |         let mut a = (1); // should suggest no `mut`, no parens
+30 |         let mut a = (1); // should suggest no `mut`, no parens
    |             ---^^
    |             |
    |             help: remove this `mut`
@@ -28,18 +28,73 @@ note: lint level defined here
 11 | #![warn(unused_mut)] // UI tests pass `-A unused`—see Issue #43896
    |         ^^^^^^^^^^
 
+warning: static is marked #[no_mangle], but not exported
+  --> $DIR/suggestions.rs:14:14
+   |
+14 | #[no_mangle] static SHENZHOU: usize = 1; // should suggest `pub`
+   |              -^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |              |
+   |              help: try making it public: `pub `
+   |
+   = note: #[warn(private_no_mangle_statics)] on by default
+
+error: const items should never be #[no_mangle]
+  --> $DIR/suggestions.rs:15:14
+   |
+15 | #[no_mangle] const DISCOVERY: usize = 1; // should suggest `pub static` rather than `const`
+   |              -----^^^^^^^^^^^^^^^^^^^^^^
+   |              |
+   |              help: try a static value: `pub static`
+   |
+   = note: #[deny(no_mangle_const_items)] on by default
+
+warning: functions generic over types must be mangled
+  --> $DIR/suggestions.rs:18:1
+   |
+17 | #[no_mangle] // should suggest removal (generics can't be no-mangle)
+   | ------------ help: remove this attribute
+18 | pub fn defiant<T>(_t: T) {}
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: #[warn(no_mangle_generic_items)] on by default
+
+warning: function is marked #[no_mangle], but not exported
+  --> $DIR/suggestions.rs:21:1
+   |
+21 | fn rio_grande() {} // should suggest `pub`
+   | -^^^^^^^^^^^^^^^^^
+   | |
+   | help: try making it public: `pub `
+   |
+   = note: #[warn(private_no_mangle_fns)] on by default
+
 warning: denote infinite loops with `loop { ... }`
-  --> $DIR/suggestions.rs:16:5
+  --> $DIR/suggestions.rs:29:5
    |
-16 |       while true { // should suggest `loop`
+29 |       while true { // should suggest `loop`
    |       ^---------
    |       |
    |  _____help: use `loop`
    | |
-17 | |         let mut a = (1); // should suggest no `mut`, no parens
-18 | |         println!("{}", a);
-19 | |     }
+30 | |         let mut a = (1); // should suggest no `mut`, no parens
+31 | |         let d = Equinox { warp_factor: 9.975 };
+32 | |         match d {
+...  |
+35 | |         println!("{}", a);
+36 | |     }
    | |_____^
    |
    = note: #[warn(while_true)] on by default
 
+warning: the `warp_factor:` in this pattern is redundant
+  --> $DIR/suggestions.rs:33:23
+   |
+33 |             Equinox { warp_factor: warp_factor } => {} // should suggest shorthand
+   |                       ------------^^^^^^^^^^^^
+   |                       |
+   |                       help: remove this
+   |
+   = note: #[warn(non_shorthand_field_patterns)] on by default
+
+error: aborting due to previous error
+