Skip to content

Commit d259d1d

Browse files
GuillaumeGomezGuillaumeGomez
authored
Rollup merge of rust-lang#82655 - SkiFire13:fix-issue-81314, r=estebank
Highlight identifier span instead of whole pattern span in `unused` lint Fixes rust-lang#81314 This pretty much just changes the span highlighted in the lint from `pat_sp` to `ident.span`. There's however an exception, which is in patterns with shorthands like `Point { y, ref mut x }`, where a suggestion to change just `x` would be invalid; in those cases I had to keep the pattern span. Another option would be suggesting something like `Point { y, x: ref mut _x }`. I also added a new test since there weren't any test that checked the `unused` lint with optional patterns.
2 parents 5a82251 + 3c63f67 commit d259d1d

8 files changed

+127
-59
lines changed

‎compiler/rustc_passes/src/liveness.rs

Lines changed: 66 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -1494,12 +1494,13 @@ impl<'tcx> Liveness<'_, 'tcx> {
14941494
// bindings, and we also consider the first pattern to be the "authoritative" set of ids.
14951495
// However, we should take the ids and spans of variables with the same name from the later
14961496
// patterns so the suggestions to prefix with underscores will apply to those too.
1497-
let mut vars: FxIndexMap<Symbol, (LiveNode, Variable, Vec<(HirId, Span)>)> = <_>::default();
1497+
let mut vars: FxIndexMap<Symbol, (LiveNode, Variable, Vec<(HirId, Span, Span)>)> =
1498+
<_>::default();
14981499

