concerning well-formed suggestions for unused shorthand field patterns

Previously, unused variables would get a note that the warning could be
silenced by prefixing the variable with an underscore, but that doesn't
work for field shorthand patterns, which the liveness analysis didn't
know about.

The "to avoid this warning" verbiage seemed unnecessary.

Resolves #47390.

Author: zackmdavis

src/librustc/middle/liveness.rs

@@ -109,7 +109,7 @@ use self::VarKind::*;
 use hir::def::*;
 use ty::{self, TyCtxt};
 use lint;
-use util::nodemap::NodeMap;
+use util::nodemap::{NodeMap, NodeSet};
 
 use std::{fmt, usize};
 use std::io::prelude::*;
@@ -244,7 +244,8 @@ struct CaptureInfo {
 #[derive(Copy, Clone, Debug)]
 struct LocalInfo {
     id: NodeId,
-    name: ast::Name
+    name: ast::Name,
+    is_shorthand: bool,
 }
 
 #[derive(Copy, Clone, Debug)]
@@ -333,6 +334,13 @@ impl<'a, 'tcx> IrMaps<'a, 'tcx> {
         }
     }
 
+    fn variable_is_shorthand(&self, var: Variable) -> bool {
+        match self.var_kinds[var.get()] {
+            Local(LocalInfo { is_shorthand, .. }) => is_shorthand,
+            Arg(..) | CleanExit => false
+        }
+    }
+
     fn set_captures(&mut self, node_id: NodeId, cs: Vec<CaptureInfo>) {
         self.capture_info_map.insert(node_id, Rc::new(cs));
     }
@@ -384,23 +392,41 @@ fn visit_local<'a, 'tcx>(ir: &mut IrMaps<'a, 'tcx>, local: &'tcx hir::Local) {
         let name = path1.node;
         ir.add_live_node_for_node(p_id, VarDefNode(sp));
         ir.add_variable(Local(LocalInfo {
-          id: p_id,
-          name,
+            id: p_id,
+            name,
+            is_shorthand: false,
         }));
     });
     intravisit::walk_local(ir, local);
 }
 
 fn visit_arm<'a, 'tcx>(ir: &mut IrMaps<'a, 'tcx>, arm: &'tcx hir::Arm) {
     for pat in &arm.pats {
+        // for struct patterns, take note of which fields used shorthand (`x`
+        // rather than `x: x`)
+        //
+        // FIXME: according to the rust-lang-nursery/rustc-guide book and
+        // librustc/README.md, `NodeId`s are to be phased out in favor of
+        // `HirId`s; however, we need to match the signature of `each_binding`,
+        // which uses `NodeIds`.
+        let mut shorthand_field_ids = NodeSet();
+        if let hir::PatKind::Struct(_, ref fields, _) = pat.node {
+            for field in fields {
+                if field.node.is_shorthand {
+                    shorthand_field_ids.insert(field.node.pat.id);
+                }
+            }
+        }
+
         pat.each_binding(|bm, p_id, sp, path1| {
             debug!("adding local variable {} from match with bm {:?}",
                    p_id, bm);
             let name = path1.node;
             ir.add_live_node_for_node(p_id, VarDefNode(sp));
             ir.add_variable(Local(LocalInfo {
                 id: p_id,
-                name,
+                name: name,
+                is_shorthand: shorthand_field_ids.contains(&p_id)
             }));
         })
     }
@@ -1483,17 +1509,26 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> {
                     self.assigned_on_exit(ln, var).is_some()
                 };
 
+                let suggest_underscore_msg = format!("consider using `_{}` instead",
+                                                     name);
                 if is_assigned {
-                    self.ir.tcx.lint_node_note(lint::builtin::UNUSED_VARIABLES, id, sp,
-                        &format!("variable `{}` is assigned to, but never used",
-                                 name),
-                        &format!("to avoid this warning, consider using `_{}` instead",
-                                 name));
+                    self.ir.tcx
+                        .lint_node_note(lint::builtin::UNUSED_VARIABLES, id, sp,
+                                        &format!("variable `{}` is assigned to, but never used",
+                                                 name),
+                                        &suggest_underscore_msg);
                 } else if name != "self" {
-                    self.ir.tcx.lint_node_note(lint::builtin::UNUSED_VARIABLES, id, sp,
-                        &format!("unused variable: `{}`", name),
-                        &format!("to avoid this warning, consider using `_{}` instead",
-                                 name));
+                    let msg = format!("unused variable: `{}`", name);
+                    let mut err = self.ir.tcx
+                        .struct_span_lint_node(lint::builtin::UNUSED_VARIABLES, id, sp, &msg);
+                    if self.ir.variable_is_shorthand(var) {
+                        err.span_suggestion(sp, "try ignoring the field",
+                                            format!("{}: _", name));
+                    } else {
+                        err.span_suggestion_short(sp, &suggest_underscore_msg,
+                                                  format!("_{}", name));
+                    }
+                    err.emit()
                 }
             }
             true

