Keep fparens

[gutnikov]

This does basically the same as proposed in #1307
Main benefit of this implementation is that it allows you to keep parentheses where you want,
without wrapping every function. So to wrap or not to wrap is up to user.

[avdg]

Hmmm... not really sure if relying on AST_Node.start is a good idea, especially given the lack of tests we have (I didn't saw many mocha tests on this and mocha tests are relative new) on them and the likelihood of AST_Node.start not being consistent.

[avdg]

There is some guarantee that this code will work if these code involving tokens got locked into tests.

https://github.com/mishoo/UglifyJS2/blob/642273c29002e7676719f489d7fcf552974118f4/lib/parse.js#L1438

[kzc]

add trailing comma

[kzc]

use outer single quoted string to avoid need for escaping

[kzc]

use outer single quoted string to avoid need for escaping

[kzc]

Something to be aware of - this PR would not work for Crockford style IIFEs:

(function(){
    console.log("hello");
}());

PR would also need to update README.

[kzc]

indent by 4

[avdg]

Hmm.. not sure how much this test overlaps with this issue. UglifyJS seems to be struggling on this test:

https://github.com/tc39/test262/blob/master/test/language/statements/variable/fn-name-cover.js

https://avdg.github.io/uglify/#ref_PR_x2d1371-bool_beautify0-from_https_x3a_x2f_x2fraw_x2egithubusercontent_x2ecom_x2ftc39_x2ftest262_x2fmaster_x2ftest_x2flanguage_x2fstatements_x2fvariable_x2ffn_x2dname_x2dcover_x2ejs

[avdg]

I would prefer looking data gathering on parser level personally. Then store that data in the AST tree.

[gutnikov]

avdg, do you mean the approach like:

1. when met function expression - check if it wrapped in parentheses
2. if so, put a flag to its ast node
3. on output: check the flag, and if true - wrap it

right?

[gutnikov]

avdg, smth like that while parsing:

              case "(":
                next();
                var ex = expression(true);
                ex.start = start;
                ex.end = S.token;
                expect(")");
                if (ex instanceof AST_Function) {
                    ex.wrappingParens = true;
                }
                return subscripts(ex, allow_calls);

[avdg]

yeah, that would be more stable long term I believe. The only obstacle would be implementation. If that fails (preferably the parser still needs to be fast, but that should be possible if the checks are simple as it is the case now), we can stick to this method for now I guess...

[avdg]

Don't forget to update the AST definitions when adding properties here: https://github.com/mishoo/UglifyJS2/blob/master/lib/ast.js

[gutnikov]

Ok, i'll take this approach then. Thanks!

[kzc]

                ex.wrappingParens = true;

Use snake_case rather than camelCase for data members.

parens would be a better name.

[gutnikov]

kzc, ok, thanx! will pay attention to all code-style issues