Rollup merge of #82655 - SkiFire13:fix-issue-81314, r=estebank

Highlight identifier span instead of whole pattern span in `unused` lint

Fixes #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.

compiler/rustc_passes/src/liveness.rs

@@ -1494,12 +1494,13 @@ impl<'tcx> Liveness<'_, 'tcx> {
         // bindings, and we also consider the first pattern to be the "authoritative" set of ids.
         // 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<Symbol, (LiveNode, Variable, Vec<(HirId, Span)>)> = <_>::default();
+        let mut vars: FxIndexMap<Symbol, (LiveNode, Variable, Vec<(HirId, Span, 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);
+            let id_and_sp = (hir_id, pat_sp, ident.span);
             vars.entry(self.ir.variable_name(var))
                 .and_modify(|(.., hir_ids_and_spans)| hir_ids_and_spans.push(id_and_sp))
                 .or_insert_with(|| (ln, var, vec![id_and_sp]));
@@ -1508,15 +1509,21 @@ impl<'tcx> Liveness<'_, 'tcx> {
         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();
+                let spans =
+                    hir_ids_and_spans.into_iter().map(|(_, _, ident_span)| ident_span).collect();
                 on_used_on_entry(spans, id, ln, var);
             } else {
                 self.report_unused(hir_ids_and_spans, ln, var);
             }
         }
     }
 
-    fn report_unused(&self, hir_ids_and_spans: Vec<(HirId, Span)>, ln: LiveNode, var: Variable) {
+    fn report_unused(
+        &self,
+        hir_ids_and_spans: Vec<(HirId, Span, 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") {
@@ -1530,62 +1537,78 @@ impl<'tcx> Liveness<'_, 'tcx> {
                 self.ir.tcx.struct_span_lint_hir(
                     lint::builtin::UNUSED_VARIABLES,
                     first_hir_id,
-                    hir_ids_and_spans.into_iter().map(|(_, sp)| sp).collect::<Vec<_>>(),
+                    hir_ids_and_spans
+                        .into_iter()
+                        .map(|(_, _, ident_span)| ident_span)
+                        .collect::<Vec<_>>(),
                     |lint| {
                         lint.build(&format!("variable `{}` is assigned to, but never used", name))
                             .note(&format!("consider using `_{}` instead", name))
                             .emit();
                     },
                 )
             } else {
-                self.ir.tcx.struct_span_lint_hir(
-                    lint::builtin::UNUSED_VARIABLES,
-                    first_hir_id,
-                    hir_ids_and_spans.iter().map(|(_, sp)| *sp).collect::<Vec<_>>(),
-                    |lint| {
-                        let mut err = lint.build(&format!("unused variable: `{}`", name));
-
-                        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<_>>(),
-                            );
+                let (shorthands, non_shorthands): (Vec<_>, Vec<_>) =
+                    hir_ids_and_spans.iter().copied().partition(|(hir_id, _, ident_span)| {
+                        let var = self.variable(*hir_id, *ident_span);
+                        self.ir.variable_is_shorthand(var)
+                    });
 
+                // 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() {
+                    let shorthands = shorthands
+                        .into_iter()
+                        .map(|(_, pat_span, _)| (pat_span, format!("{}: _", name)))
+                        .chain(
+                            non_shorthands
+                                .into_iter()
+                                .map(|(_, pat_span, _)| (pat_span, "_".to_string())),
+                        )
+                        .collect::<Vec<_>>();
+
+                    self.ir.tcx.struct_span_lint_hir(
+                        lint::builtin::UNUSED_VARIABLES,
+                        first_hir_id,
+                        hir_ids_and_spans
+                            .iter()
+                            .map(|(_, pat_span, _)| *pat_span)
+                            .collect::<Vec<_>>(),
+                        |lint| {
+                            let mut err = lint.build(&format!("unused variable: `{}`", name));
                             err.multipart_suggestion(
                                 "try ignoring the field",
                                 shorthands,
                                 Applicability::MachineApplicable,
                             );
-                        } else {
+                            err.emit()
+                        },
+                    );
+                } else {
+                    let non_shorthands = non_shorthands
+                        .into_iter()
+                        .map(|(_, _, ident_span)| (ident_span, format!("_{}", name)))
+                        .collect::<Vec<_>>();
+
+                    self.ir.tcx.struct_span_lint_hir(
+                        lint::builtin::UNUSED_VARIABLES,
+                        first_hir_id,
+                        hir_ids_and_spans
+                            .iter()
+                            .map(|(_, _, ident_span)| *ident_span)
+                            .collect::<Vec<_>>(),
+                        |lint| {
+                            let mut err = lint.build(&format!("unused variable: `{}`", name));
                             err.multipart_suggestion(
                                 "if this is intentional, prefix it with an underscore",
-                                non_shorthands
-                                    .into_iter()
-                                    .map(|(_, span)| (span, format!("_{}", name)))
-                                    .collect::<Vec<_>>(),
+                                non_shorthands,
                                 Applicability::MachineApplicable,
                             );
-                        }
-
-                        err.emit()
-                    },
-                );
+                            err.emit()
+                        },
+                    );
+                }
             }
         }
     }

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:9
+  --> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:33:13
    |
 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:10
