fix returntype for DateIntervall::createFromDateString#2038
fix returntype for DateIntervall::createFromDateString#2038ondrejmirtes merged 14 commits intophpstan:1.9.xfrom
Conversation
staabm
left a comment
There was a problem hiding this comment.
the snippet from phpstan/phpstan#8442 should be added to this PR as a NodeScopeResolverTest including proper assertType calls
|
i will look into this! 👍 |
7b4b767 to
2095c78
Compare
45062c1 to
330a2f5
Compare
b00058d to
56c47c2
Compare
|
Would be great if you could adjust other similar existing extensions which also miss the |
ondrejmirtes
left a comment
There was a problem hiding this comment.
Also - you have some random spaces in there that I don't like but CS isn't picking up them as errors. Please get rid of them.
Like: if ( count or $strings[0 ].
There was a problem hiding this comment.
This code will crash if someone has DateInterval::createFromDateString() without an argument in their code.
There was a problem hiding this comment.
true, fixed that. but it leaves me wondering, what to return when this case is found?
$arguments = $methodCall->getArgs();
if (!isset($arguments[0])) {
return null;
}i opted for the default type but running code like this with warnings supressed does not execute at all. so what would be the correct thing to do here?
There was a problem hiding this comment.
i opted for this:
assertType('DateInterval|false',DateInterval::createFromDateString());
assertType('DateInterval|false',DateInterval::createFromDateString(new stdClass()));
intresting, |
Co-authored-by: Markus Staab <maggus.staab@googlemail.com>
31606d6 to
f1aa6dd
Compare
5d52f9d to
295a93d
Compare
|
Thank you! |
this should fix phpstan/phpstan#8442
seems to be a simple fix, not sure if i done everything correct.