Skip to content

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

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 ondrejmirtes force-pushed the add_rule_call_constructor branch from 47828c4 to 6fa97ee Compare April 17, 2022 11:02
@ondrejmirtes ondrejmirtes force-pushed the add_rule_call_constructor branch from 6fa97ee to 4daa27c Compare April 17, 2022 11:03
@ondrejmirtes
Copy link
Member

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

@ondrejmirtes ondrejmirtes force-pushed the add_rule_call_constructor branch from 535a03e to 18930cc Compare April 17, 2022 11:10
@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

Development

Successfully merging this pull request may close these issues.

5 participants