Skip to content

Commit f98e971

Browse files
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.
1 parent 8b25ab0 commit f98e971

File tree

2 files changed

+36
-11
lines changed

2 files changed

+36
-11
lines changed

src/tools/rust-analyzer/crates/ide-completion/src/completions/item_list/trait_impl.rs

+26-6
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ fn complete_trait_impl_name(
8585
name: &Option<ast::Name>,
8686
kind: ImplCompletionKind,
8787
) -> Option<()> {
88-
let item = match name {
88+
let macro_file_item = match name {
8989
Some(name) => name.syntax().parent(),
9090
None => {
9191
let token = &ctx.token;
@@ -96,20 +96,20 @@ fn complete_trait_impl_name(
9696
.parent()
9797
}
9898
}?;
99-
let item = ctx.sema.original_syntax_node_rooted(&item)?;
99+
let real_file_item = ctx.sema.original_syntax_node_rooted(&macro_file_item)?;
100100
// item -> ASSOC_ITEM_LIST -> IMPL
101-
let impl_def = ast::Impl::cast(item.parent()?.parent()?)?;
101+
let impl_def = ast::Impl::cast(macro_file_item.parent()?.parent()?)?;
102102
let replacement_range = {
103103
// ctx.sema.original_ast_node(item)?;
104-
let first_child = item
104+
let first_child = real_file_item
105105
.children_with_tokens()
106106
.find(|child| {
107107
!matches!(
108108
child.kind(),
109109
SyntaxKind::COMMENT | SyntaxKind::WHITESPACE | SyntaxKind::ATTR
110110
)
111111
})
112-
.unwrap_or_else(|| SyntaxElement::Node(item.clone()));
112+
.unwrap_or_else(|| SyntaxElement::Node(real_file_item.clone()));
113113

114114
TextRange::new(first_child.text_range().start(), ctx.source_range().end())
115115
};
@@ -1658,7 +1658,7 @@ trait Trait {
16581658
impl Trait for () {
16591659
f$0
16601660
}
1661-
"#,
1661+
"#,
16621662
expect![[r#"
16631663
me fn bar(..)
16641664
me fn baz(..)
@@ -1668,5 +1668,25 @@ impl Trait for () {
16681668
kw self::
16691669
"#]],
16701670
);
1671+
check(
1672+
r#"
1673+
//- proc_macros: identity
1674+
trait Trait {
1675+
fn foo(&self) {}
1676+
fn bar(&self) {}
1677+
fn baz(&self) {}
1678+
}
1679+
1680+
#[proc_macros::identity]
1681+
impl Trait for () {
1682+
fn $0
1683+
}
1684+
"#,
1685+
expect![[r#"
1686+
me fn bar(..)
1687+
me fn baz(..)
1688+
me fn foo(..)
1689+
"#]],
1690+
);
16711691
}
16721692
}

src/tools/rust-analyzer/crates/ide-completion/src/context/analysis.rs

+10-5
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use hir::{ExpandResult, Semantics, Type, TypeInfo, Variant};
55
use ide_db::{active_parameter::ActiveParameter, RootDatabase};
66
use itertools::Either;
77
use syntax::{
8-
algo::{ancestors_at_offset, find_node_at_offset, non_trivia_sibling},
8+
algo::{self, ancestors_at_offset, find_node_at_offset, non_trivia_sibling},
99
ast::{
1010
self, AttrKind, HasArgList, HasGenericArgs, HasGenericParams, HasLoopBody, HasName,
1111
NameOrNameRef,
@@ -85,6 +85,11 @@ pub(super) fn expand_and_analyze(
8585
})
8686
}
8787

88+
fn token_at_offset_ignore_whitespace(file: &SyntaxNode, offset: TextSize) -> Option<SyntaxToken> {
89+
let token = file.token_at_offset(offset).left_biased()?;
90+
algo::skip_whitespace_token(token, Direction::Prev)
91+
}
92+
8893
/// Expand attributes and macro calls at the current cursor position for both the original file
8994
/// and fake file repeatedly. As soon as one of the two expansions fail we stop so the original
9095
/// and speculative states stay in sync.
@@ -125,9 +130,7 @@ fn expand(
125130

126131
// Left biased since there may already be an identifier token there, and we appended to it.
127132
if !sema.might_be_inside_macro_call(&fake_ident_token)
128-
&& original_file
129-
.token_at_offset(original_offset + relative_offset)
130-
.left_biased()
133+
&& token_at_offset_ignore_whitespace(&original_file, original_offset + relative_offset)
131134
.is_some_and(|original_token| !sema.might_be_inside_macro_call(&original_token))
132135
{
133136
// Recursion base case.
@@ -143,9 +146,11 @@ fn expand(
143146

144147
let parent_item =
145148
|item: &ast::Item| item.syntax().ancestors().skip(1).find_map(ast::Item::cast);
149+
let original_node = token_at_offset_ignore_whitespace(&original_file, original_offset)
150+
.and_then(|token| token.parent_ancestors().find_map(ast::Item::cast));
146151
let ancestor_items = iter::successors(
147152
Option::zip(
148-
find_node_at_offset::<ast::Item>(&original_file, original_offset),
153+
original_node,
149154
find_node_at_offset::<ast::Item>(
150155
&speculative_file,
151156
fake_ident_token.text_range().start(),

0 commit comments

Comments
 (0)