Skip to content
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

Implement @phpstan-consistent-constructor #1296

Merged
merged 5 commits into from
May 9, 2022

Conversation

canvural
Copy link
Contributor

@canvural canvural commented May 8, 2022

This PR adds a new tag called @phpstan-consistent-constructor and a rule to enforce it.

I wasn't sure which level this rule should be in, so for now I added it to level 0. But I can change it to correct one.

There are not many test cases because same things are covered by OverridingMethodRule tests. I only tested relevant stuff for this rule. But of course I can add more test cases, if there is an edge case we need to cover.

canvural added 4 commits May 8, 2022 11:01

Verified

This commit was signed with the committer’s verified signature.
axlon Choraimy Kroonstuiver
@canvural canvural force-pushed the consistent-constructor branch from f1abb37 to f5415d5 Compare May 8, 2022 15:09
Copy link
Member

@ondrejmirtes ondrejmirtes left a comment

Choose a reason for hiding this comment

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

This is almost perfect :) I think I'd like to see some edge cases handled. I think that @phpstan-consistent-constructor should be allowed above a class that doesn't have constructor at all.

Please add a test that it's possible to extend such class and that: 1) missing constructor in a subclass is not reported, 2) subclass can have a constructor with no parameters (optional parameters also allowed, 3) subclass cannot have a constructor with required parameters.

@ondrejmirtes
Copy link
Member

And also, please open a PR in https://github.com/phpstan/phpstan that updates this blogpost :) https://phpstan.org/blog/solving-phpstan-error-unsafe-usage-of-new-static Thanks

@canvural
Copy link
Contributor Author

canvural commented May 9, 2022

Added the test cases. And yeah, this case was not covered, so fixed it.

@ondrejmirtes ondrejmirtes merged commit 74c3f38 into phpstan:1.7.x May 9, 2022
@ondrejmirtes
Copy link
Member

Thank you!

@canvural canvural deleted the consistent-constructor branch May 9, 2022 11:50
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.

None yet

2 participants