Skip to content

Commit 5f021e0

Browse files
davidtwcodavidtwco
committed
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.
1 parent c325155 commit 5f021e0

17 files changed

+179
-59
lines changed

‎src/librustc/middle/liveness.rs‎

Lines changed: 50 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ use lint;
105105
use errors::Applicability;
106106
use util::nodemap::{NodeMap, HirIdMap, HirIdSet};
107107

108-
use std::collections::VecDeque;
108+
use std::collections::{BTreeMap, VecDeque};
109109
use std::{fmt, u32};
110110
use std::io::prelude::*;
111111
use std::io;
@@ -1446,7 +1446,7 @@ fn check_local<'a, 'tcx>(this: &mut Liveness<'a, 'tcx>, local: &'tcx hir::Local)
14461446
None => {
14471447
this.pat_bindings(&local.pat, |this, ln, var, sp, id| {
14481448
let span = local.pat.simple_ident().map_or(sp, |ident| ident.span);
1449-
this.warn_about_unused(span, id, ln, var);
1449+
this.warn_about_unused(vec![span], id, ln, var);
14501450
})
14511451
}
14521452
}
@@ -1455,12 +1455,29 @@ fn check_local<'a, 'tcx>(this: &mut Liveness<'a, 'tcx>, local: &'tcx hir::Local)
14551455
}
14561456

14571457
fn check_arm<'a, 'tcx>(this: &mut Liveness<'a, 'tcx>, arm: &'tcx hir::Arm) {
1458-
// only consider the first pattern; any later patterns must have
1459-
// the same bindings, and we also consider the first pattern to be
1460-
// the "authoritative" set of ids
1461-
this.arm_pats_bindings(arm.pats.first().map(|p| &**p), |this, ln, var, sp, id| {
1462-
this.warn_about_unused(sp, id, ln, var);
1463-
});
1458+
// Only consider the variable from the first pattern; any later patterns must have
1459+
// the same bindings, and we also consider the first pattern to be the "authoritative" set of
1460+
// ids. However, we should take the spans of variables with the same name from the later
1461+
// patterns so the suggestions to prefix with underscores will apply to those too.
1462+
let mut vars: BTreeMap<String, (LiveNode, Variable, HirId, Vec<Span>)> = Default::default();
1463+
1464+
for pat in &arm.pats {
1465+
this.arm_pats_bindings(Some(&*pat), |this, ln, var, sp, id| {
1466+
let name = this.ir.variable_name(var);
1467+
vars.entry(name)
1468+
.and_modify(|(.., spans)| {
1469+
spans.push(sp);
1470+
})
1471+
.or_insert_with(|| {
1472+
(ln, var, id, vec![sp])
1473+
});
1474+
});
1475+
}
1476+
1477+
for (_, (ln, var, id, spans)) in vars {
1478+
this.warn_about_unused(spans, id, ln, var);
1479+
}
1480+
14641481
intravisit::walk_arm(this, arm);
14651482
}
14661483

