6 files changed, 85 insertions, 44 deletions

diff --git a/crates/ra_hir/src/generics.rs b/crates/ra_hir/src/generics.rs
index 84fe94289..4d82fe25f 100644
--- a/crates/ra_hir/src/generics.rs
+++ b/crates/ra_hir/src/generics.rs
@@ -56,7 +56,11 @@ impl GenericParams {
             GenericDef::Function(it) => generics.fill(&*it.source(db).1, start),
             GenericDef::Struct(it) => generics.fill(&*it.source(db).1, start),
             GenericDef::Enum(it) => generics.fill(&*it.source(db).1, start),
-            GenericDef::Trait(it) => generics.fill(&*it.source(db).1, start),
+            GenericDef::Trait(it) => {
+                // traits get the Self type as an implicit first type parameter
+                generics.params.push(GenericParam { idx: start, name: Name::self_type() });
+                generics.fill(&*it.source(db).1, start + 1);
+            }
             GenericDef::TypeAlias(it) => generics.fill(&*it.source(db).1, start),
             GenericDef::ImplBlock(it) => generics.fill(&*it.source(db).1, start),
         }
diff --git a/crates/ra_hir/src/impl_block.rs b/crates/ra_hir/src/impl_block.rs
index b306874cc..822a1a0db 100644
--- a/crates/ra_hir/src/impl_block.rs
+++ b/crates/ra_hir/src/impl_block.rs
@@ -10,15 +10,13 @@ use ra_syntax::{
 use crate::{
     Const, TypeAlias, Function, HirFileId,
     HirDatabase, DefDatabase,
-    ModuleDef, Trait, Resolution,
     type_ref::TypeRef,
     ids::LocationCtx,
     resolve::Resolver,
     ty::Ty, generics::GenericParams,
+    TraitRef, code_model_api::{Module, ModuleSource}
 };
 
-use crate::code_model_api::{Module, ModuleSource};
-
 #[derive(Debug, Default, PartialEq, Eq)]
 pub struct ImplSourceMap {
     map: ArenaMap<ImplId, AstPtr<ast::ImplBlock>>,
@@ -73,7 +71,7 @@ impl ImplBlock {
         self.module
     }
 
-    pub fn target_trait_ref(&self, db: &impl DefDatabase) -> Option<TypeRef> {
+    pub fn target_trait(&self, db: &impl DefDatabase) -> Option<TypeRef> {
         db.impls_in_module(self.module).impls[self.impl_id].target_trait().cloned()
     }
 
@@ -85,16 +83,8 @@ impl ImplBlock {
         Ty::from_hir(db, &self.resolver(db), &self.target_type(db))
     }
 
-    pub fn target_trait(&self, db: &impl HirDatabase) -> Option<Trait> {
-        if let Some(TypeRef::Path(path)) = self.target_trait_ref(db) {
-            let resolver = self.resolver(db);
-            if let Some(Resolution::Def(ModuleDef::Trait(tr))) =
-                resolver.resolve_path(db, &path).take_types()
-            {
-                return Some(tr);
-            }
-        }
-        None
+    pub fn target_trait_ref(&self, db: &impl HirDatabase) -> Option<TraitRef> {
+        TraitRef::from_hir(db, &self.resolver(db), &self.target_trait(db)?)
     }
 
     pub fn items(&self, db: &impl DefDatabase) -> Vec<ImplItem> {
diff --git a/crates/ra_hir/src/lib.rs b/crates/ra_hir/src/lib.rs
index a9db23060..c284d1693 100644
--- a/crates/ra_hir/src/lib.rs
+++ b/crates/ra_hir/src/lib.rs
@@ -61,7 +61,7 @@ pub use self::{
     source_id::{AstIdMap, ErasedFileAstId},
     ids::{HirFileId, MacroDefId, MacroCallId, MacroCallLoc},
     nameres::{PerNs, Namespace, ImportId},
-    ty::{Ty, ApplicationTy, TypeCtor, Substs, display::HirDisplay, CallableDef},
+    ty::{Ty, ApplicationTy, TypeCtor, TraitRef, Substs, display::HirDisplay, CallableDef},
     impl_block::{ImplBlock, ImplItem},
     docs::{Docs, Documentation},
     adt::AdtDef,
diff --git a/crates/ra_hir/src/ty/lower.rs b/crates/ra_hir/src/ty/lower.rs
index 003a89f0d..4523b3954 100644
--- a/crates/ra_hir/src/ty/lower.rs
+++ b/crates/ra_hir/src/ty/lower.rs
@@ -17,9 +17,9 @@ use crate::{
     resolve::{Resolver, Resolution},
     path::{ PathSegment, GenericArg},
     generics::GenericParams,
-    adt::VariantDef,
+    adt::VariantDef, Trait
 };
-use super::{Ty, primitive, FnSig, Substs, TypeCtor};
+use super::{Ty, primitive, FnSig, Substs, TypeCtor, TraitRef};
 
 impl Ty {
     pub(crate) fn from_hir(db: &impl HirDatabase, resolver: &Resolver, type_ref: &TypeRef) -> Self {
@@ -115,7 +115,6 @@ impl Ty {
         segment: &PathSegment,
         resolved: TypableDef,
     ) -> Substs {
-        let mut substs = Vec::new();
         let def_generics = match resolved {
             TypableDef::Function(func) => func.generic_params(db),
             TypableDef::Struct(s) => s.generic_params(db),
@@ -124,28 +123,7 @@ impl Ty {
             TypableDef::TypeAlias(t) => t.generic_params(db),
             TypableDef::Const(_) | TypableDef::Static(_) => GenericParams::default().into(),
         };
-        let parent_param_count = def_generics.count_parent_params();
-        substs.extend((0..parent_param_count).map(|_| Ty::Unknown));
-        if let Some(generic_args) = &segment.args_and_bindings {
-            // if args are provided, it should be all of them, but we can't rely on that
-            let param_count = def_generics.params.len();
-            for arg in generic_args.args.iter().take(param_count) {
-                match arg {
-                    GenericArg::Type(type_ref) => {
-                        let ty = Ty::from_hir(db, resolver, type_ref);
-                        substs.push(ty);
-                    }
-                }
-            }
-        }
-        // add placeholders for args that were not provided
-        // FIXME: handle defaults
-        let supplied_params = substs.len();
-        for _ in supplied_params..def_generics.count_params_including_parent() {
-            substs.push(Ty::Unknown);
-        }
-        assert_eq!(substs.len(), def_generics.count_params_including_parent());
-        Substs(substs.into())
+        substs_from_path_segment(db, resolver, segment, &def_generics, false)
     }
 
     /// Collect generic arguments from a path into a `Substs`. See also
@@ -185,6 +163,73 @@ impl Ty {
     }
 }
 
+pub(super) fn substs_from_path_segment(
+    db: &impl HirDatabase,
+    resolver: &Resolver,
+    segment: &PathSegment,
+    def_generics: &GenericParams,
+    add_self_param: bool,
+) -> Substs {
+    let mut substs = Vec::new();
+    let parent_param_count = def_generics.count_parent_params();
+    substs.extend((0..parent_param_count).map(|_| Ty::Unknown));
+    if add_self_param {
+        // FIXME this add_self_param argument is kind of a hack: Traits have the
+        // Self type as an implicit first type parameter, but it can't be
+        // actually provided in the type arguments
+        substs.push(Ty::Unknown);
+    }
+    if let Some(generic_args) = &segment.args_and_bindings {
+        // if args are provided, it should be all of them, but we can't rely on that
+        let param_count = def_generics.params.len();
+        for arg in generic_args.args.iter().take(param_count) {
+            match arg {
+                GenericArg::Type(type_ref) => {
+                    let ty = Ty::from_hir(db, resolver, type_ref);
+                    substs.push(ty);
+                }
+            }
+        }
+    }
+    // add placeholders for args that were not provided
+    // FIXME: handle defaults
+    let supplied_params = substs.len();
+    for _ in supplied_params..def_generics.count_params_including_parent() {
+        substs.push(Ty::Unknown);
+    }
+    assert_eq!(substs.len(), def_generics.count_params_including_parent());
+    Substs(substs.into())
+}
+
+impl TraitRef {
+    pub(crate) fn from_hir(
+        db: &impl HirDatabase,
+        resolver: &Resolver,
+        type_ref: &TypeRef,
+    ) -> Option<Self> {
+        let path = match type_ref {
+            TypeRef::Path(path) => path,
+            _ => return None,
+        };
+        let resolved = match resolver.resolve_path(db, &path).take_types()? {
+            Resolution::Def(ModuleDef::Trait(tr)) => tr,
+            _ => return None,
+        };
+        let substs = Self::substs_from_path(db, resolver, path, resolved);
+        Some(TraitRef { trait_: resolved, substs })
+    }
+
+    fn substs_from_path(
+        db: &impl HirDatabase,
+        resolver: &Resolver,
+        path: &Path,
+        resolved: Trait,
+    ) -> Substs {
+        let segment = path.segments.last().expect("path should have at least one segment");
+        substs_from_path_segment(db, resolver, segment, &resolved.generic_params(db), true)
+    }
+}
+
 /// Build the declared type of an item. This depends on the namespace; e.g. for
 /// `struct Foo(usize)`, we have two types: The type of the struct itself, and
 /// the constructor function `(usize) -> Foo` which lives in the values
diff --git a/crates/ra_hir/src/ty/method_resolution.rs b/crates/ra_hir/src/ty/method_resolution.rs
index bb23246a6..aac7d6384 100644
--- a/crates/ra_hir/src/ty/method_resolution.rs
+++ b/crates/ra_hir/src/ty/method_resolution.rs
@@ -72,9 +72,9 @@ impl CrateImplBlocks {
 
             let target_ty = impl_block.target_ty(db);
 
-            if let Some(tr) = impl_block.target_trait(db) {
+            if let Some(tr) = impl_block.target_trait_ref(db) {
                 self.impls_by_trait
-                    .entry(tr)
+                    .entry(tr.trait_)
                     .or_insert_with(Vec::new)
                     .push((module.module_id, impl_id));
             } else {
@@ -185,6 +185,8 @@ impl Ty {
         //    well (in fact, the 'implements' condition could just be considered a
         //    'where Self: Trait' clause)
         candidates.retain(|(t, _m)| {
+            // FIXME construct substs of the correct length for the trait
+            //  - check in rustc whether it does anything smarter than putting variables for everything
             let trait_ref = TraitRef { trait_: *t, substs: Substs::single(self.clone()) };
             db.implements(trait_ref)
         });
diff --git a/crates/ra_ide_api/src/goto_definition.rs b/crates/ra_ide_api/src/goto_definition.rs
index 517dffbca..40a2bd148 100644
--- a/crates/ra_ide_api/src/goto_definition.rs
+++ b/crates/ra_ide_api/src/goto_definition.rs
@@ -104,7 +104,7 @@ pub(crate) fn reference_definition(
                     }
                 }
                 hir::PathResolution::AssocItem(assoc) => {
-                    return Exact(NavigationTarget::from_impl_item(db, assoc))
+                    return Exact(NavigationTarget::from_impl_item(db, assoc));
                 }
             }
         }