From 877fda04c5a37241b09f155847d7e27b20875b63 Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Mon, 30 Dec 2019 13:53:43 +0100 Subject: Add test --- .../src/assists/add_missing_impl_members.rs | 23 ++++++++++++++++++++++ 1 file changed, 23 insertions(+) (limited to 'crates/ra_assists') diff --git a/crates/ra_assists/src/assists/add_missing_impl_members.rs b/crates/ra_assists/src/assists/add_missing_impl_members.rs index bc49e71fe..f0dfe7780 100644 --- a/crates/ra_assists/src/assists/add_missing_impl_members.rs +++ b/crates/ra_assists/src/assists/add_missing_impl_members.rs @@ -400,6 +400,29 @@ impl Foo for S { ) } + #[test] + fn test_qualify_path_1() { + check_assist( + add_missing_impl_members, + " +mod foo { + struct Bar; + trait Foo { fn foo(&self, bar: Bar); } +} +struct S; +impl foo::Foo for S { <|> }", + " +mod foo { + struct Bar; + trait Foo { fn foo(&self, bar: Bar); } +} +struct S; +impl foo::Foo for S { + <|>fn foo(&self, bar: foo::Bar) { unimplemented!() } +}", + ); + } + #[test] fn test_empty_trait() { check_assist_not_applicable( -- cgit v1.2.3 From 4d75430e912491c19fb1a7b1a95ee812f6a8a124 Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Tue, 31 Dec 2019 16:17:08 +0100 Subject: Qualify some paths in 'add missing impl members' --- .../src/assists/add_missing_impl_members.rs | 49 +++++++++++++++++++++- 1 file changed, 47 insertions(+), 2 deletions(-) (limited to 'crates/ra_assists') diff --git a/crates/ra_assists/src/assists/add_missing_impl_members.rs b/crates/ra_assists/src/assists/add_missing_impl_members.rs index f0dfe7780..2b0726869 100644 --- a/crates/ra_assists/src/assists/add_missing_impl_members.rs +++ b/crates/ra_assists/src/assists/add_missing_impl_members.rs @@ -139,6 +139,12 @@ fn add_missing_impl_members_inner( ctx.add_assist(AssistId(assist_id), label, |edit| { let n_existing_items = impl_item_list.impl_items().count(); + let module = hir::SourceAnalyzer::new( + db, + hir::InFile::new(file_id.into(), impl_node.syntax()), + None, + ) + .module(); let substs = get_syntactic_substs(impl_node).unwrap_or_default(); let generic_def: hir::GenericDef = trait_.into(); let substs_by_param: HashMap<_, _> = generic_def @@ -150,6 +156,10 @@ fn add_missing_impl_members_inner( .collect(); let items = missing_items .into_iter() + .map(|it| match module { + Some(module) => qualify_paths(db, hir::InFile::new(file_id.into(), it), module), + None => it, + }) .map(|it| { substitute_type_params(db, hir::InFile::new(file_id.into(), it), &substs_by_param) }) @@ -227,6 +237,41 @@ fn substitute_type_params( } } +use hir::PathResolution; + +// TODO handle partial paths, with generic args +// TODO handle value ns? + +fn qualify_paths(db: &impl HirDatabase, node: hir::InFile, from: hir::Module) -> N { + let path_replacements = node + .value + .syntax() + .descendants() + .filter_map(ast::Path::cast) + .filter_map(|p| { + let analyzer = hir::SourceAnalyzer::new(db, node.with_value(p.syntax()), None); + let resolution = analyzer.resolve_path(db, &p)?; + match resolution { + PathResolution::Def(def) => { + let found_path = from.find_path(db, def)?; + Some((p, found_path.to_ast())) + } + PathResolution::Local(_) + | PathResolution::TypeParam(_) + | PathResolution::SelfType(_) => None, + PathResolution::Macro(_) => None, + PathResolution::AssocItem(_) => None, + } + }) + .collect::>(); + + if path_replacements.is_empty() { + node.value + } else { + edit::replace_descendants(&node.value, path_replacements.into_iter()) + } +} + /// Given an `ast::ImplBlock`, resolves the target trait (the one being /// implemented) to a `ast::TraitDef`. fn resolve_target_trait_def( @@ -406,14 +451,14 @@ impl Foo for S { add_missing_impl_members, " mod foo { - struct Bar; + pub struct Bar; trait Foo { fn foo(&self, bar: Bar); } } struct S; impl foo::Foo for S { <|> }", " mod foo { - struct Bar; + pub struct Bar; trait Foo { fn foo(&self, bar: Bar); } } struct S; -- cgit v1.2.3 From 5cb1f7132277e16ec4eecafbc274563c4d27158e Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Wed, 1 Jan 2020 23:08:22 +0100 Subject: More failing tests --- .../src/assists/add_missing_impl_members.rs | 127 ++++++++++++++++++++- 1 file changed, 126 insertions(+), 1 deletion(-) (limited to 'crates/ra_assists') diff --git a/crates/ra_assists/src/assists/add_missing_impl_members.rs b/crates/ra_assists/src/assists/add_missing_impl_members.rs index 2b0726869..dd62b1b78 100644 --- a/crates/ra_assists/src/assists/add_missing_impl_members.rs +++ b/crates/ra_assists/src/assists/add_missing_impl_members.rs @@ -239,9 +239,11 @@ fn substitute_type_params( use hir::PathResolution; -// TODO handle partial paths, with generic args +// TODO handle generic args +// TODO handle associated item paths // TODO handle value ns? +// FIXME extract this to a general utility as well fn qualify_paths(db: &impl HirDatabase, node: hir::InFile, from: hir::Module) -> N { let path_replacements = node .value @@ -249,6 +251,10 @@ fn qualify_paths(db: &impl HirDatabase, node: hir::InFile, from: .descendants() .filter_map(ast::Path::cast) .filter_map(|p| { + if p.segment().and_then(|s| s.param_list()).is_some() { + // don't try to qualify `Fn(Foo) -> Bar` paths, they are in prelude anyway + return None; + } let analyzer = hir::SourceAnalyzer::new(db, node.with_value(p.syntax()), None); let resolution = analyzer.resolve_path(db, &p)?; match resolution { @@ -468,6 +474,125 @@ impl foo::Foo for S { ); } + #[test] + fn test_qualify_path_generic() { + check_assist( + add_missing_impl_members, + " +mod foo { + pub struct Bar; + trait Foo { fn foo(&self, bar: Bar); } +} +struct S; +impl foo::Foo for S { <|> }", + " +mod foo { + pub struct Bar; + trait Foo { fn foo(&self, bar: Bar); } +} +struct S; +impl foo::Foo for S { + <|>fn foo(&self, bar: foo::Bar) { unimplemented!() } +}", + ); + } + + #[test] + fn test_qualify_path_and_substitute_param() { + check_assist( + add_missing_impl_members, + " +mod foo { + pub struct Bar; + trait Foo { fn foo(&self, bar: Bar); } +} +struct S; +impl foo::Foo for S { <|> }", + " +mod foo { + pub struct Bar; + trait Foo { fn foo(&self, bar: Bar); } +} +struct S; +impl foo::Foo for S { + <|>fn foo(&self, bar: foo::Bar) { unimplemented!() } +}", + ); + } + + #[test] + fn test_qualify_path_associated_item() { + check_assist( + add_missing_impl_members, + " +mod foo { + pub struct Bar; + impl Bar { type Assoc = u32; } + trait Foo { fn foo(&self, bar: Bar::Assoc); } +} +struct S; +impl foo::Foo for S { <|> }", + " +mod foo { + pub struct Bar; + impl Bar { type Assoc = u32; } + trait Foo { fn foo(&self, bar: Bar::Assoc); } +} +struct S; +impl foo::Foo for S { + <|>fn foo(&self, bar: foo::Bar::Assoc) { unimplemented!() } +}", + ); + } + + #[test] + fn test_qualify_path_nested() { + check_assist( + add_missing_impl_members, + " +mod foo { + pub struct Bar; + pub struct Baz; + trait Foo { fn foo(&self, bar: Bar); } +} +struct S; +impl foo::Foo for S { <|> }", + " +mod foo { + pub struct Bar; + pub struct Baz; + trait Foo { fn foo(&self, bar: Bar); } +} +struct S; +impl foo::Foo for S { + <|>fn foo(&self, bar: foo::Bar) { unimplemented!() } +}", + ); + } + + #[test] + fn test_qualify_path_fn_trait_notation() { + check_assist( + add_missing_impl_members, + " +mod foo { + pub trait Fn { type Output; } + trait Foo { fn foo(&self, bar: dyn Fn(u32) -> i32); } +} +struct S; +impl foo::Foo for S { <|> }", + " +mod foo { + pub trait Fn { type Output; } + trait Foo { fn foo(&self, bar: dyn Fn(u32) -> i32); } +} +struct S; +impl foo::Foo for S { + <|>fn foo(&self, bar: dyn Fn(u32) -> i32) { unimplemented!() } +}", + ); + } + #[test] fn test_empty_trait() { check_assist_not_applicable( -- cgit v1.2.3 From 4545f289a991ec3888896aac0e0bcbfac9061e80 Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Fri, 3 Jan 2020 19:58:56 +0100 Subject: Handle type args --- .../src/assists/add_missing_impl_members.rs | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) (limited to 'crates/ra_assists') diff --git a/crates/ra_assists/src/assists/add_missing_impl_members.rs b/crates/ra_assists/src/assists/add_missing_impl_members.rs index dd62b1b78..7b50fb422 100644 --- a/crates/ra_assists/src/assists/add_missing_impl_members.rs +++ b/crates/ra_assists/src/assists/add_missing_impl_members.rs @@ -156,13 +156,13 @@ fn add_missing_impl_members_inner( .collect(); let items = missing_items .into_iter() + .map(|it| { + substitute_type_params(db, hir::InFile::new(file_id.into(), it), &substs_by_param) + }) .map(|it| match module { Some(module) => qualify_paths(db, hir::InFile::new(file_id.into(), it), module), None => it, }) - .map(|it| { - substitute_type_params(db, hir::InFile::new(file_id.into(), it), &substs_by_param) - }) .map(|it| match it { ast::ImplItem::FnDef(def) => ast::ImplItem::FnDef(add_body(def)), _ => it, @@ -239,11 +239,9 @@ fn substitute_type_params( use hir::PathResolution; -// TODO handle generic args -// TODO handle associated item paths -// TODO handle value ns? - // FIXME extract this to a general utility as well +// FIXME handle value ns? +// FIXME this doesn't 'commute' with `substitute_type_params`, since type params in newly generated type arg lists don't resolve. Currently we can avoid this problem, but it's worth thinking about a solution fn qualify_paths(db: &impl HirDatabase, node: hir::InFile, from: hir::Module) -> N { let path_replacements = node .value @@ -255,12 +253,17 @@ fn qualify_paths(db: &impl HirDatabase, node: hir::InFile, from: // don't try to qualify `Fn(Foo) -> Bar` paths, they are in prelude anyway return None; } + // FIXME check if some ancestor is already being replaced, if so skip this let analyzer = hir::SourceAnalyzer::new(db, node.with_value(p.syntax()), None); let resolution = analyzer.resolve_path(db, &p)?; match resolution { PathResolution::Def(def) => { let found_path = from.find_path(db, def)?; - Some((p, found_path.to_ast())) + let args = p + .segment() + .and_then(|s| s.type_arg_list()) + .map(|arg_list| qualify_paths(db, node.with_value(arg_list), from)); + Some((p, make::path_with_type_arg_list(found_path.to_ast(), args))) } PathResolution::Local(_) | PathResolution::TypeParam(_) @@ -535,7 +538,7 @@ impl foo::Foo for S { <|> }", " mod foo { pub struct Bar; - impl Bar { type Assoc = u32; } + impl Bar { type Assoc = u32; } trait Foo { fn foo(&self, bar: Bar::Assoc); } } struct S; -- cgit v1.2.3 From def124e932f02f5961d26af6cc03f696f389205f Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Fri, 3 Jan 2020 23:05:58 +0100 Subject: Fix file ID when qualifying paths; add another failing test --- .../src/assists/add_missing_impl_members.rs | 34 ++++++++++++++++++++-- 1 file changed, 31 insertions(+), 3 deletions(-) (limited to 'crates/ra_assists') diff --git a/crates/ra_assists/src/assists/add_missing_impl_members.rs b/crates/ra_assists/src/assists/add_missing_impl_members.rs index 7b50fb422..22f1157cc 100644 --- a/crates/ra_assists/src/assists/add_missing_impl_members.rs +++ b/crates/ra_assists/src/assists/add_missing_impl_members.rs @@ -134,8 +134,9 @@ fn add_missing_impl_members_inner( return None; } - let file_id = ctx.frange.file_id; let db = ctx.db; + let file_id = ctx.frange.file_id; + let trait_file_id = trait_.source(db).file_id; ctx.add_assist(AssistId(assist_id), label, |edit| { let n_existing_items = impl_item_list.impl_items().count(); @@ -157,10 +158,10 @@ fn add_missing_impl_members_inner( let items = missing_items .into_iter() .map(|it| { - substitute_type_params(db, hir::InFile::new(file_id.into(), it), &substs_by_param) + substitute_type_params(db, hir::InFile::new(trait_file_id, it), &substs_by_param) }) .map(|it| match module { - Some(module) => qualify_paths(db, hir::InFile::new(file_id.into(), it), module), + Some(module) => qualify_paths(db, hir::InFile::new(trait_file_id, it), module), None => it, }) .map(|it| match it { @@ -259,6 +260,7 @@ fn qualify_paths(db: &impl HirDatabase, node: hir::InFile, from: match resolution { PathResolution::Def(def) => { let found_path = from.find_path(db, def)?; + // TODO fix type arg replacements being qualified let args = p .segment() .and_then(|s| s.type_arg_list()) @@ -523,6 +525,32 @@ impl foo::Foo for S { ); } + #[test] + fn test_substitute_param_no_qualify() { + // when substituting params, the substituted param should not be qualified! + check_assist( + add_missing_impl_members, + " +mod foo { + trait Foo { fn foo(&self, bar: T); } + pub struct Param; +} +struct Param; +struct S; +impl foo::Foo for S { <|> }", + " +mod foo { + trait Foo { fn foo(&self, bar: T); } + pub struct Param; +} +struct Param; +struct S; +impl foo::Foo for S { + <|>fn foo(&self, bar: Param) { unimplemented!() } +}", + ); + } + #[test] fn test_qualify_path_associated_item() { check_assist( -- cgit v1.2.3 From 12905e5b58f22df026ef30afa6f0bdf7319cbddd Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Sun, 5 Jan 2020 21:32:18 +0100 Subject: Some more refactoring --- crates/ra_assists/src/assists/add_missing_impl_members.rs | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) (limited to 'crates/ra_assists') diff --git a/crates/ra_assists/src/assists/add_missing_impl_members.rs b/crates/ra_assists/src/assists/add_missing_impl_members.rs index 22f1157cc..942b34dc1 100644 --- a/crates/ra_assists/src/assists/add_missing_impl_members.rs +++ b/crates/ra_assists/src/assists/add_missing_impl_members.rs @@ -207,25 +207,23 @@ fn get_syntactic_substs(impl_block: ast::ImplBlock) -> Option> } // FIXME: This should be a general utility (not even just for assists) -fn substitute_type_params( +fn substitute_type_params( db: &impl HirDatabase, node: hir::InFile, substs: &HashMap, ) -> N { let type_param_replacements = node - .value - .syntax() - .descendants() - .filter_map(ast::TypeRef::cast) + .clone() + .descendants::() .filter_map(|n| { - let path = match &n { + let path = match &n.value { ast::TypeRef::PathType(path_type) => path_type.path()?, _ => return None, }; - let analyzer = hir::SourceAnalyzer::new(db, node.with_value(n.syntax()), None); + let analyzer = hir::SourceAnalyzer::new(db, n.syntax(), None); let resolution = analyzer.resolve_path(db, &path)?; match resolution { - hir::PathResolution::TypeParam(tp) => Some((n, substs.get(&tp)?.clone())), + hir::PathResolution::TypeParam(tp) => Some((n.value, substs.get(&tp)?.clone())), _ => None, } }) -- cgit v1.2.3 From 15fc643e05bf8273e378243edbfb3be7aea7ce11 Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Fri, 10 Jan 2020 18:26:18 +0100 Subject: Fix ordering problem between qualifying paths and substituting params --- .../src/assists/add_missing_impl_members.rs | 121 +------------- crates/ra_assists/src/ast_transform.rs | 178 +++++++++++++++++++++ crates/ra_assists/src/lib.rs | 1 + 3 files changed, 187 insertions(+), 113 deletions(-) create mode 100644 crates/ra_assists/src/ast_transform.rs (limited to 'crates/ra_assists') diff --git a/crates/ra_assists/src/assists/add_missing_impl_members.rs b/crates/ra_assists/src/assists/add_missing_impl_members.rs index 942b34dc1..bf1136193 100644 --- a/crates/ra_assists/src/assists/add_missing_impl_members.rs +++ b/crates/ra_assists/src/assists/add_missing_impl_members.rs @@ -1,12 +1,13 @@ -use std::collections::HashMap; - -use hir::{db::HirDatabase, HasSource}; +use hir::{db::HirDatabase, HasSource, InFile}; use ra_syntax::{ ast::{self, edit, make, AstNode, NameOwner}, SmolStr, }; -use crate::{Assist, AssistCtx, AssistId}; +use crate::{ + ast_transform::{self, AstTransform, QualifyPaths, SubstituteTypeParams}, + Assist, AssistCtx, AssistId, +}; #[derive(PartialEq)] enum AddMissingImplMembersMode { @@ -146,24 +147,11 @@ fn add_missing_impl_members_inner( None, ) .module(); - let substs = get_syntactic_substs(impl_node).unwrap_or_default(); - let generic_def: hir::GenericDef = trait_.into(); - let substs_by_param: HashMap<_, _> = generic_def - .params(db) - .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()) - .collect(); + let ast_transform = QualifyPaths::new(db, module) + .or(SubstituteTypeParams::for_trait_impl(db, trait_, impl_node)); let items = missing_items .into_iter() - .map(|it| { - substitute_type_params(db, hir::InFile::new(trait_file_id, it), &substs_by_param) - }) - .map(|it| match module { - Some(module) => qualify_paths(db, hir::InFile::new(trait_file_id, it), module), - None => it, - }) + .map(|it| ast_transform::apply(&*ast_transform, InFile::new(trait_file_id, it))) .map(|it| match it { ast::ImplItem::FnDef(def) => ast::ImplItem::FnDef(add_body(def)), _ => it, @@ -188,99 +176,6 @@ fn add_body(fn_def: ast::FnDef) -> ast::FnDef { } } -// FIXME: It would probably be nicer if we could get this via HIR (i.e. get the -// trait ref, and then go from the types in the substs back to the syntax) -// FIXME: This should be a general utility (not even just for assists) -fn get_syntactic_substs(impl_block: ast::ImplBlock) -> Option> { - let target_trait = impl_block.target_trait()?; - let path_type = match target_trait { - ast::TypeRef::PathType(path) => path, - _ => return None, - }; - let type_arg_list = path_type.path()?.segment()?.type_arg_list()?; - let mut result = Vec::new(); - for type_arg in type_arg_list.type_args() { - let type_arg: ast::TypeArg = type_arg; - result.push(type_arg.type_ref()?); - } - Some(result) -} - -// FIXME: This should be a general utility (not even just for assists) -fn substitute_type_params( - db: &impl HirDatabase, - node: hir::InFile, - substs: &HashMap, -) -> N { - let type_param_replacements = node - .clone() - .descendants::() - .filter_map(|n| { - let path = match &n.value { - ast::TypeRef::PathType(path_type) => path_type.path()?, - _ => return None, - }; - let analyzer = hir::SourceAnalyzer::new(db, n.syntax(), None); - let resolution = analyzer.resolve_path(db, &path)?; - match resolution { - hir::PathResolution::TypeParam(tp) => Some((n.value, substs.get(&tp)?.clone())), - _ => None, - } - }) - .collect::>(); - - if type_param_replacements.is_empty() { - node.value - } else { - edit::replace_descendants(&node.value, type_param_replacements.into_iter()) - } -} - -use hir::PathResolution; - -// FIXME extract this to a general utility as well -// FIXME handle value ns? -// FIXME this doesn't 'commute' with `substitute_type_params`, since type params in newly generated type arg lists don't resolve. Currently we can avoid this problem, but it's worth thinking about a solution -fn qualify_paths(db: &impl HirDatabase, node: hir::InFile, from: hir::Module) -> N { - let path_replacements = node - .value - .syntax() - .descendants() - .filter_map(ast::Path::cast) - .filter_map(|p| { - if p.segment().and_then(|s| s.param_list()).is_some() { - // don't try to qualify `Fn(Foo) -> Bar` paths, they are in prelude anyway - return None; - } - // FIXME check if some ancestor is already being replaced, if so skip this - let analyzer = hir::SourceAnalyzer::new(db, node.with_value(p.syntax()), None); - let resolution = analyzer.resolve_path(db, &p)?; - match resolution { - PathResolution::Def(def) => { - let found_path = from.find_path(db, def)?; - // TODO fix type arg replacements being qualified - let args = p - .segment() - .and_then(|s| s.type_arg_list()) - .map(|arg_list| qualify_paths(db, node.with_value(arg_list), from)); - Some((p, make::path_with_type_arg_list(found_path.to_ast(), args))) - } - PathResolution::Local(_) - | PathResolution::TypeParam(_) - | PathResolution::SelfType(_) => None, - PathResolution::Macro(_) => None, - PathResolution::AssocItem(_) => None, - } - }) - .collect::>(); - - if path_replacements.is_empty() { - node.value - } else { - edit::replace_descendants(&node.value, path_replacements.into_iter()) - } -} - /// Given an `ast::ImplBlock`, resolves the target trait (the one being /// implemented) to a `ast::TraitDef`. fn resolve_target_trait_def( diff --git a/crates/ra_assists/src/ast_transform.rs b/crates/ra_assists/src/ast_transform.rs new file mode 100644 index 000000000..846661587 --- /dev/null +++ b/crates/ra_assists/src/ast_transform.rs @@ -0,0 +1,178 @@ +//! `AstTransformer`s are functions that replace nodes in an AST and can be easily combined. +use std::collections::HashMap; + +use hir::{db::HirDatabase, InFile, PathResolution}; +use ra_syntax::ast::{self, make, AstNode}; + +pub trait AstTransform<'a> { + fn get_substitution( + &self, + node: InFile<&ra_syntax::SyntaxNode>, + ) -> Option; + + fn chain_before(self, other: Box + 'a>) -> Box + 'a>; + fn or + 'a>(self, other: T) -> Box + 'a> + where + Self: Sized + 'a, + { + self.chain_before(Box::new(other)) + } +} + +struct NullTransformer; + +impl<'a> AstTransform<'a> for NullTransformer { + fn get_substitution( + &self, + _node: InFile<&ra_syntax::SyntaxNode>, + ) -> Option { + None + } + fn chain_before(self, other: Box + 'a>) -> Box + 'a> { + other + } +} + +pub struct SubstituteTypeParams<'a, DB: HirDatabase> { + db: &'a DB, + substs: HashMap, + previous: Box + 'a>, +} + +impl<'a, DB: HirDatabase> SubstituteTypeParams<'a, DB> { + pub fn for_trait_impl( + db: &'a DB, + trait_: hir::Trait, + impl_block: ast::ImplBlock, + ) -> SubstituteTypeParams<'a, DB> { + let substs = get_syntactic_substs(impl_block).unwrap_or_default(); + let generic_def: hir::GenericDef = trait_.into(); + let substs_by_param: HashMap<_, _> = generic_def + .params(db) + .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()) + .collect(); + return SubstituteTypeParams { + db, + substs: substs_by_param, + previous: Box::new(NullTransformer), + }; + + fn get_syntactic_substs(impl_block: ast::ImplBlock) -> Option> { + let target_trait = impl_block.target_trait()?; + let path_type = match target_trait { + ast::TypeRef::PathType(path) => path, + _ => return None, + }; + let type_arg_list = path_type.path()?.segment()?.type_arg_list()?; + let mut result = Vec::new(); + for type_arg in type_arg_list.type_args() { + let type_arg: ast::TypeArg = type_arg; + result.push(type_arg.type_ref()?); + } + Some(result) + } + } + fn get_substitution_inner( + &self, + node: InFile<&ra_syntax::SyntaxNode>, + ) -> Option { + let type_ref = ast::TypeRef::cast(node.value.clone())?; + let path = match &type_ref { + ast::TypeRef::PathType(path_type) => path_type.path()?, + _ => return None, + }; + let analyzer = hir::SourceAnalyzer::new(self.db, node, None); + let resolution = analyzer.resolve_path(self.db, &path)?; + match resolution { + hir::PathResolution::TypeParam(tp) => Some(self.substs.get(&tp)?.syntax().clone()), + _ => None, + } + } +} + +impl<'a, DB: HirDatabase> AstTransform<'a> for SubstituteTypeParams<'a, DB> { + fn get_substitution( + &self, + node: InFile<&ra_syntax::SyntaxNode>, + ) -> Option { + self.get_substitution_inner(node).or_else(|| self.previous.get_substitution(node)) + } + fn chain_before(self, other: Box + 'a>) -> Box + 'a> { + Box::new(SubstituteTypeParams { previous: other, ..self }) + } +} + +pub struct QualifyPaths<'a, DB: HirDatabase> { + db: &'a DB, + from: Option, + previous: Box + 'a>, +} + +impl<'a, DB: HirDatabase> QualifyPaths<'a, DB> { + pub fn new(db: &'a DB, from: Option) -> Self { + Self { db, from, previous: Box::new(NullTransformer) } + } + + fn get_substitution_inner( + &self, + node: InFile<&ra_syntax::SyntaxNode>, + ) -> Option { + // FIXME handle value ns? + let from = self.from?; + let p = ast::Path::cast(node.value.clone())?; + if p.segment().and_then(|s| s.param_list()).is_some() { + // don't try to qualify `Fn(Foo) -> Bar` paths, they are in prelude anyway + return None; + } + let analyzer = hir::SourceAnalyzer::new(self.db, node, None); + let resolution = analyzer.resolve_path(self.db, &p)?; + match resolution { + PathResolution::Def(def) => { + let found_path = from.find_path(self.db, def)?; + let args = p + .segment() + .and_then(|s| s.type_arg_list()) + .map(|arg_list| apply(self, node.with_value(arg_list))); + Some(make::path_with_type_arg_list(found_path.to_ast(), args).syntax().clone()) + } + PathResolution::Local(_) + | PathResolution::TypeParam(_) + | PathResolution::SelfType(_) => None, + PathResolution::Macro(_) => None, + PathResolution::AssocItem(_) => None, + } + } +} + +pub fn apply<'a, N: AstNode>(transformer: &dyn AstTransform<'a>, node: InFile) -> N { + let syntax = node.value.syntax(); + let result = ra_syntax::algo::replace_descendants(syntax, &|element| match element { + ra_syntax::SyntaxElement::Node(n) => { + let replacement = transformer.get_substitution(node.with_value(&n))?; + Some(replacement.into()) + } + _ => None, + }); + N::cast(result).unwrap() +} + +impl<'a, DB: HirDatabase> AstTransform<'a> for QualifyPaths<'a, DB> { + fn get_substitution( + &self, + node: InFile<&ra_syntax::SyntaxNode>, + ) -> Option { + self.get_substitution_inner(node).or_else(|| self.previous.get_substitution(node)) + } + fn chain_before(self, other: Box + 'a>) -> Box + 'a> { + Box::new(QualifyPaths { previous: other, ..self }) + } +} + +// FIXME: It would probably be nicer if we could get this via HIR (i.e. get the +// trait ref, and then go from the types in the substs back to the syntax) +// FIXME: This should be a general utility (not even just for assists) + +// FIXME: This should be a general utility (not even just for assists) diff --git a/crates/ra_assists/src/lib.rs b/crates/ra_assists/src/lib.rs index 98fb20b22..712ff6f6a 100644 --- a/crates/ra_assists/src/lib.rs +++ b/crates/ra_assists/src/lib.rs @@ -11,6 +11,7 @@ mod marks; mod doc_tests; #[cfg(test)] mod test_db; +pub mod ast_transform; use hir::db::HirDatabase; use ra_db::FileRange; -- cgit v1.2.3 From 4496e2a06a91e5825f355ce730af802643e8018a Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Fri, 10 Jan 2020 18:40:45 +0100 Subject: Apply review suggestions --- crates/ra_assists/src/ast_transform.rs | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) (limited to 'crates/ra_assists') diff --git a/crates/ra_assists/src/ast_transform.rs b/crates/ra_assists/src/ast_transform.rs index 846661587..ace59f290 100644 --- a/crates/ra_assists/src/ast_transform.rs +++ b/crates/ra_assists/src/ast_transform.rs @@ -60,6 +60,8 @@ impl<'a, DB: HirDatabase> SubstituteTypeParams<'a, DB> { previous: Box::new(NullTransformer), }; + // FIXME: It would probably be nicer if we could get this via HIR (i.e. get the + // trait ref, and then go from the types in the substs back to the syntax) fn get_syntactic_substs(impl_block: ast::ImplBlock) -> Option> { let target_trait = impl_block.target_trait()?; let path_type = match target_trait { @@ -131,12 +133,12 @@ impl<'a, DB: HirDatabase> QualifyPaths<'a, DB> { let resolution = analyzer.resolve_path(self.db, &p)?; match resolution { PathResolution::Def(def) => { - let found_path = from.find_path(self.db, def)?; + let found_path = from.find_use_path(self.db, def)?; let args = p .segment() .and_then(|s| s.type_arg_list()) .map(|arg_list| apply(self, node.with_value(arg_list))); - Some(make::path_with_type_arg_list(found_path.to_ast(), args).syntax().clone()) + Some(make::path_with_type_arg_list(path_to_ast(found_path), args).syntax().clone()) } PathResolution::Local(_) | PathResolution::TypeParam(_) @@ -171,8 +173,7 @@ impl<'a, DB: HirDatabase> AstTransform<'a> for QualifyPaths<'a, DB> { } } -// FIXME: It would probably be nicer if we could get this via HIR (i.e. get the -// trait ref, and then go from the types in the substs back to the syntax) -// FIXME: This should be a general utility (not even just for assists) - -// FIXME: This should be a general utility (not even just for assists) +fn path_to_ast(path: hir::ModPath) -> ast::Path { + let parse = ast::SourceFile::parse(&path.to_string()); + parse.tree().syntax().descendants().find_map(ast::Path::cast).unwrap() +} -- cgit v1.2.3 From ccb75f7c979b56bc62b61fadd81903e11a7f5d74 Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Fri, 10 Jan 2020 18:59:57 +0100 Subject: Use FxHashMap --- crates/ra_assists/Cargo.toml | 1 + crates/ra_assists/src/ast_transform.rs | 6 +++--- 2 files changed, 4 insertions(+), 3 deletions(-) (limited to 'crates/ra_assists') diff --git a/crates/ra_assists/Cargo.toml b/crates/ra_assists/Cargo.toml index 434e6656c..50be8d9bc 100644 --- a/crates/ra_assists/Cargo.toml +++ b/crates/ra_assists/Cargo.toml @@ -10,6 +10,7 @@ doctest = false [dependencies] format-buf = "1.0.0" join_to_string = "0.1.3" +rustc-hash = "1.0" itertools = "0.8.0" ra_syntax = { path = "../ra_syntax" } diff --git a/crates/ra_assists/src/ast_transform.rs b/crates/ra_assists/src/ast_transform.rs index ace59f290..cbddc50ac 100644 --- a/crates/ra_assists/src/ast_transform.rs +++ b/crates/ra_assists/src/ast_transform.rs @@ -1,5 +1,5 @@ //! `AstTransformer`s are functions that replace nodes in an AST and can be easily combined. -use std::collections::HashMap; +use rustc_hash::FxHashMap; use hir::{db::HirDatabase, InFile, PathResolution}; use ra_syntax::ast::{self, make, AstNode}; @@ -35,7 +35,7 @@ impl<'a> AstTransform<'a> for NullTransformer { pub struct SubstituteTypeParams<'a, DB: HirDatabase> { db: &'a DB, - substs: HashMap, + substs: FxHashMap, previous: Box + 'a>, } @@ -47,7 +47,7 @@ impl<'a, DB: HirDatabase> SubstituteTypeParams<'a, DB> { ) -> SubstituteTypeParams<'a, DB> { let substs = get_syntactic_substs(impl_block).unwrap_or_default(); let generic_def: hir::GenericDef = trait_.into(); - let substs_by_param: HashMap<_, _> = generic_def + let substs_by_param: FxHashMap<_, _> = generic_def .params(db) .into_iter() // this is a trait impl, so we need to skip the first type parameter -- this is a bit hacky -- cgit v1.2.3