8.4 - Property hooks #1143
8.4 - Property hooks #1143
Conversation
|
@cseufert I would love your feedback on this PR. |
| { | ||
| parser: { | ||
| version: "8.1", | ||
| suppressErrors: true, |
There was a problem hiding this comment.
Yes, because the array of error added to the AST is just gibberish. Only the first error is relevant: once the parser get confused/lost then it just create noise. It is very distracting/confusing when working on thigs and trying to understand what test fails and why.
| "kind": "program", | ||
| } | ||
| `; | ||
| exports[`Test classes Test that readonly method parameters are throwing errors 1`] = `"readonly properties can be used only on class constructor on line 3"`; |
There was a problem hiding this comment.
This looks like a regression.
There was a problem hiding this comment.
This is related to the change you commented in the class.test file https://github.com/glayzzle/php-parser/pull/1143/files#r1865229710
Instead of letting the parser getting lost and generate gibberish errors, the exception is fired and serialized, using toThrowErrorMatchingSnapshot.
| @@ -28,6 +29,7 @@ module.exports = Statement.extends( | |||
| readonly, | |||
| nullable, | |||
| type, | |||
| hooks, | |||
There was a problem hiding this comment.
It would probably make sense to put hooks last as this would be less breaking of a change for existing code. Also making the default [] would make the type signature simpler, and reduce ambiguity between an empty list and null, as an empty property list is not valid syntax.
There was a problem hiding this comment.
Adding the hooks parameter at the end of the list would be inconsistent with how things have been done so far.
The PR introducing support for typed class constant added type "in the middle". https://github.com/glayzzle/php-parser/pull/1136/files#diff-33d9ef64a624f7abad7378a94559475184322ac87b7c864326beea672657adbcR34
There was a problem hiding this comment.
@cseufert I do not think it is possible to put hooks as the last parameters: when we create a node, the parameter docs and location are passed in by some other layer.
There was a problem hiding this comment.
@czosel any opinion on this?
Fixes #1142
RFC - https://wiki.php.net/rfc/property-hooks
Official documentation - https://www.php.net/manual/en/language.oop5.property-hooks.php