Auto merge of #47922 - zackmdavis:and_the_case_of_the_unused_field_pattern, r=estebank

correct unused field pattern suggestions

![unused_field_pattern_local](https://user-images.githubusercontent.com/1076988/35662336-7a69488a-06cc-11e8-9901-8d22b5cf924f.png)

r? @estebank

Author: bors

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/librustc/util/nodemap.rs

@@ -13,18 +13,20 @@
 #![allow(non_snake_case)]
 
 use hir::def_id::DefId;
-use hir::ItemLocalId;
+use hir::{HirId, ItemLocalId};
 use syntax::ast;
 
 pub use rustc_data_structures::fx::FxHashMap;
 pub use rustc_data_structures::fx::FxHashSet;
 
 pub type NodeMap<T> = FxHashMap<ast::NodeId, T>;
 pub type DefIdMap<T> = FxHashMap<DefId, T>;
+pub type HirIdMap<T> = FxHashMap<HirId, T>;
 pub type ItemLocalMap<T> = FxHashMap<ItemLocalId, T>;
 
 pub type NodeSet = FxHashSet<ast::NodeId>;
 pub type DefIdSet = FxHashSet<DefId>;
+pub type HirIdSet = FxHashSet<HirId>;
 pub type ItemLocalSet = FxHashSet<ItemLocalId>;
 
 pub fn NodeMap<T>() -> NodeMap<T> { FxHashMap() }

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