javascript-decorators
javascript-decorators copied to clipboard
Shift-reduce conflicts in grammar
The current grammar has multiple shift-reduce conflicts.
-
Decoratorneeds to useLeftHandSideExpressioninstead ofAssignmentExpression, otherwise there is a conflict betweenMultiplicativeExpressionandGeneratorMethod. Example:{ @ F * G () {} }. -
CoverMemberExpressionSquareBracketsAndComputedPropertyNameisn't sufficient to handle bracket member expression and computed property names + formal parameters. Example:{ @ D ["m"] () {} }. Parse as@ D+["m"]() {}or as@ D["m"]()+{}?
I'd propose to simply disallow bracket member expressions in decorator expressions:
+1000 for the first part. It would also fix the second part, as long as it's parsed as Descriptor MethodDefinition*
The current behavior is surprising, and that would definitely fix the ambiguity with annotating generators.
:-1: for the proposed fix for the second part. The least surprising behavior would be it parsing as @ D + ["m"] () {}. There's no ambiguity if it's parsed as @ LeftHandSideExpression MethodDefinition, and it's parsed this way.
:+1: on 1
:-1: on 2. I suspect that bracket notation is going to be used way more than before, given it’s currently the only way to use symbols.
There's no ambiguity if it's parsed as @ LeftHandSideExpression MethodDefinition, and it's parsed this way.
Here's a simplified version of the current grammar, when you process it with bison you can easily identify the shift/reduce conflicts with the coverMemberExpressionSquareBracketsAndComputedPropertyName approach.
%start classElement
%error-verbose
%token ID
%%
classElement : decorator methodDefinition
| methodDefinition
;
decorator : '@' leftHandSideExpression ;
methodDefinition : propertyName '(' formalParameters ')' '{' '}' ;
propertyName : coverMemberExpressionSquareBracketsAndComputedPropertyName ;
formalParameters : /* empty */
| formalsList
;
formalsList : formalParameter
| formalsList ',' formalParameter
;
formalParameter : bindingIdentifier ;
expression : assignmentExpression ;
assignmentExpression : leftHandSideExpression ;
leftHandSideExpression : callExpression
| memberExpression
;
callExpression : memberExpression arguments ;
arguments : '(' ')'
| '(' argumentsList ')'
;
argumentsList : assignmentExpression
| argumentsList ',' assignmentExpression
;
memberExpression : primaryExpression
| memberExpression coverMemberExpressionSquareBracketsAndComputedPropertyName
;
coverMemberExpressionSquareBracketsAndComputedPropertyName : '[' expression ']' ;
primaryExpression : identifierReference ;
identifierReference : ID ;
bindingIdentifier : ID ;
%%
FYI: Just fixed (1) in https://github.com/wycats/javascript-decorators/pull/34
The grammar adds CoverMemberExpressionSquareBracketsAndComputedPropertyName to MemberExpression, which is the avenue NewExpression can take that includes terminal bracket notation. However, LHS is inclusive of CallExpression as well, and CallExpression may also have terminal bracket notation (item 4):
CallExpression[Yield] :
MemberExpression[?Yield] Arguments[?Yield]
SuperCall[?Yield]
CallExpression[?Yield] Arguments[?Yield]
CallExpression[?Yield] [ Expression[In, ?Yield] ]
CallExpression[?Yield] . IdentifierName
CallExpression[?Yield] TemplateLiteral[?Yield]
Wouldn't the grammar need to use CoverMemberExpressionSquareBracketsAndComputedPropertyName here as well?