From 988666d3005803c426762659ee410b52c041bb21 Mon Sep 17 00:00:00 2001 From: Vincent Langlet Date: Sat, 26 Jul 2025 09:54:53 +0200 Subject: [PATCH 1/7] Improve implode signature --- src/Parser/ImplodeArgVisitor.php | 32 ++++++++++++ src/Reflection/ParametersAcceptorSelector.php | 26 ++++++++++ .../CallToFunctionParametersRuleTest.php | 52 ++++++++++++++++++- .../PHPStan/Rules/Functions/data/bug-5760.php | 36 +++++++++++++ 4 files changed, 144 insertions(+), 2 deletions(-) create mode 100644 src/Parser/ImplodeArgVisitor.php create mode 100644 tests/PHPStan/Rules/Functions/data/bug-5760.php diff --git a/src/Parser/ImplodeArgVisitor.php b/src/Parser/ImplodeArgVisitor.php new file mode 100644 index 0000000000..36e0201b83 --- /dev/null +++ b/src/Parser/ImplodeArgVisitor.php @@ -0,0 +1,32 @@ +name instanceof Node\Name) { + $functionName = $node->name->toLowerString(); + if (in_array($functionName, ['implode', 'join'], true)) { + $args = $node->getRawArgs(); + if (isset($args[0])) { + $args[0]->setAttribute(self::ATTRIBUTE_NAME, true); + } + } + } + return null; + } + +} diff --git a/src/Reflection/ParametersAcceptorSelector.php b/src/Reflection/ParametersAcceptorSelector.php index 81bc3a2a99..43e5d73a39 100644 --- a/src/Reflection/ParametersAcceptorSelector.php +++ b/src/Reflection/ParametersAcceptorSelector.php @@ -15,6 +15,7 @@ use PHPStan\Parser\ClosureBindArgVisitor; use PHPStan\Parser\ClosureBindToVarVisitor; use PHPStan\Parser\CurlSetOptArgVisitor; +use PHPStan\Parser\ImplodeArgVisitor; use PHPStan\Reflection\Callables\CallableParametersAcceptor; use PHPStan\Reflection\Native\NativeParameterReflection; use PHPStan\Reflection\Php\DummyParameter; @@ -203,6 +204,31 @@ public static function selectFromArgs( ]; } + if (isset($args[0]) && (bool) $args[0]->getAttribute(ImplodeArgVisitor::ATTRIBUTE_NAME)) { + if (isset($args[1])) { + $parameters = [ + new NativeParameterReflection('separator', false, new StringType(), PassedByReference::createNo(), false, null), + new NativeParameterReflection('array', false, new ArrayType(new MixedType(), new MixedType()), PassedByReference::createNo(), false, null), + ]; + } else { + $parameters = [ + new NativeParameterReflection('array', false, new ArrayType(new MixedType(), new MixedType()), PassedByReference::createNo(), false, null), + ]; + } + + $acceptor = $parametersAcceptors[0]; + $parametersAcceptors = [ + new FunctionVariant( + $acceptor->getTemplateTypeMap(), + $acceptor->getResolvedTemplateTypeMap(), + $parameters, + $acceptor->isVariadic(), + $acceptor->getReturnType(), + $acceptor instanceof ExtendedParametersAcceptor ? $acceptor->getCallSiteVarianceMap() : TemplateTypeVarianceMap::createEmpty(), + ), + ]; + } + if (isset($args[0]) && (bool) $args[0]->getAttribute(ArrayWalkArgVisitor::ATTRIBUTE_NAME)) { $arrayWalkParameters = [ new DummyParameter('item', $scope->getIterableValueType($scope->getType($args[0]->value)), false, PassedByReference::createReadsArgument(), false, null), diff --git a/tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php b/tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php index 385fab83b5..20bd5ce564 100644 --- a/tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php +++ b/tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php @@ -322,7 +322,11 @@ public function testImplodeOnPhp74(): void if (PHP_VERSION_ID >= 80000) { $errors = [ [ - 'Parameter #2 $array of function implode expects array|null, string given.', + 'Parameter #1 $separator of function implode expects string, array given.', + 8, + ], + [ + 'Parameter #2 $array of function implode expects array, string given.', 8, ], ]; @@ -336,7 +340,11 @@ public function testImplodeOnLessThanPhp74(): void if (PHP_VERSION_ID >= 80000) { $errors = [ [ - 'Parameter #2 $array of function implode expects array|null, string given.', + 'Parameter #1 $separator of function implode expects string, array given.', + 8, + ], + [ + 'Parameter #2 $array of function implode expects array, string given.', 8, ], ]; @@ -2192,6 +2200,46 @@ public function testBug13065(): void $this->analyse([__DIR__ . '/data/bug-13065.php'], $errors); } + public function testBug5760(): void + { + $this->checkExplicitMixed = true; + $this->checkImplicitMixed = true; + $this->analyse([__DIR__ . '/data/bug-5760.php'], [ + [ + 'Parameter #2 $pieces of function join expects array, list|null given.', + 10, + ], + [ + 'Parameter #1 $glue of function join expects array, list|null given.', + 11, + ], + [ + 'Parameter #2 $array of function implode expects array, list|null given.', + 13, + ], + [ + 'Parameter #1 $separator of function implode expects array, list|null given.', + 14, + ], + [ + 'Parameter #2 $pieces of function join expects array, array|string given.', + 22, + ], + [ + 'Parameter #1 $glue of function join expects array, array|string given.', + 23, + ], + [ + 'Parameter #2 $array of function implode expects array, array|string given.', + 25, + ], + [ + 'Parameter #1 $separator of function implode expects array, array|string given.', + 26, + ], + ]); + } + #[RequiresPhp('>= 8.0')] public function testBug12317(): void { diff --git a/tests/PHPStan/Rules/Functions/data/bug-5760.php b/tests/PHPStan/Rules/Functions/data/bug-5760.php new file mode 100644 index 0000000000..f3ec36535f --- /dev/null +++ b/tests/PHPStan/Rules/Functions/data/bug-5760.php @@ -0,0 +1,36 @@ +|null $arrayOrNull + */ +function doImplode(?array $arrayOrNull): void +{ + join(',', $arrayOrNull); + join($arrayOrNull); + + implode(',', $arrayOrNull); + implode($arrayOrNull); +} + +/** + * @param array|string $union + */ +function more(array|string $union): void +{ + join(',', $union); + join($union); + + implode(',', $union); + implode($union); +} + +function success(): void +{ + join(',', ['']); + join(['']); + + implode(',', ['']); + implode(['']); +} From bcd4f9322d0b10adc0bb51e377f01bede623497c Mon Sep 17 00:00:00 2001 From: Vincent Langlet Date: Sat, 26 Jul 2025 10:29:26 +0200 Subject: [PATCH 2/7] Fix --- src/Reflection/ParametersAcceptorSelector.php | 2 +- .../Functions/CallToFunctionParametersRuleTest.php | 14 ++++++++++++++ .../Functions/data/implode-named-parameters.php | 10 ++++++++++ 3 files changed, 25 insertions(+), 1 deletion(-) create mode 100644 tests/PHPStan/Rules/Functions/data/implode-named-parameters.php diff --git a/src/Reflection/ParametersAcceptorSelector.php b/src/Reflection/ParametersAcceptorSelector.php index 43e5d73a39..90c4226052 100644 --- a/src/Reflection/ParametersAcceptorSelector.php +++ b/src/Reflection/ParametersAcceptorSelector.php @@ -205,7 +205,7 @@ public static function selectFromArgs( } if (isset($args[0]) && (bool) $args[0]->getAttribute(ImplodeArgVisitor::ATTRIBUTE_NAME)) { - if (isset($args[1])) { + if (isset($args[1]) || $args[0]->name?->name === 'array') { $parameters = [ new NativeParameterReflection('separator', false, new StringType(), PassedByReference::createNo(), false, null), new NativeParameterReflection('array', false, new ArrayType(new MixedType(), new MixedType()), PassedByReference::createNo(), false, null), diff --git a/tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php b/tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php index 20bd5ce564..627e1381b2 100644 --- a/tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php +++ b/tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php @@ -364,6 +364,20 @@ public function testImplodeOnLessThanPhp74(): void $this->analyse([__DIR__ . '/data/implode-74.php'], $errors); } + public function testImplodeNamedParameters(): void + { + $this->analyse([__DIR__ . '/data/implode-named-parameters.php'], [ + [ + 'Missing parameter $separator (string) in call to function implode.', + 6, + ], + [ + 'Missing parameter $separator (string) in call to function join.', + 7, + ], + ]); + } + public function testVariableIsNotNullAfterSeriesOfConditions(): void { require_once __DIR__ . '/data/variable-is-not-null-after-conditions.php'; diff --git a/tests/PHPStan/Rules/Functions/data/implode-named-parameters.php b/tests/PHPStan/Rules/Functions/data/implode-named-parameters.php new file mode 100644 index 0000000000..8e016b5157 --- /dev/null +++ b/tests/PHPStan/Rules/Functions/data/implode-named-parameters.php @@ -0,0 +1,10 @@ + Date: Sat, 26 Jul 2025 10:35:57 +0200 Subject: [PATCH 3/7] Fix --- src/Reflection/ParametersAcceptorSelector.php | 8 +++++--- .../Rules/Functions/CallToFunctionParametersRuleTest.php | 1 + 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/Reflection/ParametersAcceptorSelector.php b/src/Reflection/ParametersAcceptorSelector.php index 90c4226052..8025ca896f 100644 --- a/src/Reflection/ParametersAcceptorSelector.php +++ b/src/Reflection/ParametersAcceptorSelector.php @@ -204,15 +204,17 @@ public static function selectFromArgs( ]; } - if (isset($args[0]) && (bool) $args[0]->getAttribute(ImplodeArgVisitor::ATTRIBUTE_NAME)) { - if (isset($args[1]) || $args[0]->name?->name === 'array') { + if (count($args) <= 2 && (bool) $args[0]->getAttribute(ImplodeArgVisitor::ATTRIBUTE_NAME)) { + $acceptor = $parametersAcceptors[0]; + $parameters = $acceptor->getParameters(); + if (isset($args[1]) || ($args[0]->name !== null && $args[0]->name->name === 'array')) { $parameters = [ new NativeParameterReflection('separator', false, new StringType(), PassedByReference::createNo(), false, null), new NativeParameterReflection('array', false, new ArrayType(new MixedType(), new MixedType()), PassedByReference::createNo(), false, null), ]; } else { $parameters = [ - new NativeParameterReflection('array', false, new ArrayType(new MixedType(), new MixedType()), PassedByReference::createNo(), false, null), + new NativeParameterReflection('separator', false, new ArrayType(new MixedType(), new MixedType()), PassedByReference::createNo(), false, null), ]; } diff --git a/tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php b/tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php index 627e1381b2..8f5ae82c1d 100644 --- a/tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php +++ b/tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php @@ -364,6 +364,7 @@ public function testImplodeOnLessThanPhp74(): void $this->analyse([__DIR__ . '/data/implode-74.php'], $errors); } + #[RequiresPhp('>= 8.0')] public function testImplodeNamedParameters(): void { $this->analyse([__DIR__ . '/data/implode-named-parameters.php'], [ From f8d1be2c9836693ffed3740b0ca1086e990379a3 Mon Sep 17 00:00:00 2001 From: Vincent Langlet Date: Sat, 26 Jul 2025 10:54:53 +0200 Subject: [PATCH 4/7] Try --- src/Reflection/ParametersAcceptorSelector.php | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/Reflection/ParametersAcceptorSelector.php b/src/Reflection/ParametersAcceptorSelector.php index 8025ca896f..b8dfd173e3 100644 --- a/src/Reflection/ParametersAcceptorSelector.php +++ b/src/Reflection/ParametersAcceptorSelector.php @@ -209,16 +209,15 @@ public static function selectFromArgs( $parameters = $acceptor->getParameters(); if (isset($args[1]) || ($args[0]->name !== null && $args[0]->name->name === 'array')) { $parameters = [ - new NativeParameterReflection('separator', false, new StringType(), PassedByReference::createNo(), false, null), - new NativeParameterReflection('array', false, new ArrayType(new MixedType(), new MixedType()), PassedByReference::createNo(), false, null), + new NativeParameterReflection($parameters[0]->getName(), false, new StringType(), PassedByReference::createNo(), false, null), + new NativeParameterReflection($parameters[1]->getName(), false, new ArrayType(new MixedType(), new MixedType()), PassedByReference::createNo(), false, null), ]; } else { $parameters = [ - new NativeParameterReflection('separator', false, new ArrayType(new MixedType(), new MixedType()), PassedByReference::createNo(), false, null), + new NativeParameterReflection($parameters[0]->getName(), false, new ArrayType(new MixedType(), new MixedType()), PassedByReference::createNo(), false, null), ]; } - $acceptor = $parametersAcceptors[0]; $parametersAcceptors = [ new FunctionVariant( $acceptor->getTemplateTypeMap(), From 4e8384f60001d22b6d3a0623048b4e1010ed3afe Mon Sep 17 00:00:00 2001 From: Vincent Langlet Date: Sat, 26 Jul 2025 11:01:43 +0200 Subject: [PATCH 5/7] Fix --- tests/PHPStan/Levels/data/acceptTypes-5.json | 2 +- tests/PHPStan/Levels/data/acceptTypes-7.json | 2 +- .../CallToFunctionParametersRuleTest.php | 24 ++++++++++++------- 3 files changed, 18 insertions(+), 10 deletions(-) diff --git a/tests/PHPStan/Levels/data/acceptTypes-5.json b/tests/PHPStan/Levels/data/acceptTypes-5.json index 76aac5c080..4bb076e055 100644 --- a/tests/PHPStan/Levels/data/acceptTypes-5.json +++ b/tests/PHPStan/Levels/data/acceptTypes-5.json @@ -185,7 +185,7 @@ "ignorable": true }, { - "message": "Parameter #2 $array of function implode expects array|null, int given.", + "message": "Parameter #2 $array of function implode expects array, int given.", "line": 763, "ignorable": true } diff --git a/tests/PHPStan/Levels/data/acceptTypes-7.json b/tests/PHPStan/Levels/data/acceptTypes-7.json index ba67e6a9d6..216fad8987 100644 --- a/tests/PHPStan/Levels/data/acceptTypes-7.json +++ b/tests/PHPStan/Levels/data/acceptTypes-7.json @@ -165,7 +165,7 @@ "ignorable": true }, { - "message": "Parameter #2 $array of function implode expects array|null, array|int|string given.", + "message": "Parameter #2 $array of function implode expects array, array|int|string given.", "line": 756, "ignorable": true } diff --git a/tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php b/tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php index 8f5ae82c1d..69207e94f3 100644 --- a/tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php +++ b/tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php @@ -2217,39 +2217,47 @@ public function testBug13065(): void public function testBug5760(): void { + if (PHP_VERSION_ID < 80000) { + $param1Name = '$glue'; + $param2Name = '$pieces'; + } else { + $param1Name = '$separator'; + $param2Name = '$array'; + } + $this->checkExplicitMixed = true; $this->checkImplicitMixed = true; $this->analyse([__DIR__ . '/data/bug-5760.php'], [ [ - 'Parameter #2 $pieces of function join expects array, list|null given.', + sprintf('Parameter #2 %s of function join expects array, list|null given.', $param2Name), 10, ], [ - 'Parameter #1 $glue of function join expects array, list|null given.', + sprintf('Parameter #1 %s of function join expects array, list|null given.', $param1Name), 11, ], [ - 'Parameter #2 $array of function implode expects array, list|null given.', + sprintf('Parameter #2 %s of function implode expects array, list|null given.', $param2Name), 13, ], [ - 'Parameter #1 $separator of function implode expects array, list|null given.', + sprintf('Parameter #1 %s of function implode expects array, list|null given.', $param1Name), 14, ], [ - 'Parameter #2 $pieces of function join expects array, array|string given.', + sprintf('Parameter #2 %s of function join expects array, array|string given.', $param2Name), 22, ], [ - 'Parameter #1 $glue of function join expects array, array|string given.', + sprintf('Parameter #1 %s of function join expects array, array|string given.', $param1Name), 23, ], [ - 'Parameter #2 $array of function implode expects array, array|string given.', + sprintf('Parameter #2 %s of function implode expects array, array|string given.', $param2Name), 25, ], [ - 'Parameter #1 $separator of function implode expects array, array|string given.', + sprintf('Parameter #1 %s of function implode expects array, array|string given.', $param1Name), 26, ], ]); From b9a6926a7ed1b42daa2ac59f04090331edcde88d Mon Sep 17 00:00:00 2001 From: Vincent Langlet Date: Sat, 26 Jul 2025 11:40:43 +0200 Subject: [PATCH 6/7] Fix --- src/Reflection/ParametersAcceptorSelector.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Reflection/ParametersAcceptorSelector.php b/src/Reflection/ParametersAcceptorSelector.php index b8dfd173e3..a2986b2fbc 100644 --- a/src/Reflection/ParametersAcceptorSelector.php +++ b/src/Reflection/ParametersAcceptorSelector.php @@ -205,7 +205,7 @@ public static function selectFromArgs( } if (count($args) <= 2 && (bool) $args[0]->getAttribute(ImplodeArgVisitor::ATTRIBUTE_NAME)) { - $acceptor = $parametersAcceptors[0]; + $acceptor = $namedArgumentsVariants[0] ?? $parametersAcceptors[0]; $parameters = $acceptor->getParameters(); if (isset($args[1]) || ($args[0]->name !== null && $args[0]->name->name === 'array')) { $parameters = [ From f4a36cbaab5982d82f04d7e41f4e8212228edf13 Mon Sep 17 00:00:00 2001 From: Vincent Langlet Date: Sat, 26 Jul 2025 12:04:42 +0200 Subject: [PATCH 7/7] Synchro functionMap with implode --- resources/functionMap.php | 1 + resources/functionMap_php74delta.php | 1 + 2 files changed, 2 insertions(+) diff --git a/resources/functionMap.php b/resources/functionMap.php index fade07b20c..16b8e6bf6d 100644 --- a/resources/functionMap.php +++ b/resources/functionMap.php @@ -5695,6 +5695,7 @@ 'jobqueue_license_info' => ['array'], 'join' => ['string', 'glue'=>'string', 'pieces'=>'array'], 'join\'1' => ['string', 'pieces'=>'array'], +'join\'2' => ['string', 'pieces'=>'array', 'glue'=>'string'], 'jpeg2wbmp' => ['bool', 'jpegname'=>'string', 'wbmpname'=>'string', 'dest_height'=>'int', 'dest_width'=>'int', 'threshold'=>'int'], 'json_decode' => ['mixed', 'json'=>'string', 'assoc='=>'bool|null', 'depth='=>'positive-int', 'options='=>'int'], 'json_encode' => ['non-empty-string|false', 'data'=>'mixed', 'options='=>'int', 'depth='=>'positive-int'], diff --git a/resources/functionMap_php74delta.php b/resources/functionMap_php74delta.php index 9b187510c8..86bc99ef80 100644 --- a/resources/functionMap_php74delta.php +++ b/resources/functionMap_php74delta.php @@ -60,5 +60,6 @@ ], 'old' => [ 'implode\'2' => ['string', 'pieces'=>'array', 'glue'=>'string'], + 'join\'2' => ['string', 'pieces'=>'array', 'glue'=>'string'], ], ];