federicobond / solidity-parser-antlr

A Solidity parser for JS built on top of a robust ANTLR4 grammar
MIT License
155 stars 55 forks source link

Drop ParameterList and Parameter node types #76

Closed federicobond closed 5 years ago

federicobond commented 5 years ago

As mentioned here I am considering dropping the ParameterList and Parameter node types, and replacing them with VariableDeclaration (or list of VariableDeclaration).

I know this is a breaking change for at least prettier-solidity, so I would love input from @mattiaerre and @Janther. The reality is that those nodes were really not intended to be part of the original AST representation, they are created implicitly by visiting the equally named ANTLR rules.

In the long run, I would like to get rid of them, considering they do not add any expressive power to the AST, but I want to make sure I don't create a lot of trouble for you guys? Would it be relatively easy to adapt to this change?

Janther commented 5 years ago

I'm happy with this change. the code is now very modular so removing ParameterList and Parameter and replacing them with VariableDeclaration will only mean for us to remove those files and check how does printing a variable declaration differs from printing a parameter. Will have to give this a go once I see the AST against our tests but they are extensive enough that will give us most use cases.

federicobond commented 5 years ago

Opened https://github.com/federicobond/solidity-parser-antlr/pull/77 with the changes.