Skip to content

Commit 141de3f

Browse files
committed
Always issue cannot find name did-you-mean error
This PR issues "cannot find ${name}, did you mean ${name}" errors for identifiers and propery access expressions in JS files *without* `// @ts-check` and without `// @ts-nocheck`. This brings some benefits of Typescript's binder to all Javascript users, even those who haven't opted into Typescript checking. ```js export var inModule = 1 inmodule.toFixed() // errors on exports function f() { var locals = 2 locale.toFixed() // errors on locals } var object = { spaaace: 3 } object.spaaaace // error on read object.spaace = 2 // error on write object.fresh = 12 // OK, no spelling correction to offer ``` To disable the errors, add `// @ts-nocheck` to the file. To get the normal checkJs experience, add `// @ts-check`. == Why This Works == In a word: precision. This change has low recall — it misses lots of correct errors that would be nice to show — but it has high precision: almost all the errors it shows are correct. And they come with a suggested correction. Here are the ingredients: 1. For unchecked JS files, the compiler suppresses all errors except two did-you-mean name resolution errors. 2. Did-you-mean spelling correction is already tuned for high precision/low recall, and doesn't show many bogus errors even in JS. 3. For identifiers, the error is suppressed for suggestions from global files. These are often DOM feature detection, for example. 4. For property accesses, the error is suppressed for suggestions from other files, for the same reason. 5. For property accesses, the error is suppressed for `this` property accesses because the compiler doesn't understand JS constructor functions well enough. In particular, it doesn't understand any inheritance patterns. == Work Remaining == 1. Code cleanup. 2. Fix a couple of failures in existing tests. 3. Suppress errors on property access suggestions from large objects. 4. Combine (3) and (4) above to suppress errors on suggestions from other, global files. 5. A little more testing on random files to make sure that precision is good there too. 6. Have people from the regular Code editor meeting test the code and suggest ideas.
1 parent c4c6a83 commit 141de3f

14 files changed

+1081
-13
lines changed

src/compiler/checker.ts

