Skip to content

Commit

Permalink
Fix array_push with an empty constant array
Browse files Browse the repository at this point in the history
  • Loading branch information
@herndlm @ondrejmirtes
herndlm authored and ondrejmirtes committed Apr 19, 2022
1 parent 06a5c26 commit 20539b3
Show file tree
Hide file tree
Showing 6 changed files with 230 additions and 60 deletions.
125 changes: 65 additions & 60 deletions src/Analyser/NodeScopeResolver.php
View file
Expand Up @@ -6,6 +6,7 @@
use Closure;
use PhpParser\Comment\Doc;
use PhpParser\Node;
use PhpParser\Node\Arg;
use PhpParser\Node\Expr;
use PhpParser\Node\Expr\Array_;
use PhpParser\Node\Expr\ArrayDimFetch;
Expand Down Expand Up @@ -109,16 +110,15 @@
use PHPStan\Reflection\ReflectionProvider;
use PHPStan\ShouldNotHappenException;
use PHPStan\TrinaryLogic;
use PHPStan\Type\Accessory\NonEmptyArrayType;
use PHPStan\Type\ArrayType;
use PHPStan\Type\ClosureType;
use PHPStan\Type\Constant\ConstantArrayType;
use PHPStan\Type\Constant\ConstantArrayTypeBuilder;
use PHPStan\Type\Constant\ConstantBooleanType;
use PHPStan\Type\Constant\ConstantIntegerType;
use PHPStan\Type\Constant\ConstantStringType;
use PHPStan\Type\ErrorType;
use PHPStan\Type\FileTypeMapper;
use PHPStan\Type\GeneralizePrecision;
use PHPStan\Type\Generic\GenericClassStringType;
use PHPStan\Type\Generic\TemplateTypeHelper;
use PHPStan\Type\Generic\TemplateTypeMap;
Expand Down Expand Up @@ -1843,75 +1843,80 @@ function (MutatingScope $scope) use ($expr, $nodeCallback, $context): Expression
&& in_array($functionReflection->getName(), ['array_push', 'array_unshift'], true)
&& count($expr->getArgs()) >= 2
) {
$argumentTypes = [];
foreach (array_slice($expr->getArgs(), 1) as $callArg) {
$callArgType = $scope->getType($callArg->value);
if ($callArg->unpack) {
$iterableValueType = $callArgType->getIterableValueType();
if ($iterableValueType instanceof UnionType) {
foreach ($iterableValueType->getTypes() as $innerType) {
$argumentTypes[] = $innerType;
$arrayArg = $expr->getArgs()[0]->value;
$arrayType = $scope->getType($arrayArg);
$callArgs = array_slice($expr->getArgs(), 1);

/**
* @param Arg[] $callArgs
* @param callable(?Type, Type, bool=): void $setOffsetValueType
*/
$setOffsetValueTypes = static function (Scope $scope, array $callArgs, callable $setOffsetValueType, ?bool &$nonConstantArrayWasUnpacked = null): void {
foreach ($callArgs as $callArg) {
$callArgType = $scope->getType($callArg->value);
if ($callArg->unpack) {
if ($callArgType->isIterableAtLeastOnce()->no()) {
continue;
}
} else {
$argumentTypes[] = $iterableValueType;
if (!$callArgType instanceof ConstantArrayType) {
$nonConstantArrayWasUnpacked = true;
}
$iterableValueType = $callArgType->getIterableValueType();
$isOptional = !$callArgType->isIterableAtLeastOnce()->yes();
if ($iterableValueType instanceof UnionType) {
foreach ($iterableValueType->getTypes() as $innerType) {
$setOffsetValueType(null, $innerType, $isOptional);
}
} else {
$setOffsetValueType(null, $iterableValueType, $isOptional);
}
continue;
}
continue;
$setOffsetValueType(null, $callArgType);
}
};

$argumentTypes[] = $callArgType;
}
if ($arrayType instanceof ConstantArrayType) {
$prepend = $functionReflection->getName() === 'array_unshift';
$arrayTypeBuilder = $prepend ? ConstantArrayTypeBuilder::createEmpty() : ConstantArrayTypeBuilder::createFromConstantArray($arrayType);

$arrayArg = $expr->getArgs()[0]->value;
$originalArrayType = $scope->getType($arrayArg);
$constantArrays = TypeUtils::getOldConstantArrays($originalArrayType);
if (
$functionReflection->getName() === 'array_push'
|| ($originalArrayType->isArray()->yes() && count($constantArrays) === 0)
) {
$arrayType = $originalArrayType;
foreach ($argumentTypes as $argType) {
$arrayType = $arrayType->setOffsetValueType(null, $argType);
}

$scope = $scope->invalidateExpression($arrayArg)->specifyExpressionType($arrayArg, TypeCombinator::intersect($arrayType, new NonEmptyArrayType()));
} elseif (count($constantArrays) > 0) {
$defaultArrayBuilder = ConstantArrayTypeBuilder::createEmpty();
foreach ($argumentTypes as $argType) {
$defaultArrayBuilder->setOffsetValueType(null, $argType);
}
$setOffsetValueTypes(
$scope,
$callArgs,
static function (?Type $offsetType, Type $valueType, bool $optional = false) use (&$arrayTypeBuilder): void {
$arrayTypeBuilder->setOffsetValueType($offsetType, $valueType, $optional);
},
$nonConstantArrayWasUnpacked,
);

$defaultArrayType = $defaultArrayBuilder->getArray();
if (!$defaultArrayType instanceof ConstantArrayType) {
$arrayType = $originalArrayType;
foreach ($argumentTypes as $argType) {
$arrayType = $arrayType->setOffsetValueType(null, $argType);
if ($prepend) {
$keyTypes = $arrayType->getKeyTypes();
$valueTypes = $arrayType->getValueTypes();
foreach ($keyTypes as $k => $keyType) {
$arrayTypeBuilder->setOffsetValueType(
$keyType instanceof ConstantStringType ? $keyType : null,
$valueTypes[$k],
$arrayType->isOptionalKey($k),
);
}
}

$scope = $scope->invalidateExpression($arrayArg)->specifyExpressionType($arrayArg, TypeCombinator::intersect($arrayType, new NonEmptyArrayType()));
} else {
$arrayTypes = [];
foreach ($constantArrays as $constantArray) {
$arrayTypeBuilder = ConstantArrayTypeBuilder::createFromConstantArray($defaultArrayType);
foreach ($constantArray->getKeyTypes() as $i => $keyType) {
$valueType = $constantArray->getValueTypes()[$i];
if ($keyType instanceof ConstantIntegerType) {
$keyType = null;
}
$arrayTypeBuilder->setOffsetValueType(
$keyType,
$valueType,
$constantArray->isOptionalKey($i),
);
}
$arrayTypes[] = $arrayTypeBuilder->getArray();
}
$arrayType = $arrayTypeBuilder->getArray();

$scope = $scope->invalidateExpression($arrayArg)->specifyExpressionType(
$arrayArg,
TypeCombinator::union(...$arrayTypes),
);
if ($arrayType instanceof ConstantArrayType && $nonConstantArrayWasUnpacked) {
$arrayType = $arrayType->generalize(GeneralizePrecision::lessSpecific());
}
} else {
$setOffsetValueTypes(
$scope,
$callArgs,
static function (?Type $offsetType, Type $valueType) use (&$arrayType): void {
$arrayType = $arrayType->setOffsetValueType($offsetType, $valueType);
},
);
}

$scope = $scope->invalidateExpression($arrayArg)->specifyExpressionType($arrayArg, $arrayType);
}

if (
Expand Down
2 changes: 2 additions & 0 deletions tests/PHPStan/Analyser/NodeScopeResolverTest.php
View file
Expand Up @@ -561,6 +561,7 @@ public function dataFileAsserts(): iterable
yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-1870.php');
yield from $this->gatherAssertTypes(__DIR__ . '/../Rules/Methods/data/bug-5562.php');
yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-5615.php');
yield from $this->gatherAssertTypes(__DIR__ . '/data/array-unshift.php');
yield from $this->gatherAssertTypes(__DIR__ . '/data/array_map_multiple.php');
yield from $this->gatherAssertTypes(__DIR__ . '/data/range-numeric-string.php');
yield from $this->gatherAssertTypes(__DIR__ . '/data/missing-closure-native-return-typehint.php');
Expand Down Expand Up @@ -838,6 +839,7 @@ public function dataFileAsserts(): iterable
yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-6439.php');
yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-6748.php');
yield from $this->gatherAssertTypes(__DIR__ . '/data/array-search-type-specifying.php');
yield from $this->gatherAssertTypes(__DIR__ . '/data/array-push.php');
yield from $this->gatherAssertTypes(__DIR__ . '/data/array-replace.php');
yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-6889.php');
yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-6891.php');
Expand Down
58 changes: 58 additions & 0 deletions tests/PHPStan/Analyser/data/array-push.php
View file
@@ -0,0 +1,58 @@
<?php

namespace ArrayPush;

use function array_push;
use function PHPStan\Testing\assertType;

/**
* @param string[] $a
* @param int[] $b
* @param non-empty-array<int> $c
* @param array<int|string> $d
*/
function arrayPush(array $a, array $b, array $c, array $d): void
{
array_push($a, ...$b);
assertType('non-empty-array<int|string>', $a);

array_push($b, ...[]);
assertType('array<int>', $b);

array_push($c, ...[19, 'baz', false]);
assertType('non-empty-array<\'baz\'|int|false>', $c);

/** @var array<bool|null> $d1 */
$d1 = [];
array_push($d, ...$d1);
assertType('non-empty-array<bool|int|string|null>', $d);
}

function arrayPushConstantArray(): void
{
$a = ['foo' => 17, 'a', 'bar' => 18,];
array_push($a, ...[19, 'baz', false]);
assertType('array{foo: 17, 0: \'a\', bar: 18, 1: 19, 2: \'baz\', 3: false}', $a);

$b = ['foo' => 17, 'a', 'bar' => 18];
array_push($b, 19, 'baz', false);
assertType('array{foo: 17, 0: \'a\', bar: 18, 1: 19, 2: \'baz\', 3: false}', $b);

$c = ['foo' => 17, 'a', 'bar' => 18];
array_push($c, ...[]);
assertType('array{foo: 17, 0: \'a\', bar: 18}', $c);

$d = [];
array_push($d, ...[]);
assertType('array{}', $d);

$e = [];
array_push($e, 19, 'baz', false);
assertType('array{19, \'baz\', false}', $e);

$f = [17];
/** @var array<bool|null> $f1 */
$f1 = [];
array_push($f, ...$f1);
assertType('non-empty-array<int, bool|int|null>', $f);
}
58 changes: 58 additions & 0 deletions tests/PHPStan/Analyser/data/array-unshift.php
View file
@@ -0,0 +1,58 @@
<?php

namespace ArrayUnshift;

use function array_unshift;
use function PHPStan\Testing\assertType;

/**
* @param string[] $a
* @param int[] $b
* @param non-empty-array<int> $c
* @param array<int|string> $d
*/
function arrayUnshift(array $a, array $b, array $c, array $d): void
{
array_unshift($a, ...$b);
assertType('non-empty-array<int|string>', $a);

array_unshift($b, ...[]);
assertType('array<int>', $b);

array_unshift($c, ...[19, 'baz', false]);
assertType('non-empty-array<\'baz\'|int|false>', $c);

/** @var array<bool|null> $d1 */
$d1 = [];
array_unshift($d, ...$d1);
assertType('non-empty-array<bool|int|string|null>', $d);
}

function arrayUnshiftConstantArray(): void
{
$a = ['foo' => 17, 'a', 'bar' => 18,];
array_unshift($a, ...[19, 'baz', false]);
assertType('array{0: 19, 1: \'baz\', 2: false, foo: 17, 3: \'a\', bar: 18}', $a);

$b = ['foo' => 17, 'a', 'bar' => 18];
array_unshift($b, 19, 'baz', false);
assertType('array{0: 19, 1: \'baz\', 2: false, foo: 17, 3: \'a\', bar: 18}', $b);

$c = ['foo' => 17, 'a', 'bar' => 18];
array_unshift($c, ...[]);
assertType('array{foo: 17, 0: \'a\', bar: 18}', $c);

$d = [];
array_unshift($d, ...[]);
assertType('array{}', $d);

$e = [];
array_unshift($e, 19, 'baz', false);
assertType('array{19, \'baz\', false}', $e);

$f = [17];
/** @var array<bool|null> $f1 */
$f1 = [];
array_unshift($f, ...$f1);
assertType('non-empty-array<int, bool|int|null>', $f);
}
15 changes: 15 additions & 0 deletions tests/PHPStan/Rules/Variables/EmptyRuleTest.php
View file
Expand Up @@ -81,4 +81,19 @@ public function testBug970(): void
]);
}

public function testBug6974(): void
{
$this->treatPhpDocTypesAsCertain = false;
$this->analyse([__DIR__ . '/data/bug-6974.php'], [
[
'Variable $a in empty() always exists and is always falsy.',
12,
],
[
'Variable $a in empty() always exists and is not falsy.',
30,
],
]);
}

}
32 changes: 32 additions & 0 deletions tests/PHPStan/Rules/Variables/data/bug-6974.php
View file
@@ -0,0 +1,32 @@
<?php declare(strict_types = 1);

namespace Bug6974;

class A
{
public function test1(): void
{
$a = [];
$b = [];
array_push($a, ...$b);
$c = empty($a) ? 'empty' : 'non-empty';
}

public function test2(): void
{
$a = [];
/** @var mixed[] $b */
$b = [];
array_push($a, ...$b);
$c = empty($a) ? 'empty' : 'non-empty';
}

public function test3(): void
{
$a = [];
/** @var non-empty-array<mixed> $b */
$b = [];
array_push($a, ...$b);
$c = empty($a) ? 'empty' : 'non-empty';
}
}

0 comments on commit 20539b3

Please sign in to comment.