Spelling correction #15507

sandersn · 2017-05-01T18:48:33Z

Suggest spellings based on Levenshtein distance. This includes a quick fix.

Some tweaks that are possible:

Don't suggest spellings from the global scope. The main value seems to be for the tests trying to do new number(), which probably doesn't happen much in the real world.
Only report 100 suggestions and stop afterwards. If your program has more than 100 unknown symbols, it's probably missing some libraries, not full of typos.
Lower the threshold slightly to 0.6 * source.length from 0.7 * source.length. This would prevent 2-character suggestions entirely (currently they can add one letter) and restrict 3-character suggestions to adding one letter (currently they can also substitute one letter).

Performance seems to be about the same based on timing jake runtests on a 2 Ghz CPU. If I add the 100-error cutoff or stop suggestions from global scope, then I think there won't be any concerns about performance anyway.

Fixes #15333
Fixes #15565

mhegazy

I do not see any upper bounds on how often we do this.

mhegazy · 2017-05-01T19:03:48Z

tests/baselines/reference/constructorWithIncompleteTypeAnnotation.errors.txt

                                  ~
 !!! error TS1005: ',' expected.
                                    ~~~~~~
-!!! error TS2304: Cannot find name 'string'.
+!!! error TS2552: Cannot find name 'string'. Did you mean 'String'?


i though we are not doing the global scope.. where are these coming from?

I suggested some tweaks but haven't implemented them yet.

If your compilation has more than 100 unknown symbols, the problem is missing declarations, not typos.

sandersn · 2017-05-01T20:04:43Z

I added a 100-error cutoff for the spelling suggestions.

sandersn · 2017-05-02T21:17:35Z

@RyanCavanaugh @DanielRosenwasser feel like looking at a whole bunch of spelling corrections?

DanielRosenwasser · 2017-05-02T23:44:24Z

