Flow type support

[sebmck]

@RReverser Would you be interested in supporting flow types? I think it'd be a good feature to add, especially since it can be used in conjunction with JSX. I can give this a go if this is something that you're interested in. Would be aiming to pass the entire esprima-fb type test suite.

[RReverser]

@sebmck Yeah, I'm interested in parsing it (while I'm not sure what are we going to transpile it to). But currently got sick and can't really spend much time on this. If you wish, you can start implementing it in a separate branch so we would merge it when everything's done.

[sebmck]

@RReverser Probably just ignore it completely which is what react-tools does currently.

[StoneCypher]

Just came here to ask for this following https://github.com/eslint/eslint/issues/1486 and https://github.com/eslint/eslint/issues/1291#issuecomment-64103087 :)

:+1:

[sebmck]

Started this at https://github.com/RReverser/acorn-jsx/tree/flow-types. I'm going to port it over from esprima-fb to start off with and then clean it up a lot.

[sebmck]

I've committed what I've currently done since I'm not sure if I'll have time to finish it. Currently the tests for interfaces, declare statements and type aliases are commented out. There's some weird behaviour with the range and loc so I suspect that I haven't ported it over from esprima-fb very well. Also some ambiguous parsing of class identifiers.

[sebmck]

Looks like most of the range issues is actually just weird esprima-fb behaviour. For example in function foo(numVal: any){} the numVal Identifier is given an end range of after the any.

@RReverser Should the current ranges be used or do you think that it should be on parity with esprima-fb?

[sebmck]

Currently at a crossroad where the only things left are:

• Ambiguous syntax caused by class Foo<T> extends Bar<T> { }.
• Different function parameter ranges.
• Lack of lookahead making var a: number & (string | bool) impossible to check. See here.
• Arrow function param types such as ((...rest: Array<number>) => rest) because of the weird way acorn parses arrow functions.

@RReverser Any pointers?

[RReverser]

Sorry, got in stuck with other stuff here.

Regarding range - I suppose they do it in order to have node to cover both name and type so there's no characters left in between that don't "belong" to any node by location. I believe it's not critical for us and probably even better to be left separately.

Ambigous syntax should be definitely fixed. What is the problem with class Foo<T> extends Bar<T> { }?

Things like var a: number & (string | bool) should be implemented through back-fix (same way as is used in arrow functions that you call "weird" :) ). Lookahead is pretty expensive operations as you need to re-scan the input twice if your lookahead assumption was true and we still want Acorn to be the fastest one. So just parse identifier, save it to temp variable and look what's the next token (as you do here). Then just pass identifier to actual parsing function (either of group type or function).

[sebmck]

Re: ambiguous syntax. Since super classes can be any expression and not just an identifier the super class is parsed as Bar < T and the stray > causes a syntax error.

[RReverser]

Hm... As for now I don't see better ways to handle this than to add noLess to parseMaybeAssign and use it as parseMaybeAssign(false, true) at extends. This way things like class A extends B < C won't work but they're stupid and shouldn't work anyway.

[sebmck]

I've currently got everything implemented, except for the backfix. It's difficult to implement as all the type functions would have to be modified to allow for an optional id that would be passed.

Sorry about the messy branch history, I'll rebase and submit a PR once I'm done.

[sebmck]

Oh and types within arrow function parameters such as ((...rest: Array<number>) => rest). Not currently possibly because of the way acorn parses arrow function params.

[RReverser]

If you can't handle some cases, leave them and I'll implement them on my own after PR.

[davidpfahler]

@RReverser @sebmck What's the status? I see that babel has a flow transformer, so is this solved? If not, how can I help to move this forward?

[sebmck]

@davidpfahler Babel has a Flow plugin but it relies on the Babel Acorn fork.

[RReverser]

@sebmck Btw now that we have both JSX and Flow implemented as plugins - maybe time to add Flow support to this repo? Should be easier to maintain (since I'll be able to update all the methods at once as soon as breaking version of Acorn appears).

[sebmck]

@RReverser It still relies on the lookahead I patched into Acorn. I remain unconvinced that there are performance implications with having lookaheads since Esprima is already much faster and simpler with them. I also still need to update the Babel internal method names to match the new ones that I added to Acorn core.

[RReverser]

@sebmck As far as I know, @marijnh is now also convinced and interested in lookaheads in Acorn. If you're saying you already have those, feel free to PR.

As for the second part - IIUC, you need to do that with JSX as well, so shouldn't make big difference if we keep everything in one place.

[sebmck]

@RReverser Sure, I'm actually super keen on getting this into acorn-jsx as it's one step further towards not having an internal fork. I'm not super comfortable with my lookahead implementation as it's extremely hacky. It's necessary for the Flow plugin due to grouped types. I've tried implementing it without but have never been able to finish it as you have to pass the parsed expression around and you can't really "resume parsing".