Skip to content

Commit 0bfc83b

Browse files
ahejlsbergahejlsberg
committed
Merge pull request #6379 from Microsoft/forInChecking
Improved checking of for-in statements
2 parents 9b151b3 + a0fcc0f commit 0bfc83b

File tree

69 files changed

+1177
-630
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

69 files changed

+1177
-630
lines changed

‎src/compiler/checker.ts‎

Lines changed: 60 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2520,9 +2520,9 @@ namespace ts {
25202520

25212521
// Return the inferred type for a variable, parameter, or property declaration
25222522
function getTypeForVariableLikeDeclaration(declaration: VariableLikeDeclaration): Type {
2523-
// A variable declared in a for..in statement is always of type any
2523+
// A variable declared in a for..in statement is always of type string
25242524
if (declaration.parent.parent.kind === SyntaxKind.ForInStatement) {
2525-
return anyType;
2525+
return stringType;
25262526
}
25272527

25282528
if (declaration.parent.parent.kind === SyntaxKind.ForOfStatement) {
@@ -8564,6 +8564,56 @@ namespace ts {
85648564
return true;
85658565
}
85668566

8567+
/**
8568+
* Return the symbol of the for-in variable declared or referenced by the given for-in statement.
8569+
*/
8570+
function getForInVariableSymbol(node: ForInStatement): Symbol {
8571+
const initializer = node.initializer;
8572+
if (initializer.kind === SyntaxKind.VariableDeclarationList) {
8573+
const variable = (<VariableDeclarationList>initializer).declarations[0];
8574+
if (variable && !isBindingPattern(variable.name)) {
8575+
return getSymbolOfNode(variable);
8576+
}
8577+
}
8578+
else if (initializer.kind === SyntaxKind.Identifier) {
8579+
return getResolvedSymbol(<Identifier>initializer);
8580+
}
8581+
return undefined;
8582+
}
8583+
8584+
/**
8585+
* Return true if the given type is considered to have numeric property names.
8586+
*/
8587+
function hasNumericPropertyNames(type: Type) {
8588+
return getIndexTypeOfType(type, IndexKind.Number) && !getIndexTypeOfType(type, IndexKind.String);
8589+
}
8590+
8591+
/**
8592+
* Return true if given node is an expression consisting of an identifier (possibly parenthesized)
8593+
* that references a for-in variable for an object with numeric property names.
8594+
*/
8595+
function isForInVariableForNumericPropertyNames(expr: Expression) {
8596+
const e = skipParenthesizedNodes(expr);
8597+
if (e.kind === SyntaxKind.Identifier) {
8598+
const symbol = getResolvedSymbol(<Identifier>e);
8599+
if (symbol.flags & SymbolFlags.Variable) {
8600+
let child: Node = expr;
8601+
let node = expr.parent;
8602+
while (node) {
8603+
if (node.kind === SyntaxKind.ForInStatement &&
8604+
child === (<ForInStatement>node).statement &&
8605+
getForInVariableSymbol(<ForInStatement>node) === symbol &&
8606+
hasNumericPropertyNames(checkExpression((<ForInStatement>node).expression))) {
8607+
return true;
8608+
}
8609+
child = node;
8610+
node = node.parent;
8611+
}
8612+
}
8613+
}
8614+
return false;
8615+
}
8616+
85678617
function checkIndexedAccess(node: ElementAccessExpression): Type {
85688618
// Grammar checking
85698619
if (!node.argumentExpression) {
@@ -8624,7 +8674,7 @@ namespace ts {
86248674
if (isTypeAnyOrAllConstituentTypesHaveKind(indexType, TypeFlags.StringLike | TypeFlags.NumberLike | TypeFlags.ESSymbol)) {
86258675

86268676
// Try to use a number indexer.
8627-
if (isTypeAnyOrAllConstituentTypesHaveKind(indexType, TypeFlags.NumberLike)) {
8677+
if (isTypeAnyOrAllConstituentTypesHaveKind(indexType, TypeFlags.NumberLike) || isForInVariableForNumericPropertyNames(node.argumentExpression)) {
86288678
const numberIndexType = getIndexTypeOfType(objectType, IndexKind.Number);
86298679
if (numberIndexType) {
86308680
return numberIndexType;
@@ -8639,7 +8689,9 @@ namespace ts {
86398689

86408690
// Fall back to any.
86418691
if (compilerOptions.noImplicitAny && !compilerOptions.suppressImplicitAnyIndexErrors && !isTypeAny(objectType)) {
8642-
error(node, Diagnostics.Index_signature_of_object_type_implicitly_has_an_any_type);
8692+
error(node, getIndexTypeOfType(objectType, IndexKind.Number) ?
8693+
Diagnostics.Element_implicitly_has_an_any_type_because_index_expression_is_not_of_type_number :
8694+
Diagnostics.Index_signature_of_object_type_implicitly_has_an_any_type);
86438695
}
86448696

86458697
return anyType;
@@ -12698,7 +12750,8 @@ namespace ts {
1269812750
}
1269912751
// For a binding pattern, validate the initializer and exit
1270012752
if (isBindingPattern(node.name)) {
12701-
if (node.initializer) {
12753+
// Don't validate for-in initializer as it is already an error
12754+
if (node.initializer && node.parent.parent.kind !== SyntaxKind.ForInStatement) {
1270212755
checkTypeAssignableTo(checkExpressionCached(node.initializer), getWidenedTypeForVariableLikeDeclaration(node), node, /*headMessage*/ undefined);
1270312756
checkParameterInitializer(node);
1270412757
}
@@ -12708,7 +12761,8 @@ namespace ts {
1270812761
const type = getTypeOfVariableOrParameterOrProperty(symbol);
1270912762
if (node === symbol.valueDeclaration) {
1271012763
// Node is the primary declaration of the symbol, just validate the initializer
12711-
if (node.initializer) {
12764+
// Don't validate for-in initializer as it is already an error
12765+
if (node.initializer && node.parent.parent.kind !== SyntaxKind.ForInStatement) {
1271212766
checkTypeAssignableTo(checkExpressionCached(node.initializer), type, node, /*headMessage*/ undefined);
1271312767
checkParameterInitializer(node);
1271412768
}

‎src/compiler/diagnosticMessages.json‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2430,6 +2430,10 @@
24302430
"category": "Error",
24312431
"code": 7013
24322432
},
2433+
"Element implicitly has an 'any' type because index expression is not of type 'number'.": {
2434+
"category": "Error",
2435+
"code": 7015
2436+
},
24332437
"Property '{0}' implicitly has type 'any', because its 'set' accessor lacks a type annotation.": {
24342438
"category": "Error",
24352439
"code": 7016

‎tests/baselines/reference/capturedLetConstInLoop1.types‎

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,18 @@
11
=== tests/cases/compiler/capturedLetConstInLoop1.ts ===
22
//==== let
33
for (let x in {}) {
4-
>x : any
4+
>x : string
55
>{} : {}
66

77
(function() { return x});
8-
>(function() { return x}) : () => any
9-
>function() { return x} : () => any
10-
>x : any
8+
>(function() { return x}) : () => string
9+
>function() { return x} : () => string
10+
>x : string
1111

1212
(() => x);
13-
>(() => x) : () => any
14-
>() => x : () => any
15-
>x : any
13+
>(() => x) : () => string
14+
>() => x : () => string
15+
>x : string
1616
}
1717

1818
for (let x of []) {
@@ -216,18 +216,18 @@ for (let y = 0; y < 1; ++y) {
216216

217217
//=========const
218218
for (const x in {}) {
219-
>x : any
219+
>x : string
220220
>{} : {}
221221

222222
(function() { return x});
223-
>(function() { return x}) : () => any
224-
>function() { return x} : () => any
225-
>x : any
223+
>(function() { return x}) : () => string
224+
>function() { return x} : () => string
225+
>x : string
226226

227227
(() => x);
228-
>(() => x) : () => any
229-
>() => x : () => any
230-
>x : any
228+
>(() => x) : () => string
229+
>() => x : () => string
230+
>x : string
231231
}
232232

233233
for (const x of []) {

‎tests/baselines/reference/capturedLetConstInLoop1_ES6.types‎

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,18 @@
11
=== tests/cases/compiler/capturedLetConstInLoop1_ES6.ts ===
22
//==== let
33
for (let x in {}) {
4-
>x : any
4+
>x : string
55
>{} : {}
66

77
(function() { return x});
8-
>(function() { return x}) : () => any
9-
>function() { return x} : () => any
10-
>x : any
8+
>(function() { return x}) : () => string
9+
>function() { return x} : () => string
10+
>x : string
1111

1212
(() => x);
13-
>(() => x) : () => any
14-
>() => x : () => any
15-
>x : any
13+
>(() => x) : () => string
14+
>() => x : () => string
15+
>x : string
1616
}
1717

1818
for (let x of []) {
@@ -216,18 +216,18 @@ for (let y = 0; y < 1; ++y) {
216216

217217
//=========const
218218
for (const x in {}) {
219-
>x : any
219+
>x : string
220220
>{} : {}
221221

222222
(function() { return x});
223-
>(function() { return x}) : () => any
224-
>function() { return x} : () => any
225-
>x : any
223+
>(function() { return x}) : () => string
224+
>function() { return x} : () => string
225+
>x : string
226226

227227
(() => x);
228-
>(() => x) : () => any
229-
>() => x : () => any
230-
>x : any
228+
>(() => x) : () => string
229+
>() => x : () => string
230+
>x : string
231231
}
232232

233233
for (const x of []) {

‎tests/baselines/reference/capturedLetConstInLoop2.types‎

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ function foo0_1(x) {
3737
>x : any
3838

3939
for (let x in []) {
40-
>x : any
40+
>x : string
4141
>[] : undefined[]
4242

4343
let a = arguments.length;
@@ -47,17 +47,17 @@ function foo0_1(x) {
4747
>length : number
4848

4949
(function() { return x + a });
50-
>(function() { return x + a }) : () => any
51-
>function() { return x + a } : () => any
52-
>x + a : any
53-
>x : any
50+
>(function() { return x + a }) : () => string
51+
>function() { return x + a } : () => string
52+
>x + a : string
53+
>x : string
5454
>a : number
5555

5656
(() => x + a);
57-
>(() => x + a) : () => any
58-
>() => x + a : () => any
59-
>x + a : any
60-
>x : any
57+
>(() => x + a) : () => string
58+
>() => x + a : () => string
59+
>x + a : string
60+
>x : string
6161
>a : number
6262
}
6363
}
@@ -400,7 +400,7 @@ function foo0_1_c(x) {
400400
>x : any
401401

402402
for (const x in []) {
403-
>x : any
403+
>x : string
404404
>[] : undefined[]
405405

406406
const a = arguments.length;
@@ -410,17 +410,17 @@ function foo0_1_c(x) {
410410
>length : number
411411

412412
(function() { return x + a });
413-
>(function() { return x + a }) : () => any
414-
>function() { return x + a } : () => any
415-
>x + a : any
416-
>x : any
413+
>(function() { return x + a }) : () => string
414+
>function() { return x + a } : () => string
415+
>x + a : string
416+
>x : string
417417
>a : number
418418

419419
(() => x + a);
420-
>(() => x + a) : () => any
421-
>() => x + a : () => any
422-
>x + a : any
423-
>x : any
420+
>(() => x + a) : () => string
421+
>() => x + a : () => string
422+
>x + a : string
423+
>x : string
424424
>a : number
425425
}
426426
}

‎tests/baselines/reference/capturedLetConstInLoop2_ES6.types‎

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ function foo0_1(x) {
3636
>x : any
3737

3838
for (let x in []) {
39-
>x : any
39+
>x : string
4040
>[] : undefined[]
4141

4242
let a = arguments.length;
@@ -46,17 +46,17 @@ function foo0_1(x) {
4646
>length : number
4747

4848
(function() { return x + a });
49-
>(function() { return x + a }) : () => any
50-
>function() { return x + a } : () => any
51-
>x + a : any
52-
>x : any
49+
>(function() { return x + a }) : () => string
50+
>function() { return x + a } : () => string
51+
>x + a : string
52+
>x : string
5353
>a : number
5454

5555
(() => x + a);
56-
>(() => x + a) : () => any
57-
>() => x + a : () => any
58-
>x + a : any
59-
>x : any
56+
>(() => x + a) : () => string
57+
>() => x + a : () => string
58+
>x + a : string
59+
>x : string
6060
>a : number
6161
}
6262
}
@@ -399,7 +399,7 @@ function foo0_1_c(x) {
399399
>x : any
400400

401401
for (const x in []) {
402-
>x : any
402+
>x : string
403403
>[] : undefined[]
404404

405405
const a = arguments.length;
@@ -409,17 +409,17 @@ function foo0_1_c(x) {
409409
>length : number
410410

411411
(function() { return x + a });
412-
>(function() { return x + a }) : () => any
413-
>function() { return x + a } : () => any
414-
>x + a : any
415-
>x : any
412+
>(function() { return x + a }) : () => string
413+
>function() { return x + a } : () => string
414+
>x + a : string
415+
>x : string
416416
>a : number
417417

418418
(() => x + a);
419-
>(() => x + a) : () => any
420-
>() => x + a : () => any
421-
>x + a : any
422-
>x : any
419+
>(() => x + a) : () => string
420+
>() => x + a : () => string
421+
>x + a : string
422+
>x : string
423423
>a : number
424424
}
425425
}

0 commit comments

Comments
 (0)