Fix ICE in vec_box lint and add run-rustfix

`hir::Ty` doesn't seem to know anything about type bounds and
`cx.tcx.type_of(def_id)` caused an ICE when it was passed a generic type
with a bound:

```
src/librustc_typeck/collect.rs:1311: unexpected non-type Node::GenericParam: Type { default: None, synthetic: None }
```

Converting it to a proper `Ty` fixes the ICE and catches a few more
places where the lint applies.

diff --git a/clippy_lints/src/types.rs b/clippy_lints/src/types.rs
index 939dd27c441590f0c938bb4dcdf250a3641a0448..75e2f75ffcd4623df48b0fa90c0826d32c2bd819 100644 (file)
--- a/clippy_lints/src/types.rs
+++ b/clippy_lints/src/types.rs
@@ -275,26 +275,24 @@ fn check_ty(cx: &LateContext<'_, '_>, hir_ty: &hir::Ty, is_local: bool) {
                         if Some(def_id) == cx.tcx.lang_items().owned_box();
                         // At this point, we know ty is Box<T>, now get T
                         if let Some(ref last) = last_path_segment(ty_qpath).args;
-                        if let Some(ty) = last.args.iter().find_map(|arg| match arg {
+                        if let Some(boxed_ty) = last.args.iter().find_map(|arg| match arg {
                             GenericArg::Type(ty) => Some(ty),
                             GenericArg::Lifetime(_) => None,
                         });
-                        if let TyKind::Path(ref ty_qpath) = ty.node;
-                        let def = cx.tables.qpath_def(ty_qpath, ty.hir_id);
-                        if let Some(def_id) = opt_def_id(def);
-                        let boxed_type = cx.tcx.type_of(def_id);
-                        if boxed_type.is_sized(cx.tcx.at(ty.span), cx.param_env);
                         then {
-                            span_lint_and_sugg(
-                                cx,
-                                VEC_BOX,
-                                hir_ty.span,
-                                "`Vec<T>` is already on the heap, the boxing is unnecessary.",
-                                "try",
-                                format!("Vec<{}>", boxed_type),
-                                Applicability::MaybeIncorrect,
-                            );
-                            return; // don't recurse into the type
+                            let ty_ty = hir_ty_to_ty(cx.tcx, boxed_ty);
+                            if ty_ty.is_sized(cx.tcx.at(ty.span), cx.param_env) {
+                                span_lint_and_sugg(
+                                    cx,
+                                    VEC_BOX,
+                                    hir_ty.span,
+                                    "`Vec<T>` is already on the heap, the boxing is unnecessary.",
+                                    "try",
+                                    format!("Vec<{}>", ty_ty),
+                                    Applicability::MaybeIncorrect,
+                                );
+                                return; // don't recurse into the type
+                            }
                         }
                     }
                 } else if match_def_path(cx.tcx, def_id, &paths::OPTION) {

diff --git a/tests/ui/complex_types.rs b/tests/ui/complex_types.rs
index cfece3768ef853861b5d86a02d3b207f695de620..be61fb6b9be613cca8bd66f622644b253af6cabd 100644 (file)
--- a/tests/ui/complex_types.rs
+++ b/tests/ui/complex_types.rs
@@ -1,5 +1,5 @@
 #![warn(clippy::all)]
-#![allow(unused, clippy::needless_pass_by_value)]
+#![allow(unused, clippy::needless_pass_by_value, clippy::vec_box)]
 #![feature(associated_type_defaults)]
 
 type Alias = Vec<Vec<Box<(u32, u32, u32, u32)>>>; // no warning here

diff --git a/tests/ui/vec_box_sized.fixed b/tests/ui/vec_box_sized.fixed
new file mode 100644 (file)
index 0000000..a56dac8
--- /dev/null
+++ b/tests/ui/vec_box_sized.fixed
@@ -0,0 +1,36 @@
+// run-rustfix
+
+#![allow(dead_code)]
+
+struct SizedStruct(i32);
+struct UnsizedStruct([i32]);
+
+/// The following should trigger the lint
+mod should_trigger {
+    use super::SizedStruct;
+
+    struct StructWithVecBox {
+        sized_type: Vec<SizedStruct>,
+    }
+
+    struct A(Vec<SizedStruct>);
+    struct B(Vec<Vec<u32>>);
+}
+
+/// The following should not trigger the lint
+mod should_not_trigger {
+    use super::UnsizedStruct;
+
+    struct C(Vec<Box<UnsizedStruct>>);
+
+    struct StructWithVecBoxButItsUnsized {
+        unsized_type: Vec<Box<UnsizedStruct>>,
+    }
+
+    struct TraitVec<T: ?Sized> {
+        // Regression test for #3720. This was causing an ICE.
+        inner: Vec<Box<T>>,
+    }
+}
+
+fn main() {}

diff --git a/tests/ui/vec_box_sized.rs b/tests/ui/vec_box_sized.rs
index 884761675fce5c1999ddd2db8ec8143b0ef5da3d..32d1e940f2712d40a776c48d361ce4e36a25a0b1 100644 (file)
--- a/tests/ui/vec_box_sized.rs
+++ b/tests/ui/vec_box_sized.rs
@@ -1,17 +1,36 @@
-struct SizedStruct {
-    _a: i32,
-}
+// run-rustfix
 
-struct UnsizedStruct {
-    _a: [i32],
-}
+#![allow(dead_code)]
+
+struct SizedStruct(i32);
+struct UnsizedStruct([i32]);
+
+/// The following should trigger the lint
+mod should_trigger {
+    use super::SizedStruct;
 
-struct StructWithVecBox {
-    sized_type: Vec<Box<SizedStruct>>,
+    struct StructWithVecBox {
+        sized_type: Vec<Box<SizedStruct>>,
+    }
+
+    struct A(Vec<Box<SizedStruct>>);
+    struct B(Vec<Vec<Box<(u32)>>>);
 }
 
-struct StructWithVecBoxButItsUnsized {
-    unsized_type: Vec<Box<UnsizedStruct>>,
+/// The following should not trigger the lint
+mod should_not_trigger {
+    use super::UnsizedStruct;
+
+    struct C(Vec<Box<UnsizedStruct>>);
+
+    struct StructWithVecBoxButItsUnsized {
+        unsized_type: Vec<Box<UnsizedStruct>>,
+    }
+
+    struct TraitVec<T: ?Sized> {
+        // Regression test for #3720. This was causing an ICE.
+        inner: Vec<Box<T>>,
+    }
 }
 
 fn main() {}

diff --git a/tests/ui/vec_box_sized.stderr b/tests/ui/vec_box_sized.stderr
index 21b515fa48674782416470dc8e6805ea5694b5b6..b33880b46bd8886344aa40c0ac169b4a294d2a2d 100644 (file)
--- a/tests/ui/vec_box_sized.stderr
+++ b/tests/ui/vec_box_sized.stderr
@@ -1,10 +1,22 @@
 error: `Vec<T>` is already on the heap, the boxing is unnecessary.
-  --> $DIR/vec_box_sized.rs:10:17
+  --> $DIR/vec_box_sized.rs:13:21
    |
-LL |     sized_type: Vec<Box<SizedStruct>>,
-   |                 ^^^^^^^^^^^^^^^^^^^^^ help: try: `Vec<SizedStruct>`
+LL |         sized_type: Vec<Box<SizedStruct>>,
+   |                     ^^^^^^^^^^^^^^^^^^^^^ help: try: `Vec<SizedStruct>`
    |
    = note: `-D clippy::vec-box` implied by `-D warnings`
 
-error: aborting due to previous error
+error: `Vec<T>` is already on the heap, the boxing is unnecessary.
+  --> $DIR/vec_box_sized.rs:16:14
+   |
+LL |     struct A(Vec<Box<SizedStruct>>);
+   |              ^^^^^^^^^^^^^^^^^^^^^ help: try: `Vec<SizedStruct>`
+
+error: `Vec<T>` is already on the heap, the boxing is unnecessary.
+  --> $DIR/vec_box_sized.rs:17:18
+   |
+LL |     struct B(Vec<Vec<Box<(u32)>>>);
+   |                  ^^^^^^^^^^^^^^^ help: try: `Vec<u32>`
+
+error: aborting due to 3 previous errors