Change DeclarativeExpression's management of visibility and storage specifiers

[cgewecke]

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.

[federicobond]

I think this goes in a good direction, but give me a few days to better understand the tradeoffs.

In terms of style, I would have gone further and cleaned up the SourceElement rule, moving the logic to a StateVariableDeclaration.

[federicobond]

Not really a fan of this. It complicates the grammar unnecessarily.

[cgewecke]

I agree with you. It's ugly. But look down the AssignmentExpression path. Won't enforcing this distinction involve something like the above or writing separate path that reaches quite a way down?

[federicobond]

Not necessarily. Just a single rule matching the official specification:
https://github.com/federicobond/solidity-antlr4/blob/master/Solidity.g4#L57

[cgewecke]

So get rid of AssignmentExpression up there and replace with a StateVariableDeclaration that has an optional = expression, visibility and constant specifiers, no storage_location field, a value field that may be an expression or null, and don't preserve the equality operator?

[federicobond]

Yes, it would also get rid of nasty stuff like uint foo += 1; getting parsed incorrectly as a state variable declaration.

[cgewecke]

Ok, I'll revise to that. We have all the pieces here. Getting slightly closer.

[cgewecke]

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