Skip to content

[PropertyInfo] Implement Virtual Properties and Asymmetric Visibility support #58960

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 4 commits into from
Closed

[PropertyInfo] Implement Virtual Properties and Asymmetric Visibility support #58960

wants to merge 4 commits into from
+104 −30

Conversation

pan93412
Copy link
Contributor

@pan93412 pan93412 commented Nov 21, 2024 •
edited
Loading

Q A
Branch? 7.3
Bug fix? no
New feature? no
Deprecations? no
Issues Fix #58556
License MIT

Not sure if this is a bug fix or new feature, since this patch should be backported to Symfony versions that declares supporting 8.4 🤔

  • [PropertyInfo] Add Php84Dummy with Property Hooks and Asymmetric Property Visibility
  • [PropertyInfo] Implement PHP 8.4 support for isAllowedProperty
  • [PropertyInfo] Implement PHP 8.4 support for getWriteVisiblityForProperty

@pan93412 pan93412 requested a review from dunglas as a code owner November 21, 2024 14:43
@carsonbot carsonbot added this to the 7.3 milestone Nov 21, 2024
@xabbuh
Copy link
Member

xabbuh commented Nov 21, 2024

Please rebase your PR against the 5.4 branch as this is something that we will add as a bugfix.

Side note, I also opened #58959 which covers the support for asymmetric visibility. Maybe you can check if/how both PRs differ.

@pan93412
Copy link
Contributor Author

pan93412 commented Nov 21, 2024 •
edited
Loading

Please rebase your PR against the 5.4 branch as this is something that we will add as a bugfix.

Side note, I also opened #58959 which covers the support for asymmetric visibility. Maybe you can check if/how both PRs differ.

Good idea! I believe this PR is a superset of #58959, as it also fixes writeMutator and add more test cases. How about opening another PR that includes both of our changes, since your PR can pass the 8.2 and 8.3 tests?

@pan93412 pan93412 force-pushed the pan93412/fix-gh-58556 branch from aa1d379 to 9bfec15 Compare November 21, 2024 15:07
@pan93412 pan93412 mentioned this pull request Nov 21, 2024
Closed
@pan93412
Copy link
Contributor Author

Please rebase your PR against the 5.4 branch as this is something that we will add as a bugfix.

Side note, I also opened #58959 which covers the support for asymmetric visibility. Maybe you can check if/how both PRs differ.

I have separated this huge PR to #58963 and #58962. Thanks for your help!

@pan93412 pan93412 closed this Nov 21, 2024
@pan93412 pan93412 deleted the pan93412/fix-gh-58556 branch November 21, 2024 15:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dunglas dunglas Awaiting requested review from dunglas dunglas is a code owner

Assignees
No one assigned
Labels
PropertyInfo Status: Needs Review
Projects
None yet
Milestone
7.3
Development

Successfully merging this pull request may close these issues.

[PropertyInfo] PropertyInfo is not compatible with PHP 8.4 property hooks
3 participants
@pan93412 @xabbuh @carsonbot