21 files changed, 321 insertions, 155 deletions
diff --git a/Cargo.lock b/Cargo.lock
index 3f4d9204c..f8d26192d 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -845,14 +845,9 @@ checksum = "74490b50b9fbe561ac330df47c08f3f33073d2d00c150f719147d7c54522fa1b"
 [[package]]
 name = "proc-macro-hack"
-version = "0.5.12"
+version = "0.5.14"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "f918f2b601f93baa836c1c2945faef682ba5b6d4828ecb45eeb7cc3c71b811b4"
+checksum = "fcfdefadc3d57ca21cf17990a28ef4c0f7c61383a28cb7604cf4a18e6ede1420"
-dependencies = [
- "proc-macro2",
- "quote",
- "syn",
-]
 [[package]]
 name = "proc-macro2"
@@ -1480,9 +1475,9 @@ checksum = "ab16ced94dbd8a46c82fd81e3ed9a8727dac2977ea869d217bcc4ea1f122e81f"
 [[package]]
 name = "syn"
-version = "1.0.16"
+version = "1.0.17"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "123bd9499cfb380418d509322d7a6d52e5315f064fe4b3ad18a53d6b92c07859"
+checksum = "0df0eb663f387145cab623dea85b09c2c5b4b0aef44e945d928e682fce71bb03"
 dependencies = [
 "proc-macro2",
 "quote",
diff --git a/crates/ra_cargo_watch/src/conv.rs b/crates/ra_cargo_watch/src/conv.rs
index c6f8ca329..817543deb 100644
--- a/crates/ra_cargo_watch/src/conv.rs
+++ b/crates/ra_cargo_watch/src/conv.rs
@@ -1,7 +1,8 @@
 //! This module provides the functionality needed to convert diagnostics from
 //! `cargo check` json format to the LSP diagnostic format.
 use cargo_metadata::diagnostic::{
-    Diagnostic as RustDiagnostic, DiagnosticLevel, DiagnosticSpan, DiagnosticSpanMacroExpansion,
+    Applicability, Diagnostic as RustDiagnostic, DiagnosticLevel, DiagnosticSpan,
+    DiagnosticSpanMacroExpansion,
 };
 use lsp_types::{
    CodeAction, Diagnostic, DiagnosticRelatedInformation, DiagnosticSeverity, DiagnosticTag,
@@ -136,10 +137,13 @@ fn map_rust_child_diagnostic(
    let mut edit_map: HashMap<Url, Vec<TextEdit>> = HashMap::new();
    for &span in &spans {
-        if let Some(suggested_replacement) = &span.suggested_replacement {
+        match (&span.suggestion_applicability, &span.suggested_replacement) {
-            let location = map_span_to_location(span, workspace_root);
+            (Some(Applicability::MachineApplicable), Some(suggested_replacement)) => {
-            let edit = TextEdit::new(location.range, suggested_replacement.clone());
+                let location = map_span_to_location(span, workspace_root);
-            edit_map.entry(location.uri).or_default().push(edit);
+                let edit = TextEdit::new(location.range, suggested_replacement.clone());
+                edit_map.entry(location.uri).or_default().push(edit);
+            }
+            _ => {}
        }
    }
diff --git a/crates/ra_cargo_watch/src/conv/snapshots/ra_cargo_watch__conv__test__snap_clippy_pass_by_ref.snap b/crates/ra_cargo_watch/src/conv/snapshots/ra_cargo_watch__conv__test__snap_clippy_pass_by_ref.snap
index 9e8f4eff4..a59fa84fa 100644
--- a/crates/ra_cargo_watch/src/conv/snapshots/ra_cargo_watch__conv__test__snap_clippy_pass_by_ref.snap
+++ b/crates/ra_cargo_watch/src/conv/snapshots/ra_cargo_watch__conv__test__snap_clippy_pass_by_ref.snap
@@ -58,44 +58,26 @@ expression: diag
                        },
                        message: "lint level defined here",
                    },
+                    DiagnosticRelatedInformation {
+                        location: Location {
+                            uri: "file:///test/compiler/mir/tagset.rs",
+                            range: Range {
+                                start: Position {
+                                    line: 41,
+                                    character: 23,
+                                },
+                                end: Position {
+                                    line: 41,
+                                    character: 28,
+                                },
+                            },
+                        },
+                        message: "consider passing by value instead",
+                    },
                ],
            ),
            tags: None,
        },
