Merge #10438

10438: minor: Simplify r=Veykril a=Veykril

bors r+

Co-authored-by: Lukas Wirth <lukastw97@gmail.com>

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));
 

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<ast::ArgList> {
-        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(&current_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::GenericParamList>, 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<GeneratedFunctionTarget> {
+fn next_space_for_fn_after_call_site(expr: ast::CallableExpr) -> Option<GeneratedFunctionTarget> {
     let mut ancestors = expr.syntax().ancestors().peekable();
     let mut last_ancestor: Option<SyntaxNode> = None;
     while let Some(next_ancestor) = ancestors.next() {

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<ast::Expr>,
     generic_arg_list: Option<ast::GenericArgList>,
 }
@@ -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);
 

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<Item = &str> + '_ {
         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<usize>)> {
     // 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<FnCallNode> {
-        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<FnCallNode> {
-        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<ast::NameRef> {
-        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<ast::ArgList> {
-        match self {
-            FnCallNode::CallExpr(expr) => expr.arg_list(),
-            FnCallNode::MethodCallExpr(expr) => expr.arg_list(),
-        }
-    }
-}
-
 #[cfg(test)]
 mod tests;

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}

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,

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<Self>
+    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(),
+        }
+    }
+}