From abdddcb7cc65b437c34e3be98e13b3ac6e1aa2af Mon Sep 17 00:00:00 2001 From: Martin Herndl Date: Wed, 6 Jul 2022 10:17:32 +0200 Subject: [PATCH 1/2] Fix handling of `str_split` / `mb_str_split` string arg compound types --- .../StrSplitFunctionReturnTypeExtension.php | 40 ++++++++++++------- .../Analyser/LegacyNodeScopeResolverTest.php | 6 +-- .../Analyser/NodeScopeResolverTest.php | 1 + tests/PHPStan/Analyser/data/bug-7580.php | 18 +++++++++ ...rnaryOperatorConstantConditionRuleTest.php | 19 +++++++++ .../Rules/Comparison/data/bug-7580.php | 20 ++++++++++ 6 files changed, 87 insertions(+), 17 deletions(-) create mode 100644 tests/PHPStan/Analyser/data/bug-7580.php create mode 100644 tests/PHPStan/Rules/Comparison/data/bug-7580.php diff --git a/src/Type/Php/StrSplitFunctionReturnTypeExtension.php b/src/Type/Php/StrSplitFunctionReturnTypeExtension.php index 6f0cc221d5..31a6fdfe2b 100644 --- a/src/Type/Php/StrSplitFunctionReturnTypeExtension.php +++ b/src/Type/Php/StrSplitFunctionReturnTypeExtension.php @@ -16,10 +16,13 @@ use PHPStan\Type\Constant\ConstantStringType; use PHPStan\Type\DynamicFunctionReturnTypeExtension; use PHPStan\Type\IntegerType; +use PHPStan\Type\IntersectionType; use PHPStan\Type\StringType; use PHPStan\Type\Type; use PHPStan\Type\TypeCombinator; +use PHPStan\Type\TypeTraverser; use PHPStan\Type\TypeUtils; +use PHPStan\Type\UnionType; use function array_map; use function array_unique; use function count; @@ -62,6 +65,7 @@ public function getTypeFromFunctionCall(FunctionReflection $functionReflection, $splitLength = 1; } + $encoding = null; if ($functionReflection->getName() === 'mb_str_split') { if (count($functionCall->getArgs()) >= 3) { $strings = TypeUtils::getConstantStrings($scope->getType($functionCall->getArgs()[2]->value)); @@ -85,22 +89,30 @@ public function getTypeFromFunctionCall(FunctionReflection $functionReflection, } $stringType = $scope->getType($functionCall->getArgs()[0]->value); - if (!$stringType instanceof ConstantStringType) { - return TypeCombinator::intersect( - new ArrayType(new IntegerType(), new StringType()), - new NonEmptyArrayType(), - ); - } - $stringValue = $stringType->getValue(); - $items = isset($encoding) - ? mb_str_split($stringValue, $splitLength, $encoding) - : str_split($stringValue, $splitLength); - if ($items === false) { - throw new ShouldNotHappenException(); - } + return TypeTraverser::map($stringType, static function (Type $type, callable $traverse) use ($encoding, $splitLength, $scope): Type { + if ($type instanceof UnionType || $type instanceof IntersectionType) { + return $traverse($type); + } + + if (!$type instanceof ConstantStringType) { + return TypeCombinator::intersect( + new ArrayType(new IntegerType(), new StringType()), + new NonEmptyArrayType(), + ); + } + + $stringValue = $type->getValue(); + + $items = $encoding !== null + ? mb_str_split($stringValue, $splitLength, $encoding) + : str_split($stringValue, $splitLength); + if ($items === false) { + throw new ShouldNotHappenException(); + } - return self::createConstantArrayFrom($items, $scope); + return self::createConstantArrayFrom($items, $scope); + }); } /** diff --git a/tests/PHPStan/Analyser/LegacyNodeScopeResolverTest.php b/tests/PHPStan/Analyser/LegacyNodeScopeResolverTest.php index 328d46c9fc..55bb51b381 100644 --- a/tests/PHPStan/Analyser/LegacyNodeScopeResolverTest.php +++ b/tests/PHPStan/Analyser/LegacyNodeScopeResolverTest.php @@ -5356,7 +5356,7 @@ public function dataFunctions(): array '$strSplitConstantStringWithInvalidSplitLengthType', ], [ - 'non-empty-array', + 'array{\'a\'|\'g\', \'b\'|\'h\', \'c\'|\'i\', \'d\'|\'j\', \'e\'|\'k\', \'f\'|\'l\'}', '$strSplitConstantStringWithVariableStringAndConstantSplitLength', ], [ @@ -8744,7 +8744,7 @@ public function dataPhp74Functions(): array '$mbStrSplitConstantStringWithInvalidSplitLengthType', ], [ - 'non-empty-array', + 'array{\'a\'|\'g\', \'b\'|\'h\', \'c\'|\'i\', \'d\'|\'j\', \'e\'|\'k\', \'f\'|\'l\'}', '$mbStrSplitConstantStringWithVariableStringAndConstantSplitLength', ], [ @@ -8800,7 +8800,7 @@ public function dataPhp74Functions(): array '$mbStrSplitConstantStringWithInvalidSplitLengthTypeAndVariableEncoding', ], [ - 'non-empty-array', + 'array{\'a\'|\'g\', \'b\'|\'h\', \'c\'|\'i\', \'d\'|\'j\', \'e\'|\'k\', \'f\'|\'l\'}', '$mbStrSplitConstantStringWithVariableStringAndConstantSplitLengthAndValidEncoding', ], [ diff --git a/tests/PHPStan/Analyser/NodeScopeResolverTest.php b/tests/PHPStan/Analyser/NodeScopeResolverTest.php index e3a08cf635..9ce49a6429 100644 --- a/tests/PHPStan/Analyser/NodeScopeResolverTest.php +++ b/tests/PHPStan/Analyser/NodeScopeResolverTest.php @@ -923,6 +923,7 @@ public function dataFileAsserts(): iterable yield from $this->gatherAssertTypes(__DIR__ . '/data/emptyiterator.php'); yield from $this->gatherAssertTypes(__DIR__ . '/data/collected-data.php'); yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-7550.php'); + yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-7580.php'); } /** diff --git a/tests/PHPStan/Analyser/data/bug-7580.php b/tests/PHPStan/Analyser/data/bug-7580.php new file mode 100644 index 0000000000..a832f24f6e --- /dev/null +++ b/tests/PHPStan/Analyser/data/bug-7580.php @@ -0,0 +1,18 @@ +', mb_str_split($v, 1)); diff --git a/tests/PHPStan/Rules/Comparison/TernaryOperatorConstantConditionRuleTest.php b/tests/PHPStan/Rules/Comparison/TernaryOperatorConstantConditionRuleTest.php index 1403768f98..a53e0ebd59 100644 --- a/tests/PHPStan/Rules/Comparison/TernaryOperatorConstantConditionRuleTest.php +++ b/tests/PHPStan/Rules/Comparison/TernaryOperatorConstantConditionRuleTest.php @@ -92,4 +92,23 @@ public function testReportPhpDoc(): void ]); } + public function testBug7580(): void + { + $this->treatPhpDocTypesAsCertain = false; + $this->analyse([__DIR__ . '/data/bug-7580.php'], [ + [ + 'Ternary operator condition is always false.', + 6, + ], + [ + 'Ternary operator condition is always true.', + 9, + ], + [ + 'Ternary operator condition is always true.', + 20, + ], + ]); + } + } diff --git a/tests/PHPStan/Rules/Comparison/data/bug-7580.php b/tests/PHPStan/Rules/Comparison/data/bug-7580.php new file mode 100644 index 0000000000..294ed21160 --- /dev/null +++ b/tests/PHPStan/Rules/Comparison/data/bug-7580.php @@ -0,0 +1,20 @@ + Date: Wed, 6 Jul 2022 10:44:15 +0200 Subject: [PATCH 2/2] `mb_str_split` can return an empty array --- build/baseline-8.0.neon | 2 +- resources/functionMap_php74delta.php | 2 +- resources/functionMap_php80delta.php | 2 +- .../StrSplitFunctionReturnTypeExtension.php | 15 +++++++------ .../Analyser/LegacyNodeScopeResolverTest.php | 22 +++++++++---------- tests/PHPStan/Analyser/data/bug-7580.php | 2 +- ...rnaryOperatorConstantConditionRuleTest.php | 4 ---- 7 files changed, 23 insertions(+), 26 deletions(-) diff --git a/build/baseline-8.0.neon b/build/baseline-8.0.neon index 33bae32309..79d183a1e4 100644 --- a/build/baseline-8.0.neon +++ b/build/baseline-8.0.neon @@ -16,7 +16,7 @@ parameters: path: ../src/Type/Php/MbStrlenFunctionReturnTypeExtension.php - - message: "#^Strict comparison using \\=\\=\\= between non-empty-array and false will always evaluate to false\\.$#" + message: "#^Strict comparison using \\=\\=\\= between array and false will always evaluate to false\\.$#" count: 1 path: ../src/Type/Php/StrSplitFunctionReturnTypeExtension.php diff --git a/resources/functionMap_php74delta.php b/resources/functionMap_php74delta.php index ff3f99ba0d..f86a9c1db2 100644 --- a/resources/functionMap_php74delta.php +++ b/resources/functionMap_php74delta.php @@ -39,7 +39,7 @@ 'FFI::typeof' => ['FFI\CType', '&ptr'=>'FFI\CData'], 'FFI::type' => ['FFI\CType', 'type'=>'string'], 'get_mangled_object_vars' => ['array', 'obj'=>'object'], - 'mb_str_split' => ['non-empty-array|false', 'str'=>'string', 'split_length='=>'int', 'encoding='=>'string'], + 'mb_str_split' => ['array|false', 'str'=>'string', 'split_length='=>'int', 'encoding='=>'string'], 'password_algos' => ['array'], 'password_hash' => ['string|false', 'password'=>'string', 'algo'=>'string|null', 'options='=>'array'], 'password_needs_rehash' => ['bool', 'hash'=>'string', 'algo'=>'string|null', 'options='=>'array'], diff --git a/resources/functionMap_php80delta.php b/resources/functionMap_php80delta.php index fb557075d8..9a7059ec6f 100644 --- a/resources/functionMap_php80delta.php +++ b/resources/functionMap_php80delta.php @@ -74,7 +74,7 @@ 'imagescale' => ['false|object', 'im'=>'resource', 'new_width'=>'int', 'new_height='=>'int', 'method='=>'int'], 'ldap_set_rebind_proc' => ['bool', 'ldap'=>'resource', 'callback'=>'?callable'], 'mb_decode_numericentity' => ['string|false', 'string'=>'string', 'convmap'=>'array', 'encoding='=>'string'], - 'mb_str_split' => ['non-empty-array', 'str'=>'string', 'split_length='=>'positive-int', 'encoding='=>'string'], + 'mb_str_split' => ['array', 'str'=>'string', 'split_length='=>'positive-int', 'encoding='=>'string'], 'mb_strlen' => ['0|positive-int', 'str'=>'string', 'encoding='=>'string'], 'mktime' => ['int|false', 'hour'=>'int', 'minute='=>'int', 'second='=>'int', 'month='=>'int', 'day='=>'int', 'year='=>'int'], 'odbc_exec' => ['resource|false', 'connection_id'=>'resource', 'query'=>'string'], diff --git a/src/Type/Php/StrSplitFunctionReturnTypeExtension.php b/src/Type/Php/StrSplitFunctionReturnTypeExtension.php index 31a6fdfe2b..b756c22a4b 100644 --- a/src/Type/Php/StrSplitFunctionReturnTypeExtension.php +++ b/src/Type/Php/StrSplitFunctionReturnTypeExtension.php @@ -96,17 +96,18 @@ public function getTypeFromFunctionCall(FunctionReflection $functionReflection, } if (!$type instanceof ConstantStringType) { - return TypeCombinator::intersect( - new ArrayType(new IntegerType(), new StringType()), - new NonEmptyArrayType(), - ); + $returnType = new ArrayType(new IntegerType(), new StringType()); + + return $encoding === null + ? TypeCombinator::intersect($returnType, new NonEmptyArrayType()) + : $returnType; } $stringValue = $type->getValue(); - $items = $encoding !== null - ? mb_str_split($stringValue, $splitLength, $encoding) - : str_split($stringValue, $splitLength); + $items = $encoding === null + ? str_split($stringValue, $splitLength) + : mb_str_split($stringValue, $splitLength, $encoding); if ($items === false) { throw new ShouldNotHappenException(); } diff --git a/tests/PHPStan/Analyser/LegacyNodeScopeResolverTest.php b/tests/PHPStan/Analyser/LegacyNodeScopeResolverTest.php index 55bb51b381..8ce2b2fa38 100644 --- a/tests/PHPStan/Analyser/LegacyNodeScopeResolverTest.php +++ b/tests/PHPStan/Analyser/LegacyNodeScopeResolverTest.php @@ -8716,7 +8716,7 @@ public function dataPhp74Functions(): array { return [ [ - PHP_VERSION_ID < 80000 ? 'non-empty-array|false' : 'non-empty-array', + PHP_VERSION_ID < 80000 ? 'array|false' : 'array', '$mbStrSplitConstantStringWithoutDefinedParameters', ], [ @@ -8724,7 +8724,7 @@ public function dataPhp74Functions(): array '$mbStrSplitConstantStringWithoutDefinedSplitLength', ], [ - 'non-empty-array', + 'array', '$mbStrSplitStringWithoutDefinedSplitLength', ], [ @@ -8740,7 +8740,7 @@ public function dataPhp74Functions(): array '$mbStrSplitConstantStringWithFailureSplitLength', ], [ - PHP_VERSION_ID < 80000 ? 'non-empty-array|false' : 'non-empty-array', + PHP_VERSION_ID < 80000 ? 'array|false' : 'array', '$mbStrSplitConstantStringWithInvalidSplitLengthType', ], [ @@ -8748,7 +8748,7 @@ public function dataPhp74Functions(): array '$mbStrSplitConstantStringWithVariableStringAndConstantSplitLength', ], [ - PHP_VERSION_ID < 80000 ? 'non-empty-array|false' : 'non-empty-array', + PHP_VERSION_ID < 80000 ? 'array|false' : 'array', '$mbStrSplitConstantStringWithVariableStringAndVariableSplitLength', ], [ @@ -8760,7 +8760,7 @@ public function dataPhp74Functions(): array '$mbStrSplitConstantStringWithOneSplitLengthAndInvalidEncoding', ], [ - PHP_VERSION_ID < 80000 ? 'non-empty-array|false' : 'non-empty-array', + PHP_VERSION_ID < 80000 ? 'array|false' : 'array', '$mbStrSplitConstantStringWithOneSplitLengthAndVariableEncoding', ], [ @@ -8772,7 +8772,7 @@ public function dataPhp74Functions(): array '$mbStrSplitConstantStringWithGreaterSplitLengthThanStringLengthAndInvalidEncoding', ], [ - PHP_VERSION_ID < 80000 ? 'non-empty-array|false' : 'non-empty-array', + PHP_VERSION_ID < 80000 ? 'array|false' : 'array', '$mbStrSplitConstantStringWithGreaterSplitLengthThanStringLengthAndVariableEncoding', ], [ @@ -8788,7 +8788,7 @@ public function dataPhp74Functions(): array '$mbStrSplitConstantStringWithFailureSplitLengthAndVariableEncoding', ], [ - PHP_VERSION_ID < 80000 ? 'non-empty-array|false' : 'non-empty-array', + PHP_VERSION_ID < 80000 ? 'array|false' : 'array', '$mbStrSplitConstantStringWithInvalidSplitLengthTypeAndValidEncoding', ], [ @@ -8796,7 +8796,7 @@ public function dataPhp74Functions(): array '$mbStrSplitConstantStringWithInvalidSplitLengthTypeAndInvalidEncoding', ], [ - PHP_VERSION_ID < 80000 ? 'non-empty-array|false' : 'non-empty-array', + PHP_VERSION_ID < 80000 ? 'array|false' : 'array', '$mbStrSplitConstantStringWithInvalidSplitLengthTypeAndVariableEncoding', ], [ @@ -8808,11 +8808,11 @@ public function dataPhp74Functions(): array '$mbStrSplitConstantStringWithVariableStringAndConstantSplitLengthAndInvalidEncoding', ], [ - PHP_VERSION_ID < 80000 ? 'non-empty-array|false' : 'non-empty-array', + PHP_VERSION_ID < 80000 ? 'array|false' : 'array', '$mbStrSplitConstantStringWithVariableStringAndConstantSplitLengthAndVariableEncoding', ], [ - PHP_VERSION_ID < 80000 ? 'non-empty-array|false' : 'non-empty-array', + PHP_VERSION_ID < 80000 ? 'array|false' : 'array', '$mbStrSplitConstantStringWithVariableStringAndVariableSplitLengthAndValidEncoding', ], [ @@ -8820,7 +8820,7 @@ public function dataPhp74Functions(): array '$mbStrSplitConstantStringWithVariableStringAndVariableSplitLengthAndInvalidEncoding', ], [ - PHP_VERSION_ID < 80000 ? 'non-empty-array|false' : 'non-empty-array', + PHP_VERSION_ID < 80000 ? 'array|false' : 'array', '$mbStrSplitConstantStringWithVariableStringAndVariableSplitLengthAndVariableEncoding', ], ]; diff --git a/tests/PHPStan/Analyser/data/bug-7580.php b/tests/PHPStan/Analyser/data/bug-7580.php index a832f24f6e..e516bffe7d 100644 --- a/tests/PHPStan/Analyser/data/bug-7580.php +++ b/tests/PHPStan/Analyser/data/bug-7580.php @@ -15,4 +15,4 @@ function x(): string { throw new \Exception(); }; $v = x(); assertType('string', $v); -assertType('non-empty-array', mb_str_split($v, 1)); +assertType('array', mb_str_split($v, 1)); diff --git a/tests/PHPStan/Rules/Comparison/TernaryOperatorConstantConditionRuleTest.php b/tests/PHPStan/Rules/Comparison/TernaryOperatorConstantConditionRuleTest.php index a53e0ebd59..d54153136c 100644 --- a/tests/PHPStan/Rules/Comparison/TernaryOperatorConstantConditionRuleTest.php +++ b/tests/PHPStan/Rules/Comparison/TernaryOperatorConstantConditionRuleTest.php @@ -104,10 +104,6 @@ public function testBug7580(): void 'Ternary operator condition is always true.', 9, ], - [ - 'Ternary operator condition is always true.', - 20, - ], ]); }