incorrect parse for destructuring dssignment

[ltcmelo]

I suspect that the assignment_expression in following piece of code is parsed incorrectly:

let o = { x: "s", y: 0 };
({x: v, y: w} = o);

Here's the output that I get for it:

       assignment_expression   text: "{x: v, y: w} = o"
         object_pattern   text: "{x: v, y: w}"
           {   text: "{"
           pair_pattern   text: "x: v"
             property_identifier   text: "x"
             :   text: ":"
             identifier   text: "v"
           ,   text: ","
           pair_pattern   text: "y: w"
             property_identifier   text: "y"
             :   text: ":"
             identifier   text: "w"
           }   text: "}"
         =   text: "="
         identifier   text: "o"

If my suspicion is correct, then the pattern underneath the assignment should be an object_assignment_pattern rather than an object_pattern. What do you think?

Note: Related to this, I think that the "circumstances" in which the grammar of an AssignmentPattern (including an ObjectAssignmentPattern) applies isn't clear enough in the ECMA spec, so I create this issue there.

[jmdyck]

If my suspicion is correct, then the pattern underneath the assignment should be an object_assignment_pattern rather than an object_pattern. What do you think?

I might be misreading the grammar, but it looks to me like {x: v, y: w} doesn't have the correct syntax for object_assignment_pattern. If you change the example to {x: v, y=1}, then I think the y=1 is an object_assignment_pattern.

(This is made more confusing because object_assignment_pattern is unrelated to ECMAScript's ObjectAssignmentPattern.)

[ltcmelo]

Oh... I thought that object_assignment_pattern would correspond to ObjectAssignmentPattern! Thanks @jmdyck, object_assignment_pattern indeed expects = and an expression.

So an object_pattern seems to represent both ECMAScript's ObjectBindingPattern and ObjectAssignmentPattern. Despite equivalent syntactical elements within them, I wonder whether having different nodes for them would be beneficial, given that they represent different productions?

[jmdyck]

I suspect that fidelity with the ECMAScript grammar is a non-goal for tree-sitter-javascript.

[ltcmelo]

I suspect that fidelity with the ECMAScript grammar is a non-goal for tree-sitter-javascript.

I see... and I think that's fine; nevertheless it might be interesting to avoid non-terminal names that are the same as those in ECMAScript yet with a different meaning. Would do you say?

[jmdyck]

it might be interesting to avoid non-terminal names that are the same as those in ECMAScript yet with a different meaning.

That would reduce the likelihood of misunderstandings among people who are looking at both grammars. So it would have been a good idea to adopt at the start, but might be too disruptive to adopt now. (The project may make backward-compatibility guarantees re nonterminal names, I don't know.)

[amaanq]

it's okay to be disruptive - we're still pre 1.0 overall, and breaking changes aren't something we're against (though typically it should be for a good reason, and imo, achieving fidelity with the ECMAScript grammar is a good reason)

If you want to tackle this @ltcmelo, feel free to, else I can when I get time