Some of these are so off (e.g. Couldn't find any - did you mean NaN?), it might be adding an extra heuristic here. One I had in mind is that the two have to start with the same lower-cased letter.

RyanCavanaugh · 2017-05-03T04:40:21Z

tests/baselines/reference/exportNonInitializedVariablesES6.errors.txt

@@ -12,7 +12,7 @@ tests/cases/conformance/externalModules/exportNonInitializedVariablesES6.ts(3,6)
    ~~~
 !!! error TS1214: Identifier expected. 'let' is a reserved word in strict mode. Modules are automatically in strict mode.
    ~~~
-!!! error TS2304: Cannot find name 'let'.
+!!! error TS2552: Cannot find name 'let'. Did you mean 'Set'?


Don't issue suggestions in this case

RyanCavanaugh · 2017-05-03T04:42:38Z

tests/baselines/reference/newExpressionWithCast.errors.txt

@@ -1,7 +1,7 @@
 tests/cases/compiler/newExpressionWithCast.ts(3,12): error TS7009: 'new' expression, whose target lacks a construct signature, implicitly has an 'any' type.
 tests/cases/compiler/newExpressionWithCast.ts(7,13): error TS2365: Operator '>' cannot be applied to types 'boolean' and 'void'.
 tests/cases/compiler/newExpressionWithCast.ts(7,17): error TS1109: Expression expected.
-tests/cases/compiler/newExpressionWithCast.ts(7,18): error TS2304: Cannot find name 'any'.
+tests/cases/compiler/newExpressionWithCast.ts(7,18): error TS2552: Cannot find name 'any'. Did you mean 'NaN'?


We already (or should?) have a check for when an identifier exists in the type namespace but not the value namespace - we probably shouldn't issue suggestions there

This happens because checkAndReportErrorForUsingTypeAsValue doesn't have a special case to include primitives. It should though! I'll open a bug for it.

Opened #15565

I decided to fix it here because it'll make the black list smaller; if you legitimately misspell Boolen then I think we should give a suggestion.

Not for NaN though. Nobody needs more NaN in their code.

RyanCavanaugh · 2017-05-03T04:43:22Z

tests/baselines/reference/parser10.1.1-8gs.errors.txt

@@ -1,4 +1,4 @@
-tests/cases/conformance/parser/ecmascript5/parser10.1.1-8gs.ts(16,7): error TS2304: Cannot find name 'NotEarlyError'.
+tests/cases/conformance/parser/ecmascript5/parser10.1.1-8gs.ts(16,7): error TS2552: Cannot find name 'NotEarlyError'. Did you mean 'SyntaxError'?


Way too far

RyanCavanaugh · 2017-05-03T04:43:31Z

tests/baselines/reference/parser15.4.4.14-9-2.errors.txt

@@ -1,6 +1,6 @@
 tests/cases/conformance/parser/ecmascript5/parser15.4.4.14-9-2.ts(16,15): error TS2453: The type argument for type parameter 'T' cannot be inferred from the usage. Consider specifying the type arguments explicitly.
  Type argument candidate 'number' is not a valid type argument because it is not a supertype of candidate 'false'.
-tests/cases/conformance/parser/ecmascript5/parser15.4.4.14-9-2.ts(25,1): error TS2304: Cannot find name 'runTestCase'.
+tests/cases/conformance/parser/ecmascript5/parser15.4.4.14-9-2.ts(25,1): error TS2552: Cannot find name 'runTestCase'. Did you mean 'testcase'?


RyanCavanaugh · 2017-05-03T04:43:47Z

tests/baselines/reference/narrowFromAnyWithInstanceof.errors.txt

-tests/cases/conformance/types/any/narrowFromAnyWithInstanceof.ts(17,7): error TS2339: Property 'mesage' does not exist on type 'Error'.
-tests/cases/conformance/types/any/narrowFromAnyWithInstanceof.ts(22,7): error TS2339: Property 'getHuors' does not exist on type 'Date'.
+tests/cases/conformance/types/any/narrowFromAnyWithInstanceof.ts(17,7): error TS2551: Property 'mesage' does not exist on type 'Error'. Did you mean 'message'?
+tests/cases/conformance/types/any/narrowFromAnyWithInstanceof.ts(22,7): error TS2551: Property 'getHuors' does not exist on type 'Date'. Did you mean 'getHours'?


RyanCavanaugh · 2017-05-03T04:43:59Z

tests/baselines/reference/parserRealSource8.errors.txt

 tests/cases/conformance/parser/ecmascript5/parserRealSource8.ts(314,55): error TS2304: Cannot find name 'DualStringHashTable'.
 tests/cases/conformance/parser/ecmascript5/parserRealSource8.ts(314,96): error TS2304: Cannot find name 'StringHashTable'.
 tests/cases/conformance/parser/ecmascript5/parserRealSource8.ts(315,42): error TS2304: Cannot find name 'StringHashTable'.
-tests/cases/conformance/parser/ecmascript5/parserRealSource8.ts(316,44): error TS2304: Cannot find name 'ScopedMembers'.
+tests/cases/conformance/parser/ecmascript5/parserRealSource8.ts(316,44): error TS2552: Cannot find name 'ScopedMembers'. Did you mean 'funcMembers'?


RyanCavanaugh · 2017-05-03T04:44:08Z

tests/baselines/reference/parserRealSource8.errors.txt

@@ -397,7 +397,7 @@ tests/cases/conformance/parser/ecmascript5/parserRealSource8.ts(454,35): error T
 !!! error TS2304: Cannot find name 'Type'.
            var withSymbol = new WithSymbol(withStmt.minChar, context.typeFlow.checker.locationInfo.unitIndex, withType);
                                 ~~~~~~~~~~
-!!! error TS2304: Cannot find name 'WithSymbol'.
+!!! error TS2552: Cannot find name 'WithSymbol'. Did you mean 'withSymbol'?


I'd only give this one an A because running the code fix would produce var withSymbol = new withSymbol(....). And this differ-only-by-case pattern is common, so maybe only a B+.

RyanCavanaugh · 2017-05-03T04:44:57Z

tests/baselines/reference/parser_breakInIterationOrSwitchStatement4.errors.txt

@@ -1,9 +1,9 @@
-tests/cases/conformance/parser/ecmascript5/Statements/BreakStatements/parser_breakInIterationOrSwitchStatement4.ts(1,15): error TS2304: Cannot find name 'something'.
+tests/cases/conformance/parser/ecmascript5/Statements/BreakStatements/parser_breakInIterationOrSwitchStatement4.ts(1,15): error TS2552: Cannot find name 'something'. Did you mean 'String'?


RyanCavanaugh · 2017-05-03T04:46:25Z

tests/baselines/reference/parserharness.errors.txt

@@ -16,64 +16,64 @@ tests/cases/conformance/parser/ecmascript5/RealWorld/parserharness.ts(691,50): e
 tests/cases/conformance/parser/ecmascript5/RealWorld/parserharness.ts(716,47): error TS2503: Cannot find namespace 'TypeScript'.
 tests/cases/conformance/parser/ecmascript5/RealWorld/parserharness.ts(721,62): error TS2304: Cannot find name 'ITextWriter'.
 tests/cases/conformance/parser/ecmascript5/RealWorld/parserharness.ts(724,29): error TS2304: Cannot find name 'ITextWriter'.
-tests/cases/conformance/parser/ecmascript5/RealWorld/parserharness.ts(754,53): error TS2304: Cannot find name 'TypeScript'.
+tests/cases/conformance/parser/ecmascript5/RealWorld/parserharness.ts(754,53): error TS2552: Cannot find name 'TypeScript'. Did you mean 'TypeScriptLS'?


We should issue 5-10 of these max... there's no way we find 100 spelling errors in a program

RyanCavanaugh · 2017-05-03T04:46:52Z

tests/baselines/reference/propertyOrdering.errors.txt

@@ -17,7 +17,7 @@ tests/cases/compiler/propertyOrdering.ts(20,14): error TS2339: Property '_store'

          public bar() { return this.store; } // should be an error
                                     ~~~~~
-!!! error TS2339: Property 'store' does not exist on type 'Foo'.
+!!! error TS2551: Property 'store' does not exist on type 'Foo'. Did you mean '_store'?


RyanCavanaugh · 2017-05-03T04:47:42Z

tests/baselines/reference/privateIndexer2.errors.txt

@@ -13,7 +13,7 @@ tests/cases/conformance/classes/indexMemberDeclarations/privateIndexer2.ts(4,32)
                  ~
 !!! error TS1005: ']' expected.
                    ~~~~~~
-!!! error TS2304: Cannot find name 'string'.
+!!! error TS2552: Cannot find name 'string'. Did you mean 'String'?


Feels like we need to ban some global identifiers but not others. The boxed primitives, null, and NaN in particular... probably the rest can stay?

RyanCavanaugh · 2017-05-03T04:47:51Z

tests/baselines/reference/propertySignatures.errors.txt

@@ -1,6 +1,6 @@
 tests/cases/compiler/propertySignatures.ts(2,13): error TS2300: Duplicate identifier 'a'.
 tests/cases/compiler/propertySignatures.ts(2,23): error TS2300: Duplicate identifier 'a'.
-tests/cases/compiler/propertySignatures.ts(14,12): error TS2304: Cannot find name 'foo'.
+tests/cases/compiler/propertySignatures.ts(14,12): error TS2552: Cannot find name 'foo'. Did you mean 'foo1'?


RyanCavanaugh · 2017-05-03T04:48:12Z

tests/baselines/reference/scannertest1.errors.txt

                    ? c - CharacterCodes.A + 10
                          ~~~~~~~~~~~~~~
-!!! error TS2304: Cannot find name 'CharacterCodes'.
+!!! error TS2552: Cannot find name 'CharacterCodes'. Did you mean 'CharacterInfo'?


RyanCavanaugh · 2017-05-03T04:52:20Z

Left some subjective comments

My overall impression is a case-insensitive edit distance of 2 (if the string is long enough - use a lower threshold for shorter strings) is probably where we should stop. The value-add here comes when you "can't see" the spelling error (fileName / filename, getColisions / getCollisions, setLength / setLenth, etc) and changes farther than 2 are much more likely to be "seeable".

And update baselines

Previously, you would get the generic message when writing incorrect code like `let y = number`. "Cannot find name 'number'". Now the message says "'number' is a type but is used a value here." Fixes #15565

The maximum distance cutoff was being checked after the close-enough early exit. Now it's checked before. Note that `null` doesn't show up in the globals list, so it's not part of the blacklist either.

It should be all lowercase since candidates have been lowercased by the point the blacklist is checked.

sandersn · 2017-05-03T22:37:59Z

All right, the suggestions are now much higher precision. Want to take another look?

I decided to fix #15565, too, instead of blacklisting String et al outright. Imagine all the people out there haplessly writing Sting and Nubmer.

sandersn · 2017-05-08T15:46:35Z

Now that the spelling suggestions are in order, can I get a review of the implementation?

rbuckton · 2017-05-11T20:12:46Z

src/compiler/utilities.ts

@@ -4641,4 +4641,27 @@ namespace ts {
    export function unescapeIdentifier(identifier: string): string {
        return identifier.length >= 3 && identifier.charCodeAt(0) === CharacterCodes._ && identifier.charCodeAt(1) === CharacterCodes._ && identifier.charCodeAt(2) === CharacterCodes._ ? identifier.substr(1) : identifier;
    }
+
+    export function levenshtein(s1: string, s2: string): number {


Did you intend for levenshtein to be in the public TypeScript API? This file has two top-level ts namespaces, the first marked with /* @internal */ and the second without the marker. This code is in the second namespace currently.

sandersn · 2017-05-11T20:15:10Z

No, I didn't mean for it to be part of the API. I forgot about the public/private halves of the file. I'll move it to the private side.

sandersn added 3 commits May 1, 2017 11:35

Suggest spelling for unknown symbols + properties

2345ecd

Codefix:suggest spellings+update others w/new codes

1bff5b7

Update baselines

2479c07

msftclas added the cla-not-required label May 1, 2017

mhegazy reviewed May 1, 2017

View reviewed changes

sandersn added 2 commits May 1, 2017 12:58

Only 100 spelling corrections for unknown symbols

ca7d7f5

If your compilation has more than 100 unknown symbols, the problem is missing declarations, not typos.

Fix semicolon lint

eb33ba7

sandersn requested a review from RyanCavanaugh May 2, 2017 22:08

RyanCavanaugh reviewed May 3, 2017

View reviewed changes

sandersn added 5 commits May 3, 2017 09:42

Lower allowed levenshtein distance for suggestions

ee1edf0

And update baselines

Reduce max number of spelling suggestions to 10

4877343

Lower allowed length difference for suggestions

9bf5209

And update baselines

Include primitives in type-as-value error message

2a7398b

Previously, you would get the generic message when writing incorrect code like `let y = number`. "Cannot find name 'number'". Now the message says "'number' is a type but is used a value here." Fixes #15565

Blacklist some built-ins and improve max cutoff

5a7e967

The maximum distance cutoff was being checked after the close-enough early exit. Now it's checked before. Note that `null` doesn't show up in the globals list, so it's not part of the blacklist either.

sandersn added 2 commits May 3, 2017 15:28

Fix case of suggestion blacklist.

0c10098

It should be all lowercase since candidates have been lowercased by the point the blacklist is checked.

Merge branch 'master' into spelling-correction

9eaf40b

weswigham mentioned this pull request May 9, 2017

Use damerau-levenshtein algorithm for edit distance #15689

Closed

Merge branch 'master' into spelling-correction

370b561

sandersn merged commit 5143a3c into master May 9, 2017

rbuckton reviewed May 11, 2017

View reviewed changes

sandersn deleted the spelling-correction branch May 11, 2017 20:13

microsoft locked and limited conversation to collaborators Jun 14, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Spelling correction #15507

Spelling correction #15507

sandersn commented May 1, 2017 •

edited

Loading

mhegazy left a comment

mhegazy May 1, 2017

sandersn May 1, 2017

sandersn commented May 1, 2017

sandersn commented May 2, 2017

DanielRosenwasser commented May 2, 2017

RyanCavanaugh May 3, 2017

RyanCavanaugh May 3, 2017

sandersn May 3, 2017

sandersn May 3, 2017 •

edited

Loading

sandersn May 3, 2017

RyanCavanaugh May 3, 2017

RyanCavanaugh May 3, 2017

RyanCavanaugh May 3, 2017

RyanCavanaugh May 3, 2017

RyanCavanaugh May 3, 2017

sandersn May 3, 2017

RyanCavanaugh May 3, 2017

RyanCavanaugh May 3, 2017

sandersn May 3, 2017

RyanCavanaugh May 3, 2017

RyanCavanaugh May 3, 2017

RyanCavanaugh May 3, 2017

RyanCavanaugh May 3, 2017

RyanCavanaugh commented May 3, 2017

sandersn commented May 3, 2017

sandersn commented May 8, 2017

rbuckton May 11, 2017

sandersn commented May 11, 2017

		@@ -1,4 +1,4 @@
		tests/cases/conformance/parser/ecmascript5/parser10.1.1-8gs.ts(16,7): error TS2304: Cannot find name 'NotEarlyError'.
		tests/cases/conformance/parser/ecmascript5/parser10.1.1-8gs.ts(16,7): error TS2552: Cannot find name 'NotEarlyError'. Did you mean 'SyntaxError'?

		@@ -1,9 +1,9 @@
		tests/cases/conformance/parser/ecmascript5/Statements/BreakStatements/parser_breakInIterationOrSwitchStatement4.ts(1,15): error TS2304: Cannot find name 'something'.
		tests/cases/conformance/parser/ecmascript5/Statements/BreakStatements/parser_breakInIterationOrSwitchStatement4.ts(1,15): error TS2552: Cannot find name 'something'. Did you mean 'String'?

Spelling correction #15507

Spelling correction #15507

Conversation

sandersn commented May 1, 2017 • edited Loading

mhegazy left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sandersn commented May 1, 2017

sandersn commented May 2, 2017

DanielRosenwasser commented May 2, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sandersn May 3, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RyanCavanaugh commented May 3, 2017

sandersn commented May 3, 2017

sandersn commented May 8, 2017

Choose a reason for hiding this comment

sandersn commented May 11, 2017

sandersn commented May 1, 2017 •

edited

Loading

sandersn May 3, 2017 •

edited

Loading