ConsenSysMesh / solidity-parser

Solidity Parser in Javascript
138 stars 54 forks source link

Change DeclarativeExpression's management of visibility and storage specifiers #57

Closed cgewecke closed 7 years ago

cgewecke commented 7 years ago

This PR is based on @federicobond's review of PR #44. He suggested the parser should distinguish between StateVariable declarations (which may be constant and have an optional visibility specifier) and local declarations which only have an optional storage specifier. The solution discussed in #44 involved moving constant and visibility handling to a new StateVariableDeclaration rule under SourceElement and leaving DeclarativeExpression for locals.

The PR below takes a slightly different approach, in part because subsequent developments in #56. It keeps all specifier management in the DelcarativeExpression and adds a check at SourceElement --> AssignmentExpression which verifies that AssignmentExpression's left node does not have a storage specifier. It also modifies the entry for DeclarativeExpression at SourceElement to return only the node, in preparation for whatever will be done for #56.

Additionally, one line has been commented out in the existing .sol test-bed because solc doesn't seem to compile it.

library VarHasBrackets {
    string constant specialRight = "}";
    string storage specialLeft = "{"; // <-- Is this legal?
}

In sum: PR fixes #26. StateVariables can no longer be assigned a storage specifier and locals can no longer be designated constant or have visibility specifiers.

A gist of the AST for the added test can be found here.

@federicobond I'm about to add you as a collaborator on my fork of solidty-parser so you are free to revise / simplify this PR as you wish. I'm also happy to update with any suggestions you have. Also happy to close if this is not the solution you're looking for. 😄

Build not included.

cgewecke commented 7 years ago

@federicobond Made those revisions and cleaned things up a bit. Also updated AST gist