From 0943c4be8b141fd9164e50b591a4219bd2939923 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Sun, 3 Oct 2021 12:42:00 +0200 Subject: [PATCH] minor: Simplify --- crates/ide/src/call_hierarchy.rs | 23 +++-- .../src/handlers/generate_function.rs | 37 ++------ .../ide_assists/src/handlers/inline_call.rs | 37 +++----- crates/ide_db/src/call_info.rs | 94 ++++--------------- crates/ide_db/src/call_info/tests.rs | 1 - crates/syntax/src/ast.rs | 2 +- crates/syntax/src/ast/expr_ext.rs | 40 +++++++- 7 files changed, 91 insertions(+), 143 deletions(-) diff --git a/crates/ide/src/call_hierarchy.rs b/crates/ide/src/call_hierarchy.rs index 242b7b40e18..e83210f4f93 100644 --- a/crates/ide/src/call_hierarchy.rs +++ b/crates/ide/src/call_hierarchy.rs @@ -4,7 +4,6 @@ use indexmap::IndexMap; use hir::Semantics; use ide_db::{ - call_info::FnCallNode, defs::{Definition, NameClass, NameRefClass}, helpers::pick_best_token, search::FileReference, @@ -101,23 +100,27 @@ pub(crate) fn outgoing_calls(db: &RootDatabase, position: FilePosition) -> Optio _ => None, }) .flatten() - .filter_map(|node| FnCallNode::with_node_exact(&node)) + .filter_map(ast::CallableExpr::cast) .filter_map(|call_node| { - let name_ref = call_node.name_ref()?; - let func_target = match call_node { - FnCallNode::CallExpr(expr) => { - let callable = sema.type_of_expr(&expr.expr()?)?.original.as_callable(db)?; + let (nav_target, range) = match call_node { + ast::CallableExpr::Call(call) => { + let expr = call.expr()?; + let callable = sema.type_of_expr(&expr)?.original.as_callable(db)?; match callable.kind() { - hir::CallableKind::Function(it) => it.try_to_nav(db), + hir::CallableKind::Function(it) => { + let range = expr.syntax().text_range(); + it.try_to_nav(db).zip(Some(range)) + } _ => None, } } - FnCallNode::MethodCallExpr(expr) => { + ast::CallableExpr::MethodCall(expr) => { + let range = expr.name_ref()?.syntax().text_range(); let function = sema.resolve_method_call(&expr)?; - function.try_to_nav(db) + function.try_to_nav(db).zip(Some(range)) } }?; - Some((func_target, name_ref.syntax().text_range())) + Some((nav_target, range)) }) .for_each(|(nav, range)| calls.add(nav, range)); diff --git a/crates/ide_assists/src/handlers/generate_function.rs b/crates/ide_assists/src/handlers/generate_function.rs index 8a115087da6..64e5c23f7cb 100644 --- a/crates/ide_assists/src/handlers/generate_function.rs +++ b/crates/ide_assists/src/handlers/generate_function.rs @@ -51,27 +51,6 @@ pub(crate) fn generate_function(acc: &mut Assists, ctx: &AssistContext) -> Optio gen_fn(acc, ctx).or_else(|| gen_method(acc, ctx)) } -enum FuncExpr { - Func(ast::CallExpr), - Method(ast::MethodCallExpr), -} - -impl FuncExpr { - fn arg_list(&self) -> Option { - match self { - FuncExpr::Func(fn_call) => fn_call.arg_list(), - FuncExpr::Method(m_call) => m_call.arg_list(), - } - } - - fn syntax(&self) -> &SyntaxNode { - match self { - FuncExpr::Func(fn_call) => fn_call.syntax(), - FuncExpr::Method(m_call) => m_call.syntax(), - } - } -} - fn gen_fn(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { let path_expr: ast::PathExpr = ctx.find_node_at_offset()?; let call = path_expr.syntax().parent().and_then(ast::CallExpr::cast)?; @@ -254,7 +233,8 @@ impl FunctionBuilder { let needs_pub = target_module.is_some(); let target_module = target_module.or_else(|| current_module(target.syntax(), ctx))?; let fn_name = make::name(fn_name); - let (type_params, params) = fn_args(ctx, target_module, FuncExpr::Func(call.clone()))?; + let (type_params, params) = + fn_args(ctx, target_module, ast::CallableExpr::Call(call.clone()))?; let await_expr = call.syntax().parent().and_then(ast::AwaitExpr::cast); let is_async = await_expr.is_some(); @@ -284,7 +264,8 @@ impl FunctionBuilder { let needs_pub = !module_is_descendant(¤t_module(call.syntax(), ctx)?, &target_module, ctx); let fn_name = make::name(&name.text()); - let (type_params, params) = fn_args(ctx, target_module, FuncExpr::Method(call.clone()))?; + let (type_params, params) = + fn_args(ctx, target_module, ast::CallableExpr::MethodCall(call.clone()))?; let await_expr = call.syntax().parent().and_then(ast::AwaitExpr::cast); let is_async = await_expr.is_some(); @@ -392,7 +373,7 @@ fn get_fn_target( file = in_file; target } - None => next_space_for_fn_after_call_site(FuncExpr::Func(call))?, + None => next_space_for_fn_after_call_site(ast::CallableExpr::Call(call))?, }; Some((target.clone(), file, get_insert_offset(&target))) } @@ -438,7 +419,7 @@ impl GeneratedFunctionTarget { fn fn_args( ctx: &AssistContext, target_module: hir::Module, - call: FuncExpr, + call: ast::CallableExpr, ) -> Option<(Option, ast::ParamList)> { let mut arg_names = Vec::new(); let mut arg_types = Vec::new(); @@ -468,8 +449,8 @@ fn fn_args( None, make::param_list( match call { - FuncExpr::Func(_) => None, - FuncExpr::Method(_) => Some(make::self_param()), + ast::CallableExpr::Call(_) => None, + ast::CallableExpr::MethodCall(_) => Some(make::self_param()), }, params, ), @@ -553,7 +534,7 @@ fn fn_arg_type( /// directly after the current block /// We want to write the generated function directly after /// fns, impls or macro calls, but inside mods -fn next_space_for_fn_after_call_site(expr: FuncExpr) -> Option { +fn next_space_for_fn_after_call_site(expr: ast::CallableExpr) -> Option { let mut ancestors = expr.syntax().ancestors().peekable(); let mut last_ancestor: Option = None; while let Some(next_ancestor) = ancestors.next() { diff --git a/crates/ide_assists/src/handlers/inline_call.rs b/crates/ide_assists/src/handlers/inline_call.rs index d252d61a696..b3dc95df41e 100644 --- a/crates/ide_assists/src/handlers/inline_call.rs +++ b/crates/ide_assists/src/handlers/inline_call.rs @@ -12,7 +12,7 @@ use ide_db::{ use itertools::{izip, Itertools}; use syntax::{ ast::{self, edit_in_place::Indent, HasArgList, PathExpr}, - ted, AstNode, SyntaxNode, + ted, AstNode, }; use crate::{ @@ -178,7 +178,7 @@ pub(crate) fn inline_call(acc: &mut Assists, ctx: &AssistContext) -> Option<()> let name_ref: ast::NameRef = ctx.find_node_at_offset()?; let call_info = CallInfo::from_name_ref(name_ref.clone())?; let (function, label) = match &call_info.node { - CallExprNode::Call(call) => { + ast::CallableExpr::Call(call) => { let path = match call.expr()? { ast::Expr::PathExpr(path) => path.path(), _ => None, @@ -190,7 +190,7 @@ pub(crate) fn inline_call(acc: &mut Assists, ctx: &AssistContext) -> Option<()> }; (function, format!("Inline `{}`", path)) } - CallExprNode::MethodCallExpr(call) => { + ast::CallableExpr::MethodCall(call) => { (ctx.sema.resolve_method_call(call)?, format!("Inline `{}`", name_ref)) } }; @@ -223,8 +223,8 @@ pub(crate) fn inline_call(acc: &mut Assists, ctx: &AssistContext) -> Option<()> builder.replace_ast( match call_info.node { - CallExprNode::Call(it) => ast::Expr::CallExpr(it), - CallExprNode::MethodCallExpr(it) => ast::Expr::MethodCallExpr(it), + ast::CallableExpr::Call(it) => ast::Expr::CallExpr(it), + ast::CallableExpr::MethodCall(it) => ast::Expr::MethodCallExpr(it), }, replacement, ); @@ -232,22 +232,8 @@ pub(crate) fn inline_call(acc: &mut Assists, ctx: &AssistContext) -> Option<()> ) } -enum CallExprNode { - Call(ast::CallExpr), - MethodCallExpr(ast::MethodCallExpr), -} - -impl CallExprNode { - fn syntax(&self) -> &SyntaxNode { - match self { - CallExprNode::Call(it) => it.syntax(), - CallExprNode::MethodCallExpr(it) => it.syntax(), - } - } -} - struct CallInfo { - node: CallExprNode, + node: ast::CallableExpr, arguments: Vec, generic_arg_list: Option, } @@ -261,7 +247,7 @@ impl CallInfo { arguments.extend(call.arg_list()?.args()); Some(CallInfo { generic_arg_list: call.generic_arg_list(), - node: CallExprNode::MethodCallExpr(call), + node: ast::CallableExpr::MethodCall(call), arguments, }) } else if let Some(segment) = ast::PathSegment::cast(parent) { @@ -271,7 +257,7 @@ impl CallInfo { Some(CallInfo { arguments: call.arg_list()?.args().collect(), - node: CallExprNode::Call(call), + node: ast::CallableExpr::Call(call), generic_arg_list: segment.generic_arg_list(), }) } else { @@ -367,8 +353,9 @@ fn inline( ted::replace(usage.syntax(), &replacement.syntax().clone_for_update()); } }; - // izip confuses RA due to our lack of hygiene info currently losing us typeinfo + // izip confuses RA due to our lack of hygiene info currently losing us type info causing incorrect errors let usages: &[ast::PathExpr] = &*usages; + let expr: &ast::Expr = expr; match usages { // inline single use closure arguments [usage] @@ -414,8 +401,8 @@ fn inline( } let original_indentation = match node { - CallExprNode::Call(it) => it.indent_level(), - CallExprNode::MethodCallExpr(it) => it.indent_level(), + ast::CallableExpr::Call(it) => it.indent_level(), + ast::CallableExpr::MethodCall(it) => it.indent_level(), }; body.reindent_to(original_indentation); diff --git a/crates/ide_db/src/call_info.rs b/crates/ide_db/src/call_info.rs index e1d85f0411c..922a411aed1 100644 --- a/crates/ide_db/src/call_info.rs +++ b/crates/ide_db/src/call_info.rs @@ -6,7 +6,7 @@ use stdx::format_to; use syntax::{ algo, ast::{self, HasArgList, HasName}, - match_ast, AstNode, Direction, SyntaxNode, SyntaxToken, TextRange, TextSize, + AstNode, Direction, SyntaxToken, TextRange, TextSize, }; use crate::RootDatabase; @@ -25,9 +25,11 @@ impl CallInfo { pub fn parameter_labels(&self) -> impl Iterator + '_ { self.parameters.iter().map(move |&it| &self.signature[it]) } + pub fn parameter_ranges(&self) -> &[TextRange] { &self.parameters } + fn push_param(&mut self, param: &str) { if !self.signature.ends_with('(') { self.signature.push_str(", "); @@ -115,31 +117,24 @@ fn call_info_impl( token: SyntaxToken, ) -> Option<(hir::Callable, Option)> { // Find the calling expression and it's NameRef - let calling_node = FnCallNode::with_node(&token.parent()?)?; + let parent = token.parent()?; + let calling_node = parent.ancestors().filter_map(ast::CallableExpr::cast).find(|it| { + it.arg_list() + .map_or(false, |it| it.syntax().text_range().contains(token.text_range().start())) + })?; let callable = match &calling_node { - FnCallNode::CallExpr(call) => { - sema.type_of_expr(&call.expr()?)?.adjusted().as_callable(sema.db)? + ast::CallableExpr::Call(call) => { + let expr = call.expr()?; + sema.type_of_expr(&expr)?.adjusted().as_callable(sema.db) } - FnCallNode::MethodCallExpr(call) => sema.resolve_method_call_as_callable(call)?, - }; + ast::CallableExpr::MethodCall(call) => sema.resolve_method_call_as_callable(call), + }?; let active_param = if let Some(arg_list) = calling_node.arg_list() { - // Number of arguments specified at the call site - let num_args_at_callsite = arg_list.args().count(); - - let arg_list_range = arg_list.syntax().text_range(); - if !arg_list_range.contains_inclusive(token.text_range().start()) { - cov_mark::hit!(call_info_bad_offset); - return None; - } - let param = std::cmp::min( - num_args_at_callsite, - arg_list - .args() - .take_while(|arg| arg.syntax().text_range().end() <= token.text_range().start()) - .count(), - ); - + let param = arg_list + .args() + .take_while(|arg| arg.syntax().text_range().end() <= token.text_range().start()) + .count(); Some(param) } else { None @@ -175,60 +170,5 @@ impl ActiveParameter { } } -#[derive(Debug)] -pub enum FnCallNode { - CallExpr(ast::CallExpr), - MethodCallExpr(ast::MethodCallExpr), -} - -impl FnCallNode { - fn with_node(syntax: &SyntaxNode) -> Option { - syntax.ancestors().find_map(|node| { - match_ast! { - match node { - ast::CallExpr(it) => Some(FnCallNode::CallExpr(it)), - ast::MethodCallExpr(it) => { - let arg_list = it.arg_list()?; - if !arg_list.syntax().text_range().contains_range(syntax.text_range()) { - return None; - } - Some(FnCallNode::MethodCallExpr(it)) - }, - _ => None, - } - } - }) - } - - pub fn with_node_exact(node: &SyntaxNode) -> Option { - match_ast! { - match node { - ast::CallExpr(it) => Some(FnCallNode::CallExpr(it)), - ast::MethodCallExpr(it) => Some(FnCallNode::MethodCallExpr(it)), - _ => None, - } - } - } - - pub fn name_ref(&self) -> Option { - match self { - FnCallNode::CallExpr(call_expr) => Some(match call_expr.expr()? { - ast::Expr::PathExpr(path_expr) => path_expr.path()?.segment()?.name_ref()?, - _ => return None, - }), - FnCallNode::MethodCallExpr(call_expr) => { - call_expr.syntax().children().find_map(ast::NameRef::cast) - } - } - } - - fn arg_list(&self) -> Option { - match self { - FnCallNode::CallExpr(expr) => expr.arg_list(), - FnCallNode::MethodCallExpr(expr) => expr.arg_list(), - } - } -} - #[cfg(test)] mod tests; diff --git a/crates/ide_db/src/call_info/tests.rs b/crates/ide_db/src/call_info/tests.rs index b585085f381..8cdfcd027a3 100644 --- a/crates/ide_db/src/call_info/tests.rs +++ b/crates/ide_db/src/call_info/tests.rs @@ -362,7 +362,6 @@ pub fn foo(mut r: WriteHandler<()>) { #[test] fn call_info_bad_offset() { - cov_mark::check!(call_info_bad_offset); check( r#" fn foo(x: u32, y: u32) -> u32 {x + y} diff --git a/crates/syntax/src/ast.rs b/crates/syntax/src/ast.rs index e57c3d2fe44..8732593af9c 100644 --- a/crates/syntax/src/ast.rs +++ b/crates/syntax/src/ast.rs @@ -18,7 +18,7 @@ use crate::{ }; pub use self::{ - expr_ext::{ArrayExprKind, BlockModifier, ElseBranch, LiteralKind}, + expr_ext::{ArrayExprKind, BlockModifier, CallableExpr, ElseBranch, LiteralKind}, generated::{nodes::*, tokens::*}, node_ext::{ AttrKind, FieldKind, Macro, NameLike, NameOrNameRef, PathSegmentKind, SelfParamKind, diff --git a/crates/syntax/src/ast/expr_ext.rs b/crates/syntax/src/ast/expr_ext.rs index 7363ad905ae..f421f34b3dc 100644 --- a/crates/syntax/src/ast/expr_ext.rs +++ b/crates/syntax/src/ast/expr_ext.rs @@ -10,7 +10,7 @@ use crate::{ }, AstToken, SyntaxKind::*, - SyntaxToken, T, + SyntaxNode, SyntaxToken, T, }; impl ast::HasAttrs for ast::Expr {} @@ -312,3 +312,41 @@ impl ast::RecordExprField { self.syntax().ancestors().find_map(ast::RecordExpr::cast).unwrap() } } + +#[derive(Debug, Clone, PartialEq, Eq, Hash)] +pub enum CallableExpr { + Call(ast::CallExpr), + MethodCall(ast::MethodCallExpr), +} + +impl ast::HasAttrs for CallableExpr {} +impl ast::HasArgList for CallableExpr {} + +impl AstNode for CallableExpr { + fn can_cast(kind: parser::SyntaxKind) -> bool + where + Self: Sized, + { + ast::CallExpr::can_cast(kind) || ast::MethodCallExpr::can_cast(kind) + } + + fn cast(syntax: SyntaxNode) -> Option + where + Self: Sized, + { + if let Some(it) = ast::CallExpr::cast(syntax.clone()) { + Some(Self::Call(it)) + } else if let Some(it) = ast::MethodCallExpr::cast(syntax.clone()) { + Some(Self::MethodCall(it)) + } else { + None + } + } + + fn syntax(&self) -> &SyntaxNode { + match self { + Self::Call(it) => it.syntax(), + Self::MethodCall(it) => it.syntax(), + } + } +}