Repository: rust-analyzer/rust-analyzer
Pull request #2271 (2019-11-15 22:30:53)

Force passing Source when creating a SourceAnalyzer

matklad: bors r+
🤖
bors[bot]: # Merge conflict

Patch:
diff --git a/crates/ra_assists/src/assist_ctx.rs b/crates/ra_assists/src/assist_ctx.rs
index 71f7ce1b19..0ea84d5488 100644
--- a/crates/ra_assists/src/assist_ctx.rs
+++ b/crates/ra_assists/src/assist_ctx.rs
@@ -117,7 +117,7 @@ impl<'a, DB: HirDatabase> AssistCtx<'a, DB> {
         node: &SyntaxNode,
         offset: Option<TextUnit>,
     ) -> SourceAnalyzer {
-        SourceAnalyzer::new(self.db, self.frange.file_id, node, offset)
+        SourceAnalyzer::new(self.db, hir::Source::new(self.frange.file_id.into(), node), offset)
     }
 
     pub(crate) fn covering_node_for_range(&self, range: TextRange) -> SyntaxElement {
diff --git a/crates/ra_hir/src/from_source.rs b/crates/ra_hir/src/from_source.rs
index 1c26756c97..79152a57c4 100644
--- a/crates/ra_hir/src/from_source.rs
+++ b/crates/ra_hir/src/from_source.rs
@@ -1,7 +1,7 @@
 //! FIXME: write short doc here
 
 use hir_def::{ModuleId, StructId, StructOrUnionId, UnionId};
-use hir_expand::{name::AsName, AstId, MacroDefId, MacroDefKind};
+use hir_expand::name::AsName;
 use ra_syntax::{
     ast::{self, AstNode, NameOwner},
     match_ast,
@@ -11,8 +11,8 @@ use crate::{
     db::{AstDatabase, DefDatabase, HirDatabase},
     ids::{AstItemDef, LocationCtx},
     Const, DefWithBody, Enum, EnumVariant, FieldSource, Function, HasBody, HasSource, ImplBlock,
-    Local, MacroDef, Module, ModuleSource, Source, Static, Struct, StructField, Trait, TypeAlias,
-    Union, VariantDef,
+    Local, Module, ModuleSource, Source, Static, Struct, StructField, Trait, TypeAlias, Union,
+    VariantDef,
 };
 
 pub trait FromSource: Sized {
@@ -77,22 +77,7 @@ impl FromSource for TypeAlias {
         Some(TypeAlias { id })
     }
 }
-
-impl FromSource for MacroDef {
-    type Ast = ast::MacroCall;
-    fn from_source(db: &(impl DefDatabase + AstDatabase), src: Source<Self::Ast>) -> Option<Self> {
-        let kind = MacroDefKind::Declarative;
-
-        let module_src = ModuleSource::from_child_node(db, src.as_ref().map(|it| it.syntax()));
-        let module = Module::from_definition(db, Source::new(src.file_id, module_src))?;
-        let krate = module.krate().crate_id();
-
-        let ast_id = AstId::new(src.file_id, db.ast_id_map(src.file_id).ast_id(&src.ast));
-
-        let id: MacroDefId = MacroDefId { krate, ast_id, kind };
-        Some(MacroDef { id })
-    }
-}
+// FIXME: add impl FromSource for MacroDef
 
 impl FromSource for ImplBlock {
     type Ast = ast::ImplBlock;
diff --git a/crates/ra_hir/src/source_binder.rs b/crates/ra_hir/src/source_binder.rs
index 540ddd0b5d..5764dc26d1 100644
--- a/crates/ra_hir/src/source_binder.rs
+++ b/crates/ra_hir/src/source_binder.rs
@@ -12,7 +12,6 @@ use hir_def::{
     path::known,
 };
 use hir_expand::{name::AsName, Source};
-use ra_db::FileId;
 use ra_syntax::{
     ast::{self, AstNode},
     match_ast, AstPtr,
@@ -30,38 +29,32 @@ use crate::{
     HirFileId, Local, MacroDef, Module, Name, Path, Resolver, Static, Struct, Ty,
 };
 
-fn try_get_resolver_for_node(
-    db: &impl HirDatabase,
-    file_id: FileId,
-    node: &SyntaxNode,
-) -> Option<Resolver> {
+fn try_get_resolver_for_node(db: &impl HirDatabase, node: Source<&SyntaxNode>) -> Option<Resolver> {
     match_ast! {
-        match node {
+        match (node.ast) {
             ast::Module(it) => {
-                let src = crate::Source { file_id: file_id.into(), ast: it };
+                let src = node.with_ast(it);
                 Some(crate::Module::from_declaration(db, src)?.resolver(db))
             },
              ast::SourceFile(it) => {
-                let src =
-                    crate::Source { file_id: file_id.into(), ast: crate::ModuleSource::SourceFile(it) };
+                let src = node.with_ast(crate::ModuleSource::SourceFile(it));
                 Some(crate::Module::from_definition(db, src)?.resolver(db))
             },
             ast::StructDef(it) => {
-                let src = crate::Source { file_id: file_id.into(), ast: it };
+                let src = node.with_ast(it);
                 Some(Struct::from_source(db, src)?.resolver(db))
             },
             ast::EnumDef(it) => {
-                let src = crate::Source { file_id: file_id.into(), ast: it };
+                let src = node.with_ast(it);
                 Some(Enum::from_source(db, src)?.resolver(db))
             },
-            _ => {
-                if node.kind() == FN_DEF || node.kind() == CONST_DEF || node.kind() == STATIC_DEF {
-                    Some(def_with_body_from_child_node(db, Source::new(file_id.into(), node))?.resolver(db))
-                } else {
-                    // FIXME add missing cases
-                    None
+            _ => match node.ast.kind() {
+                FN_DEF | CONST_DEF | STATIC_DEF => {
+                    Some(def_with_body_from_child_node(db, node)?.resolver(db))
                 }
-            },
+                // FIXME add missing cases
+                _ => None
+            }
         }
     }
 }
@@ -90,7 +83,6 @@ fn def_with_body_from_child_node(
 /// original source files. It should not be used inside the HIR itself.
 #[derive(Debug)]
 pub struct SourceAnalyzer {
-    // FIXME: this doesn't handle macros at all
     file_id: HirFileId,
     resolver: Resolver,
     body_owner: Option<DefWithBody>,
@@ -137,20 +129,16 @@ pub struct ReferenceDescriptor {
 impl SourceAnalyzer {
     pub fn new(
         db: &impl HirDatabase,
-        file_id: FileId,
-        node: &SyntaxNode,
+        node: Source<&SyntaxNode>,
         offset: Option<TextUnit>,
     ) -> SourceAnalyzer {
-        let node_source = Source::new(file_id.into(), node);
-        let def_with_body = def_with_body_from_child_node(db, node_source);
+        let def_with_body = def_with_body_from_child_node(db, node);
         if let Some(def) = def_with_body {
             let source_map = def.body_source_map(db);
             let scopes = def.expr_scopes(db);
             let scope = match offset {
-                None => scope_for(&scopes, &source_map, node_source),
-                Some(offset) => {
-                    scope_for_offset(&scopes, &source_map, Source::new(file_id.into(), offset))
-                }
+                None => scope_for(&scopes, &source_map, node),
+                Some(offset) => scope_for_offset(&scopes, &source_map, node.with_ast(offset)),
             };
             let resolver = expr::resolver_for_scope(db, def, scope);
             SourceAnalyzer {
@@ -159,19 +147,20 @@ impl SourceAnalyzer {
                 body_source_map: Some(source_map),
                 infer: Some(def.infer(db)),
                 scopes: Some(scopes),
-                file_id: file_id.into(),
+                file_id: node.file_id,
             }
         } else {
             SourceAnalyzer {
                 resolver: node
+                    .ast
                     .ancestors()
-                    .find_map(|node| try_get_resolver_for_node(db, file_id, &node))
+                    .find_map(|it| try_get_resolver_for_node(db, node.with_ast(&it)))
                     .unwrap_or_default(),
                 body_owner: None,
                 body_source_map: None,
                 infer: None,
                 scopes: None,
-                file_id: file_id.into(),
+                file_id: node.file_id,
             }
         }
     }
diff --git a/crates/ra_ide_api/src/call_info.rs b/crates/ra_ide_api/src/call_info.rs
index 3572825b57..41ee815112 100644
--- a/crates/ra_ide_api/src/call_info.rs
+++ b/crates/ra_ide_api/src/call_info.rs
@@ -19,7 +19,11 @@ pub(crate) fn call_info(db: &RootDatabase, position: FilePosition) -> Option<Cal
     let calling_node = FnCallNode::with_node(&syntax, position.offset)?;
     let name_ref = calling_node.name_ref()?;
 
-    let analyzer = hir::SourceAnalyzer::new(db, position.file_id, name_ref.syntax(), None);
+    let analyzer = hir::SourceAnalyzer::new(
+        db,
+        hir::Source::new(position.file_id.into(), name_ref.syntax()),
+        None,
+    );
     let (mut call_info, has_self) = match &calling_node {
         FnCallNode::CallExpr(expr) => {
             //FIXME: apply subst
diff --git a/crates/ra_ide_api/src/completion/completion_context.rs b/crates/ra_ide_api/src/completion/completion_context.rs
index 64cbc0f982..0906a4e1b6 100644
--- a/crates/ra_ide_api/src/completion/completion_context.rs
+++ b/crates/ra_ide_api/src/completion/completion_context.rs
@@ -58,8 +58,11 @@ impl<'a> CompletionContext<'a> {
         );
         let token =
             original_parse.tree().syntax().token_at_offset(position.offset).left_biased()?;
-        let analyzer =
-            hir::SourceAnalyzer::new(db, position.file_id, &token.parent(), Some(position.offset));
+        let analyzer = hir::SourceAnalyzer::new(
+            db,
+            hir::Source::new(position.file_id.into(), &token.parent()),
+            Some(position.offset),
+        );
         let mut ctx = CompletionContext {
             db,
             analyzer,
diff --git a/crates/ra_ide_api/src/goto_type_definition.rs b/crates/ra_ide_api/src/goto_type_definition.rs
index 71146591df..2327cb1e71 100644
--- a/crates/ra_ide_api/src/goto_type_definition.rs
+++ b/crates/ra_ide_api/src/goto_type_definition.rs
@@ -18,7 +18,8 @@ pub(crate) fn goto_type_definition(
             .find(|n| ast::Expr::cast(n.clone()).is_some() || ast::Pat::cast(n.clone()).is_some())
     })?;
 
-    let analyzer = hir::SourceAnalyzer::new(db, position.file_id, &node, None);
+    let analyzer =
+        hir::SourceAnalyzer::new(db, hir::Source::new(position.file_id.into(), &node), None);
 
     let ty: hir::Ty = if let Some(ty) =
         ast::Expr::cast(node.clone()).and_then(|e| analyzer.type_of(db, &e))
diff --git a/crates/ra_ide_api/src/hover.rs b/crates/ra_ide_api/src/hover.rs
index 07d511fb31..92b4b1f79b 100644
--- a/crates/ra_ide_api/src/hover.rs
+++ b/crates/ra_ide_api/src/hover.rs
@@ -230,7 +230,8 @@ pub(crate) fn type_of(db: &RootDatabase, frange: FileRange) -> Option<String> {
         .ancestors()
         .take_while(|it| it.text_range() == leaf_node.text_range())
         .find(|it| ast::Expr::cast(it.clone()).is_some() || ast::Pat::cast(it.clone()).is_some())?;
-    let analyzer = hir::SourceAnalyzer::new(db, frange.file_id, &node, None);
+    let analyzer =
+        hir::SourceAnalyzer::new(db, hir::Source::new(frange.file_id.into(), &node), None);
     let ty = if let Some(ty) = ast::Expr::cast(node.clone()).and_then(|e| analyzer.type_of(db, &e))
     {
         ty
diff --git a/crates/ra_ide_api/src/inlay_hints.rs b/crates/ra_ide_api/src/inlay_hints.rs
index 2ff10b89a8..0cd9598483 100644
--- a/crates/ra_ide_api/src/inlay_hints.rs
+++ b/crates/ra_ide_api/src/inlay_hints.rs
@@ -32,6 +32,7 @@ fn get_inlay_hints(
     file_id: FileId,
     node: &SyntaxNode,
 ) -> Option<Vec<InlayHint>> {
+    let analyzer = SourceAnalyzer::new(db, hir::Source::new(file_id.into(), node), None);
     match_ast! {
         match node {
             ast::LetStmt(it) => {
@@ -39,11 +40,9 @@ fn get_inlay_hints(
                     return None;
                 }
                 let pat = it.pat()?;
-                let analyzer = SourceAnalyzer::new(db, file_id, it.syntax(), None);
                 Some(get_pat_type_hints(db, &analyzer, pat, false))
             },
             ast::LambdaExpr(it) => {
-                let analyzer = SourceAnalyzer::new(db, file_id, it.syntax(), None);
                 it.param_list().map(|param_list| {
                     param_list
                         .params()
@@ -56,21 +55,17 @@ fn get_inlay_hints(
             },
             ast::ForExpr(it) => {
                 let pat = it.pat()?;
-                let analyzer = SourceAnalyzer::new(db, file_id, it.syntax(), None);
                 Some(get_pat_type_hints(db, &analyzer, pat, false))
             },
             ast::IfExpr(it) => {
                 let pat = it.condition()?.pat()?;
-                let analyzer = SourceAnalyzer::new(db, file_id, it.syntax(), None);
                 Some(get_pat_type_hints(db, &analyzer, pat, true))
             },
             ast::WhileExpr(it) => {
                 let pat = it.condition()?.pat()?;
-                let analyzer = SourceAnalyzer::new(db, file_id, it.syntax(), None);
                 Some(get_pat_type_hints(db, &analyzer, pat, true))
             },
             ast::MatchArmList(it) => {
-                let analyzer = SourceAnalyzer::new(db, file_id, it.syntax(), None);
                 Some(
                     it
                         .arms()
diff --git a/crates/ra_ide_api/src/references.rs b/crates/ra_ide_api/src/references.rs
index faa88d9882..9cb9433e7b 100644
--- a/crates/ra_ide_api/src/references.rs
+++ b/crates/ra_ide_api/src/references.rs
@@ -369,21 +369,6 @@ mod tests {
         assert_eq!(refs.len(), 2);
     }
 
-    #[test]
-    fn test_find_all_refs_macro_def() {
-        let code = r#"
-        #[macro_export]
-        macro_rules! m1<|> { () => (()) }
-
-        fn foo() {
-            m1();
-            m1();
-        }"#;
-
-        let refs = get_all_refs(code);
-        assert_eq!(refs.len(), 3);
-    }
-
     fn get_all_refs(text: &str) -> ReferenceSearchResult {
         let (analysis, position) = single_file_with_position(text);
         analysis.find_all_refs(position, None).unwrap().unwrap()
diff --git a/crates/ra_ide_api/src/references/classify.rs b/crates/ra_ide_api/src/references/classify.rs
index 0eeaa7f384..0228c634d8 100644
--- a/crates/ra_ide_api/src/references/classify.rs
+++ b/crates/ra_ide_api/src/references/classify.rs
@@ -21,6 +21,7 @@ pub(crate) fn classify_name(
     let parent = name.syntax().parent()?;
     let file_id = file_id.into();
 
+    // FIXME: add ast::MacroCall(it)
     match_ast! {
         match parent {
             ast::BindPat(it) => {
@@ -103,19 +104,6 @@ pub(crate) fn classify_name(
                     Some(from_module_def(db, def.into(), None))
                 }
             },
-            ast::MacroCall(it) => {
-                let src = hir::Source { file_id, ast: it};
-                let def = hir::MacroDef::from_source(db, src.clone())?;
-
-                let module_src = ModuleSource::from_child_node(db, src.as_ref().map(|it| it.syntax()));
-                let module = Module::from_definition(db, Source::new(file_id, module_src))?;
-
-                Some(NameDefinition {
-                    visibility: None,
-                    container: module,
-                    kind: NameKind::Macro(def),
-                })
-            },
             _ => None,
         }
     }
@@ -129,7 +117,8 @@ pub(crate) fn classify_name_ref(
     let _p = profile("classify_name_ref");
 
     let parent = name_ref.syntax().parent()?;
-    let analyzer = SourceAnalyzer::new(db, file_id, name_ref.syntax(), None);
+    let analyzer =
+        SourceAnalyzer::new(db, hir::Source::new(file_id.into(), name_ref.syntax()), None);
 
     if let Some(method_call) = ast::MethodCallExpr::cast(parent.clone()) {
         tested_by!(goto_definition_works_for_methods);
diff --git a/crates/ra_syntax/src/lib.rs b/crates/ra_syntax/src/lib.rs
index 5dcb6a95a6..9931fec847 100644
--- a/crates/ra_syntax/src/lib.rs
+++ b/crates/ra_syntax/src/lib.rs
@@ -176,9 +176,11 @@ impl SourceFile {
 /// ```
 #[macro_export]
 macro_rules! match_ast {
-    (match $node:ident {
+    (match $node:ident { $($tt:tt)* }) => { match_ast!(match ($node) { $($tt)* }) };
+
+    (match ($node:expr) {
         $( ast::$ast:ident($it:ident) => $res:block, )*
-        _ => $catch_all:expr,
+        _ => $catch_all:expr $(,)?
     }) => {{
         $( if let Some($it) = ast::$ast::cast($node.clone()) $res else )*
         { $catch_all }


Test patch:
diff --git a/crates/ra_hir/src/ty/tests.rs b/crates/ra_hir/src/ty/tests.rs
index fe9346c783..9a26e02fa2 100644
--- a/crates/ra_hir/src/ty/tests.rs
+++ b/crates/ra_hir/src/ty/tests.rs
@@ -1,3 +1,6 @@
+mod never_type;
+mod coercion;
+
 use std::fmt::Write;
 use std::sync::Arc;
 
@@ -11,7 +14,7 @@ use ra_syntax::{
 use test_utils::covers;
 
 use crate::{
-    expr::BodySourceMap, test_db::TestDB, ty::display::HirDisplay, ty::InferenceResult,
+    expr::BodySourceMap, test_db::TestDB, ty::display::HirDisplay, ty::InferenceResult, Source,
     SourceAnalyzer,
 };
 
@@ -19,9 +22,6 @@ use crate::{
 // against snapshots of the expected results using insta. Use cargo-insta to
 // update the snapshots.
 
-mod never_type;
-mod coercion;
-
 #[test]
 fn cfg_impl_block() {
     let (db, pos) = TestDB::with_position(
@@ -4609,7 +4609,8 @@ fn test<T, U>() where T: Trait<U::Item>, U: Trait<T::Item> {
 fn type_at_pos(db: &TestDB, pos: FilePosition) -> String {
     let file = db.parse(pos.file_id).ok().unwrap();
     let expr = algo::find_node_at_offset::<ast::Expr>(file.syntax(), pos.offset).unwrap();
-    let analyzer = SourceAnalyzer::new(db, pos.file_id, expr.syntax(), Some(pos.offset));
+    let analyzer =
+        SourceAnalyzer::new(db, Source::new(pos.file_id.into(), expr.syntax()), Some(pos.offset));
     let ty = analyzer.type_of(db, &expr).unwrap();
     ty.display(db).to_string()
 }
@@ -4674,7 +4675,7 @@ fn infer(content: &str) -> String {
 
     for node in source_file.syntax().descendants() {
         if node.kind() == FN_DEF || node.kind() == CONST_DEF || node.kind() == STATIC_DEF {
-            let analyzer = SourceAnalyzer::new(&db, file_id, &node, None);
+            let analyzer = SourceAnalyzer::new(&db, Source::new(file_id.into(), &node), None);
             infer_def(analyzer.inference_result(), analyzer.body_source_map());
         }
     }
@@ -4715,7 +4716,7 @@ fn typing_whitespace_inside_a_function_should_not_invalidate_types() {
         let file = db.parse(pos.file_id).ok().unwrap();
         let node = file.syntax().token_at_offset(pos.offset).right_biased().unwrap().parent();
         let events = db.log_executed(|| {
-            SourceAnalyzer::new(&db, pos.file_id, &node, None);
+            SourceAnalyzer::new(&db, Source::new(pos.file_id.into(), &node), None);
         });
         assert!(format!("{:?}", events).contains("infer"))
     }
@@ -4735,7 +4736,7 @@ fn typing_whitespace_inside_a_function_should_not_invalidate_types() {
         let file = db.parse(pos.file_id).ok().unwrap();
         let node = file.syntax().token_at_offset(pos.offset).right_biased().unwrap().parent();
         let events = db.log_executed(|| {
-            SourceAnalyzer::new(&db, pos.file_id, &node, None);
+            SourceAnalyzer::new(&db, Source::new(pos.file_id.into(), &node), None);
         });
         assert!(!format!("{:?}", events).contains("infer"), "{:#?}", events)
     }