Skip to content

Adds empty object narrowing to type guards #4051

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

Closed
wants to merge 5 commits into from
Closed

Adds empty object narrowing to type guards #4051

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
54efca0
Adds void narrowing tests
tinganho Jul 28, 2015
005bee5
Adds void narrowing
tinganho Jul 28, 2015
f052e02
Fixes some comment issues
tinganho Jul 29, 2015
b02f039
Addresses CR feedback
tinganho Jul 29, 2015
fe266b0
Change exhuast type to empty object type
tinganho Aug 5, 2015
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
121 changes: 104 additions & 17 deletions src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2190,7 +2190,7 @@ namespace ts {

/**
* Push an entry on the type resolution stack. If an entry with the given target and the given property name
* is already on the stack, and no entries in between already have a type, then a circularity has occurred.
* is already on the stack, and no entries in between already have a type, then a circularity has occurred.
* In this case, the result values of the existing entry and all entries pushed after it are changed to false,
* and the value false is returned. Otherwise, the new entry is just pushed onto the stack, and true is returned.
* In order to see if the same query has already been done before, the target object and the propertyName both
Expand Down Expand Up @@ -2360,7 +2360,7 @@ namespace ts {
if (isBindingPattern(declaration.parent)) {
return getTypeForBindingElement(<BindingElement>declaration);
}

// Use type from type annotation if one is present
if (declaration.type) {
return getTypeFromTypeNode(declaration.type);
Expand All @@ -2381,12 +2381,12 @@ namespace ts {
return type;
}
}

// Use the type of the initializer expression if one is present
if (declaration.initializer) {
return checkExpressionCached(declaration.initializer);
}

// If it is a short-hand property assignment, use the type of the identifier
if (declaration.kind === SyntaxKind.ShorthandPropertyAssignment) {
return checkIdentifier(<Identifier>declaration.name);
Expand Down Expand Up @@ -2482,10 +2482,10 @@ namespace ts {
// tools see the actual type.
return declaration.kind !== SyntaxKind.PropertyAssignment ? getWidenedType(type) : type;
}

// Rest parameters default to type any[], other parameters default to type any
type = declaration.dotDotDotToken ? anyArrayType : anyType;

// Report implicit any errors unless this is a private property within an ambient declaration
if (reportErrors && compilerOptions.noImplicitAny) {
let root = getRootDeclaration(declaration);
Expand Down Expand Up @@ -4364,11 +4364,11 @@ namespace ts {
return getInferredType(context, i);
}
}
return t;
return t;
};

mapper.context = context;
return mapper;
return mapper;
}

function identityMapper(type: Type): Type {
Expand Down Expand Up @@ -5051,7 +5051,7 @@ namespace ts {
let saveErrorInfo = errorInfo;

// Because the "abstractness" of a class is the same across all construct signatures
// (internally we are checking the corresponding declaration), it is enough to perform
// (internally we are checking the corresponding declaration), it is enough to perform
// the check and report an error once over all pairs of source and target construct signatures.
let sourceSig = sourceSignatures[0];
// Note that in an extends-clause, targetSignatures is stripped, so the check never proceeds.
Expand Down Expand Up @@ -5997,12 +5997,18 @@ namespace ts {
}
}



// Get the narrowed type of a given symbol at a given location
function getNarrowedTypeOfSymbol(symbol: Symbol, node: Node) {
let type = getTypeOfSymbol(symbol);
// Only narrow when symbol is variable of type any or an object, union, or type parameter type
if (node && symbol.flags & SymbolFlags.Variable) {
if (isTypeAny(type) || type.flags & (TypeFlags.ObjectType | TypeFlags.Union | TypeFlags.TypeParameter)) {
// `firstNarrowedTypeFromIfStatement` is only used when type is of union type.
let originalType = type;
let firstNarrowedTypeFromIfStatement: Type;

loop: while (node.parent) {
let child = node;
node = node.parent;
Expand All @@ -6011,7 +6017,88 @@ namespace ts {
case SyntaxKind.IfStatement:
// In a branch of an if statement, narrow based on controlling expression
if (child !== (<IfStatement>node).expression) {

// We want to narrow a type to an empty object type `{}` when we exhaust all types.
// We can accomplish it by simply check if a clause return the same type as the
// ongoing narrowing type. Given that the ongoing narrowing type have been narrowed
// to one distinct type.
//
// Example:
//
// let x: A | B | C;
//
// if (isA(x)) { // isA(...) clause returns A and the ongoing narrowing type has
// // the value of A and it is a distinct type so narrow x to an
// // empty object type.
//
// }
// else if (isB(x)) { // narrow x to A
// }
// else if (isC(x)) // narrow x to A | B
// }
// else { // x begins with A | B | C
//
// x // an empty object type
// }
//
if (!(type.flags & TypeFlags.Union)) {
if (originalType.flags & TypeFlags.Union &&
// Use the original type to check the narrowed type instead of the ongoing narrowing type.
// This is to deal with cases like below:
//
// let x: A | B | C;
//
// if (isB(x)) { // narrow x to A.
Copy link
Member

Choose a reason for hiding this comment

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

Do you mean A | C?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No I meant A. It remove B from A(which is A).

// }
// else if (isB(x)) { // narrow x to A
// }
// else if (isC(x)) { // narrow x to A | B
// }
// else { // x begins with A | B | C
// x
// }
//
// Which is the same as:
//
// let x: A | B | C;
//
// if (isA(x)) { // narrow x to A (but also removes A).
// }
// else if (isB(x)) { // narrow x to A
Copy link
Member

Choose a reason for hiding this comment

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

Do you mean "to B"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No it removes B from A | B.

// }
// else if (isC(x)) {// narrow x to A | B
Copy link
Member

Choose a reason for hiding this comment

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

Why "A | B"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It removes C from A | B | C.

Copy link
Member

Choose a reason for hiding this comment

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

But at this point you're already narrowed down to C.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm referring to x in the else clause below. So it narrows it to A | B by removing C from A | B | C. If I was referring to a x inside this else if(isC(x)) then it will narrow to C.

// }
// else { //x begins with A | B | C
// x
// }
//
// But we want only the latter to narrow the type to an empty object type.
//
type === narrowType(originalType, (<IfStatement>node).expression, /*assumeTrue*/ true) &&
// If the first narrowed type yield a distinct type(not a union type). And it encounters
// a narrowed type that yields the same type in some upper branches. It would then narrow
// the type to an empty object type instead of the distinct type. The below check, guards
// us from narrowing it to an empty object type instead of the distinct type.
//
// if (isA(x)) { // narrow x to an empty object type
// }
// else if (isB(x)) { // narrow x to A
Copy link
Member

