search

glayzzle / php-parser

:herb: NodeJS PHP Parser - extract AST or tokens
https://php-parser.glayzzle.com/
BSD 3-Clause "New" or "Revised" License
534 stars 71 forks source link

feat: support for readonly class #1046

Closed genintho closed 1 year ago

genintho commented 1 year ago

Is this approach ok?

According to https://php.watch/versions/8.2/readonly-classes

Abstract classes final classes can also be declared readonly. The order of the keywords does not make a difference.

I know some tests will fail, I am showing this to verify that my approach is ok.

genintho commented 1 year ago

@czosel would love your feedback on this approach.

czosel commented 1 year ago

Hi @genintho - the general approach looks good to me! :+1:

genintho commented 1 year ago

Not sure why CI is breaking now. Running the typescript command locally show no difference. 🤔 I must be missing something.

genintho commented 1 year ago

@czosel this is ready for review.

I had to install @types/node to fix CI. In the CI, we globally install the most recent version of typescript, which conflict with the version implicitly installed in the repo.

I would love an alternative solution, but I already waste a couple of hours on this.

czosel commented 1 year ago

@genintho it seems the typescript issue was due to outdated transitive dependencies (see here for details) - I just fixed it in #1050 by regenerating the lock file. Can you revert the changes to package.json and yarn.lock and rebase on main?

genintho commented 1 year ago

@czosel happy new year! 😄

I did what you asked. We should be good to go.

czosel commented 1 year ago

@genintho thank you, all the best for you as well! :tada:

LGTM :+1:

czosel commented 1 year ago

Released in v3.1.3 :tada: