Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reflect control flow effects of optional chains in equality checks #33821

Merged
merged 8 commits into from
Oct 6, 2019
Merged

Reflect control flow effects of optional chains in equality checks #33821

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
719b779
Reflect CFA effects of optional chains in equality checks
ahejlsberg Oct 4, 2019
2447bd7
Update tests
ahejlsberg Oct 4, 2019
7d32355
Accept new baselines
ahejlsberg Oct 4, 2019
42500d0
Add tests
ahejlsberg Oct 4, 2019
464d1a7
Accept new baselines
ahejlsberg Oct 4, 2019
f7b22a0
Support typeof and instanceof
ahejlsberg Oct 5, 2019
f885181
Add tests
ahejlsberg Oct 5, 2019
99cec3a
Accept new baselines
ahejlsberg Oct 5, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions src/compiler/binder.ts
View file
Open in desktop
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
Open in desktop
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) {
rbuckton marked this conversation as resolved.
Show resolved Hide resolved
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
Open in desktop
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