This repository has been archived by the owner. It is now read-only.

Parse flow nested array type annotations like number[][] #219

Merged
merged 3 commits into from Nov 13, 2016

Conversation

Projects
None yet
4 participants
@bxt
Copy link
Contributor

bxt commented Nov 12, 2016

Q A
Bug fix? yes
Breaking change? no
New feature? yes
Deprecations? no
Spec compliancy? yes?
Tests added/pass? yes
Fixed tickets #217
License MIT

Flow supports nested array types like number[][], while babylon throws a syntax error, as noted in #217 . This error caused problems upstream, e.g. when trying to strip the types with babel, see babel/babel#4831. The workaround was to use Array<number[]>. To solve this, I adjusted the flowParsePostfixType method to parse 0-n [] postfixes and return the correct AST. I added test cases with:

var a: number[][]
var a: number[][][]

For the tests' expected.jsons I copied flow's ASTs from astexplorer.net and adjusted the range keys etc. to match babylon's format. I was curious if there could be any interference with JS arrays, so I checked with the code of flow's parser. And indeed – they check for line terminators there, so I added another test case with:

var a: number
[]

And made sure this parses as a program with a body of a VariableDeclaration and an ExpressionStatement.

Let me know what you think. ☺️

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Nov 12, 2016

Current coverage is 97.69% (diff: 100%)

Merging #219 into master will increase coverage by <.01%

@@             master       #219   diff @@
==========================================
  Files            19         19          
  Lines          3212       3214     +2   
  Methods         336        336          
  Messages          0          0          
  Branches        838        838          
==========================================
+ Hits           3138       3140     +2   
  Misses           30         30          
  Partials         44         44          

Powered by Codecov. Last update 6cb0235...9e0fcac

@danez

This comment has been minimized.

Copy link
Member

danez commented Nov 12, 2016

Awesome, thanks for the contribution. Looks good, but I'm currently on my phone will check later when I'm home.

@hzoo hzoo added the Tag: Bug Fix label Nov 12, 2016

@danez danez added the area: flow label Nov 13, 2016

@danez

danez approved these changes Nov 13, 2016

Copy link
Member

danez left a comment

Thanks, looks perfect.

@danez danez merged commit 01ed943 into babel:master Nov 13, 2016

3 checks passed

codecov/patch 100% of diff hit (target 97.69%)
Details
codecov/project 97.69% (+<.01%) compared to 6cb0235
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@hzoo

This comment has been minimized.

Copy link
Member

hzoo commented Nov 14, 2016

Awesome contribution @bxt (also your issue description was beautiful)

@bxt bxt deleted the bxt:fix-217 branch Nov 14, 2016

@bxt

This comment has been minimized.

Copy link
Contributor Author

bxt commented Nov 14, 2016

@danez @hzoo Thank you ☺️ nice to work with you.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.