Apply suggestions from code review

3 files changed, 328 insertions, 65 deletions

diff --git a/crates/ra_hir/src/code_model.rs b/crates/ra_hir/src/code_model.rs
index d0a8199a6..ffd5278ec 100644
--- a/crates/ra_hir/src/code_model.rs
+++ b/crates/ra_hir/src/code_model.rs
@@ -26,8 +26,8 @@ use hir_ty::{
     autoderef,
     display::{HirDisplayError, HirFormatter},
     expr::ExprValidator,
-    method_resolution, ApplicationTy, Canonical, InEnvironment, Substs, TraitEnvironment, TraitRef,
-    Ty, TyDefId, TypeCtor,
+    method_resolution, ApplicationTy, Canonical, GenericPredicate, InEnvironment, Substs,
+    TraitEnvironment, Ty, TyDefId, TypeCtor,
 };
 use ra_db::{CrateId, CrateName, Edition, FileId};
 use ra_prof::profile;
@@ -1379,8 +1379,17 @@ impl Type {
         self.ty.value.dyn_trait().map(Into::into)
     }
 
-    pub fn as_impl_trait(&self, db: &dyn HirDatabase) -> Option<Trait> {
-        self.ty.value.impl_trait_ref(db).map(|it| it.trait_.into())
+    pub fn as_impl_traits(&self, db: &dyn HirDatabase) -> Option<Vec<Trait>> {
+        self.ty.value.impl_trait_bounds(db).map(|it| {
+            it.into_iter()
+                .filter_map(|pred| match pred {
+                    hir_ty::GenericPredicate::Implemented(trait_ref) => {
+                        Some(Trait::from(trait_ref.trait_))
+                    }
+                    _ => None,
+                })
+                .collect()
+        })
     }
 
     pub fn as_associated_type_parent_trait(&self, db: &dyn HirDatabase) -> Option<Trait> {
@@ -1410,71 +1419,77 @@ impl Type {
     }
 
     pub fn walk(&self, db: &dyn HirDatabase, mut cb: impl FnMut(Type)) {
-        // TypeWalk::walk does not preserve items order!
-        fn walk_substs(db: &dyn HirDatabase, substs: &Substs, cb: &mut impl FnMut(Type)) {
+        // TypeWalk::walk for a Ty at first visits parameters and only after that the Ty itself.
+        // We need a different order here.
+
+        fn walk_substs(
+            db: &dyn HirDatabase,
+            type_: &Type,
+            substs: &Substs,
+            cb: &mut impl FnMut(Type),
+        ) {
             for ty in substs.iter() {
-                walk_ty(db, ty, cb);
+                walk_type(db, &type_.derived(ty.clone()), cb);
             }
         }
 
-        fn walk_trait(
+        fn walk_bounds(
             db: &dyn HirDatabase,
-            ty: Ty,
-            trait_ref: &TraitRef,
+            type_: &Type,
+            bounds: &[GenericPredicate],
             cb: &mut impl FnMut(Type),
         ) {
-            let def_db: &dyn DefDatabase = db.upcast();
-            let resolver = trait_ref.trait_.resolver(def_db);
-            let krate = trait_ref.trait_.lookup(def_db).container.module(def_db).krate;
-            cb(Type::new_with_resolver_inner(db, krate, &resolver, ty));
-            walk_substs(db, &trait_ref.substs, cb);
+            for pred in bounds {
+                match pred {
+                    GenericPredicate::Implemented(trait_ref) => {
+                        cb(type_.clone());
+                        walk_substs(db, type_, &trait_ref.substs, cb);
+                    }
+                    _ => (),
+                }
+            }
         }
 
-        fn walk_ty(db: &dyn HirDatabase, ty: &Ty, cb: &mut impl FnMut(Type)) {
-            let def_db: &dyn DefDatabase = db.upcast();
-            let ty = ty.strip_references();
+        fn walk_type(db: &dyn HirDatabase, type_: &Type, cb: &mut impl FnMut(Type)) {
+            let ty = type_.ty.value.strip_references();
             match ty {
                 Ty::Apply(ApplicationTy { ctor, parameters }) => {
                     match ctor {
-                        TypeCtor::Adt(adt) => {
-                            cb(Type::from_def(db, adt.module(def_db).krate, *adt));
+                        TypeCtor::Adt(_) => {
+                            cb(type_.derived(ty.clone()));
                         }
                         TypeCtor::AssociatedType(_) => {
-                            if let Some(trait_id) = ty.associated_type_parent_trait(db) {
-                                let resolver = trait_id.resolver(def_db);
-                                let krate = trait_id.lookup(def_db).container.module(def_db).krate;
-                                cb(Type::new_with_resolver_inner(db, krate, &resolver, ty.clone()));
+                            if let Some(_) = ty.associated_type_parent_trait(db) {
+                                cb(type_.derived(ty.clone()));
                             }
                         }
                         _ => (),
                     }
 
                     // adt params, tuples, etc...
-                    walk_substs(db, parameters, cb);
+                    walk_substs(db, type_, parameters, cb);
                 }
                 Ty::Opaque(opaque_ty) => {
-                    if let Some(trait_ref) = ty.impl_trait_ref(db) {
-                        walk_trait(db, ty.clone(), &trait_ref, cb);
+                    if let Some(bounds) = ty.impl_trait_bounds(db) {
+                        walk_bounds(db, &type_.derived(ty.clone()), &bounds, cb);
                     }
 
-                    walk_substs(db, &opaque_ty.parameters, cb);
+                    walk_substs(db, type_, &opaque_ty.parameters, cb);
                 }
                 Ty::Placeholder(_) => {
-                    if let Some(trait_ref) = ty.impl_trait_ref(db) {
-                        walk_trait(db, ty.clone(), &trait_ref, cb);
+                    if let Some(bounds) = ty.impl_trait_bounds(db) {
+                        walk_bounds(db, &type_.derived(ty.clone()), &bounds, cb);
                     }
                 }
-                Ty::Dyn(_) => {
-                    if let Some(trait_ref) = ty.dyn_trait_ref() {
-                        walk_trait(db, ty.clone(), trait_ref, cb);
-                    }
+                Ty::Dyn(bounds) => {
+                    walk_bounds(db, &type_.derived(ty.clone()), bounds.as_ref(), cb);
                 }
 
                 _ => (),
             }
         }
 
-        walk_ty(db, &self.ty.value, &mut cb);
+        walk_type(db, self, &mut cb);
     }
 }
 
diff --git a/crates/ra_hir_ty/src/lib.rs b/crates/ra_hir_ty/src/lib.rs
index 25ab9d872..f22232324 100644
--- a/crates/ra_hir_ty/src/lib.rs
+++ b/crates/ra_hir_ty/src/lib.rs
@@ -73,6 +73,7 @@ pub use lower::{
 pub use traits::{InEnvironment, Obligation, ProjectionPredicate, TraitEnvironment};
 
 pub use chalk_ir::{BoundVar, DebruijnIndex};
+use itertools::Itertools;
 
 /// A type constructor or type name: this might be something like the primitive
 /// type `bool`, a struct like `Vec`, or things like function pointers or
@@ -815,6 +816,11 @@ impl Ty {
         }
     }
 
+    /// If this is a `dyn Trait`, returns that trait.
+    pub fn dyn_trait(&self) -> Option<TraitId> {
+        self.dyn_trait_ref().map(|it| it.trait_)
+    }
+
     fn builtin_deref(&self) -> Option<Ty> {
         match self {
             Ty::Apply(a_ty) => match a_ty.ctor {
@@ -867,18 +873,7 @@ impl Ty {
         }
     }
 
-    /// If this is a `dyn Trait`, returns that trait.
-    pub fn dyn_trait(&self) -> Option<TraitId> {
-        match self {
-            Ty::Dyn(predicates) => predicates.iter().find_map(|pred| match pred {
-                GenericPredicate::Implemented(tr) => Some(tr.trait_),
-                _ => None,
-            }),
-            _ => None,
-        }
-    }
-
-    pub fn impl_trait_ref(&self, db: &dyn HirDatabase) -> Option<TraitRef> {
+    pub fn impl_trait_bounds(&self, db: &dyn HirDatabase) -> Option<Vec<GenericPredicate>> {
         match self {
             Ty::Opaque(opaque_ty) => {
                 let predicates = match opaque_ty.opaque_ty_id {
@@ -892,25 +887,21 @@ impl Ty {
                     }
                 };
 
-                predicates.and_then(|it| {
-                    it.value.iter().find_map(|pred| match pred {
-                        GenericPredicate::Implemented(tr) => Some(tr.clone()),
-                        _ => None,
-                    })
-                })
+                predicates.map(|it| it.value)
             }
             Ty::Placeholder(id) => {
                 let generic_params = db.generic_params(id.parent);
                 let param_data = &generic_params.types[id.local_id];
                 match param_data.provenance {
-                    hir_def::generics::TypeParamProvenance::ArgumentImplTrait => db
-                        .generic_predicates_for_param(*id)
-                        .into_iter()
-                        .map(|pred| pred.value.clone())
-                        .find_map(|pred| match pred {
-                            GenericPredicate::Implemented(tr) => Some(tr.clone()),
-                            _ => None,
-                        }),
+                    hir_def::generics::TypeParamProvenance::ArgumentImplTrait => {
+                        let predicates = db
+                            .generic_predicates_for_param(*id)
+                            .into_iter()
+                            .map(|pred| pred.value.clone())
+                            .collect_vec();
+
+                        Some(predicates)
+                    }
                     _ => None,
                 }
             }
@@ -926,6 +917,12 @@ impl Ty {
                     _ => None,
                 }
             }
+            Ty::Projection(projection_ty) => {
+                match projection_ty.associated_ty.lookup(db.upcast()).container {
+                    AssocContainerId::TraitId(trait_id) => Some(trait_id),
+                    _ => None,
+                }
+            }
             _ => None,
         }
     }
@@ -1104,10 +1101,10 @@ pub enum OpaqueTyId {
 
 #[derive(Clone, PartialEq, Eq, Debug, Hash)]
 pub struct ReturnTypeImplTraits {
-    pub impl_traits: Vec<ReturnTypeImplTrait>,
+    pub(crate) impl_traits: Vec<ReturnTypeImplTrait>,
 }
 
 #[derive(Clone, PartialEq, Eq, Debug, Hash)]
-pub struct ReturnTypeImplTrait {
+pub(crate) struct ReturnTypeImplTrait {
     pub bounds: Binders<Vec<GenericPredicate>>,
 }
diff --git a/crates/ra_ide/src/hover.rs b/crates/ra_ide/src/hover.rs
index c2909e200..d870e4cbc 100644
--- a/crates/ra_ide/src/hover.rs
+++ b/crates/ra_ide/src/hover.rs
@@ -248,8 +248,8 @@ fn goto_type_action(db: &RootDatabase, def: Definition) -> Option<HoverAction> {
                     push_new_def(adt.into());
                 } else if let Some(trait_) = t.as_dyn_trait() {
                     push_new_def(trait_.into());
-                } else if let Some(trait_) = t.as_impl_trait(db) {
-                    push_new_def(trait_.into());
+                } else if let Some(traits) = t.as_impl_traits(db) {
+                    traits.into_iter().for_each(|it| push_new_def(it.into()));
                 } else if let Some(trait_) = t.as_associated_type_parent_trait(db) {
                     push_new_def(trait_.into());
                 }
@@ -1735,6 +1735,176 @@ fn func(foo: i32) { if true { <|>foo; }; }
     }
 
     #[test]
+    fn test_hover_return_impl_traits_has_goto_type_action() {
+        let (_, actions) = check_hover_result(
+            "
+            //- /main.rs
+            trait Foo {}
+            trait Bar {}
+
+            fn foo() -> impl Foo + Bar {}
+
+            fn main() {
+                let s<|>t = foo();
+            }
+            ",
+            &["impl Foo + Bar"],
+        );
+        assert_debug_snapshot!(actions,
+            @r###"
+            [
+                GoToType(
+                    [
+                        HoverGotoTypeData {
+                            mod_path: "Foo",
+                            nav: NavigationTarget {
+                                file_id: FileId(
+                                    1,
+                                ),
+                                full_range: 0..12,
+                                name: "Foo",
+                                kind: TRAIT_DEF,
+                                focus_range: Some(
+                                    6..9,
+                                ),
+                                container_name: None,
+                                description: Some(
+                                    "trait Foo",
+                                ),
+                                docs: None,
+                            },
+                        },
+                        HoverGotoTypeData {
+                            mod_path: "Bar",
+                            nav: NavigationTarget {
+                                file_id: FileId(
+                                    1,
+                                ),
+                                full_range: 13..25,
+                                name: "Bar",
+                                kind: TRAIT_DEF,
+                                focus_range: Some(
+                                    19..22,
+                                ),
+                                container_name: None,
+                                description: Some(
+                                    "trait Bar",
+                                ),
+                                docs: None,
+                            },
+                        },
+                    ],
+                ),
+            ]
+            "###);
+    }
+
+    #[test]
+    fn test_hover_generic_return_impl_traits_has_goto_type_action() {
+        let (_, actions) = check_hover_result(
+            "
+            //- /main.rs
+            trait Foo<T> {}
+            trait Bar<T> {}
+            struct S1 {}
+            struct S2 {}
+
+            fn foo() -> impl Foo<S1> + Bar<S2> {}
+
+            fn main() {
+                let s<|>t = foo();
+            }
+            ",
+            &["impl Foo<S1> + Bar<S2>"],
+        );
+        assert_debug_snapshot!(actions,
+            @r###"
+            [
+                GoToType(
+                    [
+                        HoverGotoTypeData {
+                            mod_path: "Foo",
+                            nav: NavigationTarget {
+                                file_id: FileId(
+                                    1,
+                                ),
+                                full_range: 0..15,
+                                name: "Foo",
+                                kind: TRAIT_DEF,
+                                focus_range: Some(
+                                    6..9,
+                                ),
+                                container_name: None,
+                                description: Some(
+                                    "trait Foo",
+                                ),
+                                docs: None,
+                            },
+                        },
+                        HoverGotoTypeData {
+                            mod_path: "Bar",
+                            nav: NavigationTarget {
+                                file_id: FileId(
+                                    1,
+                                ),
+                                full_range: 16..31,
+                                name: "Bar",
+                                kind: TRAIT_DEF,
+                                focus_range: Some(
+                                    22..25,
+                                ),
+                                container_name: None,
+                                description: Some(
+                                    "trait Bar",
+                                ),
+                                docs: None,
+                            },
+                        },
+                        HoverGotoTypeData {
+                            mod_path: "S1",
+                            nav: NavigationTarget {
+                                file_id: FileId(
+                                    1,
+                                ),
+                                full_range: 32..44,
+                                name: "S1",
+                                kind: STRUCT_DEF,
+                                focus_range: Some(
+                                    39..41,
+                                ),
+                                container_name: None,
+                                description: Some(
+                                    "struct S1",
+                                ),
+                                docs: None,
+                            },
+                        },
+                        HoverGotoTypeData {
+                            mod_path: "S2",
+                            nav: NavigationTarget {
+                                file_id: FileId(
+                                    1,
+                                ),
+                                full_range: 45..57,
+                                name: "S2",
+                                kind: STRUCT_DEF,
+                                focus_range: Some(
+                                    52..54,
+                                ),
+                                container_name: None,
+                                description: Some(
+                                    "struct S2",
+                                ),
+                                docs: None,
+                            },
+                        },
+                    ],
+                ),
+            ]
+               "###);
+    }
+
+    #[test]
     fn test_hover_arg_impl_trait_has_goto_type_action() {
         let (_, actions) = check_hover_result(
             "
@@ -1775,6 +1945,87 @@ fn func(foo: i32) { if true { <|>foo; }; }
     }
 
     #[test]
+    fn test_hover_arg_impl_traits_has_goto_type_action() {
+        let (_, actions) = check_hover_result(
+            "
+            //- /lib.rs
+            trait Foo {}
+            trait Bar<T> {}
+            struct S{}
+
+            fn foo(ar<|>g: &impl Foo + Bar<S>) {}
+            ",
+            &["&impl Foo + Bar<S>"],
+        );
+        assert_debug_snapshot!(actions,
+            @r###"
+            [
+                GoToType(
+                    [
+                        HoverGotoTypeData {
+                            mod_path: "Foo",
+                            nav: NavigationTarget {
+                                file_id: FileId(
+                                    1,
+                                ),
+                                full_range: 0..12,
+                                name: "Foo",
+                                kind: TRAIT_DEF,
+                                focus_range: Some(
+                                    6..9,
+                                ),
+                                container_name: None,
+                                description: Some(
+                                    "trait Foo",
+                                ),
+                                docs: None,
+                            },
+                        },
+                        HoverGotoTypeData {
+                            mod_path: "Bar",
+                            nav: NavigationTarget {
+                                file_id: FileId(
+                                    1,
+                                ),
+                                full_range: 13..28,
+                                name: "Bar",
+                                kind: TRAIT_DEF,
+                                focus_range: Some(
+                                    19..22,
+                                ),
+                                container_name: None,
+                                description: Some(
+                                    "trait Bar",
+                                ),
+                                docs: None,
+                            },
+                        },
+                        HoverGotoTypeData {
+                            mod_path: "S",
+                            nav: NavigationTarget {
+                                file_id: FileId(
+                                    1,
+                                ),
+                                full_range: 29..39,
+                                name: "S",
+                                kind: STRUCT_DEF,
+                                focus_range: Some(
+                                    36..37,
+                                ),
+                                container_name: None,
+                                description: Some(
+                                    "struct S",
+                                ),
+                                docs: None,
+                            },
+                        },
+                    ],
+                ),
+            ]
+            "###);
+    }
+
+    #[test]
     fn test_hover_arg_generic_impl_trait_has_goto_type_action() {
         let (_, actions) = check_hover_result(
             "