-        fixes: [
+        fixes: [],
-            CodeAction {
-                title: "consider passing by value instead",
-                kind: Some(
-                    "quickfix",
-                ),
-                diagnostics: None,
-                edit: Some(
-                    WorkspaceEdit {
-                        changes: Some(
-                            {
-                                "file:///test/compiler/mir/tagset.rs": [
-                                    TextEdit {
-                                        range: Range {
-                                            start: Position {
-                                                line: 41,
-                                                character: 23,
-                                            },
-                                            end: Position {
-                                                line: 41,
-                                                character: 28,
-                                            },
-                                        },
-                                        new_text: "self",
-                                    },
-                                ],
-                            },
-                        ),
-                        document_changes: None,
-                    },
-                ),
-                command: None,
-                is_preferred: None,
-            },
-        ],
    },
 ]
diff --git a/crates/ra_cargo_watch/src/lib.rs b/crates/ra_cargo_watch/src/lib.rs
index bffe5eb00..7c525c430 100644
--- a/crates/ra_cargo_watch/src/lib.rs
+++ b/crates/ra_cargo_watch/src/lib.rs
@@ -8,9 +8,10 @@ use lsp_types::{
    WorkDoneProgressEnd, WorkDoneProgressReport,
 };
 use std::{
+    error, fmt,
    io::{BufRead, BufReader},
    path::{Path, PathBuf},
-    process::{Child, Command, Stdio},
+    process::{Command, Stdio},
    thread::JoinHandle,
    time::Instant,
 };