Lines changed: 31 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2057,10 +2057,22 @@ namespace ts {
20572057
let suggestion: Symbol | undefined;
20582058
if (suggestedNameNotFoundMessage && suggestionCount < maximumSuggestionCount) {
20592059
suggestion = getSuggestedSymbolForNonexistentSymbol(originalLocation, name, meaning);
2060-
const isGlobalScopeAugmentationDeclaration = suggestion && suggestion.valueDeclaration && isAmbientModule(suggestion.valueDeclaration) && isGlobalScopeAugmentation(suggestion.valueDeclaration);
2060+
const isGlobalScopeAugmentationDeclaration = suggestion?.valueDeclaration && isAmbientModule(suggestion.valueDeclaration) && isGlobalScopeAugmentation(suggestion.valueDeclaration);
20612061
if (isGlobalScopeAugmentationDeclaration) {
20622062
suggestion = undefined;
20632063
}
2064+
// (1) when allowjs is on but checkjs is not, for JS[X] files that don't have either ts-check OR ts-nocheck
2065+
// skip suggestions when the suggestion comes from a global scope
2066+
// This will prevent it from always showing up
2067+
const file = getSourceFileOfNode(lastLocation)
2068+
const suggestionFile = getSourceFileOfNode(suggestion?.valueDeclaration)
2069+
if (file && suggestionFile) {
2070+
const isCheckJs = isCheckJsEnabledForFile(file, compilerOptions)
2071+
const isTsNoCheck = !!file.checkJsDirective && file.checkJsDirective.enabled === false
2072+
if (!isCheckJs && !isTsNoCheck && (file.scriptKind === ScriptKind.JS || file.scriptKind === ScriptKind.JSX) && isGlobalSourceFile(suggestionFile) && file !== suggestionFile) {
2073+
suggestion = undefined
2074+
}
2075+
}
20642076
if (suggestion) {
20652077
const suggestionName = symbolToString(suggestion);
20662078
const diagnostic = error(errorLocation, suggestedNameNotFoundMessage, diagnosticName(nameArg!), suggestionName);
@@ -14570,22 +14582,33 @@ namespace ts {
1457014582
* Should all count as literals and not print errors on access or assignment of possibly existing properties.
1457114583
* This mirrors the behavior of the index signature propagation, to which this behaves similarly (but doesn't affect assignability or inference).
1457214584
*/
14573-
function isJSLiteralType(type: Type): boolean {
14585+
function isJSLiteralType(type: Type, reference?: Node): boolean {
1457414586
if (noImplicitAny) {
1457514587
return false; // Flag is meaningless under `noImplicitAny` mode
1457614588
}
14589+
// TODO: Move this check outside isJSLiteralType; it doesn't make sense here
14590+
const file = getSourceFileOfNode(reference)
14591+
if (file) {
14592+
const isCheckJs = isCheckJsEnabledForFile(file, compilerOptions)
14593+
const isTsNoCheck = !!file.checkJsDirective && file.checkJsDirective.enabled === false
14594+
if (!isCheckJs && !isTsNoCheck && (file.scriptKind === ScriptKind.JS || file.scriptKind === ScriptKind.JSX)) {
14595+
const parentFile = forEach(type.symbol?.declarations, getSourceFileOfNode)
14596+
return file !== parentFile || !!reference && isPropertyAccessExpression(reference) && reference.expression.kind === SyntaxKind.ThisKeyword
14597+
}
14598+
}
14599+
1457714600
if (getObjectFlags(type) & ObjectFlags.JSLiteral) {
1457814601
return true;
1457914602
}
1458014603
if (type.flags & TypeFlags.Union) {
14581-
return every((type as UnionType).types, isJSLiteralType);
14604+
return every((type as UnionType).types, t => isJSLiteralType(t, reference));
1458214605
}
1458314606
if (type.flags & TypeFlags.Intersection) {
14584-
return some((type as IntersectionType).types, isJSLiteralType);
14607+
return some((type as IntersectionType).types, t => isJSLiteralType(t, reference));
1458514608
}
1458614609
if (type.flags & TypeFlags.Instantiable) {
1458714610
const constraint = getResolvedBaseConstraint(type);
14588-
return constraint !== type && isJSLiteralType(constraint);
14611+
return constraint !== type && isJSLiteralType(constraint, reference);
1458914612
}
1459014613
return false;
1459114614
}
@@ -14684,7 +14707,7 @@ namespace ts {
1468414707
if (indexType.flags & TypeFlags.Never) {
1468514708
return neverType;
1468614709
}
14687-
if (isJSLiteralType(objectType)) {
14710+
if (isJSLiteralType(objectType, accessNode)) {
1468814711
return anyType;
1468914712
}
1469014713
if (accessExpression && !isConstEnumObjectType(objectType)) {
@@ -14755,7 +14778,7 @@ namespace ts {
1475514778
return undefined;
1475614779
}
1475714780
}
14758-
if (isJSLiteralType(objectType)) {
14781+
if (isJSLiteralType(objectType, accessNode)) {
1475914782
return anyType;
1476014783
}
1476114784
if (accessNode) {
@@ -27419,7 +27442,7 @@ namespace ts {
2741927442
if (!prop) {
2742027443
const indexInfo = !isPrivateIdentifier(right) && (assignmentKind === AssignmentKind.None || !isGenericObjectType(leftType) || isThisTypeParameter(leftType)) ? getIndexInfoOfType(apparentType, IndexKind.String) : undefined;
2742127444
if (!(indexInfo && indexInfo.type)) {
27422-
if (isJSLiteralType(leftType)) {
27445+
if (isJSLiteralType(leftType, node)) {
2742327446
return anyType;
2742427447
}
2742527448
if (leftType.symbol === globalThisSymbol) {

src/compiler/program.ts

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1916,11 +1916,16 @@ namespace ts {
19161916
const isCheckJs = isCheckJsEnabledForFile(sourceFile, options);
19171917
const isTsNoCheck = !!sourceFile.checkJsDirective && sourceFile.checkJsDirective.enabled === false;
19181918
// By default, only type-check .ts, .tsx, 'Deferred' and 'External' files (external files are added by plugins)
1919-
const includeBindAndCheckDiagnostics = !isTsNoCheck && (sourceFile.scriptKind === ScriptKind.TS || sourceFile.scriptKind === ScriptKind.TSX ||
1920-
sourceFile.scriptKind === ScriptKind.External || isCheckJs || sourceFile.scriptKind === ScriptKind.Deferred);
1921-
const bindDiagnostics: readonly Diagnostic[] = includeBindAndCheckDiagnostics ? sourceFile.bindDiagnostics : emptyArray;
1922-
const checkDiagnostics = includeBindAndCheckDiagnostics ? typeChecker.getDiagnostics(sourceFile, cancellationToken) : emptyArray;
1923-
1919+
const includeBindAndCheckDiagnostics = !isTsNoCheck && (sourceFile.scriptKind === ScriptKind.TS || sourceFile.scriptKind === ScriptKind.TSX || sourceFile.scriptKind === ScriptKind.JS || sourceFile.scriptKind === ScriptKind.JSX
1920+
|| sourceFile.scriptKind === ScriptKind.External || sourceFile.scriptKind === ScriptKind.Deferred);
1921+
let bindDiagnostics: readonly Diagnostic[] = includeBindAndCheckDiagnostics ? sourceFile.bindDiagnostics : emptyArray;
1922+
let checkDiagnostics = includeBindAndCheckDiagnostics ? typeChecker.getDiagnostics(sourceFile, cancellationToken) : emptyArray;
1923+
if (!isCheckJs && (sourceFile.scriptKind === ScriptKind.JS || sourceFile.scriptKind === ScriptKind.JSX)) {
1924+
// TODO: It can just be checkDiagnostics srsly
1925+
// TODO: Recheck user tests and random compilation to see how much this adds
1926+
bindDiagnostics = bindDiagnostics.filter(d => d.code === Diagnostics.Cannot_find_name_0_Did_you_mean_1.code || d.code === Diagnostics.Property_0_does_not_exist_on_type_1_Did_you_mean_2.code) // MORE TO COME
1927+
checkDiagnostics = checkDiagnostics.filter(d => d.code === Diagnostics.Cannot_find_name_0_Did_you_mean_1.code || d.code === Diagnostics.Property_0_does_not_exist_on_type_1_Did_you_mean_2.code)
1928+
}
19241929
return getMergedBindAndCheckDiagnostics(sourceFile, includeBindAndCheckDiagnostics, bindDiagnostics, checkDiagnostics, isCheckJs ? sourceFile.jsDocDiagnostics : undefined);
19251930
});
19261931
}
Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
tests/cases/conformance/salsa/other.js(3,1): error TS2552: Cannot find name 'Jon'. Did you mean 'John'?
2+
tests/cases/conformance/salsa/spellingCheckedJS.js(3,1): error TS2552: Cannot find name 'inmodule'. Did you mean 'inModule'?
3+
tests/cases/conformance/salsa/spellingCheckedJS.js(7,5): error TS2552: Cannot find name 'locale'. Did you mean 'locals'?
4+
tests/cases/conformance/salsa/spellingCheckedJS.js(13,21): error TS2551: Property 'none' does not exist on type 'Classe'. Did you mean 'non'?
5+
tests/cases/conformance/salsa/spellingCheckedJS.js(19,22): error TS2339: Property 'none' does not exist on type 'Classe'.
6+
tests/cases/conformance/salsa/spellingCheckedJS.js(31,12): error TS2551: Property 'getGMTDate' does not exist on type 'Date'. Did you mean 'getUTCDate'?
7+
tests/cases/conformance/salsa/spellingCheckedJS.js(34,14): error TS2552: Cannot find name 'setIntegral'. Did you mean 'setInterval'?
8+
tests/cases/conformance/salsa/spellingCheckedJS.js(35,1): error TS2552: Cannot find name 'AudioBuffin'. Did you mean 'AudioBuffer'?
9+
tests/cases/conformance/salsa/spellingCheckedJS.js(37,1): error TS2552: Cannot find name 'Jon'. Did you mean 'John'?
10+
11+
12+
==== tests/cases/conformance/salsa/spellingCheckedJS.js (8 errors) ====
13+
// @ts-check
14+
export var inModule = 1
15+
inmodule.toFixed()
16+
~~~~~~~~
17+
!!! error TS2552: Cannot find name 'inmodule'. Did you mean 'inModule'?
18+
19+
function f() {
20+
var locals = 2
21+
locale.toFixed()
22+
~~~~~~
23+
!!! error TS2552: Cannot find name 'locale'. Did you mean 'locals'?
24+
!!! related TS2728 tests/cases/conformance/salsa/spellingCheckedJS.js:6:9: 'locals' is declared here.
25+
}
26+
class Classe {
27+
non = 'oui'
28+
methode() {
29+
// no error on 'this' references
30+
return this.none
31+
~~~~
32+
!!! error TS2551: Property 'none' does not exist on type 'Classe'. Did you mean 'non'?
33+
!!! related TS2728 tests/cases/conformance/salsa/spellingCheckedJS.js:10:5: 'non' is declared here.
34+
}
35+
}
36+
class Derivee extends Classe {
37+
methode() {
38+
// no error on 'super' references
39+
return super.none
40+
~~~~
41+
!!! error TS2339: Property 'none' does not exist on type 'Classe'.
42+
}
43+
}
44+
45+
46+
var object = {
47+
spaaace: 3
48+
}
49+
object.spaaaace // error on read
50+
object.spaace = 12 // error on write
51+
object.fresh = 12 // OK
52+
other.puuuce // OK, from another file
53+
new Date().getGMTDate() // OK, from another file
54+
~~~~~~~~~~
55+
!!! error TS2551: Property 'getGMTDate' does not exist on type 'Date'. Did you mean 'getUTCDate'?
56+
!!! related TS2728 /.ts/lib.es5.d.ts:757:5: 'getUTCDate' is declared here.
57+
58+
// No suggestions for globals from other files
59+
const atoc = setIntegral(() => console.log('ok'), 500)
60+
~~~~~~~~~~~
61+
!!! error TS2552: Cannot find name 'setIntegral'. Did you mean 'setInterval'?
62+
!!! related TS2728 /.ts/lib.dom.d.ts:19626:18: 'setInterval' is declared here.
63+
AudioBuffin // etc
64+
~~~~~~~~~~~
65+
!!! error TS2552: Cannot find name 'AudioBuffin'. Did you mean 'AudioBuffer'?
66+
!!! related TS2728 /.ts/lib.dom.d.ts:2246:13: 'AudioBuffer' is declared here.
67+
Jimmy
68+
Jon
69+
~~~
70+
!!! error TS2552: Cannot find name 'Jon'. Did you mean 'John'?
71+
!!! related TS2728 tests/cases/conformance/salsa/other.js:2:5: 'John' is declared here.
72+
73+
==== tests/cases/conformance/salsa/other.js (1 errors) ====
74+
var Jimmy = 1
75+
var John = 2
76+
Jon // error, it's from the same file
77+
~~~
78+
!!! error TS2552: Cannot find name 'Jon'. Did you mean 'John'?
79+
!!! related TS2728 tests/cases/conformance/salsa/other.js:2:5: 'John' is declared here.
80+
var other = {
81+
puuce: 4
82+
}
83+
Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
=== tests/cases/conformance/salsa/spellingCheckedJS.js ===
2+
// @ts-check
3+
export var inModule = 1
4+
>inModule : Symbol(inModule, Decl(spellingCheckedJS.js, 1, 10))
5+
6+
inmodule.toFixed()
7+
8+
function f() {
9+
>f : Symbol(f, Decl(spellingCheckedJS.js, 2, 18))
10+
11+
var locals = 2
12+
>locals : Symbol(locals, Decl(spellingCheckedJS.js, 5, 7))
13+
14+
locale.toFixed()
15+
}
16+
class Classe {
17+
>Classe : Symbol(Classe, Decl(spellingCheckedJS.js, 7, 1))
18+
19+
non = 'oui'
20+
>non : Symbol(Classe.non, Decl(spellingCheckedJS.js, 8, 14))
21+
22+
methode() {
23+
>methode : Symbol(Classe.methode, Decl(spellingCheckedJS.js, 9, 15))
24+
25+
// no error on 'this' references
26+
return this.none
27+
>this : Symbol(Classe, Decl(spellingCheckedJS.js, 7, 1))
28+
}
29+
}
30+
class Derivee extends Classe {
31+
>Derivee : Symbol(Derivee, Decl(spellingCheckedJS.js, 14, 1))
32+
>Classe : Symbol(Classe, Decl(spellingCheckedJS.js, 7, 1))
33+
34+
methode() {
35+
>methode : Symbol(Derivee.methode, Decl(spellingCheckedJS.js, 15, 30))
36+
37+
// no error on 'super' references
38+
return super.none
39+
>super : Symbol(Classe, Decl(spellingCheckedJS.js, 7, 1))
40+
}
41+
}
42+
43+
44+
var object = {
45+
>object : Symbol(object, Decl(spellingCheckedJS.js, 23, 3), Decl(spellingCheckedJS.js, 26, 15), Decl(spellingCheckedJS.js, 27, 18))
46+
47+
spaaace: 3
48+
>spaaace : Symbol(spaaace, Decl(spellingCheckedJS.js, 23, 14))
49+
}
50+
object.spaaaace // error on read
51+
>object : Symbol(object, Decl(spellingCheckedJS.js, 23, 3), Decl(spellingCheckedJS.js, 26, 15), Decl(spellingCheckedJS.js, 27, 18))
52+
53+
object.spaace = 12 // error on write
54+
>object : Symbol(object, Decl(spellingCheckedJS.js, 23, 3), Decl(spellingCheckedJS.js, 26, 15), Decl(spellingCheckedJS.js, 27, 18))
55+
56+
object.fresh = 12 // OK
57+
>object : Symbol(object, Decl(spellingCheckedJS.js, 23, 3), Decl(spellingCheckedJS.js, 26, 15), Decl(spellingCheckedJS.js, 27, 18))
58+
59+
other.puuuce // OK, from another file
60+
>other : Symbol(other, Decl(other.js, 3, 3))
61+
62+
new Date().getGMTDate() // OK, from another file
63+
>Date : Symbol(Date, Decl(lib.es5.d.ts, --, --), Decl(lib.es5.d.ts, --, --), Decl(lib.es5.d.ts, --, --), Decl(lib.scripthost.d.ts, --, --))
64+
65+
// No suggestions for globals from other files
66+
const atoc = setIntegral(() => console.log('ok'), 500)
67+
>atoc : Symbol(atoc, Decl(spellingCheckedJS.js, 33, 5))
68+
>console.log : Symbol(Console.log, Decl(lib.dom.d.ts, --, --))
69+
>console : Symbol(console, Decl(lib.dom.d.ts, --, --))
70+
>log : Symbol(Console.log, Decl(lib.dom.d.ts, --, --))
71+
72+
AudioBuffin // etc
73+
Jimmy
74+
>Jimmy : Symbol(Jimmy, Decl(other.js, 0, 3))
75+
76+
Jon
77+
78+
=== tests/cases/conformance/salsa/other.js ===
79+
var Jimmy = 1
80+
>Jimmy : Symbol(Jimmy, Decl(other.js, 0, 3))
81+
82+
var John = 2
83+
>John : Symbol(John, Decl(other.js, 1, 3))
84+
85+
Jon // error, it's from the same file
86+
var other = {
87+
>other : Symbol(other, Decl(other.js, 3, 3))
88+
89+
puuce: 4
90+
>puuce : Symbol(puuce, Decl(other.js, 3, 13))
91+
}
92+

0 commit comments

Comments
 (0)