Fix expression scope calculation when within macro expansions

Veykril · Veykril · commit bfb187aacd23 · 2024-06-30T13:26:17.000+02:00
diff --git a/Cargo.toml b/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.
diff --git a/crates/hir-expand/src/files.rs b/crates/hir-expand/src/files.rs
@@ -158,6 +158,40 @@ impl<FileId: Copy, N: AstNode> InFileWrapper<FileId, &N> {
 // region:specific impls
 
 impl InFile<&SyntaxNode> {
+    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.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.cloned()), succ)
+    }
+
     /// 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.
diff --git a/crates/hir/src/source_analyzer.rs b/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.value.text_range().contains_inclusive(offset),
+                    "{:?} not in {:?}",
+                    offset,
+                    node.value.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,17 @@ 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.value.kind()) || ast::MacroCall::can_cast(it.value.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))
 }
 
diff --git a/crates/ide-assists/src/handlers/bool_to_enum.rs b/crates/ide-assists/src/handlers/bool_to_enum.rs
@@ -76,7 +76,11 @@ pub(crate) fn bool_to_enum(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option
 
             let usages = definition.usages(&ctx.sema).all();
             add_enum_def(edit, ctx, &usages, target_node, &target_module);
-            replace_usages(edit, ctx, usages, definition, &target_module);
+            let mut delayed_mutations = Vec::new();
+            replace_usages(edit, ctx, usages, definition, &target_module, &mut delayed_mutations);
+            for (scope, path) in delayed_mutations {
+                insert_use(&scope, path, &ctx.config.insert_use);
+            }
         },
     )
 }
@@ -197,6 +201,7 @@ fn replace_usages(
     usages: UsageSearchResult,
     target_definition: Definition,
     target_module: &hir::Module,
+    delayed_mutations: &mut Vec<(ImportScope, ast::Path)>,
 ) {
     for (file_id, references) in usages {
         edit.edit_file(file_id);
@@ -217,6 +222,7 @@ fn replace_usages(
                             def.usages(&ctx.sema).all(),
                             target_definition,
                             target_module,
+                            delayed_mutations,
                         )
                     }
                 } else if let Some(initializer) = find_assignment_usage(&name) {
@@ -255,6 +261,7 @@ fn replace_usages(
                                     def.usages(&ctx.sema).all(),
                                     target_definition,
                                     target_module,
+                                    delayed_mutations,
                                 )
                             }
                         }
@@ -306,7 +313,7 @@ fn replace_usages(
                         ImportScope::Module(it) => ImportScope::Module(edit.make_mut(it)),
                         ImportScope::Block(it) => ImportScope::Block(edit.make_mut(it)),
                     };
-                    insert_use(&scope, path, &ctx.config.insert_use);
+                    delayed_mutations.push((scope, path));
                 }
             },
         )
diff --git a/crates/ide/src/runnables.rs b/crates/ide/src/runnables.rs
@@ -78,15 +78,14 @@ impl RunnableKind {
 }
 
 impl Runnable {
-    // test package::module::testname
     pub fn label(&self, target: Option<&str>) -> String {
         match &self.kind {
             RunnableKind::Test { test_id, .. } => format!("test {test_id}"),
             RunnableKind::TestMod { path } => format!("test-mod {path}"),
             RunnableKind::Bench { test_id } => format!("bench {test_id}"),
             RunnableKind::DocTest { test_id, .. } => format!("doctest {test_id}"),
             RunnableKind::Bin => {
-                target.map_or_else(|| "run binary".to_owned(), |t| format!("run {t}"))
+                format!("run {}", target.unwrap_or("binary"))
             }
         }
     }
@@ -513,11 +512,11 @@ impl TestAttr {
     }
 }
 
