Unnamed repository; edit this file 'description' to name the repository.
Reject recursive calls in inline_call
| -rw-r--r-- | crates/ide_assists/src/handlers/inline_call.rs | 83 | ||||
| -rw-r--r-- | crates/ide_db/src/search.rs | 5 |
2 files changed, 67 insertions, 21 deletions
diff --git a/crates/ide_assists/src/handlers/inline_call.rs b/crates/ide_assists/src/handlers/inline_call.rs index 40231bd948..42a2ca23f7 100644 --- a/crates/ide_assists/src/handlers/inline_call.rs +++ b/crates/ide_assists/src/handlers/inline_call.rs @@ -1,7 +1,10 @@ use ast::make; use hir::{db::HirDatabase, HasSource, PathResolution, Semantics, TypeInfo}; use ide_db::{ - base_db::FileId, defs::Definition, path_transform::PathTransform, search::FileReference, + base_db::{FileId, FileRange}, + defs::Definition, + path_transform::PathTransform, + search::{FileReference, SearchScope}, RootDatabase, }; use itertools::izip; @@ -54,11 +57,14 @@ use crate::{ // } // ``` pub(crate) fn inline_into_callers(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { + let def_file = ctx.frange.file_id; let name = ctx.find_node_at_offset::<ast::Name>()?; - let func_syn = name.syntax().parent().and_then(ast::Fn::cast)?; - let func_body = func_syn.body()?; - let param_list = func_syn.param_list()?; - let function = ctx.sema.to_def(&func_syn)?; + let ast_func = name.syntax().parent().and_then(ast::Fn::cast)?; + let func_body = ast_func.body()?; + let param_list = ast_func.param_list()?; + + let function = ctx.sema.to_def(&ast_func)?; + let params = get_fn_params(ctx.sema.db, function, ¶m_list)?; let usages = Definition::ModuleDef(hir::ModuleDef::Function(function)).usages(&ctx.sema); @@ -66,19 +72,28 @@ pub(crate) fn inline_into_callers(acc: &mut Assists, ctx: &AssistContext) -> Opt return None; } + let is_recursive_fn = usages + .clone() + .in_scope(SearchScope::file_range(FileRange { + file_id: def_file, + range: func_body.syntax().text_range(), + })) + .at_least_one(); + if is_recursive_fn { + cov_mark::hit!(inline_into_callers_recursive); + return None; + } + acc.add( AssistId("inline_into_callers", AssistKind::RefactorInline), "Inline into all callers", name.syntax().text_range(), |builder| { - let def_file = ctx.frange.file_id; - let usages = - Definition::ModuleDef(hir::ModuleDef::Function(function)).usages(&ctx.sema); let mut usages = usages.all(); let current_file_usage = usages.references.remove(&def_file); - let mut can_remove = true; - let mut inline_refs = |file_id, refs: Vec<FileReference>| { + let mut remove_def = true; + let mut inline_refs_for_file = |file_id, refs: Vec<FileReference>| { builder.edit_file(file_id); let count = refs.len(); let name_refs = refs.into_iter().filter_map(|file_ref| match file_ref.name { @@ -124,18 +139,18 @@ pub(crate) fn inline_into_callers(acc: &mut Assists, ctx: &AssistContext) -> Opt ); }) .count(); - can_remove &= replaced == count; + remove_def &= replaced == count; }; for (file_id, refs) in usages.into_iter() { - inline_refs(file_id, refs); + inline_refs_for_file(file_id, refs); } if let Some(refs) = current_file_usage { - inline_refs(def_file, refs); + inline_refs_for_file(def_file, refs); } else { builder.edit_file(def_file); } - if can_remove { - builder.delete(func_syn.syntax().text_range()); + if remove_def { + builder.delete(ast_func.syntax().text_range()); } }, ) @@ -201,10 +216,15 @@ pub(crate) fn inline_call(acc: &mut Assists, ctx: &AssistContext) -> Option<()> ) }; - let hir::InFile { value: function_source, file_id } = function.source(ctx.db())?; - let fn_body = function_source.body()?; - let param_list = function_source.param_list()?; + let fn_source = function.source(ctx.db())?; + let fn_body = fn_source.value.body()?; + let param_list = fn_source.value.param_list()?; + let FileRange { file_id, range } = fn_source.syntax().original_file_range(ctx.sema.db); + if file_id == ctx.frange.file_id && range.contains(ctx.frange.range.start()) { + cov_mark::hit!(inline_call_recursive); + return None; + } let params = get_fn_params(ctx.sema.db, function, ¶m_list)?; if call_info.arguments.len() != params.len() { @@ -220,7 +240,6 @@ pub(crate) fn inline_call(acc: &mut Assists, ctx: &AssistContext) -> Option<()> label, syntax.text_range(), |builder| { - let file_id = file_id.original_file(ctx.sema.db); let replacement = inline(&ctx.sema, file_id, function, &fn_body, ¶ms, &call_info); builder.replace_ast( @@ -970,4 +989,30 @@ fn foo() { "#, ); } + + #[test] + fn inline_callers_recursive() { + cov_mark::check!(inline_into_callers_recursive); + check_assist_not_applicable( + inline_into_callers, + r#" +fn foo$0() { + foo(); +} +"#, + ); + } + + #[test] + fn inline_call_recursive() { + cov_mark::check!(inline_call_recursive); + check_assist_not_applicable( + inline_call, + r#" +fn foo() { + foo$0(); +} +"#, + ); + } } diff --git a/crates/ide_db/src/search.rs b/crates/ide_db/src/search.rs index a84e6b3ba4..00acfde243 100644 --- a/crates/ide_db/src/search.rs +++ b/crates/ide_db/src/search.rs @@ -315,6 +315,7 @@ impl Definition { } } +#[derive(Clone)] pub struct FindUsages<'a> { def: Definition, sema: &'a Semantics<'a, RootDatabase>, @@ -341,7 +342,7 @@ impl<'a> FindUsages<'a> { self } - pub fn at_least_one(self) -> bool { + pub fn at_least_one(&self) -> bool { let mut found = false; self.search(&mut |_, _| { found = true; @@ -359,7 +360,7 @@ impl<'a> FindUsages<'a> { res } - fn search(self, sink: &mut dyn FnMut(FileId, FileReference) -> bool) { + fn search(&self, sink: &mut dyn FnMut(FileId, FileReference) -> bool) { let _p = profile::span("FindUsages:search"); let sema = self.sema; |