From e4b1a7971d7b4ed61d27af44e3169cb26595ae13 Mon Sep 17 00:00:00 2001 From: "Zack M. Davis" Date: Wed, 31 Jan 2018 20:56:01 -0800 Subject: [PATCH] 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. --- src/librustc/middle/liveness.rs | 63 ++++++++++++++----- ...47390-unused-variable-in-struct-pattern.rs | 34 ++++++++++ ...0-unused-variable-in-struct-pattern.stderr | 40 ++++++++++++ src/test/ui/span/issue-24690.stderr | 3 +- 4 files changed, 124 insertions(+), 16 deletions(-) create mode 100644 src/test/ui/lint/issue-47390-unused-variable-in-struct-pattern.rs create mode 100644 src/test/ui/lint/issue-47390-unused-variable-in-struct-pattern.stderr diff --git a/src/librustc/middle/liveness.rs b/src/librustc/middle/liveness.rs index 297586f140e34..10497c95e27d0 100644 --- a/src/librustc/middle/liveness.rs +++ b/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) { 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 @@ 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 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 index 0000000000000..a68b4f7635292 --- /dev/null +++ b/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 or the MIT license +// , 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 index 0000000000000..694fe69e01648 --- /dev/null +++ b/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)] + diff --git a/src/test/ui/span/issue-24690.stderr b/src/test/ui/span/issue-24690.stderr index 7e19c7492ce0b..31728dbf08db2 100644 --- a/src/test/ui/span/issue-24690.stderr +++ b/src/test/ui/span/issue-24690.stderr @@ -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