Skip to content

Commit

Permalink
Fix another bug with completion of trait items inside macros
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
ChayimFriedman2 committed Jan 20, 2025
1 parent 8b25ab0 commit f98e971
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -96,20 +96,20 @@ 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!(
child.kind(),
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())
};
Expand Down Expand Up @@ -1658,7 +1658,7 @@ trait Trait {
impl Trait for () {
f$0
}
"#,
"#,
expect![[r#"
me fn bar(..)
me fn baz(..)
Expand All @@ -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(..)
"#]],
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand All @@ -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(),
Expand Down

0 comments on commit f98e971

Please sign in to comment.