From bfb187aacd23e3af4d074322625c84a263a63b6f Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Sun, 30 Jun 2024 13:26:17 +0200 Subject: [PATCH] Fix expression scope calculation when within macro expansions --- Cargo.toml | 4 +- crates/hir-expand/src/files.rs | 34 ++++++++++++ crates/hir/src/source_analyzer.rs | 52 +++++++++++-------- .../ide-assists/src/handlers/bool_to_enum.rs | 11 +++- crates/ide/src/runnables.rs | 11 ++-- .../test_data/highlight_macros.html | 10 ++++ crates/ide/src/syntax_highlighting/tests.rs | 10 ++++ 7 files changed, 99 insertions(+), 33 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 583c7bbe33..18c9bb5ce0 100644 --- 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 index 743fac50f4..90d9e61a69 100644 --- a/crates/hir-expand/src/files.rs +++ b/crates/hir-expand/src/files.rs @@ -158,6 +158,40 @@ impl InFileWrapper { // region:specific impls impl InFile<&SyntaxNode> { + pub fn parent_ancestors_with_macros( + self, + db: &dyn db::ExpandDatabase, + ) -> impl Iterator> + '_ { + let succ = move |node: &InFile| 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> + '_ { + let succ = move |node: &InFile| 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 index 8e71a54f80..be8e9c49a0 100644 --- 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, ) -> 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, + ) -> 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, + infer: Option>, ) -> 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 { - 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 index c95e24693d..2aeca0bae0 100644 --- 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 index 2feea09840..a68ee4f867 100644 --- a/crates/ide/src/runnables.rs +++ b/crates/ide/src/runnables.rs @@ -78,7 +78,6 @@ 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}"), @@ -86,7 +85,7 @@ impl Runnable { 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 index 573b3d4bd5..17411fefbd 100644 --- 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 @@ pre { color: #DCDCCC; background: #3F3F3F; font-size: 22px; padd } } +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)); diff --git a/crates/ide/src/syntax_highlighting/tests.rs b/crates/ide/src/syntax_highlighting/tests.rs index 08acfca2cb..5f711600a2 100644 --- 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); fn main() { + struct TestLocal; + // regression test, TestLocal here used to not resolve + let _: S; + format_args!("Hello, {}!", (92,).0); dont_color_me_braces!(); noop!(noop!(1));