Repository: rust-analyzer/rust-analyzer
Pull request #3429 (2020-03-03 18:46:59)

Fix panic on eager expansion
When lazy expanding inside an eager macro, its *parent* file of that lazy macro call must be already exists such that a panic is occurred because that parent file is the eager macro we are processing.

This PR fix this bug by store the argument syntax node as another eager macro id for that purpose.

Personally I don't know if it is a good answer for this bug. 





Patch:
diff --git a/crates/ra_hir_expand/src/eager.rs b/crates/ra_hir_expand/src/eager.rs
index 7fcdfab5ae..f95f37ede1 100644
--- a/crates/ra_hir_expand/src/eager.rs
+++ b/crates/ra_hir_expand/src/eager.rs
@@ -37,9 +37,25 @@ pub fn expand_eager_macro(
 ) -> Option<EagerMacroId> {
     let args = macro_call.value.token_tree()?;
     let parsed_args = mbe::ast_to_token_tree(&args)?.0;
-    let parsed_args = mbe::token_tree_to_syntax_node(&parsed_args, FragmentKind::Expr).ok()?.0;
-    let result = eager_macro_recur(db, macro_call.with_value(parsed_args.syntax_node()), resolver)?;
 
+    // Note:
+    // When `lazy_expand` is called, its *parent* file must be already exists.
+    // Here we store an eager macro id for the argument expanded subtree here
+    // for that purpose.
+    let arg_id: MacroCallId = db
+        .intern_eager_expansion({
+            EagerCallLoc {
+                def,
+                fragment: FragmentKind::Expr,
+                subtree: Arc::new(parsed_args.clone()),
+                file_id: macro_call.file_id,
+            }
+        })
+        .into();
+
+    let parsed_args = mbe::token_tree_to_syntax_node(&parsed_args, FragmentKind::Expr).ok()?.0;
+    let result =
+        eager_macro_recur(db, InFile::new(arg_id.as_file(), parsed_args.syntax_node()), resolver)?;
     let subtree = to_subtree(&result)?;
 
     if let MacroDefKind::BuiltInEager(eager) = def.kind {
diff --git a/crates/ra_ide/src/display/navigation_target.rs b/crates/ra_ide/src/display/navigation_target.rs
index d57451cc8c..c9c14561a8 100644
--- a/crates/ra_ide/src/display/navigation_target.rs
+++ b/crates/ra_ide/src/display/navigation_target.rs
@@ -3,7 +3,7 @@
 use either::Either;
 use hir::{original_range, AssocItem, FieldSource, HasSource, InFile, ModuleSource};
 use ra_db::{FileId, SourceDatabase};
-use ra_ide_db::{defs::Definition, RootDatabase};
+use ra_ide_db::RootDatabase;
 use ra_syntax::{
     ast::{self, DocCommentsOwner, NameOwner},
     match_ast, AstNode, SmolStr,
@@ -11,7 +11,11 @@ use ra_syntax::{
     TextRange,
 };
 
-use crate::FileSymbol;
+use crate::{
+    // expand::original_range,
+    references::Definition,
+    FileSymbol,
+};
 
 use super::short_label::ShortLabel;
 
diff --git a/crates/ra_ide/src/goto_definition.rs b/crates/ra_ide/src/goto_definition.rs
index 4a8107d60e..76ee232a3a 100644
--- a/crates/ra_ide/src/goto_definition.rs
+++ b/crates/ra_ide/src/goto_definition.rs
@@ -1,10 +1,7 @@
 //! FIXME: write short doc here
 
 use hir::Semantics;
-use ra_ide_db::{
-    defs::{classify_name, classify_name_ref},
-    symbol_index, RootDatabase,
-};
+use ra_ide_db::{defs::classify_name, symbol_index, RootDatabase};
 use ra_syntax::{
     ast::{self},
     match_ast, AstNode,
@@ -14,6 +11,7 @@ use ra_syntax::{
 
 use crate::{
     display::{ToNav, TryToNav},
+    references::classify_name_ref,
     FilePosition, NavigationTarget, RangeInfo,
 };
 
@@ -96,7 +94,7 @@ pub(crate) fn reference_definition(
 
 #[cfg(test)]
 mod tests {
-    use test_utils::assert_eq_text;
+    use test_utils::{assert_eq_text, covers};
 
     use crate::mock_analysis::analysis_and_position;
 
@@ -208,6 +206,7 @@ mod tests {
 
     #[test]
     fn goto_def_for_macros() {
+        covers!(goto_def_for_macros);
         check_goto(
             "
             //- /lib.rs
@@ -224,6 +223,7 @@ mod tests {
 
     #[test]
     fn goto_def_for_macros_from_other_crates() {
+        covers!(goto_def_for_macros);
         check_goto(
             "
             //- /lib.rs
@@ -335,6 +335,7 @@ mod tests {
 
     #[test]
     fn goto_def_for_methods() {
+        covers!(goto_def_for_methods);
         check_goto(
             "
             //- /lib.rs
@@ -354,6 +355,7 @@ mod tests {
 
     #[test]
     fn goto_def_for_fields() {
+        covers!(goto_def_for_fields);
         check_goto(
             "
             //- /lib.rs
@@ -372,6 +374,7 @@ mod tests {
 
     #[test]
     fn goto_def_for_record_fields() {
+        covers!(goto_def_for_record_fields);
         check_goto(
             "
             //- /lib.rs
@@ -784,6 +787,7 @@ mod tests {
 
     #[test]
     fn goto_def_for_field_init_shorthand() {
+        covers!(goto_def_for_field_init_shorthand);
         check_goto(
             "
             //- /lib.rs
diff --git a/crates/ra_ide/src/hover.rs b/crates/ra_ide/src/hover.rs
index e9c6825570..26c8996bc1 100644
--- a/crates/ra_ide/src/hover.rs
+++ b/crates/ra_ide/src/hover.rs
@@ -2,7 +2,7 @@
 
 use hir::{Adt, HasSource, HirDisplay, Semantics};
 use ra_ide_db::{
-    defs::{classify_name, classify_name_ref, Definition},
+    defs::{classify_name, Definition},
     RootDatabase,
 };
 use ra_syntax::{
@@ -14,6 +14,7 @@ use ra_syntax::{
 
 use crate::{
     display::{macro_label, rust_code_markup, rust_code_markup_with_doc, ShortLabel},
+    references::classify_name_ref,
     FilePosition, RangeInfo,
 };
 
diff --git a/crates/ra_ide/src/marks.rs b/crates/ra_ide/src/marks.rs
index 1236cb773c..7b8b727b4a 100644
--- a/crates/ra_ide/src/marks.rs
+++ b/crates/ra_ide/src/marks.rs
@@ -3,6 +3,11 @@
 test_utils::marks!(
     inserts_angle_brackets_for_generics
     inserts_parens_for_function_calls
+    goto_def_for_macros
+    goto_def_for_methods
+    goto_def_for_fields
+    goto_def_for_record_fields
+    goto_def_for_field_init_shorthand
     call_info_bad_offset
     dont_complete_current_use
     test_resolve_parent_module_on_module_decl
diff --git a/crates/ra_ide/src/references.rs b/crates/ra_ide/src/references.rs
index 67be3f6c9b..95a5c1914d 100644
--- a/crates/ra_ide/src/references.rs
+++ b/crates/ra_ide/src/references.rs
@@ -9,16 +9,14 @@
 //! at the index that the match starts at and its tree parent is
 //! resolved to the search element definition, we get a reference.
 
+mod classify;
 mod rename;
 mod search_scope;
 
 use hir::Semantics;
 use once_cell::unsync::Lazy;
 use ra_db::SourceDatabaseExt;
-use ra_ide_db::{
-    defs::{classify_name, classify_name_ref, Definition},
-    RootDatabase,
-};
+use ra_ide_db::RootDatabase;
 use ra_prof::profile;
 use ra_syntax::{
     algo::find_node_at_offset,
@@ -29,7 +27,11 @@ use test_utils::tested_by;
 
 use crate::{display::TryToNav, FilePosition, FileRange, NavigationTarget, RangeInfo};
 
-pub(crate) use self::rename::rename;
+pub(crate) use self::{
+    classify::{classify_name_ref, NameRefClass},
+    rename::rename,
+};
+pub(crate) use ra_ide_db::defs::{classify_name, Definition};
 
 pub use self::search_scope::SearchScope;
 
diff --git a/crates/ra_ide/src/references/classify.rs b/crates/ra_ide/src/references/classify.rs
new file mode 100644
index 0000000000..0bbf893f87
--- /dev/null
+++ b/crates/ra_ide/src/references/classify.rs
@@ -0,0 +1,84 @@
+//! Functions that are used to classify an element from its definition or reference.
+
+use hir::{Local, PathResolution, Semantics};
+use ra_ide_db::defs::Definition;
+use ra_ide_db::RootDatabase;
+use ra_prof::profile;
+use ra_syntax::{ast, AstNode};
+use test_utils::tested_by;
+
+pub enum NameRefClass {
+    Definition(Definition),
+    FieldShorthand { local: Local, field: Definition },
+}
+
+impl NameRefClass {
+    pub fn definition(self) -> Definition {
+        match self {
+            NameRefClass::Definition(def) => def,
+            NameRefClass::FieldShorthand { local, field: _ } => Definition::Local(local),
+        }
+    }
+}
+
+pub(crate) fn classify_name_ref(
+    sema: &Semantics<RootDatabase>,
+    name_ref: &ast::NameRef,
+) -> Option<NameRefClass> {
+    let _p = profile("classify_name_ref");
+
+    let parent = name_ref.syntax().parent()?;
+
+    if let Some(method_call) = ast::MethodCallExpr::cast(parent.clone()) {
+        tested_by!(goto_def_for_methods);
+        if let Some(func) = sema.resolve_method_call(&method_call) {
+            return Some(NameRefClass::Definition(Definition::ModuleDef(func.into())));
+        }
+    }
+
+    if let Some(field_expr) = ast::FieldExpr::cast(parent.clone()) {
+        tested_by!(goto_def_for_fields);
+        if let Some(field) = sema.resolve_field(&field_expr) {
+            return Some(NameRefClass::Definition(Definition::StructField(field)));
+        }
+    }
+
+    if let Some(record_field) = ast::RecordField::cast(parent.clone()) {
+        tested_by!(goto_def_for_record_fields);
+        tested_by!(goto_def_for_field_init_shorthand);
+        if let Some((field, local)) = sema.resolve_record_field(&record_field) {
+            let field = Definition::StructField(field);
+            let res = match local {
+                None => NameRefClass::Definition(field),
+                Some(local) => NameRefClass::FieldShorthand { field, local },
+            };
+            return Some(res);
+        }
+    }
+
+    if let Some(macro_call) = parent.ancestors().find_map(ast::MacroCall::cast) {
+        tested_by!(goto_def_for_macros);
+        if let Some(macro_def) = sema.resolve_macro_call(&macro_call) {
+            return Some(NameRefClass::Definition(Definition::Macro(macro_def)));
+        }
+    }
+
+    let path = name_ref.syntax().ancestors().find_map(ast::Path::cast)?;
+    let resolved = sema.resolve_path(&path)?;
+    let res = match resolved {
+        PathResolution::Def(def) => Definition::ModuleDef(def),
+        PathResolution::AssocItem(item) => {
+            let def = match item {
+                hir::AssocItem::Function(it) => it.into(),
+                hir::AssocItem::Const(it) => it.into(),
+                hir::AssocItem::TypeAlias(it) => it.into(),
+            };
+            Definition::ModuleDef(def)
+        }
+        PathResolution::Local(local) => Definition::Local(local),
+        PathResolution::TypeParam(par) => Definition::TypeParam(par),
+        PathResolution::Macro(def) => Definition::Macro(def),
+        PathResolution::SelfType(impl_def) => Definition::SelfType(impl_def),
+    };
+    Some(NameRefClass::Definition(res))
+}
diff --git a/crates/ra_ide/src/syntax_highlighting.rs b/crates/ra_ide/src/syntax_highlighting.rs
index 7fc94d3cdb..9e0ee21589 100644
--- a/crates/ra_ide/src/syntax_highlighting.rs
+++ b/crates/ra_ide/src/syntax_highlighting.rs
@@ -7,7 +7,7 @@ mod tests;
 
 use hir::{Name, Semantics};
 use ra_ide_db::{
-    defs::{classify_name, classify_name_ref, Definition, NameClass, NameRefClass},
+    defs::{classify_name, Definition, NameClass},
     RootDatabase,
 };
 use ra_prof::profile;
@@ -19,7 +19,11 @@ use ra_syntax::{
 };
 use rustc_hash::FxHashMap;
 
-use crate::{call_info::call_info_for_token, Analysis, FileId};
+use crate::{
+    call_info::call_info_for_token,
+    references::{classify_name_ref, NameRefClass},
+    Analysis, FileId,
+};
 
 pub(crate) use html::highlight_as_html;
 pub use tags::{Highlight, HighlightModifier, HighlightModifiers, HighlightTag};
diff --git a/crates/ra_ide_db/src/defs.rs b/crates/ra_ide_db/src/defs.rs
index f057435bf6..215daa4418 100644
--- a/crates/ra_ide_db/src/defs.rs
+++ b/crates/ra_ide_db/src/defs.rs
@@ -6,8 +6,8 @@
 // FIXME: this badly needs rename/rewrite (matklad, 2020-02-06).
 
 use hir::{
-    Adt, FieldSource, HasSource, ImplDef, Local, MacroDef, Module, ModuleDef, Name, PathResolution,
-    Semantics, StructField, TypeParam,
+    Adt, FieldSource, HasSource, ImplDef, Local, MacroDef, Module, ModuleDef, Name, Semantics,
+    StructField, TypeParam,
 };
 use ra_prof::profile;
 use ra_syntax::{
@@ -117,8 +117,6 @@ impl NameClass {
 }
 
 pub fn classify_name(sema: &Semantics<RootDatabase>, name: &ast::Name) -> Option<NameClass> {
-    let _p = profile("classify_name");
-
     if let Some(bind_pat) = name.syntax().parent().and_then(ast::BindPat::cast) {
         if let Some(def) = sema.resolve_bind_pat_to_const(&bind_pat) {
             return Some(NameClass::ConstReference(Definition::ModuleDef(def)));
@@ -129,6 +127,7 @@ pub fn classify_name(sema: &Semantics<RootDatabase>, name: &ast::Name) -> Option
 }
 
 fn classify_name_inner(sema: &Semantics<RootDatabase>, name: &ast::Name) -> Option<Definition> {
+    let _p = profile("classify_name");
     let parent = name.syntax().parent()?;
 
     match_ast! {
@@ -193,74 +192,3 @@ fn classify_name_inner(sema: &Semantics<RootDatabase>, name: &ast::Name) -> Opti
         }
     }
 }
-
-pub enum NameRefClass {
-    Definition(Definition),
-    FieldShorthand { local: Local, field: Definition },
-}
-
-impl NameRefClass {
-    pub fn definition(self) -> Definition {
-        match self {
-            NameRefClass::Definition(def) => def,
-            NameRefClass::FieldShorthand { local, field: _ } => Definition::Local(local),
-        }
-    }
-}
-
-pub fn classify_name_ref(
-    sema: &Semantics<RootDatabase>,
-    name_ref: &ast::NameRef,
-) -> Option<NameRefClass> {
-    let _p = profile("classify_name_ref");
-
-    let parent = name_ref.syntax().parent()?;
-
-    if let Some(method_call) = ast::MethodCallExpr::cast(parent.clone()) {
-        if let Some(func) = sema.resolve_method_call(&method_call) {
-            return Some(NameRefClass::Definition(Definition::ModuleDef(func.into())));
-        }
-    }
-
-    if let Some(field_expr) = ast::FieldExpr::cast(parent.clone()) {
-        if let Some(field) = sema.resolve_field(&field_expr) {
-            return Some(NameRefClass::Definition(Definition::StructField(field)));
-        }
-    }
-
-    if let Some(record_field) = ast::RecordField::cast(parent.clone()) {
-        if let Some((field, local)) = sema.resolve_record_field(&record_field) {
-            let field = Definition::StructField(field);
-            let res = match local {
-                None => NameRefClass::Definition(field),
-                Some(local) => NameRefClass::FieldShorthand { field, local },
-            };
-            return Some(res);
-        }
-    }
-
-    if let Some(macro_call) = parent.ancestors().find_map(ast::MacroCall::cast) {
-        if let Some(macro_def) = sema.resolve_macro_call(&macro_call) {
-            return Some(NameRefClass::Definition(Definition::Macro(macro_def)));
-        }
-    }
-
-    let path = name_ref.syntax().ancestors().find_map(ast::Path::cast)?;
-    let resolved = sema.resolve_path(&path)?;
-    let res = match resolved {
-        PathResolution::Def(def) => Definition::ModuleDef(def),
-        PathResolution::AssocItem(item) => {
-            let def = match item {
-                hir::AssocItem::Function(it) => it.into(),
-                hir::AssocItem::Const(it) => it.into(),
-                hir::AssocItem::TypeAlias(it) => it.into(),
-            };
-            Definition::ModuleDef(def)
-        }
-        PathResolution::Local(local) => Definition::Local(local),
-        PathResolution::TypeParam(par) => Definition::TypeParam(par),
-        PathResolution::Macro(def) => Definition::Macro(def),
-        PathResolution::SelfType(impl_def) => Definition::SelfType(impl_def),
-    };
-    Some(NameRefClass::Definition(res))
-}


Test patch:
diff --git a/crates/ra_hir_ty/src/tests/macros.rs b/crates/ra_hir_ty/src/tests/macros.rs
index 55386c0303..5d0efa0f40 100644
--- a/crates/ra_hir_ty/src/tests/macros.rs
+++ b/crates/ra_hir_ty/src/tests/macros.rs
@@ -438,6 +438,27 @@ fn main() {
     );
 }
 
+#[test]
+fn infer_builtin_macros_concat_with_lazy() {
+    assert_snapshot!(
+        infer(r#"
+macro_rules! hello {() => {"hello"}}
+
+#[rustc_builtin_macro]
+macro_rules! concat {() => {}}
+
+fn main() {
+    let x = concat!(hello!(), concat!("world", "!"));
+}
+"#),
+        @r###"
+    ![0; 13) '"helloworld!"': &str
+    [104; 161) '{     ...")); }': ()
+    [114; 115) 'x': &str
+    "###
+    );
+}
+
 #[test]
 fn infer_derive_clone_simple() {
     let (db, pos) = TestDB::with_position(