-
-
Notifications
You must be signed in to change notification settings - Fork 62
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
CS 4.0, PHPStan 0.9, PHPUnit 7, static asserts, composer.lock, merged stages #43
CS 4.0, PHPStan 0.9, PHPUnit 7, static asserts, composer.lock, merged stages #43
Conversation
1fda269
to
4f8a477
Compare
4f8a477
to
a190f41
Compare
a190f41
to
62dc422
Compare
34d0205
to
b288091
Compare
Hmm I think I totally confused Composer with this crazy branch name, had to do fixups in different normal branch & do reset... Really strange. |
phpstan.tests.neon
Outdated
@@ -1,3 +1,6 @@ | |||
parameters: | |||
ignoreErrors: | |||
- '#::__construct\(\) does not call parent constructor from#' | |||
|
|||
# dynamic properties consufe static analysis |
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.
-consufe
+confuse
$this->instantiator->instantiate('DoctrineTest\\InstantiatorTestAsset\\SimpleSerializableAsset'); | ||
$this->instantiator->instantiate('DoctrineTest\\InstantiatorTestAsset\\SerializableArrayObjectAsset'); | ||
$this->instantiator->instantiate('DoctrineTest\\InstantiatorTestAsset\\UnCloneableAsset'); | ||
$this->instantiator->instantiate(self::class); |
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.
Do we have a rule for this changes?
-__CLASS__
+self::class
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.
No.
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.
So, why? :)
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.
Because other places already use ::class
, and just because it's better. :D
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.
Tbh, I'd fancy such a rule - it would remove the ambiguity once and for all 👍
b288091
to
7831db6
Compare
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.
LGTM 👍
Class naming rules ignored as no changes to the class names in stable are possible.