Skip to content

Commit 9bb13b0

Browse files
herndlmondrejmirtes
herndlm
authored and
ondrejmirtes
committed
Fix array_push with an empty constant array
1 parent cf43e1f commit 9bb13b0

File tree

6 files changed

+230
-60
lines changed

6 files changed

+230
-60
lines changed

‎src/Analyser/NodeScopeResolver.php‎

Lines changed: 65 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
use Closure;
77
use PhpParser\Comment\Doc;
88
use PhpParser\Node;
9+
use PhpParser\Node\Arg;
910
use PhpParser\Node\Expr;
1011
use PhpParser\Node\Expr\Array_;
1112
use PhpParser\Node\Expr\ArrayDimFetch;
@@ -109,16 +110,15 @@
109110
use PHPStan\Reflection\ReflectionProvider;
110111
use PHPStan\ShouldNotHappenException;
111112
use PHPStan\TrinaryLogic;
112-
use PHPStan\Type\Accessory\NonEmptyArrayType;
113113
use PHPStan\Type\ArrayType;
114114
use PHPStan\Type\ClosureType;
115115
use PHPStan\Type\Constant\ConstantArrayType;
116116
use PHPStan\Type\Constant\ConstantArrayTypeBuilder;
117117
use PHPStan\Type\Constant\ConstantBooleanType;
118-
use PHPStan\Type\Constant\ConstantIntegerType;
119118
use PHPStan\Type\Constant\ConstantStringType;
120119
use PHPStan\Type\ErrorType;
121120
use PHPStan\Type\FileTypeMapper;
121+
use PHPStan\Type\GeneralizePrecision;
122122
use PHPStan\Type\Generic\GenericClassStringType;
123123
use PHPStan\Type\Generic\TemplateTypeHelper;
124124
use PHPStan\Type\Generic\TemplateTypeMap;
@@ -1843,75 +1843,80 @@ function (MutatingScope $scope) use ($expr, $nodeCallback, $context): Expression
18431843
&& in_array($functionReflection->getName(), ['array_push', 'array_unshift'], true)
18441844
&& count($expr->getArgs()) >= 2
18451845
) {
1846-
$argumentTypes = [];
1847-
foreach (array_slice($expr->getArgs(), 1) as $callArg) {
1848-
$callArgType = $scope->getType($callArg->value);
1849-
if ($callArg->unpack) {
1850-
$iterableValueType = $callArgType->getIterableValueType();
1851-
if ($iterableValueType instanceof UnionType) {
1852-
foreach ($iterableValueType->getTypes() as $innerType) {
1853-
$argumentTypes[] = $innerType;
1846+
$arrayArg = $expr->getArgs()[0]->value;
1847+
$arrayType = $scope->getType($arrayArg);
1848+
$callArgs = array_slice($expr->getArgs(), 1);
1849+
1850+
/**
1851+
* @param Arg[] $callArgs
1852+
* @param callable(?Type, Type, bool=): void $setOffsetValueType
1853+
*/
1854+
$setOffsetValueTypes = static function (Scope $scope, array $callArgs, callable $setOffsetValueType, ?bool &$nonConstantArrayWasUnpacked = null): void {
1855+
foreach ($callArgs as $callArg) {
1856+
$callArgType = $scope->getType($callArg->value);
1857+
if ($callArg->unpack) {
1858+
if ($callArgType->isIterableAtLeastOnce()->no()) {
1859+
continue;
18541860
}
1855-
} else {
1856-
$argumentTypes[] = $iterableValueType;
1861+
if (!$callArgType instanceof ConstantArrayType) {
1862+
$nonConstantArrayWasUnpacked = true;
1863+
}
1864+
$iterableValueType = $callArgType->getIterableValueType();
1865+
$isOptional = !$callArgType->isIterableAtLeastOnce()->yes();
1866+
if ($iterableValueType instanceof UnionType) {
1867+
foreach ($iterableValueType->getTypes() as $innerType) {
1868+
$setOffsetValueType(null, $innerType, $isOptional);
1869+
}
1870+
} else {
1871+
$setOffsetValueType(null, $iterableValueType, $isOptional);
1872+
}
1873+
continue;
18571874
}
1858-
continue;
1875+
$setOffsetValueType(null, $callArgType);
18591876
}
1877+
};
18601878

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

1864-
$arrayArg = $expr->getArgs()[0]->value;
1865-
$originalArrayType = $scope->getType($arrayArg);
1866-
$constantArrays = TypeUtils::getOldConstantArrays($originalArrayType);
1867-
if (
1868-
$functionReflection->getName() === 'array_push'
1869-
|| ($originalArrayType->isArray()->yes() && count($constantArrays) === 0)
1870-
) {
1871-
$arrayType = $originalArrayType;
1872-
foreach ($argumentTypes as $argType) {
1873-
$arrayType = $arrayType->setOffsetValueType(null, $argType);
1874-
}
1875-
1876-
$scope = $scope->invalidateExpression($arrayArg)->specifyExpressionType($arrayArg, TypeCombinator::intersect($arrayType, new NonEmptyArrayType()));
1877-
} elseif (count($constantArrays) > 0) {
1878-
$defaultArrayBuilder = ConstantArrayTypeBuilder::createEmpty();
1879-
foreach ($argumentTypes as $argType) {
1880-
$defaultArrayBuilder->setOffsetValueType(null, $argType);
1881-
}
1883+
$setOffsetValueTypes(
1884+
$scope,
1885+
$callArgs,
1886+
static function (?Type $offsetType, Type $valueType, bool $optional = false) use (&$arrayTypeBuilder): void {
1887+
$arrayTypeBuilder->setOffsetValueType($offsetType, $valueType, $optional);
1888+
},
1889+
$nonConstantArrayWasUnpacked,
1890+
);
18821891

1883-
$defaultArrayType = $defaultArrayBuilder->getArray();
1884-
if (!$defaultArrayType instanceof ConstantArrayType) {
1885-
$arrayType = $originalArrayType;
1886-
foreach ($argumentTypes as $argType) {
1887-
$arrayType = $arrayType->setOffsetValueType(null, $argType);
1892+
if ($prepend) {
1893+
$keyTypes = $arrayType->getKeyTypes();
1894+
$valueTypes = $arrayType->getValueTypes();
1895+
foreach ($keyTypes as $k => $keyType) {
1896+
$arrayTypeBuilder->setOffsetValueType(
1897+
$keyType instanceof ConstantStringType ? $keyType : null,
1898+
$valueTypes[$k],
1899+
$arrayType->isOptionalKey($k),
1900+
);
18881901
}
1902+
}
18891903

1890-
$scope = $scope->invalidateExpression($arrayArg)->specifyExpressionType($arrayArg, TypeCombinator::intersect($arrayType, new NonEmptyArrayType()));
1891-
} else {
1892-
$arrayTypes = [];
1893-
foreach ($constantArrays as $constantArray) {
1894-
$arrayTypeBuilder = ConstantArrayTypeBuilder::createFromConstantArray($defaultArrayType);
1895-
foreach ($constantArray->getKeyTypes() as $i => $keyType) {
1896-
$valueType = $constantArray->getValueTypes()[$i];
1897-
if ($keyType instanceof ConstantIntegerType) {
1898-
$keyType = null;
1899-
}
1900-
$arrayTypeBuilder->setOffsetValueType(
1901-
$keyType,
1902-
$valueType,
1903-
$constantArray->isOptionalKey($i),
1904-
);
1905-
}
1906-
$arrayTypes[] = $arrayTypeBuilder->getArray();
1907-
}
1904+
$arrayType = $arrayTypeBuilder->getArray();
19081905

1909-
$scope = $scope->invalidateExpression($arrayArg)->specifyExpressionType(
1910-
$arrayArg,
1911-
TypeCombinator::union(...$arrayTypes),
1912-
);
1906+
if ($arrayType instanceof ConstantArrayType && $nonConstantArrayWasUnpacked) {
1907+
$arrayType = $arrayType->generalize(GeneralizePrecision::lessSpecific());
19131908
}
1909+
} else {
1910+
$setOffsetValueTypes(
1911+
$scope,
1912+
$callArgs,
1913+
static function (?Type $offsetType, Type $valueType) use (&$arrayType): void {
1914+
$arrayType = $arrayType->setOffsetValueType($offsetType, $valueType);
1915+
},
1916+
);
19141917
}
1918+
1919+
$scope = $scope->invalidateExpression($arrayArg)->specifyExpressionType($arrayArg, $arrayType);
19151920
}
19161921

19171922
if (

‎tests/PHPStan/Analyser/NodeScopeResolverTest.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -561,6 +561,7 @@ public function dataFileAsserts(): iterable
561561
yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-1870.php');
562562
yield from $this->gatherAssertTypes(__DIR__ . '/../Rules/Methods/data/bug-5562.php');
563563
yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-5615.php');
564+
yield from $this->gatherAssertTypes(__DIR__ . '/data/array-unshift.php');
564565
yield from $this->gatherAssertTypes(__DIR__ . '/data/array_map_multiple.php');
565566
yield from $this->gatherAssertTypes(__DIR__ . '/data/range-numeric-string.php');
566567
yield from $this->gatherAssertTypes(__DIR__ . '/data/missing-closure-native-return-typehint.php');
@@ -838,6 +839,7 @@ public function dataFileAsserts(): iterable
838839
yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-6439.php');
839840
yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-6748.php');
840841
yield from $this->gatherAssertTypes(__DIR__ . '/data/array-search-type-specifying.php');
842+
yield from $this->gatherAssertTypes(__DIR__ . '/data/array-push.php');
841843
yield from $this->gatherAssertTypes(__DIR__ . '/data/array-replace.php');
842844
yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-6889.php');
843845
yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-6891.php');

‎tests/PHPStan/Analyser/data/array-push.php‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
<?php
2+
3+
namespace ArrayPush;
4+
5+
use function array_push;
6+
use function PHPStan\Testing\assertType;
7+
8+
/**
9+
* @param string[] $a
10+
* @param int[] $b
11+
* @param non-empty-array<int> $c
12+
* @param array<int|string> $d
13+
*/
14+
function arrayPush(array $a, array $b, array $c, array $d): void
15+
{
16+
array_push($a, ...$b);
17+
assertType('non-empty-array<int|string>', $a);
18+
19+
array_push($b, ...[]);
20+
assertType('array<int>', $b);
21+
22+
array_push($c, ...[19, 'baz', false]);
23+
assertType('non-empty-array<\'baz\'|int|false>', $c);
24+
25+
/** @var array<bool|null> $d1 */
26+
$d1 = [];
27+
array_push($d, ...$d1);
28+
assertType('non-empty-array<bool|int|string|null>', $d);
29+
}
30+
31+
function arrayPushConstantArray(): void
32+
{
33+
$a = ['foo' => 17, 'a', 'bar' => 18,];
34+
array_push($a, ...[19, 'baz', false]);
35+
assertType('array{foo: 17, 0: \'a\', bar: 18, 1: 19, 2: \'baz\', 3: false}', $a);
36+
37+
$b = ['foo' => 17, 'a', 'bar' => 18];
38+
array_push($b, 19, 'baz', false);
39+
assertType('array{foo: 17, 0: \'a\', bar: 18, 1: 19, 2: \'baz\', 3: false}', $b);
40+
41+
$c = ['foo' => 17, 'a', 'bar' => 18];
42+
array_push($c, ...[]);
43+
assertType('array{foo: 17, 0: \'a\', bar: 18}', $c);
44+
45+
$d = [];
46+
array_push($d, ...[]);
47+
assertType('array{}', $d);
48+
49+
$e = [];
50+
array_push($e, 19, 'baz', false);
51+
assertType('array{19, \'baz\', false}', $e);
52+
53+
$f = [17];
54+
/** @var array<bool|null> $f1 */
55+
$f1 = [];
56+
array_push($f, ...$f1);
57+
assertType('non-empty-array<int, bool|int|null>', $f);
58+
}

‎tests/PHPStan/Analyser/data/array-unshift.php‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
<?php
2+
3+
namespace ArrayUnshift;
4+
5+
use function array_unshift;
6+
use function PHPStan\Testing\assertType;
7+
8+
/**
9+
* @param string[] $a
10+
* @param int[] $b
11+
* @param non-empty-array<int> $c
12+
* @param array<int|string> $d
13+
*/
14+
function arrayUnshift(array $a, array $b, array $c, array $d): void
15+
{
16+
array_unshift($a, ...$b);
17+
assertType('non-empty-array<int|string>', $a);
18+
19+
array_unshift($b, ...[]);
20+
assertType('array<int>', $b);
21+
22+
array_unshift($c, ...[19, 'baz', false]);
23+
assertType('non-empty-array<\'baz\'|int|false>', $c);
24+
25+
/** @var array<bool|null> $d1 */
26+
$d1 = [];
27+
array_unshift($d, ...$d1);
28+
assertType('non-empty-array<bool|int|string|null>', $d);
29+
}
30+
31+
function arrayUnshiftConstantArray(): void
32+
{
33+
$a = ['foo' => 17, 'a', 'bar' => 18,];
34+
array_unshift($a, ...[19, 'baz', false]);
35+
assertType('array{0: 19, 1: \'baz\', 2: false, foo: 17, 3: \'a\', bar: 18}', $a);
36+
37+
$b = ['foo' => 17, 'a', 'bar' => 18];
38+
array_unshift($b, 19, 'baz', false);
39+
assertType('array{0: 19, 1: \'baz\', 2: false, foo: 17, 3: \'a\', bar: 18}', $b);
40+
41+
$c = ['foo' => 17, 'a', 'bar' => 18];
42+
array_unshift($c, ...[]);
43+
assertType('array{foo: 17, 0: \'a\', bar: 18}', $c);
44+
45+
$d = [];
46+
array_unshift($d, ...[]);
47+
assertType('array{}', $d);
48+
49+
$e = [];
50+
array_unshift($e, 19, 'baz', false);
51+
assertType('array{19, \'baz\', false}', $e);
52+
53+
$f = [17];
54+
/** @var array<bool|null> $f1 */
55+
$f1 = [];
56+
array_unshift($f, ...$f1);
57+
assertType('non-empty-array<int, bool|int|null>', $f);
58+
}

‎tests/PHPStan/Rules/Variables/EmptyRuleTest.php‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,4 +81,19 @@ public function testBug970(): void
8181
]);
8282
}
8383

84+
public function testBug6974(): void
85+
{
86+
$this->treatPhpDocTypesAsCertain = false;
87+
$this->analyse([__DIR__ . '/data/bug-6974.php'], [
88+
[
89+
'Variable $a in empty() always exists and is always falsy.',
90+
12,
91+
],
92+
[
93+
'Variable $a in empty() always exists and is not falsy.',
94+
30,
95+
],
96+
]);
97+
}
98+
8499
}

‎tests/PHPStan/Rules/Variables/data/bug-6974.php‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace Bug6974;
4+
5+
class A
6+
{
7+
public function test1(): void
8+
{
9+
$a = [];
10+
$b = [];
11+
array_push($a, ...$b);
12+
$c = empty($a) ? 'empty' : 'non-empty';
13+
}
14+
15+
public function test2(): void
16+
{
17+
$a = [];
18+
/** @var mixed[] $b */
19+
$b = [];
20+
array_push($a, ...$b);
21+
$c = empty($a) ? 'empty' : 'non-empty';
22+
}
23+
24+
public function test3(): void
25+
{
26+
$a = [];
27+
/** @var non-empty-array<mixed> $b */
28+
$b = [];
29+
array_push($a, ...$b);
30+
$c = empty($a) ? 'empty' : 'non-empty';
31+
}
32+
}

0 commit comments

Comments
 (0)