Skip to content

Commit 4ec21ed

Browse files
committed
Fix handling of aruments in the emitter
Two problems are fixed: * `isArgumentsLocalBinding` did only `PropertyAccessExpression`, now it's also doing `PropertyAssignment` (doesn't affect other files, since it's only used in the emitter). * `visitShorthandPropertyAssignment` should call `visitIdentifier` on the synthesized id. (For completion it might be better to make it visit the the original?) Fixes #38594.
1 parent 6a3513b commit 4ec21ed

File tree

6 files changed

+130
-16
lines changed

6 files changed

+130
-16
lines changed

src/compiler/checker.ts

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -35977,15 +35977,16 @@ namespace ts {
3597735977
// Emitter support
3597835978

3597935979
function isArgumentsLocalBinding(nodeIn: Identifier): boolean {
35980-
if (!isGeneratedIdentifier(nodeIn)) {
35981-
const node = getParseTreeNode(nodeIn, isIdentifier);
35982-
if (node) {
35983-
const isPropertyName = node.parent.kind === SyntaxKind.PropertyAccessExpression && (<PropertyAccessExpression>node.parent).name === node;
35984-
return !isPropertyName && getReferencedValueSymbol(node) === argumentsSymbol;
35985-
}
35986-
}
35987-
35988-
return false;
35980+
// Note: does not handle isShorthandPropertyAssignment (and probably a few more)
35981+
if (isGeneratedIdentifier(nodeIn)) return false;
35982+
const node = getParseTreeNode(nodeIn, isIdentifier);
35983+
if (!node) return false;
35984+
const parent = node.parent;
35985+
if (!parent) return false;
35986+
const isPropertyName = ((isPropertyAccessExpression(parent)
35987+
|| isPropertyAssignment(parent))
35988+
&& parent.name === node);
35989+
return !isPropertyName && getReferencedValueSymbol(node) === argumentsSymbol;
3598935990
}
3599035991

3599135992
function moduleExportsSomeValue(moduleReferenceExpression: Expression): boolean {

src/compiler/transformers/es2015.ts

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -583,13 +583,10 @@ namespace ts {
583583
if (!convertedLoopState) {
584584
return node;
585585
}
586-
if (isGeneratedIdentifier(node)) {
587-
return node;
588-
}
589-
if (node.escapedText !== "arguments" || !resolver.isArgumentsLocalBinding(node)) {
590-
return node;
586+
if (resolver.isArgumentsLocalBinding(node)) {
587+
return convertedLoopState.argumentsName || (convertedLoopState.argumentsName = createUniqueName("arguments"));
591588
}
592-
return convertedLoopState.argumentsName || (convertedLoopState.argumentsName = createUniqueName("arguments"));
589+
return node;
593590
}
594591

595592
function visitBreakOrContinueStatement(node: BreakOrContinueStatement): Statement {
@@ -3517,7 +3514,7 @@ namespace ts {
35173514
return setTextRange(
35183515
createPropertyAssignment(
35193516
node.name,
3520-
getSynthesizedClone(node.name)
3517+
visitIdentifier(getSynthesizedClone(node.name))
35213518
),
35223519
/*location*/ node
35233520
);
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
//// [argumentsAsPropertyName2.ts]
2+
// target: es5
3+
4+
function foo() {
5+
for (let x = 0; x < 1; ++x) {
6+
let i : number;
7+
[].forEach(function () { i });
8+
({ arguments: 0 });
9+
({ arguments });
10+
({ arguments: arguments });
11+
}
12+
}
13+
14+
15+
//// [argumentsAsPropertyName2.js]
16+
// target: es5
17+
function foo() {
18+
var _loop_1 = function (x) {
19+
var i;
20+
[].forEach(function () { i; });
21+
({ arguments: 0 });
22+
({ arguments: arguments_1 });
23+
({ arguments: arguments_1 });
24+
};
25+
var arguments_1 = arguments;
26+
for (var x = 0; x < 1; ++x) {
27+
_loop_1(x);
28+
}
29+
}
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
=== tests/cases/compiler/argumentsAsPropertyName2.ts ===
2+
// target: es5
3+
4+
function foo() {
5+
>foo : Symbol(foo, Decl(argumentsAsPropertyName2.ts, 0, 0))
6+
7+
for (let x = 0; x < 1; ++x) {
8+
>x : Symbol(x, Decl(argumentsAsPropertyName2.ts, 3, 12))
9+
>x : Symbol(x, Decl(argumentsAsPropertyName2.ts, 3, 12))
10+
>x : Symbol(x, Decl(argumentsAsPropertyName2.ts, 3, 12))
11+
12+
let i : number;
13+
>i : Symbol(i, Decl(argumentsAsPropertyName2.ts, 4, 11))
14+
15+
[].forEach(function () { i });
16+
>[].forEach : Symbol(Array.forEach, Decl(lib.es5.d.ts, --, --))
17+
>forEach : Symbol(Array.forEach, Decl(lib.es5.d.ts, --, --))
18+
>i : Symbol(i, Decl(argumentsAsPropertyName2.ts, 4, 11))
19+
20+
({ arguments: 0 });
21+
>arguments : Symbol(arguments, Decl(argumentsAsPropertyName2.ts, 6, 10))
22+
23+
({ arguments });
24+
>arguments : Symbol(arguments, Decl(argumentsAsPropertyName2.ts, 7, 10))
25+
26+
({ arguments: arguments });
27+
>arguments : Symbol(arguments, Decl(argumentsAsPropertyName2.ts, 8, 10))
28+
>arguments : Symbol(arguments)
29+
}
30+
}
31+
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
=== tests/cases/compiler/argumentsAsPropertyName2.ts ===
2+
// target: es5
3+
4+
function foo() {
5+
>foo : () => void
6+
7+
for (let x = 0; x < 1; ++x) {
8+
>x : number
9+
>0 : 0
10+
>x < 1 : boolean
11+
>x : number
12+
>1 : 1
13+
>++x : number
14+
>x : number
15+
16+
let i : number;
17+
>i : number
18+
19+
[].forEach(function () { i });
20+
>[].forEach(function () { i }) : void
21+
>[].forEach : (callbackfn: (value: any, index: number, array: any[]) => void, thisArg?: any) => void
22+
>[] : undefined[]
23+
>forEach : (callbackfn: (value: any, index: number, array: any[]) => void, thisArg?: any) => void
24+
>function () { i } : () => void
25+
>i : number
26+
27+
({ arguments: 0 });
28+
>({ arguments: 0 }) : { arguments: number; }
29+
>{ arguments: 0 } : { arguments: number; }
30+
>arguments : number
31+
>0 : 0
32+
33+
({ arguments });
34+
>({ arguments }) : { arguments: IArguments; }
35+
>{ arguments } : { arguments: IArguments; }
36+
>arguments : IArguments
37+
38+
({ arguments: arguments });
39+
>({ arguments: arguments }) : { arguments: IArguments; }
40+
>{ arguments: arguments } : { arguments: IArguments; }
41+
>arguments : IArguments
42+
>arguments : IArguments
43+
}
44+
}
45+
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
// target: es5
2+
3+
function foo() {
4+
for (let x = 0; x < 1; ++x) {
5+
let i : number;
6+
[].forEach(function () { i });
7+
({ arguments: 0 });
8+
({ arguments });
9+
({ arguments: arguments });
10+
}
11+
}

0 commit comments

Comments
 (0)