From d73fb3acdd33712b7d6267e8fc11173596c3e07a Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Wed, 8 Nov 2017 09:48:36 -0800 Subject: [PATCH 1/3] Narrow property access from string index signatures Previously these accesses did not use control flow to narrow property accesses of undefined properties that are resolved from a string index signature. Now the use control flow to narrow these just like normal properties. --- src/compiler/checker.ts | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 7bce66892dec9..e2a419900c730 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -15221,7 +15221,7 @@ namespace ts { if (indexInfo.isReadonly && (isAssignmentTarget(node) || isDeleteTarget(node))) { error(node, Diagnostics.Index_signature_in_type_0_only_permits_reading, typeToString(apparentType)); } - return indexInfo.type; + return getFlowTypeOfPropertyAccess(node, /*prop*/ undefined, indexInfo.type, getAssignmentTargetKind(node)); } if (right.escapedText && !checkAndReportErrorForExtendingInterface(node)) { reportNonexistentProperty(right, type.flags & TypeFlags.TypeParameter && (type as TypeParameter).isThisType ? apparentType : type); @@ -15246,16 +15246,21 @@ namespace ts { return unknownType; } } + return getFlowTypeOfPropertyAccess(node, prop, propType, assignmentKind); + } - // Only compute control flow type if this is a property access expression that isn't an - // assignment target, and the referenced property was declared as a variable, property, - // accessor, or optional method. - if (node.kind !== SyntaxKind.PropertyAccessExpression || assignmentKind === AssignmentKind.Definite || - !(prop.flags & (SymbolFlags.Variable | SymbolFlags.Property | SymbolFlags.Accessor)) && - !(prop.flags & SymbolFlags.Method && propType.flags & TypeFlags.Union)) { - return propType; + /** + * Only compute control flow type if this is a property access expression that isn't an + * assignment target, and the referenced property was declared as a variable, property, + * accessor, or optional method. + */ + function getFlowTypeOfPropertyAccess(node: PropertyAccessExpression | QualifiedName, prop: Symbol | undefined, type: Type, assignmentKind: AssignmentKind) { + if (node.kind !== SyntaxKind.PropertyAccessExpression || + assignmentKind === AssignmentKind.Definite || + prop && !(prop.flags & (SymbolFlags.Variable | SymbolFlags.Property | SymbolFlags.Accessor)) && !(prop.flags & SymbolFlags.Method && type.flags & TypeFlags.Union)) { + return type; } - const flowType = getFlowTypeOfReference(node, propType); + const flowType = getFlowTypeOfReference(node, type); return assignmentKind ? getBaseTypeOfLiteralType(flowType) : flowType; } From 6c74b81d7eeb67abf3f17cdcc9f1c858916bdd1f Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Wed, 8 Nov 2017 09:50:39 -0800 Subject: [PATCH 2/3] Test:narrow properties from string index signatures --- .../reference/controlFlowStringIndex.js | 13 ++++++++++ .../reference/controlFlowStringIndex.symbols | 18 +++++++++++++ .../reference/controlFlowStringIndex.types | 26 +++++++++++++++++++ .../controlFlow/controlFlowStringIndex.ts | 6 +++++ 4 files changed, 63 insertions(+) create mode 100644 tests/baselines/reference/controlFlowStringIndex.js create mode 100644 tests/baselines/reference/controlFlowStringIndex.symbols create mode 100644 tests/baselines/reference/controlFlowStringIndex.types create mode 100644 tests/cases/conformance/controlFlow/controlFlowStringIndex.ts diff --git a/tests/baselines/reference/controlFlowStringIndex.js b/tests/baselines/reference/controlFlowStringIndex.js new file mode 100644 index 0000000000000..456ae6afcd4c3 --- /dev/null +++ b/tests/baselines/reference/controlFlowStringIndex.js @@ -0,0 +1,13 @@ +//// [controlFlowStringIndex.ts] +type A = { [index: string]: number | null }; +declare const value: A; +if (value.foo !== null) { + value.foo.toExponential() +} + + +//// [controlFlowStringIndex.js] +"use strict"; +if (value.foo !== null) { + value.foo.toExponential(); +} diff --git a/tests/baselines/reference/controlFlowStringIndex.symbols b/tests/baselines/reference/controlFlowStringIndex.symbols new file mode 100644 index 0000000000000..aad08e48a9b58 --- /dev/null +++ b/tests/baselines/reference/controlFlowStringIndex.symbols @@ -0,0 +1,18 @@ +=== tests/cases/conformance/controlFlow/controlFlowStringIndex.ts === +type A = { [index: string]: number | null }; +>A : Symbol(A, Decl(controlFlowStringIndex.ts, 0, 0)) +>index : Symbol(index, Decl(controlFlowStringIndex.ts, 0, 12)) + +declare const value: A; +>value : Symbol(value, Decl(controlFlowStringIndex.ts, 1, 13)) +>A : Symbol(A, Decl(controlFlowStringIndex.ts, 0, 0)) + +if (value.foo !== null) { +>value : Symbol(value, Decl(controlFlowStringIndex.ts, 1, 13)) + + value.foo.toExponential() +>value.foo.toExponential : Symbol(Number.toExponential, Decl(lib.d.ts, --, --)) +>value : Symbol(value, Decl(controlFlowStringIndex.ts, 1, 13)) +>toExponential : Symbol(Number.toExponential, Decl(lib.d.ts, --, --)) +} + diff --git a/tests/baselines/reference/controlFlowStringIndex.types b/tests/baselines/reference/controlFlowStringIndex.types new file mode 100644 index 0000000000000..d1722dd692564 --- /dev/null +++ b/tests/baselines/reference/controlFlowStringIndex.types @@ -0,0 +1,26 @@ +=== tests/cases/conformance/controlFlow/controlFlowStringIndex.ts === +type A = { [index: string]: number | null }; +>A : A +>index : string +>null : null + +declare const value: A; +>value : A +>A : A + +if (value.foo !== null) { +>value.foo !== null : boolean +>value.foo : number | null +>value : A +>foo : number | null +>null : null + + value.foo.toExponential() +>value.foo.toExponential() : string +>value.foo.toExponential : (fractionDigits?: number | undefined) => string +>value.foo : number +>value : A +>foo : number +>toExponential : (fractionDigits?: number | undefined) => string +} + diff --git a/tests/cases/conformance/controlFlow/controlFlowStringIndex.ts b/tests/cases/conformance/controlFlow/controlFlowStringIndex.ts new file mode 100644 index 0000000000000..78acf6d654e2a --- /dev/null +++ b/tests/cases/conformance/controlFlow/controlFlowStringIndex.ts @@ -0,0 +1,6 @@ +// @strict: true +type A = { [index: string]: number | null }; +declare const value: A; +if (value.foo !== null) { + value.foo.toExponential() +} From 2548aced3f0482c579ef8e00f25d8f370974fb3c Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Wed, 8 Nov 2017 10:56:30 -0800 Subject: [PATCH 3/3] Add a couple of test cases --- .../reference/controlFlowStringIndex.js | 9 ++++++- .../reference/controlFlowStringIndex.symbols | 24 +++++++++++++++---- .../reference/controlFlowStringIndex.types | 19 ++++++++++++++- .../controlFlow/controlFlowStringIndex.ts | 7 +++++- 4 files changed, 51 insertions(+), 8 deletions(-) diff --git a/tests/baselines/reference/controlFlowStringIndex.js b/tests/baselines/reference/controlFlowStringIndex.js index 456ae6afcd4c3..db4d4a30ab950 100644 --- a/tests/baselines/reference/controlFlowStringIndex.js +++ b/tests/baselines/reference/controlFlowStringIndex.js @@ -1,8 +1,13 @@ //// [controlFlowStringIndex.ts] -type A = { [index: string]: number | null }; +type A = { + other: number | null; + [index: string]: number | null +}; declare const value: A; if (value.foo !== null) { value.foo.toExponential() + value.other // should still be number | null + value.bar // should still be number | null } @@ -10,4 +15,6 @@ if (value.foo !== null) { "use strict"; if (value.foo !== null) { value.foo.toExponential(); + value.other; // should still be number | null + value.bar; // should still be number | null } diff --git a/tests/baselines/reference/controlFlowStringIndex.symbols b/tests/baselines/reference/controlFlowStringIndex.symbols index aad08e48a9b58..5c7d626d4a79c 100644 --- a/tests/baselines/reference/controlFlowStringIndex.symbols +++ b/tests/baselines/reference/controlFlowStringIndex.symbols @@ -1,18 +1,32 @@ === tests/cases/conformance/controlFlow/controlFlowStringIndex.ts === -type A = { [index: string]: number | null }; +type A = { >A : Symbol(A, Decl(controlFlowStringIndex.ts, 0, 0)) ->index : Symbol(index, Decl(controlFlowStringIndex.ts, 0, 12)) + other: number | null; +>other : Symbol(other, Decl(controlFlowStringIndex.ts, 0, 10)) + + [index: string]: number | null +>index : Symbol(index, Decl(controlFlowStringIndex.ts, 2, 5)) + +}; declare const value: A; ->value : Symbol(value, Decl(controlFlowStringIndex.ts, 1, 13)) +>value : Symbol(value, Decl(controlFlowStringIndex.ts, 4, 13)) >A : Symbol(A, Decl(controlFlowStringIndex.ts, 0, 0)) if (value.foo !== null) { ->value : Symbol(value, Decl(controlFlowStringIndex.ts, 1, 13)) +>value : Symbol(value, Decl(controlFlowStringIndex.ts, 4, 13)) value.foo.toExponential() >value.foo.toExponential : Symbol(Number.toExponential, Decl(lib.d.ts, --, --)) ->value : Symbol(value, Decl(controlFlowStringIndex.ts, 1, 13)) +>value : Symbol(value, Decl(controlFlowStringIndex.ts, 4, 13)) >toExponential : Symbol(Number.toExponential, Decl(lib.d.ts, --, --)) + + value.other // should still be number | null +>value.other : Symbol(other, Decl(controlFlowStringIndex.ts, 0, 10)) +>value : Symbol(value, Decl(controlFlowStringIndex.ts, 4, 13)) +>other : Symbol(other, Decl(controlFlowStringIndex.ts, 0, 10)) + + value.bar // should still be number | null +>value : Symbol(value, Decl(controlFlowStringIndex.ts, 4, 13)) } diff --git a/tests/baselines/reference/controlFlowStringIndex.types b/tests/baselines/reference/controlFlowStringIndex.types index d1722dd692564..8c5f82cbf80d8 100644 --- a/tests/baselines/reference/controlFlowStringIndex.types +++ b/tests/baselines/reference/controlFlowStringIndex.types @@ -1,9 +1,16 @@ === tests/cases/conformance/controlFlow/controlFlowStringIndex.ts === -type A = { [index: string]: number | null }; +type A = { >A : A + + other: number | null; +>other : number | null +>null : null + + [index: string]: number | null >index : string >null : null +}; declare const value: A; >value : A >A : A @@ -22,5 +29,15 @@ if (value.foo !== null) { >value : A >foo : number >toExponential : (fractionDigits?: number | undefined) => string + + value.other // should still be number | null +>value.other : number | null +>value : A +>other : number | null + + value.bar // should still be number | null +>value.bar : number | null +>value : A +>bar : number | null } diff --git a/tests/cases/conformance/controlFlow/controlFlowStringIndex.ts b/tests/cases/conformance/controlFlow/controlFlowStringIndex.ts index 78acf6d654e2a..28b75eaba7000 100644 --- a/tests/cases/conformance/controlFlow/controlFlowStringIndex.ts +++ b/tests/cases/conformance/controlFlow/controlFlowStringIndex.ts @@ -1,6 +1,11 @@ // @strict: true -type A = { [index: string]: number | null }; +type A = { + other: number | null; + [index: string]: number | null +}; declare const value: A; if (value.foo !== null) { value.foo.toExponential() + value.other // should still be number | null + value.bar // should still be number | null }