@@ -1551,7 +1568,7 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> {
15511568
let var = self.variable(hir_id, sp);
15521569
// Ignore unused self.
15531570
if ident.name != keywords::SelfLower.name() {
1554-
if !self.warn_about_unused(sp, hir_id, entry_ln, var) {
1571+
if !self.warn_about_unused(vec![sp], hir_id, entry_ln, var) {
15551572
if self.live_on_entry(entry_ln, var).is_none() {
15561573
self.report_dead_assign(hir_id, sp, var, true);
15571574
}
@@ -1563,14 +1580,14 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> {
15631580

15641581
fn warn_about_unused_or_dead_vars_in_pat(&mut self, pat: &hir::Pat) {
15651582
self.pat_bindings(pat, |this, ln, var, sp, id| {
1566-
if !this.warn_about_unused(sp, id, ln, var) {
1583+
if !this.warn_about_unused(vec![sp], id, ln, var) {
15671584
this.warn_about_dead_assign(sp, id, ln, var);
15681585
}
15691586
})
15701587
}
15711588

15721589
fn warn_about_unused(&self,
1573-
sp: Span,
1590+
spans: Vec<Span>,
15741591
hir_id: HirId,
15751592
ln: LiveNode,
15761593
var: Variable)
@@ -1587,33 +1604,36 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> {
15871604
self.assigned_on_exit(ln, var).is_some()
15881605
};
15891606

1590-
let suggest_underscore_msg = format!("consider using `_{}` instead", name);
1591-
15921607
if is_assigned {
1593-
self.ir.tcx
1594-
.lint_hir_note(lint::builtin::UNUSED_VARIABLES, hir_id, sp,
1595-
&format!("variable `{}` is assigned to, but never used",
1596-
name),
1597-
&suggest_underscore_msg);
1608+
self.ir.tcx.lint_hir_note(
1609+
lint::builtin::UNUSED_VARIABLES,
1610+
hir_id,
1611+
spans.clone(),
1612+
&format!("variable `{}` is assigned to, but never used", name),
1613+
&format!("consider using `_{}` instead", name),
1614+
);
15981615
} else if name != "self" {
1599-
let msg = format!("unused variable: `{}`", name);
1600-
let mut err = self.ir.tcx
1601-
.struct_span_lint_hir(lint::builtin::UNUSED_VARIABLES, hir_id, sp, &msg);
1616+
let mut err = self.ir.tcx.struct_span_lint_hir(
1617+
lint::builtin::UNUSED_VARIABLES,
1618+
hir_id,
1619+
spans.clone(),
1620+
&format!("unused variable: `{}`", name),
1621+
);
1622+
16021623
if self.ir.variable_is_shorthand(var) {
1603-
err.span_suggestion(
1604-
sp,
1624+
err.multipart_suggestion(
16051625
"try ignoring the field",
1606-
format!("{}: _", name),
1607-
Applicability::MachineApplicable,
1626+
spans.iter().map(|span| (*span, format!("{}: _", name))).collect(),
1627+
Applicability::MachineApplicable
16081628
);
16091629
} else {
1610-
err.span_suggestion_short(
1611-
sp,
1612-
&suggest_underscore_msg,
1613-
format!("_{}", name),
1630+
err.multipart_suggestion(
1631+
"consider prefixing with an underscore",
1632+
spans.iter().map(|span| (*span, format!("_{}", name))).collect(),
16141633
Applicability::MachineApplicable,
16151634
);
16161635
}
1636+
16171637
err.emit()
16181638
}
16191639
}
@@ -1623,11 +1643,7 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> {
16231643
}
16241644
}
16251645

1626-
fn warn_about_dead_assign(&self,
1627-
sp: Span,
1628-
hir_id: HirId,
1629-
ln: LiveNode,
1630-
var: Variable) {
1646+
fn warn_about_dead_assign(&self, sp: Span, hir_id: HirId, ln: LiveNode, var: Variable) {
16311647
if self.live_on_exit(ln, var).is_none() {
16321648
self.report_dead_assign(hir_id, sp, var, false);
16331649
}

‎src/test/ui/issues/issue-17999.stderr‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ error: unused variable: `x`
22
--> $DIR/issue-17999.rs:5:13
33
|
44
LL | let x = (); //~ ERROR: unused variable: `x`
5-
| ^ help: consider using `_x` instead
5+
| ^ help: consider prefixing with an underscore: `_x`
66
|
77
note: lint level defined here
88
--> $DIR/issue-17999.rs:1:9
@@ -14,7 +14,7 @@ error: unused variable: `a`
1414
--> $DIR/issue-17999.rs:7:13
1515
|
1616
LL | a => {} //~ ERROR: unused variable: `a`
17-
| ^ help: consider using `_a` instead
17+
| ^ help: consider prefixing with an underscore: `_a`
1818

1919
error: aborting due to 2 previous errors
2020

‎src/test/ui/issues/issue-22599.stderr‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ error: unused variable: `a`
22
--> $DIR/issue-22599.rs:8:19
33
|
44
LL | v = match 0 { a => 0 }; //~ ERROR: unused variable: `a`
5-
| ^ help: consider using `_a` instead
5+
| ^ help: consider prefixing with an underscore: `_a`
66
|
77
note: lint level defined here
88
--> $DIR/issue-22599.rs:1:9

‎src/test/ui/issues/issue-56685.rs‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
#![allow(dead_code)]
2+
#![deny(unused_variables)]
3+
4+
// This test aims to check that unused variable suggestions update bindings in all
5+
// match arms.
6+
7+
fn main() {
8+
enum E {
9+
A(i32,),
10+
B(i32,),
11+
}
12+
13+
match E::A(1) {
14+
E::A(x) | E::B(x) => {}
15+
//~^ ERROR unused variable: `x`
16+
}
17+
18+
enum F {
19+
A(i32, i32,),
20+
B(i32, i32,),
21+
C(i32, i32,),
22+
}
23+
24+
let _ = match F::A(1, 2) {
25+
F::A(x, y) | F::B(x, y) => { y },
26+
//~^ ERROR unused variable: `x`
27+
F::C(a, b) => { 3 }
28+
//~^ ERROR unused variable: `a`
29+
//~^^ ERROR unused variable: `b`
30+
};
31+
32+
let _ = if let F::A(x, y) | F::B(x, y) = F::A(1, 2) {
33+
//~^ ERROR unused variable: `x`
34+
y
35+
} else {
36+
3
37+
};
38+
39+
while let F::A(x, y) | F::B(x, y) = F::A(1, 2) {
40+
//~^ ERROR unused variable: `x`
41+
let _ = y;
42+
break;
43+
}
44+
}

‎src/test/ui/issues/issue-56685.stderr‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
error: unused variable: `x`
2+
--> $DIR/issue-56685.rs:14:14
3+
|
4+
LL | E::A(x) | E::B(x) => {}
5+
| ^ ^
6+
|
7+
note: lint level defined here
8+
--> $DIR/issue-56685.rs:2:9
9+
|
10+
LL | #![deny(unused_variables)]
11+
| ^^^^^^^^^^^^^^^^
12+
help: consider prefixing with an underscore
13+
|
14+
LL | E::A(_x) | E::B(_x) => {}
15+
| ^^ ^^
16+
17+
error: unused variable: `x`
18+
--> $DIR/issue-56685.rs:25:14
19+
|
20+
LL | F::A(x, y) | F::B(x, y) => { y },
21+
| ^ ^
22+
help: consider prefixing with an underscore
23+
|
24+
LL | F::A(_x, y) | F::B(_x, y) => { y },
25+
| ^^ ^^
26+
27+
error: unused variable: `a`
28+
--> $DIR/issue-56685.rs:27:14
29+
|
30+
LL | F::C(a, b) => { 3 }
31+
| ^ help: consider prefixing with an underscore: `_a`
32+
33+
error: unused variable: `b`
34+
--> $DIR/issue-56685.rs:27:17
35+
|
36+
LL | F::C(a, b) => { 3 }
37+
| ^ help: consider prefixing with an underscore: `_b`
38+
39+
error: unused variable: `x`
40+
--> $DIR/issue-56685.rs:32:25
41+
|
42+
LL | let _ = if let F::A(x, y) | F::B(x, y) = F::A(1, 2) {
43+
| ^ ^
44+
help: consider prefixing with an underscore
45+
|
46+
LL | let _ = if let F::A(_x, y) | F::B(_x, y) = F::A(1, 2) {
47+
| ^^ ^^
48+
49+
error: unused variable: `x`
50+
--> $DIR/issue-56685.rs:39:20
51+
|
52+
LL | while let F::A(x, y) | F::B(x, y) = F::A(1, 2) {
53+
| ^ ^
54+
help: consider prefixing with an underscore
55+
|
56+
LL | while let F::A(_x, y) | F::B(_x, y) = F::A(1, 2) {
57+
| ^^ ^^
58+
59+
error: aborting due to 6 previous errors
60+

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ warning: unused variable: `i_think_continually`
22
--> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:26:9
33
|
44
LL | let i_think_continually = 2;
5-
| ^^^^^^^^^^^^^^^^^^^ help: consider using `_i_think_continually` instead
5+
| ^^^^^^^^^^^^^^^^^^^ help: consider prefixing with an underscore: `_i_think_continually`
66
|
77
note: lint level defined here
88
--> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:5:9
@@ -15,19 +15,19 @@ warning: unused variable: `mut_unused_var`
1515
--> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:33:13
1616
|
1717
LL | let mut mut_unused_var = 1;
18-
| ^^^^^^^^^^^^^^ help: consider using `_mut_unused_var` instead
18+
| ^^^^^^^^^^^^^^ help: consider prefixing with an underscore: `_mut_unused_var`
1919

2020
warning: unused variable: `var`
2121
--> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:35:14
2222
|
2323
LL | let (mut var, unused_var) = (1, 2);
24-
| ^^^ help: consider using `_var` instead
24+
| ^^^ help: consider prefixing with an underscore: `_var`
2525

2626
warning: unused variable: `unused_var`
2727
--> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:35:19
2828
|
2929
LL | let (mut var, unused_var) = (1, 2);
30-
| ^^^^^^^^^^ help: consider using `_unused_var` instead
30+
| ^^^^^^^^^^ help: consider prefixing with an underscore: `_unused_var`
3131

3232
warning: unused variable: `corridors_of_light`
3333
--> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:37:26

‎src/test/ui/lint/lint-removed-allow.stderr‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ error: unused variable: `unused`
22
--> $DIR/lint-removed-allow.rs:8:17
33
|
44
LL | fn main() { let unused = (); } //~ ERROR unused
5-
| ^^^^^^ help: consider using `_unused` instead
5+
| ^^^^^^ help: consider prefixing with an underscore: `_unused`
66
|
77
note: lint level defined here
88
--> $DIR/lint-removed-allow.rs:7:8

‎src/test/ui/lint/lint-removed-cmdline.stderr‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ error: unused variable: `unused`
66
--> $DIR/lint-removed-cmdline.rs:12:17
77
|
88
LL | fn main() { let unused = (); }
9-
| ^^^^^^ help: consider using `_unused` instead
9+
| ^^^^^^ help: consider prefixing with an underscore: `_unused`
1010
|
1111
note: lint level defined here
1212
--> $DIR/lint-removed-cmdline.rs:11:8

‎src/test/ui/lint/lint-removed.stderr‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ error: unused variable: `unused`
1010
--> $DIR/lint-removed.rs:8:17
1111
|
1212
LL | fn main() { let unused = (); } //~ ERROR unused
13-
| ^^^^^^ help: consider using `_unused` instead
13+
| ^^^^^^ help: consider prefixing with an underscore: `_unused`
1414
|
1515
note: lint level defined here
1616
--> $DIR/lint-removed.rs:7:8

‎src/test/ui/lint/lint-renamed-allow.stderr‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ error: unused variable: `unused`
22
--> $DIR/lint-renamed-allow.rs:8:17
33
|
44
LL | fn main() { let unused = (); } //~ ERROR unused
5-
| ^^^^^^ help: consider using `_unused` instead
5+
| ^^^^^^ help: consider prefixing with an underscore: `_unused`
66
|
77
note: lint level defined here
88
--> $DIR/lint-renamed-allow.rs:7:8

0 commit comments

Comments
 (0)