Skip to content

Support destructuring in for-in #54853

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
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
8 changes: 2 additions & 6 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41529,10 +41529,6 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
// VarDecl must be a variable declaration without a type annotation that declares a variable of type Any,
// and Expr must be an expression of type Any, an object type, or a type parameter type.
if (node.initializer.kind === SyntaxKind.VariableDeclarationList) {
const variable = (node.initializer as VariableDeclarationList).declarations[0];
if (variable && isBindingPattern(variable.name)) {
error(variable.name, Diagnostics.The_left_hand_side_of_a_for_in_statement_cannot_be_a_destructuring_pattern);
}
checkVariableDeclarationList(node.initializer as VariableDeclarationList);
}
else {
Expand All @@ -41542,8 +41538,8 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
// and Expr must be an expression of type Any, an object type, or a type parameter type.
const varExpr = node.initializer;
const leftType = checkExpression(varExpr);
if (varExpr.kind === SyntaxKind.ArrayLiteralExpression || varExpr.kind === SyntaxKind.ObjectLiteralExpression) {
error(varExpr, Diagnostics.The_left_hand_side_of_a_for_in_statement_cannot_be_a_destructuring_pattern);
if (isAssignmentPattern(varExpr)) {
checkDestructuringAssignment(varExpr, getIndexTypeOrString(rightType));
}
else if (!isTypeAssignableTo(getIndexTypeOrString(rightType), leftType)) {
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 not sure if this should actually be using getIndexTypeOrString(rightType). I used it because the branch below this uses that but because it does use it this code typechecks but, IMHO, it shouldn't (TS playground):

function test(obj: { a: 1; b: 2 }) {
  let key: "a" | "b";
  for (key in obj) {
  }
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Created this PR to address/discuss this: #54856

error(varExpr, Diagnostics.The_left_hand_side_of_a_for_in_statement_must_be_of_type_string_or_any);
Expand Down
4 changes: 0 additions & 4 deletions src/compiler/diagnosticMessages.json
Original file line number Diff line number Diff line change
Expand Up @@ -2372,10 +2372,6 @@
"category": "Error",
"code": 2490
},
"The left-hand side of a 'for...in' statement cannot be a destructuring pattern.": {
"category": "Error",
"code": 2491
},
"Cannot redeclare identifier '{0}' in catch clause.": {
"category": "Error",
"code": 2492
Expand Down
46 changes: 41 additions & 5 deletions src/compiler/transformers/es2015.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ import {
flattenDestructuringAssignment,
flattenDestructuringBinding,
FlattenLevel,
ForInOrOfStatement,
ForInStatement,
ForOfStatement,
ForStatement,
Expand Down Expand Up @@ -85,6 +86,7 @@ import {
isArrayLiteralExpression,
isArrowFunction,
isAssignmentExpression,
isAssignmentPattern,
isBinaryExpression,
isBindingPattern,
isBlock,
Expand Down Expand Up @@ -2684,11 +2686,16 @@ export function transformES2015(context: TransformationContext): (x: SourceFile
}

function visitForInStatement(node: ForInStatement, outermostLabeledStatement: LabeledStatement | undefined) {
const needsConversionForDestructuring = node.initializer.kind === SyntaxKind.VariableDeclarationList ?
isBindingPattern((node.initializer as VariableDeclarationList).declarations[0].name) :
isAssignmentPattern(node.initializer);
return visitIterationStatementWithFacts(
HierarchyFacts.ForInOrForOfStatementExcludes,
HierarchyFacts.ForInOrForOfStatementIncludes,
node,
outermostLabeledStatement);
outermostLabeledStatement,
needsConversionForDestructuring ? convertForInStatementForDestructuring : undefined
);
}

function visitForOfStatement(node: ForOfStatement, outermostLabeledStatement: LabeledStatement | undefined): VisitResult<Statement> {
Expand All @@ -2700,7 +2707,7 @@ export function transformES2015(context: TransformationContext): (x: SourceFile
compilerOptions.downlevelIteration ? convertForOfStatementForIterable : convertForOfStatementForArray);
}

function convertForOfStatementHead(node: ForOfStatement, boundValue: Expression, convertedLoopBodyStatements: Statement[] | undefined) {
function convertForInOrOfStatementHead(node: ForInOrOfStatement, boundValue: Expression, convertedLoopBodyStatements: Statement[] | undefined) {
const statements: Statement[] = [];
const initializer = node.initializer;
if (isVariableDeclarationList(initializer)) {
Expand Down Expand Up @@ -2800,6 +2807,36 @@ export function transformES2015(context: TransformationContext): (x: SourceFile
);
}

function convertForInStatementForDestructuring(node: ForInStatement, outermostLabeledStatement: LabeledStatement | undefined, convertedLoopBodyStatements: Statement[] | undefined): Statement {
const lhsReference = factory.createUniqueName("key");

const forInStatement = setEmitFlags(
setTextRange(
factory.createForInStatement(
/*initializer*/ setEmitFlags(
setTextRange(
factory.createVariableDeclarationList([
setTextRange(factory.createVariableDeclaration(lhsReference, /*exclamationToken*/ undefined, /*type*/ undefined, /*initializer*/ undefined), node.initializer),
]),
node.expression
),
EmitFlags.NoHoisting
),
/*expression*/ node.expression,
/*statement*/ convertForInOrOfStatementHead(
node,
lhsReference,
convertedLoopBodyStatements
)
),
/*location*/ node
),
EmitFlags.NoTokenTrailingSourceMaps
);

return factory.restoreEnclosingLabel(forInStatement, outermostLabeledStatement, convertedLoopState && resetLabel);
}

function convertForOfStatementForArray(node: ForOfStatement, outermostLabeledStatement: LabeledStatement | undefined, convertedLoopBodyStatements: Statement[] | undefined): Statement {
// The following ES6 code:
//
Expand Down Expand Up @@ -2856,7 +2893,7 @@ export function transformES2015(context: TransformationContext): (x: SourceFile
node.expression
),
/*incrementor*/ setTextRange(factory.createPostfixIncrement(counter), node.expression),
/*statement*/ convertForOfStatementHead(
/*statement*/ convertForInOrOfStatementHead(
node,
factory.createElementAccessExpression(rhsReference, counter),
convertedLoopBodyStatements
Expand All @@ -2867,7 +2904,6 @@ export function transformES2015(context: TransformationContext): (x: SourceFile

// Disable trailing source maps for the OpenParenToken to align source map emit with the old emitter.
setEmitFlags(forStatement, EmitFlags.NoTokenTrailingSourceMaps);
setTextRange(forStatement, node);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this was redundant - this call is a mutation and the same call is already done when assigning to forStatement

return factory.restoreEnclosingLabel(forStatement, outermostLabeledStatement, convertedLoopState && resetLabel);
}

Expand Down Expand Up @@ -2905,7 +2941,7 @@ export function transformES2015(context: TransformationContext): (x: SourceFile
),
/*condition*/ factory.createLogicalNot(factory.createPropertyAccessExpression(result, "done")),
/*incrementor*/ factory.createAssignment(result, next),
/*statement*/ convertForOfStatementHead(
/*statement*/ convertForInOrOfStatementHead(
node,
factory.createPropertyAccessExpression(result, "value"),
convertedLoopBodyStatements
Expand Down
32 changes: 32 additions & 0 deletions src/testRunner/unittests/evaluation/destructuring.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,4 +65,36 @@ describe("unittests:: evaluation:: destructuring", () => {
assert.deepEqual(result.output, { a: 1, b: { y: 2 } });
});
});
describe("correct evaluation in for-in with array pattern", () => {
it("ES5", () => {
const result = evaluator.evaluateTypeScript(`
export let output: string;
for (const [first] in { test: 1 }) { output = first; }
`, { target: ts.ScriptTarget.ES5, downlevelIteration: true });
assert.deepEqual(result.output, "t");
});
it("ESNext", () => {
const result = evaluator.evaluateTypeScript(`
export let output: string;
for (const [first] in { test: 1 }) { output = first; }
`, { target: ts.ScriptTarget.ESNext });
assert.deepEqual(result.output, "t");
});
});
describe("correct evaluation in for-in with object pattern", () => {
it("ES5", () => {
const result = evaluator.evaluateTypeScript(`
export let output: number;
for (const { length } in { test: 1 }) { output = length; }
`, { target: ts.ScriptTarget.ES5, downlevelIteration: true });
assert.deepEqual(result.output, 4);
});
it("ESNext", () => {
const result = evaluator.evaluateTypeScript(`
export let output: number;
for (const { length } in { test: 1 }) { output = length; }
`, { target: ts.ScriptTarget.ESNext });
assert.deepEqual(result.output, 4);
});
});
});
Original file line number Diff line number Diff line change
@@ -1,10 +1,7 @@
for-inStatementsDestructuring.ts(1,10): error TS2461: Type 'string' is not an array type.
for-inStatementsDestructuring.ts(1,10): error TS2491: The left-hand side of a 'for...in' statement cannot be a destructuring pattern.


==== for-inStatementsDestructuring.ts (2 errors) ====
==== for-inStatementsDestructuring.ts (1 errors) ====
for (var [a, b] in []) {}
~~~~~~
!!! error TS2461: Type 'string' is not an array type.
~~~~~~
!!! error TS2491: The left-hand side of a 'for...in' statement cannot be a destructuring pattern.
!!! error TS2461: Type 'string' is not an array type.
4 changes: 3 additions & 1 deletion tests/baselines/reference/for-inStatementsDestructuring.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,6 @@
for (var [a, b] in []) {}

//// [for-inStatementsDestructuring.js]
for (var _a = void 0, a = _a[0], b = _a[1] in []) { }
for (var key_1 in []) {
var a = key_1[0], b = key_1[1];
}
Original file line number Diff line number Diff line change
@@ -1,12 +1,9 @@
for-inStatementsDestructuring2.ts(1,10): error TS2491: The left-hand side of a 'for...in' statement cannot be a destructuring pattern.
for-inStatementsDestructuring2.ts(1,11): error TS2339: Property 'a' does not exist on type 'String'.
for-inStatementsDestructuring2.ts(1,14): error TS2339: Property 'b' does not exist on type 'String'.


==== for-inStatementsDestructuring2.ts (3 errors) ====
==== for-inStatementsDestructuring2.ts (2 errors) ====
for (var {a, b} in []) {}
~~~~~~
!!! error TS2491: The left-hand side of a 'for...in' statement cannot be a destructuring pattern.
~
!!! error TS2339: Property 'a' does not exist on type 'String'.
~
Expand Down
4 changes: 3 additions & 1 deletion tests/baselines/reference/for-inStatementsDestructuring2.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,6 @@
for (var {a, b} in []) {}

//// [for-inStatementsDestructuring2.js]
for (var _a = void 0, a = _a.a, b = _a.b in []) { }
for (var key_1 in []) {
var a = key_1.a, b = key_1.b;
}
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
for-inStatementsDestructuring3.ts(2,6): error TS2491: The left-hand side of a 'for...in' statement cannot be a destructuring pattern.
for-inStatementsDestructuring3.ts(2,6): error TS2461: Type '"length" | "toString" | "toLocaleString" | "pop" | "push" | "concat" | "join" | "reverse" | "shift" | "slice" | "sort" | "splice" | "unshift" | "indexOf" | "lastIndexOf" | "every" | "some" | "forEach" | "map" | "filter" | "reduce" | "reduceRight"' is not an array type.


==== for-inStatementsDestructuring3.ts (1 errors) ====
var a, b;
for ([a, b] in []) { }
~~~~~~
!!! error TS2491: The left-hand side of a 'for...in' statement cannot be a destructuring pattern.
!!! error TS2461: Type '"length" | "toString" | "toLocaleString" | "pop" | "push" | "concat" | "join" | "reverse" | "shift" | "slice" | "sort" | "splice" | "unshift" | "indexOf" | "lastIndexOf" | "every" | "some" | "forEach" | "map" | "filter" | "reduce" | "reduceRight"' is not an array type.
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
for-inStatementsDestructuring4.ts(2,6): error TS2491: The left-hand side of a 'for...in' statement cannot be a destructuring pattern.
for-inStatementsDestructuring4.ts(2,7): error TS2339: Property 'a' does not exist on type '"length" | "toString" | "toLocaleString" | "pop" | "push" | "concat" | "join" | "reverse" | "shift" | "slice" | "sort" | "splice" | "unshift" | "indexOf" | "lastIndexOf" | "every" | "some" | "forEach" | "map" | "filter" | "reduce" | "reduceRight"'.
for-inStatementsDestructuring4.ts(2,10): error TS2339: Property 'b' does not exist on type '"length" | "toString" | "toLocaleString" | "pop" | "push" | "concat" | "join" | "reverse" | "shift" | "slice" | "sort" | "splice" | "unshift" | "indexOf" | "lastIndexOf" | "every" | "some" | "forEach" | "map" | "filter" | "reduce" | "reduceRight"'.


==== for-inStatementsDestructuring4.ts (1 errors) ====
==== for-inStatementsDestructuring4.ts (2 errors) ====
var a, b;
for ({a, b} in []) { }
~~~~~~
!!! error TS2491: The left-hand side of a 'for...in' statement cannot be a destructuring pattern.
~
!!! error TS2339: Property 'a' does not exist on type '"length" | "toString" | "toLocaleString" | "pop" | "push" | "concat" | "join" | "reverse" | "shift" | "slice" | "sort" | "splice" | "unshift" | "indexOf" | "lastIndexOf" | "every" | "some" | "forEach" | "map" | "filter" | "reduce" | "reduceRight"'.
~
!!! error TS2339: Property 'b' does not exist on type '"length" | "toString" | "toLocaleString" | "pop" | "push" | "concat" | "join" | "reverse" | "shift" | "slice" | "sort" | "splice" | "unshift" | "indexOf" | "lastIndexOf" | "every" | "some" | "forEach" | "map" | "filter" | "reduce" | "reduceRight"'.
4 changes: 3 additions & 1 deletion tests/baselines/reference/for-inStatementsDestructuring4.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,6 @@ for ({a, b} in []) { }

//// [for-inStatementsDestructuring4.js]
var a, b;
for ({ a: a, b: b } in []) { }
for (var key_1 in []) {
a = key_1.a, b = key_1.b;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
for-inStatementsDestructuring5.ts(4,6): error TS2461: Type 'string' is not an array type.


==== for-inStatementsDestructuring5.ts (1 errors) ====
let a: string;
declare let obj: { [s: string]: string };

for ([a] in obj) {}
~~~
!!! error TS2461: Type 'string' is not an array type.

Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
//// [tests/cases/conformance/statements/for-inStatements/for-inStatementsDestructuring5.ts] ////

//// [for-inStatementsDestructuring5.ts]
let a: string;
declare let obj: { [s: string]: string };

for ([a] in obj) {}


//// [for-inStatementsDestructuring5.js]
var a;
for ([a] in obj) { }
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
//// [tests/cases/conformance/statements/for-inStatements/for-inStatementsDestructuring5.ts] ////

=== for-inStatementsDestructuring5.ts ===
let a: string;
>a : Symbol(a, Decl(for-inStatementsDestructuring5.ts, 0, 3))

declare let obj: { [s: string]: string };
>obj : Symbol(obj, Decl(for-inStatementsDestructuring5.ts, 1, 11))
>s : Symbol(s, Decl(for-inStatementsDestructuring5.ts, 1, 20))

for ([a] in obj) {}
>a : Symbol(a, Decl(for-inStatementsDestructuring5.ts, 0, 3))
>obj : Symbol(obj, Decl(for-inStatementsDestructuring5.ts, 1, 11))

Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
//// [tests/cases/conformance/statements/for-inStatements/for-inStatementsDestructuring5.ts] ////

=== for-inStatementsDestructuring5.ts ===
let a: string;
>a : string

declare let obj: { [s: string]: string };
>obj : { [s: string]: string; }
>s : string

for ([a] in obj) {}
>[a] : [string]
>a : string
>obj : { [s: string]: string; }

Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
//// [tests/cases/conformance/statements/for-inStatements/for-inStatementsDestructuring5.ts] ////

//// [for-inStatementsDestructuring5.ts]
let a: string;
declare let obj: { [s: string]: string };

for ([a] in obj) {}


//// [for-inStatementsDestructuring5.js]
let a;
for ([a] in obj) { }
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
//// [tests/cases/conformance/statements/for-inStatements/for-inStatementsDestructuring5.ts] ////

=== for-inStatementsDestructuring5.ts ===
let a: string;
>a : Symbol(a, Decl(for-inStatementsDestructuring5.ts, 0, 3))

declare let obj: { [s: string]: string };
>obj : Symbol(obj, Decl(for-inStatementsDestructuring5.ts, 1, 11))
>s : Symbol(s, Decl(for-inStatementsDestructuring5.ts, 1, 20))

for ([a] in obj) {}
>a : Symbol(a, Decl(for-inStatementsDestructuring5.ts, 0, 3))
>obj : Symbol(obj, Decl(for-inStatementsDestructuring5.ts, 1, 11))

Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
//// [tests/cases/conformance/statements/for-inStatements/for-inStatementsDestructuring5.ts] ////

=== for-inStatementsDestructuring5.ts ===
let a: string;
>a : string

declare let obj: { [s: string]: string };
>obj : { [s: string]: string; }
>s : string

for ([a] in obj) {}
>[a] : [string]
>a : string
>obj : { [s: string]: string; }

Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
for-inStatementsDestructuring6.ts(5,6): error TS2461: Type 'string' is not an array type.


==== for-inStatementsDestructuring6.ts (1 errors) ====
let b: number;
let c: string[];
declare let obj: { [s: string]: string };

for ([b, ...c] in obj) {}
~~~~~~~~~
!!! error TS2461: Type 'string' is not an array type.

Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
//// [tests/cases/conformance/statements/for-inStatements/for-inStatementsDestructuring6.ts] ////

//// [for-inStatementsDestructuring6.ts]
let b: number;
let c: string[];
declare let obj: { [s: string]: string };

for ([b, ...c] in obj) {}


//// [for-inStatementsDestructuring6.js]
var b;
var c;
for (var key_1 in obj) {
b = key_1[0], c = key_1.slice(1);
}
Loading