diff options
7 files changed, 229 insertions, 52 deletions
diff --git a/src/librustc_passes/liveness.rs b/src/librustc_passes/liveness.rs index ee71d09cb21..24f6d1a9c58 100644 --- a/src/librustc_passes/liveness.rs +++ b/src/librustc_passes/liveness.rs @@ -1492,28 +1492,33 @@ impl<'tcx> Liveness<'_, 'tcx> { ) { // In an or-pattern, only consider the variable; 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 + // However, we should take the ids and 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: FxIndexMap<String, (LiveNode, Variable, HirId, Vec<Span>)> = <_>::default(); + let mut vars: FxIndexMap<String, (LiveNode, Variable, Vec<(HirId, Span)>)> = <_>::default(); pat.each_binding(|_, hir_id, pat_sp, ident| { let ln = entry_ln.unwrap_or_else(|| self.live_node(hir_id, pat_sp)); let var = self.variable(hir_id, ident.span); + let id_and_sp = (hir_id, pat_sp); vars.entry(self.ir.variable_name(var)) - .and_modify(|(.., spans)| spans.push(ident.span)) - .or_insert_with(|| (ln, var, hir_id, vec![ident.span])); + .and_modify(|(.., hir_ids_and_spans)| hir_ids_and_spans.push(id_and_sp)) + .or_insert_with(|| (ln, var, vec![id_and_sp])); }); - for (_, (ln, var, id, spans)) in vars { + for (_, (ln, var, hir_ids_and_spans)) in vars { if self.used_on_entry(ln, var) { + let id = hir_ids_and_spans[0].0; + let spans = hir_ids_and_spans.into_iter().map(|(_, sp)| sp).collect(); on_used_on_entry(spans, id, ln, var); } else { - self.report_unused(spans, id, ln, var); + self.report_unused(hir_ids_and_spans, ln, var); } } } - fn report_unused(&self, spans: Vec<Span>, hir_id: HirId, ln: LiveNode, var: Variable) { + fn report_unused(&self, hir_ids_and_spans: Vec<(HirId, Span)>, ln: LiveNode, var: Variable) { + let first_hir_id = hir_ids_and_spans[0].0; + if let Some(name) = self.should_warn(var).filter(|name| name != "self") { // annoying: for parameters in funcs like `fn(x: i32) // {ret}`, there is only one node, so asking about @@ -1524,8 +1529,8 @@ impl<'tcx> Liveness<'_, 'tcx> { if is_assigned { self.ir.tcx.struct_span_lint_hir( lint::builtin::UNUSED_VARIABLES, - hir_id, - spans, + first_hir_id, + hir_ids_and_spans.into_iter().map(|(_, sp)| sp).collect::<Vec<_>>(), |lint| { lint.build(&format!("variable `{}` is assigned to, but never used", name)) .note(&format!("consider using `_{}` instead", name)) @@ -1535,31 +1540,49 @@ impl<'tcx> Liveness<'_, 'tcx> { } else { self.ir.tcx.struct_span_lint_hir( lint::builtin::UNUSED_VARIABLES, - hir_id, - spans.clone(), + first_hir_id, + hir_ids_and_spans.iter().map(|(_, sp)| *sp).collect::<Vec<_>>(), |lint| { let mut err = lint.build(&format!("unused variable: `{}`", name)); - if self.ir.variable_is_shorthand(var) { - if let Node::Binding(pat) = self.ir.tcx.hir().get(hir_id) { - // Handle `ref` and `ref mut`. - let spans = spans - .iter() - .map(|_span| (pat.span, format!("{}: _", name))) - .collect(); - - err.multipart_suggestion( - "try ignoring the field", - spans, - Applicability::MachineApplicable, - ); - } + + let (shorthands, non_shorthands): (Vec<_>, Vec<_>) = + hir_ids_and_spans.into_iter().partition(|(hir_id, span)| { + let var = self.variable(*hir_id, *span); + self.ir.variable_is_shorthand(var) + }); + + let mut shorthands = shorthands + .into_iter() + .map(|(_, span)| (span, format!("{}: _", name))) + .collect::<Vec<_>>(); + + // If we have both shorthand and non-shorthand, prefer the "try ignoring + // the field" message, and suggest `_` for the non-shorthands. If we only + // have non-shorthand, then prefix with an underscore instead. + if !shorthands.is_empty() { + shorthands.extend( + non_shorthands + .into_iter() + .map(|(_, span)| (span, "_".to_string())) + .collect::<Vec<_>>(), + ); + + err.multipart_suggestion( + "try ignoring the field", + shorthands, + Applicability::MachineApplicable, + ); } else { err.multipart_suggestion( "if this is intentional, prefix it with an underscore", - spans.iter().map(|span| (*span, format!("_{}", name))).collect(), + non_shorthands + .into_iter() + .map(|(_, span)| (span, format!("_{}", name))) + .collect::<Vec<_>>(), Applicability::MachineApplicable, ); } + err.emit() }, ); 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 2ef655efdbd..3efd87f6a5f 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 @@ -12,16 +12,16 @@ LL | #![warn(unused)] // UI tests pass `-A unused` (#43896) = note: `#[warn(unused_variables)]` implied by `#[warn(unused)]` warning: unused variable: `mut_unused_var` - --> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:33:13 + --> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:33:9 | LL | let mut mut_unused_var = 1; - | ^^^^^^^^^^^^^^ help: if this is intentional, prefix it with an underscore: `_mut_unused_var` + | ^^^^^^^^^^^^^^^^^^ help: if this is intentional, prefix it with an underscore: `_mut_unused_var` warning: unused variable: `var` - --> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:37:14 + --> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:37:10 | LL | let (mut var, unused_var) = (1, 2); - | ^^^ help: if this is intentional, prefix it with an underscore: `_var` + | ^^^^^^^ help: if this is intentional, prefix it with an underscore: `_var` warning: unused variable: `unused_var` --> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:37:19 @@ -36,10 +36,10 @@ LL | 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:46:30 + --> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:46:26 | LL | mut hours_are_suns, - | ^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^ | = note: consider using `_hours_are_suns` instead diff --git a/src/test/ui/lint/issue-54180-unused-ref-field.stderr b/src/test/ui/lint/issue-54180-unused-ref-field.stderr index 840ecc0ce09..c501aa25f13 100644 --- a/src/test/ui/lint/issue-54180-unused-ref-field.stderr +++ b/src/test/ui/lint/issue-54180-unused-ref-field.stderr @@ -1,10 +1,8 @@ error: unused variable: `field` - --> $DIR/issue-54180-unused-ref-field.rs:20:26 + --> $DIR/issue-54180-unused-ref-field.rs:20:22 | LL | E::Variant { ref field } => (), - | ----^^^^^ - | | - | help: try ignoring the field: `field: _` + | ^^^^^^^^^ help: try ignoring the field: `field: _` | note: the lint level is defined here --> $DIR/issue-54180-unused-ref-field.rs:3:9 @@ -20,20 +18,16 @@ LL | let _: i32 = points.iter().map(|Point { x, y }| y).sum(); | ^ help: try ignoring the field: `x: _` error: unused variable: `f1` - --> $DIR/issue-54180-unused-ref-field.rs:26:17 + --> $DIR/issue-54180-unused-ref-field.rs:26:13 | LL | let S { ref f1 } = s; - | ----^^ - | | - | help: try ignoring the field: `f1: _` + | ^^^^^^ help: try ignoring the field: `f1: _` error: unused variable: `x` - --> $DIR/issue-54180-unused-ref-field.rs:32:28 + --> $DIR/issue-54180-unused-ref-field.rs:32:20 | LL | Point { y, ref mut x } => y, - | --------^ - | | - | help: try ignoring the field: `x: _` + | ^^^^^^^^^ help: try ignoring the field: `x: _` error: aborting due to 4 previous errors diff --git a/src/test/ui/lint/issue-67691-unused-field-in-or-pattern.rs b/src/test/ui/lint/issue-67691-unused-field-in-or-pattern.rs new file mode 100644 index 00000000000..b21f1ef6b26 --- /dev/null +++ b/src/test/ui/lint/issue-67691-unused-field-in-or-pattern.rs @@ -0,0 +1,86 @@ +// FIXME: should be run-rustfix, but rustfix doesn't currently support multipart suggestions, see +// #53934 + +#![feature(or_patterns)] +#![deny(unused)] + +pub enum MyEnum { + A { i: i32, j: i32 }, + B { i: i32, j: i32 }, +} + +pub enum MixedEnum { + A { i: i32 }, + B(i32), +} + +pub fn no_ref(x: MyEnum) { + use MyEnum::*; + + match x { + A { i, j } | B { i, j } => { //~ ERROR unused variable + println!("{}", i); + } + } +} + +pub fn with_ref(x: MyEnum) { + use MyEnum::*; + + match x { + A { i, ref j } | B { i, ref j } => { //~ ERROR unused variable + println!("{}", i); + } + } +} + +pub fn inner_no_ref(x: Option<MyEnum>) { + use MyEnum::*; + + match x { + Some(A { i, j } | B { i, j }) => { //~ ERROR unused variable + println!("{}", i); + } + + _ => {} + } +} + +pub fn inner_with_ref(x: Option<MyEnum>) { + use MyEnum::*; + + match x { + Some(A { i, ref j } | B { i, ref j }) => { //~ ERROR unused variable + println!("{}", i); + } + + _ => {} + } +} + +pub fn mixed_no_ref(x: MixedEnum) { + match x { + MixedEnum::A { i } | MixedEnum::B(i) => { //~ ERROR unused variable + println!("match"); + } + } +} + +pub fn mixed_with_ref(x: MixedEnum) { + match x { + MixedEnum::A { ref i } | MixedEnum::B(ref i) => { //~ ERROR unused variable + println!("match"); + } + } +} + +pub fn main() { + no_ref(MyEnum::A { i: 1, j: 2 }); + with_ref(MyEnum::A { i: 1, j: 2 }); + + inner_no_ref(Some(MyEnum::A { i: 1, j: 2 })); + inner_with_ref(Some(MyEnum::A { i: 1, j: 2 })); + + mixed_no_ref(MixedEnum::B(5)); + mixed_with_ref(MixedEnum::B(5)); +} diff --git a/src/test/ui/lint/issue-67691-unused-field-in-or-pattern.stderr b/src/test/ui/lint/issue-67691-unused-field-in-or-pattern.stderr new file mode 100644 index 00000000000..9cff2900908 --- /dev/null +++ b/src/test/ui/lint/issue-67691-unused-field-in-or-pattern.stderr @@ -0,0 +1,74 @@ +error: unused variable: `j` + --> $DIR/issue-67691-unused-field-in-or-pattern.rs:21:16 + | +LL | A { i, j } | B { i, j } => { + | ^ ^ + | +note: the lint level is defined here + --> $DIR/issue-67691-unused-field-in-or-pattern.rs:5:9 + | +LL | #![deny(unused)] + | ^^^^^^ + = note: `#[deny(unused_variables)]` implied by `#[deny(unused)]` +help: try ignoring the field + | +LL | A { i, j: _ } | B { i, j: _ } => { + | ^^^^ ^^^^ + +error: unused variable: `j` + --> $DIR/issue-67691-unused-field-in-or-pattern.rs:31:16 + | +LL | A { i, ref j } | B { i, ref j } => { + | ^^^^^ ^^^^^ + | +help: try ignoring the field + | +LL | A { i, j: _ } | B { i, j: _ } => { + | ^^^^ ^^^^ + +error: unused variable: `j` + --> $DIR/issue-67691-unused-field-in-or-pattern.rs:41:21 + | +LL | Some(A { i, j } | B { i, j }) => { + | ^ ^ + | +help: try ignoring the field + | +LL | Some(A { i, j: _ } | B { i, j: _ }) => { + | ^^^^ ^^^^ + +error: unused variable: `j` + --> $DIR/issue-67691-unused-field-in-or-pattern.rs:53:21 + | +LL | Some(A { i, ref j } | B { i, ref j }) => { + | ^^^^^ ^^^^^ + | +help: try ignoring the field + | +LL | Some(A { i, j: _ } | B { i, j: _ }) => { + | ^^^^ ^^^^ + +error: unused variable: `i` + --> $DIR/issue-67691-unused-field-in-or-pattern.rs:63:24 + | +LL | MixedEnum::A { i } | MixedEnum::B(i) => { + | ^ ^ + | +help: try ignoring the field + | +LL | MixedEnum::A { i: _ } | MixedEnum::B(_) => { + | ^^^^ ^ + +error: unused variable: `i` + --> $DIR/issue-67691-unused-field-in-or-pattern.rs:71:24 + | +LL | MixedEnum::A { ref i } | MixedEnum::B(ref i) => { + | ^^^^^ ^^^^^ + | +help: try ignoring the field + | +LL | MixedEnum::A { i: _ } | MixedEnum::B(_) => { + | ^^^^ ^ + +error: aborting due to 6 previous errors + diff --git a/src/test/ui/liveness/liveness-dead.stderr b/src/test/ui/liveness/liveness-dead.stderr index 12680ab1156..e9d20cf981f 100644 --- a/src/test/ui/liveness/liveness-dead.stderr +++ b/src/test/ui/liveness/liveness-dead.stderr @@ -1,8 +1,8 @@ error: value assigned to `x` is never read - --> $DIR/liveness-dead.rs:9:13 + --> $DIR/liveness-dead.rs:9:9 | LL | let mut x: isize = 3; - | ^ + | ^^^^^ | note: the lint level is defined here --> $DIR/liveness-dead.rs:2:9 @@ -20,10 +20,10 @@ LL | x = 4; = help: maybe it is overwritten before being read? error: value passed to `x` is never read - --> $DIR/liveness-dead.rs:20:11 + --> $DIR/liveness-dead.rs:20:7 | LL | fn f4(mut x: i32) { - | ^ + | ^^^^^ | = help: maybe it is overwritten before being read? diff --git a/src/test/ui/liveness/liveness-unused.stderr b/src/test/ui/liveness/liveness-unused.stderr index 4a6122681a9..2c5550ac47f 100644 --- a/src/test/ui/liveness/liveness-unused.stderr +++ b/src/test/ui/liveness/liveness-unused.stderr @@ -44,10 +44,10 @@ LL | let x = 3; | ^ help: if this is intentional, prefix it with an underscore: `_x` error: variable `x` is assigned to, but never used - --> $DIR/liveness-unused.rs:30:13 + --> $DIR/liveness-unused.rs:30:9 | LL | let mut x = 3; - | ^ + | ^^^^^ | = note: consider using `_x` instead @@ -65,10 +65,10 @@ LL | #![deny(unused_assignments)] = help: maybe it is overwritten before being read? error: variable `z` is assigned to, but never used - --> $DIR/liveness-unused.rs:37:13 + --> $DIR/liveness-unused.rs:37:9 | LL | let mut z = 3; - | ^ + | ^^^^^ | = note: consider using `_z` instead |