From bb3edc88b9486f500fbd1ec8b75b1a6815104991 Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Mon, 20 May 2019 11:32:41 -0700 Subject: [PATCH 1/5] Control flow for element access expressions Draft version, just want to see how performance is --- src/compiler/checker.ts | 40 ++++++++++++++++++- .../controlFlow/controlFlowElementAccess2.ts | 13 ++++++ 2 files changed, 52 insertions(+), 1 deletion(-) create mode 100644 tests/cases/conformance/controlFlow/controlFlowElementAccess2.ts diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 18c1f42f838fb..0d540a9dc633e 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -20503,7 +20503,45 @@ namespace ts { AccessFlags.Writing | (isGenericObjectType(objectType) && !isThisTypeParameter(objectType) ? AccessFlags.NoIndexSignatures : 0) : AccessFlags.None; const indexedAccessType = getIndexedAccessTypeOrUndefined(objectType, effectiveIndexType, node, accessFlags) || errorType; - return checkIndexedAccessIndexType(indexedAccessType, node); + const prop = indexedAccessType.symbol; + + // 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. + // TODO: This probably doesn't handle element access expressions + const assignmentKind = getAssignmentTargetKind(node); + if (node.kind !== SyntaxKind.ElementAccessExpression || + assignmentKind === AssignmentKind.Definite || + prop && !(prop.flags & (SymbolFlags.Variable | SymbolFlags.Property | SymbolFlags.Accessor)) && !(prop.flags & SymbolFlags.Method && indexedAccessType.flags & TypeFlags.Union)) { + return checkIndexedAccessIndexType(indexedAccessType, node); + } + // If strict null checks and strict property initialization checks are enabled, if we have + // a this.xxx property access, if the property is an instance property without an initializer, + // and if we are in a constructor of the same class as the property declaration, assume that + // the property is uninitialized at the top of the control flow. + let assumeUninitialized = false; + if (strictNullChecks && strictPropertyInitialization && node.expression.kind === SyntaxKind.ThisKeyword) { + const declaration = prop && prop.valueDeclaration; + if (declaration && isInstancePropertyWithoutInitializer(declaration)) { + const flowContainer = getControlFlowContainer(node); + if (flowContainer.kind === SyntaxKind.Constructor && flowContainer.parent === declaration.parent) { + assumeUninitialized = true; + } + } + } + else if (strictNullChecks && prop && prop.valueDeclaration && + isPropertyAccessExpression(prop.valueDeclaration) && + getAssignmentDeclarationPropertyAccessKind(prop.valueDeclaration) && + getControlFlowContainer(node) === getControlFlowContainer(prop.valueDeclaration)) { + assumeUninitialized = true; + } + const flowType = getFlowTypeOfReference(node, indexedAccessType, assumeUninitialized ? getOptionalType(indexedAccessType) : indexedAccessType); + if (assumeUninitialized && !(getFalsyFlags(indexedAccessType) & TypeFlags.Undefined) && getFalsyFlags(flowType) & TypeFlags.Undefined) { + error(indexExpression, Diagnostics.Property_0_is_used_before_being_assigned, symbolToString(prop!)); // TODO: GH#18217 + // Return the declared type to reduce follow-on errors + return indexedAccessType; + } + return checkIndexedAccessIndexType(assignmentKind ? getBaseTypeOfLiteralType(flowType) : flowType, node); } function checkThatExpressionIsProperSymbolReference(expression: Expression, expressionType: Type, reportError: boolean): boolean { diff --git a/tests/cases/conformance/controlFlow/controlFlowElementAccess2.ts b/tests/cases/conformance/controlFlow/controlFlowElementAccess2.ts new file mode 100644 index 0000000000000..fa0592c973ee3 --- /dev/null +++ b/tests/cases/conformance/controlFlow/controlFlowElementAccess2.ts @@ -0,0 +1,13 @@ +// @strict: true +declare const config: { + [key: string]: boolean | { prop: string }; +}; + +if (typeof config['works'] !== 'boolean') { + config.works.prop = 'test'; // ok + config['works'].prop = 'test'; // error, config['works']: boolean | { 'prop': string } +} +if (typeof config.works !== 'boolean') { + config['works'].prop = 'test'; // error, config['works']: boolean | { 'prop': string } + config.works.prop = 'test'; // ok +} From 307fad1c473155d470f76aa56f7f7762451b6d0c Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Mon, 20 May 2019 12:51:54 -0700 Subject: [PATCH 2/5] Add baselines --- .../reference/controlFlowElementAccess2.js | 25 ++++++++ .../controlFlowElementAccess2.symbols | 37 +++++++++++ .../reference/controlFlowElementAccess2.types | 63 +++++++++++++++++++ 3 files changed, 125 insertions(+) create mode 100644 tests/baselines/reference/controlFlowElementAccess2.js create mode 100644 tests/baselines/reference/controlFlowElementAccess2.symbols create mode 100644 tests/baselines/reference/controlFlowElementAccess2.types diff --git a/tests/baselines/reference/controlFlowElementAccess2.js b/tests/baselines/reference/controlFlowElementAccess2.js new file mode 100644 index 0000000000000..050fc15b8af1f --- /dev/null +++ b/tests/baselines/reference/controlFlowElementAccess2.js @@ -0,0 +1,25 @@ +//// [controlFlowElementAccess2.ts] +declare const config: { + [key: string]: boolean | { prop: string }; +}; + +if (typeof config['works'] !== 'boolean') { + config.works.prop = 'test'; // ok + config['works'].prop = 'test'; // error, config['works']: boolean | { 'prop': string } +} +if (typeof config.works !== 'boolean') { + config['works'].prop = 'test'; // error, config['works']: boolean | { 'prop': string } + config.works.prop = 'test'; // ok +} + + +//// [controlFlowElementAccess2.js] +"use strict"; +if (typeof config['works'] !== 'boolean') { + config.works.prop = 'test'; // ok + config['works'].prop = 'test'; // error, config['works']: boolean | { 'prop': string } +} +if (typeof config.works !== 'boolean') { + config['works'].prop = 'test'; // error, config['works']: boolean | { 'prop': string } + config.works.prop = 'test'; // ok +} diff --git a/tests/baselines/reference/controlFlowElementAccess2.symbols b/tests/baselines/reference/controlFlowElementAccess2.symbols new file mode 100644 index 0000000000000..5cdd890a75973 --- /dev/null +++ b/tests/baselines/reference/controlFlowElementAccess2.symbols @@ -0,0 +1,37 @@ +=== tests/cases/conformance/controlFlow/controlFlowElementAccess2.ts === +declare const config: { +>config : Symbol(config, Decl(controlFlowElementAccess2.ts, 0, 13)) + + [key: string]: boolean | { prop: string }; +>key : Symbol(key, Decl(controlFlowElementAccess2.ts, 1, 5)) +>prop : Symbol(prop, Decl(controlFlowElementAccess2.ts, 1, 30)) + +}; + +if (typeof config['works'] !== 'boolean') { +>config : Symbol(config, Decl(controlFlowElementAccess2.ts, 0, 13)) + + config.works.prop = 'test'; // ok +>config.works.prop : Symbol(prop, Decl(controlFlowElementAccess2.ts, 1, 30)) +>config : Symbol(config, Decl(controlFlowElementAccess2.ts, 0, 13)) +>prop : Symbol(prop, Decl(controlFlowElementAccess2.ts, 1, 30)) + + config['works'].prop = 'test'; // error, config['works']: boolean | { 'prop': string } +>config['works'].prop : Symbol(prop, Decl(controlFlowElementAccess2.ts, 1, 30)) +>config : Symbol(config, Decl(controlFlowElementAccess2.ts, 0, 13)) +>prop : Symbol(prop, Decl(controlFlowElementAccess2.ts, 1, 30)) +} +if (typeof config.works !== 'boolean') { +>config : Symbol(config, Decl(controlFlowElementAccess2.ts, 0, 13)) + + config['works'].prop = 'test'; // error, config['works']: boolean | { 'prop': string } +>config['works'].prop : Symbol(prop, Decl(controlFlowElementAccess2.ts, 1, 30)) +>config : Symbol(config, Decl(controlFlowElementAccess2.ts, 0, 13)) +>prop : Symbol(prop, Decl(controlFlowElementAccess2.ts, 1, 30)) + + config.works.prop = 'test'; // ok +>config.works.prop : Symbol(prop, Decl(controlFlowElementAccess2.ts, 1, 30)) +>config : Symbol(config, Decl(controlFlowElementAccess2.ts, 0, 13)) +>prop : Symbol(prop, Decl(controlFlowElementAccess2.ts, 1, 30)) +} + diff --git a/tests/baselines/reference/controlFlowElementAccess2.types b/tests/baselines/reference/controlFlowElementAccess2.types new file mode 100644 index 0000000000000..65dca4de50226 --- /dev/null +++ b/tests/baselines/reference/controlFlowElementAccess2.types @@ -0,0 +1,63 @@ +=== tests/cases/conformance/controlFlow/controlFlowElementAccess2.ts === +declare const config: { +>config : { [key: string]: boolean | { prop: string; }; } + + [key: string]: boolean | { prop: string }; +>key : string +>prop : string + +}; + +if (typeof config['works'] !== 'boolean') { +>typeof config['works'] !== 'boolean' : boolean +>typeof config['works'] : "string" | "number" | "bigint" | "boolean" | "symbol" | "undefined" | "object" | "function" +>config['works'] : boolean | { prop: string; } +>config : { [key: string]: boolean | { prop: string; }; } +>'works' : "works" +>'boolean' : "boolean" + + config.works.prop = 'test'; // ok +>config.works.prop = 'test' : "test" +>config.works.prop : string +>config.works : { prop: string; } +>config : { [key: string]: boolean | { prop: string; }; } +>works : { prop: string; } +>prop : string +>'test' : "test" + + config['works'].prop = 'test'; // error, config['works']: boolean | { 'prop': string } +>config['works'].prop = 'test' : "test" +>config['works'].prop : string +>config['works'] : { prop: string; } +>config : { [key: string]: boolean | { prop: string; }; } +>'works' : "works" +>prop : string +>'test' : "test" +} +if (typeof config.works !== 'boolean') { +>typeof config.works !== 'boolean' : boolean +>typeof config.works : "string" | "number" | "bigint" | "boolean" | "symbol" | "undefined" | "object" | "function" +>config.works : boolean | { prop: string; } +>config : { [key: string]: boolean | { prop: string; }; } +>works : boolean | { prop: string; } +>'boolean' : "boolean" + + config['works'].prop = 'test'; // error, config['works']: boolean | { 'prop': string } +>config['works'].prop = 'test' : "test" +>config['works'].prop : string +>config['works'] : { prop: string; } +>config : { [key: string]: boolean | { prop: string; }; } +>'works' : "works" +>prop : string +>'test' : "test" + + config.works.prop = 'test'; // ok +>config.works.prop = 'test' : "test" +>config.works.prop : string +>config.works : { prop: string; } +>config : { [key: string]: boolean | { prop: string; }; } +>works : { prop: string; } +>prop : string +>'test' : "test" +} + From 339c87e53badfc2e019351b2622134b0b8f35bdb Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Mon, 20 May 2019 13:26:01 -0700 Subject: [PATCH 3/5] Fix cast lint --- 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 0d540a9dc633e..78e5f90ae5d15 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -20537,7 +20537,7 @@ namespace ts { } const flowType = getFlowTypeOfReference(node, indexedAccessType, assumeUninitialized ? getOptionalType(indexedAccessType) : indexedAccessType); if (assumeUninitialized && !(getFalsyFlags(indexedAccessType) & TypeFlags.Undefined) && getFalsyFlags(flowType) & TypeFlags.Undefined) { - error(indexExpression, Diagnostics.Property_0_is_used_before_being_assigned, symbolToString(prop!)); // TODO: GH#18217 + error(indexExpression, Diagnostics.Property_0_is_used_before_being_assigned, symbolToString(prop)); // Return the declared type to reduce follow-on errors return indexedAccessType; } From 1494f9f9084e3b2ecedc4f3a98c32bc5063e6e5b Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Wed, 29 May 2019 14:23:14 -0700 Subject: [PATCH 4/5] Cleanup to share code path --- src/compiler/checker.ts | 57 +++++++++-------------------------------- 1 file changed, 12 insertions(+), 45 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index d61ea4b5d971a..ea5fb1607ce03 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -20239,10 +20239,15 @@ namespace ts { } propType = getConstraintForLocation(getTypeOfSymbol(prop), node); } + return getFlowTypeOfAccessExpression(node, prop, propType, right); + } + + function getFlowTypeOfAccessExpression(node: ElementAccessExpression | PropertyAccessExpression | QualifiedName, prop: Symbol | undefined, propType: Type, errorNode: Node) { // 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 || + const assignmentKind = getAssignmentTargetKind(node); + if (node.kind !== SyntaxKind.ElementAccessExpression && node.kind !== SyntaxKind.PropertyAccessExpression || assignmentKind === AssignmentKind.Definite || prop && !(prop.flags & (SymbolFlags.Variable | SymbolFlags.Property | SymbolFlags.Accessor)) && !(prop.flags & SymbolFlags.Method && propType.flags & TypeFlags.Union)) { return propType; @@ -20252,7 +20257,7 @@ namespace ts { // and if we are in a constructor of the same class as the property declaration, assume that // the property is uninitialized at the top of the control flow. let assumeUninitialized = false; - if (strictNullChecks && strictPropertyInitialization && left.kind === SyntaxKind.ThisKeyword) { + if (strictNullChecks && strictPropertyInitialization && node.expression.kind === SyntaxKind.ThisKeyword) { const declaration = prop && prop.valueDeclaration; if (declaration && isInstancePropertyWithoutInitializer(declaration)) { const flowContainer = getControlFlowContainer(node); @@ -20262,14 +20267,14 @@ namespace ts { } } else if (strictNullChecks && prop && prop.valueDeclaration && - isPropertyAccessExpression(prop.valueDeclaration) && - getAssignmentDeclarationPropertyAccessKind(prop.valueDeclaration) && - getControlFlowContainer(node) === getControlFlowContainer(prop.valueDeclaration)) { + isPropertyAccessExpression(prop.valueDeclaration) && + getAssignmentDeclarationPropertyAccessKind(prop.valueDeclaration) && + getControlFlowContainer(node) === getControlFlowContainer(prop.valueDeclaration)) { assumeUninitialized = true; } const flowType = getFlowTypeOfReference(node, propType, assumeUninitialized ? getOptionalType(propType) : propType); if (assumeUninitialized && !(getFalsyFlags(propType) & TypeFlags.Undefined) && getFalsyFlags(flowType) & TypeFlags.Undefined) { - error(right, Diagnostics.Property_0_is_used_before_being_assigned, symbolToString(prop!)); // TODO: GH#18217 + error(errorNode, Diagnostics.Property_0_is_used_before_being_assigned, symbolToString(prop!)); // Return the declared type to reduce follow-on errors return propType; } @@ -20624,45 +20629,7 @@ namespace ts { AccessFlags.Writing | (isGenericObjectType(objectType) && !isThisTypeParameter(objectType) ? AccessFlags.NoIndexSignatures : 0) : AccessFlags.None; const indexedAccessType = getIndexedAccessTypeOrUndefined(objectType, effectiveIndexType, node, accessFlags) || errorType; - const prop = indexedAccessType.symbol; - - // 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. - // TODO: This probably doesn't handle element access expressions - const assignmentKind = getAssignmentTargetKind(node); - if (node.kind !== SyntaxKind.ElementAccessExpression || - assignmentKind === AssignmentKind.Definite || - prop && !(prop.flags & (SymbolFlags.Variable | SymbolFlags.Property | SymbolFlags.Accessor)) && !(prop.flags & SymbolFlags.Method && indexedAccessType.flags & TypeFlags.Union)) { - return checkIndexedAccessIndexType(indexedAccessType, node); - } - // If strict null checks and strict property initialization checks are enabled, if we have - // a this.xxx property access, if the property is an instance property without an initializer, - // and if we are in a constructor of the same class as the property declaration, assume that - // the property is uninitialized at the top of the control flow. - let assumeUninitialized = false; - if (strictNullChecks && strictPropertyInitialization && node.expression.kind === SyntaxKind.ThisKeyword) { - const declaration = prop && prop.valueDeclaration; - if (declaration && isInstancePropertyWithoutInitializer(declaration)) { - const flowContainer = getControlFlowContainer(node); - if (flowContainer.kind === SyntaxKind.Constructor && flowContainer.parent === declaration.parent) { - assumeUninitialized = true; - } - } - } - else if (strictNullChecks && prop && prop.valueDeclaration && - isPropertyAccessExpression(prop.valueDeclaration) && - getAssignmentDeclarationPropertyAccessKind(prop.valueDeclaration) && - getControlFlowContainer(node) === getControlFlowContainer(prop.valueDeclaration)) { - assumeUninitialized = true; - } - const flowType = getFlowTypeOfReference(node, indexedAccessType, assumeUninitialized ? getOptionalType(indexedAccessType) : indexedAccessType); - if (assumeUninitialized && !(getFalsyFlags(indexedAccessType) & TypeFlags.Undefined) && getFalsyFlags(flowType) & TypeFlags.Undefined) { - error(indexExpression, Diagnostics.Property_0_is_used_before_being_assigned, symbolToString(prop)); - // Return the declared type to reduce follow-on errors - return indexedAccessType; - } - return checkIndexedAccessIndexType(assignmentKind ? getBaseTypeOfLiteralType(flowType) : flowType, node); + return checkIndexedAccessIndexType(getFlowTypeOfAccessExpression(node, indexedAccessType.symbol, indexedAccessType, indexExpression), node); } function checkThatExpressionIsProperSymbolReference(expression: Expression, expressionType: Type, reportError: boolean): boolean { From 274e444836fa6232b0338cd69cc55744848533b4 Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Wed, 29 May 2019 14:33:01 -0700 Subject: [PATCH 5/5] Fix errant diffs --- src/compiler/checker.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index ea5fb1607ce03..9a8e2dc980f3d 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -20267,14 +20267,14 @@ namespace ts { } } else if (strictNullChecks && prop && prop.valueDeclaration && - isPropertyAccessExpression(prop.valueDeclaration) && - getAssignmentDeclarationPropertyAccessKind(prop.valueDeclaration) && - getControlFlowContainer(node) === getControlFlowContainer(prop.valueDeclaration)) { + isPropertyAccessExpression(prop.valueDeclaration) && + getAssignmentDeclarationPropertyAccessKind(prop.valueDeclaration) && + getControlFlowContainer(node) === getControlFlowContainer(prop.valueDeclaration)) { assumeUninitialized = true; } const flowType = getFlowTypeOfReference(node, propType, assumeUninitialized ? getOptionalType(propType) : propType); if (assumeUninitialized && !(getFalsyFlags(propType) & TypeFlags.Undefined) && getFalsyFlags(flowType) & TypeFlags.Undefined) { - error(errorNode, Diagnostics.Property_0_is_used_before_being_assigned, symbolToString(prop!)); + error(errorNode, Diagnostics.Property_0_is_used_before_being_assigned, symbolToString(prop!)); // TODO: GH#18217 // Return the declared type to reduce follow-on errors return propType; }