]> git.lizzy.rs Git - rust.git/commitdiff
Rollup merge of #47922 - zackmdavis:and_the_case_of_the_unused_field_pattern, r=estebank
authorManish Goregaokar <manishsmail@gmail.com>
Wed, 7 Feb 2018 16:30:52 +0000 (08:30 -0800)
committerGitHub <noreply@github.com>
Wed, 7 Feb 2018 16:30:52 +0000 (08:30 -0800)
correct unused field pattern suggestions

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

r? @estebank

src/librustc/middle/liveness.rs
src/librustc/util/nodemap.rs
src/test/ui/lint/issue-47390-unused-variable-in-struct-pattern.rs [new file with mode: 0644]
src/test/ui/lint/issue-47390-unused-variable-in-struct-pattern.stderr [new file with mode: 0644]
src/test/ui/span/issue-24690.stderr

index 297586f140e346e91391f075d68a2d56adf11224..10497c95e27d07e79531034c15120e0c48406392 100644 (file)
 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 @@ fn variable_name(&self, var: Variable) -> String {
         }
     }
 
+    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,8 +392,9 @@ 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);
@@ -393,6 +402,22 @@ fn visit_local<'a, 'tcx>(ir: &mut IrMaps<'a, 'tcx>, local: &'tcx hir::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);
@@ -400,7 +425,8 @@ fn visit_arm<'a, 'tcx>(ir: &mut IrMaps<'a, 'tcx>, arm: &'tcx hir::Arm) {
             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 @@ fn warn_about_unused(&self,
                     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
index 674f67d5cd2f188b6dc7d89dc849de8c5c4969dc..f98a8f834df8a912f1bdb914f1d904eed1c6270e 100644 (file)
@@ -13,7 +13,7 @@
 #![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 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() }
diff --git a/src/test/ui/lint/issue-47390-unused-variable-in-struct-pattern.rs b/src/test/ui/lint/issue-47390-unused-variable-in-struct-pattern.rs
new file mode 100644 (file)
index 0000000..a68b4f7
--- /dev/null
@@ -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;
+    }
+}
diff --git a/src/test/ui/lint/issue-47390-unused-variable-in-struct-pattern.stderr b/src/test/ui/lint/issue-47390-unused-variable-in-struct-pattern.stderr
new file mode 100644 (file)
index 0000000..694fe69
--- /dev/null
@@ -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)]
+
index 7e19c7492ce0bdf032bb562882d39d9068a8d907..31728dbf08db2b642c6d3bc6f68a567a02e93be7 100644 (file)
@@ -2,7 +2,7 @@ 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
@@ -10,7 +10,6 @@ note: lint level defined here
 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