Fix completion with backticks, underscores, numbers

[zanaptak]

This is to address the following issues:

• Underscores or special names with backticks are problematic with auto-completion Underscores or special names with backticks are problematic with auto-completion #3406
• Completion completes too much Completion completes too much #4515
• Completion list appears to treat digit/underscore in identifier as a boundary Completion list appears to treat digit/underscore in identifier as a boundary #6134

Which are also listed in this issue:

• Rollup issue for completion (ctrl+space, ctrl+J, etc. - not autocompletion) Rollup issue for completion (ctrl+space, ctrl+J, etc. - not autocompletion) #6486

Before PR:

After PR:

The problem (if I traced the right code) is that the default logic (erroneously?) only captures identifier-start characters to the left of the cursor, and further defines start characters as letters only.

https://github.com/dotnet/roslyn/blob/823039e98e56e527d40b4fbcb6bf9755ce02855c/src/Features/Core/Portable/Completion/CompletionService.cs#L102

C# overrides this to include identifier-part characters so I've used the same logic here, this handles underscores and numbers.

For backticks, my initial code shows it can work but is not ready for primetime. Simple character matching isn't enough to properly restrict the range especially in the case of partial entry. As in let x = (``typing here... , ``already-typed``), how to know it shouldn't include the comma? Is this an already-considered scenario somewhere in the existing tooling codebase? I'd appreciate any guidance or pointers to help with this.

[zanaptak]

I've changed it to use the Tokenizer directly, so that it can easily handle multiple backticks, backticks in strings, etc.

Unfortunately it turns out the rest of the completion pipeline is not as robust. E.g. something like let x = ["``"; A.``#1``; B. works fine with the new completion span logic, but the later completion list logic fails to understand that B is the qualifying type in ProvideCompletionsAsyncAux.

I'll play around some more to see if feasible to update, otherwise maybe it's ok to leave some edge cases if the majority of typical cases work.

[cartermp]

otherwise maybe it's ok to leave some edge cases if the majority of typical cases work.

I agree. I'm happy to review with aims to take this as is provided that tests pass. Just let me know if you want to proceed with that or hold off until you're finished experimenting a little more.

[zanaptak]

Ok I'll skip digging into the rest of the pipeline for now so this can move forward.

[cartermp]

Thanks! Mind marking this as no longer WIP? Then we can review.

[zanaptak]

Ready to go, hopefully the test structure is ok, made it a separate utility function.

[cartermp]

This is great, thanks! Will merge subject to CI passing.

[abelbraaksma]

Wonderful work! Since specifically for awkward names, completion is a must have, and removing redundant words or backticks was quite a pain. Very happy you solved this!!