Unnamed repository; edit this file 'description' to name the repository.
Fix another bug with completion of trait items inside macros
This time, when completing the keyword (e.g. `fn` + whitespace). The bug was actually a double-bug: First, we did not resolve the impl in the macro-expanded file but in the real file, which of course cannot work. Second, in analysis the whitespace was correlated with the `impl` and not the incomplete `fn`, which caused fake (where we insert an identifier after the whitespace) and real expansions to go out of sync, which failed analysis. The fix is to skip whitespaces in analysis.
Chayim Refael Friedman 2025-01-21
parent a282659 · commit ce17596
-rw-r--r--crates/ide-completion/src/completions/item_list/trait_impl.rs32
-rw-r--r--crates/ide-completion/src/context/analysis.rs15
2 files changed, 36 insertions, 11 deletions
diff --git a/crates/ide-completion/src/completions/item_list/trait_impl.rs b/crates/ide-completion/src/completions/item_list/trait_impl.rs
index 246b125266..18629529b7 100644
--- a/crates/ide-completion/src/completions/item_list/trait_impl.rs
+++ b/crates/ide-completion/src/completions/item_list/trait_impl.rs
@@ -85,7 +85,7 @@ fn complete_trait_impl_name(
name: &Option<ast::Name>,
kind: ImplCompletionKind,
) -> Option<()> {
- let item = match name {
+ let macro_file_item = match name {
Some(name) => name.syntax().parent(),
None => {
let token = &ctx.token;
@@ -96,12 +96,12 @@ fn complete_trait_impl_name(
.parent()
}
}?;
- let item = ctx.sema.original_syntax_node_rooted(&item)?;
+ let real_file_item = ctx.sema.original_syntax_node_rooted(&macro_file_item)?;
// item -> ASSOC_ITEM_LIST -> IMPL
- let impl_def = ast::Impl::cast(item.parent()?.parent()?)?;
+ let impl_def = ast::Impl::cast(macro_file_item.parent()?.parent()?)?;
let replacement_range = {
// ctx.sema.original_ast_node(item)?;
- let first_child = item
+ let first_child = real_file_item
.children_with_tokens()
.find(|child| {
!matches!(
@@ -109,7 +109,7 @@ fn complete_trait_impl_name(
SyntaxKind::COMMENT | SyntaxKind::WHITESPACE | SyntaxKind::ATTR
)
})
- .unwrap_or_else(|| SyntaxElement::Node(item.clone()));
+ .unwrap_or_else(|| SyntaxElement::Node(real_file_item.clone()));
TextRange::new(first_child.text_range().start(), ctx.source_range().end())
};
@@ -1658,7 +1658,7 @@ trait Trait {
impl Trait for () {
f$0
}
- "#,
+ "#,
expect![[r#"
me fn bar(..)
me fn baz(..)
@@ -1668,5 +1668,25 @@ impl Trait for () {
kw self::
"#]],
);
+ check(
+ r#"
+//- proc_macros: identity
+trait Trait {
+ fn foo(&self) {}
+ fn bar(&self) {}
+ fn baz(&self) {}
+}
+
+#[proc_macros::identity]
+impl Trait for () {
+ fn $0
+}
+ "#,
+ expect![[r#"
+ me fn bar(..)
+ me fn baz(..)
+ me fn foo(..)
+ "#]],
+ );
}
}
diff --git a/crates/ide-completion/src/context/analysis.rs b/crates/ide-completion/src/context/analysis.rs
index 3c4d489c0f..562f60dbe2 100644
--- a/crates/ide-completion/src/context/analysis.rs
+++ b/crates/ide-completion/src/context/analysis.rs
@@ -5,7 +5,7 @@ use hir::{ExpandResult, Semantics, Type, TypeInfo, Variant};
use ide_db::{active_parameter::ActiveParameter, RootDatabase};
use itertools::Either;
use syntax::{
- algo::{ancestors_at_offset, find_node_at_offset, non_trivia_sibling},
+ algo::{self, ancestors_at_offset, find_node_at_offset, non_trivia_sibling},
ast::{
self, AttrKind, HasArgList, HasGenericArgs, HasGenericParams, HasLoopBody, HasName,
NameOrNameRef,
@@ -85,6 +85,11 @@ pub(super) fn expand_and_analyze(
})
}
+fn token_at_offset_ignore_whitespace(file: &SyntaxNode, offset: TextSize) -> Option<SyntaxToken> {
+ let token = file.token_at_offset(offset).left_biased()?;
+ algo::skip_whitespace_token(token, Direction::Prev)
+}
+
/// Expand attributes and macro calls at the current cursor position for both the original file
/// and fake file repeatedly. As soon as one of the two expansions fail we stop so the original
/// and speculative states stay in sync.
@@ -125,9 +130,7 @@ fn expand(
// Left biased since there may already be an identifier token there, and we appended to it.
if !sema.might_be_inside_macro_call(&fake_ident_token)
- && original_file
- .token_at_offset(original_offset + relative_offset)
- .left_biased()
+ && token_at_offset_ignore_whitespace(&original_file, original_offset + relative_offset)
.is_some_and(|original_token| !sema.might_be_inside_macro_call(&original_token))
{
// Recursion base case.
@@ -143,9 +146,11 @@ fn expand(
let parent_item =
|item: &ast::Item| item.syntax().ancestors().skip(1).find_map(ast::Item::cast);
+ let original_node = token_at_offset_ignore_whitespace(&original_file, original_offset)
+ .and_then(|token| token.parent_ancestors().find_map(ast::Item::cast));
let ancestor_items = iter::successors(
Option::zip(
- find_node_at_offset::<ast::Item>(&original_file, original_offset),
+ original_node,
find_node_at_offset::<ast::Item>(
&speculative_file,
fake_ident_token.text_range().start(),