Choose a reason for hiding this comment

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

Why "to A"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The first else if clause below narrow the union type to A. This else if clause remove B from A.

// }
// else if (isA(x)) { // narrow x to A
//
// x // is an empty object type, but should be A
// }
//
type !== firstNarrowedTypeFromIfStatement) {

return emptyObjectType;
}
}

narrowedType = narrowType(type, (<IfStatement>node).expression, /*assumeTrue*/ child === (<IfStatement>node).thenStatement);
if (type.flags & TypeFlags.Union && !firstNarrowedTypeFromIfStatement) {
firstNarrowedTypeFromIfStatement = narrowedType;
}
}
break;
case SyntaxKind.ConditionalExpression:
Expand Down Expand Up @@ -10465,7 +10552,7 @@ namespace ts {
}
if (!superCallStatement) {
error(node, Diagnostics.A_super_call_must_be_the_first_statement_in_the_constructor_when_a_class_contains_initialized_properties_or_has_parameter_properties);
}
}
else {
// In such a required super call, it is a compile-time error for argument expressions to reference this.
markThisReferencesAsErrors(superCallStatement.expression);
Expand Down Expand Up @@ -13068,7 +13155,7 @@ namespace ts {
Debug.assert(node.kind === SyntaxKind.Identifier);
return <Identifier>node;
}

function checkExternalImportOrExportDeclaration(node: ImportDeclaration | ImportEqualsDeclaration | ExportDeclaration): boolean {
let moduleName = getExternalModuleName(node);
if (!nodeIsMissing(moduleName) && moduleName.kind !== SyntaxKind.StringLiteral) {
Expand Down Expand Up @@ -13679,7 +13766,7 @@ namespace ts {
case SyntaxKind.ClassDeclaration:
case SyntaxKind.InterfaceDeclaration:
// If we didn't come from static member of class or interface,
// add the type parameters into the symbol table
// add the type parameters into the symbol table
// (type parameters of classDeclaration/classExpression and interface are in member property of the symbol.
// Note: that the memberFlags come from previous iteration.
if (!(memberFlags & NodeFlags.Static)) {
Expand Down Expand Up @@ -14160,9 +14247,9 @@ namespace ts {
}
// const enums and modules that contain only const enums are not considered values from the emit perespective
// unless 'preserveConstEnums' option is set to true
return target !== unknownSymbol &&
target &&
target.flags & SymbolFlags.Value &&
return target !== unknownSymbol &&
target &&
target.flags & SymbolFlags.Value &&
(compilerOptions.preserveConstEnums || !isConstEnumOrConstEnumOnlyModule(target));
}

Expand Down Expand Up @@ -14233,7 +14320,7 @@ namespace ts {
function isFunctionType(type: Type): boolean {
return type.flags & TypeFlags.ObjectType && getSignaturesOfType(type, SignatureKind.Call).length > 0;
}

function getTypeReferenceSerializationKind(node: TypeReferenceNode): TypeReferenceSerializationKind {
// Resolve the symbol as a value to ensure the type can be reached at runtime during emit.
let symbol = resolveEntityName(node.typeName, SymbolFlags.Value, /*ignoreErrors*/ true);
Expand Down
144 changes: 144 additions & 0 deletions tests/baselines/reference/typeGuardNarrowingToVoidType.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,144 @@
//// [typeGuardNarrowingToVoidType.ts]
let x: number | string | boolean;

if (typeof x === "number") {
x.toPrecision();
}
else if (typeof x === "string") {
x.charCodeAt(1);
}
else if(typeof x === "boolean") {
x.valueOf();
}
else {
x
}

if(typeof x === "boolean") {
x.valueOf();
}
else if (typeof x === "number") {
x.toPrecision();
}
else if (typeof x === "string") {
x.charCodeAt(1);
}
else if(typeof x === "boolean") {
x.valueOf();
}
else {
x
}

class A { a: string; }
class B { b: string; }
class C { c: string; }

declare function isA(x: any): x is A;
declare function isB(x: any): x is B;
declare function isC(x: any): x is C;

let y: A | B | C;

if (isA(y)) {
y.a;
}
else if(isB(y)){
y.b;
}
else if(isC(y)) {
y.c;
}
else{
y
}

if (isB(y)) {
y.b;
}
else if (isA(y)) {
y.a;
}
else if(isB(y)){
y.b;
}
else if(isC(y)) {
y.c;
}
else{
y
}


//// [typeGuardNarrowingToVoidType.js]
var x;
if (typeof x === "number") {
x.toPrecision();
}
else if (typeof x === "string") {
x.charCodeAt(1);
}
else if (typeof x === "boolean") {
x.valueOf();
}
else {
x;
}
if (typeof x === "boolean") {
x.valueOf();
}
else if (typeof x === "number") {
x.toPrecision();
}
else if (typeof x === "string") {
x.charCodeAt(1);
}
else if (typeof x === "boolean") {
x.valueOf();
}
else {
x;
}
var A = (function () {
function A() {
}
return A;
})();
var B = (function () {
function B() {
}
return B;
})();
var C = (function () {
function C() {
}
return C;
})();
var y;
if (isA(y)) {
y.a;
}
else if (isB(y)) {
y.b;
}
else if (isC(y)) {
y.c;
}
else {
y;
}
if (isB(y)) {
y.b;
}
else if (isA(y)) {
y.a;
}
else if (isB(y)) {
y.b;
}
else if (isC(y)) {
y.c;
}
else {
y;
}
Loading