Skip to content

Commit 12cdaec

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 a41ade7 commit 12cdaec

17 files changed

+181
-56
lines changed

‎src/librustc/middle/liveness.rs

+52-31
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,7 @@ use ty::query::{Providers, queries};
104104
use lint;
105105
use errors::Applicability;
106106
use util::nodemap::{NodeMap, HirIdMap, HirIdSet};
107+
use rustc_data_structures::fx::FxHashMap;
107108

108109
use std::collections::VecDeque;
109110
use std::{fmt, u32};
@@ -1446,7 +1447,7 @@ fn check_local<'a, 'tcx>(this: &mut Liveness<'a, 'tcx>, local: &'tcx hir::Local)
14461447
None => {
14471448
this.pat_bindings(&local.pat, |this, ln, var, sp, id| {
14481449
let span = local.pat.simple_ident().map_or(sp, |ident| ident.span);
1449-
this.warn_about_unused(span, id, ln, var);
1450+
this.warn_about_unused(vec![span], id, ln, var);
14501451
})
14511452
}
14521453
}
@@ -1455,12 +1456,29 @@ fn check_local<'a, 'tcx>(this: &mut Liveness<'a, 'tcx>, local: &'tcx hir::Local)
14551456
}
14561457

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

@@ -1551,7 +1569,7 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> {
15511569
let var = self.variable(hir_id, sp);
15521570
// Ignore unused self.
15531571
if ident.name != keywords::SelfLower.name() {
1554-
if !self.warn_about_unused(sp, hir_id, entry_ln, var) {
1572+
if !self.warn_about_unused(vec![sp], hir_id, entry_ln, var) {
15551573
if self.live_on_entry(entry_ln, var).is_none() {
15561574
self.report_dead_assign(hir_id, sp, var, true);
15571575
}
@@ -1563,14 +1581,14 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> {
15631581

15641582
fn warn_about_unused_or_dead_vars_in_pat(&mut self, pat: &hir::Pat) {
15651583
self.pat_bindings(pat, |this, ln, var, sp, id| {
1566-
if !this.warn_about_unused(sp, id, ln, var) {
1584+
if !this.warn_about_unused(vec![sp], id, ln, var) {
15671585
this.warn_about_dead_assign(sp, id, ln, var);
15681586
}
15691587
})
15701588
}
15711589

15721590
fn warn_about_unused(&self,
1573-
sp: Span,
1591+
spans: Vec<Span>,
15741592
hir_id: HirId,
15751593
ln: LiveNode,
15761594
var: Variable)
@@ -1587,29 +1605,36 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> {
15871605
self.assigned_on_exit(ln, var).is_some()
15881606
};
15891607

1590-
let suggest_underscore_msg = format!("consider using `_{}` instead", name);
1591-
15921608
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);
1609+
self.ir.tcx.lint_hir_note(
1610+
lint::builtin::UNUSED_VARIABLES,
1611+
hir_id,
1612+
spans.clone(),
1613+
&format!("variable `{}` is assigned to, but never used", name),
1614+
&format!("consider using `_{}` instead", name),
1615+
);
15981616
} 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);
1617+
let mut err = self.ir.tcx.struct_span_lint_hir(
1618+
lint::builtin::UNUSED_VARIABLES,
1619+
hir_id,
1620+
spans.clone(),
1621+
&format!("unused variable: `{}`", name),
1622+
);
1623+
16021624
if self.ir.variable_is_shorthand(var) {
1603-
err.span_suggestion_with_applicability(sp, "try ignoring the field",
1604-
format!("{}: _", name),
1605-
Applicability::MachineApplicable);
1625+
err.multipart_suggestion_with_applicability(
1626+
"try ignoring the field",
1627+
spans.iter().map(|span| (*span, format!("{}: _", name))).collect(),
1628+
Applicability::MachineApplicable
1629+
);
16061630
} else {
1607-
err.span_suggestion_short_with_applicability(
1608-
sp, &suggest_underscore_msg,
1609-
format!("_{}", name),
1631+
err.multipart_suggestion_with_applicability(
1632+
"consider prefixing with an underscore",
1633+
spans.iter().map(|span| (*span, format!("_{}", name))).collect(),
16101634
Applicability::MachineApplicable,
16111635
);
16121636
}
1637+
16131638
err.emit()
16141639
}
16151640
}
@@ -1619,11 +1644,7 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> {
16191644
}
16201645
}
16211646

1622-
fn warn_about_dead_assign(&self,
1623-
sp: Span,
1624-
hir_id: HirId,
1625-
ln: LiveNode,
1626-
var: Variable) {
1647+
fn warn_about_dead_assign(&self, sp: Span, hir_id: HirId, ln: LiveNode, var: Variable) {
16271648
if self.live_on_exit(ln, var).is_none() {
16281649
self.report_dead_assign(hir_id, sp, var, false);
16291650
}

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

+2-2
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

+1-1
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

+44
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

+60
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: `b`
28+
--> $DIR/issue-56685.rs:27:17
29+
|
30+
LL | F::C(a, b) => { 3 }
31+
| ^ help: consider prefixing with an underscore: `_b`
32+
33+
error: unused variable: `a`
34+
--> $DIR/issue-56685.rs:27:14
35+
|
36+
LL | F::C(a, b) => { 3 }
37+
| ^ help: consider prefixing with an underscore: `_a`
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

+4-4
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

+1-1
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

+1-1
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

+1-1
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

+1-1
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

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ error: unused variable: `unused`
66
--> $DIR/lint-renamed-cmdline.rs:8: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-renamed-cmdline.rs:7:8

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ error: unused variable: `unused`
1010
--> $DIR/lint-renamed.rs:4: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-renamed.rs:3:8

‎src/test/ui/lint/lint-uppercase-variables.stderr

+1-1
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ warning: unused variable: `Foo`
88
--> $DIR/lint-uppercase-variables.rs:22:9
99
|
1010
LL | Foo => {}
11-
| ^^^ help: consider using `_Foo` instead
11+
| ^^^ help: consider prefixing with an underscore: `_Foo`
1212
|
1313
note: lint level defined here
1414
--> $DIR/lint-uppercase-variables.rs:1:9

0 commit comments

Comments
 (0)