From 31a1a4af9d0847a3c4af634dff540cfe0a870fe7 Mon Sep 17 00:00:00 2001 From: Wesley Wigham Date: Wed, 3 Jan 2018 16:41:50 -0800 Subject: [PATCH 1/3] Use apparent type of original type to handle indexes --- src/compiler/checker.ts | 2 +- ...efiniteAssignmentOfDestructuredVariable.js | 28 ++++++++++++ ...teAssignmentOfDestructuredVariable.symbols | 42 +++++++++++++++++ ...niteAssignmentOfDestructuredVariable.types | 45 +++++++++++++++++++ ...efiniteAssignmentOfDestructuredVariable.ts | 16 +++++++ 5 files changed, 132 insertions(+), 1 deletion(-) create mode 100644 tests/baselines/reference/definiteAssignmentOfDestructuredVariable.js create mode 100644 tests/baselines/reference/definiteAssignmentOfDestructuredVariable.symbols create mode 100644 tests/baselines/reference/definiteAssignmentOfDestructuredVariable.types create mode 100644 tests/cases/compiler/definiteAssignmentOfDestructuredVariable.ts diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index caa5a55c0c8cf..9df62086f10b2 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -13338,7 +13338,7 @@ namespace ts { return convertAutoToAny(flowType); } } - else if (!assumeInitialized && !(getFalsyFlags(type) & TypeFlags.Undefined) && getFalsyFlags(flowType) & TypeFlags.Undefined) { + else if (!assumeInitialized && !(getFalsyFlags(getApparentType(type)) & TypeFlags.Undefined) && getFalsyFlags(flowType) & TypeFlags.Undefined) { error(node, Diagnostics.Variable_0_is_used_before_being_assigned, symbolToString(symbol)); // Return the declared type to reduce follow-on errors return type; diff --git a/tests/baselines/reference/definiteAssignmentOfDestructuredVariable.js b/tests/baselines/reference/definiteAssignmentOfDestructuredVariable.js new file mode 100644 index 0000000000000..8fcc3efaaf938 --- /dev/null +++ b/tests/baselines/reference/definiteAssignmentOfDestructuredVariable.js @@ -0,0 +1,28 @@ +//// [definiteAssignmentOfDestructuredVariable.ts] +// https://github.com/Microsoft/TypeScript/issues/20994 +interface Options { + a?: number | object; + b: () => void; +} + +class C { + foo!: { [P in keyof T]: T[P] } + + method() { + let { a, b } = this.foo; + !(a && b); + a; + } +} + +//// [definiteAssignmentOfDestructuredVariable.js] +var C = /** @class */ (function () { + function C() { + } + C.prototype.method = function () { + var _a = this.foo, a = _a.a, b = _a.b; + !(a && b); + a; + }; + return C; +}()); diff --git a/tests/baselines/reference/definiteAssignmentOfDestructuredVariable.symbols b/tests/baselines/reference/definiteAssignmentOfDestructuredVariable.symbols new file mode 100644 index 0000000000000..64726c73de756 --- /dev/null +++ b/tests/baselines/reference/definiteAssignmentOfDestructuredVariable.symbols @@ -0,0 +1,42 @@ +=== tests/cases/compiler/definiteAssignmentOfDestructuredVariable.ts === +// https://github.com/Microsoft/TypeScript/issues/20994 +interface Options { +>Options : Symbol(Options, Decl(definiteAssignmentOfDestructuredVariable.ts, 0, 0)) + + a?: number | object; +>a : Symbol(Options.a, Decl(definiteAssignmentOfDestructuredVariable.ts, 1, 19)) + + b: () => void; +>b : Symbol(Options.b, Decl(definiteAssignmentOfDestructuredVariable.ts, 2, 24)) +} + +class C { +>C : Symbol(C, Decl(definiteAssignmentOfDestructuredVariable.ts, 4, 1)) +>T : Symbol(T, Decl(definiteAssignmentOfDestructuredVariable.ts, 6, 8)) +>Options : Symbol(Options, Decl(definiteAssignmentOfDestructuredVariable.ts, 0, 0)) + + foo!: { [P in keyof T]: T[P] } +>foo : Symbol(C.foo, Decl(definiteAssignmentOfDestructuredVariable.ts, 6, 28)) +>P : Symbol(P, Decl(definiteAssignmentOfDestructuredVariable.ts, 7, 13)) +>T : Symbol(T, Decl(definiteAssignmentOfDestructuredVariable.ts, 6, 8)) +>T : Symbol(T, Decl(definiteAssignmentOfDestructuredVariable.ts, 6, 8)) +>P : Symbol(P, Decl(definiteAssignmentOfDestructuredVariable.ts, 7, 13)) + + method() { +>method : Symbol(C.method, Decl(definiteAssignmentOfDestructuredVariable.ts, 7, 34)) + + let { a, b } = this.foo; +>a : Symbol(a, Decl(definiteAssignmentOfDestructuredVariable.ts, 10, 13)) +>b : Symbol(b, Decl(definiteAssignmentOfDestructuredVariable.ts, 10, 16)) +>this.foo : Symbol(C.foo, Decl(definiteAssignmentOfDestructuredVariable.ts, 6, 28)) +>this : Symbol(C, Decl(definiteAssignmentOfDestructuredVariable.ts, 4, 1)) +>foo : Symbol(C.foo, Decl(definiteAssignmentOfDestructuredVariable.ts, 6, 28)) + + !(a && b); +>a : Symbol(a, Decl(definiteAssignmentOfDestructuredVariable.ts, 10, 13)) +>b : Symbol(b, Decl(definiteAssignmentOfDestructuredVariable.ts, 10, 16)) + + a; +>a : Symbol(a, Decl(definiteAssignmentOfDestructuredVariable.ts, 10, 13)) + } +} diff --git a/tests/baselines/reference/definiteAssignmentOfDestructuredVariable.types b/tests/baselines/reference/definiteAssignmentOfDestructuredVariable.types new file mode 100644 index 0000000000000..47e011b14038f --- /dev/null +++ b/tests/baselines/reference/definiteAssignmentOfDestructuredVariable.types @@ -0,0 +1,45 @@ +=== tests/cases/compiler/definiteAssignmentOfDestructuredVariable.ts === +// https://github.com/Microsoft/TypeScript/issues/20994 +interface Options { +>Options : Options + + a?: number | object; +>a : number | object | undefined + + b: () => void; +>b : () => void +} + +class C { +>C : C +>T : T +>Options : Options + + foo!: { [P in keyof T]: T[P] } +>foo : { [P in keyof T]: T[P]; } +>P : P +>T : T +>T : T +>P : P + + method() { +>method : () => void + + let { a, b } = this.foo; +>a : T["a"] +>b : T["b"] +>this.foo : { [P in keyof T]: T[P]; } +>this : this +>foo : { [P in keyof T]: T[P]; } + + !(a && b); +>!(a && b) : false +>(a && b) : T["b"] +>a && b : T["b"] +>a : T["a"] +>b : T["b"] + + a; +>a : number | object | undefined + } +} diff --git a/tests/cases/compiler/definiteAssignmentOfDestructuredVariable.ts b/tests/cases/compiler/definiteAssignmentOfDestructuredVariable.ts new file mode 100644 index 0000000000000..800a9df831385 --- /dev/null +++ b/tests/cases/compiler/definiteAssignmentOfDestructuredVariable.ts @@ -0,0 +1,16 @@ +// @strictNullChecks: true +// https://github.com/Microsoft/TypeScript/issues/20994 +interface Options { + a?: number | object; + b: () => void; +} + +class C { + foo!: { [P in keyof T]: T[P] } + + method() { + let { a, b } = this.foo; + !(a && b); + a; + } +} \ No newline at end of file From 30b06c777e34aa5d3777f77ca27f8d7f762edb0f Mon Sep 17 00:00:00 2001 From: Wesley Wigham Date: Wed, 17 Jan 2018 16:05:43 -0800 Subject: [PATCH 2/3] Redo older fix causing new bug by extending getDeclaredOrApparentType instead of getTypeWithFacts --- src/compiler/checker.ts | 26 +++++++------------ ...niteAssignmentOfDestructuredVariable.types | 2 +- ...strictNullNotNullIndexTypeShouldWork.types | 8 +++--- 3 files changed, 14 insertions(+), 22 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 9df62086f10b2..96ea56c98d956 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -4308,7 +4308,8 @@ namespace ts { if (strictNullChecks && declaration.flags & NodeFlags.Ambient && isParameterDeclaration(declaration)) { parentType = getNonNullableType(parentType); } - const declaredType = getTypeOfPropertyOfType(parentType, text); + const propType = getTypeOfPropertyOfType(parentType, text); + const declaredType = propType && getDeclaredOrApparentType(propType, declaration.name); type = declaredType && getFlowTypeOfReference(declaration, declaredType) || isNumericLiteralName(text) && getIndexTypeOfType(parentType, IndexKind.Number) || getIndexTypeOfType(parentType, IndexKind.String); @@ -12039,16 +12040,6 @@ namespace ts { } function getTypeWithFacts(type: Type, include: TypeFacts) { - if (type.flags & TypeFlags.IndexedAccess) { - // TODO (weswig): This is a substitute for a lazy negated type to remove the types indicated by the TypeFacts from the (potential) union the IndexedAccess refers to - // - See discussion in https://github.com/Microsoft/TypeScript/pull/19275 for details, and test `strictNullNotNullIndexTypeShouldWork` for current behavior - const baseConstraint = getBaseConstraintOfType(type) || emptyObjectType; - const result = filterType(baseConstraint, t => (getTypeFacts(t) & include) !== 0); - if (result !== baseConstraint) { - return result; - } - return type; - } return filterType(type, t => (getTypeFacts(t) & include) !== 0); } @@ -13162,19 +13153,20 @@ namespace ts { const parent = node.parent; return parent.kind === SyntaxKind.PropertyAccessExpression || parent.kind === SyntaxKind.CallExpression && (parent).expression === node || - parent.kind === SyntaxKind.ElementAccessExpression && (parent).expression === node; + parent.kind === SyntaxKind.ElementAccessExpression && (parent).expression === node || + parent.kind === SyntaxKind.NonNullExpression || + parent.kind === SyntaxKind.BindingElement && (parent).name === node && !!(parent).initializer; } function typeHasNullableConstraint(type: Type) { return type.flags & TypeFlags.TypeVariable && maybeTypeOfKind(getBaseConstraintOfType(type) || emptyObjectType, TypeFlags.Nullable); } - function getDeclaredOrApparentType(symbol: Symbol, node: Node) { + function getDeclaredOrApparentType(type: Type, node: Node) { // When a node is the left hand expression of a property access, element access, or call expression, // and the type of the node includes type variables with constraints that are nullable, we fetch the // apparent type of the node *before* performing control flow analysis such that narrowings apply to // the constraint type. - const type = getTypeOfSymbol(symbol); if (isApparentTypePosition(node) && forEachType(type, typeHasNullableConstraint)) { return mapType(getWidenedType(type), getApparentType); } @@ -13264,7 +13256,7 @@ namespace ts { checkCollisionWithCapturedNewTargetVariable(node, node); checkNestedBlockScopedBinding(node, symbol); - const type = getDeclaredOrApparentType(localOrExportSymbol, node); + const type = getDeclaredOrApparentType(getTypeOfSymbol(localOrExportSymbol), node); const assignmentKind = getAssignmentTargetKind(node); if (assignmentKind) { @@ -13338,7 +13330,7 @@ namespace ts { return convertAutoToAny(flowType); } } - else if (!assumeInitialized && !(getFalsyFlags(getApparentType(type)) & TypeFlags.Undefined) && getFalsyFlags(flowType) & TypeFlags.Undefined) { + else if (!assumeInitialized && !(getFalsyFlags(type) & TypeFlags.Undefined) && getFalsyFlags(flowType) & TypeFlags.Undefined) { error(node, Diagnostics.Variable_0_is_used_before_being_assigned, symbolToString(symbol)); // Return the declared type to reduce follow-on errors return type; @@ -15816,7 +15808,7 @@ namespace ts { return unknownType; } } - propType = getDeclaredOrApparentType(prop, node); + propType = getDeclaredOrApparentType(getTypeOfSymbol(prop), 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, diff --git a/tests/baselines/reference/definiteAssignmentOfDestructuredVariable.types b/tests/baselines/reference/definiteAssignmentOfDestructuredVariable.types index 47e011b14038f..36ff5028305a1 100644 --- a/tests/baselines/reference/definiteAssignmentOfDestructuredVariable.types +++ b/tests/baselines/reference/definiteAssignmentOfDestructuredVariable.types @@ -40,6 +40,6 @@ class C { >b : T["b"] a; ->a : number | object | undefined +>a : T["a"] } } diff --git a/tests/baselines/reference/strictNullNotNullIndexTypeShouldWork.types b/tests/baselines/reference/strictNullNotNullIndexTypeShouldWork.types index 76515fbdd5321..e401a3d4950c0 100644 --- a/tests/baselines/reference/strictNullNotNullIndexTypeShouldWork.types +++ b/tests/baselines/reference/strictNullNotNullIndexTypeShouldWork.types @@ -23,11 +23,11 @@ class Test { this.attrs.params!.name; >this.attrs.params!.name : string >this.attrs.params! : { name: string; } ->this.attrs.params : T["params"] +>this.attrs.params : { name: string; } | undefined >this.attrs : Readonly >this : this >attrs : Readonly ->params : T["params"] +>params : { name: string; } | undefined >name : string } } @@ -80,10 +80,10 @@ class Test2 { return this.attrs.params!; // Return type should maintain relationship with `T` after being not-null-asserted, ideally >this.attrs.params! : { name: string; } ->this.attrs.params : T["params"] +>this.attrs.params : { name: string; } | undefined >this.attrs : Readonly >this : this >attrs : Readonly ->params : T["params"] +>params : { name: string; } | undefined } } From 5dfffb1d0efb57d42e86903414b180bc47b4d0e9 Mon Sep 17 00:00:00 2001 From: Wesley Wigham Date: Fri, 19 Jan 2018 13:50:22 -0800 Subject: [PATCH 3/3] Rename symbol --- 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 96ea56c98d956..a37c7124d9ab2 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -4309,7 +4309,7 @@ namespace ts { parentType = getNonNullableType(parentType); } const propType = getTypeOfPropertyOfType(parentType, text); - const declaredType = propType && getDeclaredOrApparentType(propType, declaration.name); + const declaredType = propType && getApparentTypeForLocation(propType, declaration.name); type = declaredType && getFlowTypeOfReference(declaration, declaredType) || isNumericLiteralName(text) && getIndexTypeOfType(parentType, IndexKind.Number) || getIndexTypeOfType(parentType, IndexKind.String); @@ -13162,7 +13162,7 @@ namespace ts { return type.flags & TypeFlags.TypeVariable && maybeTypeOfKind(getBaseConstraintOfType(type) || emptyObjectType, TypeFlags.Nullable); } - function getDeclaredOrApparentType(type: Type, node: Node) { + function getApparentTypeForLocation(type: Type, node: Node) { // When a node is the left hand expression of a property access, element access, or call expression, // and the type of the node includes type variables with constraints that are nullable, we fetch the // apparent type of the node *before* performing control flow analysis such that narrowings apply to @@ -13256,7 +13256,7 @@ namespace ts { checkCollisionWithCapturedNewTargetVariable(node, node); checkNestedBlockScopedBinding(node, symbol); - const type = getDeclaredOrApparentType(getTypeOfSymbol(localOrExportSymbol), node); + const type = getApparentTypeForLocation(getTypeOfSymbol(localOrExportSymbol), node); const assignmentKind = getAssignmentTargetKind(node); if (assignmentKind) { @@ -15808,7 +15808,7 @@ namespace ts { return unknownType; } } - propType = getDeclaredOrApparentType(getTypeOfSymbol(prop), node); + propType = getApparentTypeForLocation(getTypeOfSymbol(prop), 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,