From 022fbefffad0d7c402ac5607457f2828decb2188 Mon Sep 17 00:00:00 2001 From: vsrs Date: Thu, 11 Jun 2020 23:06:58 +0300 Subject: Apply suggestions from code review --- crates/ra_hir/src/code_model.rs | 85 ++++++++------ crates/ra_hir_ty/src/lib.rs | 53 ++++----- crates/ra_ide/src/hover.rs | 255 +++++++++++++++++++++++++++++++++++++++- 3 files changed, 328 insertions(+), 65 deletions(-) (limited to 'crates') 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 { - self.ty.value.impl_trait_ref(db).map(|it| it.trait_.into()) + pub fn as_impl_traits(&self, db: &dyn HirDatabase) -> Option> { + 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 { @@ -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 { + self.dyn_trait_ref().map(|it| it.trait_) + } + fn builtin_deref(&self) -> Option { 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 { - 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 { + pub fn impl_trait_bounds(&self, db: &dyn HirDatabase) -> Option> { 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, + pub(crate) impl_traits: Vec, } #[derive(Clone, PartialEq, Eq, Debug, Hash)] -pub struct ReturnTypeImplTrait { +pub(crate) struct ReturnTypeImplTrait { pub bounds: Binders>, } 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 { 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()); } @@ -1734,6 +1734,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 {} + trait Bar {} + struct S1 {} + struct S2 {} + + 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..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( @@ -1774,6 +1944,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 {} + struct S{} + + fn foo(ar<|>g: &impl Foo + Bar) {} + ", + &["&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..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( -- cgit v1.2.3