-
Notifications
You must be signed in to change notification settings - Fork 542
Better typing for mb_convert_encoding #3749
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,11 +5,10 @@ | |
use PhpParser\Node\Expr\FuncCall; | ||
use PHPStan\Analyser\Scope; | ||
use PHPStan\Reflection\FunctionReflection; | ||
use PHPStan\Type\ArrayType; | ||
use PHPStan\Type\Constant\ConstantBooleanType; | ||
use PHPStan\Type\DynamicFunctionReturnTypeExtension; | ||
use PHPStan\Type\IntegerType; | ||
use PHPStan\Type\StringType; | ||
use PHPStan\Type\Type; | ||
use PHPStan\Type\UnionType; | ||
|
||
final class MbConvertEncodingFunctionReturnTypeExtension implements DynamicFunctionReturnTypeExtension | ||
{ | ||
|
@@ -25,18 +24,14 @@ public function getTypeFromFunctionCall( | |
Scope $scope, | ||
): ?Type | ||
{ | ||
if (!isset($functionCall->getArgs()[0])) { | ||
$args = $functionCall->getArgs(); | ||
if (!isset($args[0])) { | ||
return null; | ||
} | ||
|
||
$argType = $scope->getType($functionCall->getArgs()[0]->value); | ||
$isString = $argType->isString(); | ||
$isArray = $argType->isArray(); | ||
$compare = $isString->compareTo($isArray); | ||
if ($compare === $isString) { | ||
return new StringType(); | ||
} elseif ($compare === $isArray) { | ||
return new ArrayType(new IntegerType(), new StringType()); | ||
$argType = $scope->getType($args[0]->value); | ||
if ($argType->isString()->yes() || $argType->isArray()->yes()) { | ||
return new UnionType([$argType, new ConstantBooleanType(false)]); | ||
jack-worman marked this conversation as resolved.
Show resolved
Hide resolved
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
should be used. And it's not ok for constants array or constant strings. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think #3914 does the job |
||
} | ||
|
||
return null; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,10 +1,10 @@ | ||
<?php // lint < 8.0 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Existing bug are generally not removed but updated. (to see the diff) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. reverted delete |
||
<?php | ||
|
||
namespace Bug3336; | ||
|
||
function (array $arr, string $str, $mixed): void { | ||
\PHPStan\Testing\assertType('array<int, string>', mb_convert_encoding($arr)); | ||
\PHPStan\Testing\assertType('string', mb_convert_encoding($str)); | ||
\PHPStan\Testing\assertType('array<int, string>|string|false', mb_convert_encoding($mixed)); | ||
\PHPStan\Testing\assertType('array<int, string>|string|false', mb_convert_encoding()); | ||
\PHPStan\Testing\assertType('array|false', mb_convert_encoding($arr)); | ||
\PHPStan\Testing\assertType('string|false', mb_convert_encoding($str)); | ||
\PHPStan\Testing\assertType('array<array<bool|float|int|string|null>|bool|float|int|string|null>|string|false', mb_convert_encoding($mixed)); | ||
\PHPStan\Testing\assertType('array<array<bool|float|int|string|null>|bool|float|int|string|null>|string|false', mb_convert_encoding()); | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
<?php | ||
|
||
namespace PHPStan\Analyser\nsrt\mb_convert_encoding; | ||
|
||
/** | ||
* @param array{foo: string, bar: int} $structuredArray | ||
* @param list<string> $stringList | ||
* @param list<int> $intList | ||
*/ | ||
function test_mb_convert_encoding( | ||
mixed $mixed, | ||
string $string, | ||
array $mixedArray, | ||
array $structuredArray, | ||
array $stringList, | ||
array $intList, | ||
): void { | ||
\PHPStan\Testing\assertType('array<array<bool|float|int|string|null>|bool|float|int|string|null>|string|false', mb_convert_encoding($mixed, 'UTF-8')); | ||
\PHPStan\Testing\assertType('string|false', mb_convert_encoding($string, 'UTF-8')); | ||
\PHPStan\Testing\assertType('array|false', mb_convert_encoding($mixedArray, 'UTF-8')); | ||
\PHPStan\Testing\assertType('array{foo: string, bar: int}|false', mb_convert_encoding($structuredArray, 'UTF-8')); | ||
\PHPStan\Testing\assertType('list<string>|false', mb_convert_encoding($stringList, 'UTF-8')); | ||
\PHPStan\Testing\assertType('list<int>|false', mb_convert_encoding($intList, 'UTF-8')); | ||
}; |
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.
Is there a way to represent a recursive type?
Or is only going 3 layers deep good enough?
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.
I think the best would be to use
string|array<mixed>|false