From 0c156af20d74b2e23c2b862110a5ed5fa8d65a51 Mon Sep 17 00:00:00 2001 From: sapir Date: Wed, 1 Jan 2020 01:51:37 +0200 Subject: [PATCH 1/4] Add tests for issue #67691 --- ...sue-67691-unused-field-in-or-pattern.fixed | 61 +++++++++++++++++++ .../issue-67691-unused-field-in-or-pattern.rs | 61 +++++++++++++++++++ ...ue-67691-unused-field-in-or-pattern.stderr | 52 ++++++++++++++++ 3 files changed, 174 insertions(+) create mode 100644 src/test/ui/lint/issue-67691-unused-field-in-or-pattern.fixed create mode 100644 src/test/ui/lint/issue-67691-unused-field-in-or-pattern.rs create mode 100644 src/test/ui/lint/issue-67691-unused-field-in-or-pattern.stderr diff --git a/src/test/ui/lint/issue-67691-unused-field-in-or-pattern.fixed b/src/test/ui/lint/issue-67691-unused-field-in-or-pattern.fixed new file mode 100644 index 0000000000000..7ec0b33f919b7 --- /dev/null +++ b/src/test/ui/lint/issue-67691-unused-field-in-or-pattern.fixed @@ -0,0 +1,61 @@ +// run-rustfix + +#![feature(or_patterns)] +#![deny(unused)] + +pub enum MyEnum { + A { i: i32, j: i32 }, + B { i: i32, j: 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, j: _ } | B { i, j: _ } => { //~ ERROR unused variable + println!("{}", i); + } + } +} + +pub fn inner_no_ref(x: Option) { + use MyEnum::*; + + match x { + Some(A { i, j: _ } | B { i, j: _ }) => { //~ ERROR unused variable + println!("{}", i); + } + + _ => {} + } +} + +pub fn inner_with_ref(x: Option) { + use MyEnum::*; + + match x { + Some(A { i, j: _ } | B { i, j: _ }) => { //~ ERROR unused variable + println!("{}", i); + } + + _ => {} + } +} + +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 })); +} 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 0000000000000..d43c637767ad9 --- /dev/null +++ b/src/test/ui/lint/issue-67691-unused-field-in-or-pattern.rs @@ -0,0 +1,61 @@ +// run-rustfix + +#![feature(or_patterns)] +#![deny(unused)] + +pub enum MyEnum { + A { i: i32, j: i32 }, + B { i: i32, j: 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) { + use MyEnum::*; + + match x { + Some(A { i, j } | B { i, j }) => { //~ ERROR unused variable + println!("{}", i); + } + + _ => {} + } +} + +pub fn inner_with_ref(x: Option) { + use MyEnum::*; + + match x { + Some(A { i, ref j } | B { i, ref j }) => { //~ ERROR unused variable + println!("{}", i); + } + + _ => {} + } +} + +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 })); +} 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 0000000000000..bda53c6733183 --- /dev/null +++ b/src/test/ui/lint/issue-67691-unused-field-in-or-pattern.stderr @@ -0,0 +1,52 @@ +error: unused variable: `j` + --> $DIR/issue-67691-unused-field-in-or-pattern.rs:15:16 + | +LL | A { i, j } | B { i, j } => { + | ^ ^ + | +note: the lint level is defined here + --> $DIR/issue-67691-unused-field-in-or-pattern.rs:4: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:25: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:35: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:47: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: aborting due to 5 previous errors + From e22e443208b4e6e1f0bf430877b19ecbf7a3d0ca Mon Sep 17 00:00:00 2001 From: sapir Date: Wed, 1 Jan 2020 01:20:34 +0200 Subject: [PATCH 2/4] Try to fix warning for unused variables in or patterns, issue #67691 --- src/librustc_passes/liveness.rs | 71 ++++++++++++------- ...0-unused-variable-in-struct-pattern.stderr | 12 ++-- .../lint/issue-54180-unused-ref-field.stderr | 18 ++--- ...sue-67691-unused-field-in-or-pattern.fixed | 24 +++++++ .../issue-67691-unused-field-in-or-pattern.rs | 24 +++++++ ...ue-67691-unused-field-in-or-pattern.stderr | 32 +++++++-- src/test/ui/liveness/liveness-dead.stderr | 8 +-- src/test/ui/liveness/liveness-unused.stderr | 8 +-- 8 files changed, 140 insertions(+), 57 deletions(-) diff --git a/src/librustc_passes/liveness.rs b/src/librustc_passes/liveness.rs index ee71d09cb21e9..74a2d99789680 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)> = <_>::default(); + let mut vars: FxIndexMap)> = <_>::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, 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::>(), |lint| { lint.build(&format!("variable `{}` is assigned to, but never used", name)) .note(&format!("consider using `_{}` instead", name)) @@ -1535,31 +1540,45 @@ 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::>(), |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::>(); + + let non_shorthands = non_shorthands + .into_iter() + .map(|(_, span)| (span, format!("_{}", name))) + .collect::>(); + + // If we have both shorthand and non-shorthand, prefer the "try ignoring + // the field" message. + if !shorthands.is_empty() { + shorthands.extend(non_shorthands); + + 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, 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 cc675a709a2c6..413a51d4e5dd7 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 840ecc0ce09ee..c501aa25f1352 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.fixed b/src/test/ui/lint/issue-67691-unused-field-in-or-pattern.fixed index 7ec0b33f919b7..cfe3760620205 100644 --- a/src/test/ui/lint/issue-67691-unused-field-in-or-pattern.fixed +++ b/src/test/ui/lint/issue-67691-unused-field-in-or-pattern.fixed @@ -8,6 +8,11 @@ pub enum MyEnum { B { i: i32, j: i32 }, } +pub enum MixedEnum { + A { i: i32 }, + B(i32), +} + pub fn no_ref(x: MyEnum) { use MyEnum::*; @@ -52,10 +57,29 @@ pub fn inner_with_ref(x: Option) { } } +pub fn mixed_no_ref(x: MixedEnum) { + match x { + MixedEnum::A { i: _ } | MixedEnum::B(_i) => { + println!("match"); + } + } +} + +pub fn mixed_with_ref(x: MixedEnum) { + match x { + MixedEnum::A { i: _ } | MixedEnum::B(_i) => { + 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.rs b/src/test/ui/lint/issue-67691-unused-field-in-or-pattern.rs index d43c637767ad9..4b19f85dfa477 100644 --- 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 @@ -8,6 +8,11 @@ pub enum MyEnum { B { i: i32, j: i32 }, } +pub enum MixedEnum { + A { i: i32 }, + B(i32), +} + pub fn no_ref(x: MyEnum) { use MyEnum::*; @@ -52,10 +57,29 @@ pub fn inner_with_ref(x: Option) { } } +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 index bda53c6733183..4e9d02abacd71 100644 --- 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 @@ -1,5 +1,5 @@ error: unused variable: `j` - --> $DIR/issue-67691-unused-field-in-or-pattern.rs:15:16 + --> $DIR/issue-67691-unused-field-in-or-pattern.rs:20:16 | LL | A { i, j } | B { i, j } => { | ^ ^ @@ -16,7 +16,7 @@ LL | A { i, j: _ } | B { i, j: _ } => { | ^^^^ ^^^^ error: unused variable: `j` - --> $DIR/issue-67691-unused-field-in-or-pattern.rs:25:16 + --> $DIR/issue-67691-unused-field-in-or-pattern.rs:30:16 | LL | A { i, ref j } | B { i, ref j } => { | ^^^^^ ^^^^^ @@ -27,7 +27,7 @@ LL | A { i, j: _ } | B { i, j: _ } => { | ^^^^ ^^^^ error: unused variable: `j` - --> $DIR/issue-67691-unused-field-in-or-pattern.rs:35:21 + --> $DIR/issue-67691-unused-field-in-or-pattern.rs:40:21 | LL | Some(A { i, j } | B { i, j }) => { | ^ ^ @@ -38,7 +38,7 @@ LL | Some(A { i, j: _ } | B { i, j: _ }) => { | ^^^^ ^^^^ error: unused variable: `j` - --> $DIR/issue-67691-unused-field-in-or-pattern.rs:47:21 + --> $DIR/issue-67691-unused-field-in-or-pattern.rs:52:21 | LL | Some(A { i, ref j } | B { i, ref j }) => { | ^^^^^ ^^^^^ @@ -48,5 +48,27 @@ help: try ignoring the field LL | Some(A { i, j: _ } | B { i, j: _ }) => { | ^^^^ ^^^^ -error: aborting due to 5 previous errors +error: unused variable: `i` + --> $DIR/issue-67691-unused-field-in-or-pattern.rs:62:24 + | +LL | MixedEnum::A { i } | MixedEnum::B(i) => { + | ^ ^ + | +help: try ignoring the field + | +LL | MixedEnum::A { i: _ } | MixedEnum::B(_i) => { + | ^^^^ ^^ + +error: unused variable: `i` + --> $DIR/issue-67691-unused-field-in-or-pattern.rs:70:24 + | +LL | MixedEnum::A { ref i } | MixedEnum::B(ref i) => { + | ^^^^^ ^^^^^ + | +help: try ignoring the field + | +LL | MixedEnum::A { i: _ } | MixedEnum::B(_i) => { + | ^^^^ ^^ + +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 12680ab11568f..e9d20cf981fbd 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 42187330a3eb1..1ea8461439320 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 From a8e3d0b71e2b79b0ce880541e5a0f0fe303a1a8d Mon Sep 17 00:00:00 2001 From: sapir Date: Fri, 6 Mar 2020 10:03:34 +0200 Subject: [PATCH 3/4] Replace non-shorthand variables with _, not _var --- src/librustc_passes/liveness.rs | 20 +++++++++++-------- ...sue-67691-unused-field-in-or-pattern.fixed | 4 ++-- ...ue-67691-unused-field-in-or-pattern.stderr | 8 ++++---- 3 files changed, 18 insertions(+), 14 deletions(-) diff --git a/src/librustc_passes/liveness.rs b/src/librustc_passes/liveness.rs index 74a2d99789680..24f6d1a9c5894 100644 --- a/src/librustc_passes/liveness.rs +++ b/src/librustc_passes/liveness.rs @@ -1556,15 +1556,16 @@ impl<'tcx> Liveness<'_, 'tcx> { .map(|(_, span)| (span, format!("{}: _", name))) .collect::>(); - let non_shorthands = non_shorthands - .into_iter() - .map(|(_, span)| (span, format!("_{}", name))) - .collect::>(); - // If we have both shorthand and non-shorthand, prefer the "try ignoring - // the field" message. + // 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); + shorthands.extend( + non_shorthands + .into_iter() + .map(|(_, span)| (span, "_".to_string())) + .collect::>(), + ); err.multipart_suggestion( "try ignoring the field", @@ -1574,7 +1575,10 @@ impl<'tcx> Liveness<'_, 'tcx> { } else { err.multipart_suggestion( "if this is intentional, prefix it with an underscore", - non_shorthands, + non_shorthands + .into_iter() + .map(|(_, span)| (span, format!("_{}", name))) + .collect::>(), Applicability::MachineApplicable, ); } diff --git a/src/test/ui/lint/issue-67691-unused-field-in-or-pattern.fixed b/src/test/ui/lint/issue-67691-unused-field-in-or-pattern.fixed index cfe3760620205..f842fcebe1f8f 100644 --- a/src/test/ui/lint/issue-67691-unused-field-in-or-pattern.fixed +++ b/src/test/ui/lint/issue-67691-unused-field-in-or-pattern.fixed @@ -59,7 +59,7 @@ pub fn inner_with_ref(x: Option) { pub fn mixed_no_ref(x: MixedEnum) { match x { - MixedEnum::A { i: _ } | MixedEnum::B(_i) => { + MixedEnum::A { i: _ } | MixedEnum::B(_) => { println!("match"); } } @@ -67,7 +67,7 @@ pub fn mixed_no_ref(x: MixedEnum) { pub fn mixed_with_ref(x: MixedEnum) { match x { - MixedEnum::A { i: _ } | MixedEnum::B(_i) => { + MixedEnum::A { i: _ } | MixedEnum::B(_) => { println!("match"); } } 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 index 4e9d02abacd71..8aefe243a944b 100644 --- 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 @@ -56,8 +56,8 @@ LL | MixedEnum::A { i } | MixedEnum::B(i) => { | help: try ignoring the field | -LL | MixedEnum::A { i: _ } | MixedEnum::B(_i) => { - | ^^^^ ^^ +LL | MixedEnum::A { i: _ } | MixedEnum::B(_) => { + | ^^^^ ^ error: unused variable: `i` --> $DIR/issue-67691-unused-field-in-or-pattern.rs:70:24 @@ -67,8 +67,8 @@ LL | MixedEnum::A { ref i } | MixedEnum::B(ref i) => { | help: try ignoring the field | -LL | MixedEnum::A { i: _ } | MixedEnum::B(_i) => { - | ^^^^ ^^ +LL | MixedEnum::A { i: _ } | MixedEnum::B(_) => { + | ^^^^ ^ error: aborting due to 6 previous errors From 32216383fa2f16047f1130f1b7bc6dfea0272272 Mon Sep 17 00:00:00 2001 From: sapir Date: Fri, 10 Apr 2020 03:32:49 +0300 Subject: [PATCH 4/4] Replace run-rustfix for issue 67691 test with a FIXME --- ...sue-67691-unused-field-in-or-pattern.fixed | 85 ------------------- .../issue-67691-unused-field-in-or-pattern.rs | 3 +- ...ue-67691-unused-field-in-or-pattern.stderr | 14 +-- 3 files changed, 9 insertions(+), 93 deletions(-) delete mode 100644 src/test/ui/lint/issue-67691-unused-field-in-or-pattern.fixed diff --git a/src/test/ui/lint/issue-67691-unused-field-in-or-pattern.fixed b/src/test/ui/lint/issue-67691-unused-field-in-or-pattern.fixed deleted file mode 100644 index f842fcebe1f8f..0000000000000 --- a/src/test/ui/lint/issue-67691-unused-field-in-or-pattern.fixed +++ /dev/null @@ -1,85 +0,0 @@ -// run-rustfix - -#![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, j: _ } | B { i, j: _ } => { //~ ERROR unused variable - println!("{}", i); - } - } -} - -pub fn inner_no_ref(x: Option) { - use MyEnum::*; - - match x { - Some(A { i, j: _ } | B { i, j: _ }) => { //~ ERROR unused variable - println!("{}", i); - } - - _ => {} - } -} - -pub fn inner_with_ref(x: Option) { - use MyEnum::*; - - match x { - Some(A { i, j: _ } | B { i, j: _ }) => { //~ ERROR unused variable - println!("{}", i); - } - - _ => {} - } -} - -pub fn mixed_no_ref(x: MixedEnum) { - match x { - MixedEnum::A { i: _ } | MixedEnum::B(_) => { - println!("match"); - } - } -} - -pub fn mixed_with_ref(x: MixedEnum) { - match x { - MixedEnum::A { i: _ } | MixedEnum::B(_) => { - 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.rs b/src/test/ui/lint/issue-67691-unused-field-in-or-pattern.rs index 4b19f85dfa477..b21f1ef6b2603 100644 --- 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 @@ -1,4 +1,5 @@ -// run-rustfix +// FIXME: should be run-rustfix, but rustfix doesn't currently support multipart suggestions, see +// #53934 #![feature(or_patterns)] #![deny(unused)] 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 index 8aefe243a944b..9cff2900908e6 100644 --- 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 @@ -1,11 +1,11 @@ error: unused variable: `j` - --> $DIR/issue-67691-unused-field-in-or-pattern.rs:20:16 + --> $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:4:9 + --> $DIR/issue-67691-unused-field-in-or-pattern.rs:5:9 | LL | #![deny(unused)] | ^^^^^^ @@ -16,7 +16,7 @@ LL | A { i, j: _ } | B { i, j: _ } => { | ^^^^ ^^^^ error: unused variable: `j` - --> $DIR/issue-67691-unused-field-in-or-pattern.rs:30:16 + --> $DIR/issue-67691-unused-field-in-or-pattern.rs:31:16 | LL | A { i, ref j } | B { i, ref j } => { | ^^^^^ ^^^^^ @@ -27,7 +27,7 @@ LL | A { i, j: _ } | B { i, j: _ } => { | ^^^^ ^^^^ error: unused variable: `j` - --> $DIR/issue-67691-unused-field-in-or-pattern.rs:40:21 + --> $DIR/issue-67691-unused-field-in-or-pattern.rs:41:21 | LL | Some(A { i, j } | B { i, j }) => { | ^ ^ @@ -38,7 +38,7 @@ LL | Some(A { i, j: _ } | B { i, j: _ }) => { | ^^^^ ^^^^ error: unused variable: `j` - --> $DIR/issue-67691-unused-field-in-or-pattern.rs:52:21 + --> $DIR/issue-67691-unused-field-in-or-pattern.rs:53:21 | LL | Some(A { i, ref j } | B { i, ref j }) => { | ^^^^^ ^^^^^ @@ -49,7 +49,7 @@ LL | Some(A { i, j: _ } | B { i, j: _ }) => { | ^^^^ ^^^^ error: unused variable: `i` - --> $DIR/issue-67691-unused-field-in-or-pattern.rs:62:24 + --> $DIR/issue-67691-unused-field-in-or-pattern.rs:63:24 | LL | MixedEnum::A { i } | MixedEnum::B(i) => { | ^ ^ @@ -60,7 +60,7 @@ LL | MixedEnum::A { i: _ } | MixedEnum::B(_) => { | ^^^^ ^ error: unused variable: `i` - --> $DIR/issue-67691-unused-field-in-or-pattern.rs:70:24 + --> $DIR/issue-67691-unused-field-in-or-pattern.rs:71:24 | LL | MixedEnum::A { ref i } | MixedEnum::B(ref i) => { | ^^^^^ ^^^^^