Macro string literal completion edits not replacing prior text

[RoloEdits]

rust-analyzer version: rust-analyzer 0.0.0 (498b94b730 2024-06-17)

rustc version: rustc 1.74.0 (79e9716c9 2023-11-13)

editor or extension: helix

code snippet to reproduce:

// add your code here
use chrono::serde::ts_milliseconds;
use chrono::{DateTime, Utc};
use serde::Deserialize;

#[derive(Deserialize)]
struct A {
    #[serde(with = "ts_mill")]
    time: DateTime<Utc>,
}

fn main() {
    println!("Hello, world!");
}

Originally thought it was a helix issue, but was directed here after some discussion. https://github.com/helix-editor/helix/discussions/11020

[Veykril]

Hmm, we shouldn't trigger those there at all actually given we are inside a string token. It's not really possibly for us to treat this completion correctly inside a string.

[aaishwarymishra]

@rustbot claim

[aaishwarymishra]

I would like to try fixing this, @Veykril can you please help a little with relevant code and some instruction if possible as it would be my first contribution. I would appreciate it.

[ChayimFriedman2]

@BLANKatGITHUB If you want to disable completion inside strings, this should be simple - check if ctx.original_token is a string here.

[Veykril]

I think we need to check if original_token and self_token (bad name, this is the token in the expansion) here https://github.com/rust-lang/rust-analyzer/blob/ba1c9146fcca65fa9c40d1e4e73bdcb898a8de46/crates/ide-completion/src/context/analysis.rs#L278-L283 differ in terms of what they are.

So a basic check on equality of their syntax kind ought to be enough + if that check fails (that is they differ) do another check if both of them are identifiers (as keywords get their own kind but we want to treat that as identifiers), ie. in pseudo-ish code if original_token.kind() != self_token.kind() && is_ident_like(original_token.kind()) != is_ident_like(self_token.kind()), we should break out and dont do completions, we have an is_ident_like function somehwere (might be a method on SyntaxKind

[aaishwarymishra]

@BLANKatGITHUB If you want to disable completion inside strings, this should be simple - check if ctx.original_token is a string here.

Thanks for help but its still kind of failing 2 test's, I would appreciate your help as its my first time contributing.

[ChayimFriedman2]

What do the failed tests print?

[aaishwarymishra]

What do the failed tests print?

I kind of implemented

if ctx.original_token == syntax::Syntax::STRING{ return None }

it failed 2 tests now I am trying kind of simple code if trigger_character == Some('"') { return None; } Sorry if I am doing anything wrong and taking your time, I have never worked on big project like this so I dont understand much of code base, but I would like to help the project if I can , I hope you can guide me

[aaishwarymishra]

I think we need to check if original_token and self_token (bad name, this is the token in the expansion) here

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

Lines 278 to 283 in ba1c914

fn analyze( sema: &Semantics<'_, RootDatabase>, expansion_result: ExpansionResult, original_token: &SyntaxToken, self_token: &SyntaxToken, ) -> Option<(CompletionAnalysis, (Option<Type>, Optionast::NameOrNameRef), QualifierCtx)> {

differ in terms of what they are. So a basic check on equality of their syntax kind ought to be enough + if that check fails (that is they differ) do another check if both of them are identifiers (as keywords get their own kind but we want to treat that as identifiers), ie. in pseudo-ish code if original_token.kind() != self_token.kind() && is_ident_like(original_token.kind()) != is_ident_like(self_token.kind()), we should break out and dont do completions, we have an is_ident_like function somehwere (might be a method on SyntaxKind

I cant find 'is_ident_like' can you please help a little more , I am trying both suggestion given to me in this issue, I kind of implemented one and submitted for review I am not sure about it so help fom expierenced people will be appreciated

[Veykril]

Its this one https://github.com/rust-lang/rust-analyzer/blob/597d8e837a0a1a48349164d4057f6e42765d7a32/crates/parser/src/syntax_kind.rs#L34

[aaishwarymishra]

Umm I added if (original_token.kind() != self_token.kind()) && (SyntaxKind::is_any_identifier(original_token.kind()) != SyntaxKind::is_any_identifier(self_token.kind())) { return None; }

but it failed some tests

'failures: render::macro_::tests::complete_missing_macro_arg tests::expression::in_macro_expr_frag '

In first step it was unwrapping a None value, and in second there was no completion for non string items

[vishruth-thimmaiah]

@rustbot claim

[vishruth-thimmaiah]

So a basic check on equality of their syntax kind ought to be enough + if that check fails (that is they differ) do another check if both of them are identifiers (as keywords get their own kind but we want to treat that as identifiers), ie. in pseudo-ish code if original_token.kind() != self_token.kind() && is_ident_like(original_token.kind()) != is_ident_like(self_token.kind()), we should break out and dont do completions, we have an is_ident_like function somehwere (might be a method on SyntaxKind

@Veykril - I attempted to implement your suggested solution, but it wasn't successful. Instead, I found a working alternative approach: comparing the token types by checking if original_token.kind() is a string while self_token.kind() is not. Could you please review if this is a valid approach or if there might be edge cases I should consider?

[vishruth-thimmaiah]

I attempted to implement your suggested solution, but it wasn't successful.

I did some digging, and looks like commit - 15d2d509d0fff1e46c5a7db16e7a692ee26ae029 is the reason your suggestion isn't working - for the code snippet provided, the value of self_token is COLON2, instead of IDENT.

[Veykril]

Oh my condition is wrong, that should be !(original_token.kind() == self_token.kind() || (is_ident_like(original_token.kind()) && is_ident_like(self_token.kind()))) or with the negation pulled further in original_token.kind() != self_token.kind() && !(is_ident_like(original_token.kind()) && is_ident_like(self_token.kind()))

That is, skip completions if the kinds differ unless they differ in being keywords / identifiers. -> Do completions if the kinds are the same or both kinds are some pairing of identifiers or keywords.