Simplify SprintfFunctionDynamicReturnTypeExtension #3188
Conversation
|
|
||
| if (count($formatType->getConstantStrings()) > 0) { |
There was a problem hiding this comment.
the loop is the same as before, just the if statement was unwrapped
|
the new composer/composer errors are valid because the typing before this PR was too precise. we can't know whether |
|
This pull request has been marked as ready for review. |
| } | ||
| if ( | ||
| $functionReflection->getName() === 'vsprintf' | ||
| && count($args) === 2 |
There was a problem hiding this comment.
Please add some tests in case when there is $arg->unpack === true involved.
| @@ -48,50 +48,64 @@ public function getTypeFromFunctionCall( | |||
| } | |||
|
|
|||
| $formatType = $scope->getType($args[0]->value); | |||
| if (count($args) === 1) { | |||
| return null; | |||
There was a problem hiding this comment.
We know what would happen so we could be more precise: https://3v4l.org/DeSsG
| assertType('string', sprintf($nonFalsey)); | ||
| assertType("'foo'", sprintf('foo')); | ||
| assertType("string", sprintf(...$arr)); | ||
| assertType("non-falsy-string", sprintf('%s', ...$arr)); // should be 'string' |
There was a problem hiding this comment.
the wrong non-falsy-string types will be fixed after we integrated the remaining parts of #3168
|
Thank you. |
|
Hey, if I have |
|
@staabm how do you find the time to work so much on PHPStan. I'm doing something wrong 😅 recently too busy and I do lack motivation to work on it at night.. |
extracted from /pull/3168
for
sprintfwe already report an error and forvsprintfwe will emit a error after vprintf/vsprintf require a non-empty-array argument #3126