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

Add rule call constructor #1208

Merged
merged 9 commits into from Apr 17, 2022
Merged

Conversation

muno92
Copy link
Contributor

@muno92 muno92 commented Apr 12, 2022

Implementation for phpstan/phpstan/issues/7022.

Notes

  • Check calling __construct more than twice in constructor is out of scope
    Because it is a bit difficult.

@zonuexe
Copy link
Contributor

zonuexe commented Apr 12, 2022

Already pointed out how to define rules
phpstan/phpstan#7022 (comment) phpstan/phpstan#7022 (comment)

@muno92
Copy link
Contributor Author

muno92 commented Apr 12, 2022

Already pointed out how to define rules phpstan/phpstan#7022 (comment) phpstan/phpstan#7022 (comment)

I'll split Rule


return $this->isCollectCallingConstructor($node, $scope)
? []
: ['__construct should not be called outside constructor.'];
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
: ['__construct should not be called outside constructor.'];
: ['__construct() should not be called outside constructor.'];

Copy link
Contributor Author

Choose a reason for hiding this comment

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

LGTM

This message is written in other lines, so I changed those.

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.

Thank you. I'm gonna take over and bring it over the finish line 😊

@sasezaki
Copy link
Contributor

[IMO]
The rules in this PR implementation also determine that $self->__construct is also a violation,
eg. https://phpstan.org/r/0affd0af-b9e0-4ffe-9a69-ff7915120d19

but since the number of use cases is expected to be small, it can be ignored.

@muno92
Copy link
Contributor Author

muno92 commented Apr 13, 2022

[IMO]
The rules in this PR implementation also determine that $self->__construct is also a violation,
eg. https://phpstan.org/r/0affd0af-b9e0-4ffe-9a69-ff7915120d19
but since the number of use cases is expected to be small, it can be ignored.

I think, above case is no need of using __construct().
https://3v4l.org/v25j9

Is this wrong understanding?

@sasezaki
Copy link
Contributor

I think, above case is no need of using __construct().

Yep, it can be rewrite such of your example.
There is no problem with $self->__construct being a rule error, I said.

@muno92
Copy link
Contributor Author

muno92 commented Apr 13, 2022

Yep, it can be rewrite such of your example. There is no problem with $self->__construct being a rule error, I said.

Sorry. I misread your first comment.

@ondrejmirtes
Copy link
Member

I've pushed some improvements, feel free to check out the commits :)

@ondrejmirtes ondrejmirtes merged commit 58d7df3 into phpstan:1.6.x Apr 17, 2022
@ondrejmirtes
Copy link
Member

Thank you.

@muno92
Copy link
Contributor Author

muno92 commented Apr 17, 2022

Thank you, too.

@ondrejmirtes
Copy link
Member

FYI I've moved these rules to phpstan-strict-rules (124b30f, phpstan/phpstan-strict-rules@0c82c96) because it's a better fit there. According to some opinions, these rules are too opinionated to be part of the "mainstream" PHPStan. Thanks for understanding.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants