Skip to content

"arguments" incorrectly munged to "arguments_1" in numerous circumstances in a for loop targeting ES5 #38594

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

Closed
Taytay opened this issue May 15, 2020 · 0 comments · Fixed by #38880
Assignees
Labels
Bug A bug in TypeScript

Comments

@Taytay
Copy link

Taytay commented May 15, 2020

TypeScript Version: Nightly (also, 3.9.2, 3.7.4, and probably others)

Search Terms:
arguments property, arguments_1 renamed

Expected behavior:
When targeting ES5, an object declared like so:
{arguments: "foo"}
is emitted to javascript as:
{arguments: "foo"}

Actual behavior:
The object is emitted to javascript as:
{arguments_1: "foo"}

See below code sample for more related bugs.

Related Issues:

A similar variant was reported and correctly fixed here:
#13586

This exact bug was previously reported here, and closed as "fixed" by the reporter. (I don't think it was actually fixed though).
#33540

Code

// Compile with ES5 target
function myFunc(_param: string) {
    for (let x = 0; x < 1; ++x){
        let i : number;
        // It's essential to have an inner function declared that references a variable defined in the loop
        [].forEach(function () { i })

        let brokenObj1 = {
            arguments: 0
        }
        // Actual:   {"arguments_1":0}
        // Expected: {"arguments":0}
        console.log(JSON.stringify(brokenObj1)); 

        let brokenObj2 = {
            arguments
        }
        // Actual:   {"arguments":{"0":0}}
        // Expected: {"arguments":{"0":"paramValue"}}
        // (Becuase it erroneously references the arguments of the generated loop function `_loop_1`)
        console.log(JSON.stringify(brokenObj2)); 
        

        let brokenObj3 = {
            arguments: arguments
        }
        // Actual:   {"arguments_1":{"0":"paramValue"}}
        // Expected: {"arguments":{"0":"paramValue"}}
        console.log(JSON.stringify(brokenObj3)); 
    }
}

myFunc("paramValue");
Output
"use strict";
// Compile with ES5 target
function myFunc(_param) {
    var _loop_1 = function (x) {
        var i;
        // It's essential to have an inner function declared that references a variable defined in the loop
        [].forEach(function () { i; });
        var brokenObj1 = {
            arguments_1: 0
        };
        // Actual:   {"arguments_1":0}
        // Expected: {"arguments":0}
        console.log(JSON.stringify(brokenObj1));
        var brokenObj2 = {
            arguments: arguments
        };
        // Actual:   {"arguments":{"0":0}}
        // Expected: {"arguments":{"0":"paramValue"}}
        // (Becuase it erroneously references the arguments of the generated loop function `_loop_1`)
        console.log(JSON.stringify(brokenObj2));
        var brokenObj3 = {
            arguments_1: arguments_1
        };
        // Actual:   {"arguments_1":{"0":"paramValue"}}
        // Expected: {"arguments":{"0":"paramValue"}}
        console.log(JSON.stringify(brokenObj3));
    };
    var arguments_1 = arguments;
    for (var x = 0; x < 1; ++x) {
        _loop_1(x);
    }
}
myFunc("paramValue");
Compiler Options
{
  "compilerOptions": {
    "noImplicitAny": true,
    "strictNullChecks": true,
    "strictFunctionTypes": true,
    "strictPropertyInitialization": true,
    "strictBindCallApply": true,
    "noImplicitThis": true,
    "noImplicitReturns": true,
    "useDefineForClassFields": false,
    "alwaysStrict": true,
    "allowUnreachableCode": false,
    "allowUnusedLabels": false,
    "downlevelIteration": false,
    "noEmitHelpers": false,
    "noLib": false,
    "noStrictGenericChecks": false,
    "noUnusedLocals": false,
    "noUnusedParameters": false,
    "esModuleInterop": true,
    "preserveConstEnums": false,
    "removeComments": false,
    "skipLibCheck": false,
    "checkJs": false,
    "allowJs": false,
    "declaration": true,
    "experimentalDecorators": false,
    "emitDecoratorMetadata": false,
    "target": "ES2017",
    "module": "ESNext"
  }
}

Playground Link: Provided

P.S. Typescript is awesome. We ❤️Typescript!

@RyanCavanaugh RyanCavanaugh added the Bug A bug in TypeScript label May 20, 2020
@RyanCavanaugh RyanCavanaugh added this to the TypeScript 4.0 milestone May 20, 2020
elibarzilay added a commit to elibarzilay/TypeScript that referenced this issue Jun 1, 2020
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 microsoft#38594.
elibarzilay added a commit to elibarzilay/TypeScript that referenced this issue Jun 11, 2020
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 microsoft#38594.
elibarzilay added a commit that referenced this issue Jun 13, 2020
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants