Skip to content

Commit 95584e9

Browse files
ahejlsbergahejlsberg
committed
Addressing CR feedback
1 parent d70494f commit 95584e9

File tree

1 file changed

+66
-41
lines changed

1 file changed

+66
-41
lines changed

‎src/compiler/checker.ts

Lines changed: 66 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -1157,7 +1157,7 @@ module ts {
11571157
}
11581158
}
11591159

1160-
function writeTypeList(types: Type[], union?: boolean) {
1160+
function writeTypeList(types: Type[], union: boolean) {
11611161
for (var i = 0; i < types.length; i++) {
11621162
if (i > 0) {
11631163
if (union) {
@@ -1166,6 +1166,8 @@ module ts {
11661166
writePunctuation(writer, union ? SyntaxKind.BarToken : SyntaxKind.CommaToken);
11671167
writeSpace(writer);
11681168
}
1169+
// Don't output function type literals in unions because '() => string | () => number' would be parsed
1170+
// as a function type that returns a union type. Instead output '{ (): string; } | { (): number; }'.
11691171
writeType(types[i], /*allowFunctionOrConstructorTypeLiteral*/ !union);
11701172
}
11711173
}
@@ -1181,14 +1183,14 @@ module ts {
11811183
else {
11821184
writeSymbol(type.target.symbol, writer, enclosingDeclaration, SymbolFlags.Type);
11831185
writePunctuation(writer, SyntaxKind.LessThanToken);
1184-
writeTypeList(type.typeArguments);
1186+
writeTypeList(type.typeArguments, /*union*/ false);
11851187
writePunctuation(writer, SyntaxKind.GreaterThanToken);
11861188
}
11871189
}
11881190

11891191
function writeTupleType(type: TupleType) {
11901192
writePunctuation(writer, SyntaxKind.OpenBracketToken);
1191-
writeTypeList(type.elementTypes);
1193+
writeTypeList(type.elementTypes, /*union*/ false);
11921194
writePunctuation(writer, SyntaxKind.CloseBracketToken);
11931195
}
11941196

@@ -1744,7 +1746,7 @@ module ts {
17441746
function getTypeOfUnionProperty(symbol: Symbol): Type {
17451747
var links = getSymbolLinks(symbol);
17461748
if (!links.type) {
1747-
var types = map(links.unionType.types, t => getTypeOfSymbol(getPropertyOfType(getApparentType(t), symbol.name) || undefinedSymbol));
1749+
var types = map(links.unionType.types, t => getTypeOfSymbol(getPropertyOfType(getApparentType(t), symbol.name)));
17481750
links.type = getUnionType(types);
17491751
}
17501752
return links.type;
@@ -2069,25 +2071,37 @@ module ts {
20692071
}
20702072

20712073
function signatureListsIdentical(s: Signature[], t: Signature[]): boolean {
2072-
if (s.length !== t.length) return false;
2074+
if (s.length !== t.length) {
2075+
return false;
2076+
}
20732077
for (var i = 0; i < s.length; i++) {
2074-
if (!compareSignatures(s[i], t[i], false, isTypeIdenticalTo)) return false;
2078+
if (!compareSignatures(s[i], t[i], /*compareReturnTypes*/ false, isTypeIdenticalTo)) {
2079+
return false;
2080+
}
20752081
}
20762082
return true;
20772083
}
20782084

2085+
// If the lists of call or construct signatures in the given types are all identical except for return types,
2086+
// and if none of the signatures are generic, return a list of signatures that has substitutes a union of the
2087+
// return types of the corresponding signatures in each resulting signature.
20792088
function getUnionSignatures(types: Type[], kind: SignatureKind): Signature[] {
20802089
var signatureLists = map(types, t => getSignaturesOfType(t, kind));
2081-
var baseSignatures = signatureLists[0];
2082-
for (var i = 0; i < baseSignatures.length; i++) {
2083-
if (baseSignatures[i].typeParameters) return emptyArray;
2090+
var signatures = signatureLists[0];
2091+
for (var i = 0; i < signatures.length; i++) {
2092+
if (signatures[i].typeParameters) {
2093+
return emptyArray;
2094+
}
20842095
}
20852096
for (var i = 1; i < signatureLists.length; i++) {
2086-
if (!signatureListsIdentical(baseSignatures, signatureLists[i])) return emptyArray;
2097+
if (!signatureListsIdentical(signatures, signatureLists[i])) {
2098+
return emptyArray;
2099+
}
20872100
}
2088-
var result = map(baseSignatures, cloneSignature);
2101+
var result = map(signatures, cloneSignature);
20892102
for (var i = 0; i < result.length; i++) {
20902103
var s = result[i];
2104+
// Clear resolved return type we possibly got from cloneSignature
20912105
s.resolvedReturnType = undefined;
20922106
s.unionSignatures = map(signatureLists, signatures => signatures[i]);
20932107
}
@@ -2098,7 +2112,9 @@ module ts {
20982112
var indexTypes: Type[] = [];
20992113
for (var i = 0; i < types.length; i++) {
21002114
var indexType = getIndexTypeOfType(types[i], kind);
2101-
if (!indexType) return undefined;
2115+
if (!indexType) {
2116+
return undefined;
2117+
}
21022118
indexTypes.push(indexType);
21032119
}
21042120
return getUnionType(indexTypes);
@@ -2113,14 +2129,16 @@ module ts {
21132129
}
21142130
});
21152131
if (types.length <= 1) {
2116-
var res = types.length ? resolveObjectTypeMembers(types[0]) : emptyObjectType;
2117-
setObjectTypeMembers(type, res.members, res.callSignatures, res.constructSignatures, res.stringIndexType, res.numberIndexType);
2132+
var resolved = types.length ? resolveObjectTypeMembers(types[0]) : emptyObjectType;
2133+
setObjectTypeMembers(type, resolved.members, resolved.callSignatures, resolved.constructSignatures, resolved.stringIndexType, resolved.numberIndexType);
21182134
return;
21192135
}
21202136
var members: SymbolTable = {};
21212137
forEach(getPropertiesOfType(types[0]), prop => {
21222138
for (var i = 1; i < types.length; i++) {
2123-
if (!getPropertyOfType(types[i], prop.name)) return;
2139+
if (!getPropertyOfType(types[i], prop.name)) {
2140+
return;
2141+
}
21242142
}
21252143
var symbol = <TransientSymbol>createSymbol(SymbolFlags.UnionProperty | SymbolFlags.Transient, prop.name);
21262144
symbol.unionType = type;
@@ -2662,7 +2680,9 @@ module ts {
26622680
else {
26632681
var i = 0;
26642682
var id = type.id;
2665-
while (i < sortedSet.length && sortedSet[i].id < id) i++;
2683+
while (i < sortedSet.length && sortedSet[i].id < id) {
2684+
i++;
2685+
}
26662686
if (i === sortedSet.length || sortedSet[i].id !== id) {
26672687
sortedSet.splice(i, 0, type);
26682688
}
@@ -2677,7 +2697,9 @@ module ts {
26772697

26782698
function isSubtypeOfAny(candidate: Type, types: Type[]): boolean {
26792699
for (var i = 0, len = types.length; i < len; i++) {
2680-
if (candidate !== types[i] && isTypeSubtypeOf(candidate, types[i])) return true;
2700+
if (candidate !== types[i] && isTypeSubtypeOf(candidate, types[i])) {
2701+
return true;
2702+
}
26812703
}
26822704
return false;
26832705
}
@@ -3467,7 +3489,7 @@ module ts {
34673489
return false;
34683490
}
34693491
for (var i = 0, len = sourceSignatures.length; i < len; ++i) {
3470-
if (!compareSignatures(sourceSignatures[i], targetSignatures[i], /*returnTypes*/ true, isRelatedTo)) {
3492+
if (!compareSignatures(sourceSignatures[i], targetSignatures[i], /*compareReturnTypes*/ true, isRelatedTo)) {
34713493
return false;
34723494
}
34733495
}
@@ -3535,7 +3557,7 @@ module ts {
35353557
}
35363558
}
35373559

3538-
function compareSignatures(source: Signature, target: Signature, returnTypes: boolean, compareTypes: (s: Type, t: Type) => boolean): boolean {
3560+
function compareSignatures(source: Signature, target: Signature, compareReturnTypes: boolean, compareTypes: (s: Type, t: Type) => boolean): boolean {
35393561
if (source === target) {
35403562
return true;
35413563
}
@@ -3568,7 +3590,7 @@ module ts {
35683590
return false;
35693591
}
35703592
}
3571-
return !returnTypes || compareTypes(getReturnTypeOfSignature(source), getReturnTypeOfSignature(target));
3593+
return !compareReturnTypes || compareTypes(getReturnTypeOfSignature(source), getReturnTypeOfSignature(target));
35723594
}
35733595

35743596
function isSupertypeOfEach(candidate: Type, types: Type[]): boolean {
@@ -3876,20 +3898,14 @@ module ts {
38763898
if (type.flags & TypeFlags.Union) {
38773899
var types = (<UnionType>type).types;
38783900
if (forEach(types, t => t.flags & subtractMask)) {
3879-
var newTypes: Type[] = [];
3880-
forEach(types, t => {
3881-
if (!(t.flags & subtractMask)) {
3882-
newTypes.push(t);
3883-
}
3884-
});
3885-
return getUnionType(newTypes);
3901+
return getUnionType(filter(types, t => !(t.flags & subtractMask)));
38863902
}
38873903
}
38883904
return type;
38893905
}
38903906

38913907
// Check if a given variable is assigned within a given syntax node
3892-
function IsVariableAssignedWithin(symbol: Symbol, node: Node): boolean {
3908+
function isVariableAssignedWithin(symbol: Symbol, node: Node): boolean {
38933909
var links = getNodeLinks(node);
38943910
if (links.assignmentChecks) {
38953911
var cachedResult = links.assignmentChecks[symbol.id];
@@ -3981,29 +3997,32 @@ module ts {
39813997
case SyntaxKind.IfStatement:
39823998
// In a branch of an if statement, narrow based on controlling expression
39833999
if (child !== (<IfStatement>node).expression) {
3984-
narrowedType = narrowType(type, (<IfStatement>node).expression, child === (<IfStatement>node).thenStatement);
4000+
narrowedType = narrowType(type, (<IfStatement>node).expression, /*assumeTrue*/ child === (<IfStatement>node).thenStatement);
39854001
}
39864002
break;
39874003
case SyntaxKind.ConditionalExpression:
39884004
// In a branch of a conditional expression, narrow based on controlling condition
39894005
if (child !== (<ConditionalExpression>node).condition) {
3990-
narrowedType = narrowType(type, (<ConditionalExpression>node).condition, child === (<ConditionalExpression>node).whenTrue);
4006+
narrowedType = narrowType(type, (<ConditionalExpression>node).condition, /*assumeTrue*/ child === (<ConditionalExpression>node).whenTrue);
39914007
}
39924008
break;
39934009
case SyntaxKind.BinaryExpression:
39944010
// In the right operand of an && or ||, narrow based on left operand
39954011
if (child === (<BinaryExpression>node).right) {
39964012
if ((<BinaryExpression>node).operator === SyntaxKind.AmpersandAmpersandToken) {
3997-
narrowedType = narrowType(type, (<BinaryExpression>node).left, true);
4013+
narrowedType = narrowType(type, (<BinaryExpression>node).left, /*assumeTrue*/ true);
39984014
}
39994015
else if ((<BinaryExpression>node).operator === SyntaxKind.BarBarToken) {
4000-
narrowedType = narrowType(type, (<BinaryExpression>node).left, false);
4016+
narrowedType = narrowType(type, (<BinaryExpression>node).left, /*assumeTrue*/ false);
40014017
}
40024018
}
40034019
break;
40044020
}
40054021
// Only use narrowed type if construct contains no assignments to variable
4006-
if (narrowedType !== type && !IsVariableAssignedWithin(symbol, node)) {
4022+
if (narrowedType !== type) {
4023+
if (isVariableAssignedWithin(symbol, node)) {
4024+
break;
4025+
}
40074026
type = narrowedType;
40084027
}
40094028
}
@@ -4039,30 +4058,36 @@ module ts {
40394058
function narrowTypeByAnd(type: Type, expr: BinaryExpression, assumeTrue: boolean): Type {
40404059
if (assumeTrue) {
40414060
// The assumed result is true, therefore we narrow assuming each operand to be true.
4042-
return narrowType(narrowType(type, expr.left, true), expr.right, true);
4061+
return narrowType(narrowType(type, expr.left, /*assumeTrue*/ true), expr.right, /*assumeTrue*/ true);
40434062
}
40444063
else {
4045-
// The assumed result is true. This means either the first operand was false, or the first operand was true
4064+
// The assumed result is false. This means either the first operand was false, or the first operand was true
40464065
// and the second operand was false. We narrow with those assumptions and union the two resulting types.
4047-
return getUnionType([narrowType(type, expr.left, false), narrowType(narrowType(type, expr.left, true), expr.right, false)]);
4066+
return getUnionType([
4067+
narrowType(type, expr.left, /*assumeTrue*/ false),
4068+
narrowType(narrowType(type, expr.left, /*assumeTrue*/ true), expr.right, /*assumeTrue*/ false)
4069+
]);
40484070
}
40494071
}
40504072

40514073
function narrowTypeByOr(type: Type, expr: BinaryExpression, assumeTrue: boolean): Type {
40524074
if (assumeTrue) {
40534075
// The assumed result is true. This means either the first operand was true, or the first operand was false
40544076
// and the second operand was true. We narrow with those assumptions and union the two resulting types.
4055-
return getUnionType([narrowType(type, expr.left, true), narrowType(narrowType(type, expr.left, false), expr.right, true)]);
4077+
return getUnionType([
4078+
narrowType(type, expr.left, /*assumeTrue*/ true),
4079+
narrowType(narrowType(type, expr.left, /*assumeTrue*/ false), expr.right, /*assumeTrue*/ true)
4080+
]);
40564081
}
40574082
else {
40584083
// The assumed result is false, therefore we narrow assuming each operand to be false.
4059-
return narrowType(narrowType(type, expr.left, false), expr.right, false);
4084+
return narrowType(narrowType(type, expr.left, /*assumeTrue*/ false), expr.right, /*assumeTrue*/ false);
40604085
}
40614086
}
40624087

40634088
function narrowTypeByInstanceof(type: Type, expr: BinaryExpression, assumeTrue: boolean): Type {
4064-
// Check that we have variable symbol on the left
4065-
if (expr.left.kind !== SyntaxKind.Identifier || getResolvedSymbol(<Identifier>expr.left) !== symbol) {
4089+
// Check that assumed result is true and we have variable symbol on the left
4090+
if (!assumeTrue || expr.left.kind !== SyntaxKind.Identifier || getResolvedSymbol(<Identifier>expr.left) !== symbol) {
40664091
return type;
40674092
}
40684093
// Check that right operand is a function type with a prototype property
@@ -7198,7 +7223,7 @@ module ts {
71987223
return checkArrayType(<ArrayTypeNode>node);
71997224
case SyntaxKind.TupleType:
72007225
return checkTupleType(<TupleTypeNode>node);
7201-
case SyntaxKind.TupleType:
7226+
case SyntaxKind.UnionType:
72027227
return checkUnionType(<UnionTypeNode>node);
72037228
case SyntaxKind.FunctionDeclaration:
72047229
return checkFunctionDeclaration(<FunctionDeclaration>node);

0 commit comments

Comments
 (0)