Add DateTimeModifyReturnTypeExtension #1267
Conversation
d22c593 to
6b57ce2
Compare
There was a problem hiding this comment.
This code can cause PHPStan to report warnings: https://3v4l.org/9eHGQ The @ operator should be used
There was a problem hiding this comment.
This will crash if someone calls ->modify() without args.
There was a problem hiding this comment.
I don't like the usage of inheritance here. Instead, the class name should be asked for in the constructor and the class should be registered two times with different arguments.
There was a problem hiding this comment.
What's the scenario for this if? I'd rather have you to check count($constantStrings) === 0 first and return $defaultReturnType then.
There was a problem hiding this comment.
It's similar too DateTimeConstructorThrowTypeExtension.
I would say it's to handle a phpdoc like 'foo'|'bar'|somethingElseWhichIsNotAConstantString or a situation
if ($foo) {
$arg = 'foo';
} else {
$arg = ... ;
}
$dt->modify($arg);
72a8920 to
cbd992b
Compare
|
I don't think the failure are related to my PR @ondrejmirtes |
cbd992b to
463b203
Compare
ondrejmirtes
left a comment
There was a problem hiding this comment.
Coding standard failed, otherwise it looks good to merge.
|
Also please note I changed the configuration: c2e3f22 (no backslash needed) |
Fixed. Also, should it target the 1.7.x branch or 1.6.x one ? |
|
1.6.x would be better, thanks. |
c6fa23a to
5b980c1
Compare
Done :) |
|
Thank you! |
Closes phpstan/phpstan#4927
I think it also closes phpstan/phpstan#1934. There is a lot more method listed:
But I never succeed to get
falsewith those method, except when I passed a parameter with a wrong type.This is reported by phpstan as an error, so I don't think a returnType extension might be needed to say it's return false in php 7.4.