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.

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

Improve type guards for type variables #15576

Merged
merged 5 commits into from
May 7, 2017
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
43 changes: 34 additions & 9 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5775,11 +5775,11 @@ namespace ts {
const t = type.flags & TypeFlags.TypeVariable ? getBaseConstraintOfType(type) || emptyObjectType : type;
return t.flags & TypeFlags.Intersection ? getApparentTypeOfIntersectionType(<IntersectionType>t) :
t.flags & TypeFlags.StringLike ? globalStringType :
t.flags & TypeFlags.NumberLike ? globalNumberType :
t.flags & TypeFlags.BooleanLike ? globalBooleanType :
t.flags & TypeFlags.ESSymbol ? getGlobalESSymbolType(/*reportErrors*/ languageVersion >= ScriptTarget.ES2015) :
t.flags & TypeFlags.NonPrimitive ? emptyObjectType :
t;
t.flags & TypeFlags.NumberLike ? globalNumberType :
t.flags & TypeFlags.BooleanLike ? globalBooleanType :
t.flags & TypeFlags.ESSymbol ? getGlobalESSymbolType(/*reportErrors*/ languageVersion >= ScriptTarget.ES2015) :
t.flags & TypeFlags.NonPrimitive ? emptyObjectType :
t;
}

function createUnionOrIntersectionProperty(containingType: UnionOrIntersectionType, name: string): Symbol {
Expand Down Expand Up @@ -10439,11 +10439,13 @@ namespace ts {
// Return the flow cache key for a "dotted name" (i.e. a sequence of identifiers
// separated by dots). The key consists of the id of the symbol referenced by the
// leftmost identifier followed by zero or more property names separated by dots.
// The result is undefined if the reference isn't a dotted name.
// The result is undefined if the reference isn't a dotted name. We prefix nodes
// occurring in an apparent type position with '@' because the control flow type
// of such nodes may be based on the apparent type instead of the declared type.
function getFlowCacheKey(node: Node): string {
if (node.kind === SyntaxKind.Identifier) {
const symbol = getResolvedSymbol(<Identifier>node);
return symbol !== unknownSymbol ? "" + getSymbolId(symbol) : undefined;
return symbol !== unknownSymbol ? (isApparentTypePosition(node) ? "@" : "") + getSymbolId(symbol) : undefined;
}
if (node.kind === SyntaxKind.ThisKeyword) {
return "0";
Expand Down Expand Up @@ -11708,6 +11710,29 @@ namespace ts {
return annotationIncludesUndefined ? getTypeWithFacts(declaredType, TypeFacts.NEUndefined) : declaredType;
}

function isApparentTypePosition(node: Node) {
const parent = node.parent;
return parent.kind === SyntaxKind.PropertyAccessExpression ||
Copy link
Contributor

Choose a reason for hiding this comment

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

&& (<PropertyAccessExpression >parent).expression === node; ?

Copy link
Member Author

Choose a reason for hiding this comment

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

No need to. This function is only ever called on the left hand side of a property access expression (the right hand side is just an identifier and not actually an expression).

parent.kind === SyntaxKind.CallExpression && (<CallExpression>parent).expression === node ||
parent.kind === SyntaxKind.ElementAccessExpression && (<ElementAccessExpression>parent).expression === node;
}

function typeHasNullableConstraint(type: Type) {
return type.flags & TypeFlags.TypeVariable && maybeTypeOfKind(getBaseConstraintOfType(type) || emptyObjectType, TypeFlags.Nullable);
}

function getDeclaredOrApparentType(symbol: Symbol, 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);
}
return type;
}

function checkIdentifier(node: Identifier): Type {
const symbol = getResolvedSymbol(node);
if (symbol === unknownSymbol) {
Expand Down Expand Up @@ -11783,7 +11808,7 @@ namespace ts {
checkCollisionWithCapturedNewTargetVariable(node, node);
checkNestedBlockScopedBinding(node, symbol);

const type = getTypeOfSymbol(localOrExportSymbol);
const type = getDeclaredOrApparentType(localOrExportSymbol, node);
const declaration = localOrExportSymbol.valueDeclaration;
const assignmentKind = getAssignmentTargetKind(node);

Expand Down Expand Up @@ -14141,7 +14166,7 @@ namespace ts {

checkPropertyAccessibility(node, left, apparentType, prop);

const propType = getTypeOfSymbol(prop);
const propType = getDeclaredOrApparentType(prop, node);
const assignmentKind = getAssignmentTargetKind(node);

if (assignmentKind) {
Expand Down
158 changes: 158 additions & 0 deletions tests/baselines/reference/typeVariableTypeGuards.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,158 @@
//// [typeVariableTypeGuards.ts]
// Repro from #14091

interface Foo {
foo(): void
}

class A<P extends Partial<Foo>> {
props: Readonly<P>
doSomething() {
this.props.foo && this.props.foo()
}
}

// Repro from #14415

interface Banana {
color: 'yellow';
}

class Monkey<T extends Banana | undefined> {
a: T;
render() {
if (this.a) {
this.a.color;
}
}
}

interface BigBanana extends Banana {
}

class BigMonkey extends Monkey<BigBanana> {
render() {
if (this.a) {
this.a.color;
}
}
}

// Another repro

type Item = {
(): string;
x: string;
}

function f1<T extends Item | undefined>(obj: T) {
if (obj) {
obj.x;
obj["x"];
obj();
}
}

function f2<T extends Item | undefined>(obj: T | undefined) {
if (obj) {
obj.x;
obj["x"];
obj();
}
}

function f3<T extends Item | undefined>(obj: T | null) {
if (obj) {
obj.x;
obj["x"];
obj();
}
}

function f4<T extends string[] | undefined>(obj: T | undefined, x: number) {
if (obj) {
obj[x].length;
}
}

function f5<T, K extends keyof T>(obj: T | undefined, key: K) {
if (obj) {
obj[key];
}
}


//// [typeVariableTypeGuards.js]
"use strict";
// Repro from #14091
var __extends = (this && this.__extends) || (function () {
var extendStatics = Object.setPrototypeOf ||
({ __proto__: [] } instanceof Array && function (d, b) { d.__proto__ = b; }) ||
function (d, b) { for (var p in b) if (b.hasOwnProperty(p)) d[p] = b[p]; };
return function (d, b) {
extendStatics(d, b);
function __() { this.constructor = d; }
d.prototype = b === null ? Object.create(b) : (__.prototype = b.prototype, new __());
};
})();
var A = (function () {
function A() {
}
A.prototype.doSomething = function () {
this.props.foo && this.props.foo();
};
return A;
}());
var Monkey = (function () {
function Monkey() {
}
Monkey.prototype.render = function () {
if (this.a) {
this.a.color;
}
};
return Monkey;
}());
var BigMonkey = (function (_super) {
__extends(BigMonkey, _super);
function BigMonkey() {
return _super !== null && _super.apply(this, arguments) || this;
}
BigMonkey.prototype.render = function () {
if (this.a) {
this.a.color;
}
};
return BigMonkey;
}(Monkey));
function f1(obj) {
if (obj) {
obj.x;
obj["x"];
obj();
}
}
function f2(obj) {
if (obj) {
obj.x;
obj["x"];
obj();
}
}
function f3(obj) {
if (obj) {
obj.x;
obj["x"];
obj();
}
}
function f4(obj, x) {
if (obj) {
obj[x].length;
}
}
function f5(obj, key) {
if (obj) {
obj[key];
}
}
Loading