Auto merge of #17518 - Veykril:expr-scopes-mac, r=Veykril

fix: Fix expression scope calculation when within macro expansions

Author: bors

Cargo.toml

@@ -10,9 +10,7 @@ license = "MIT OR Apache-2.0"
 authors = ["rust-analyzer team"]
 
 [profile.dev]
-# Disabling debug info speeds up builds a bunch,
-# and we don't rely on it for debugging that much.
-debug = 0
+debug = 1
 
 [profile.dev.package]
 # These speed up local tests.

crates/hir-def/src/data.rs

@@ -637,10 +637,6 @@ impl<'a> AssocItemCollector<'a> {
                     attr,
                 ) {
                     Ok(ResolvedAttr::Macro(call_id)) => {
-                        // If proc attribute macro expansion is disabled, skip expanding it here
-                        if !self.db.expand_proc_attr_macros() {
-                            continue 'attrs;
-                        }
                         let loc = self.db.lookup_intern_macro_call(call_id);
                         if let MacroDefKind::ProcMacro(_, exp, _) = loc.def.kind {
                             // If there's no expander for the proc macro (e.g. the

crates/hir-def/src/nameres/collector.rs

@@ -83,7 +83,9 @@ pub(super) fn collect_defs(db: &dyn DefDatabase, def_map: DefMap, tree_id: TreeI
                     let name = Name::new_text_dont_use(it.name.clone());
                     (
                         name,
-                        if it.disabled {
+                        if !db.expand_proc_attr_macros() {
+                            CustomProcMacroExpander::dummy()
+                        } else if it.disabled {
                             CustomProcMacroExpander::disabled()
                         } else {
                             CustomProcMacroExpander::new(hir_expand::proc_macro::ProcMacroId::new(
@@ -1331,16 +1333,6 @@ impl DefCollector<'_> {
 
                     let call_id = call_id();
                     if let MacroDefKind::ProcMacro(_, exp, _) = def.kind {
-                        // If proc attribute macro expansion is disabled, skip expanding it here
-                        if !self.db.expand_proc_attr_macros() {
-                            self.def_map.diagnostics.push(DefDiagnostic::unresolved_proc_macro(
-                                directive.module_id,
-                                self.db.lookup_intern_macro_call(call_id).kind,
-                                def.krate,
-                            ));
-                            return recollect_without(self);
-                        }
-
                         // If there's no expander for the proc macro (e.g.
                         // because proc macros are disabled, or building the
                         // proc macro crate failed), report this and skip

crates/hir-def/src/nameres/diagnostics.rs

@@ -17,16 +17,47 @@ use crate::{
 
 #[derive(Debug, PartialEq, Eq)]
 pub enum DefDiagnosticKind {
-    UnresolvedModule { ast: AstId<ast::Module>, candidates: Box<[String]> },
-    UnresolvedExternCrate { ast: AstId<ast::ExternCrate> },
-    UnresolvedImport { id: ItemTreeId<item_tree::Use>, index: Idx<ast::UseTree> },
-    UnconfiguredCode { ast: ErasedAstId, cfg: CfgExpr, opts: CfgOptions },
-    UnresolvedProcMacro { ast: MacroCallKind, krate: CrateId },
-    UnresolvedMacroCall { ast: MacroCallKind, path: ModPath },
-    UnimplementedBuiltinMacro { ast: AstId<ast::Macro> },
-    InvalidDeriveTarget { ast: AstId<ast::Item>, id: usize },
-    MalformedDerive { ast: AstId<ast::Adt>, id: usize },
-    MacroDefError { ast: AstId<ast::Macro>, message: String },
+    UnresolvedModule {
+        ast: AstId<ast::Module>,
+        candidates: Box<[String]>,
+    },
+    UnresolvedExternCrate {
+        ast: AstId<ast::ExternCrate>,
+    },
+    UnresolvedImport {
+        id: ItemTreeId<item_tree::Use>,
+        index: Idx<ast::UseTree>,
+    },
+    UnconfiguredCode {
+        ast: ErasedAstId,
+        cfg: CfgExpr,
+        opts: CfgOptions,
+    },
+    /// A proc-macro that is lacking an expander, this might be due to build scripts not yet having
+    /// run or proc-macro expansion being disabled.
+    UnresolvedProcMacro {
+        ast: MacroCallKind,
+        krate: CrateId,
+    },
+    UnresolvedMacroCall {
+        ast: MacroCallKind,
+        path: ModPath,
+    },
+    UnimplementedBuiltinMacro {
+        ast: AstId<ast::Macro>,
+    },
+    InvalidDeriveTarget {
+        ast: AstId<ast::Item>,
+        id: usize,
+    },
+    MalformedDerive {
+        ast: AstId<ast::Adt>,
+        id: usize,
+    },
+    MacroDefError {
+        ast: AstId<ast::Macro>,
+        message: String,
+    },
 }
 
 #[derive(Clone, Debug, PartialEq, Eq)]
@@ -92,10 +123,6 @@ impl DefDiagnostic {
         Self { in_module: container, kind: DefDiagnosticKind::UnconfiguredCode { ast, cfg, opts } }
     }
 
-    // FIXME: Whats the difference between this and unresolved_macro_call
-    // FIXME: This is used for a lot of things, unresolved proc macros, disabled proc macros, etc
-    // yet the diagnostic handler in ide-diagnostics has to figure out what happened because this
-    // struct loses all that information!
     pub fn unresolved_proc_macro(
         container: LocalModuleId,
         ast: MacroCallKind,

crates/hir-expand/src/files.rs

@@ -1,4 +1,6 @@
 //! Things to wrap other things in file ids.
+use std::borrow::Borrow;
+
 use either::Either;
 use span::{
     AstIdNode, ErasedFileAstId, FileAstId, FileId, FileRange, HirFileId, HirFileIdRepr,
@@ -76,6 +78,13 @@ impl<FileKind: Copy, T> InFileWrapper<FileKind, T> {
     pub fn as_ref(&self) -> InFileWrapper<FileKind, &T> {
         self.with_value(&self.value)
     }
+
+    pub fn borrow<U>(&self) -> InFileWrapper<FileKind, &U>
+    where
+        T: Borrow<U>,
+    {
+        self.with_value(self.value.borrow())
+    }
 }
 
 impl<FileKind: Copy, T: Clone> InFileWrapper<FileKind, &T> {
@@ -156,30 +165,69 @@ impl<FileId: Copy, N: AstNode> InFileWrapper<FileId, &N> {
 }
 
 // region:specific impls
+impl<SN: Borrow<SyntaxNode>> InRealFile<SN> {
+    pub fn file_range(&self) -> FileRange {
+        FileRange { file_id: self.file_id, range: self.value.borrow().text_range() }
+    }
+}
+
+impl<SN: Borrow<SyntaxNode>> InFile<SN> {
+    pub fn parent_ancestors_with_macros(
+        self,
+        db: &dyn db::ExpandDatabase,
+    ) -> impl Iterator<Item = InFile<SyntaxNode>> + '_ {
+        let succ = move |node: &InFile<SyntaxNode>| match node.value.parent() {
+            Some(parent) => Some(node.with_value(parent)),
+            None => db
+                .lookup_intern_macro_call(node.file_id.macro_file()?.macro_call_id)
+                .to_node_item(db)
+                .syntax()
+                .cloned()
+                .map(|node| node.parent())
+                .transpose(),
+        };
+        std::iter::successors(succ(&self.borrow().cloned()), succ)
+    }
+
+    pub fn ancestors_with_macros(
+        self,
+        db: &dyn db::ExpandDatabase,
+    ) -> impl Iterator<Item = InFile<SyntaxNode>> + '_ {
+        let succ = move |node: &InFile<SyntaxNode>| match node.value.parent() {
+            Some(parent) => Some(node.with_value(parent)),
+            None => db
+                .lookup_intern_macro_call(node.file_id.macro_file()?.macro_call_id)
+                .to_node_item(db)
+                .syntax()
+                .cloned()
+                .map(|node| node.parent())
+                .transpose(),
+        };
+        std::iter::successors(Some(self.borrow().cloned()), succ)
+    }
+
+    pub fn kind(&self) -> parser::SyntaxKind {
+        self.value.borrow().kind()
+    }
+
+    pub fn text_range(&self) -> TextRange {
+        self.value.borrow().text_range()
+    }
 
-impl InFile<&SyntaxNode> {
     /// Falls back to the macro call range if the node cannot be mapped up fully.
     ///
     /// For attributes and derives, this will point back to the attribute only.
     /// For the entire item use [`InFile::original_file_range_full`].
     pub fn original_file_range_rooted(self, db: &dyn db::ExpandDatabase) -> FileRange {
-        self.map(SyntaxNode::text_range).original_node_file_range_rooted(db)
+        self.borrow().map(SyntaxNode::text_range).original_node_file_range_rooted(db)
     }
 
     /// Falls back to the macro call range if the node cannot be mapped up fully.
     pub fn original_file_range_with_macro_call_body(
         self,
         db: &dyn db::ExpandDatabase,
     ) -> FileRange {
-        self.map(SyntaxNode::text_range).original_node_file_range_with_macro_call_body(db)
-    }
-
-    /// Attempts to map the syntax node back up its macro calls.
-    pub fn original_file_range_opt(
-        self,
-        db: &dyn db::ExpandDatabase,
-    ) -> Option<(FileRange, SyntaxContextId)> {
-        self.map(SyntaxNode::text_range).original_node_file_range_opt(db)
+        self.borrow().map(SyntaxNode::text_range).original_node_file_range_with_macro_call_body(db)
     }
 
     pub fn original_syntax_node_rooted(
@@ -190,16 +238,19 @@ impl InFile<&SyntaxNode> {
         // as we don't have node inputs otherwise and therefore can't find an `N` node in the input
         let file_id = match self.file_id.repr() {
             HirFileIdRepr::FileId(file_id) => {
-                return Some(InRealFile { file_id, value: self.value.clone() })
+                return Some(InRealFile { file_id, value: self.value.borrow().clone() })
             }
             HirFileIdRepr::MacroFile(m) if m.is_attr_macro(db) => m,
             _ => return None,
         };
 
-        let FileRange { file_id, range } =
-            map_node_range_up_rooted(db, &db.expansion_span_map(file_id), self.value.text_range())?;
+        let FileRange { file_id, range } = map_node_range_up_rooted(
+            db,
+            &db.expansion_span_map(file_id),
+            self.value.borrow().text_range(),
+        )?;
 
-        let kind = self.value.kind();
+        let kind = self.kind();
         let value = db
             .parse(file_id)
             .syntax_node()
@@ -211,6 +262,16 @@ impl InFile<&SyntaxNode> {
     }
 }
 
+impl InFile<&SyntaxNode> {
+    /// Attempts to map the syntax node back up its macro calls.
+    pub fn original_file_range_opt(
+        self,
+        db: &dyn db::ExpandDatabase,
+    ) -> Option<(FileRange, SyntaxContextId)> {
+        self.borrow().map(SyntaxNode::text_range).original_node_file_range_opt(db)
+    }
+}
+
 impl InMacroFile<SyntaxToken> {
     pub fn upmap_once(
         self,

crates/hir/src/source_analyzer.rs

@@ -68,38 +68,44 @@ impl SourceAnalyzer {
     pub(crate) fn new_for_body(
         db: &dyn HirDatabase,
         def: DefWithBodyId,
-        node @ InFile { file_id, .. }: InFile<&SyntaxNode>,
+        node: InFile<&SyntaxNode>,
         offset: Option<TextSize>,
     ) -> SourceAnalyzer {
-        let (body, source_map) = db.body_with_source_map(def);
-        let scopes = db.expr_scopes(def);
-        let scope = match offset {
-            None => scope_for(&scopes, &source_map, node),
-            Some(offset) => scope_for_offset(db, &scopes, &source_map, node.file_id, offset),
-        };
-        let resolver = resolver_for_scope(db.upcast(), def, scope);
-        SourceAnalyzer {
-            resolver,
-            def: Some((def, body, source_map)),
-            infer: Some(db.infer(def)),
-            file_id,
-        }
+        Self::new_for_body_(db, def, node, offset, Some(db.infer(def)))
     }
 
     pub(crate) fn new_for_body_no_infer(
+        db: &dyn HirDatabase,
+        def: DefWithBodyId,
+        node: InFile<&SyntaxNode>,
+        offset: Option<TextSize>,
+    ) -> SourceAnalyzer {
+        Self::new_for_body_(db, def, node, offset, None)
+    }
+
+    pub(crate) fn new_for_body_(
         db: &dyn HirDatabase,
         def: DefWithBodyId,
         node @ InFile { file_id, .. }: InFile<&SyntaxNode>,
         offset: Option<TextSize>,
+        infer: Option<Arc<InferenceResult>>,
     ) -> SourceAnalyzer {
         let (body, source_map) = db.body_with_source_map(def);
         let scopes = db.expr_scopes(def);
         let scope = match offset {
-            None => scope_for(&scopes, &source_map, node),
-            Some(offset) => scope_for_offset(db, &scopes, &source_map, node.file_id, offset),
+            None => scope_for(db, &scopes, &source_map, node),
+            Some(offset) => {
+                debug_assert!(
+                    node.text_range().contains_inclusive(offset),
+                    "{:?} not in {:?}",
+                    offset,
+                    node.text_range()
+                );
+                scope_for_offset(db, &scopes, &source_map, node.file_id, offset)
+            }
         };
         let resolver = resolver_for_scope(db.upcast(), def, scope);
-        SourceAnalyzer { resolver, def: Some((def, body, source_map)), infer: None, file_id }
+        SourceAnalyzer { resolver, def: Some((def, body, source_map)), infer, file_id }
     }
 
     pub(crate) fn new_for_resolver(
@@ -662,7 +668,6 @@ impl SourceAnalyzer {
             return resolved;
         }
 
-        // This must be a normal source file rather than macro file.
         let ctx = LowerCtx::new(db.upcast(), self.file_id);
         let hir_path = Path::from_src(&ctx, path.clone())?;
 
@@ -955,14 +960,15 @@ impl SourceAnalyzer {
 }
 
 fn scope_for(
+    db: &dyn HirDatabase,
     scopes: &ExprScopes,
     source_map: &BodySourceMap,
     node: InFile<&SyntaxNode>,
 ) -> Option<ScopeId> {
-    node.value
-        .ancestors()
-        .filter_map(ast::Expr::cast)
-        .filter_map(|it| source_map.node_expr(InFile::new(node.file_id, &it)))
+    node.ancestors_with_macros(db.upcast())
+        .take_while(|it| !ast::Item::can_cast(it.kind()) || ast::MacroCall::can_cast(it.kind()))
+        .filter_map(|it| it.map(ast::Expr::cast).transpose())
+        .filter_map(|it| source_map.node_expr(it.as_ref()))
         .find_map(|it| scopes.scope_for(it))
 }
 
@@ -988,8 +994,8 @@ fn scope_for_offset(
                     Some(it.file_id.macro_file()?.call_node(db.upcast()))
                 })
                 .find(|it| it.file_id == from_file)
-                .filter(|it| it.value.kind() == SyntaxKind::MACRO_CALL)?;
-            Some((source.value.text_range(), scope))
+                .filter(|it| it.kind() == SyntaxKind::MACRO_CALL)?;
+            Some((source.text_range(), scope))
         })
         .filter(|(expr_range, _scope)| expr_range.start() <= offset && offset <= expr_range.end())
         // find containing scope

crates/hir/src/term_search/tactics.rs

@@ -598,9 +598,8 @@ pub(super) fn famous_types<'a, DB: HirDatabase>(
         Expr::FamousType { ty: Type::new(db, module.id, TyBuilder::unit()), value: "()" },
     ]
     .into_iter()
-    .map(|exprs| {
+    .inspect(|exprs| {
         lookup.insert(exprs.ty(db), std::iter::once(exprs.clone()));
-        exprs
     })
     .filter(|expr| expr.ty(db).could_unify_with_deeply(db, &ctx.goal))
 }