Remove length limit on spelling suggestions; use levenshteinWithMax for performance#19937
Remove length limit on spelling suggestions; use levenshteinWithMax for performance#199374 commits merged into
Conversation
| nameLowerCase.length < 3 || | ||
| candidateNameLowerCase === "eval" || | ||
| candidateNameLowerCase === "intl" || | ||
| candidateNameLowerCase === "undefined" || |
There was a problem hiding this comment.
Why are these excluded? I'd really like to be able to automatically fix undefnied.
There was a problem hiding this comment.
The suggestions were not good at one point. That might have changed during final tweaks to the numbers, because I ended up less likely to give suggestions for short identifiers than when I was mid-development. That's specifically why the 3-4 character identifiers are here. Maybe undefined is no longer affected.
Coverage is pretty good at this point, so if you remove items from this list, you should see what bad suggestions happen.
There was a problem hiding this comment.
(If any; it may be fine to suggest undefined now)
|
Why move this? It makes reading the diff unnecessarily difficult, and I think it's only used in the checker anyway. |
| @@ -1,4 +1,4 @@ | ||
| tests/cases/conformance/internalModules/exportDeclarations/ModuleWithExportedAndNonExportedFunctions.ts(28,13): error TS2339: Property 'fn2' does not exist on type 'typeof A'. | ||
| tests/cases/conformance/internalModules/exportDeclarations/ModuleWithExportedAndNonExportedFunctions.ts(28,13): error TS2551: Property 'fn2' does not exist on type 'typeof A'. Did you mean 'fn'? |
There was a problem hiding this comment.
This should not have changed. The lower-bound cutoff was specifically designed to leave deletions from 3-character identifiers alone since they are really hard to miss visually.
(The same is almost as true of 4-character identifiers, as on the next line, but I remember coming up with a couple of convincing counterexamples.)
| continue; | ||
| } | ||
| if (distance < 3) { | ||
| justCheckExactMatches = true; |
There was a problem hiding this comment.
While we're here, why have this? Why shouldn't we look for result with a distance of 1 even after one with a distance of 2 is available?
There was a problem hiding this comment.
A lot of these fast paths exist because originally this function was called on all unknown identifiers, which usually results in running Levenshtein distance on over 1000 identifiers from the global scope, plus any lexical scopes. Currently this only happens until 10 suggestions have been found, which could still be a significant fraction of the unknown identifiers in a program if no good candidates are found.
Also, this function is called for unknown property accesses and unknown imports (I may be misremembering since I didn’t write that one), both of which have smaller search spaces. Future uses could result in larger search spaces though.
5dac325 to
5d4240c
Compare
Removes the length limit so we can get spelling suggestions for
Diagnostics.new_T_cannot_be_used_to_create_an_aray_Use_new_Array_T_instead.Algorithm is roughly based on https://www.lemoda.net/c/edit-distance-with-max/. I fixed a bug where they set
col_min = INT_MAXwhen they should have set it toi(which is the value at the 0th element in the column).