Skip to content

Commit

Permalink
Fix handling unpacked argument with constant arrays
Browse files Browse the repository at this point in the history
  • Loading branch information
@ondrejmirtes
ondrejmirtes committed Jul 1, 2020
1 parent 6fd85e3 commit a21012d
Show file tree
Hide file tree
Showing 3 changed files with 145 additions and 20 deletions.
77 changes: 60 additions & 17 deletions src/Rules/FunctionCallParametersCheck.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,14 @@

namespace PHPStan\Rules;

use PhpParser\Node\Expr;
use PHPStan\Analyser\Scope;
use PHPStan\Reflection\ParametersAcceptor;
use PHPStan\Type\ErrorType;
use PHPStan\Type\NeverType;
use PHPStan\Type\Type;
use PHPStan\Type\TypeCombinator;
use PHPStan\Type\TypeUtils;
use PHPStan\Type\VerbosityLevel;
use PHPStan\Type\VoidType;

Expand Down Expand Up @@ -66,10 +69,57 @@ public function check(
$functionParametersMaxCount = -1;
}

$errors = [];
$invokedParametersCount = count($funcCall->args);
foreach ($funcCall->args as $arg) {
/** @var array<int, array{Expr, Type, bool}> $arguments */
$arguments = [];
/** @var array<int, \PhpParser\Node\Arg> $args */
$args = $funcCall->args;
foreach ($args as $i => $arg) {
$type = $scope->getType($arg->value);
if ($arg->unpack) {
$arrays = TypeUtils::getConstantArrays($type);
if (count($arrays) > 0) {
$minKeys = null;
foreach ($arrays as $array) {
$keysCount = count($array->getKeyTypes());
if ($minKeys !== null && $keysCount >= $minKeys) {
continue;
}

$minKeys = $keysCount;
}

for ($j = 0; $j < $minKeys; $j++) {
$types = [];
foreach ($arrays as $constantArray) {
$types[] = $constantArray->getValueTypes()[$j];
}
$arguments[] = [
$arg->value,
TypeCombinator::union(...$types),
false,
];
}
} else {
$arguments[] = [
$arg->value,
$type->getIterableValueType(),
true,
];
}
continue;
}

$arguments[] = [
$arg->value,
$type,
false,
];
}

$errors = [];
$invokedParametersCount = count($arguments);
foreach ($arguments as $i => [$argumentValue, $argumentValueType, $unpack]) {
if ($unpack) {
$invokedParametersCount = max($functionParametersMinCount, $functionParametersMaxCount);
break;
}
Expand Down Expand Up @@ -115,13 +165,11 @@ public function check(

$parameters = $parametersAcceptor->getParameters();

/** @var array<int, \PhpParser\Node\Arg> $args */
$args = $funcCall->args;
foreach ($args as $i => $argument) {
if ($this->checkArgumentTypes && $argument->unpack) {
foreach ($arguments as $i => [$argumentValue, $argumentValueType, $unpack]) {
if ($this->checkArgumentTypes && $unpack) {
$iterableTypeResult = $this->ruleLevelHelper->findTypeToCheck(
$scope,
$argument->value,
$argumentValue,
'',
static function (Type $type): bool {
return $type->isIterable()->yes();
Expand Down Expand Up @@ -154,11 +202,6 @@ static function (Type $type): bool {
}

$parameterType = $parameter->getType();

$argumentValueType = $scope->getType($argument->value);
if ($argument->unpack) {
$argumentValueType = $argumentValueType->getIterableValueType();
}
if (
$this->checkArgumentTypes
&& !$parameter->passedByReference()->createsNewVariable()
Expand All @@ -177,10 +220,10 @@ static function (Type $type): bool {
if (
!$this->checkArgumentsPassedByReference
|| !$parameter->passedByReference()->yes()
|| $argument->value instanceof \PhpParser\Node\Expr\Variable
|| $argument->value instanceof \PhpParser\Node\Expr\ArrayDimFetch
|| $argument->value instanceof \PhpParser\Node\Expr\PropertyFetch
|| $argument->value instanceof \PhpParser\Node\Expr\StaticPropertyFetch
|| $argumentValue instanceof \PhpParser\Node\Expr\Variable
|| $argumentValue instanceof \PhpParser\Node\Expr\ArrayDimFetch
|| $argumentValue instanceof \PhpParser\Node\Expr\PropertyFetch
|| $argumentValue instanceof \PhpParser\Node\Expr\StaticPropertyFetch
) {
continue;
}
Expand Down
43 changes: 40 additions & 3 deletions tests/PHPStan/Rules/Methods/CallMethodsRuleTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -436,15 +436,15 @@ public function testCallMethods(): void
1379,
],
[
'Only iterables can be unpacked, array<int>|null given in argument #3.',
'Only iterables can be unpacked, array<int>|null given in argument #5.',
1456,
],
[
'Only iterables can be unpacked, int given in argument #4.',
'Only iterables can be unpacked, int given in argument #6.',
1456,
],
[
'Only iterables can be unpacked, string given in argument #5.',
'Only iterables can be unpacked, string given in argument #7.',
1456,
],
[
Expand Down Expand Up @@ -864,10 +864,30 @@ public function testCallVariadicMethods(): void
'Parameter #4 ...$strings of method CallVariadicMethods\Foo::doVariadicString() expects string, int given.',
42,
],
[
'Parameter #5 ...$strings of method CallVariadicMethods\Foo::doVariadicString() expects string, int given.',
42,
],
[
'Parameter #6 ...$strings of method CallVariadicMethods\Foo::doVariadicString() expects string, int given.',
42,
],
[
'Method CallVariadicMethods\Foo::doIntegerParameters() invoked with 3 parameters, 2 required.',
43,
],
[
'Parameter #1 $foo of method CallVariadicMethods\Foo::doIntegerParameters() expects int, string given.',
43,
],
[
'Parameter #2 $bar of method CallVariadicMethods\Foo::doIntegerParameters() expects int, string given.',
43,
],
[
'Method CallVariadicMethods\Foo::doIntegerParameters() invoked with 3 parameters, 2 required.',
44,
],
[
'Parameter #1 ...$strings of method CallVariadicMethods\Bar::variadicStrings() expects string, int given.',
85,
Expand Down Expand Up @@ -1433,4 +1453,21 @@ public function testBug3445(): void
]);
}

public function testBug3481(): void
{
$this->checkThisOnly = false;
$this->checkNullables = true;
$this->checkUnionTypes = true;
$this->analyse([__DIR__ . '/data/bug-3481.php'], [
[
'Method Bug3481\Foo::doSomething() invoked with 2 parameters, 3 required.',
34,
],
[
'Parameter #1 $a of method Bug3481\Foo::doSomething() expects string, int|string given.',
44,
],
]);
}

}
45 changes: 45 additions & 0 deletions tests/PHPStan/Rules/Methods/data/bug-3481.php
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
<?php

namespace Bug3481;

class Foo
{
/**
* @param string $a
* @param int $b
* @param string $c
*/
public function doSomething($a, $b, $c): void
{
}
}

function (): void {
$args = [
'foo',
1,
'bar',
];
$foo = new Foo();
$foo->doSomething(...$args);
};

function (): void {
$args = ['foo', 1];
if (rand(0, 1)) {
$args[] = 'bar';
}

$foo = new Foo();
$foo->doSomething(...$args);
};

function (): void {
$args = ['foo', 1, 'string'];
if (rand(0, 1)) {
$args[0] = 1;
}

$foo = new Foo();
$foo->doSomething(...$args);
};

0 comments on commit a21012d

Please sign in to comment.