Skip to content

Commit

Permalink
Merge pull request #33821 from microsoft/fix33806
Browse files Browse the repository at this point in the history 
Reflect control flow effects of optional chains in equality checks
  • Loading branch information
@ahejlsberg
ahejlsberg authored Oct 6, 2019
2 parents d552675 + 99cec3a commit afe6f4c
Show file tree
Hide file tree
Showing 7 changed files with 1,957 additions and 28 deletions.
5 changes: 2 additions & 3 deletions src/compiler/binder.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -856,9 +856,8 @@ namespace ts {
function isNarrowableReference(expr: Expression): boolean {
return expr.kind === SyntaxKind.Identifier || expr.kind === SyntaxKind.ThisKeyword || expr.kind === SyntaxKind.SuperKeyword ||
(isPropertyAccessExpression(expr) || isNonNullExpression(expr) || isParenthesizedExpression(expr)) && isNarrowableReference(expr.expression) ||
isElementAccessExpression(expr) &&
isStringOrNumericLiteralLike(expr.argumentExpression) &&
isNarrowableReference(expr.expression);
isElementAccessExpression(expr) && isStringOrNumericLiteralLike(expr.argumentExpression) && isNarrowableReference(expr.expression) ||
isOptionalChain(expr);
}

function hasNarrowableArgument(expr: CallExpression) {
Expand Down
37 changes: 37 additions & 0 deletions src/compiler/checker.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18124,6 +18124,16 @@ namespace ts {
return false;
}

function optionalChainContainsReference(source: Node, target: Node) {
while (isOptionalChain(source)) {
source = source.expression;
if (isMatchingReference(source, target)) {
return true;
}
}
return false;
}

// Return true if target is a property access xxx.yyy, source is a property access xxx.zzz, the declared
// type of xxx is a union type, and yyy is a property that is possibly a discriminant. We consider a property
// a possible discriminant if its type differs in the constituents of containing union type, and if every
Expand Down Expand Up @@ -19350,6 +19360,14 @@ namespace ts {
if (isMatchingReference(reference, right)) {
return narrowTypeByEquality(type, operator, left, assumeTrue);
}
if (assumeTrue && strictNullChecks) {
if (optionalChainContainsReference(left, reference)) {
type = narrowTypeByOptionalChainContainment(type, operator, right);
}
else if (optionalChainContainsReference(right, reference)) {
type = narrowTypeByOptionalChainContainment(type, operator, left);
}
}
if (isMatchingReferenceDiscriminant(left, declaredType)) {
return narrowTypeByDiscriminant(type, <AccessExpression>left, t => narrowTypeByEquality(t, operator, right, assumeTrue));
}
Expand All @@ -19374,6 +19392,18 @@ namespace ts {
return type;
}

function narrowTypeByOptionalChainContainment(type: Type, operator: SyntaxKind, value: Expression): Type {
// We are in the true branch of obj?.foo === value or obj?.foo !== value. We remove undefined and null from
// the type of obj if (a) the operator is === and the type of value doesn't include undefined or (b) the
// operator is !== and the type of value is undefined.
const valueType = getTypeOfExpression(value);
return operator === SyntaxKind.EqualsEqualsToken && !(getTypeFacts(valueType) & TypeFacts.EQUndefinedOrNull) ||
operator === SyntaxKind.EqualsEqualsEqualsToken && !(getTypeFacts(valueType) & TypeFacts.EQUndefined) ||
operator === SyntaxKind.ExclamationEqualsToken && valueType.flags & TypeFlags.Nullable ||
operator === SyntaxKind.ExclamationEqualsEqualsToken && valueType.flags & TypeFlags.Undefined ?
getTypeWithFacts(type, TypeFacts.NEUndefinedOrNull) : type;
}

function narrowTypeByEquality(type: Type, operator: SyntaxKind, value: Expression, assumeTrue: boolean): Type {
if (type.flags & TypeFlags.Any) {
return type;
Expand Down Expand Up @@ -19424,6 +19454,10 @@ namespace ts {
// We have '==', '!=', '===', or !==' operator with 'typeof xxx' and string literal operands
const target = getReferenceCandidate(typeOfExpr.expression);
if (!isMatchingReference(reference, target)) {
if (assumeTrue && (operator === SyntaxKind.EqualsEqualsToken || operator === SyntaxKind.EqualsEqualsEqualsToken) &&
strictNullChecks && optionalChainContainsReference(target, reference)) {
return getTypeWithFacts(type, TypeFacts.NEUndefinedOrNull);
}
// For a reference of the form 'x.y', a 'typeof x === ...' type guard resets the
// narrowed type of 'y' to its declared type.
if (containsMatchingReference(reference, target)) {
Expand Down Expand Up @@ -19605,6 +19639,9 @@ namespace ts {
function narrowTypeByInstanceof(type: Type, expr: BinaryExpression, assumeTrue: boolean): Type {
const left = getReferenceCandidate(expr.left);
if (!isMatchingReference(reference, left)) {
if (assumeTrue && strictNullChecks && optionalChainContainsReference(left, reference)) {
return getTypeWithFacts(type, TypeFacts.NEUndefinedOrNull);
}
// For a reference of the form 'x.y', an 'x instanceof T' type guard resets the
// narrowed type of 'y' to its declared type. We do this because preceding 'x.y'
// references might reference a different 'y' property. However, we make an exception
Expand Down
215 changes: 209 additions & 6 deletions tests/baselines/reference/controlFlowOptionalChain.errors.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ tests/cases/conformance/controlFlow/controlFlowOptionalChain.ts(35,5): error TS2
tests/cases/conformance/controlFlow/controlFlowOptionalChain.ts(39,1): error TS2722: Cannot invoke an object which is possibly 'undefined'.
tests/cases/conformance/controlFlow/controlFlowOptionalChain.ts(52,5): error TS2532: Object is possibly 'undefined'.
tests/cases/conformance/controlFlow/controlFlowOptionalChain.ts(57,1): error TS2532: Object is possibly 'undefined'.
tests/cases/conformance/controlFlow/controlFlowOptionalChain.ts(62,5): error TS2532: Object is possibly 'undefined'.
tests/cases/conformance/controlFlow/controlFlowOptionalChain.ts(68,5): error TS2532: Object is possibly 'undefined'.
tests/cases/conformance/controlFlow/controlFlowOptionalChain.ts(72,1): error TS2532: Object is possibly 'undefined'.
tests/cases/conformance/controlFlow/controlFlowOptionalChain.ts(83,5): error TS2532: Object is possibly 'undefined'.
Expand All @@ -20,9 +19,21 @@ tests/cases/conformance/controlFlow/controlFlowOptionalChain.ts(112,1): error TS
tests/cases/conformance/controlFlow/controlFlowOptionalChain.ts(130,5): error TS2532: Object is possibly 'undefined'.
tests/cases/conformance/controlFlow/controlFlowOptionalChain.ts(134,1): error TS2532: Object is possibly 'undefined'.
tests/cases/conformance/controlFlow/controlFlowOptionalChain.ts(153,9): error TS2775: Assertions require every name in the call target to be declared with an explicit type annotation.
tests/cases/conformance/controlFlow/controlFlowOptionalChain.ts(208,9): error TS2532: Object is possibly 'undefined'.
tests/cases/conformance/controlFlow/controlFlowOptionalChain.ts(211,9): error TS2532: Object is possibly 'undefined'.
tests/cases/conformance/controlFlow/controlFlowOptionalChain.ts(214,9): error TS2532: Object is possibly 'undefined'.
tests/cases/conformance/controlFlow/controlFlowOptionalChain.ts(217,9): error TS2532: Object is possibly 'undefined'.
tests/cases/conformance/controlFlow/controlFlowOptionalChain.ts(220,9): error TS2532: Object is possibly 'undefined'.
tests/cases/conformance/controlFlow/controlFlowOptionalChain.ts(223,9): error TS2532: Object is possibly 'undefined'.
tests/cases/conformance/controlFlow/controlFlowOptionalChain.ts(238,9): error TS2532: Object is possibly 'undefined'.
tests/cases/conformance/controlFlow/controlFlowOptionalChain.ts(241,9): error TS2532: Object is possibly 'undefined'.
tests/cases/conformance/controlFlow/controlFlowOptionalChain.ts(244,9): error TS2532: Object is possibly 'undefined'.
tests/cases/conformance/controlFlow/controlFlowOptionalChain.ts(271,9): error TS2532: Object is possibly 'undefined'.
tests/cases/conformance/controlFlow/controlFlowOptionalChain.ts(274,9): error TS2532: Object is possibly 'undefined'.
tests/cases/conformance/controlFlow/controlFlowOptionalChain.ts(277,9): error TS2532: Object is possibly 'undefined'.


==== tests/cases/conformance/controlFlow/controlFlowOptionalChain.ts (22 errors) ====
==== tests/cases/conformance/controlFlow/controlFlowOptionalChain.ts (33 errors) ====
// assignments in shortcutting chain
declare const o: undefined | {
[key: string]: any;
Expand Down Expand Up @@ -99,10 +110,8 @@ tests/cases/conformance/controlFlow/controlFlowOptionalChain.ts(153,9): error TS

declare const o3: { x: 1, y: string } | { x: 2, y: number } | undefined;
if (o3?.x === 1) {
o3; // TODO: should be `{ x: y, y: string }`
o3.x; // TODO: should not be an error.
~~
!!! error TS2532: Object is possibly 'undefined'.
o3;
o3.x;
o3?.x;
}
else {
Expand Down Expand Up @@ -227,4 +236,198 @@ tests/cases/conformance/controlFlow/controlFlowOptionalChain.ts(153,9): error TS
x;
}
}

type Thing = { foo: string | number, bar(): number, baz: object };

function f10(o: Thing | undefined, value: number) {
if (o?.foo === value) {
o.foo;
}
if (o?.["foo"] === value) {
o["foo"];
}
if (o?.bar() === value) {
o.bar;
}
if (o?.foo == value) {
o.foo;
}
if (o?.["foo"] == value) {
o["foo"];
}
if (o?.bar() == value) {
o.bar;
}
}

function f11(o: Thing | null, value: number) {
if (o?.foo === value) {
o.foo;
}
if (o?.["foo"] === value) {
o["foo"];
}
if (o?.bar() === value) {
o.bar;
}
if (o?.foo == value) {
o.foo;
}
if (o?.["foo"] == value) {
o["foo"];
}
if (o?.bar() == value) {
o.bar;
}
}

function f12(o: Thing | undefined, value: number | undefined) {
if (o?.foo === value) {
o.foo; // Error
~
!!! error TS2532: Object is possibly 'undefined'.
}
if (o?.["foo"] === value) {
o["foo"]; // Error
~
!!! error TS2532: Object is possibly 'undefined'.
}
if (o?.bar() === value) {
o.bar; // Error
~
!!! error TS2532: Object is possibly 'undefined'.
}
if (o?.foo == value) {
o.foo; // Error
~
!!! error TS2532: Object is possibly 'undefined'.
}
if (o?.["foo"] == value) {
o["foo"]; // Error
~
!!! error TS2532: Object is possibly 'undefined'.
}
if (o?.bar() == value) {
o.bar; // Error
~
!!! error TS2532: Object is possibly 'undefined'.
}
}

function f12a(o: Thing | undefined, value: number | null) {
if (o?.foo === value) {
o.foo;
}
if (o?.["foo"] === value) {
o["foo"];
}
if (o?.bar() === value) {
o.bar;
}
if (o?.foo == value) {
o.foo; // Error
~
!!! error TS2532: Object is possibly 'undefined'.
}
if (o?.["foo"] == value) {
o["foo"]; // Error
~
!!! error TS2532: Object is possibly 'undefined'.
}
if (o?.bar() == value) {
o.bar; // Error
~
!!! error TS2532: Object is possibly 'undefined'.
}
}

function f13(o: Thing | undefined) {
if (o?.foo !== undefined) {
o.foo;
}
if (o?.["foo"] !== undefined) {
o["foo"];
}
if (o?.bar() !== undefined) {
o.bar;
}
if (o?.foo != undefined) {
o.foo;
}
if (o?.["foo"] != undefined) {
o["foo"];
}
if (o?.bar() != undefined) {
o.bar;
}
}

function f13a(o: Thing | undefined) {
if (o?.foo !== null) {
o.foo; // Error
~
!!! error TS2532: Object is possibly 'undefined'.
}
if (o?.["foo"] !== null) {
o["foo"]; // Error
~
!!! error TS2532: Object is possibly 'undefined'.
}
if (o?.bar() !== null) {
o.bar; // Error
~
!!! error TS2532: Object is possibly 'undefined'.
}
if (o?.foo != null) {
o.foo;
}
if (o?.["foo"] != null) {
o["foo"];
}
if (o?.bar() != null) {
o.bar;
}
}

function f14(o: Thing | null) {
if (o?.foo !== undefined) {
o.foo;
}
if (o?.["foo"] !== undefined) {
o["foo"];
}
if (o?.bar() !== undefined) {
o.bar;
}
}

function f20(o: Thing | undefined) {
if (typeof o?.foo === "number") {
o.foo;
}
if (typeof o?.["foo"] === "number") {
o["foo"];
}
if (typeof o?.bar() === "number") {
o.bar;
}
if (o?.baz instanceof Error) {
o.baz;
}
}

function f21(o: Thing | null) {
if (typeof o?.foo === "number") {
o.foo;
}
if (typeof o?.["foo"] === "number") {
o["foo"];
}
if (typeof o?.bar() === "number") {
o.bar;
}
if (o?.baz instanceof Error) {
o.baz;
}
}

Loading

0 comments on commit afe6f4c

Please sign in to comment.