Codefix "Correct spelling" stops working when 3 spaces are missing, or suggests bad replacement

[AlCalzone]

Bug Report

🔎 Search Terms

codefix spelling spaces

🕗 Version & Regression Information

• This is the behavior in every version I tried

⏯ Playground Link

Playground Link

💻 Code

enum NamesWithSpaces {
	"NoSpace",
	"One Space",
	"Has Two Spaces",
	"This Has Three Spaces",
	"And This Has Four Spaces",

	"Block One",
	"Block  Two",
	"Block   Three",
}

const test0 = NamesWithSpaces.NoSpace;
const test1 = NamesWithSpaces.OneSpace; // Codefix available
const test2 = NamesWithSpaces.HasTwoSpaces; // Codefix available
const test3 = NamesWithSpaces.ThisHasThreeSpaces; // NO codefix available
const test4 = NamesWithSpaces.AndThisHasFourSpaces; // NO codefix available

const block1 = NamesWithSpaces.BlockOne; // Codefix available
const block2 = NamesWithSpaces.BlockTwo; // Codefix available
const block3 = NamesWithSpaces.BlockThree; // NO codefix available

🙁 Actual behavior

Only the enum members with less than 3 spaces in the correct name have a suggestion to correct spelling (TS2551). The ones with 3 or more spaces don't (TS2339).

It seems to depend on the metric used to compare the strings. If we add a bunch of characters, the suggestions become even weirder:

enum NamesWithSpaces {
	"This Has Three Spaces_________________________________________________________",
	"And This Has Four Spaces_________________________________________________________",
}

const test3 = NamesWithSpaces.ThisHasThreeSpaces_________________________________________________________;
// ^ NO codefix available
const test4 = NamesWithSpaces.AndThisHasFourSpaces_________________________________________________________;
// Suggests "This Has Three Spaces_________________________________________________________"

The absolute (not normalized to string length) Damerau–Levenshtein distance (computed using https://github.com/el3um4s/Damerau-Levenshtein) between the correct replacements are 3 or 4 respectively (adding 3 or 4 spaces). The distance to the incorrect (but suggested) replacement is 10.

🙂 Expected behavior

Every one of those enum members should have a codefix available to correct spelling (by adding the spaces).

[RyanCavanaugh]

We experimented with several different heuristics when making this codefix, trying to balance performance as well as not having too many false positives/negatives. Here's the comment

     * Given a name and a list of names that are *not* equal to the name, return a spelling suggestion if there is one that is close enough.
     * Names less than length 3 only check for case-insensitive equality.
     *
     * find the candidate with the smallest Levenshtein distance,
     *    except for candidates:
     *      * With no name
     *      * Whose length differs from the target name by more than 0.34 of the length of the name.
     *      * Whose levenshtein distance is more than 0.4 of the length of the name
     *        (0.4 allows 1 substitution/transposition for every 5 characters,
     *         and 1 insertion/deletion at 3 characters)

If there's actually a bug in our diff algorithm we can take a PR but I don't think the current behavior violates any invariant.

[AlCalzone]

@RyanCavanaugh Thanks for digging out the criteria. I tried to calculate them by hand, results below:

Name A	Length A	Candidate B	Length B	Length difference normalized	Levenshtein	Levenshtein normalized
OneSpace	8	One Space	9	0,125 ✔	1	0,125 ✔
HasTwoSpaces	12	Has Two Spaces	14	0,167 ✔	2	0,167 ✔
ThisHasThreeSpaces	18	This Has Three Spaces	21	0,167 ✔	3	0,167 ✔
AndThisHasFourSpaces	20	And This Has Four Spaces	24	0,200 ✔	4	0,200 ✔
ThisHasThreeSpaces_________________________________________________________	75	This Has Three Spaces_________________________________________________________	78	0,040 ✔	3	0,040 ✔
AndThisHasFourSpaces_________________________________________________________	77	This Has Three Spaces_________________________________________________________	78	0,013 ✔	10	0,130 ✔
ThisHasThreeSpaces_________________________________________________________	75	And This Has Four Spaces_________________________________________________________	81	0,080 ✔	11	0,147 ✔
AndThisHasFourSpaces_________________________________________________________	77	And This Has Four Spaces_________________________________________________________	81	0,052 ✔	3	0,039 ✔

Based on the criteria you listed, all the above are actually valid candidates, so all of the typos should have a replacement suggested. And for the example where the wrong replacement is suggested, the one I would consider to be correct actually has the smallest levenshtein distance, so it should be the suggested one.

[AlCalzone]

Ok, upon further investigation I am inclined to believe this Math.min was supposed to be a Math.max:

TypeScript/src/compiler/core.ts

Line 2067 in 44d6b51

const maximumLengthDifference = Math.min(2, Math.floor(name.length * 0.34));

The way it is now, 1-2 length names allow a maximum length difference of 0, 3...5 character names allow a maximum length difference of 1 and all others a difference of 2. Combined with the additional check that the length is 3 or higher, this effectively disables the critera length difference < 0.34*length.

In a quick test where I changed the .min to a .max, all suggestions above started working as expected.

[AlCalzone]

PR is up

[fatcerberus]

A classic bug. 😄 Math.min/.max are great for clamping, but it's so easy to get them mixed up...

[RyanCavanaugh]

Great find, thanks for investigating.

If I had a nickel for every time I tried to use max to make an "at most", I'd have at least... no at most... no at least twenty nickels