-
Notifications
You must be signed in to change notification settings - Fork 504
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.
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
Do not report constructor unused parameter if class implements an Interface that defines a constructor #3777
Do not report constructor unused parameter if class implements an Interface that defines a constructor #3777
Conversation
Again there is a failing check which seems unrelated to this PR |
@@ -45,6 +45,12 @@ public function processNode(Node $node, Scope $scope): array | |||
return []; | |||
} | |||
|
|||
foreach ($node->getClassReflection()->getInterfaces() as $interface) { | |||
if ($interface->hasMethod('__construct')) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hasConstructor
would be more precise
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And above instead of strtolower($method->getName()) !== '__construct'
we should use $method->isConstructor()
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ondrejmirtes rebased and updated
this PR made me think of classes which implement more then one interface and multiple https://3v4l.org/easGk maybe thats something to loot at next (followup PR) @carlos-granados :) - as we are missing a reported error |
14e0d1e
to
d989c87
Compare
we need a similar fix for abstract base-classes https://phpstan.org/r/2652fd08-bb25-431a-8f0f-b28b3c096dc6 |
d989c87
to
fa389ed
Compare
…erface that defines a constructor
fa389ed
to
1698c7f
Compare
I rebased this branch and fixed the linting issue |
Thank you. For completeness it should also look for abstract constructor in parent classes and implemented traits. |
If the class implements an interface that defines a constructor, then the class constructor needs to match the signature of the constructor in the interface. In this case we should not report any unused constructor parameters and this is what this PR achieves
Fixes phpstan/phpstan#11454