Skip to content

Improve implode signature #4159

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

Open
wants to merge 7 commits into
base: 2.1.x
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions resources/functionMap.php
Original file line number Diff line number Diff line change
Expand Up @@ -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'],
Expand Down
1 change: 1 addition & 0 deletions resources/functionMap_php74delta.php
Original file line number Diff line number Diff line change
Expand Up @@ -60,5 +60,6 @@
],
'old' => [
'implode\'2' => ['string', 'pieces'=>'array', 'glue'=>'string'],
'join\'2' => ['string', 'pieces'=>'array', 'glue'=>'string'],
],
];
32 changes: 32 additions & 0 deletions src/Parser/ImplodeArgVisitor.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
<?php declare(strict_types = 1);

namespace PHPStan\Parser;

use Override;
use PhpParser\Node;
use PhpParser\NodeVisitorAbstract;
use PHPStan\DependencyInjection\AutowiredService;
use function in_array;

#[AutowiredService]
final class ImplodeArgVisitor extends NodeVisitorAbstract
{

public const ATTRIBUTE_NAME = 'isImplodeArg';

#[Override]
public function enterNode(Node $node): ?Node
{
if ($node instanceof Node\Expr\FuncCall && $node->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;
}

}
27 changes: 27 additions & 0 deletions src/Reflection/ParametersAcceptorSelector.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -203,6 +204,32 @@ public static function selectFromArgs(
];
}

if (count($args) <= 2 && (bool) $args[0]->getAttribute(ImplodeArgVisitor::ATTRIBUTE_NAME)) {
$acceptor = $namedArgumentsVariants[0] ?? $parametersAcceptors[0];
$parameters = $acceptor->getParameters();
if (isset($args[1]) || ($args[0]->name !== null && $args[0]->name->name === 'array')) {
$parameters = [
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($parameters[0]->getName(), false, new ArrayType(new MixedType(), new MixedType()), PassedByReference::createNo(), false, null),
];
}

$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),
Expand Down
2 changes: 1 addition & 1 deletion tests/PHPStan/Levels/data/acceptTypes-5.json
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion tests/PHPStan/Levels/data/acceptTypes-7.json
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
75 changes: 73 additions & 2 deletions tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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,
],
];
Expand All @@ -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,
],
];
Expand All @@ -356,6 +364,21 @@ 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'], [
[
'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';
Expand Down Expand Up @@ -2192,6 +2215,54 @@ public function testBug13065(): void
$this->analyse([__DIR__ . '/data/bug-13065.php'], $errors);
}

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'], [
[
sprintf('Parameter #2 %s of function join expects array, list<int>|null given.', $param2Name),
10,
],
[
sprintf('Parameter #1 %s of function join expects array, list<int>|null given.', $param1Name),
11,
],
[
sprintf('Parameter #2 %s of function implode expects array, list<int>|null given.', $param2Name),
13,
],
[
sprintf('Parameter #1 %s of function implode expects array, list<int>|null given.', $param1Name),
14,
],
[
sprintf('Parameter #2 %s of function join expects array, array<string>|string given.', $param2Name),
22,
],
[
sprintf('Parameter #1 %s of function join expects array, array<string>|string given.', $param1Name),
23,
],
[
sprintf('Parameter #2 %s of function implode expects array, array<string>|string given.', $param2Name),
25,
],
[
sprintf('Parameter #1 %s of function implode expects array, array<string>|string given.', $param1Name),
26,
],
]);
}

#[RequiresPhp('>= 8.0')]
public function testBug12317(): void
{
Expand Down
36 changes: 36 additions & 0 deletions tests/PHPStan/Rules/Functions/data/bug-5760.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
<?php declare(strict_types=1);

namespace Bug5760;

/**
* @param list<int>|null $arrayOrNull
*/
function doImplode(?array $arrayOrNull): void
{
join(',', $arrayOrNull);
join($arrayOrNull);

implode(',', $arrayOrNull);
implode($arrayOrNull);
}

/**
* @param array<string>|string $union
*/
function more(array|string $union): void
{
join(',', $union);
join($union);

implode(',', $union);
implode($union);
}

function success(): void
{
join(',', ['']);
join(['']);

implode(',', ['']);
implode(['']);
}
10 changes: 10 additions & 0 deletions tests/PHPStan/Rules/Functions/data/implode-named-parameters.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<?php declare(strict_types = 1);

namespace ImplodeNamedParameters;

function (): void {
implode(array: ['']); // error
join(array: ['']); // error
implode(separator: '', array: ['']);
join(separator: '', array: ['']);
};
Loading