src/test/ui/lint/issue-47390-unused-variable-in-struct-pattern.rs

@@ -0,0 +1,34 @@
+// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+// must-compile-successfully
+
+#![warn(unused)] // UI tests pass `-A unused` (#43896)
+
+struct SoulHistory {
+    corridors_of_light: usize,
+    hours_are_suns: bool,
+    endless_and_singing: bool
+}
+
+fn main() {
+    let i_think_continually = 2;
+    let who_from_the_womb_remembered = SoulHistory {
+        corridors_of_light: 5,
+        hours_are_suns: true,
+        endless_and_singing: true
+    };
+
+    if let SoulHistory { corridors_of_light,
+                         mut hours_are_suns,
+                         endless_and_singing: true } = who_from_the_womb_remembered {
+        hours_are_suns = false;
+    }
+}

src/test/ui/lint/issue-47390-unused-variable-in-struct-pattern.stderr

@@ -0,0 +1,40 @@
+warning: unused variable: `i_think_continually`
+  --> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:22:9
+   |
+22 |     let i_think_continually = 2;
+   |         ^^^^^^^^^^^^^^^^^^^ help: consider using `_i_think_continually` instead
+   |
+note: lint level defined here
+  --> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:13:9
+   |
+13 | #![warn(unused)] // UI tests pass `-A unused` (#43896)
+   |         ^^^^^^
+   = note: #[warn(unused_variables)] implied by #[warn(unused)]
+
+warning: unused variable: `corridors_of_light`
+  --> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:29:26
+   |
+29 |     if let SoulHistory { corridors_of_light,
+   |                          ^^^^^^^^^^^^^^^^^^ help: try ignoring the field: `corridors_of_light: _`
+
+warning: variable `hours_are_suns` is assigned to, but never used
+  --> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:30:26
+   |
+30 |                          mut hours_are_suns,
+   |                          ^^^^^^^^^^^^^^^^^^
+   |
+   = note: consider using `_hours_are_suns` instead
+
+warning: value assigned to `hours_are_suns` is never read
+  --> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:32:9
+   |
+32 |         hours_are_suns = false;
+   |         ^^^^^^^^^^^^^^
+   |
+note: lint level defined here
+  --> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:13:9
+   |
+13 | #![warn(unused)] // UI tests pass `-A unused` (#43896)
+   |         ^^^^^^
+   = note: #[warn(unused_assignments)] implied by #[warn(unused)]
+

src/test/ui/span/issue-24690.stderr

@@ -2,15 +2,14 @@ warning: unused variable: `theOtherTwo`
   --> $DIR/issue-24690.rs:23:9
    |
 23 |     let theOtherTwo = 2; //~ WARN should have a snake case name
-   |         ^^^^^^^^^^^
+   |         ^^^^^^^^^^^ help: consider using `_theOtherTwo` instead
    |
 note: lint level defined here
   --> $DIR/issue-24690.rs:18:9
    |
 18 | #![warn(unused)]
    |         ^^^^^^
    = note: #[warn(unused_variables)] implied by #[warn(unused)]
-   = note: to avoid this warning, consider using `_theOtherTwo` instead
 
 warning: variable `theTwo` should have a snake case name such as `the_two`
   --> $DIR/issue-24690.rs:22:9