Auto merge of #10399 - samueltardieu:issue-10396, r=Manishearth

Do not suggest to derive `Default` on generics with implicit arguments Fixes #10396 changelog: FP: [`derivable_impls`]: do not suggest to derive `Default` on generics with implicit arguments
rust-lang · Feb 27, 2023 · 2742ac0 · 2742ac0
2 parents ba7fd68 + c82ff00
commit 2742ac0
Show file tree

Hide file tree

Showing 3 changed files with 87 additions and 8 deletions.
diff --git 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,15 +189,15 @@ 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);
             if !child_attrs.iter().any(|attr| attr.doc_str().is_some());
 
             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
@@ -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
@@ -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() {}