Consistent returns for read_variable_declaration#1141
Conversation
| value = this.next().read_expr(); | ||
| } else { | ||
| this.expect([",", ";", "="]); | ||
| return result(propName, null, nullable, type, attrs || []); |
There was a problem hiding this comment.
here, readonly is not passed.
| "kind": "identifier", | ||
| "name": "onst", | ||
| }, | ||
| "nullable": Name { |
There was a problem hiding this comment.
The snapshot show that what was set was incorrect: instead of a boolean value we had some objects.
| ); | ||
| value = this.next().read_expr(); | ||
| } else { | ||
| this.expect([",", ";", "="]); |
There was a problem hiding this comment.
I think we need to drop the "=" in th eexpect, we already handled that with the previous if condition.
| this.expect([",", ";", "="]); | |
| this.expect([",", ";"]); |
There was a problem hiding this comment.
"," and ";" are handled by the first if.
I think this line is here to show an error message indicating that we expect one of those 3 things.
I figured this out while working on adding support for property hooks.
cseufert
left a comment
cseufert
left a comment
There was a problem hiding this comment.
Looks good, if we could just streamline the if else when looking for a default value, they we can merge this.
|
I do not have permission to merge 😄 . |
3 participants
While working on adding support for property hooks, I realized that the function parsing class property does not always call the "return" function with the same parameters: in some instance, the parameter
readonlyis not being passed done.This PR fix that by having 1 call to return despite multiple branches in the logic.