Rollup merge of rust-lang#124580 - gurry:124556-suggest-remove-tuple-field, r=jackh726

Suggest removing unused tuple fields if they are the last fields

Fixes rust-lang#124556

We now check if dead/unused fields are the last fields of the tuple and suggest their removal instead of suggesting them to be changed to `()`.

Author: fmease

compiler/rustc_passes/messages.ftl

@@ -595,6 +595,15 @@ passes_pass_by_value =
 
 passes_proc_macro_bad_sig = {$kind} has incorrect signature
 
+passes_remove_fields =
+    consider removing { $num ->
+      [one] this
+     *[other] these
+    } { $num ->
+      [one] field
+     *[other] fields
+    }
+
 passes_repr_conflicting =
     conflicting representation hints
 

compiler/rustc_passes/src/dead.rs

@@ -24,8 +24,7 @@ use rustc_target::abi::FieldIdx;
 use std::mem;
 
 use crate::errors::{
-    ChangeFieldsToBeOfUnitType, IgnoredDerivedImpls, MultipleDeadCodes, ParentInfo,
-    UselessAssignment,
+    ChangeFields, IgnoredDerivedImpls, MultipleDeadCodes, ParentInfo, UselessAssignment,
 };
 
 // Any local node that may call something in its body block should be
@@ -1071,17 +1070,50 @@ impl<'tcx> DeadVisitor<'tcx> {
             };
 
         let diag = match report_on {
-            ReportOn::TupleField => MultipleDeadCodes::UnusedTupleStructFields {
-                multiple,
-                num,
-                descr,
-                participle,
-                name_list,
-                change_fields_suggestion: ChangeFieldsToBeOfUnitType { num, spans: spans.clone() },
-                parent_info,
-                ignored_derived_impls,
-            },
-
+            ReportOn::TupleField => {
+                let tuple_fields = if let Some(parent_id) = parent_item
+                    && let node = tcx.hir_node_by_def_id(parent_id)
+                    && let hir::Node::Item(hir::Item {
+                        kind: hir::ItemKind::Struct(hir::VariantData::Tuple(fields, _, _), _),
+                        ..
+                    }) = node
+                {
+                    *fields
+                } else {
+                    &[]
+                };
+
+                let trailing_tuple_fields = if tuple_fields.len() >= dead_codes.len() {
+                    LocalDefIdSet::from_iter(
+                        tuple_fields
+                            .iter()
+                            .skip(tuple_fields.len() - dead_codes.len())
+                            .map(|f| f.def_id),
+                    )
+                } else {
+                    LocalDefIdSet::default()
+                };
+
+                let fields_suggestion =
+                    // Suggest removal if all tuple fields are at the end.
+                    // Otherwise suggest removal or changing to unit type
+                    if dead_codes.iter().all(|dc| trailing_tuple_fields.contains(&dc.def_id)) {
+                        ChangeFields::Remove { num }
+                    } else {
+                        ChangeFields::ChangeToUnitTypeOrRemove { num, spans: spans.clone() }
+                    };
+
+                MultipleDeadCodes::UnusedTupleStructFields {
+                    multiple,
+                    num,
+                    descr,
+                    participle,
+                    name_list,
+                    change_fields_suggestion: fields_suggestion,
+                    parent_info,
+                    ignored_derived_impls,
+                }
+            }
             ReportOn::NamedField => MultipleDeadCodes::DeadCodes {
                 multiple,
                 num,

compiler/rustc_passes/src/errors.rs

@@ -1574,7 +1574,7 @@ pub enum MultipleDeadCodes<'tcx> {
         participle: &'tcx str,
         name_list: DiagSymbolList,
         #[subdiagnostic]
-        change_fields_suggestion: ChangeFieldsToBeOfUnitType,
+        change_fields_suggestion: ChangeFields,
         #[subdiagnostic]
         parent_info: Option<ParentInfo<'tcx>>,
         #[subdiagnostic]
@@ -1601,11 +1601,18 @@ pub struct IgnoredDerivedImpls {
 }
 
 #[derive(Subdiagnostic)]
-#[multipart_suggestion(passes_change_fields_to_be_of_unit_type, applicability = "has-placeholders")]
-pub struct ChangeFieldsToBeOfUnitType {
-    pub num: usize,
-    #[suggestion_part(code = "()")]
-    pub spans: Vec<Span>,
+pub enum ChangeFields {
+    #[multipart_suggestion(
+        passes_change_fields_to_be_of_unit_type,
+        applicability = "has-placeholders"
+    )]
+    ChangeToUnitTypeOrRemove {
+        num: usize,
+        #[suggestion_part(code = "()")]
+        spans: Vec<Span>,
+    },
+    #[help(passes_remove_fields)]
+    Remove { num: usize },
 }
 
 #[derive(Diagnostic)]

tests/ui/lint/dead-code/tuple-struct-field.rs

@@ -5,15 +5,20 @@ use std::marker::PhantomData;
 
 const LEN: usize = 4;
 
-struct SingleUnused(i32, [u8; LEN], String);
-//~^ ERROR: field `1` is never read
+struct UnusedAtTheEnd(i32, f32, [u8; LEN], String, u8);
+//~^ ERROR:fields `1`, `2`, `3`, and `4` are never read
+//~| NOTE: fields in this struct
+//~| HELP: consider removing these fields
+
+struct UnusedJustOneField(i32);
+//~^ ERROR: field `0` is never read
 //~| NOTE: field in this struct
-//~| HELP: consider changing the field to be of unit type
+//~| HELP: consider removing this field
 
-struct MultipleUnused(i32, f32, String, u8);
-//~^ ERROR: fields `0`, `1`, `2`, and `3` are never read
+struct UnusedInTheMiddle(i32, f32, String, u8, u32);
+//~^ ERROR: fields `1`, `2`, and `4` are never read
 //~| NOTE: fields in this struct
-//~| HELP: consider changing the fields to be of unit type
+//~| HELP: consider changing the fields to be of unit type to suppress this warning while preserving the field numbering, or remove the fields
 
 struct GoodUnit(());
 
@@ -23,15 +28,19 @@ struct Void;
 struct GoodVoid(Void);
 
 fn main() {
-    let w = SingleUnused(42, [0, 1, 2, 3], "abc".to_string());
-    let _ = w.0;
-    let _ = w.2;
+    let u1 = UnusedAtTheEnd(42, 3.14, [0, 1, 2, 3], "def".to_string(), 4u8);
+    let _ = u1.0;
+
+    let _ = UnusedJustOneField(42);
+
+    let u2 = UnusedInTheMiddle(42, 3.14, "def".to_string(), 4u8, 5);
+    let _ = u2.0;
+    let _ = u2.3;
 
-    let m = MultipleUnused(42, 3.14, "def".to_string(), 4u8);
 
     let gu = GoodUnit(());
     let gp = GoodPhantom(PhantomData);
     let gv = GoodVoid(Void);
 
-    let _ = (gu, gp, gv, m);
+    let _ = (gu, gp, gv);
 }

tests/ui/lint/dead-code/tuple-struct-field.stderr

@@ -1,33 +1,40 @@
-error: field `1` is never read
-  --> $DIR/tuple-struct-field.rs:8:26
+error: fields `1`, `2`, `3`, and `4` are never read
+  --> $DIR/tuple-struct-field.rs:8:28
    |
-LL | struct SingleUnused(i32, [u8; LEN], String);
-   |        ------------      ^^^^^^^^^
+LL | struct UnusedAtTheEnd(i32, f32, [u8; LEN], String, u8);
+   |        --------------      ^^^  ^^^^^^^^^  ^^^^^^  ^^
    |        |
-   |        field in this struct
+   |        fields in this struct
    |
+   = help: consider removing these fields
 note: the lint level is defined here
   --> $DIR/tuple-struct-field.rs:1:9
    |
 LL | #![deny(dead_code)]
    |         ^^^^^^^^^
-help: consider changing the field to be of unit type to suppress this warning while preserving the field numbering, or remove the field
+
+error: field `0` is never read
+  --> $DIR/tuple-struct-field.rs:13:27
+   |
+LL | struct UnusedJustOneField(i32);
+   |        ------------------ ^^^
+   |        |
+   |        field in this struct
    |
-LL | struct SingleUnused(i32, (), String);
-   |                          ~~
+   = help: consider removing this field
 
-error: fields `0`, `1`, `2`, and `3` are never read
-  --> $DIR/tuple-struct-field.rs:13:23
+error: fields `1`, `2`, and `4` are never read
+  --> $DIR/tuple-struct-field.rs:18:31
    |
-LL | struct MultipleUnused(i32, f32, String, u8);
-   |        -------------- ^^^  ^^^  ^^^^^^  ^^
+LL | struct UnusedInTheMiddle(i32, f32, String, u8, u32);
+   |        -----------------      ^^^  ^^^^^^      ^^^
    |        |
    |        fields in this struct
    |
 help: consider changing the fields to be of unit type to suppress this warning while preserving the field numbering, or remove the fields
    |
-LL | struct MultipleUnused((), (), (), ());
-   |                       ~~  ~~  ~~  ~~
+LL | struct UnusedInTheMiddle(i32, (), (), u8, ());
+   |                               ~~  ~~      ~~
 
-error: aborting due to 2 previous errors
+error: aborting due to 3 previous errors