fix: correctly handle non-ascii uppercase characters in search index key generation and index queries - #23388
NickelWenzel wants to merge 5 commits into
Conversation
…x query consistent Previously the search index generation would not lower non-ascii characters while the symbol index query would. As a first step to fix the non-ascii upper case character handling a helper method which lowers ascii and non-ascii characters is introduced in stdx and used consistently for search index key generation of the import map and for index queries. AI disclaimer: I used Claude Opus 5 for the initial draft of this commit but reviewed and reworked it. The commit message is my own.
Use to_lowercase_chars also for checking match candidates in exact and prefix mode. Non-ascii lowercasing does not preserve length so comparing character by character prevents panicking. Adapt the fuzzy logic for non-ascii characters as well. AI disclaimer: I used Claude Opus 5 for the initial draft of this commit but reviewed and reworked it. The commit message is my own.
…import candidates The case insensitive prefix arm used `zip`, which stops at the shorter side and returned true whenever the query was longer than the name. Using to_lowercase_chars fixes that as well. AI disclaimer: I used Claude Opus 5 for the initial draft of this commit but reviewed and reworked it. The commit message is my own.
the test cover auto import assist, flyimport completion and workspace symbol search. Those failed before the previous commits. AI disclaimer: I used Claude Opus 5 for the initial draft of this commit but reviewed and reworked it. The commit message is my own.
Short-cuircuiting when possible avoids the slower comparison after calling `char::to_lowercase`. AI disclaimer: I used Claude Opus 5 for the initial draft of this commit but reviewed and reworked it. The commit message is my own.
rustbot
added
the
S-waiting-on-review
|
First, this does not really fix the full issue, because lowercasing then comparing is just not possible in Unicode. Second, I truly wonder if we should fix this at all. Unicode identifiers are possible in Rust, that's true, but I've never seen somebody using them in production code. Do some people do? Maybe. Should we support all features for them, even at a cost? Maybe not. |
|
CC @rust-lang/rust-analyzer. |
True, but with the changes here, what could happen is that you'd get false positive on matches which IMHO is preferable over false negatives (not getting any matches) which is the current behavior.
I get this and i was a bit worried that this issue might be considered too niche to fix it. From my own experience I can say that I'm working in a production code base where the domain language is german so we use german terms and we are actually affected by the uppercase unicode handling and it is annoying. Also RustRover does not have this issue if that makes any difference. Anyways if the issue should be fixed but you don't feel comfortable merging my solution here I'd be happy to chip away at it to bring it in a mergable form. |
3 participants
Before this PR:
After this PR:
With the changes introduced here non-ascii uppercase characters are consistently lowered throughout search index key generation and index queries.
Doing so required adapting some of logic for checking match candidates and fuzzy search.
As the changes had some negative performance implications the last commit introduces some performance improvements for cases with ascii-only characters.
Fixes #23387
AI disclaimer: I used Claude Opus 5 to find the cause for the issues as well for drafting the fix.
As the changes touch
hir-defthe AI_POLICY.md requires me to include a citation of the rustc code responsible for the change I did, along with an explanation of how my change follows from it in case this is not immediately clear.So I think it's reasonable to cite the following lines from the rustc code which specify what if a character is valid as a character of an identifier (see also here). From this follows that unicode characters (upper- and lowercase) are to be expected in Identifiers and therefore RA should be able to handle them correctly.