From 5f021e0023b3b471c5c9c39e07f70d344cc604b0 Mon Sep 17 00:00:00 2001 From: David Wood Date: Fri, 25 Jan 2019 16:56:27 +0100 Subject: [PATCH] Unused variable suggestions on all patterns. This commit extends existing suggestions to prefix unused variable bindings in match arms with an underscore so that it applies to all patterns in a match arm. --- src/librustc/middle/liveness.rs | 84 +++++++++++-------- src/test/ui/issues/issue-17999.stderr | 4 +- src/test/ui/issues/issue-22599.stderr | 2 +- src/test/ui/issues/issue-56685.rs | 44 ++++++++++ src/test/ui/issues/issue-56685.stderr | 60 +++++++++++++ ...0-unused-variable-in-struct-pattern.stderr | 8 +- src/test/ui/lint/lint-removed-allow.stderr | 2 +- src/test/ui/lint/lint-removed-cmdline.stderr | 2 +- src/test/ui/lint/lint-removed.stderr | 2 +- src/test/ui/lint/lint-renamed-allow.stderr | 2 +- src/test/ui/lint/lint-renamed-cmdline.stderr | 2 +- src/test/ui/lint/lint-renamed.stderr | 2 +- .../ui/lint/lint-uppercase-variables.stderr | 2 +- src/test/ui/liveness/liveness-unused.stderr | 16 ++-- src/test/ui/never-assign-dead-code.stderr | 2 +- .../ui/proc-macro/attributes-included.stderr | 2 +- src/test/ui/span/issue-24690.stderr | 2 +- 17 files changed, 179 insertions(+), 59 deletions(-) create mode 100644 src/test/ui/issues/issue-56685.rs create mode 100644 src/test/ui/issues/issue-56685.stderr diff --git a/src/librustc/middle/liveness.rs b/src/librustc/middle/liveness.rs index cc0dd71738fce..4ef8e1a0cf95b 100644 --- a/src/librustc/middle/liveness.rs +++ b/src/librustc/middle/liveness.rs @@ -105,7 +105,7 @@ use lint; use errors::Applicability; use util::nodemap::{NodeMap, HirIdMap, HirIdSet}; -use std::collections::VecDeque; +use std::collections::{BTreeMap, VecDeque}; use std::{fmt, u32}; use std::io::prelude::*; use std::io; @@ -1446,7 +1446,7 @@ fn check_local<'a, 'tcx>(this: &mut Liveness<'a, 'tcx>, local: &'tcx hir::Local) None => { this.pat_bindings(&local.pat, |this, ln, var, sp, id| { let span = local.pat.simple_ident().map_or(sp, |ident| ident.span); - this.warn_about_unused(span, id, ln, var); + this.warn_about_unused(vec![span], id, ln, var); }) } } @@ -1455,12 +1455,29 @@ fn check_local<'a, 'tcx>(this: &mut Liveness<'a, 'tcx>, local: &'tcx hir::Local) } fn check_arm<'a, 'tcx>(this: &mut Liveness<'a, 'tcx>, arm: &'tcx hir::Arm) { - // only consider the first pattern; any later patterns must have - // the same bindings, and we also consider the first pattern to be - // the "authoritative" set of ids - this.arm_pats_bindings(arm.pats.first().map(|p| &**p), |this, ln, var, sp, id| { - this.warn_about_unused(sp, id, ln, var); - }); + // Only consider the variable from the first pattern; any later patterns must have + // the same bindings, and we also consider the first pattern to be the "authoritative" set of + // ids. However, we should take the spans of variables with the same name from the later + // patterns so the suggestions to prefix with underscores will apply to those too. + let mut vars: BTreeMap)> = Default::default(); + + for pat in &arm.pats { + this.arm_pats_bindings(Some(&*pat), |this, ln, var, sp, id| { + let name = this.ir.variable_name(var); + vars.entry(name) + .and_modify(|(.., spans)| { + spans.push(sp); + }) + .or_insert_with(|| { + (ln, var, id, vec![sp]) + }); + }); + } + + for (_, (ln, var, id, spans)) in vars { + this.warn_about_unused(spans, id, ln, var); + } + intravisit::walk_arm(this, arm); } @@ -1551,7 +1568,7 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> { let var = self.variable(hir_id, sp); // Ignore unused self. if ident.name != keywords::SelfLower.name() { - if !self.warn_about_unused(sp, hir_id, entry_ln, var) { + if !self.warn_about_unused(vec![sp], hir_id, entry_ln, var) { if self.live_on_entry(entry_ln, var).is_none() { self.report_dead_assign(hir_id, sp, var, true); } @@ -1563,14 +1580,14 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> { fn warn_about_unused_or_dead_vars_in_pat(&mut self, pat: &hir::Pat) { self.pat_bindings(pat, |this, ln, var, sp, id| { - if !this.warn_about_unused(sp, id, ln, var) { + if !this.warn_about_unused(vec![sp], id, ln, var) { this.warn_about_dead_assign(sp, id, ln, var); } }) } fn warn_about_unused(&self, - sp: Span, + spans: Vec, hir_id: HirId, ln: LiveNode, var: Variable) @@ -1587,33 +1604,36 @@ 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_hir_note(lint::builtin::UNUSED_VARIABLES, hir_id, sp, - &format!("variable `{}` is assigned to, but never used", - name), - &suggest_underscore_msg); + self.ir.tcx.lint_hir_note( + lint::builtin::UNUSED_VARIABLES, + hir_id, + spans.clone(), + &format!("variable `{}` is assigned to, but never used", name), + &format!("consider using `_{}` instead", name), + ); } else if name != "self" { - let msg = format!("unused variable: `{}`", name); - let mut err = self.ir.tcx - .struct_span_lint_hir(lint::builtin::UNUSED_VARIABLES, hir_id, sp, &msg); + let mut err = self.ir.tcx.struct_span_lint_hir( + lint::builtin::UNUSED_VARIABLES, + hir_id, + spans.clone(), + &format!("unused variable: `{}`", name), + ); + if self.ir.variable_is_shorthand(var) { - err.span_suggestion( - sp, + err.multipart_suggestion( "try ignoring the field", - format!("{}: _", name), - Applicability::MachineApplicable, + spans.iter().map(|span| (*span, format!("{}: _", name))).collect(), + Applicability::MachineApplicable ); } else { - err.span_suggestion_short( - sp, - &suggest_underscore_msg, - format!("_{}", name), + err.multipart_suggestion( + "consider prefixing with an underscore", + spans.iter().map(|span| (*span, format!("_{}", name))).collect(), Applicability::MachineApplicable, ); } + err.emit() } } @@ -1623,11 +1643,7 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> { } } - fn warn_about_dead_assign(&self, - sp: Span, - hir_id: HirId, - ln: LiveNode, - var: Variable) { + fn warn_about_dead_assign(&self, sp: Span, hir_id: HirId, ln: LiveNode, var: Variable) { if self.live_on_exit(ln, var).is_none() { self.report_dead_assign(hir_id, sp, var, false); } diff --git a/src/test/ui/issues/issue-17999.stderr b/src/test/ui/issues/issue-17999.stderr index 3f32dc1540232..51c0c757a80c1 100644 --- a/src/test/ui/issues/issue-17999.stderr +++ b/src/test/ui/issues/issue-17999.stderr @@ -2,7 +2,7 @@ error: unused variable: `x` --> $DIR/issue-17999.rs:5:13 | LL | let x = (); //~ ERROR: unused variable: `x` - | ^ help: consider using `_x` instead + | ^ help: consider prefixing with an underscore: `_x` | note: lint level defined here --> $DIR/issue-17999.rs:1:9 @@ -14,7 +14,7 @@ error: unused variable: `a` --> $DIR/issue-17999.rs:7:13 | LL | a => {} //~ ERROR: unused variable: `a` - | ^ help: consider using `_a` instead + | ^ help: consider prefixing with an underscore: `_a` error: aborting due to 2 previous errors diff --git a/src/test/ui/issues/issue-22599.stderr b/src/test/ui/issues/issue-22599.stderr index 5c2ed7455f132..bc4949da6f785 100644 --- a/src/test/ui/issues/issue-22599.stderr +++ b/src/test/ui/issues/issue-22599.stderr @@ -2,7 +2,7 @@ error: unused variable: `a` --> $DIR/issue-22599.rs:8:19 | LL | v = match 0 { a => 0 }; //~ ERROR: unused variable: `a` - | ^ help: consider using `_a` instead + | ^ help: consider prefixing with an underscore: `_a` | note: lint level defined here --> $DIR/issue-22599.rs:1:9 diff --git a/src/test/ui/issues/issue-56685.rs b/src/test/ui/issues/issue-56685.rs new file mode 100644 index 0000000000000..f320c99ed15d1 --- /dev/null +++ b/src/test/ui/issues/issue-56685.rs @@ -0,0 +1,44 @@ +#![allow(dead_code)] +#![deny(unused_variables)] + +// This test aims to check that unused variable suggestions update bindings in all +// match arms. + +fn main() { + enum E { + A(i32,), + B(i32,), + } + + match E::A(1) { + E::A(x) | E::B(x) => {} + //~^ ERROR unused variable: `x` + } + + enum F { + A(i32, i32,), + B(i32, i32,), + C(i32, i32,), + } + + let _ = match F::A(1, 2) { + F::A(x, y) | F::B(x, y) => { y }, + //~^ ERROR unused variable: `x` + F::C(a, b) => { 3 } + //~^ ERROR unused variable: `a` + //~^^ ERROR unused variable: `b` + }; + + let _ = if let F::A(x, y) | F::B(x, y) = F::A(1, 2) { + //~^ ERROR unused variable: `x` + y + } else { + 3 + }; + + while let F::A(x, y) | F::B(x, y) = F::A(1, 2) { + //~^ ERROR unused variable: `x` + let _ = y; + break; + } +} diff --git a/src/test/ui/issues/issue-56685.stderr b/src/test/ui/issues/issue-56685.stderr new file mode 100644 index 0000000000000..4a461c72b2417 --- /dev/null +++ b/src/test/ui/issues/issue-56685.stderr @@ -0,0 +1,60 @@ +error: unused variable: `x` + --> $DIR/issue-56685.rs:14:14 + | +LL | E::A(x) | E::B(x) => {} + | ^ ^ + | +note: lint level defined here + --> $DIR/issue-56685.rs:2:9 + | +LL | #![deny(unused_variables)] + | ^^^^^^^^^^^^^^^^ +help: consider prefixing with an underscore + | +LL | E::A(_x) | E::B(_x) => {} + | ^^ ^^ + +error: unused variable: `x` + --> $DIR/issue-56685.rs:25:14 + | +LL | F::A(x, y) | F::B(x, y) => { y }, + | ^ ^ +help: consider prefixing with an underscore + | +LL | F::A(_x, y) | F::B(_x, y) => { y }, + | ^^ ^^ + +error: unused variable: `a` + --> $DIR/issue-56685.rs:27:14 + | +LL | F::C(a, b) => { 3 } + | ^ help: consider prefixing with an underscore: `_a` + +error: unused variable: `b` + --> $DIR/issue-56685.rs:27:17 + | +LL | F::C(a, b) => { 3 } + | ^ help: consider prefixing with an underscore: `_b` + +error: unused variable: `x` + --> $DIR/issue-56685.rs:32:25 + | +LL | let _ = if let F::A(x, y) | F::B(x, y) = F::A(1, 2) { + | ^ ^ +help: consider prefixing with an underscore + | +LL | let _ = if let F::A(_x, y) | F::B(_x, y) = F::A(1, 2) { + | ^^ ^^ + +error: unused variable: `x` + --> $DIR/issue-56685.rs:39:20 + | +LL | while let F::A(x, y) | F::B(x, y) = F::A(1, 2) { + | ^ ^ +help: consider prefixing with an underscore + | +LL | while let F::A(_x, y) | F::B(_x, y) = F::A(1, 2) { + | ^^ ^^ + +error: aborting due to 6 previous errors + 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 index ab12aef2474f3..8fd98e0a3db2e 100644 --- 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 @@ -2,7 +2,7 @@ warning: unused variable: `i_think_continually` --> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:26:9 | LL | let i_think_continually = 2; - | ^^^^^^^^^^^^^^^^^^^ help: consider using `_i_think_continually` instead + | ^^^^^^^^^^^^^^^^^^^ help: consider prefixing with an underscore: `_i_think_continually` | note: lint level defined here --> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:5:9 @@ -15,19 +15,19 @@ warning: unused variable: `mut_unused_var` --> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:33:13 | LL | let mut mut_unused_var = 1; - | ^^^^^^^^^^^^^^ help: consider using `_mut_unused_var` instead + | ^^^^^^^^^^^^^^ help: consider prefixing with an underscore: `_mut_unused_var` warning: unused variable: `var` --> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:35:14 | LL | let (mut var, unused_var) = (1, 2); - | ^^^ help: consider using `_var` instead + | ^^^ help: consider prefixing with an underscore: `_var` warning: unused variable: `unused_var` --> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:35:19 | LL | let (mut var, unused_var) = (1, 2); - | ^^^^^^^^^^ help: consider using `_unused_var` instead + | ^^^^^^^^^^ help: consider prefixing with an underscore: `_unused_var` warning: unused variable: `corridors_of_light` --> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:37:26 diff --git a/src/test/ui/lint/lint-removed-allow.stderr b/src/test/ui/lint/lint-removed-allow.stderr index 3af983165eb17..f796a14d63796 100644 --- a/src/test/ui/lint/lint-removed-allow.stderr +++ b/src/test/ui/lint/lint-removed-allow.stderr @@ -2,7 +2,7 @@ error: unused variable: `unused` --> $DIR/lint-removed-allow.rs:8:17 | LL | fn main() { let unused = (); } //~ ERROR unused - | ^^^^^^ help: consider using `_unused` instead + | ^^^^^^ help: consider prefixing with an underscore: `_unused` | note: lint level defined here --> $DIR/lint-removed-allow.rs:7:8 diff --git a/src/test/ui/lint/lint-removed-cmdline.stderr b/src/test/ui/lint/lint-removed-cmdline.stderr index 191509c5a180c..d46ef6b9237fd 100644 --- a/src/test/ui/lint/lint-removed-cmdline.stderr +++ b/src/test/ui/lint/lint-removed-cmdline.stderr @@ -6,7 +6,7 @@ error: unused variable: `unused` --> $DIR/lint-removed-cmdline.rs:12:17 | LL | fn main() { let unused = (); } - | ^^^^^^ help: consider using `_unused` instead + | ^^^^^^ help: consider prefixing with an underscore: `_unused` | note: lint level defined here --> $DIR/lint-removed-cmdline.rs:11:8 diff --git a/src/test/ui/lint/lint-removed.stderr b/src/test/ui/lint/lint-removed.stderr index 34fa85b0d8b9f..55f010348fe1e 100644 --- a/src/test/ui/lint/lint-removed.stderr +++ b/src/test/ui/lint/lint-removed.stderr @@ -10,7 +10,7 @@ error: unused variable: `unused` --> $DIR/lint-removed.rs:8:17 | LL | fn main() { let unused = (); } //~ ERROR unused - | ^^^^^^ help: consider using `_unused` instead + | ^^^^^^ help: consider prefixing with an underscore: `_unused` | note: lint level defined here --> $DIR/lint-removed.rs:7:8 diff --git a/src/test/ui/lint/lint-renamed-allow.stderr b/src/test/ui/lint/lint-renamed-allow.stderr index 390b4c3f38abd..b2eeeae8f8e6d 100644 --- a/src/test/ui/lint/lint-renamed-allow.stderr +++ b/src/test/ui/lint/lint-renamed-allow.stderr @@ -2,7 +2,7 @@ error: unused variable: `unused` --> $DIR/lint-renamed-allow.rs:8:17 | LL | fn main() { let unused = (); } //~ ERROR unused - | ^^^^^^ help: consider using `_unused` instead + | ^^^^^^ help: consider prefixing with an underscore: `_unused` | note: lint level defined here --> $DIR/lint-renamed-allow.rs:7:8 diff --git a/src/test/ui/lint/lint-renamed-cmdline.stderr b/src/test/ui/lint/lint-renamed-cmdline.stderr index 78a05bdc89f01..6247ee0aff833 100644 --- a/src/test/ui/lint/lint-renamed-cmdline.stderr +++ b/src/test/ui/lint/lint-renamed-cmdline.stderr @@ -6,7 +6,7 @@ error: unused variable: `unused` --> $DIR/lint-renamed-cmdline.rs:8:17 | LL | fn main() { let unused = (); } - | ^^^^^^ help: consider using `_unused` instead + | ^^^^^^ help: consider prefixing with an underscore: `_unused` | note: lint level defined here --> $DIR/lint-renamed-cmdline.rs:7:8 diff --git a/src/test/ui/lint/lint-renamed.stderr b/src/test/ui/lint/lint-renamed.stderr index 1296fef5a4c63..b140a93ab38bd 100644 --- a/src/test/ui/lint/lint-renamed.stderr +++ b/src/test/ui/lint/lint-renamed.stderr @@ -10,7 +10,7 @@ error: unused variable: `unused` --> $DIR/lint-renamed.rs:4:17 | LL | fn main() { let unused = (); } //~ ERROR unused - | ^^^^^^ help: consider using `_unused` instead + | ^^^^^^ help: consider prefixing with an underscore: `_unused` | note: lint level defined here --> $DIR/lint-renamed.rs:3:8 diff --git a/src/test/ui/lint/lint-uppercase-variables.stderr b/src/test/ui/lint/lint-uppercase-variables.stderr index 0741179c4a4dd..f2267f351ddda 100644 --- a/src/test/ui/lint/lint-uppercase-variables.stderr +++ b/src/test/ui/lint/lint-uppercase-variables.stderr @@ -8,7 +8,7 @@ warning: unused variable: `Foo` --> $DIR/lint-uppercase-variables.rs:22:9 | LL | Foo => {} - | ^^^ help: consider using `_Foo` instead + | ^^^ help: consider prefixing with an underscore: `_Foo` | note: lint level defined here --> $DIR/lint-uppercase-variables.rs:1:9 diff --git a/src/test/ui/liveness/liveness-unused.stderr b/src/test/ui/liveness/liveness-unused.stderr index 17cdacb859f02..49795faa59c8d 100644 --- a/src/test/ui/liveness/liveness-unused.stderr +++ b/src/test/ui/liveness/liveness-unused.stderr @@ -15,7 +15,7 @@ error: unused variable: `x` --> $DIR/liveness-unused.rs:8:7 | LL | fn f1(x: isize) { - | ^ help: consider using `_x` instead + | ^ help: consider prefixing with an underscore: `_x` | note: lint level defined here --> $DIR/liveness-unused.rs:2:9 @@ -27,19 +27,19 @@ error: unused variable: `x` --> $DIR/liveness-unused.rs:12:8 | LL | fn f1b(x: &mut isize) { - | ^ help: consider using `_x` instead + | ^ help: consider prefixing with an underscore: `_x` error: unused variable: `x` --> $DIR/liveness-unused.rs:20:9 | LL | let x: isize; - | ^ help: consider using `_x` instead + | ^ help: consider prefixing with an underscore: `_x` error: unused variable: `x` --> $DIR/liveness-unused.rs:25:9 | LL | let x = 3; - | ^ help: consider using `_x` instead + | ^ help: consider prefixing with an underscore: `_x` error: variable `x` is assigned to, but never used --> $DIR/liveness-unused.rs:30:13 @@ -74,25 +74,25 @@ error: unused variable: `i` --> $DIR/liveness-unused.rs:59:12 | LL | Some(i) => { - | ^ help: consider using `_i` instead + | ^ help: consider prefixing with an underscore: `_i` error: unused variable: `x` --> $DIR/liveness-unused.rs:79:9 | LL | for x in 1..10 { } - | ^ help: consider using `_x` instead + | ^ help: consider prefixing with an underscore: `_x` error: unused variable: `x` --> $DIR/liveness-unused.rs:84:10 | LL | for (x, _) in [1, 2, 3].iter().enumerate() { } - | ^ help: consider using `_x` instead + | ^ help: consider prefixing with an underscore: `_x` error: unused variable: `x` --> $DIR/liveness-unused.rs:89:13 | LL | for (_, x) in [1, 2, 3].iter().enumerate() { - | ^ help: consider using `_x` instead + | ^ help: consider prefixing with an underscore: `_x` error: variable `x` is assigned to, but never used --> $DIR/liveness-unused.rs:112:9 diff --git a/src/test/ui/never-assign-dead-code.stderr b/src/test/ui/never-assign-dead-code.stderr index c59fd938e8326..bcc20a0d703c0 100644 --- a/src/test/ui/never-assign-dead-code.stderr +++ b/src/test/ui/never-assign-dead-code.stderr @@ -21,7 +21,7 @@ warning: unused variable: `x` --> $DIR/never-assign-dead-code.rs:9:9 | LL | let x: ! = panic!("aah"); //~ WARN unused - | ^ help: consider using `_x` instead + | ^ help: consider prefixing with an underscore: `_x` | note: lint level defined here --> $DIR/never-assign-dead-code.rs:5:9 diff --git a/src/test/ui/proc-macro/attributes-included.stderr b/src/test/ui/proc-macro/attributes-included.stderr index ea5be5a3be0e5..87b6fb9539fb4 100644 --- a/src/test/ui/proc-macro/attributes-included.stderr +++ b/src/test/ui/proc-macro/attributes-included.stderr @@ -2,7 +2,7 @@ warning: unused variable: `a` --> $DIR/attributes-included.rs:17:9 | LL | let a: i32 = "foo"; //~ WARN: unused variable - | ^ help: consider using `_a` instead + | ^ help: consider prefixing with an underscore: `_a` | note: lint level defined here --> $DIR/attributes-included.rs:4:9 diff --git a/src/test/ui/span/issue-24690.stderr b/src/test/ui/span/issue-24690.stderr index f052f866c901b..964e323b83ed4 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:13:9 | LL | let theOtherTwo = 2; //~ WARN should have a snake case name - | ^^^^^^^^^^^ help: consider using `_theOtherTwo` instead + | ^^^^^^^^^^^ help: consider prefixing with an underscore: `_theOtherTwo` | note: lint level defined here --> $DIR/issue-24690.rs:8:9