@@ -70,10 +71,10 @@ impl std::ops::Drop for CheckWatcher {
    fn drop(&mut self) {
        if let Some(handle) = self.handle.take() {
            // Take the sender out of the option
-            let recv = self.cmd_send.take();
+            let cmd_send = self.cmd_send.take();
            // Dropping the sender finishes the thread loop
-            drop(recv);
+            drop(cmd_send);
            // Join the thread, it should finish shortly. We don't really care
            // whether it panicked, so it is safe to ignore the result
@@ -246,11 +247,21 @@ enum CheckEvent {
    End,
 }
+#[derive(Debug)]
+pub struct CargoError(String);
+impl fmt::Display for CargoError {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        write!(f, "Cargo failed: {}", self.0)
+    }
+}
+impl error::Error for CargoError {}
 pub fn run_cargo(
    args: &[String],
    current_dir: Option<&Path>,
    on_message: &mut dyn FnMut(cargo_metadata::Message) -> bool,
-) -> Child {
+) -> Result<(), CargoError> {
    let mut command = Command::new("cargo");
    if let Some(current_dir) = current_dir {
        command.current_dir(current_dir);
@@ -273,6 +284,8 @@ pub fn run_cargo(
    // simply skip a line if it doesn't parse, which just ignores any
    // erroneus output.
    let stdout = BufReader::new(child.stdout.take().unwrap());
+    let mut read_at_least_one_message = false;
    for line in stdout.lines() {
        let line = match line {
            Ok(line) => line,
@@ -291,12 +304,31 @@ pub fn run_cargo(
            }
        };
+        read_at_least_one_message = true;
        if !on_message(message) {
            break;
        }
    }
-    child
+    // It is okay to ignore the result, as it only errors if the process is already dead
+    let _ = child.kill();
+    let err_msg = match child.wait() {
+        Ok(exit_code) if !exit_code.success() && !read_at_least_one_message => {
+            // FIXME: Read the stderr to display the reason, see `read2()` reference in PR comment:
+            // https://github.com/rust-analyzer/rust-analyzer/pull/3632#discussion_r395605298
+            format!(
+                "the command produced no valid metadata (exit code: {:?}): cargo {}",
+                exit_code,
+                args.join(" ")
+            )
+        }
+        Err(err) => format!("io error: {:?}", err),
+        Ok(_) => return Ok(()),
+    };
+    Err(CargoError(err_msg))
 }
 impl WatchThread {
@@ -325,7 +357,7 @@ impl WatchThread {
                // which will break out of the loop, and continue the shutdown
                let _ = message_send.send(CheckEvent::Begin);
-                let mut child = run_cargo(&args, Some(&workspace_root), &mut |message| {
+                let res = run_cargo(&args, Some(&workspace_root), &mut |message| {
                    // Skip certain kinds of messages to only spend time on what's useful
                    match &message {
                        Message::CompilerArtifact(artifact) if artifact.fresh => return true,
@@ -334,26 +366,19 @@ impl WatchThread {
                        _ => {}
                    }
-                    match message_send.send(CheckEvent::Msg(message)) {
+                    // if the send channel was closed, we want to shutdown
-                        Ok(()) => {}
+                    message_send.send(CheckEvent::Msg(message)).is_ok()
-                        Err(_err) => {
-                            // The send channel was closed, so we want to shutdown
-                            return false;
-                        }
-                    };
-                    true
                });
+                if let Err(err) = res {
+                    // FIXME: make the `message_send` to be `Sender<Result<CheckEvent, CargoError>>`
+                    // to display user-caused misconfiguration errors instead of just logging them here
+                    log::error!("Cargo watcher failed {:?}", err);
+                }
                // We can ignore any error here, as we are already in the progress
                // of shutting down.
                let _ = message_send.send(CheckEvent::End);
-                // It is okay to ignore the result, as it only errors if the process is already dead
-                let _ = child.kill();
-                // Again, we don't care about the exit status so just ignore the result
-                let _ = child.wait();
            }))
        } else {
            None
diff --git a/crates/ra_hir/src/lib.rs b/crates/ra_hir/src/lib.rs
index 9f59d590c..713d45f48 100644
--- a/crates/ra_hir/src/lib.rs
+++ b/crates/ra_hir/src/lib.rs
@@ -4,6 +4,18 @@
 //! The principal difference between HIR and syntax trees is that HIR is bound
 //! to a particular crate instance. That is, it has cfg flags and features
 //! applied. So, the relation between syntax and HIR is many-to-one.
+//!
+//! HIR is the public API of the all of the compiler logic above syntax trees.
+//! It is written in "OO" style. Each type is self contained (as in, it knows it's
+//! parents and full context). It should be "clean code".
+//!
+//! `ra_hir_*` crates are the implementation of the compiler logic.
+//! They are written in "ECS" style, with relatively little abstractions.
+//! Many types are not self-contained, and explicitly use local indexes, arenas, etc.
+//!
+//! `ra_hir` is what insulates the "we don't know how to actually write an incremental compiler"
+//! from the ide with completions, hovers, etc. It is a (soft, internal) boundary:
+//! https://www.tedinski.com/2018/02/06/system-boundaries.html.
 #![recursion_limit = "512"]
diff --git a/crates/ra_hir/src/semantics.rs b/crates/ra_hir/src/semantics.rs
index 55e634528..d982f6ffa 100644
--- a/crates/ra_hir/src/semantics.rs
+++ b/crates/ra_hir/src/semantics.rs
@@ -12,7 +12,8 @@ use hir_expand::ExpansionInfo;
 use ra_db::{FileId, FileRange};
 use ra_prof::profile;
 use ra_syntax::{
-    algo::skip_trivia_token, ast, AstNode, Direction, SyntaxNode, SyntaxToken, TextRange, TextUnit,
+    algo::{find_node_at_offset, skip_trivia_token},
+    ast, AstNode, Direction, SyntaxNode, SyntaxToken, TextRange, TextUnit,
 };
 use rustc_hash::{FxHashMap, FxHashSet};
@@ -108,6 +109,17 @@ impl<'db, DB: HirDatabase> Semantics<'db, DB> {
        token.value
    }
+    pub fn descend_node_at_offset<N: ast::AstNode>(
+        &self,
+        node: &SyntaxNode,
+        offset: TextUnit,
+    ) -> Option<N> {
+        // Handle macro token cases
+        node.token_at_offset(offset)
+            .map(|token| self.descend_into_macros(token))
+            .find_map(|it| self.ancestors_with_macros(it.parent()).find_map(N::cast))
+    }
    pub fn original_range(&self, node: &SyntaxNode) -> FileRange {
        let node = self.find_file(node.clone());
        original_range(self.db, node.as_ref())
@@ -129,6 +141,8 @@ impl<'db, DB: HirDatabase> Semantics<'db, DB> {
            .kmerge_by(|node1, node2| node1.text_range().len() < node2.text_range().len())
    }
+    /// Find a AstNode by offset inside SyntaxNode, if it is inside *Macrofile*,
+    /// search up until it is of the target AstNode type
    pub fn find_node_at_offset_with_macros<N: AstNode>(
        &self,
        node: &SyntaxNode,
@@ -137,6 +151,19 @@ impl<'db, DB: HirDatabase> Semantics<'db, DB> {
        self.ancestors_at_offset_with_macros(node, offset).find_map(N::cast)
    }
+    /// Find a AstNode by offset inside SyntaxNode, if it is inside *MacroCall*,
+    /// descend it and find again
+    pub fn find_node_at_offset_with_descend<N: AstNode>(
+        &self,
+        node: &SyntaxNode,
+        offset: TextUnit,
+    ) -> Option<N> {
+        if let Some(it) = find_node_at_offset(&node, offset) {
+            return Some(it);
+        }
+        self.descend_node_at_offset(&node, offset)
+    }
    pub fn type_of_expr(&self, expr: &ast::Expr) -> Option<Type> {
        self.analyze(expr.syntax()).type_of(self.db, &expr)
    }
diff --git a/crates/ra_hir_def/src/body.rs b/crates/ra_hir_def/src/body.rs
index 27a297e8b..5f9d53ecb 100644
--- a/crates/ra_hir_def/src/body.rs
+++ b/crates/ra_hir_def/src/body.rs
@@ -30,6 +30,7 @@ pub(crate) struct Expander {
    hygiene: Hygiene,
    ast_id_map: Arc<AstIdMap>,
    module: ModuleId,
+    recursive_limit: usize,
 }
 impl Expander {
@@ -41,7 +42,7 @@ impl Expander {
        let crate_def_map = db.crate_def_map(module.krate);
        let hygiene = Hygiene::new(db.upcast(), current_file_id);
        let ast_id_map = db.ast_id_map(current_file_id);
-        Expander { crate_def_map, current_file_id, hygiene, ast_id_map, module }
+        Expander { crate_def_map, current_file_id, hygiene, ast_id_map, module, recursive_limit: 0 }
    }
    pub(crate) fn enter_expand<T: ast::AstNode>(
@@ -50,6 +51,10 @@ impl Expander {
        local_scope: Option<&ItemScope>,
        macro_call: ast::MacroCall,
    ) -> Option<(Mark, T)> {
+        if self.recursive_limit > 1024 {
+            return None;
+        }
        let macro_call = InFile::new(self.current_file_id, &macro_call);
        if let Some(call_id) = macro_call.as_call_id(db, |path| {
@@ -73,6 +78,7 @@ impl Expander {
                    self.hygiene = Hygiene::new(db.upcast(), file_id);
                    self.current_file_id = file_id;
                    self.ast_id_map = db.ast_id_map(file_id);
+                    self.recursive_limit += 1;
                    return Some((mark, expr));
                }
@@ -88,6 +94,7 @@ impl Expander {
        self.hygiene = Hygiene::new(db.upcast(), mark.file_id);
        self.current_file_id = mark.file_id;
        self.ast_id_map = mem::take(&mut mark.ast_id_map);
+        self.recursive_limit -= 1;
        mark.bomb.defuse();
    }
diff --git a/crates/ra_hir_def/src/body/lower.rs b/crates/ra_hir_def/src/body/lower.rs
index e8c58ed32..3cf0c66ea 100644
--- a/crates/ra_hir_def/src/body/lower.rs
+++ b/crates/ra_hir_def/src/body/lower.rs
@@ -453,7 +453,7 @@ impl ExprCollector<'_> {
                }
            }
            ast::Expr::MacroCall(e) => {
-                if let Some(name) = is_macro_rules(&e) {
+                if let Some(name) = e.is_macro_rules().map(|it| it.as_name()) {
                    let mac = MacroDefId {
                        krate: Some(self.expander.module.krate),
                        ast_id: Some(self.expander.ast_id(&e)),
@@ -697,16 +697,6 @@ impl ExprCollector<'_> {
    }
 }
-fn is_macro_rules(m: &ast::MacroCall) -> Option<Name> {
-    let name = m.path()?.segment()?.name_ref()?.as_name();
-    if name == name![macro_rules] {
-        Some(m.name()?.as_name())
-    } else {
-        None
-    }
-}
 impl From<ast::BinOp> for BinaryOp {
    fn from(ast_op: ast::BinOp) -> Self {
        match ast_op {
diff --git a/crates/ra_hir_expand/src/db.rs b/crates/ra_hir_expand/src/db.rs
index d171d2dfd..5a696542f 100644
--- a/crates/ra_hir_expand/src/db.rs
+++ b/crates/ra_hir_expand/src/db.rs
@@ -6,7 +6,7 @@ use mbe::{ExpandResult, MacroRules};
 use ra_db::{salsa, SourceDatabase};
 use ra_parser::FragmentKind;
 use ra_prof::profile;
-use ra_syntax::{AstNode, Parse, SyntaxKind::*, SyntaxNode};
+use ra_syntax::{algo::diff, AstNode, Parse, SyntaxKind::*, SyntaxNode};
 use crate::{
    ast_id_map::AstIdMap, BuiltinDeriveExpander, BuiltinFnLikeExpander, EagerCallLoc, EagerMacroId,
@@ -238,7 +238,7 @@ pub fn parse_macro_with_arg(
    } else {
        db.macro_expand(macro_call_id)
    };
-    if let Some(err) = err {
+    if let Some(err) = &err {
        // Note:
        // The final goal we would like to make all parse_macro success,
        // such that the following log will not call anyway.
@@ -272,7 +272,25 @@ pub fn parse_macro_with_arg(
    let fragment_kind = to_fragment_kind(db, macro_call_id);
    let (parse, rev_token_map) = mbe::token_tree_to_syntax_node(&tt, fragment_kind).ok()?;
-    Some((parse, Arc::new(rev_token_map)))
+    if err.is_none() {
+        Some((parse, Arc::new(rev_token_map)))
+    } else {
+        // FIXME:
+        // In future, we should propagate the actual error with recovery information
+        // instead of ignore the error here.
+        // Safe check for recurisve identity macro
+        let node = parse.syntax_node();
+        let file: HirFileId = macro_file.into();
+        let call_node = file.call_node(db)?;
+        if !diff(&node, &call_node.value).is_empty() {
+            Some((parse, Arc::new(rev_token_map)))
+        } else {
+            None
+        }
+    }
 }
 /// Given a `MacroCallId`, return what `FragmentKind` it belongs to.
diff --git a/crates/ra_hir_ty/src/tests/regression.rs b/crates/ra_hir_ty/src/tests/regression.rs
index 14c8ed3a9..a02e3ee05 100644
--- a/crates/ra_hir_ty/src/tests/regression.rs
+++ b/crates/ra_hir_ty/src/tests/regression.rs
@@ -453,3 +453,34 @@ pub mod str {
    // should be Option<char>, but currently not because of Chalk ambiguity problem
    assert_eq!("(Option<{unknown}>, Option<{unknown}>)", super::type_at_pos(&db, pos));
 }
+#[test]
+fn issue_3642_bad_macro_stackover() {
+    let (db, pos) = TestDB::with_position(
+        r#"
+//- /main.rs
+#[macro_export]
+macro_rules! match_ast {
+    (match $node:ident { $($tt:tt)* }) => { match_ast!(match ($node) { $($tt)* }) };
+    (match ($node:expr) {
+        $( ast::$ast:ident($it:ident) => $res:expr, )*
+        _ => $catch_all:expr $(,)?
+    }) => {{
+        $( if let Some($it) = ast::$ast::cast($node.clone()) { $res } else )*
+        { $catch_all }
+    }};
+}
+fn main() {
+    let anchor<|> = match_ast! {
+        match parent {
+            as => {},
+            _ => return None
+        }
+    };
+}"#,
+    );
+    assert_eq!("()", super::type_at_pos(&db, pos));
+}
diff --git a/crates/ra_ide/src/references.rs b/crates/ra_ide/src/references.rs
index 3ea0ac230..746cc86ba 100644
--- a/crates/ra_ide/src/references.rs
+++ b/crates/ra_ide/src/references.rs
@@ -94,12 +94,16 @@ pub(crate) fn find_all_refs(
    let sema = Semantics::new(db);
    let syntax = sema.parse(position.file_id).syntax().clone();
-    let (opt_name, search_kind) =
+    let (opt_name, search_kind) = if let Some(name) =
-        if let Some(name) = get_struct_def_name_for_struct_literal_search(&syntax, position) {
+        get_struct_def_name_for_struct_literal_search(&sema, &syntax, position)
-            (Some(name), ReferenceKind::StructLiteral)
+    {
-        } else {
+        (Some(name), ReferenceKind::StructLiteral)
-            (find_node_at_offset::<ast::Name>(&syntax, position.offset), ReferenceKind::Other)
+    } else {
-        };
+        (
+            sema.find_node_at_offset_with_descend::<ast::Name>(&syntax, position.offset),
+            ReferenceKind::Other,
+        )
+    };
    let RangeInfo { range, info: def } = find_name(&sema, &syntax, position, opt_name)?;
@@ -131,7 +135,8 @@ fn find_name(
        let range = name.syntax().text_range();
        return Some(RangeInfo::new(range, def));
    }
-    let name_ref = find_node_at_offset::<ast::NameRef>(&syntax, position.offset)?;
+    let name_ref =
+        sema.find_node_at_offset_with_descend::<ast::NameRef>(&syntax, position.offset)?;
    let def = classify_name_ref(sema, &name_ref)?.definition();
    let range = name_ref.syntax().text_range();
    Some(RangeInfo::new(range, def))
@@ -157,6 +162,7 @@ fn decl_access(def: &Definition, syntax: &SyntaxNode, range: TextRange) -> Optio
 }
 fn get_struct_def_name_for_struct_literal_search(
+    sema: &Semantics<RootDatabase>,
    syntax: &SyntaxNode,
    position: FilePosition,
 ) -> Option<ast::Name> {
@@ -164,10 +170,18 @@ fn get_struct_def_name_for_struct_literal_search(
        if right.kind() != SyntaxKind::L_CURLY && right.kind() != SyntaxKind::L_PAREN {
            return None;
        }
-        if let Some(name) = find_node_at_offset::<ast::Name>(&syntax, left.text_range().start()) {
+        if let Some(name) =
+            sema.find_node_at_offset_with_descend::<ast::Name>(&syntax, left.text_range().start())
+        {
            return name.syntax().ancestors().find_map(ast::StructDef::cast).and_then(|l| l.name());
        }
-        if find_node_at_offset::<ast::TypeParamList>(&syntax, left.text_range().start()).is_some() {
+        if sema
+            .find_node_at_offset_with_descend::<ast::TypeParamList>(
+                &syntax,
+                left.text_range().start(),
+            )
+            .is_some()
+        {
            return left.ancestors().find_map(ast::StructDef::cast).and_then(|l| l.name());
        }
    }
diff --git a/crates/ra_ide/src/references/rename.rs b/crates/ra_ide/src/references/rename.rs
index 7d1190af9..9acc6158a 100644
--- a/crates/ra_ide/src/references/rename.rs
+++ b/crates/ra_ide/src/references/rename.rs
@@ -250,6 +250,63 @@ mod tests {
    }
    #[test]
+    fn test_rename_for_macro_args_rev() {
+        test_rename(
+            r#"
+    macro_rules! foo {($i:ident) => {$i} }
+    fn main() {
+        let a = "test";
+        foo!(a<|>);
+    }"#,
+            "b",
+            r#"
+    macro_rules! foo {($i:ident) => {$i} }
+    fn main() {
+        let b = "test";
+        foo!(b);
+    }"#,
+        );
+    }
+    #[test]
+    fn test_rename_for_macro_define_fn() {
+        test_rename(
+            r#"
+    macro_rules! define_fn {($id:ident) => { fn $id{} }}
+    define_fn!(foo);
+    fn main() {
+        fo<|>o();
+    }"#,
+            "bar",
+            r#"
+    macro_rules! define_fn {($id:ident) => { fn $id{} }}
+    define_fn!(bar);
+    fn main() {
+        bar();
+    }"#,
+        );
+    }
+    #[test]
+    fn test_rename_for_macro_define_fn_rev() {
+        test_rename(
+            r#"
+    macro_rules! define_fn {($id:ident) => { fn $id{} }}
+    define_fn!(fo<|>o);
+    fn main() {
+        foo();
+    }"#,
+            "bar",
+            r#"
+    macro_rules! define_fn {($id:ident) => { fn $id{} }}
+    define_fn!(bar);
+    fn main() {
+        bar();
+    }"#,
+        );
+    }
+    #[test]
    fn test_rename_for_param_inside() {
        test_rename(
            r#"
diff --git a/crates/ra_ide_db/src/search.rs b/crates/ra_ide_db/src/search.rs
index cf78d3e41..117454695 100644
--- a/crates/ra_ide_db/src/search.rs
+++ b/crates/ra_ide_db/src/search.rs
@@ -10,9 +10,7 @@ use hir::{DefWithBody, HasSource, ModuleSource, Semantics};
 use once_cell::unsync::Lazy;
 use ra_db::{FileId, FileRange, SourceDatabaseExt};
 use ra_prof::profile;
-use ra_syntax::{
+use ra_syntax::{ast, match_ast, AstNode, TextRange, TextUnit};
-    algo::find_node_at_offset, ast, match_ast, AstNode, TextRange, TextUnit, TokenAtOffset,
-};
 use rustc_hash::FxHashMap;
 use test_utils::tested_by;
@@ -219,21 +217,11 @@ impl Definition {
                    continue;
                }
-                let name_ref =
+                let name_ref: ast::NameRef =
-                    if let Some(name_ref) = find_node_at_offset::<ast::NameRef>(&tree, offset) {
+                    if let Some(name_ref) = sema.find_node_at_offset_with_descend(&tree, offset) {
                        name_ref
                    } else {
-                        // Handle macro token cases
+                        continue;
-                        let token = match tree.token_at_offset(offset) {
-                            TokenAtOffset::None => continue,
-                            TokenAtOffset::Single(t) => t,
-                            TokenAtOffset::Between(_, t) => t,
-                        };
-                        let expanded = sema.descend_into_macros(token);
-                        match ast::NameRef::cast(expanded.parent()) {
-                            Some(name_ref) => name_ref,
-                            _ => continue,
-                        }
                    };
                // FIXME: reuse sb
diff --git a/crates/ra_ide_db/src/symbol_index.rs b/crates/ra_ide_db/src/symbol_index.rs
index 884359ee3..0f46f93c1 100644
--- a/crates/ra_ide_db/src/symbol_index.rs
+++ b/crates/ra_ide_db/src/symbol_index.rs
@@ -362,6 +362,13 @@ fn to_symbol(node: &SyntaxNode) -> Option<(SmolStr, SyntaxNodePtr, TextRange)> {
            ast::TypeAliasDef(it) => { decl(it) },
            ast::ConstDef(it) => { decl(it) },
            ast::StaticDef(it) => { decl(it) },
+            ast::MacroCall(it) => {
+                if it.is_macro_rules().is_some() {
+                    decl(it)
+                } else {
+                    None
+                }
+            },
            _ => None,
        }
    }
diff --git a/crates/ra_project_model/src/cargo_workspace.rs b/crates/ra_project_model/src/cargo_workspace.rs
index c2857dbfc..c7f9bd873 100644
--- a/crates/ra_project_model/src/cargo_workspace.rs
+++ b/crates/ra_project_model/src/cargo_workspace.rs
@@ -6,7 +6,7 @@ use std::{
 };
 use anyhow::{Context, Result};
-use cargo_metadata::{CargoOpt, Message, MetadataCommand, PackageId};
+use cargo_metadata::{BuildScript, CargoOpt, Message, MetadataCommand, PackageId};
 use ra_arena::{Arena, Idx};
 use ra_cargo_watch::run_cargo;
 use ra_db::Edition;
@@ -254,7 +254,7 @@ pub fn load_out_dirs(
        "check".to_string(),
        "--message-format=json".to_string(),
        "--manifest-path".to_string(),
-        format!("{}", cargo_toml.display()),
+        cargo_toml.display().to_string(),
    ];
    if cargo_features.all_features {
@@ -263,19 +263,15 @@ pub fn load_out_dirs(
        // FIXME: `NoDefaultFeatures` is mutual exclusive with `SomeFeatures`
        // https://github.com/oli-obk/cargo_metadata/issues/79
        args.push("--no-default-features".to_string());
-    } else if !cargo_features.features.is_empty() {
+    } else {
-        for feature in &cargo_features.features {
+        args.extend(cargo_features.features.iter().cloned());
-            args.push(feature.clone());
-        }
    }
-    let mut res = FxHashMap::default();
+    let mut acc = FxHashMap::default();
-    let mut child = run_cargo(&args, cargo_toml.parent(), &mut |message| {
+    let res = run_cargo(&args, cargo_toml.parent(), &mut |message| {
        match message {
-            Message::BuildScriptExecuted(message) => {
+            Message::BuildScriptExecuted(BuildScript { package_id, out_dir, .. }) => {
-                let package_id = message.package_id;
+                acc.insert(package_id, out_dir);
-                let out_dir = message.out_dir;
-                res.insert(package_id, out_dir);
            }
            Message::CompilerArtifact(_) => (),
@@ -285,6 +281,9 @@ pub fn load_out_dirs(
        true
    });
-    let _ = child.wait();
+    if let Err(err) = res {
-    res
+        log::error!("Failed to load outdirs: {:?}", err);
+    }
+    acc
 }
diff --git a/crates/ra_project_model/src/lib.rs b/crates/ra_project_model/src/lib.rs
index b500a74fb..a3ef9acdc 100644
--- a/crates/ra_project_model/src/lib.rs
+++ b/crates/ra_project_model/src/lib.rs
@@ -336,7 +336,7 @@ impl ProjectWorkspace {
                                extern_source,
                            );
                            if cargo[tgt].kind == TargetKind::Lib {
-                                lib_tgt = Some(crate_id);
+                                lib_tgt = Some((crate_id, cargo[tgt].name.clone()));
                                pkg_to_lib_crate.insert(pkg, crate_id);
                            }
                            if cargo[tgt].is_proc_macro {
@@ -363,7 +363,7 @@ impl ProjectWorkspace {
                    // Set deps to the core, std and to the lib target of the current package
                    for &from in pkg_crates.get(&pkg).into_iter().flatten() {
-                        if let Some(to) = lib_tgt {
+                        if let Some((to, name)) = lib_tgt.clone() {
                            if to != from
                                && crate_graph
                                    .add_dep(
@@ -371,7 +371,7 @@ impl ProjectWorkspace {
                                        // For root projects with dashes in their name,
                                        // cargo metadata does not do any normalization,
                                        // so we do it ourselves currently
-                                        CrateName::normalize_dashes(&cargo[pkg].name),
+                                        CrateName::normalize_dashes(&name),
                                        to,
                                    )
                                    .is_err()
diff --git a/crates/ra_syntax/src/algo.rs b/crates/ra_syntax/src/algo.rs
index 344cf0fbe..ffdbdc767 100644
--- a/crates/ra_syntax/src/algo.rs
+++ b/crates/ra_syntax/src/algo.rs
@@ -95,6 +95,10 @@ impl TreeDiff {
            builder.replace(from.text_range(), to.to_string())
        }
    }
+    pub fn is_empty(&self) -> bool {
+        self.replacements.is_empty()
+    }
 }
 /// Finds minimal the diff, which, applied to `from`, will result in `to`.
diff --git a/crates/ra_syntax/src/ast/extensions.rs b/crates/ra_syntax/src/ast/extensions.rs
index c3ae8f90e..392731dac 100644
--- a/crates/ra_syntax/src/ast/extensions.rs
+++ b/crates/ra_syntax/src/ast/extensions.rs
@@ -4,7 +4,7 @@
 use itertools::Itertools;
 use crate::{
-    ast::{self, child_opt, children, AstNode, AttrInput, SyntaxNode},
+    ast::{self, child_opt, children, AstNode, AttrInput, NameOwner, SyntaxNode},
    SmolStr, SyntaxElement,
    SyntaxKind::*,
    SyntaxToken, T,
@@ -514,3 +514,14 @@ impl ast::Visibility {
        self.syntax().children_with_tokens().any(|it| it.kind() == T![super])
    }
 }
+impl ast::MacroCall {
+    pub fn is_macro_rules(&self) -> Option<ast::Name> {
+        let name_ref = self.path()?.segment()?.name_ref()?;
+        if name_ref.text() == "macro_rules" {
+            self.name()
+        } else {
+            None
+        }
+    }
+}
diff --git a/xtask/src/codegen/gen_assists_docs.rs b/xtask/src/codegen/gen_assists_docs.rs
index 6da5ca89e..31d606535 100644
--- a/xtask/src/codegen/gen_assists_docs.rs
+++ b/xtask/src/codegen/gen_assists_docs.rs
@@ -4,7 +4,7 @@ use std::{fs, path::Path};
 use crate::{
    codegen::{self, extract_comment_blocks_with_empty_lines, Mode},
-    project_root, Result,
+    project_root, rust_files, Result,
 };
 pub fn generate_assists_docs(mode: Mode) -> Result<()> {
@@ -46,12 +46,8 @@ fn reveal_hash_comments(text: &str) -> String {
 fn collect_assists() -> Result<Vec<Assist>> {
    let mut res = Vec::new();
-    for entry in fs::read_dir(project_root().join(codegen::ASSISTS_DIR))? {
+    for path in rust_files(&project_root().join(codegen::ASSISTS_DIR)) {
-        let entry = entry?;
+        collect_file(&mut res, path.as_path())?;
-        let path = entry.path();
-        if path.is_file() {
-            collect_file(&mut res, path.as_path())?;
-        }
    }
    res.sort_by(|lhs, rhs| lhs.id.cmp(&rhs.id));
    return Ok(res);
diff --git a/xtask/src/lib.rs b/xtask/src/lib.rs
index e1472e85d..4f01f84fb 100644
--- a/xtask/src/lib.rs
+++ b/xtask/src/lib.rs
@@ -17,6 +17,7 @@ use std::{
    path::{Path, PathBuf},
    process::{Command, Stdio},
 };
+use walkdir::{DirEntry, WalkDir};
 use crate::{
    codegen::Mode,
@@ -37,6 +38,21 @@ pub fn project_root() -> PathBuf {
    .to_path_buf()
 }
+pub fn rust_files(path: &Path) -> impl Iterator<Item = PathBuf> {
+    let iter = WalkDir::new(path);
+    return iter
+        .into_iter()
+        .filter_entry(|e| !is_hidden(e))
+        .map(|e| e.unwrap())
+        .filter(|e| !e.file_type().is_dir())
+        .map(|e| e.into_path())
+        .filter(|path| path.extension().map(|it| it == "rs").unwrap_or(false));
+    fn is_hidden(entry: &DirEntry) -> bool {
+        entry.file_name().to_str().map(|s| s.starts_with('.')).unwrap_or(false)
+    }
+}
 pub fn run_rustfmt(mode: Mode) -> Result<()> {
    let _dir = pushd(project_root());
    ensure_rustfmt()?;
diff --git a/xtask/tests/tidy-tests/main.rs b/xtask/tests/tidy-tests/main.rs
index 5ae86c87c..80911a68e 100644
--- a/xtask/tests/tidy-tests/main.rs
+++ b/xtask/tests/tidy-tests/main.rs
@@ -5,13 +5,12 @@ use std::{
    path::{Path, PathBuf},
 };
-use walkdir::{DirEntry, WalkDir};
+use xtask::{not_bash::fs2, project_root, rust_files};
-use xtask::{not_bash::fs2, project_root};
 #[test]
 fn rust_files_are_tidy() {
    let mut tidy_docs = TidyDocs::default();
-    for path in rust_files() {
+    for path in rust_files(&project_root().join("crates")) {
        let text = fs2::read_to_string(&path).unwrap();
        check_todo(&path, &text);
        check_trailing_ws(&path, &text);
@@ -142,19 +141,3 @@ fn is_exclude_dir(p: &Path, dirs_to_exclude: &[&str]) -> bool {
    false
 }
-fn rust_files() -> impl Iterator<Item = PathBuf> {
-    let crates = project_root().join("crates");
-    let iter = WalkDir::new(crates);
-    return iter
-        .into_iter()
-        .filter_entry(|e| !is_hidden(e))
-        .map(|e| e.unwrap())
-        .filter(|e| !e.file_type().is_dir())
-        .map(|e| e.into_path())
-        .filter(|path| path.extension().map(|it| it == "rs").unwrap_or(false));
-    fn is_hidden(entry: &DirEntry) -> bool {
-        entry.file_name().to_str().map(|s| s.starts_with('.')).unwrap_or(false)
-    }
-}