From 193b0fbe32cd3db7bfda3bda3c1ff82ba5fb667b Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Wed, 30 May 2018 16:29:17 -0700 Subject: [PATCH 1/5] Fix duplicate errors in js special assignments --- src/compiler/checker.ts | 11 ++++--- ...checkSpecialPropertyAssignments.errors.txt | 19 ++++++++++++ .../checkSpecialPropertyAssignments.symbols | 26 ++++++++++++++++ .../checkSpecialPropertyAssignments.types | 31 +++++++++++++++++++ .../salsa/checkSpecialPropertyAssignments.ts | 14 +++++++++ 5 files changed, 97 insertions(+), 4 deletions(-) create mode 100644 tests/baselines/reference/checkSpecialPropertyAssignments.errors.txt create mode 100644 tests/baselines/reference/checkSpecialPropertyAssignments.symbols create mode 100644 tests/baselines/reference/checkSpecialPropertyAssignments.types create mode 100644 tests/cases/conformance/salsa/checkSpecialPropertyAssignments.ts diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index dc40e686eebc1..0fac3d750110f 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -598,6 +598,7 @@ namespace ts { SkipContextSensitive = 1, // Skip context sensitive function expressions Inferential = 2, // Inferential typing Contextual = 3, // Normal type checking informed by a contextual type, therefore not cacheable + NoDeferredCheck = 4, // Don't add any nodes for deferred checking } const enum CallbackCheck { @@ -4627,7 +4628,7 @@ namespace ts { // function/class/{} assignments are fresh declarations, not property assignments, so only add prototype assignments const specialDeclaration = getAssignedJavascriptInitializer(symbol.valueDeclaration); if (specialDeclaration) { - return getWidenedLiteralType(checkExpressionCached(specialDeclaration)); + return getWidenedLiteralType(checkExpressionCached(specialDeclaration, CheckMode.NoDeferredCheck)); } const types: Type[] = []; let constructorTypes: Type[] | undefined; @@ -20878,7 +20879,7 @@ namespace ts { case SyntaxKind.ParenthesizedExpression: return checkParenthesizedExpression(node, checkMode); case SyntaxKind.ClassExpression: - return checkClassExpression(node); + return checkClassExpression(node, checkMode); case SyntaxKind.FunctionExpression: case SyntaxKind.ArrowFunction: return checkFunctionExpressionOrObjectLiteralMethod(node, checkMode); @@ -24298,9 +24299,11 @@ namespace ts { return true; } - function checkClassExpression(node: ClassExpression): Type { + function checkClassExpression(node: ClassExpression, checkMode: CheckMode | undefined): Type { checkClassLikeDeclaration(node); - checkNodeDeferred(node); + if (checkMode !== CheckMode.NoDeferredCheck) { + checkNodeDeferred(node); + } return getTypeOfSymbol(getSymbolOfNode(node)); } diff --git a/tests/baselines/reference/checkSpecialPropertyAssignments.errors.txt b/tests/baselines/reference/checkSpecialPropertyAssignments.errors.txt new file mode 100644 index 0000000000000..6e4e3f3830503 --- /dev/null +++ b/tests/baselines/reference/checkSpecialPropertyAssignments.errors.txt @@ -0,0 +1,19 @@ +tests/cases/conformance/salsa/bug24252.js(8,9): error TS2322: Type 'string[]' is not assignable to type 'number[]'. + Type 'string' is not assignable to type 'number'. + + +==== tests/cases/conformance/salsa/bug24252.js (1 errors) ==== + var A = {}; + A.B = class { + m() { + /** @type {string[]} */ + var x = []; + /** @type {number[]} */ + var y; + y = x; + ~ +!!! error TS2322: Type 'string[]' is not assignable to type 'number[]'. +!!! error TS2322: Type 'string' is not assignable to type 'number'. + } + }; + \ No newline at end of file diff --git a/tests/baselines/reference/checkSpecialPropertyAssignments.symbols b/tests/baselines/reference/checkSpecialPropertyAssignments.symbols new file mode 100644 index 0000000000000..f15009746edf1 --- /dev/null +++ b/tests/baselines/reference/checkSpecialPropertyAssignments.symbols @@ -0,0 +1,26 @@ +=== tests/cases/conformance/salsa/bug24252.js === +var A = {}; +>A : Symbol(A, Decl(bug24252.js, 0, 3), Decl(bug24252.js, 0, 11)) + +A.B = class { +>A.B : Symbol(B, Decl(bug24252.js, 0, 11)) +>A : Symbol(A, Decl(bug24252.js, 0, 3), Decl(bug24252.js, 0, 11)) +>B : Symbol(B, Decl(bug24252.js, 0, 11)) + + m() { +>m : Symbol(B.m, Decl(bug24252.js, 1, 13)) + + /** @type {string[]} */ + var x = []; +>x : Symbol(x, Decl(bug24252.js, 4, 11)) + + /** @type {number[]} */ + var y; +>y : Symbol(y, Decl(bug24252.js, 6, 11)) + + y = x; +>y : Symbol(y, Decl(bug24252.js, 6, 11)) +>x : Symbol(x, Decl(bug24252.js, 4, 11)) + } +}; + diff --git a/tests/baselines/reference/checkSpecialPropertyAssignments.types b/tests/baselines/reference/checkSpecialPropertyAssignments.types new file mode 100644 index 0000000000000..b87ed53553278 --- /dev/null +++ b/tests/baselines/reference/checkSpecialPropertyAssignments.types @@ -0,0 +1,31 @@ +=== tests/cases/conformance/salsa/bug24252.js === +var A = {}; +>A : { [x: string]: any; B: typeof B; } +>{} : { [x: string]: any; B: typeof B; } + +A.B = class { +>A.B = class { m() { /** @type {string[]} */ var x = []; /** @type {number[]} */ var y; y = x; }} : typeof B +>A.B : typeof B +>A : { [x: string]: any; B: typeof B; } +>B : typeof B +>class { m() { /** @type {string[]} */ var x = []; /** @type {number[]} */ var y; y = x; }} : typeof B + + m() { +>m : () => void + + /** @type {string[]} */ + var x = []; +>x : string[] +>[] : undefined[] + + /** @type {number[]} */ + var y; +>y : number[] + + y = x; +>y = x : string[] +>y : number[] +>x : string[] + } +}; + diff --git a/tests/cases/conformance/salsa/checkSpecialPropertyAssignments.ts b/tests/cases/conformance/salsa/checkSpecialPropertyAssignments.ts new file mode 100644 index 0000000000000..31ba8fbd9becf --- /dev/null +++ b/tests/cases/conformance/salsa/checkSpecialPropertyAssignments.ts @@ -0,0 +1,14 @@ +// @noEmit: true +// @checkJs: true +// @allowJs: true +// @Filename: bug24252.js +var A = {}; +A.B = class { + m() { + /** @type {string[]} */ + var x = []; + /** @type {number[]} */ + var y; + y = x; + } +}; From 0b626eb7718aa8376a4527c74d4c27bc851a3cb5 Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Thu, 31 May 2018 08:36:27 -0700 Subject: [PATCH 2/5] Simplify checkExpressionCached call to checkExpression --- src/compiler/checker.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 0fac3d750110f..2f4ef683d7991 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -4628,7 +4628,7 @@ namespace ts { // function/class/{} assignments are fresh declarations, not property assignments, so only add prototype assignments const specialDeclaration = getAssignedJavascriptInitializer(symbol.valueDeclaration); if (specialDeclaration) { - return getWidenedLiteralType(checkExpressionCached(specialDeclaration, CheckMode.NoDeferredCheck)); + return getWidenedLiteralType(checkExpression(specialDeclaration, CheckMode.NoDeferredCheck)); } const types: Type[] = []; let constructorTypes: Type[] | undefined; From dd45a4754884f57f457cd79851af407c7ad14755 Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Tue, 26 Jun 2018 10:02:38 -0700 Subject: [PATCH 3/5] Accept baselines after merge --- .../reference/checkSpecialPropertyAssignments.symbols | 4 ++-- .../reference/checkSpecialPropertyAssignments.types | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/baselines/reference/checkSpecialPropertyAssignments.symbols b/tests/baselines/reference/checkSpecialPropertyAssignments.symbols index f15009746edf1..8de3cd208c325 100644 --- a/tests/baselines/reference/checkSpecialPropertyAssignments.symbols +++ b/tests/baselines/reference/checkSpecialPropertyAssignments.symbols @@ -3,9 +3,9 @@ var A = {}; >A : Symbol(A, Decl(bug24252.js, 0, 3), Decl(bug24252.js, 0, 11)) A.B = class { ->A.B : Symbol(B, Decl(bug24252.js, 0, 11)) +>A.B : Symbol(A.B, Decl(bug24252.js, 0, 11)) >A : Symbol(A, Decl(bug24252.js, 0, 3), Decl(bug24252.js, 0, 11)) ->B : Symbol(B, Decl(bug24252.js, 0, 11)) +>B : Symbol(A.B, Decl(bug24252.js, 0, 11)) m() { >m : Symbol(B.m, Decl(bug24252.js, 1, 13)) diff --git a/tests/baselines/reference/checkSpecialPropertyAssignments.types b/tests/baselines/reference/checkSpecialPropertyAssignments.types index b87ed53553278..879038b6f9471 100644 --- a/tests/baselines/reference/checkSpecialPropertyAssignments.types +++ b/tests/baselines/reference/checkSpecialPropertyAssignments.types @@ -1,12 +1,12 @@ === tests/cases/conformance/salsa/bug24252.js === var A = {}; ->A : { [x: string]: any; B: typeof B; } ->{} : { [x: string]: any; B: typeof B; } +>A : typeof A +>{} : { [x: string]: any; } A.B = class { >A.B = class { m() { /** @type {string[]} */ var x = []; /** @type {number[]} */ var y; y = x; }} : typeof B >A.B : typeof B ->A : { [x: string]: any; B: typeof B; } +>A : typeof A >B : typeof B >class { m() { /** @type {string[]} */ var x = []; /** @type {number[]} */ var y; y = x; }} : typeof B From 1c6eb7a03a3c74ef6a707d6659b945d3db64e7df Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Tue, 26 Jun 2018 10:56:43 -0700 Subject: [PATCH 4/5] Use Map for deferredNodes and improve NoDeferredCheck comment I added an assert when a duplicate was added, but it caused 18 failures in our test suite. --- src/compiler/checker.ts | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index bcb3ae8679ff3..10ec74d833bf8 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -465,7 +465,7 @@ namespace ts { let deferredGlobalImportMetaType: ObjectType; let deferredGlobalExtractSymbol: Symbol; - let deferredNodes: Node[] | undefined; + let deferredNodes: Map | undefined; const allPotentiallyUnusedIdentifiers = createMap(); // key is file name let flowLoopStart = 0; @@ -613,7 +613,7 @@ namespace ts { SkipContextSensitive = 1, // Skip context sensitive function expressions Inferential = 2, // Inferential typing Contextual = 3, // Normal type checking informed by a contextual type, therefore not cacheable - NoDeferredCheck = 4, // Don't add any nodes for deferred checking + NoDeferredCheck = 4, // Don't add any nodes for deferred checking (currently only used for class expressions) } const enum CallbackCheck { @@ -26073,12 +26073,13 @@ namespace ts { // Delaying the type check of the body ensures foo has been assigned a type. function checkNodeDeferred(node: Node) { if (deferredNodes) { - deferredNodes.push(node); + const id = "" + getNodeId(node); + deferredNodes.set(id, node); } } function checkDeferredNodes() { - for (const node of deferredNodes!) { + deferredNodes!.forEach(node => { switch (node.kind) { case SyntaxKind.FunctionExpression: case SyntaxKind.ArrowFunction: @@ -26094,7 +26095,7 @@ namespace ts { checkClassExpressionDeferred(node); break; } - } + }); } function checkSourceFile(node: SourceFile) { @@ -26136,7 +26137,7 @@ namespace ts { clear(potentialThisCollisions); clear(potentialNewTargetCollisions); - deferredNodes = []; + deferredNodes = createMap(); forEach(node.statements, checkSourceElement); checkDeferredNodes(); From 3712a2c6884075a543120640fd503f473385f3a7 Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Tue, 26 Jun 2018 11:09:44 -0700 Subject: [PATCH 5/5] Remove NoDeferredCheck --- src/compiler/checker.ts | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 10ec74d833bf8..166220f90081e 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -613,7 +613,6 @@ namespace ts { SkipContextSensitive = 1, // Skip context sensitive function expressions Inferential = 2, // Inferential typing Contextual = 3, // Normal type checking informed by a contextual type, therefore not cacheable - NoDeferredCheck = 4, // Don't add any nodes for deferred checking (currently only used for class expressions) } const enum CallbackCheck { @@ -4655,7 +4654,7 @@ namespace ts { // function/class/{} assignments are fresh declarations, not property assignments, so only add prototype assignments const specialDeclaration = getAssignedJavascriptInitializer(symbol.valueDeclaration); if (specialDeclaration) { - return getWidenedLiteralType(checkExpression(specialDeclaration, CheckMode.NoDeferredCheck)); + return getWidenedLiteralType(checkExpressionCached(specialDeclaration)); } const types: Type[] = []; let constructorTypes: Type[] | undefined; @@ -21252,7 +21251,7 @@ namespace ts { case SyntaxKind.ParenthesizedExpression: return checkParenthesizedExpression(node, checkMode); case SyntaxKind.ClassExpression: - return checkClassExpression(node, checkMode); + return checkClassExpression(node); case SyntaxKind.FunctionExpression: case SyntaxKind.ArrowFunction: return checkFunctionExpressionOrObjectLiteralMethod(node, checkMode); @@ -24703,11 +24702,9 @@ namespace ts { return true; } - function checkClassExpression(node: ClassExpression, checkMode: CheckMode | undefined): Type { + function checkClassExpression(node: ClassExpression): Type { checkClassLikeDeclaration(node); - if (checkMode !== CheckMode.NoDeferredCheck) { - checkNodeDeferred(node); - } + checkNodeDeferred(node); return getTypeOfSymbol(getSymbolOfNode(node)); }