Apply suggestions from code review

Author: vsrs

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);
     }
 }
 

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>>,
 }