No need for parens for new expressions without args

[amasad]

No description provided.

[codecov-io]

Current coverage is 89.11%

Merging #3111 into master will decrease coverage by -0.04% as of 752a77e

@@            master   #3111   diff @@
======================================
  Files          214     214       
  Stmts        15472   15473     +1
  Branches         0       0       
  Methods          0       0       
======================================
- Hit          13794   13788     -6
  Partial          0       0       
- Missed        1678    1685     +7

Review entire Coverage Diff as of 752a77e

Powered by Codecov. Updated on successful CI builds.

[hzoo]

I would not of found those other edge cases - I wonder if there could be a tool that searches through codebases prints out the different types of AST to look for these. I guess you can also try to generate code from the spec but I would have even less idea about that?

[amasad]

One thing to do is to check how existing tools do this. I went through uglify's code and it seems like we're handling everything they're handling.

[hzoo]

Ah yeah I guess that works as a baseline - unless they are messing up something as well.

I remember sebmck mentioning everything.js in an old issue (found it: https://phabricator.babeljs.io/T2191) which has some other forms - https://github.com/michaelficarra/everything.js/blob/9bdd8a0a9fd571ecca612d53d417890e5a9409d4/es5.js#L77-L80

[amasad]

Ah that's useful. I can do some more testing using that

[hzoo]

Yeah don't need all of those but with

new X();
new Y()();
new F().z;

new x();
new new x()();
new x[0]();
new x.a();
new x[0].a();
new x.a[0]();

new x;
new new x;
new new x();
new new x().a;
new new x()[0];

I get

new X;new Y()();new F().z;
new x;new new x;new x[0];new x.a;new x[0].a;new x.a[0];
new x;new new x;new new x;new new x().a;new new x()[0];

[amasad]

looks correct?

[hzoo]

Yeah looks good

[phantom10111]

@amasad Some test cases that produce incorrect results:

new new x()(a);
new new x()()();
new new x()(a)();
new new x()()(a);
new new x()(a)(a);

Produces

new new x(a);new new x()();new new x(a)();new new x()(a);new new x(a)(a);

All those have empty parens first so it's possible they are all the same edge case.

[hzoo]

Ok && !(t.isNewExpression(parent) && parent.arguments.length > 0) seems to fix it

[hzoo]

new new x()();
new new x()(a);
new new x(a)();
new new x(a)(a);

new new x;new new x()(a);new new x(a);new new x(a)(a);

[hzoo]

Added the other commit in 21f7665

[hzoo]

Actually I'm not sure how we would do this for multiple calls of new (my patch only works for new new but not new new new). I don't think we have a reference to the parent of the parent in this module - it's only export function NewExpression(node: Object, parent: Object) {.

Who would do even new new anyway?

Maybe it would be better to just not do anything for multiple nested NewExpression.

[hzoo]

Talked with @amasad - just going to return if the parent of the NewExpression is a NewExpression