Fix extract_function with macro arg

crates/ide_assists/src/handlers/extract_function.rs

@@ -2,7 +2,7 @@ use std::{hash::BuildHasherDefault, iter};
 
 use ast::make;
 use either::Either;
-use hir::{HirDisplay, Local, Semantics, TypeInfo};
+use hir::{HirDisplay, InFile, Local, Semantics, TypeInfo};
 use ide_db::{
     defs::{Definition, NameRefClass},
     search::{FileReference, ReferenceAccess, SearchScope},
@@ -17,7 +17,7 @@ use syntax::{
         edit::{AstNodeEdit, IndentLevel},
         AstNode,
     },
-    match_ast, ted,
+    match_ast, ted, SyntaxElement,
     SyntaxKind::{self, COMMENT},
     SyntaxNode, SyntaxToken, TextRange, TextSize, TokenAtOffset, WalkEvent, T,
 };
@@ -582,38 +582,45 @@ impl FunctionBody {
         &self,
         sema: &Semantics<RootDatabase>,
     ) -> (FxIndexSet<Local>, Option<ast::SelfParam>) {
-        // FIXME: currently usages inside macros are not found
         let mut self_param = None;
         let mut res = FxIndexSet::default();
-        self.walk_expr(&mut |expr| {
+        let mut cb = |name_ref: Option<_>| {
-            let name_ref = match expr {
+            let local_ref =
-                ast::Expr::PathExpr(path_expr) => {
+                match name_ref.and_then(|name_ref| NameRefClass::classify(sema, &name_ref)) {
-                    path_expr.path().and_then(|it| it.as_single_name_ref())
+                    Some(
-                }
-                _ => return,
-            };
-            if let Some(name_ref) = name_ref {
-                if let Some(
                         NameRefClass::Definition(Definition::Local(local_ref))
                         | NameRefClass::FieldShorthand { local_ref, field_ref: _ },
-                ) = NameRefClass::classify(sema, &name_ref)
+                    ) => local_ref,
-                {
+                    _ => return,
-                    if local_ref.is_self(sema.db) {
+                };
-                        match local_ref.source(sema.db).value {
+            let InFile { file_id, value } = local_ref.source(sema.db);
+            // locals defined inside macros are not relevant to us
+            if !file_id.is_macro() {
+                match value {
                     Either::Right(it) => {
                         self_param.replace(it);
                     }
                     Either::Left(_) => {
-                                stdx::never!(
-                                    "Local::is_self returned true, but source is IdentPat"
-                                );
-                            }
-                        }
-                    } else {
                         res.insert(local_ref);
                     }
                 }
             }
+        };
+        self.walk_expr(&mut |expr| match expr {
+            ast::Expr::PathExpr(path_expr) => {
+                cb(path_expr.path().and_then(|it| it.as_single_name_ref()))
+            }
+            ast::Expr::MacroCall(call) => {
+                if let Some(tt) = call.token_tree() {
+                    tt.syntax()
+                        .children_with_tokens()
+                        .flat_map(SyntaxElement::into_token)
+                        .filter(|it| it.kind() == SyntaxKind::IDENT)
+                        .flat_map(|t| sema.descend_into_macros_many(t))
+                        .for_each(|t| cb(t.parent().and_then(ast::NameRef::cast)));
+                }
+            }
+            _ => (),
         });
         (res, self_param)
     }
@@ -4155,6 +4162,35 @@ fn foo() {
 fn $0fun_name(y: &mut Foo) {
     y.foo();
 }
+"#,
+        );
+    }
+
+    #[test]
+    fn extract_with_macro_arg() {
+        check_assist(
+            extract_function,
+            r#"
+macro_rules! m {
+    ($val:expr) => { $val };
+}
+fn main() {
+    let bar = "bar";
+    $0m!(bar);$0
+}
+"#,
+            r#"
+macro_rules! m {
+    ($val:expr) => { $val };
+}
+fn main() {
+    let bar = "bar";
+    fun_name(bar);
+}
+
+fn $0fun_name(bar: &str) {
+    m!(bar);
+}
 "#,
         );
     }

crates/syntax/src/ast/expr_ext.rs

@@ -77,6 +77,11 @@ impl ast::Expr {
                 }
                 // Don't skip subtree since we want to process the expression child next
                 Some(ast::Stmt::ExprStmt(_)) => (),
+                // This might be an expression
+                Some(ast::Stmt::Item(ast::Item::MacroCall(mcall))) => {
+                    cb(WalkEvent::Enter(ast::Expr::MacroCall(mcall)));
+                    preorder.skip_subtree();
+                }
                 // skip inner items which might have their own expressions
                 Some(ast::Stmt::Item(_)) => preorder.skip_subtree(),
                 None => {