(Seemingly) irrelevant data being created

duaraghav8 commented 7 years ago

Code:

contract Foo {
    bytes32 x;
}

sp.parse (code).body [0] yields:

{
  "type": "ContractStatement",
  "name": "Full",
  "is": [],
  "body": [
    [
      {
        "type": "DeclarativeExpression",
        "name": "x",
        "literal": {
          "type": "Type",
          "literal": "bytes32",
          "members": [],
          "array_parts": [],
          "start": 15,
          "end": 22
        },
        "is_constant": false,
        "is_public": false,
        "is_private": false,
        "is_internal": false,
        "is_storage": false,
        "is_memory": false,
        "start": 15,
        "end": 24
      },
      [],
      [
        [],
        ";"
      ]
    ]
  ],
  "start": 0,
  "end": 26
}

Observer the body of the ContractStatement (first object is DeclarativeExpression, which is fine. But what's happening after this?). Its taking semicolon as a separate entity, whereas the semicolon, together with an expression before that, make ExpressionStatement.

My fix in solparse creates the following structure:

{
  "type": "ContractStatement",
  "name": "Full",
  "is": [],
  "body": [
    {
      "type": "ExpressionStatement",
      "expression": {
        "type": "DeclarativeExpression",
        "name": "x",
        "literal": {
          "type": "Type",
          "literal": "bytes32",
          "members": [],
          "array_parts": [],
          "start": 15,
          "end": 22
        },
        "is_constant": false,
        "is_public": false,
        "is_private": false,
        "is_internal": false,
        "is_memory": false,
        "start": 15,
        "end": 24
      },
      "start": 15,
      "end": 25
    }
  ],
  "start": 0,
  "end": 26
}

@cgewecke is working on moving all fixes in solparse to solidity-parser. Please comment on the structure elicited by solparse above, so we can finalize.

cgewecke commented 7 years ago

@duaraghav8 What did you change in solidity-parser to get the solparse output?

duaraghav8 commented 7 years ago

@cgewecke https://github.com/duaraghav8/solparse/blob/master/solidity-parser.pegjs#L1230

federicobond commented 7 years ago

OK, the problem is caused by this rule:

SourceElement
  = AssignmentExpression __ EOS
  / DeclarativeExpression __ EOS
  / EnumDeclaration
  ...

Those first two rules do not parse to AssignmentExpression or DeclarativeExpression nodes but to arrays with the whitespace and EOS result as items too.

I propose to define StateVariableDeclaration as this:

StateVariableDeclaration
  = declaration:(AssignmentExpression / DeclarativeExpression) __ EOS
  {
    // Do some node manipulation here
  }

And replace it in the SourceElement rule, which would also bring the parser more in line with the official grammar.

The problem is that the layout for the AssignmentExpression and DeclarativeExpression are very different. AssignmentExpression has left, right and operator keys, where left is a DeclarativeExpression. StateVariableDeclaration should return a node with a consistent layout. Since operator does not make sense in this context, perhaps value should be the key containing the right-hand side expression for this node.

What do you think?

ConsenSysMesh / solidity-parser

(Seemingly) irrelevant data being created #56