Skip to content

fix: Pass promoted property default value#1555

Open
aragon999 wants to merge 3 commits intoRoave:6.70.xfrom
aragon999:fix/promoted-property-default-value
Open

fix: Pass promoted property default value#1555
aragon999 wants to merge 3 commits intoRoave:6.70.xfrom
aragon999:fix/promoted-property-default-value

Conversation

@aragon999
Copy link
Copy Markdown

@aragon999 aragon999 commented Mar 31, 2026

I ran into the issue, when changing normal properties to promoted properties: shopware/shopware#15889 in combination with the roave-backward-compatibility-check.

I think actually the test Roave\BetterReflectionTest\Reflection\ReflectionPropertyTest::testIsPromoted had a bug, but I still added a new one, according to the contribution guidelines.

@Ocramius
Copy link
Copy Markdown
Member

Build failure is not related to your work, @aragon999: patch is valid.

Man, the infection+psalm issue is plaguing this repository now.

@aragon999
Copy link
Copy Markdown
Author

I see, thank you for the quick response, and also thank you for the correction of the import order 🙈

I'm afraid I don't know enough about the combination of "infection+psalm" to be of help in that regard, but thank you for maintaining the project :-)

@aragon999
Copy link
Copy Markdown
Author

I noticed that with this change, reflection helper fails when the default value is an object. I will look into it, how it can be solved.

@Ocramius
Copy link
Copy Markdown
Member

I may need to roll back the dependencies in here: will first try and see if I can reproduce the problem locally, or if it's a CI issue.

@Ocramius
Copy link
Copy Markdown
Member

reflection helper fails when the default value is an object.

That's normal: BetterReflection cannot instantiate any objects, since that would imply autoloading, which is forbidden by the core invariant of this library.

The entire point of BetterReflection is to reflect without causing side-effects (which occur in ext-reflection instead)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants