Skip to content

Limit "type guards as assertions" behavior #10118

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.

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

Merged
merged 3 commits into from
Aug 3, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
74 changes: 46 additions & 28 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ namespace ts {
const flowLoopKeys: string[] = [];
const flowLoopTypes: Type[][] = [];
const visitedFlowNodes: FlowNode[] = [];
const visitedFlowTypes: Type[] = [];
const visitedFlowTypes: FlowType[] = [];
const potentialThisCollisions: Node[] = [];
const awaitedTypeStack: number[] = [];

Expand Down Expand Up @@ -8086,21 +8086,33 @@ namespace ts {
f(type) ? type : neverType;
}

function isIncomplete(flowType: FlowType) {
return flowType.flags === 0;
}

function getTypeFromFlowType(flowType: FlowType) {
return flowType.flags === 0 ? (<IncompleteType>flowType).type : <Type>flowType;
}

function createFlowType(type: Type, incomplete: boolean): FlowType {
return incomplete ? { flags: 0, type } : type;
}

function getFlowTypeOfReference(reference: Node, declaredType: Type, assumeInitialized: boolean, includeOuterFunctions: boolean) {
let key: string;
if (!reference.flowNode || assumeInitialized && !(declaredType.flags & TypeFlags.Narrowable)) {
return declaredType;
}
const initialType = assumeInitialized ? declaredType : includeFalsyTypes(declaredType, TypeFlags.Undefined);
const visitedFlowStart = visitedFlowCount;
const result = getTypeAtFlowNode(reference.flowNode);
const result = getTypeFromFlowType(getTypeAtFlowNode(reference.flowNode));
visitedFlowCount = visitedFlowStart;
if (reference.parent.kind === SyntaxKind.NonNullExpression && getTypeWithFacts(result, TypeFacts.NEUndefinedOrNull) === neverType) {
return declaredType;
}
return result;

function getTypeAtFlowNode(flow: FlowNode): Type {
function getTypeAtFlowNode(flow: FlowNode): FlowType {
while (true) {
if (flow.flags & FlowFlags.Shared) {
// We cache results of flow type resolution for shared nodes that were previously visited in
Expand All @@ -8112,7 +8124,7 @@ namespace ts {
}
}
}
let type: Type;
let type: FlowType;
if (flow.flags & FlowFlags.Assignment) {
type = getTypeAtFlowAssignment(<FlowAssignment>flow);
if (!type) {
Expand Down Expand Up @@ -8180,41 +8192,44 @@ namespace ts {
return undefined;
}

function getTypeAtFlowCondition(flow: FlowCondition) {
let type = getTypeAtFlowNode(flow.antecedent);
function getTypeAtFlowCondition(flow: FlowCondition): FlowType {
const flowType = getTypeAtFlowNode(flow.antecedent);
let type = getTypeFromFlowType(flowType);
if (type !== neverType) {
// If we have an antecedent type (meaning we're reachable in some way), we first
// attempt to narrow the antecedent type. If that produces the nothing type, then
// we take the type guard as an indication that control could reach here in a
// manner not understood by the control flow analyzer (e.g. a function argument
// has an invalid type, or a nested function has possibly made an assignment to a
// captured variable). We proceed by reverting to the declared type and then
// attempt to narrow the antecedent type. If that produces the never type, and if
// the antecedent type is incomplete (i.e. a transient type in a loop), then we
// take the type guard as an indication that control *could* reach here once we
// have the complete type. We proceed by reverting to the declared type and then
// narrow that.
const assumeTrue = (flow.flags & FlowFlags.TrueCondition) !== 0;
type = narrowType(type, flow.expression, assumeTrue);
if (type === neverType) {
if (type === neverType && isIncomplete(flowType)) {
type = narrowType(declaredType, flow.expression, assumeTrue);
}
}
return type;
return createFlowType(type, isIncomplete(flowType));
}

function getTypeAtSwitchClause(flow: FlowSwitchClause) {
const type = getTypeAtFlowNode(flow.antecedent);
function getTypeAtSwitchClause(flow: FlowSwitchClause): FlowType {
const flowType = getTypeAtFlowNode(flow.antecedent);
let type = getTypeFromFlowType(flowType);
const expr = flow.switchStatement.expression;
if (isMatchingReference(reference, expr)) {
return narrowTypeBySwitchOnDiscriminant(type, flow.switchStatement, flow.clauseStart, flow.clauseEnd);
type = narrowTypeBySwitchOnDiscriminant(type, flow.switchStatement, flow.clauseStart, flow.clauseEnd);
}
if (isMatchingPropertyAccess(expr)) {
return narrowTypeByDiscriminant(type, <PropertyAccessExpression>expr, t => narrowTypeBySwitchOnDiscriminant(t, flow.switchStatement, flow.clauseStart, flow.clauseEnd));
else if (isMatchingPropertyAccess(expr)) {
type = narrowTypeByDiscriminant(type, <PropertyAccessExpression>expr, t => narrowTypeBySwitchOnDiscriminant(t, flow.switchStatement, flow.clauseStart, flow.clauseEnd));
}
return type;
return createFlowType(type, isIncomplete(flowType));
}

function getTypeAtFlowBranchLabel(flow: FlowLabel) {
function getTypeAtFlowBranchLabel(flow: FlowLabel): FlowType {
const antecedentTypes: Type[] = [];
let seenIncomplete = false;
for (const antecedent of flow.antecedents) {
const type = getTypeAtFlowNode(antecedent);
const flowType = getTypeAtFlowNode(antecedent);
const type = getTypeFromFlowType(flowType);
// If the type at a particular antecedent path is the declared type and the
// reference is known to always be assigned (i.e. when declared and initial types
// are the same), there is no reason to process more antecedents since the only
Expand All @@ -8225,11 +8240,14 @@ namespace ts {
if (!contains(antecedentTypes, type)) {
antecedentTypes.push(type);
}
if (isIncomplete(flowType)) {
seenIncomplete = true;
}
}
return getUnionType(antecedentTypes);
return createFlowType(getUnionType(antecedentTypes), seenIncomplete);
}

function getTypeAtFlowLoopLabel(flow: FlowLabel) {
function getTypeAtFlowLoopLabel(flow: FlowLabel): FlowType {
// If we have previously computed the control flow type for the reference at
// this flow loop junction, return the cached type.
const id = getFlowNodeId(flow);
Expand All @@ -8241,12 +8259,12 @@ namespace ts {
return cache[key];
}
// If this flow loop junction and reference are already being processed, return
// the union of the types computed for each branch so far. We should never see
// an empty array here because the first antecedent of a loop junction is always
// the non-looping control flow path that leads to the top.
// the union of the types computed for each branch so far, marked as incomplete.
// We should never see an empty array here because the first antecedent of a loop
// junction is always the non-looping control flow path that leads to the top.
for (let i = flowLoopStart; i < flowLoopCount; i++) {
if (flowLoopNodes[i] === flow && flowLoopKeys[i] === key) {
return getUnionType(flowLoopTypes[i]);
return createFlowType(getUnionType(flowLoopTypes[i]), /*incomplete*/ true);
}
}
// Add the flow loop junction and reference to the in-process stack and analyze
Expand All @@ -8257,7 +8275,7 @@ namespace ts {
flowLoopTypes[flowLoopCount] = antecedentTypes;
for (const antecedent of flow.antecedents) {
flowLoopCount++;
const type = getTypeAtFlowNode(antecedent);
const type = getTypeFromFlowType(getTypeAtFlowNode(antecedent));
flowLoopCount--;
// If we see a value appear in the cache it is a sign that control flow analysis
// was restarted and completed by checkExpressionCached. We can simply pick up
Expand Down
10 changes: 10 additions & 0 deletions src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1606,6 +1606,16 @@ namespace ts {
antecedent: FlowNode;
}

export type FlowType = Type | IncompleteType;

// Incomplete types occur during control flow analysis of loops. An IncompleteType
// is distinguished from a regular type by a flags value of zero. Incomplete type
// objects are internal to the getFlowTypeOfRefecence function and never escape it.
export interface IncompleteType {
flags: TypeFlags; // No flags set
Copy link
Member

@weswigham weswigham Aug 3, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we have numeric literal types and enum literal types now, shouldn't this just be TypeFlags.None or 0? (Which would alleviate the need for the long explanatory comment above. And possibly allow you to remove some of the casts you have in the implementation.)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TypeFlags isn't a union enum type (because it contains non-literal initializers and is intended to be used as bit flags), so there is no TypeFlags.None type. I could use 0, but then it would be less obvious that flags is overlaid on the similarly named property in Type. Either way, the comment is still necessary.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IDK, I think 0 carries more meaning; since you're using only a sentinel value for the member, encoding the sentinel value in the type seems like a good documentation choice. And 0 should still be assignable to TypeFlags, so it should still overlay just fine, right?

type: Type; // The type marked incomplete
}

export interface AmdDependency {
path: string;
name: string;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,5 +55,5 @@ function rawr(dino: RexOrRaptor) {
throw "Unexpected " + dino;
>"Unexpected " + dino : string
>"Unexpected " : string
>dino : "t-rex"
>dino : never
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ if (typeof stringOrNumber === "number") {
>"number" : "number"

stringOrNumber;
>stringOrNumber : string
>stringOrNumber : never
}
}

Expand All @@ -31,6 +31,6 @@ if (typeof stringOrNumber === "number" && typeof stringOrNumber !== "number") {
>"number" : "number"

stringOrNumber;
>stringOrNumber : string
>stringOrNumber : never
}

6 changes: 3 additions & 3 deletions tests/baselines/reference/typeGuardTypeOfUndefined.types
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ function test2(a: any) {
>"boolean" : "boolean"

a;
>a : boolean
>a : never
}
else {
a;
Expand Down Expand Up @@ -129,7 +129,7 @@ function test5(a: boolean | void) {
}
else {
a;
>a : void
>a : never
}
}
else {
Expand Down Expand Up @@ -188,7 +188,7 @@ function test7(a: boolean | void) {
}
else {
a;
>a : void
>a : never
}
}

Expand Down
12 changes: 6 additions & 6 deletions tests/baselines/reference/typeGuardsAsAssertions.types
Original file line number Diff line number Diff line change
Expand Up @@ -193,10 +193,10 @@ function f1() {
>x : undefined

x; // string | number (guard as assertion)
>x : string | number
>x : never
}
x; // string | number | undefined
>x : string | number | undefined
>x : undefined
}

function f2() {
Expand All @@ -216,10 +216,10 @@ function f2() {
>"string" : "string"

x; // string (guard as assertion)
>x : string
>x : never
}
x; // string | undefined
>x : string | undefined
>x : undefined
}

function f3() {
Expand All @@ -239,7 +239,7 @@ function f3() {
return;
}
x; // string | number (guard as assertion)
>x : string | number
>x : never
}

function f4() {
Expand Down Expand Up @@ -281,7 +281,7 @@ function f5(x: string | number) {
>"number" : "number"

x; // number (guard as assertion)
>x : number
>x : never
}
else {
x; // string | number
Expand Down
5 changes: 4 additions & 1 deletion tests/baselines/reference/typeGuardsInIfStatement.errors.txt
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
tests/cases/conformance/expressions/typeGuards/typeGuardsInIfStatement.ts(22,10): error TS2354: No best common type exists among return expressions.
tests/cases/conformance/expressions/typeGuards/typeGuardsInIfStatement.ts(31,10): error TS2354: No best common type exists among return expressions.
tests/cases/conformance/expressions/typeGuards/typeGuardsInIfStatement.ts(49,10): error TS2354: No best common type exists among return expressions.
tests/cases/conformance/expressions/typeGuards/typeGuardsInIfStatement.ts(139,17): error TS2339: Property 'toString' does not exist on type 'never'.


==== tests/cases/conformance/expressions/typeGuards/typeGuardsInIfStatement.ts (3 errors) ====
==== tests/cases/conformance/expressions/typeGuards/typeGuardsInIfStatement.ts (4 errors) ====
// In the true branch statement of an 'if' statement,
// the type of a variable or parameter is narrowed by any type guard in the 'if' condition when true.
// In the false branch statement of an 'if' statement,
Expand Down Expand Up @@ -149,5 +150,7 @@ tests/cases/conformance/expressions/typeGuards/typeGuardsInIfStatement.ts(49,10)
return typeof x === "number"
? x.toString() // number
: x.toString(); // boolean | string
~~~~~~~~
!!! error TS2339: Property 'toString' does not exist on type 'never'.
}
}