ConsenSysMesh / solidity-parser

Solidity Parser in Javascript
137 stars 53 forks source link

Need support for 'var (x, y)' #9

Closed duaraghav8 closed 7 years ago

duaraghav8 commented 8 years ago

Originally reported by 4gn3s

Following code

var (x, y) = (10, 20)

is legal in Solidity, but not yet parsed by SP. Though it recognises (10, 20) as SequenceExpression, var (x, y) causes parsing error.

Will be making a PR for this soon.

raineorshine commented 7 years ago

Yup, ran into this, too (with the help of @nicksavers). Somewhere around here: https://github.com/ConsenSys/solidity-parser/blob/master/solidity.pegjs#L1123-L1142

duaraghav8 commented 7 years ago

@raineorshine I've implemented this feature on my fork (npm install solparse) and am trying to get solidity devs to test it. Once I'm satisfied, I'll push the changes here. If you find time, do test it out and let me know in case improvements need to be made

Samples of statements I've been testing out: var (x, y, z) = (10, "Hello world", 9028.2897); var (x, y, z) = getMyTuple ();

raineorshine commented 7 years ago

Awesome, thanks!

On Sun, Oct 9, 2016 at 1:39 PM Raghav Dua notifications@github.com wrote:

@raineorshine https://github.com/raineorshine I've implemented this feature on my fork (npm install solparse) and am trying to get solidity devs to test it. Once I'm satisfied, I'll push the changes here. If you find time, do test it out and let me know in case improvements need to be made

Samples of statements I've been testing out: var (x, y, x) = (10, "Hello world", 9028.2897); var (x, y, x) = getMyTuple ();

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ConsenSys/solidity-parser/issues/9#issuecomment-252507538, or mute the thread https://github.com/notifications/unsubscribe-auth/AAtyxESj3fLHj1p0EsOKlgKbLJL9WLu5ks5qyUMBgaJpZM4JuRKy .

cgewecke commented 7 years ago

@duaraghav8 Do you have any interest in submitting a PR to SP for this?

duaraghav8 commented 7 years ago

ok so currently something like:

var (x, y) = (10, 20)

gets parsed into:

{
  "type": "VariableDeclarationTuple",
  "declarations": [
    {
      "type": "VariableDeclarator",
      "id": {
        "type": "Identifier",
        "name": "x",
        "start": 5,
        "end": 6
      },
      "init": null,
      "start": 5,
      "end": 6
    },
    {
      "type": "VariableDeclarator",
      "id": {
        "type": "Identifier",
        "name": "y",
        "start": 7,
        "end": 8
      },
      "init": null,
      "start": 7,
      "end": 8
    }
  ],
  "init": {
    "type": "SequenceExpression",
    "expressions": [
      {
        "type": "Literal",
        "value": 10,
        "start": 13,
        "end": 15
      },
      {
        "type": "Literal",
        "value": 20,
        "start": 17,
        "end": 19
      }
    ]
  },
  "start": 0,
  "end": 20
}

I'd like opinions on this before I push, @federicobond @tcoulter @raineorshine @cgewecke

@cgewecke as you mentioned here, could you tell me why the current code is still a crasher?

cgewecke commented 7 years ago

@duaraghav8 SP still errors on the comma after x, expecting a close paren.

var(x,y) = (10,20);

So you would open a PR that adds the variable tuple parsing. . . basically from your line 1160 to your line 1187. One difference is that the 'noInit' rules have been removed from SP recently. Are those required for your implementation to work?