Try to fix warning for unused variables in or patterns, issue #67691

8 files changed, 140 insertions, 57 deletions

diff --git a/src/librustc_passes/liveness.rs b/src/librustc_passes/liveness.rs
index ee71d09cb21..74a2d997896 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,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::<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<_>>();
+
+                        let non_shorthands = non_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.
+                        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 cc675a709a2..413a51d4e5d 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.fixed b/src/test/ui/lint/issue-67691-unused-field-in-or-pattern.fixed
index 7ec0b33f919..cfe37606202 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<MyEnum>) {
     }
 }
 
+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 d43c637767a..4b19f85dfa4 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<MyEnum>) {
     }
 }
 
+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 bda53c67331..4e9d02abacd 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 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 42187330a3e..1ea84614393 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