Skip to content
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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider optional keys in ConstantArrayType removeFirst and removeLast #1352

Merged
merged 1 commit into from May 27, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
37 changes: 18 additions & 19 deletions src/Analyser/NodeScopeResolver.php
Expand Up @@ -126,6 +126,7 @@
use PHPStan\Type\Generic\TemplateTypeHelper;
use PHPStan\Type\Generic\TemplateTypeMap;
use PHPStan\Type\IntegerType;
use PHPStan\Type\IntersectionType;
use PHPStan\Type\MixedType;
use PHPStan\Type\NeverType;
use PHPStan\Type\NullType;
Expand Down Expand Up @@ -1813,29 +1814,27 @@ function (MutatingScope $scope) use ($expr, $nodeCallback, $context): Expression
&& count($expr->getArgs()) >= 1
) {
$arrayArg = $expr->getArgs()[0]->value;
$constantArrays = TypeUtils::getConstantArrays($scope->getType($arrayArg));
$arrayArgType = $scope->getType($arrayArg);
$scope = $scope->invalidateExpression($arrayArg);
if (count($constantArrays) > 0) {
$resultArrayTypes = [];

foreach ($constantArrays as $constantArray) {
if ($functionReflection->getName() === 'array_pop') {
$resultArrayTypes[] = $constantArray->removeLast();
} else {
$resultArrayTypes[] = $constantArray->removeFirst();
}
$functionName = $functionReflection->getName();
$arrayArgType = TypeTraverser::map($arrayArgType, static function (Type $type, callable $traverse) use ($functionName): Type {
if ($type instanceof UnionType || $type instanceof IntersectionType) {
return $traverse($type);
}

$scope = $scope->specifyExpressionType(
$arrayArg,
TypeCombinator::union(...$resultArrayTypes),
);
} else {
$arrays = TypeUtils::getAnyArrays($scope->getType($arrayArg));
if (count($arrays) > 0) {
$scope = $scope->specifyExpressionType($arrayArg, TypeCombinator::union(...$arrays));
if ($type instanceof ConstantArrayType) {
return $functionName === 'array_pop' ? $type->removeLast() : $type->removeFirst();
}
}
if ($type->isIterableAtLeastOnce()->yes()) {
return $type->toArray();
}
return $type;
});

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

if (
Expand Down
22 changes: 20 additions & 2 deletions src/Type/Constant/ConstantArrayType.php
Expand Up @@ -558,7 +558,17 @@ public function removeLast(): self
$keyTypes = $this->keyTypes;
$valueTypes = $this->valueTypes;
$optionalKeys = $this->optionalKeys;
unset($optionalKeys[$i]);

if ($this->isOptionalKey($i)) {
unset($optionalKeys[$i]);
// Removing the last optional element makes the previous non-optional element optional
for ($j = $i - 1; $j >= 0; $j--) {
if (!$this->isOptionalKey($j)) {
$optionalKeys[] = $j;
break;
}
}
}

$removedKeyType = array_pop($keyTypes);
array_pop($valueTypes);
Expand All @@ -577,17 +587,25 @@ public function removeLast(): self
public function removeFirst(): Type
{
$builder = ConstantArrayTypeBuilder::createEmpty();
$makeNextNonOptionalOptional = false;
foreach ($this->keyTypes as $i => $keyType) {
$isOptional = $this->isOptionalKey($i);
if ($i === 0) {
$makeNextNonOptionalOptional = $isOptional;
continue;
}

if (!$isOptional && $makeNextNonOptionalOptional) {
$isOptional = true;
$makeNextNonOptionalOptional = false;
}

$valueType = $this->valueTypes[$i];
if ($keyType instanceof ConstantIntegerType) {
$keyType = null;
}

$builder->setOffsetValueType($keyType, $valueType);
$builder->setOffsetValueType($keyType, $valueType, $isOptional);
}

return $builder->getArray();
Expand Down
2 changes: 2 additions & 0 deletions tests/PHPStan/Analyser/NodeScopeResolverTest.php
Expand Up @@ -179,6 +179,7 @@ public function dataFileAsserts(): iterable

yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-3985.php');

yield from $this->gatherAssertTypes(__DIR__ . '/data/array-shift.php');
yield from $this->gatherAssertTypes(__DIR__ . '/data/array-slice.php');

yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-3990.php');
Expand Down Expand Up @@ -800,6 +801,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-pop.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/array-reverse.php');
Expand Down
53 changes: 53 additions & 0 deletions tests/PHPStan/Analyser/data/array-pop.php
@@ -0,0 +1,53 @@
<?php declare(strict_types = 1);

namespace ArrayPop;

use function PHPStan\Testing\assertType;

class Foo
{

public function nonEmpty(array $arr): void
{
/** @var non-empty-array<string> $arr */
assertType('string', array_pop($arr));
assertType('array<string>', $arr);
}

public function normalArrays(array $arr): void
{
/** @var array<int, string> $arr */
assertType('string|null', array_pop($arr));
assertType('array<int, string>', $arr);
}

public function compoundTypes(array $arr): void
{
/** @var string[]|int[] $arr */
assertType('int|string|null', array_pop($arr));
assertType('array<int|string>', $arr);
}

public function constantArrays(array $arr): void
{
/** @var array{a: 0, b: 1, c: 2} $arr */
assertType('2', array_pop($arr));
assertType('array{a: 0, b: 1}', $arr);
}

public function constantArraysWithOptionalKeys(array $arr): void
{
/** @var array{a?: 0, b: 1, c: 2} $arr */
assertType('0|2', array_pop($arr)); // should be 2
assertType('array{a?: 0, b: 1}', $arr);

/** @var array{a: 0, b?: 1, c: 2} $arr */
assertType('1|2', array_pop($arr)); // should be 2
assertType('array{a: 0, b?: 1}', $arr);

/** @var array{a: 0, b: 1, c?: 2} $arr */
assertType('1|2', array_pop($arr));
assertType('array{a: 0, b?: 1}', $arr);
}

}
53 changes: 53 additions & 0 deletions tests/PHPStan/Analyser/data/array-shift.php
@@ -0,0 +1,53 @@
<?php declare(strict_types = 1);

namespace ArrayShift;

use function PHPStan\Testing\assertType;

class Foo
{

public function nonEmpty(array $arr): void
{
/** @var non-empty-array<string> $arr */
assertType('string', array_shift($arr));
assertType('array<string>', $arr);
}

public function normalArrays(array $arr): void
{
/** @var array<int, string> $arr */
assertType('string|null', array_shift($arr));
assertType('array<int, string>', $arr);
}

public function compoundTypes(array $arr): void
{
/** @var string[]|int[] $arr */
assertType('int|string|null', array_shift($arr));
assertType('array<int|string>', $arr);
}

public function constantArrays(array $arr): void
{
/** @var array{a: 0, b: 1, c: 2} $arr */
assertType('0', array_shift($arr));
assertType('array{b: 1, c: 2}', $arr);
}

public function constantArraysWithOptionalKeys(array $arr): void
{
/** @var array{a?: 0, b: 1, c: 2} $arr */
assertType('1', array_shift($arr)); // should be 0|1
assertType('array{b?: 1, c: 2}', $arr);

/** @var array{a: 0, b?: 1, c: 2} $arr */
assertType('0', array_shift($arr));
assertType('array{b?: 1, c: 2}', $arr);

/** @var array{a: 0, b: 1, c?: 2} $arr */
assertType('0', array_shift($arr));
assertType('array{b: 1, c?: 2}', $arr);
}

}