924466 - integrate acorn with the devtools

Status: RESOLVED FIXED

(DevTools :: Debugger, defect, P3)

Description

[Brandon Benvie [:benvie]]

We'd like to add a third party JavaScript parser to the tree because Reflect.parse isn't adequate for our needs [1]. The two options we have that produce compatible AST are acorn [2] and esprima [3].

acorn pros:
* faster
* succinct code (~40% the code to do the same thing)
* better error-recovering parser

esprima pros:
* better tokenizer exposed
* has support for most ES6 features (in its harmony branch)


[1] https://groups.google.com/forum/#!topic/mozilla.dev.developer-tools/cWANPm0FWVM
[2] https://github.com/marijnh/acorn
[3] https://github.com/ariya/esprima

Comment 1

[Brandon Benvie [:benvie]]

The decision here also would have an impact on which tool we use for code completion. Tern [1] depends on acorn, while aulx [2] depends on esprima. These tools rely on the lenient parsing modes and tokenizers that each of the parsers provide, which don't have compatible APIs.

Comment 2

[Brandon Benvie [:benvie]]

[1] https://github.com/marijnh/tern
[2] https://github.com/espadrine/aulx

Comment 3

[Brandon Benvie [:benvie]]

After discussing it with a number of people, the consensus is to use acorn and to address the things it is missing via either contributing changes back to acorn or to ask acorn's owner (Marijn Haverbeke) about getting them implemented.

Comment 4

[Girish Sharma [:Optimizer]]

Be vary that this means that we will have to put some (maybe a lot of ? ) extra effort in:
- Making the Acorn's lexar better (as compared to esprima)
- Having the support of ES6 up to the level so that ACorn is actually useful in Chrome tools like browser debugger, browser level scratchpad, etc.

[This is not a discussion, I am okay with any option. Just wanted put it out here amidst the fact that we are already getting bitten by the lack of ES6 support in other external tools like CM, escodegen etc.]

Comment 5

[Brandon Benvie [:benvie]]

Attachment: acorn.patch

Most of the work is done here. I'd probably like to add some more tests though.

Comment 6

[Brandon Benvie [:benvie]]

Bug 932416 is the legal review submission.

Comment 7

[Brandon Benvie [:benvie]]

Attachment: acorn.patch

Fixes test_same_ast.js, adds test_parse_dammit.js.

Comment 8

[Brandon Benvie [:benvie]]

Attachment: acorn.patch

Adds acorn to license.html.

Comment 9

[Brandon Benvie [:benvie]]

Attachment: acorn.patch

Clean up the tests a bit. https://tbpl.mozilla.org/?tree=Try&rev=1c3c634bf814

Comment 10

[Brandon Benvie [:benvie]]

Comment on attachment 824287 [details] [diff] [review]
acorn.patch

robcee for the general review
dcamp for loader changes
gps for adding a new folder to the build system

Comment 11

[Gregory Szorc [:gps]]

Comment on attachment 824287 [details] [diff] [review]
acorn.patch

Review of attachment 824287 [details] [diff] [review]:
-----------------------------------------------------------------

Covers just the build bits.

Comment 12

[Dave Camp (:dcamp)]

Comment on attachment 824287 [details] [diff] [review]
acorn.patch

Review of attachment 824287 [details] [diff] [review]:
-----------------------------------------------------------------

I hate to be that guy, but could we name the test test_lenient_parser.js?

Comment 13

[Brandon Benvie [:benvie]]

(In reply to Dave Camp (:dcamp) from comment #12)
> I hate to be that guy, but could we name the test test_lenient_parser.js?

Hah, of course.

Comment 14

[Dave Camp (:dcamp)]

Comment on attachment 824287 [details] [diff] [review]
acorn.patch

Review of attachment 824287 [details] [diff] [review]:
-----------------------------------------------------------------

Looks ok to me, assuming that file is renamed.

Comment 15

[Brandon Benvie [:benvie]]

Attachment: acorn.patch

Rebase and a few changes:

* Rename test_parse_dammit.js to test_lenient_parser.js
* Remove package.json.js since it's not actually needed

Comment 16

[Rob Campbell [:rc] (:robcee)]

Comment on attachment 825584 [details] [diff] [review]
acorn.patch

Review of attachment 825584 [details] [diff] [review]:
-----------------------------------------------------------------

looks ok to me too.

Comment 17

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

https://hg.mozilla.org/integration/fx-team/rev/7e81c446b6ca

Comment 18

[Jim Mathies [:jimm]]

https://hg.mozilla.org/mozilla-central/rev/7e81c446b6ca

Comment 19

[Frederik Braun [:freddy] (reo duty for Fx124)]

Sorry for picking up this old bug, but do you have a tracker for porting ES6 features to acorn?

Comment 20

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

I don't think so. Bug 938203 also depends on acorn supporting ES6.

Comment 21

[Ingvar Stepanyan (RReverser)]

Acorn ES6 support is coming in https://github.com/marijnh/acorn/pull/110.