Skip to content

Fix the behavior of ClassPropertyAssignToConstructorPromotionRector when types do not match#7972

Merged
TomasVotruba merged 14 commits intorectorphp:mainfrom
mspirkov:fix-property-promotion
Apr 23, 2026
Merged

Fix the behavior of ClassPropertyAssignToConstructorPromotionRector when types do not match#7972
TomasVotruba merged 14 commits intorectorphp:mainfrom
mspirkov:fix-property-promotion

Conversation

@mspirkov
Copy link
Copy Markdown
Contributor

I encountered a problem where the property type and parameter type did not match, and the parameter type was replaced with the property type. This led to an issue because the DI container could only resolve a specific implementation of the interface and could not resolve the interface itself, resulting in a crash when attempting to instantiate the class.
I believe it would be more appropriate to use the parameter type in this case. I would appreciate your feedback :)

@samsonasik
Copy link
Copy Markdown
Member

samsonasik commented Apr 21, 2026

Could you show the diff on https://getrector.com/demo/ first? thank you.

@mspirkov
Copy link
Copy Markdown
Contributor Author

mspirkov commented Apr 21, 2026

Yes. Here's the diff: https://getrector.com/demo/4534848e-f616-4cf9-ae28-46adbf6a8859

These changes can cause the following errors:

Exception: Entry "WithInterfaceAndItsImplementation" cannot be resolved: Entry "SomeInterface" cannot be resolved: the class is not instantiable

@TomasVotruba TomasVotruba requested a review from samsonasik April 21, 2026 19:31
Comment thread rules/Php80/Rector/Class_/ClassPropertyAssignToConstructorPromotionRector.php Outdated
@mspirkov mspirkov requested a review from samsonasik April 22, 2026 08:34
@samsonasik
Copy link
Copy Markdown
Member

Could you split into separate fixtures per new use case? That will be easier to check

@mspirkov
Copy link
Copy Markdown
Contributor Author

Could you split into separate fixtures per new use case? That will be easier to check

It sounds logical. I'll do it now

@mspirkov
Copy link
Copy Markdown
Contributor Author

I made separate fixtures

Copy link
Copy Markdown
Member

@samsonasik samsonasik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me 👍

@samsonasik samsonasik requested a review from TomasVotruba April 23, 2026 15:40
@TomasVotruba TomasVotruba merged commit e300872 into rectorphp:main Apr 23, 2026
61 checks passed
@TomasVotruba
Copy link
Copy Markdown
Member

Let's go:) thank you

@mspirkov mspirkov deleted the fix-property-promotion branch April 23, 2026 15:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants