SpiderMonkey AST conversion by RReverser · Pull Request #526 · mishoo/UglifyJS (original) (raw)

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 andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation5 Commits4 Checks0 Files changed

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

Improved conversion from SpiderMonkey AST and added back-conversion from UglifyJS AST to SpiderMonkey format.

• Added directives recognition in SM AST.
• Moved semi-standard SM Property type to separate handler.
• Added const recognition from SM AST.
• Removed redundant this-as-identifier recognition.
• Removed redundant rules for abstract SM types.
• Described CatchClause using string syntax.
• Added support for semi-standard range tuple as location source.
• Added back-conversion support (to be improved).

• Explicitly forbidden multiple catch clauses as SM-specific feature.
• Simplified describing of UglifyJS->Mozilla AST conversion rules.
• Moved alias rules to single place.
• Removed usage of dynamic type bindings in generated code (speed-up).

Thank you for the patch! Merged.

I didn't do much testing, but seems that at least what we already had still works (i.e. --acorn). Perhaps it would be useful to add tests for this...

You're welcome!

Perhaps it would be useful to add tests for this...

Yeah. I'm not sure if I have time to add detailed unit tests for different types of nodes, but I've tested locally on jQuery source with following steps:

1. Parse code with Acorn ({locations: true, ranges: true}) to SpiderMonkey AST.
2. Convert to UglifyJS AST.
3. Convert back to SpiderMonkey AST.
4. Deeply compare result with original SM AST.
5. Visually check results - there should be only missing locations in nodes that are represented as strings in UglifyJS AST.

I could convert step 5 to automatic comparison with expected set of differences and provide this suite as unit test. I know it's not an ideal solution, but it could work as a temporary one.

Btw, I've already pushed one more fix for mangled names - 5e314bf - but, seems, it didn't land in time to be included in this PR, so I'll send it together with tests.

@RReverser or @mishoo - kind of hard to tell just by looking at the diffs and the few docs on the subject - is there a command line flag for uglifyjs to produce uglify AST or Spidermonkey AST output? Or is this functionality only available at the API level?