Add failing test for GetParameterToConstructorInjectionRector #275
Conversation
The expected behavior would be that the parent constructor is only called once.
|
That is indeed strange we ca probably handle this in the rector rule @mttsch do you want to fix it yourself? |
|
@johanadivare I think that I am too unfamiliar with the code to fix this in the near future. |
Fair i had a quick look but i dont see where the double constructor is added |
|
@johanadivare I took a quick look when creating this issue and I think it relates to |
If a promoted property is added to a class constructor and if this constructor method is already present in the class, there is no need to add a parent constructor call to this method just because a new property is added to the child's constructor. Adding the parent constructor call is only necessary when the child class currently has no constructor when the promoted property is added so that a new constructor method has to be added to the child class. Fixes rectorphp/rector-symfony#275
|
@johanadivare I think that I have found the issue and fixed it in rectorphp/rector-src#3094. |
If a promoted property is added to a class constructor and if this constructor method is already present in the class, there is no need to add a parent constructor call to this method just because a new promoted property is added to the child's constructor. Adding the parent constructor call is only necessary when the child class currently has no constructor when the promoted property is added so that a new constructor method has to be added to the child class. Fixes rectorphp/rector-symfony#275
|
@TomasVotruba Should this PR be reopened because it was auto-closed by merging rectorphp/rector-src@9a93704? |
|
It seems this was a bug that was fixed in rectorphp/rector-src@9a93704 If it's not the case, it should be re-opened. |
|
@TomasVotruba Yes, it was fixed by that commit but the test in that commit covers a different rule than this test does so it would cover the change for two different scenarios, which might not be necessary. I was asking about the reopening because I lack the permission to do so. |
|
I see :) In that case, next time the "closes x" should link only issue, not the PR. This test fixture should've been part of the fixing PR so it's complete work in one PR. |
The fix was in a different repository, thus different PRs. |
|
I see, got it 👍 Thanks for explaning |
The expected behavior would be that the parent constructor is only called once.