-const RUSTDOC_FENCES: [&str; 2] = ["```", "~~~"];
-const RUSTDOC_CODE_BLOCK_ATTRIBUTES_RUNNABLE: &[&str] =
-    &["", "rust", "should_panic", "edition2015", "edition2018", "edition2021"];
-
 fn has_runnable_doc_test(attrs: &hir::Attrs) -> bool {
+    const RUSTDOC_FENCES: [&str; 2] = ["```", "~~~"];
+    const RUSTDOC_CODE_BLOCK_ATTRIBUTES_RUNNABLE: &[&str] =
+        &["", "rust", "should_panic", "edition2015", "edition2018", "edition2021"];
+
     docs_from_attrs(attrs).map_or(false, |doc| {
         let mut in_code_block = false;
 
diff --git a/crates/ide/src/syntax_highlighting/test_data/highlight_macros.html b/crates/ide/src/syntax_highlighting/test_data/highlight_macros.html
@@ -94,10 +94,20 @@
     <span class="brace">}</span>
 <span class="brace">}</span>
 
+<span class="keyword">macro_rules</span><span class="macro_bang">!</span> <span class="macro declaration">id</span> <span class="brace">{</span>
+    <span class="parenthesis">(</span><span class="punctuation">$</span><span class="parenthesis">(</span><span class="punctuation">$</span>tt<span class="colon">:</span>tt<span class="parenthesis">)</span><span class="punctuation">*</span><span class="parenthesis">)</span> <span class="operator">=</span><span class="angle">&gt;</span> <span class="brace">{</span>
+        <span class="punctuation">$</span><span class="parenthesis">(</span><span class="punctuation">$</span>tt<span class="parenthesis">)</span><span class="punctuation">*</span>
+    <span class="brace">}</span><span class="semicolon">;</span>
+<span class="brace">}</span>
 
 <span class="macro default_library library">include</span><span class="macro_bang">!</span><span class="parenthesis macro">(</span><span class="macro default_library library macro">concat</span><span class="macro_bang macro">!</span><span class="parenthesis macro">(</span><span class="string_literal macro">"foo/"</span><span class="string_literal macro">,</span> <span class="string_literal macro">"foo.rs"</span><span class="parenthesis macro">)</span><span class="parenthesis macro">)</span><span class="semicolon">;</span>
 
+<span class="keyword">struct</span> <span class="struct declaration">S</span><span class="angle">&lt;</span><span class="type_param declaration">T</span><span class="angle">&gt;</span><span class="parenthesis">(</span><span class="type_param">T</span><span class="parenthesis">)</span><span class="semicolon">;</span>
 <span class="keyword">fn</span> <span class="function declaration">main</span><span class="parenthesis">(</span><span class="parenthesis">)</span> <span class="brace">{</span>
+    <span class="keyword">struct</span> <span class="struct declaration">TestLocal</span><span class="semicolon">;</span>
+    <span class="comment">// regression test, TestLocal here used to not resolve</span>
+    <span class="keyword">let</span> <span class="punctuation">_</span><span class="colon">:</span> <span class="struct">S</span><span class="angle">&lt;</span><span class="macro">id</span><span class="macro_bang">!</span><span class="bracket macro">[</span><span class="struct macro">TestLocal</span><span class="bracket macro">]</span><span class="angle">&gt;</span><span class="semicolon">;</span>
+
     <span class="macro default_library library">format_args</span><span class="macro_bang">!</span><span class="parenthesis macro">(</span><span class="string_literal macro">"Hello, </span><span class="format_specifier">{</span><span class="format_specifier">}</span><span class="string_literal macro">!"</span><span class="comma macro">,</span> <span class="parenthesis macro">(</span><span class="numeric_literal macro">92</span><span class="comma macro">,</span><span class="parenthesis macro">)</span><span class="operator macro">.</span><span class="field library macro">0</span><span class="parenthesis macro">)</span><span class="semicolon">;</span>
     <span class="macro">dont_color_me_braces</span><span class="macro_bang">!</span><span class="parenthesis macro">(</span><span class="parenthesis macro">)</span><span class="semicolon">;</span>
     <span class="macro">noop</span><span class="macro_bang">!</span><span class="parenthesis macro">(</span><span class="macro macro">noop</span><span class="macro_bang macro">!</span><span class="parenthesis macro">(</span><span class="numeric_literal macro">1</span><span class="parenthesis macro">)</span><span class="parenthesis macro">)</span><span class="semicolon">;</span>
diff --git a/crates/ide/src/syntax_highlighting/tests.rs b/crates/ide/src/syntax_highlighting/tests.rs
@@ -102,10 +102,20 @@ macro without_args {
     }
 }
 
+macro_rules! id {
+    ($($tt:tt)*) => {
+        $($tt)*
+    };
+}
 
 include!(concat!("foo/", "foo.rs"));
 
+struct S<T>(T);
 fn main() {
+    struct TestLocal;
+    // regression test, TestLocal here used to not resolve
+    let _: S<id![TestLocal]>;
+
     format_args!("Hello, {}!", (92,).0);
     dont_color_me_braces!();
     noop!(noop!(1));

Original file line number	Diff line number	Diff line change
`@@ -76,7 +76,11 @@ pub(crate) fn bool_to_enum(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option`
`76`	`76`
`77`	`77`	`let usages = definition.usages(&ctx.sema).all();`
`78`	`78`	`add_enum_def(edit, ctx, &usages, target_node, &target_module);`
`79`		`- replace_usages(edit, ctx, usages, definition, &target_module);`
	`79`	`+ let mut delayed_mutations = Vec::new();`
	`80`	`+ replace_usages(edit, ctx, usages, definition, &target_module, &mut delayed_mutations);`
	`81`	`+ for (scope, path) in delayed_mutations {`
	`82`	`+ insert_use(&scope, path, &ctx.config.insert_use);`
	`83`	`+ }`
`80`	`84`	`},`
`81`	`85`	`)`
`82`	`86`	`}`
`@@ -197,6 +201,7 @@ fn replace_usages(`
`197`	`201`	`usages: UsageSearchResult,`
`198`	`202`	`target_definition: Definition,`
`199`	`203`	`target_module: &hir::Module,`
	`204`	`+ delayed_mutations: &mut Vec<(ImportScope, ast::Path)>,`
`200`	`205`	`) {`
`201`	`206`	`for (file_id, references) in usages {`
`202`	`207`	`edit.edit_file(file_id);`
`@@ -217,6 +222,7 @@ fn replace_usages(`
`217`	`222`	`def.usages(&ctx.sema).all(),`
`218`	`223`	`target_definition,`
`219`	`224`	`target_module,`
	`225`	`+ delayed_mutations,`
`220`	`226`	`)`
`221`	`227`	`}`
`222`	`228`	`} else if let Some(initializer) = find_assignment_usage(&name) {`
`@@ -255,6 +261,7 @@ fn replace_usages(`
`255`	`261`	`def.usages(&ctx.sema).all(),`
`256`	`262`	`target_definition,`
`257`	`263`	`target_module,`
	`264`	`+ delayed_mutations,`
`258`	`265`	`)`
`259`	`266`	`}`
`260`	`267`	`}`
`@@ -306,7 +313,7 @@ fn replace_usages(`
`306`	`313`	`ImportScope::Module(it) => ImportScope::Module(edit.make_mut(it)),`
`307`	`314`	`ImportScope::Block(it) => ImportScope::Block(edit.make_mut(it)),`
`308`	`315`	`};`
`309`		`- insert_use(&scope, path, &ctx.config.insert_use);`
	`316`	`+ delayed_mutations.push((scope, path));`
`310`	`317`	`}`
`311`	`318`	`},`
`312`	`319`	`)`
Original file line number	Diff line number	Diff line change
`@@ -78,15 +78,14 @@ impl RunnableKind {`
`78`	`78`	`}`
`79`	`79`
`80`	`80`	`impl Runnable {`
`81`		`- // test package::module::testname`
`82`	`81`	`pub fn label(&self, target: Option<&str>) -> String {`
`83`	`82`	`match &self.kind {`
`84`	`83`	`RunnableKind::Test { test_id, .. } => format!("test {test_id}"),`
`85`	`84`	`RunnableKind::TestMod { path } => format!("test-mod {path}"),`
`86`	`85`	`RunnableKind::Bench { test_id } => format!("bench {test_id}"),`
`87`	`86`	`RunnableKind::DocTest { test_id, .. } => format!("doctest {test_id}"),`
`88`	`87`	`RunnableKind::Bin => {`
`89`		`- target.map_or_else(\|\| "run binary".to_owned(), \|t\| format!("run {t}"))`
	`88`	`+ format!("run {}", target.unwrap_or("binary"))`
`90`	`89`	`}`
`91`	`90`	`}`
`92`	`91`	`}`
`@@ -513,11 +512,11 @@ impl TestAttr {`
`513`	`512`	`}`
`514`	`513`	`}`
`515`	`514`
`516`		-const RUSTDOC_FENCES: [&str; 2] = ["```", "~~~"];
`517`		`-const RUSTDOC_CODE_BLOCK_ATTRIBUTES_RUNNABLE: &[&str] =`
`518`		`- &["", "rust", "should_panic", "edition2015", "edition2018", "edition2021"];`
`519`		`-`
`520`	`515`	`fn has_runnable_doc_test(attrs: &hir::Attrs) -> bool {`
	`516`	+ const RUSTDOC_FENCES: [&str; 2] = ["```", "~~~"];
	`517`	`+ const RUSTDOC_CODE_BLOCK_ATTRIBUTES_RUNNABLE: &[&str] =`
	`518`	`+ &["", "rust", "should_panic", "edition2015", "edition2018", "edition2021"];`
	`519`	`+`
`521`	`520`	`docs_from_attrs(attrs).map_or(false, \|doc\| {`
`522`	`521`	`let mut in_code_block = false;`
`523`	`522`