14991500
pat.each_binding(|_, hir_id, pat_sp, ident| {
15001501
let ln = entry_ln.unwrap_or_else(|| self.live_node(hir_id, pat_sp));
15011502
let var = self.variable(hir_id, ident.span);
1502-
let id_and_sp = (hir_id, pat_sp);
1503+
let id_and_sp = (hir_id, pat_sp, ident.span);
15031504
vars.entry(self.ir.variable_name(var))
15041505
.and_modify(|(.., hir_ids_and_spans)| hir_ids_and_spans.push(id_and_sp))
15051506
.or_insert_with(|| (ln, var, vec![id_and_sp]));
@@ -1508,15 +1509,21 @@ impl<'tcx> Liveness<'_, 'tcx> {
15081509
for (_, (ln, var, hir_ids_and_spans)) in vars {
15091510
if self.used_on_entry(ln, var) {
15101511
let id = hir_ids_and_spans[0].0;
1511-
let spans = hir_ids_and_spans.into_iter().map(|(_, sp)| sp).collect();
1512+
let spans =
1513+
hir_ids_and_spans.into_iter().map(|(_, _, ident_span)| ident_span).collect();
15121514
on_used_on_entry(spans, id, ln, var);
15131515
} else {
15141516
self.report_unused(hir_ids_and_spans, ln, var);
15151517
}
15161518
}
15171519
}
15181520

1519-
fn report_unused(&self, hir_ids_and_spans: Vec<(HirId, Span)>, ln: LiveNode, var: Variable) {
1521+
fn report_unused(
1522+
&self,
1523+
hir_ids_and_spans: Vec<(HirId, Span, Span)>,
1524+
ln: LiveNode,
1525+
var: Variable,
1526+
) {
15201527
let first_hir_id = hir_ids_and_spans[0].0;
15211528

15221529
if let Some(name) = self.should_warn(var).filter(|name| name != "self") {
@@ -1530,62 +1537,78 @@ impl<'tcx> Liveness<'_, 'tcx> {
15301537
self.ir.tcx.struct_span_lint_hir(
15311538
lint::builtin::UNUSED_VARIABLES,
15321539
first_hir_id,
1533-
hir_ids_and_spans.into_iter().map(|(_, sp)| sp).collect::<Vec<_>>(),
1540+
hir_ids_and_spans
1541+
.into_iter()
1542+
.map(|(_, _, ident_span)| ident_span)
1543+
.collect::<Vec<_>>(),
15341544
|lint| {
15351545
lint.build(&format!("variable `{}` is assigned to, but never used", name))
15361546
.note(&format!("consider using `_{}` instead", name))
15371547
.emit();
15381548
},
15391549
)
15401550
} else {
1541-
self.ir.tcx.struct_span_lint_hir(
1542-
lint::builtin::UNUSED_VARIABLES,
1543-
first_hir_id,
1544-
hir_ids_and_spans.iter().map(|(_, sp)| *sp).collect::<Vec<_>>(),
1545-
|lint| {
1546-
let mut err = lint.build(&format!("unused variable: `{}`", name));
1547-
1548-
let (shorthands, non_shorthands): (Vec<_>, Vec<_>) =
1549-
hir_ids_and_spans.into_iter().partition(|(hir_id, span)| {
1550-
let var = self.variable(*hir_id, *span);
1551-
self.ir.variable_is_shorthand(var)
1552-
});
1553-
1554-
let mut shorthands = shorthands
1555-
.into_iter()
1556-
.map(|(_, span)| (span, format!("{}: _", name)))
1557-
.collect::<Vec<_>>();
1558-
1559-
// If we have both shorthand and non-shorthand, prefer the "try ignoring
1560-
// the field" message, and suggest `_` for the non-shorthands. If we only
1561-
// have non-shorthand, then prefix with an underscore instead.
1562-
if !shorthands.is_empty() {
1563-
shorthands.extend(
1564-
non_shorthands
1565-
.into_iter()
1566-
.map(|(_, span)| (span, "_".to_string()))
1567-
.collect::<Vec<_>>(),
1568-
);
1551+
let (shorthands, non_shorthands): (Vec<_>, Vec<_>) =
1552+
hir_ids_and_spans.iter().copied().partition(|(hir_id, _, ident_span)| {
1553+
let var = self.variable(*hir_id, *ident_span);
1554+
self.ir.variable_is_shorthand(var)
1555+
});
15691556

1557+
// If we have both shorthand and non-shorthand, prefer the "try ignoring
1558+
// the field" message, and suggest `_` for the non-shorthands. If we only
1559+
// have non-shorthand, then prefix with an underscore instead.
1560+
if !shorthands.is_empty() {
1561+
let shorthands = shorthands
1562+
.into_iter()
1563+
.map(|(_, pat_span, _)| (pat_span, format!("{}: _", name)))
1564+
.chain(
1565+
non_shorthands
1566+
.into_iter()
1567+
.map(|(_, pat_span, _)| (pat_span, "_".to_string())),
1568+
)
1569+
.collect::<Vec<_>>();
1570+
1571+
self.ir.tcx.struct_span_lint_hir(
1572+
lint::builtin::UNUSED_VARIABLES,
1573+
first_hir_id,
1574+
hir_ids_and_spans
1575+
.iter()
1576+
.map(|(_, pat_span, _)| *pat_span)
1577+
.collect::<Vec<_>>(),
1578+
|lint| {
1579+
let mut err = lint.build(&format!("unused variable: `{}`", name));
15701580
err.multipart_suggestion(
15711581
"try ignoring the field",
15721582
shorthands,
15731583
Applicability::MachineApplicable,
15741584
);
1575-
} else {
1585+
err.emit()
1586+
},
1587+
);
1588+
} else {
1589+
let non_shorthands = non_shorthands
1590+
.into_iter()
1591+
.map(|(_, _, ident_span)| (ident_span, format!("_{}", name)))
1592+
.collect::<Vec<_>>();
1593+
1594+
self.ir.tcx.struct_span_lint_hir(
1595+
lint::builtin::UNUSED_VARIABLES,
1596+
first_hir_id,
1597+
hir_ids_and_spans
1598+
.iter()
1599+
.map(|(_, _, ident_span)| *ident_span)
1600+
.collect::<Vec<_>>(),
1601+
|lint| {
1602+
let mut err = lint.build(&format!("unused variable: `{}`", name));
15761603
err.multipart_suggestion(
15771604
"if this is intentional, prefix it with an underscore",
1578-
non_shorthands
1579-
.into_iter()
1580-
.map(|(_, span)| (span, format!("_{}", name)))
1581-
.collect::<Vec<_>>(),
1605+
non_shorthands,
15821606
Applicability::MachineApplicable,
15831607
);
1584-
}
1585-
1586-
err.emit()
1587-
},
1588-
);
1608+
err.emit()
1609+
},
1610+
);
1611+
}
15891612
}
15901613
}
15911614
}

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,16 +12,16 @@ LL | #![warn(unused)] // UI tests pass `-A unused` (#43896)
1212
= note: `#[warn(unused_variables)]` implied by `#[warn(unused)]`
1313

1414
warning: unused variable: `mut_unused_var`
15-
--> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:33:9
15+
--> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:33:13
1616
|
1717
LL | let mut mut_unused_var = 1;
18-
| ^^^^^^^^^^^^^^^^^^ help: if this is intentional, prefix it with an underscore: `_mut_unused_var`
18+
| ^^^^^^^^^^^^^^ help: if this is intentional, prefix it with an underscore: `_mut_unused_var`
1919

2020
warning: unused variable: `var`
21-
--> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:37:10
21+
--> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:37:14
2222
|
2323
LL | let (mut var, unused_var) = (1, 2);
24-
| ^^^^^^^ help: if this is intentional, prefix it with an underscore: `_var`
24+
| ^^^ help: if this is intentional, prefix it with an underscore: `_var`
2525

2626
warning: unused variable: `unused_var`
2727
--> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:37:19
@@ -36,10 +36,10 @@ LL | if let SoulHistory { corridors_of_light,
3636
| ^^^^^^^^^^^^^^^^^^ help: try ignoring the field: `corridors_of_light: _`
3737

3838
warning: variable `hours_are_suns` is assigned to, but never used
39-
--> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:46:26
39+
--> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:46:30
4040
|
4141
LL | mut hours_are_suns,
42-
| ^^^^^^^^^^^^^^^^^^
42+
| ^^^^^^^^^^^^^^
4343
|
4444
= note: consider using `_hours_are_suns` instead
4545

‎src/test/ui/lint/issue-81314-unused-span-ident.fixed

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
// run-rustfix
2+
// Regression test for #81314: Unused variable lint should
3+
// span only the identifier and not the rest of the pattern
4+
5+
#![deny(unused)]
6+
7+
fn main() {
8+
let [_rest @ ..] = [1, 2, 3]; //~ ERROR unused variable
9+
}
10+
11+
pub fn foo([_rest @ ..]: &[i32]) { //~ ERROR unused variable
12+
}

‎src/test/ui/lint/issue-81314-unused-span-ident.rs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
// run-rustfix
2+
// Regression test for #81314: Unused variable lint should
3+
// span only the identifier and not the rest of the pattern
4+
5+
#![deny(unused)]
6+
7+
fn main() {
8+
let [rest @ ..] = [1, 2, 3]; //~ ERROR unused variable
9+
}
10+
11+
pub fn foo([rest @ ..]: &[i32]) { //~ ERROR unused variable
12+
}

‎src/test/ui/lint/issue-81314-unused-span-ident.stderr

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
error: unused variable: `rest`
2+
--> $DIR/issue-81314-unused-span-ident.rs:8:10
3+
|
4+
LL | let [rest @ ..] = [1, 2, 3];
5+
| ^^^^ help: if this is intentional, prefix it with an underscore: `_rest`
6+
|
7+
note: the lint level is defined here
8+
--> $DIR/issue-81314-unused-span-ident.rs:5:9
9+
|
10+
LL | #![deny(unused)]
11+
| ^^^^^^
12+
= note: `#[deny(unused_variables)]` implied by `#[deny(unused)]`
13+
14+
error: unused variable: `rest`
15+
--> $DIR/issue-81314-unused-span-ident.rs:11:13
16+
|
17+
LL | pub fn foo([rest @ ..]: &[i32]) {
18+
| ^^^^ help: if this is intentional, prefix it with an underscore: `_rest`
19+
20+
error: aborting due to 2 previous errors
21+

‎src/test/ui/liveness/liveness-consts.stderr

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
warning: variable `a` is assigned to, but never used
2-
--> $DIR/liveness-consts.rs:7:9
2+
--> $DIR/liveness-consts.rs:7:13
33
|
44
LL | let mut a = 0;
5-
| ^^^^^
5+
| ^
66
|
77
note: the lint level is defined here
88
--> $DIR/liveness-consts.rs:2:9

‎src/test/ui/liveness/liveness-dead.stderr

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
error: value assigned to `x` is never read
2-
--> $DIR/liveness-dead.rs:9:9
2+
--> $DIR/liveness-dead.rs:9:13
33
|
44
LL | let mut x: isize = 3;
5-
| ^^^^^
5+
| ^
66
|
77
note: the lint level is defined here
88
--> $DIR/liveness-dead.rs:2:9
@@ -20,10 +20,10 @@ LL | x = 4;
2020
= help: maybe it is overwritten before being read?
2121

2222
error: value passed to `x` is never read
23-
--> $DIR/liveness-dead.rs:20:7
23+
--> $DIR/liveness-dead.rs:20:11
2424
|
2525
LL | fn f4(mut x: i32) {
26-
| ^^^^^
26+
| ^
2727
|
2828
= help: maybe it is overwritten before being read?
2929

‎src/test/ui/liveness/liveness-unused.stderr

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -44,10 +44,10 @@ LL | let x = 3;
4444
| ^ help: if this is intentional, prefix it with an underscore: `_x`
4545

4646
error: variable `x` is assigned to, but never used
47-
--> $DIR/liveness-unused.rs:30:9
47+
--> $DIR/liveness-unused.rs:30:13
4848
|
4949
LL | let mut x = 3;
50-
| ^^^^^
50+
| ^
5151
|
5252
= note: consider using `_x` instead
5353

@@ -65,10 +65,10 @@ LL | #![deny(unused_assignments)]
6565
= help: maybe it is overwritten before being read?
6666

6767
error: variable `z` is assigned to, but never used
68-
--> $DIR/liveness-unused.rs:37:9
68+
--> $DIR/liveness-unused.rs:37:13
6969
|
7070
LL | let mut z = 3;
71-
| ^^^^^
71+
| ^
7272
|
7373
= note: consider using `_z` instead
7474

0 commit comments

Comments
 (0)