+  --> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:37:14
    |
 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:26
+  --> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:46:30
    |
 LL |                          mut hours_are_suns,
-   |                          ^^^^^^^^^^^^^^^^^^
+   |                              ^^^^^^^^^^^^^^
    |
    = note: consider using `_hours_are_suns` instead
 

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

@@ -0,0 +1,12 @@
+// run-rustfix
+// Regression test for #81314: Unused variable lint should
+// span only the identifier and not the rest of the pattern
+
+#![deny(unused)]
+
+fn main() {
+    let [_rest @ ..] = [1, 2, 3]; //~ ERROR unused variable
+}
+
+pub fn foo([_rest @ ..]: &[i32]) { //~ ERROR unused variable
+}

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

@@ -0,0 +1,12 @@
+// run-rustfix
+// Regression test for #81314: Unused variable lint should
+// span only the identifier and not the rest of the pattern
+
+#![deny(unused)]
+
+fn main() {
+    let [rest @ ..] = [1, 2, 3]; //~ ERROR unused variable
+}
+
+pub fn foo([rest @ ..]: &[i32]) { //~ ERROR unused variable
+}

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

@@ -0,0 +1,21 @@
+error: unused variable: `rest`
+  --> $DIR/issue-81314-unused-span-ident.rs:8:10
+   |
+LL |     let [rest @ ..] = [1, 2, 3];
+   |          ^^^^ help: if this is intentional, prefix it with an underscore: `_rest`
+   |
+note: the lint level is defined here
+  --> $DIR/issue-81314-unused-span-ident.rs:5:9
+   |
+LL | #![deny(unused)]
+   |         ^^^^^^
+   = note: `#[deny(unused_variables)]` implied by `#[deny(unused)]`
+
+error: unused variable: `rest`
+  --> $DIR/issue-81314-unused-span-ident.rs:11:13
+   |
+LL | pub fn foo([rest @ ..]: &[i32]) {
+   |             ^^^^ help: if this is intentional, prefix it with an underscore: `_rest`
+
+error: aborting due to 2 previous errors
+

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

@@ -1,8 +1,8 @@
 warning: variable `a` is assigned to, but never used
-  --> $DIR/liveness-consts.rs:7:9
+  --> $DIR/liveness-consts.rs:7:13
    |
 LL |     let mut a = 0;
-   |         ^^^^^
+   |             ^
    |
 note: the lint level is defined here
   --> $DIR/liveness-consts.rs:2:9

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

@@ -1,8 +1,8 @@
 error: value assigned to `x` is never read
-  --> $DIR/liveness-dead.rs:9:9
+  --> $DIR/liveness-dead.rs:9:13
    |
 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:7
+  --> $DIR/liveness-dead.rs:20:11
    |
 LL | fn f4(mut x: i32) {
-   |       ^^^^^
+   |           ^
    |
    = help: maybe it is overwritten before being read?
 

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:9
+  --> $DIR/liveness-unused.rs:30:13
    |
 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:9
+  --> $DIR/liveness-unused.rs:37:13
    |
 LL |     let mut z = 3;
-   |         ^^^^^
+   |             ^
    |
    = note: consider using `_z` instead