In VS Code, rust-analyzer prefers postfix completions over methods/traits

[davidbarsky]

Just as a heads up, as of #17073, postfix completions/helpers are showing up first in VS Code. With a rust-analyzer built at c037130, these are the completions I'm getting:

With a rust-analyzer built at af72874, I'm getting the postfix completions first:

I'm not sure how intentional that is, but it certainly feels a bit like a bug to me.

Originally posted by @davidbarsky in #17073 (comment)

[davidbarsky]

Note that adding "editor.snippetSuggestions": "bottom" to the settings brought back the prior behavior, but I'm not sure it's a good idea to users to make that change.

[davidbarsky]

Is this because VS Code considers 1.ok to be a better match for 1. than oxxf (some fns)? 🤔

I think so.

We might need to keep the order with sort_text.

In rst-analyzer itself or the extension?

[roife]

r-a would calculate a "score" based on various factors and converts it into sort_text.

But it is quite strange because I found that the score for borrow() is 15 while the score for box is 13, yet box ranks higher.

[pascalkuthe]

VSCode does not follow the spec and both filters and sorts incomplete completion lists based on text score (and most servers including ra rely on that to some degree so every other client is forced to follow this non-standard behavior if we dont want broken completions). The real fix would for vscode to comply with the LSP spec.

I am not sure you can do much about this server side if sorttext is already set correctly.

[pascalkuthe]

Thus will probably not be a huge problem once you typed one or two characters since then the real completions will have a better textual score. So you could just hide postfix completions when there is a trigger char (after . and ::)

[davidbarsky]

VSCode does not follow the spec and both filters and sorts incomplete completion lists based on text score (and most servers including ra rely on that to some degree so every other client is forced to follow this non-standard behavior if we dont want broken completions). The real fix would for vscode to comply with the LSP spec.

That's unfortunately a common situation, but that's not really something we can do in the next week.

Thus will probably not be a huge problem once you typed one or two characters since then the real completions will have a better textual score. So you could just hide postfix completions when there is a trigger char (after . and ::)

I think you underestimate how much of an issue this might be, especially given that people would write a single dot completion to determine what methods are available to them. Given that VS Code is the primary supported editor of rust-analyzer, we'll need to find a workaround.

[pascalkuthe]

I meant to hide postfix completions like ok after trigger chars. That would allow scrolling trough available methods without seeing the postfix snippets.

Once you start actually typing text you can show the postfix snuppets again because then the score of whatever you are looking for will be higher than postfix snippets (except when you are actually looking for postfix snippets)

[davidbarsky]

I meant to hide postfix completions like ok after trigger chars. That would allow scrolling trough available methods without seeing the postfix snippets.

That could work, but I'm not sure how you're proposing to distinguish a trigger character for postfix completions versus a method completions (I'm thinking of some postfix snippets like .match).

The alternative I'm considering is to scoring behavior by editor (e.g., special-casing VS Code).

[pascalkuthe]

That could work, but I'm not sure how you're proposing to distinguish a trigger character for postfix completions versus a method completions (I'm thinking of some postfix snippets like .match).

Afaim postfix completions (like .ok, .match,...) only show up after periods right now. The PR that regressed this made RA include the text before the period (and the period itself) in the filter text. So I thought you could simply check if:

• filterText contains a period
• the completion trigger position is immieditly after a period

If both are true the completion is not send to the client.

If this solution works without side-effects that would be ideal since it would be editor agnostic.

The alternative I'm considering is to change behavior in scoring by editor—special case VS Code.

As far as I can tell the problem here is that vscode does uncofigurable and incorrect client side scoring so I don't think you would be able to do editor specific scoring on the server side.

This could also be just an issue with the ra side scoring/sort text but if I understand the comment above correctly (which matches with what I found recently looking trough the vscode codebase). Vscode always does client side scoring even when you disabled it by marking the completion as incomplete.

[davidbarsky]

Afaim postfix completions (like .ok, .match,...) only show up after periods right now. The PR that regressed this made RA include the text before the period (and the period itself) in the filter text. So I thought you could simply check if:

• filterText contains a period
• the completion trigger position is immieditly after a period

If both are true the completion is not send to the client.

From my testing, this seems to effectively disable postfix/magic completions entirely, especially since there isn't any other way to activate them beyond dot completions.

In reference to your prior comment about the LSP specification, I'm not sure I agree with your interpretation for the following reasons:

• The phrasing of "[t]he range of the edit must be a single line range and it must contain the position at which completion has been requested" appears to only be in reference to the primary textEdit on CompletionItem, not additionalTextEdits.
• Similarly, the constraint of "the range of the edit" does not appear on TextEdit, which makes me think the aforementioned range/position constraint is only applicable to CompletionItem::textEdit, not TextEdits in general.
• The documentation for additionalTextEdits says "[a]dditional text edits should be used to change text unrelated to the current cursor position", with imports being a cited as an example. However, adding a &mut before a . also fits the definition of "unrelated", albeit more tenuously. Therefore, I think rust-analyzer has the latitude to be more liberal with its usage of additionalTextEdits because of the distinction between "should" and "must", despite the difficulties with multi-cursor edits.

As far as I can tell the problem here is that vscode does uncofigurable and incorrect client side scoring so I don't think you would be able to do editor specific scoring on the server side.

The approach I had in mind was to gate the behavior introduced in #17073 by the editor name, but that feels like a nightmare to test/maintain. I have no love lost for VS Code, but the behavior is technically configurable by the end-user, just not language servers. Regardless, I think it's problematic to require end-users to make a change to their editor in order to return to behavior that they're used to, which is why I'm personally inclined to revert #17073.

[davidbarsky]

Anyways, I was talking @Wilfred about VS Code's autocomplete/intellisense, and he pointed out that it is very good about finding applicable completions even if you mistype a portion of your what you expect to be autocompleted, and I think a portion of that magic-when-it-works experience is them factoring in the starting point of a line range. Moving the range to start prior to the cursor is probably getting VS Code to prioritize those completions more because they might be using that as a proxy for "the user is in the middle of writing a long method, so we should suggest that".