From 7e6eb67f0d5ccdff3dc98a5c67eb0ac7b98a3aa1 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Tue, 10 Aug 2021 14:39:56 +0200 Subject: [PATCH] Substitute generic types in inline_call --- .../ide_assists/src/handlers/inline_call.rs | 59 ++++++++++++-- crates/ide_assists/src/utils.rs | 8 +- .../src/completions/trait_impl.rs | 11 +-- crates/ide_db/src/path_transform.rs | 78 +++++++++++++------ 4 files changed, 116 insertions(+), 40 deletions(-) diff --git a/crates/ide_assists/src/handlers/inline_call.rs b/crates/ide_assists/src/handlers/inline_call.rs index c60dd81eb06..90d83501f2c 100644 --- a/crates/ide_assists/src/handlers/inline_call.rs +++ b/crates/ide_assists/src/handlers/inline_call.rs @@ -1,6 +1,6 @@ use ast::make; use hir::{HasSource, PathResolution, TypeInfo}; -use ide_db::{defs::Definition, search::FileReference}; +use ide_db::{defs::Definition, path_transform::PathTransform, search::FileReference}; use itertools::izip; use syntax::{ ast::{self, edit::AstNodeEdit, ArgListOwner}, @@ -34,7 +34,7 @@ use crate::{ // } // ``` pub(crate) fn inline_call(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { - let (label, function, arguments, expr) = + let (label, function, arguments, generic_arg_list, expr) = if let Some(path_expr) = ctx.find_node_at_offset::() { let call = path_expr.syntax().parent().and_then(ast::CallExpr::cast)?; let path = path_expr.path()?; @@ -48,6 +48,7 @@ pub(crate) fn inline_call(acc: &mut Assists, ctx: &AssistContext) -> Option<()> format!("Inline `{}`", path), function, call.arg_list()?.args().collect(), + path.segment().and_then(|it| it.generic_arg_list()), ast::Expr::CallExpr(call), ) } else { @@ -57,10 +58,16 @@ pub(crate) fn inline_call(acc: &mut Assists, ctx: &AssistContext) -> Option<()> let function = ctx.sema.resolve_method_call(&call)?; let mut arguments = vec![receiver]; arguments.extend(call.arg_list()?.args()); - (format!("Inline `{}`", name_ref), function, arguments, ast::Expr::MethodCallExpr(call)) + ( + format!("Inline `{}`", name_ref), + function, + arguments, + call.generic_arg_list(), + ast::Expr::MethodCallExpr(call), + ) }; - inline_(acc, ctx, label, function, arguments, expr) + inline_(acc, ctx, label, function, arguments, expr, generic_arg_list) } pub(crate) fn inline_( @@ -70,6 +77,7 @@ pub(crate) fn inline_( function: hir::Function, arg_list: Vec, expr: ast::Expr, + generic_arg_list: Option, ) -> Option<()> { let hir::InFile { value: function_source, file_id } = function.source(ctx.db())?; let param_list = function_source.param_list()?; @@ -100,14 +108,14 @@ pub(crate) fn inline_( return None; } - let body = function_source.body()?; + let fn_body = function_source.body()?; acc.add( AssistId("inline_call", AssistKind::RefactorInline), label, expr.syntax().text_range(), |builder| { - let body = body.clone_for_update(); + let body = fn_body.clone_for_update(); let file_id = file_id.original_file(ctx.sema.db); let usages_for_locals = |local| { @@ -198,6 +206,15 @@ pub(crate) fn inline_( } } } + if let Some(generic_arg_list) = generic_arg_list { + PathTransform::function_call( + &ctx.sema.scope(expr.syntax()), + &ctx.sema.scope(fn_body.syntax()), + function, + generic_arg_list, + ) + .apply(body.syntax()); + } let original_indentation = expr.indent_level(); let replacement = body.reset_indent().indent(original_indentation); @@ -644,6 +661,36 @@ fn main() { x as u32 }; } +"#, + ); + } + + // FIXME: const generics aren't being substituted, this is blocked on better support for them + #[test] + fn inline_substitutes_generics() { + check_assist( + inline_call, + r#" +fn foo() { + bar::() +} + +fn bar() {} + +fn main() { + foo::$0(); +} +"#, + r#" +fn foo() { + bar::() +} + +fn bar() {} + +fn main() { + bar::(); +} "#, ); } diff --git a/crates/ide_assists/src/utils.rs b/crates/ide_assists/src/utils.rs index b4b9b6af80d..a4e4a00f78d 100644 --- a/crates/ide_assists/src/utils.rs +++ b/crates/ide_assists/src/utils.rs @@ -129,15 +129,11 @@ pub fn add_trait_assoc_items_to_impl( ) -> (ast::Impl, ast::AssocItem) { let source_scope = sema.scope_for_def(trait_); - let transform = PathTransform { - subst: (trait_, impl_.clone()), - source_scope: &source_scope, - target_scope: &target_scope, - }; + let transform = PathTransform::trait_impl(&target_scope, &source_scope, trait_, impl_.clone()); let items = items.into_iter().map(|assoc_item| { let assoc_item = assoc_item.clone_for_update(); - transform.apply(assoc_item.clone()); + transform.apply(assoc_item.syntax()); edit::remove_attrs_and_docs(&assoc_item).clone_subtree().clone_for_update() }); diff --git a/crates/ide_completion/src/completions/trait_impl.rs b/crates/ide_completion/src/completions/trait_impl.rs index 3713be0a176..75b02583bf2 100644 --- a/crates/ide_completion/src/completions/trait_impl.rs +++ b/crates/ide_completion/src/completions/trait_impl.rs @@ -186,13 +186,14 @@ fn get_transformed_assoc_item( let trait_ = impl_def.trait_(ctx.db)?; let source_scope = &ctx.sema.scope_for_def(trait_); let target_scope = &ctx.sema.scope(impl_def.source(ctx.db)?.syntax().value); - let transform = PathTransform { - subst: (trait_, impl_def.source(ctx.db)?.value), - source_scope, + let transform = PathTransform::trait_impl( target_scope, - }; + source_scope, + trait_, + impl_def.source(ctx.db)?.value, + ); - transform.apply(assoc_item.clone()); + transform.apply(assoc_item.syntax()); Some(match assoc_item { ast::AssocItem::Fn(func) => ast::AssocItem::Fn(edit::remove_attrs_and_docs(&func)), _ => assoc_item, diff --git a/crates/ide_db/src/path_transform.rs b/crates/ide_db/src/path_transform.rs index 00a88116cb7..b8b1307a3c0 100644 --- a/crates/ide_db/src/path_transform.rs +++ b/crates/ide_db/src/path_transform.rs @@ -5,7 +5,7 @@ use hir::{HirDisplay, SemanticsScope}; use rustc_hash::FxHashMap; use syntax::{ ast::{self, AstNode}, - ted, + ted, SyntaxNode, }; /// `PathTransform` substitutes path in SyntaxNodes in bulk. @@ -32,38 +32,70 @@ use syntax::{ /// } /// ``` pub struct PathTransform<'a> { - pub subst: (hir::Trait, ast::Impl), - pub target_scope: &'a SemanticsScope<'a>, - pub source_scope: &'a SemanticsScope<'a>, + generic_def: hir::GenericDef, + substs: Vec, + target_scope: &'a SemanticsScope<'a>, + source_scope: &'a SemanticsScope<'a>, } impl<'a> PathTransform<'a> { - pub fn apply(&self, item: ast::AssocItem) { - if let Some(ctx) = self.build_ctx() { - ctx.apply(item) + pub fn trait_impl( + target_scope: &'a SemanticsScope<'a>, + source_scope: &'a SemanticsScope<'a>, + trait_: hir::Trait, + impl_: ast::Impl, + ) -> PathTransform<'a> { + PathTransform { + source_scope, + target_scope, + generic_def: trait_.into(), + substs: get_syntactic_substs(impl_).unwrap_or_default(), } } + + pub fn function_call( + target_scope: &'a SemanticsScope<'a>, + source_scope: &'a SemanticsScope<'a>, + function: hir::Function, + generic_arg_list: ast::GenericArgList, + ) -> PathTransform<'a> { + PathTransform { + source_scope, + target_scope, + generic_def: function.into(), + substs: get_type_args_from_arg_list(generic_arg_list).unwrap_or_default(), + } + } + + pub fn apply(&self, syntax: &SyntaxNode) { + if let Some(ctx) = self.build_ctx() { + ctx.apply(syntax) + } + } + fn build_ctx(&self) -> Option> { let db = self.source_scope.db; let target_module = self.target_scope.module()?; let source_module = self.source_scope.module()?; - - let substs = get_syntactic_substs(self.subst.1.clone()).unwrap_or_default(); - let generic_def: hir::GenericDef = self.subst.0.into(); - let substs_by_param: FxHashMap<_, _> = generic_def + let skip = match self.generic_def { + // this is a trait impl, so we need to skip the first type parameter -- this is a bit hacky + hir::GenericDef::Trait(_) => 1, + _ => 0, + }; + let substs_by_param: FxHashMap<_, _> = self + .generic_def .type_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) + .skip(skip) // The actual list of trait type parameters may be longer than the one // used in the `impl` block due to trailing default type parameters. // For that case we extend the `substs` with an empty iterator so we // can still hit those trailing values and check if they actually have // a default type. If they do, go for that type from `hir` to `ast` so // the resulting change can be applied correctly. - .zip(substs.into_iter().map(Some).chain(std::iter::repeat(None))) + .zip(self.substs.iter().map(Some).chain(std::iter::repeat(None))) .filter_map(|(k, v)| match v { - Some(v) => Some((k, v)), + Some(v) => Some((k, v.clone())), None => { let default = k.default(db)?; Some(( @@ -73,7 +105,6 @@ impl<'a> PathTransform<'a> { } }) .collect(); - let res = Ctx { substs: substs_by_param, target_module, source_scope: self.source_scope }; Some(res) } @@ -86,8 +117,8 @@ struct Ctx<'a> { } impl<'a> Ctx<'a> { - fn apply(&self, item: ast::AssocItem) { - for event in item.syntax().preorder() { + fn apply(&self, item: &SyntaxNode) { + for event in item.preorder() { let node = match event { syntax::WalkEvent::Enter(_) => continue, syntax::WalkEvent::Leave(it) => it, @@ -149,13 +180,14 @@ fn get_syntactic_substs(impl_def: ast::Impl) -> Option> { }; let generic_arg_list = path_type.path()?.segment()?.generic_arg_list()?; + get_type_args_from_arg_list(generic_arg_list) +} + +fn get_type_args_from_arg_list(generic_arg_list: ast::GenericArgList) -> Option> { let mut result = Vec::new(); for generic_arg in generic_arg_list.generic_args() { - match generic_arg { - ast::GenericArg::TypeArg(type_arg) => result.push(type_arg.ty()?), - ast::GenericArg::AssocTypeArg(_) - | ast::GenericArg::LifetimeArg(_) - | ast::GenericArg::ConstArg(_) => (), + if let ast::GenericArg::TypeArg(type_arg) = generic_arg { + result.push(type_arg.ty()?) } }