Uglifyjs: AST_Node.from_mozilla_ast fails on setter

[kyriosli]

Bug report

ES5

Uglify version (2.x ~ 3.x)

JavaScript input a = {set foo(u) {} }

ast = ug.AST_Node.from_mozilla_ast({
    "type": "Program",
    "body": [
        {
            "type": "ExpressionStatement",
            "expression": {
                "type": "AssignmentExpression",
                "operator": "=",
                "left": {
                    "type": "Identifier",
                    "name": "a"
                },
                "right": {
                    "type": "ObjectExpression",
                    "properties": [
                        {
                            "type": "Property",
                            "key": {
                                "type": "Identifier",
                                "name": "foo"
                            },
                            "computed": false,
                            "value": {
                                "type": "FunctionExpression",
                                "id": null,
                                "params": [
                                    {
                                        "type": "Identifier",
                                        "name": "u"
                                    }
                                ],
                                "body": {
                                    "type": "BlockStatement",
                                    "body": []
                                },
                                "generator": false,
                                "expression": false
                            },
                            "kind": "set",
                            "method": false,
                            "shorthand": false
                        }
                    ]
                }
            }
        }
    ],
    "sourceType": "script"
});

console.log(ast.body[0].body.right.properties[0])

outputs:

AST_Node {
  ...
  value:
   ...,
  key: 'foo' }

expected:

AST_Node {
  ...
  value:
  ...,
  key:
   AST_Node {
     ...
     thedef: undefined,
     name: 'foo',
     scope: undefined } }

[alexlamsl]

foo is an instance of AST_ObjectSetter rather than a string.

Moreover, I couldn't produce the same Mozilla AST as you do in your example with a={set foo(u){}}:

$ echo 'a={set foo(u){}}' | uglify-js -o spidermonkey
{
  "type": "Program",
  "body": [
    {
      "type": "ExpressionStatement",
      "expression": {
        "type": "AssignmentExpression",
        "operator": "=",
        "left": {
          "type": "Identifier",
          "name": "a"
        },
        "right": {
          "type": "ObjectExpression",
          "properties": [
            {
              "type": "Property",
              "kind": "set",
              "key": {
                "type": "Literal",
                "value": {
                  "name": "foo"
                }
              },
              "value": {
                "type": "FunctionExpression",
                "id": null,
                "params": [
                  {
                    "type": "Identifier",
                    "name": "u"
                  }
                ],
                "body": {
                  "type": "BlockStatement",
                  "body": []
                }
              }
            }
          ]
        }
      }
    }
  ]
}

(Souce positional information removed for brevity.)

[alexlamsl]

There is certainly a bug with object setters because the round-trip test wouldn't pass:

$ echo 'a={set foo(u){}}' | uglify-js -o spidermonkey | uglify-js -p spidermonkey
ERROR: Object doesn't support property or method 'print'
   at Anonymous function (Function code:4930:9)

Without object setter, it works:

$ echo 'a={foo:function(u){}}' | uglify-js -o spidermonkey | uglify-js -p spidermonkey
a={foo:function(u){}};

[kyriosli]

@alexlamsl The AST input is generated via esprima, I ran into this bug when I was intending to pipe babel's ast output into uglify-js. There's a difference between UglifyJS's spidermonkey output and Esprima: the type of the property key is an Identifier rather than a Literal.

(see here )

As well as I know, for an object literal with a setter, UglifyJS.parse yields a property node which has its key as an AST_Token, and its value.name as null.

[alexlamsl]

@kyriosli thanks for the clarification.

Please consider adding the example you've got in https://github.com/mishoo/UglifyJS2/issues/1914#issue-227981641 into test/mocha/spidermonkey.js - the description of the test can be Should parse from esprima correctly (a={set foo(u){}}).

[kzc]

I ran into this bug when I was intending to pipe babel's ast output into uglify-js

off topic - Babel/Babylon has a different native AST format, but it appears that an estree plugin now exists: https://github.com/babel/babylon/pull/277

[alexlamsl]

@kzc the one thing that struck me is how test/mozilla-ast.js isn't picking up any of this - but it might be based on some old/odd versions of estree or something.

[kzc]

it might be based on some old/odd versions of estree or something.

Either that or it simply has gaps in its test coverage.

[alexlamsl]

@kzc crazy/lazy idea - how about we borrow the codegen from test/ufuzz.js, push it through esprima then AST_Node.from_mozilla_ast() for the round-trip test?

It almost seems like it could replace test/mozilla-ast.js altogether.

[kzc]

crazy/lazy idea - how about we borrow the codegen from test/ufuzz.js, push it through esprima then AST_Node.from_mozilla_ast() for the round-trip test?

(movie voice) That's so crazy it just might work!

As long as it's only run within the UGLIFYJS_TEST_ALL=1 tests, sure.

I guess esprima could be a devDependency only. The version of acorn in packages.json for the old 2.x --acorn flag is prehistoric. Maybe we should get rid of these altogether in 3.x?

  "devDependencies": {
    "acorn": "~0.6.0",
    "escodegen": "~1.3.3",
    "esfuzz": "~0.3.1",
    "estraverse": "~1.5.1",

[kzc]

@alexlamsl Wait - one problem - I don't think test/ufuzz.js generates setters and getters.

[alexlamsl]

Wait - one problem - I don't think test/ufuzz.js generates setters and getters.

We can fix that :wink:

[kzc]

If the names of the fuzzed setters and getters would match the names of the already existing SAFE_KEYS for object literal properties in test/ufuzz.js then that fuzzed code would be exercised.

Edit: it would mean that pure_getters couldn't be used in fuzz tests though.

[alexlamsl]

@kzc pure_getters=true isn't getting fuzzed for known issues anyway.

pure_getters=strict is being fuzzed by default.

[alexlamsl]

Latest versions of esprima & acorn both don't have dependencies (great!) and have similar download statistics.

Which one shall we pick for test/mozilla-ast.js rewrite?

[kzc]

Which one shall we pick for test/mozilla-ast.js rewrite?

I prefer acorn because it is used by rollup and buble and I have some familiarity with it.