Auto merge of #7677 - surechen:edit_large_enum_variant, r=camsteffen

fix bug for large_enum_variants

Fix the discussion problem in the issue of https://github.com/rust-lang/rust-clippy/issues/7666#issuecomment-919654291

About the false positive problem of case:
```rust
enum LargeEnum6 {
    A,
    B([u8;255]),
    C([u8;200]),
}
```

changelog: Fix largest_enum_variant wrongly identifying the second largest variant.

3 files changed, 171 insertions, 71 deletions

diff --git a/clippy_lints/src/large_enum_variant.rs b/clippy_lints/src/large_enum_variant.rs
index e4b8e754628..392166237be 100644
--- a/clippy_lints/src/large_enum_variant.rs
+++ b/clippy_lints/src/large_enum_variant.rs
@@ -1,13 +1,14 @@
 //! lint when there is a large size difference between variants on an enum
 
 use clippy_utils::diagnostics::span_lint_and_then;
-use clippy_utils::source::snippet_opt;
+use clippy_utils::source::snippet_with_applicability;
 use rustc_errors::Applicability;
-use rustc_hir::{Item, ItemKind, VariantData};
+use rustc_hir::{Item, ItemKind};
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_middle::lint::in_external_macro;
 use rustc_middle::ty::layout::LayoutOf;
 use rustc_session::{declare_tool_lint, impl_lint_pass};
+use rustc_span::source_map::Span;
 
 declare_clippy_lint! {
     /// ### What it does
@@ -58,6 +59,17 @@ impl LargeEnumVariant {
     }
 }
 
+struct FieldInfo {
+    ind: usize,
+    size: u64,
+}
+
+struct VariantInfo {
+    ind: usize,
+    size: u64,
+    fields_size: Vec<FieldInfo>,
+}
+
 impl_lint_pass!(LargeEnumVariant => [LARGE_ENUM_VARIANT]);
 
 impl<'tcx> LateLintPass<'tcx> for LargeEnumVariant {
@@ -68,72 +80,95 @@ impl<'tcx> LateLintPass<'tcx> for LargeEnumVariant {
         if let ItemKind::Enum(ref def, _) = item.kind {
             let ty = cx.tcx.type_of(item.def_id);
             let adt = ty.ty_adt_def().expect("already checked whether this is an enum");
-
-            let mut largest_variant: Option<(_, _)> = None;
-            let mut second_variant: Option<(_, _)> = None;
-
-            for (i, variant) in adt.variants.iter().enumerate() {
-                let size: u64 = variant
-                    .fields
-                    .iter()
-                    .filter_map(|f| {
-                        let ty = cx.tcx.type_of(f.did);
-                        // don't count generics by filtering out everything
-                        // that does not have a layout
-                        cx.layout_of(ty).ok().map(|l| l.size.bytes())
-                    })
-                    .sum();
-
-                let grouped = (size, (i, variant));
-
-                if grouped.0 >= largest_variant.map_or(0, |x| x.0) {
-                    second_variant = largest_variant;
-                    largest_variant = Some(grouped);
-                }
+            if adt.variants.len() <= 1 {
+                return;
             }
+            let mut variants_size: Vec<VariantInfo> = adt
+                .variants
+                .iter()
+                .enumerate()
+                .map(|(i, variant)| {
+                    let mut fields_size = Vec::new();
+                    let size: u64 = variant
+                        .fields
+                        .iter()
+                        .enumerate()
+                        .filter_map(|(i, f)| {
+                            let ty = cx.tcx.type_of(f.did);
+                            // don't count generics by filtering out everything
+                            // that does not have a layout
+                            cx.layout_of(ty).ok().map(|l| {
+                                let size = l.size.bytes();
+                                fields_size.push(FieldInfo { ind: i, size });
+                                size
+                            })
+                        })
+                        .sum();
+                    VariantInfo {
+                        ind: i,
+                        size,
+                        fields_size,
+                    }
+                })
+                .collect();
 
-            if let (Some(largest), Some(second)) = (largest_variant, second_variant) {
-                let difference = largest.0 - second.0;
+            variants_size.sort_by(|a, b| (b.size.cmp(&a.size)));
 
-                if difference > self.maximum_size_difference_allowed {
-                    let (i, variant) = largest.1;
+            let mut difference = variants_size[0].size - variants_size[1].size;
+            if difference > self.maximum_size_difference_allowed {
+                let help_text = "consider boxing the large fields to reduce the total size of the enum";
+                span_lint_and_then(
+                    cx,
+                    LARGE_ENUM_VARIANT,
+                    def.variants[variants_size[0].ind].span,
+                    "large size difference between variants",
+                    |diag| {
+                        diag.span_label(
+                            def.variants[variants_size[0].ind].span,
+                            &format!("this variant is {} bytes", variants_size[0].size),
+                        );
+                        diag.span_note(
+                            def.variants[variants_size[1].ind].span,
+                            &format!("and the second-largest variant is {} bytes:", variants_size[1].size),
+                        );
 
-                    let help_text = "consider boxing the large fields to reduce the total size of the enum";
-                    span_lint_and_then(
-                        cx,
-                        LARGE_ENUM_VARIANT,
-                        def.variants[i].span,
-                        "large size difference between variants",
-                        |diag| {
-                            diag.span_label(
-                                def.variants[(largest.1).0].span,
-                                &format!("this variant is {} bytes", largest.0),
-                            );
-                            diag.span_note(
-                                def.variants[(second.1).0].span,
-                                &format!("and the second-largest variant is {} bytes:", second.0),
-                            );
-                            if variant.fields.len() == 1 {
-                                let span = match def.variants[i].data {
-                                    VariantData::Struct(fields, ..) | VariantData::Tuple(fields, ..) => {
-                                        fields[0].ty.span
-                                    },
-                                    VariantData::Unit(..) => unreachable!(),
-                                };
-                                if let Some(snip) = snippet_opt(cx, span) {
-                                    diag.span_suggestion(
-                                        span,
-                                        help_text,
-                                        format!("Box<{}>", snip),
-                                        Applicability::MaybeIncorrect,
-                                    );
-                                    return;
+                        let fields = def.variants[variants_size[0].ind].data.fields();
+                        variants_size[0].fields_size.sort_by(|a, b| (a.size.cmp(&b.size)));
+                        let mut applicability = Applicability::MaybeIncorrect;
+                        let sugg: Vec<(Span, String)> = variants_size[0]
+                            .fields_size
+                            .iter()
+                            .rev()
+                            .map_while(|val| {
+                                if difference > self.maximum_size_difference_allowed {
+                                    difference = difference.saturating_sub(val.size);
+                                    Some((
+                                        fields[val.ind].ty.span,
+                                        format!(
+                                            "Box<{}>",
+                                            snippet_with_applicability(
+                                                cx,
+                                                fields[val.ind].ty.span,
+                                                "..",
+                                                &mut applicability
+                                            )
+                                            .into_owned()
+                                        ),
+                                    ))
+                                } else {
+                                    None
                                 }
-                            }
-                            diag.span_help(def.variants[i].span, help_text);
-                        },
-                    );
-                }
+                            })
+                            .collect();
+
+                        if !sugg.is_empty() {
+                            diag.multipart_suggestion(help_text, sugg, Applicability::MaybeIncorrect);
+                            return;
+                        }
+
+                        diag.span_help(def.variants[variants_size[0].ind].span, help_text);
+                    },
+                );
             }
         }
     }
diff --git a/tests/ui/large_enum_variant.rs b/tests/ui/large_enum_variant.rs
index d22fee3f27b..b45cc849eae 100644
--- a/tests/ui/large_enum_variant.rs
+++ b/tests/ui/large_enum_variant.rs
@@ -35,6 +35,7 @@ enum LargeEnum2 {
     VariantOk(i32, u32),
     ContainingLargeEnum(LargeEnum),
 }
+
 enum LargeEnum3 {
     ContainingMoreThanOneField(i32, [i32; 8000], [i32; 9500]),
     VoidVariant,
@@ -56,6 +57,23 @@ enum LargeEnumOk {
     LargeB([i32; 8001]),
 }
 
+enum LargeEnum6 {
+    A,
+    B([u8; 255]),
+    C([u8; 200]),
+}
+
+enum LargeEnum7 {
+    A,
+    B([u8; 1255]),
+    C([u8; 200]),
+}
+
+enum LargeEnum8 {
+    VariantOk(i32, u32),
+    ContainingMoreThanOneField([i32; 8000], [i32; 2], [i32; 9500], [i32; 30]),
+}
+
 fn main() {
     large_enum_variant!();
 }
diff --git a/tests/ui/large_enum_variant.stderr b/tests/ui/large_enum_variant.stderr
index 0eac28fbd35..899f97ce2e1 100644
--- a/tests/ui/large_enum_variant.stderr
+++ b/tests/ui/large_enum_variant.stderr
@@ -32,30 +32,45 @@ LL |     ContainingLargeEnum(Box<LargeEnum>),
    |                         ~~~~~~~~~~~~~~
 
 error: large size difference between variants
-  --> $DIR/large_enum_variant.rs:46:5
+  --> $DIR/large_enum_variant.rs:40:5
+   |
+LL |     ContainingMoreThanOneField(i32, [i32; 8000], [i32; 9500]),
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this variant is 70004 bytes
+   |
+note: and the second-largest variant is 8 bytes:
+  --> $DIR/large_enum_variant.rs:42:5
+   |
+LL |     StructLikeLittle { x: i32, y: i32 },
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+help: consider boxing the large fields to reduce the total size of the enum
+   |
+LL |     ContainingMoreThanOneField(i32, Box<[i32; 8000]>, Box<[i32; 9500]>),
+   |                                     ~~~~~~~~~~~~~~~~  ~~~~~~~~~~~~~~~~
+
+error: large size difference between variants
+  --> $DIR/large_enum_variant.rs:47:5
    |
 LL |     StructLikeLarge { x: [i32; 8000], y: i32 },
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this variant is 32004 bytes
    |
 note: and the second-largest variant is 8 bytes:
-  --> $DIR/large_enum_variant.rs:45:5
+  --> $DIR/large_enum_variant.rs:46:5
    |
 LL |     VariantOk(i32, u32),
    |     ^^^^^^^^^^^^^^^^^^^
 help: consider boxing the large fields to reduce the total size of the enum
-  --> $DIR/large_enum_variant.rs:46:5
    |
-LL |     StructLikeLarge { x: [i32; 8000], y: i32 },
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+LL |     StructLikeLarge { x: Box<[i32; 8000]>, y: i32 },
+   |                          ~~~~~~~~~~~~~~~~
 
 error: large size difference between variants
-  --> $DIR/large_enum_variant.rs:51:5
+  --> $DIR/large_enum_variant.rs:52:5
    |
 LL |     StructLikeLarge2 { x: [i32; 8000] },
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this variant is 32000 bytes
    |
 note: and the second-largest variant is 8 bytes:
-  --> $DIR/large_enum_variant.rs:50:5
+  --> $DIR/large_enum_variant.rs:51:5
    |
 LL |     VariantOk(i32, u32),
    |     ^^^^^^^^^^^^^^^^^^^
@@ -64,5 +79,37 @@ help: consider boxing the large fields to reduce the total size of the enum
 LL |     StructLikeLarge2 { x: Box<[i32; 8000]> },
    |                           ~~~~~~~~~~~~~~~~
 
-error: aborting due to 4 previous errors
+error: large size difference between variants
+  --> $DIR/large_enum_variant.rs:68:5
+   |
+LL |     B([u8; 1255]),
+   |     ^^^^^^^^^^^^^ this variant is 1255 bytes
+   |
+note: and the second-largest variant is 200 bytes:
+  --> $DIR/large_enum_variant.rs:69:5
+   |
+LL |     C([u8; 200]),
+   |     ^^^^^^^^^^^^
+help: consider boxing the large fields to reduce the total size of the enum
+   |
+LL |     B(Box<[u8; 1255]>),
+   |       ~~~~~~~~~~~~~~~
+
+error: large size difference between variants
+  --> $DIR/large_enum_variant.rs:74:5
+   |
+LL |     ContainingMoreThanOneField([i32; 8000], [i32; 2], [i32; 9500], [i32; 30]),
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this variant is 70128 bytes
+   |
+note: and the second-largest variant is 8 bytes:
+  --> $DIR/large_enum_variant.rs:73:5
+   |
+LL |     VariantOk(i32, u32),
+   |     ^^^^^^^^^^^^^^^^^^^
+help: consider boxing the large fields to reduce the total size of the enum
+   |
+LL |     ContainingMoreThanOneField(Box<[i32; 8000]>, [i32; 2], Box<[i32; 9500]>, [i32; 30]),
+   |                                ~~~~~~~~~~~~~~~~            ~~~~~~~~~~~~~~~~
+
+error: aborting due to 7 previous errors