From d25baf1c9771a0693bb86aafa130030b8b4e9862 Mon Sep 17 00:00:00 2001 From: Anders Hejlsberg Date: Fri, 30 Nov 2018 16:28:10 -0800 Subject: [PATCH 1/3] Handle destructuring in control flow reference matching --- src/compiler/checker.ts | 37 ++++++++++++++++++++++++------------- 1 file changed, 24 insertions(+), 13 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index b0c22b613411e..372a1ac5f2c56 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -14615,15 +14615,15 @@ namespace ts { getAccessedPropertyName(source as PropertyAccessExpression | ElementAccessExpression) === getAccessedPropertyName(target) && isMatchingReference((source as PropertyAccessExpression | ElementAccessExpression).expression, target.expression); case SyntaxKind.BindingElement: - if (target.kind !== SyntaxKind.PropertyAccessExpression) return false; - const t = target as PropertyAccessExpression; - if (t.name.escapedText !== getBindingElementNameText(source as BindingElement)) return false; - if (source.parent.parent.kind === SyntaxKind.BindingElement && isMatchingReference(source.parent.parent, t.expression)) { - return true; - } - if (source.parent.parent.kind === SyntaxKind.VariableDeclaration) { - const maybeId = (source.parent.parent as VariableDeclaration).initializer; - return !!maybeId && isMatchingReference(maybeId, t.expression); + if (target.kind === SyntaxKind.PropertyAccessExpression && (target).name.escapedText === getBindingElementNameText(source)) { + const ancestor = source.parent.parent; + if (ancestor.kind === SyntaxKind.BindingElement) { + return isMatchingReference(ancestor, (target).expression); + } + if (ancestor.kind === SyntaxKind.VariableDeclaration) { + const initializer = (ancestor).initializer; + return !!initializer && isMatchingReference(initializer, (target).expression); + } } } return false; @@ -14635,14 +14635,25 @@ namespace ts { undefined; } + function getReferenceParent(source: Node) { + if (source.kind === SyntaxKind.PropertyAccessExpression) { + return (source).expression; + } + if (source.kind === SyntaxKind.BindingElement) { + const ancestor = source.parent.parent; + return ancestor.kind === SyntaxKind.VariableDeclaration ? (ancestor).initializer : ancestor; + } + return undefined; + } + function containsMatchingReference(source: Node, target: Node) { - while (source.kind === SyntaxKind.PropertyAccessExpression) { - source = (source).expression; - if (isMatchingReference(source, target)) { + let parent = getReferenceParent(source); + while (parent) { + if (isMatchingReference(parent, target)) { return true; } + parent = getReferenceParent(parent); } - return false; } // Return true if target is a property access xxx.yyy, source is a property access xxx.zzz, the declared From 290eff9722a2f8a0f267c30a3b3e78a16489b985 Mon Sep 17 00:00:00 2001 From: Anders Hejlsberg Date: Fri, 30 Nov 2018 16:28:21 -0800 Subject: [PATCH 2/3] Add regression test --- .../compiler/controlFlowDestructuringLoop.ts | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) create mode 100644 tests/cases/compiler/controlFlowDestructuringLoop.ts diff --git a/tests/cases/compiler/controlFlowDestructuringLoop.ts b/tests/cases/compiler/controlFlowDestructuringLoop.ts new file mode 100644 index 0000000000000..bf3a68cb2d8d2 --- /dev/null +++ b/tests/cases/compiler/controlFlowDestructuringLoop.ts @@ -0,0 +1,24 @@ +// @strict: true + +// Repro from #28758 + +interface NumVal { val: number; } +interface StrVal { val: string; } +type Val = NumVal | StrVal; + +function isNumVal(x: Val): x is NumVal { + return typeof x.val === 'number'; +} + +function foo(things: Val[]): void { + for (const thing of things) { + if (isNumVal(thing)) { + const { val } = thing; + val.toFixed(2); + } + else { + const { val } = thing; + val.length; + } + } +} \ No newline at end of file From e96824377bf7b1a7b6012567d2ba900aa799f6fb Mon Sep 17 00:00:00 2001 From: Anders Hejlsberg Date: Fri, 30 Nov 2018 16:28:29 -0800 Subject: [PATCH 3/3] Accept new baselines --- .../reference/controlFlowDestructuringLoop.js | 43 +++++++++++++ .../controlFlowDestructuringLoop.symbols | 63 +++++++++++++++++++ .../controlFlowDestructuringLoop.types | 61 ++++++++++++++++++ 3 files changed, 167 insertions(+) create mode 100644 tests/baselines/reference/controlFlowDestructuringLoop.js create mode 100644 tests/baselines/reference/controlFlowDestructuringLoop.symbols create mode 100644 tests/baselines/reference/controlFlowDestructuringLoop.types diff --git a/tests/baselines/reference/controlFlowDestructuringLoop.js b/tests/baselines/reference/controlFlowDestructuringLoop.js new file mode 100644 index 0000000000000..d7ff51b825978 --- /dev/null +++ b/tests/baselines/reference/controlFlowDestructuringLoop.js @@ -0,0 +1,43 @@ +//// [controlFlowDestructuringLoop.ts] +// Repro from #28758 + +interface NumVal { val: number; } +interface StrVal { val: string; } +type Val = NumVal | StrVal; + +function isNumVal(x: Val): x is NumVal { + return typeof x.val === 'number'; +} + +function foo(things: Val[]): void { + for (const thing of things) { + if (isNumVal(thing)) { + const { val } = thing; + val.toFixed(2); + } + else { + const { val } = thing; + val.length; + } + } +} + +//// [controlFlowDestructuringLoop.js] +"use strict"; +// Repro from #28758 +function isNumVal(x) { + return typeof x.val === 'number'; +} +function foo(things) { + for (var _i = 0, things_1 = things; _i < things_1.length; _i++) { + var thing = things_1[_i]; + if (isNumVal(thing)) { + var val = thing.val; + val.toFixed(2); + } + else { + var val = thing.val; + val.length; + } + } +} diff --git a/tests/baselines/reference/controlFlowDestructuringLoop.symbols b/tests/baselines/reference/controlFlowDestructuringLoop.symbols new file mode 100644 index 0000000000000..46b7ecd58428b --- /dev/null +++ b/tests/baselines/reference/controlFlowDestructuringLoop.symbols @@ -0,0 +1,63 @@ +=== tests/cases/compiler/controlFlowDestructuringLoop.ts === +// Repro from #28758 + +interface NumVal { val: number; } +>NumVal : Symbol(NumVal, Decl(controlFlowDestructuringLoop.ts, 0, 0)) +>val : Symbol(NumVal.val, Decl(controlFlowDestructuringLoop.ts, 2, 18)) + +interface StrVal { val: string; } +>StrVal : Symbol(StrVal, Decl(controlFlowDestructuringLoop.ts, 2, 33)) +>val : Symbol(StrVal.val, Decl(controlFlowDestructuringLoop.ts, 3, 18)) + +type Val = NumVal | StrVal; +>Val : Symbol(Val, Decl(controlFlowDestructuringLoop.ts, 3, 33)) +>NumVal : Symbol(NumVal, Decl(controlFlowDestructuringLoop.ts, 0, 0)) +>StrVal : Symbol(StrVal, Decl(controlFlowDestructuringLoop.ts, 2, 33)) + +function isNumVal(x: Val): x is NumVal { +>isNumVal : Symbol(isNumVal, Decl(controlFlowDestructuringLoop.ts, 4, 27)) +>x : Symbol(x, Decl(controlFlowDestructuringLoop.ts, 6, 18)) +>Val : Symbol(Val, Decl(controlFlowDestructuringLoop.ts, 3, 33)) +>x : Symbol(x, Decl(controlFlowDestructuringLoop.ts, 6, 18)) +>NumVal : Symbol(NumVal, Decl(controlFlowDestructuringLoop.ts, 0, 0)) + + return typeof x.val === 'number'; +>x.val : Symbol(val, Decl(controlFlowDestructuringLoop.ts, 2, 18), Decl(controlFlowDestructuringLoop.ts, 3, 18)) +>x : Symbol(x, Decl(controlFlowDestructuringLoop.ts, 6, 18)) +>val : Symbol(val, Decl(controlFlowDestructuringLoop.ts, 2, 18), Decl(controlFlowDestructuringLoop.ts, 3, 18)) +} + +function foo(things: Val[]): void { +>foo : Symbol(foo, Decl(controlFlowDestructuringLoop.ts, 8, 1)) +>things : Symbol(things, Decl(controlFlowDestructuringLoop.ts, 10, 13)) +>Val : Symbol(Val, Decl(controlFlowDestructuringLoop.ts, 3, 33)) + + for (const thing of things) { +>thing : Symbol(thing, Decl(controlFlowDestructuringLoop.ts, 11, 14)) +>things : Symbol(things, Decl(controlFlowDestructuringLoop.ts, 10, 13)) + + if (isNumVal(thing)) { +>isNumVal : Symbol(isNumVal, Decl(controlFlowDestructuringLoop.ts, 4, 27)) +>thing : Symbol(thing, Decl(controlFlowDestructuringLoop.ts, 11, 14)) + + const { val } = thing; +>val : Symbol(val, Decl(controlFlowDestructuringLoop.ts, 13, 19)) +>thing : Symbol(thing, Decl(controlFlowDestructuringLoop.ts, 11, 14)) + + val.toFixed(2); +>val.toFixed : Symbol(Number.toFixed, Decl(lib.es5.d.ts, --, --)) +>val : Symbol(val, Decl(controlFlowDestructuringLoop.ts, 13, 19)) +>toFixed : Symbol(Number.toFixed, Decl(lib.es5.d.ts, --, --)) + } + else { + const { val } = thing; +>val : Symbol(val, Decl(controlFlowDestructuringLoop.ts, 17, 19)) +>thing : Symbol(thing, Decl(controlFlowDestructuringLoop.ts, 11, 14)) + + val.length; +>val.length : Symbol(String.length, Decl(lib.es5.d.ts, --, --)) +>val : Symbol(val, Decl(controlFlowDestructuringLoop.ts, 17, 19)) +>length : Symbol(String.length, Decl(lib.es5.d.ts, --, --)) + } + } +} diff --git a/tests/baselines/reference/controlFlowDestructuringLoop.types b/tests/baselines/reference/controlFlowDestructuringLoop.types new file mode 100644 index 0000000000000..a4735399f9e4a --- /dev/null +++ b/tests/baselines/reference/controlFlowDestructuringLoop.types @@ -0,0 +1,61 @@ +=== tests/cases/compiler/controlFlowDestructuringLoop.ts === +// Repro from #28758 + +interface NumVal { val: number; } +>val : number + +interface StrVal { val: string; } +>val : string + +type Val = NumVal | StrVal; +>Val : Val + +function isNumVal(x: Val): x is NumVal { +>isNumVal : (x: Val) => x is NumVal +>x : Val + + return typeof x.val === 'number'; +>typeof x.val === 'number' : boolean +>typeof x.val : "string" | "number" | "bigint" | "boolean" | "symbol" | "undefined" | "object" | "function" +>x.val : string | number +>x : Val +>val : string | number +>'number' : "number" +} + +function foo(things: Val[]): void { +>foo : (things: Val[]) => void +>things : Val[] + + for (const thing of things) { +>thing : Val +>things : Val[] + + if (isNumVal(thing)) { +>isNumVal(thing) : boolean +>isNumVal : (x: Val) => x is NumVal +>thing : Val + + const { val } = thing; +>val : number +>thing : NumVal + + val.toFixed(2); +>val.toFixed(2) : string +>val.toFixed : (fractionDigits?: number | undefined) => string +>val : number +>toFixed : (fractionDigits?: number | undefined) => string +>2 : 2 + } + else { + const { val } = thing; +>val : string +>thing : StrVal + + val.length; +>val.length : number +>val : string +>length : number + } + } +}