Merge #4445

4445: Correctly fill default type parameters r=flodiebold a=montekki

Fixes #3877 

So, basically even if the parameters are omitted from the `impl` block, check the parameters in `trait` if they have a default type, and if they do go from `hir` to `ast::TypeArg`. I've added a helper for that but I am not sure that it's a proper way to go from `hir` to `ast` here.

Co-authored-by: Fedor Sakharov <[email protected]>

5 files changed, 92 insertions, 2 deletions

diff --git a/crates/ra_assists/src/ast_transform.rs b/crates/ra_assists/src/ast_transform.rs
index 9ac65ab39..3079a02a2 100644
--- a/crates/ra_assists/src/ast_transform.rs
+++ b/crates/ra_assists/src/ast_transform.rs
@@ -1,7 +1,7 @@
 //! `AstTransformer`s are functions that replace nodes in an AST and can be easily combined.
 use rustc_hash::FxHashMap;
 
-use hir::{PathResolution, SemanticsScope};
+use hir::{HirDisplay, PathResolution, SemanticsScope};
 use ra_ide_db::RootDatabase;
 use ra_syntax::{
     algo::SyntaxRewriter,
@@ -51,7 +51,27 @@ impl<'a> SubstituteTypeParams<'a> {
             .into_iter()
             // this is a trait impl, so we need to skip the first type parameter -- this is a bit hacky
             .skip(1)
-            .zip(substs.into_iter())
+            // The actual list of trait type parameters may be longer than the one
+            // used in the `impl` block due to trailing default type parametrs.
+            // For that case we extend the `substs` with an empty iterator so we
+            // can still hit those trailing values and check if they actually have
+            // a default type. If they do, go for that type from `hir` to `ast` so
+            // the resulting change can be applied correctly.
+            .zip(substs.into_iter().map(Some).chain(std::iter::repeat(None)))
+            .filter_map(|(k, v)| match v {
+                Some(v) => Some((k, v)),
+                None => {
+                    let default = k.default(source_scope.db)?;
+                    Some((
+                        k,
+                        ast::make::type_ref(
+                            &default
+                                .display_source_code(source_scope.db, source_scope.module()?.into())
+                                .ok()?,
+                        ),
+                    ))
+                }
+            })
             .collect();
         return SubstituteTypeParams {
             source_scope,
diff --git a/crates/ra_assists/src/handlers/add_missing_impl_members.rs b/crates/ra_assists/src/handlers/add_missing_impl_members.rs
index c1ce87914..22e1156d2 100644
--- a/crates/ra_assists/src/handlers/add_missing_impl_members.rs
+++ b/crates/ra_assists/src/handlers/add_missing_impl_members.rs
@@ -618,4 +618,54 @@ impl Foo for S {
 }"#,
         )
     }
+
+    #[test]
+    fn test_generic_single_default_parameter() {
+        check_assist(
+            add_missing_impl_members,
+            r#"
+trait Foo<T = Self> {
+    fn bar(&self, other: &T);
+}
+
+struct S;
+impl Foo for S { <|> }"#,
+            r#"
+trait Foo<T = Self> {
+    fn bar(&self, other: &T);
+}
+
+struct S;
+impl Foo for S {
+    <|>fn bar(&self, other: &Self) {
+        todo!()
+    }
+}"#,
+        )
+    }
+
+    #[test]
+    fn test_generic_default_parameter_is_second() {
+        check_assist(
+            add_missing_impl_members,
+            r#"
+trait Foo<T1, T2 = Self> {
+    fn bar(&self, this: &T1, that: &T2);
+}
+
+struct S<T>;
+impl Foo<T> for S<T> { <|> }"#,
+            r#"
+trait Foo<T1, T2 = Self> {
+    fn bar(&self, this: &T1, that: &T2);
+}
+
+struct S<T>;
+impl Foo<T> for S<T> {
+    <|>fn bar(&self, this: &T, that: &Self) {
+        todo!()
+    }
+}"#,
+        )
+    }
 }
diff --git a/crates/ra_hir/src/code_model.rs b/crates/ra_hir/src/code_model.rs
index e8e3211fc..840cfdfc8 100644
--- a/crates/ra_hir/src/code_model.rs
+++ b/crates/ra_hir/src/code_model.rs
@@ -989,6 +989,17 @@ impl TypeParam {
             ty: InEnvironment { value: ty, environment },
         }
     }
+
+    pub fn default(self, db: &dyn HirDatabase) -> Option<Type> {
+        let params = db.generic_defaults(self.id.parent);
+        let local_idx = hir_ty::param_idx(db, self.id)?;
+        let resolver = self.id.parent.resolver(db.upcast());
+        let environment = TraitEnvironment::lower(db, &resolver);
+        params.get(local_idx).cloned().map(|ty| Type {
+            krate: self.id.parent.module(db.upcast()).krate,
+            ty: InEnvironment { value: ty, environment },
+        })
+    }
 }
 
 // FIXME: rename from `ImplDef` to `Impl`
diff --git a/crates/ra_hir_ty/src/lib.rs b/crates/ra_hir_ty/src/lib.rs
index e8f3482fe..ccc4348f4 100644
--- a/crates/ra_hir_ty/src/lib.rs
+++ b/crates/ra_hir_ty/src/lib.rs
@@ -427,6 +427,11 @@ impl Substs {
     }
 }
 
+/// Return an index of a parameter in the generic type parameter list by it's id.
+pub fn param_idx(db: &dyn HirDatabase, id: TypeParamId) -> Option<usize> {
+    generics(db.upcast(), id.parent).param_idx(id)
+}
+
 #[derive(Debug, Clone)]
 pub struct SubstsBuilder {
     vec: Vec<Ty>,
diff --git a/crates/ra_syntax/src/ast/make.rs b/crates/ra_syntax/src/ast/make.rs
index 12c5228f5..d0e960fb4 100644
--- a/crates/ra_syntax/src/ast/make.rs
+++ b/crates/ra_syntax/src/ast/make.rs
@@ -13,6 +13,10 @@ pub fn name_ref(text: &str) -> ast::NameRef {
     ast_from_text(&format!("fn f() {{ {}; }}", text))
 }
 
+pub fn type_ref(text: &str) -> ast::TypeRef {
+    ast_from_text(&format!("impl {} for D {{}};", text))
+}
+
 pub fn path_segment(name_ref: ast::NameRef) -> ast::PathSegment {
     ast_from_text(&format!("use {};", name_ref))
 }