about summary refs log tree commit diff
diff options
context:
space:
mode:
authorSamuel Tardieu <sam@rfc1149.net>2023-02-25 15:49:59 +0100
committerSamuel Tardieu <sam@rfc1149.net>2023-02-26 15:44:04 +0100
commitc82ff00539c4441eddecd12151f54c30d1b03430 (patch)
treecdddcf17d029b34a8d4385ef1ba9b5b39649b1d1
parent149392b0baa4730c68f3c3eadf5c6ed7b16b85a4 (diff)
downloadrust-c82ff00539c4441eddecd12151f54c30d1b03430.tar.gz
rust-c82ff00539c4441eddecd12151f54c30d1b03430.zip
Do not suggest to derive Default on generics with implicit arguments
-rw-r--r--clippy_lints/src/derivable_impls.rs21
-rw-r--r--tests/ui/derivable_impls.fixed37
-rw-r--r--tests/ui/derivable_impls.rs37
3 files changed, 87 insertions, 8 deletions
diff --git a/clippy_lints/src/derivable_impls.rs b/clippy_lints/src/derivable_impls.rs
index f95b8ccf067..549ef2d24ca 100644
--- a/clippy_lints/src/derivable_impls.rs
+++ b/clippy_lints/src/derivable_impls.rs
@@ -8,7 +8,7 @@ use rustc_hir::{
     Body, Expr, ExprKind, GenericArg, Impl, ImplItemKind, Item, ItemKind, Node, PathSegment, QPath, Ty, TyKind,
 };
 use rustc_lint::{LateContext, LateLintPass};
-use rustc_middle::ty::{AdtDef, DefIdTree};
+use rustc_middle::ty::{Adt, AdtDef, DefIdTree, SubstsRef};
 use rustc_session::{declare_tool_lint, impl_lint_pass};
 use rustc_span::sym;
 
@@ -81,13 +81,18 @@ fn check_struct<'tcx>(
     self_ty: &Ty<'_>,
     func_expr: &Expr<'_>,
     adt_def: AdtDef<'_>,
+    substs: SubstsRef<'_>,
 ) {
     if let TyKind::Path(QPath::Resolved(_, p)) = self_ty.kind {
-        if let Some(PathSegment { args: Some(a), .. }) = p.segments.last() {
-            for arg in a.args {
-                if !matches!(arg, GenericArg::Lifetime(_)) {
-                    return;
-                }
+        if let Some(PathSegment { args, .. }) = p.segments.last() {
+            let args = args.map(|a| a.args).unwrap_or(&[]);
+
+            // substs contains the generic parameters of the type declaration, while args contains the arguments
+            // used at instantiation time. If both len are not equal, it means that some parameters were not
+            // provided (which means that the default values were used); in this case we will not risk
+            // suggesting too broad a rewrite. We won't either if any argument is a type or a const.
+            if substs.len() != args.len() || args.iter().any(|arg| !matches!(arg, GenericArg::Lifetime(_))) {
+                return;
             }
         }
     }
@@ -184,7 +189,7 @@ impl<'tcx> LateLintPass<'tcx> for DerivableImpls {
             if let Some(Node::ImplItem(impl_item)) = cx.tcx.hir().find(impl_item_hir);
             if let ImplItemKind::Fn(_, b) = &impl_item.kind;
             if let Body { value: func_expr, .. } = cx.tcx.hir().body(*b);
-            if let Some(adt_def) = cx.tcx.type_of(item.owner_id).subst_identity().ty_adt_def();
+            if let &Adt(adt_def, substs) = cx.tcx.type_of(item.owner_id).subst_identity().kind();
             if let attrs = cx.tcx.hir().attrs(item.hir_id());
             if !attrs.iter().any(|attr| attr.doc_str().is_some());
             if let child_attrs = cx.tcx.hir().attrs(impl_item_hir);
@@ -192,7 +197,7 @@ impl<'tcx> LateLintPass<'tcx> for DerivableImpls {
 
             then {
                 if adt_def.is_struct() {
-                    check_struct(cx, item, self_ty, func_expr, adt_def);
+                    check_struct(cx, item, self_ty, func_expr, adt_def, substs);
                 } else if adt_def.is_enum() && self.msrv.meets(msrvs::DEFAULT_ENUM_ATTRIBUTE) {
                     check_enum(cx, item, func_expr, adt_def);
                 }
diff --git a/tests/ui/derivable_impls.fixed b/tests/ui/derivable_impls.fixed
index ee8456f5deb..89ec33a0d8f 100644
--- a/tests/ui/derivable_impls.fixed
+++ b/tests/ui/derivable_impls.fixed
@@ -231,4 +231,41 @@ impl Default for NonExhaustiveEnum {
     }
 }
 
+// https://github.com/rust-lang/rust-clippy/issues/10396
+
+#[derive(Default)]
+struct DefaultType;
+
+struct GenericType<T = DefaultType> {
+    t: T,
+}
+
+impl Default for GenericType {
+    fn default() -> Self {
+        Self { t: Default::default() }
+    }
+}
+
+struct InnerGenericType<T> {
+    t: T,
+}
+
+impl Default for InnerGenericType<DefaultType> {
+    fn default() -> Self {
+        Self { t: Default::default() }
+    }
+}
+
+struct OtherGenericType<T = DefaultType> {
+    inner: InnerGenericType<T>,
+}
+
+impl Default for OtherGenericType {
+    fn default() -> Self {
+        Self {
+            inner: Default::default(),
+        }
+    }
+}
+
 fn main() {}
diff --git a/tests/ui/derivable_impls.rs b/tests/ui/derivable_impls.rs
index 14af419bcad..def6e41162f 100644
--- a/tests/ui/derivable_impls.rs
+++ b/tests/ui/derivable_impls.rs
@@ -267,4 +267,41 @@ impl Default for NonExhaustiveEnum {
     }
 }
 
+// https://github.com/rust-lang/rust-clippy/issues/10396
+
+#[derive(Default)]
+struct DefaultType;
+
+struct GenericType<T = DefaultType> {
+    t: T,
+}
+
+impl Default for GenericType {
+    fn default() -> Self {
+        Self { t: Default::default() }
+    }
+}
+
+struct InnerGenericType<T> {
+    t: T,
+}
+
+impl Default for InnerGenericType<DefaultType> {
+    fn default() -> Self {
+        Self { t: Default::default() }
+    }
+}
+
+struct OtherGenericType<T = DefaultType> {
+    inner: InnerGenericType<T>,
+}
+
+impl Default for OtherGenericType {
+    fn default() -> Self {
+        Self {
+            inner: Default::default(),
+        }
+    }
+}
+
 fn main() {}