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.

Sign up for GitHub

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

Improve count() narrowing of constant arrays #3709

Merged
merged 7 commits into from
Mar 10, 2025
Merged

Improve count() narrowing of constant arrays #3709

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
9534154
Improve `count()` narrowing of constant arrays
herndlm Dec 4, 2024
a12aaac
Consider int range offset in truthy context
herndlm Dec 5, 2024
74894a3
Add another regression test
herndlm Dec 24, 2024
a8b9b55
Clean up a bit more aggressively
herndlm Dec 25, 2024
6866fd4
Avoid specifying incorrect falsey context
herndlm Dec 25, 2024
f647fc8
Simplify by not using TypeTraverser
herndlm Dec 26, 2024
6153190
Simplify and improve :)
herndlm Jan 6, 2025
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
217 changes: 90 additions & 127 deletions src/Analyser/TypeSpecifier.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -272,22 +272,21 @@ public function specifyTypesInCondition(
) {
$argType = $scope->getType($expr->right->getArgs()[0]->value);

if ($argType instanceof UnionType) {
$sizeType = null;
if ($leftType instanceof ConstantIntegerType) {
if ($orEqual) {
$sizeType = IntegerRangeType::createAllGreaterThanOrEqualTo($leftType->getValue());
} else {
$sizeType = IntegerRangeType::createAllGreaterThan($leftType->getValue());
}
} elseif ($leftType instanceof IntegerRangeType) {
$sizeType = $leftType;
if ($leftType instanceof ConstantIntegerType) {
if ($orEqual) {
$sizeType = IntegerRangeType::createAllGreaterThanOrEqualTo($leftType->getValue());
} else {
$sizeType = IntegerRangeType::createAllGreaterThan($leftType->getValue());
}
} elseif ($leftType instanceof IntegerRangeType) {
$sizeType = $leftType->shift($offset);
} else {
$sizeType = $leftType;
}

$narrowed = $this->narrowUnionByArraySize($expr->right, $argType, $sizeType, $context, $scope, $expr);
if ($narrowed !== null) {
return $narrowed;
}
$specifiedTypes = $this->specifyTypesForCountFuncCall($expr->right, $argType, $sizeType, $context, $scope, $expr);
if ($specifiedTypes !== null) {
$result = $result->unionWith($specifiedTypes);
}

if (
Expand Down Expand Up @@ -1046,115 +1045,95 @@ public function specifyTypesInCondition(
return (new SpecifiedTypes([], []))->setRootExpr($expr);
}

private function narrowUnionByArraySize(FuncCall $countFuncCall, UnionType $argType, ?Type $sizeType, TypeSpecifierContext $context, Scope $scope, ?Expr $rootExpr): ?SpecifiedTypes
private function specifyTypesForCountFuncCall(
FuncCall $countFuncCall,
Type $type,
Type $sizeType,
TypeSpecifierContext $context,
Scope $scope,
Expr $rootExpr,
): ?SpecifiedTypes
{
if ($sizeType === null) {
return null;
}

if (count($countFuncCall->getArgs()) === 1) {
$isNormalCount = TrinaryLogic::createYes();
} else {
$mode = $scope->getType($countFuncCall->getArgs()[1]->value);
$isNormalCount = (new ConstantIntegerType(COUNT_NORMAL))->isSuperTypeOf($mode)->result->or($argType->getIterableValueType()->isArray()->negate());
$isNormalCount = (new ConstantIntegerType(COUNT_NORMAL))->isSuperTypeOf($mode)->result->or($type->getIterableValueType()->isArray()->negate());
}

$isList = $type->isList();
if (
$isNormalCount->yes()
&& $argType->isConstantArray()->yes()
!$isNormalCount->yes()
|| (!$type->isConstantArray()->yes() && !$isList->yes())
|| $type->isIterableAtLeastOnce()->no() // array{} cannot be used for further narrowing
) {
$result = [];
foreach ($argType->getTypes() as $innerType) {
$arraySize = $innerType->getArraySize();
$isSize = $sizeType->isSuperTypeOf($arraySize);
if ($context->truthy()) {
if ($isSize->no()) {
continue;
}

$constArray = $this->turnListIntoConstantArray($countFuncCall, $innerType, $sizeType, $scope);
if ($constArray !== null) {
$innerType = $constArray;
}
}
if ($context->falsey()) {
if (!$isSize->yes()) {
continue;
}
}

$result[] = $innerType;
}

return $this->create($countFuncCall->getArgs()[0]->value, TypeCombinator::union(...$result), $context, $scope)->setRootExpr($rootExpr);
return null;
}

return null;
}

private function turnListIntoConstantArray(FuncCall $countFuncCall, Type $type, Type $sizeType, Scope $scope): ?Type
{
$argType = $scope->getType($countFuncCall->getArgs()[0]->value);

if (count($countFuncCall->getArgs()) === 1) {
$isNormalCount = TrinaryLogic::createYes();
} else {
$mode = $scope->getType($countFuncCall->getArgs()[1]->value);
$isNormalCount = (new ConstantIntegerType(COUNT_NORMAL))->isSuperTypeOf($mode)->result->or($argType->getIterableValueType()->isArray()->negate());
}
$resultTypes = [];
foreach ($type->getArrays() as $arrayType) {
$isSizeSuperTypeOfArraySize = $sizeType->isSuperTypeOf($arrayType->getArraySize());
if ($isSizeSuperTypeOfArraySize->no()) {
continue;
}

if (
$isNormalCount->yes()
&& $type->isList()->yes()
&& $sizeType instanceof ConstantIntegerType
&& $sizeType->getValue() < ConstantArrayTypeBuilder::ARRAY_COUNT_LIMIT
) {
// turn optional offsets non-optional
$valueTypesBuilder = ConstantArrayTypeBuilder::createEmpty();
for ($i = 0; $i < $sizeType->getValue(); $i++) {
$offsetType = new ConstantIntegerType($i);
$valueTypesBuilder->setOffsetValueType($offsetType, $type->getOffsetValueType($offsetType));
if ($context->falsey() && $isSizeSuperTypeOfArraySize->maybe()) {
continue;
}
return $valueTypesBuilder->getArray();
}

if (
$isNormalCount->yes()
&& $type->isList()->yes()
&& $sizeType instanceof IntegerRangeType
&& $sizeType->getMin() !== null
) {
// turn optional offsets non-optional
$valueTypesBuilder = ConstantArrayTypeBuilder::createEmpty();
for ($i = 0; $i < $sizeType->getMin(); $i++) {
$offsetType = new ConstantIntegerType($i);
$valueTypesBuilder->setOffsetValueType($offsetType, $type->getOffsetValueType($offsetType));
}
if ($sizeType->getMax() !== null) {
for ($i = $sizeType->getMin(); $i < $sizeType->getMax(); $i++) {
if (
$isList->yes()
&& $sizeType instanceof ConstantIntegerType
&& $sizeType->getValue() < ConstantArrayTypeBuilder::ARRAY_COUNT_LIMIT
) {
// turn optional offsets non-optional
$valueTypesBuilder = ConstantArrayTypeBuilder::createEmpty();
for ($i = 0; $i < $sizeType->getValue(); $i++) {
$offsetType = new ConstantIntegerType($i);
$valueTypesBuilder->setOffsetValueType($offsetType, $type->getOffsetValueType($offsetType), true);
$valueTypesBuilder->setOffsetValueType($offsetType, $arrayType->getOffsetValueType($offsetType));
}
} elseif ($type->isConstantArray()->yes()) {
for ($i = $sizeType->getMin();; $i++) {
$resultTypes[] = $valueTypesBuilder->getArray();
continue;
}

if (
$isList->yes()
&& $sizeType instanceof IntegerRangeType
&& $sizeType->getMin() !== null
) {
// turn optional offsets non-optional
$valueTypesBuilder = ConstantArrayTypeBuilder::createEmpty();
for ($i = 0; $i < $sizeType->getMin(); $i++) {
$offsetType = new ConstantIntegerType($i);
$hasOffset = $type->hasOffsetValueType($offsetType);
if ($hasOffset->no()) {
break;
$valueTypesBuilder->setOffsetValueType($offsetType, $arrayType->getOffsetValueType($offsetType));
}
if ($sizeType->getMax() !== null) {
for ($i = $sizeType->getMin(); $i < $sizeType->getMax(); $i++) {
$offsetType = new ConstantIntegerType($i);
$valueTypesBuilder->setOffsetValueType($offsetType, $arrayType->getOffsetValueType($offsetType), true);
}
$valueTypesBuilder->setOffsetValueType($offsetType, $type->getOffsetValueType($offsetType), !$hasOffset->yes());
} elseif ($arrayType->isConstantArray()->yes()) {
for ($i = $sizeType->getMin();; $i++) {
$offsetType = new ConstantIntegerType($i);
$hasOffset = $arrayType->hasOffsetValueType($offsetType);
if ($hasOffset->no()) {
break;
}
$valueTypesBuilder->setOffsetValueType($offsetType, $arrayType->getOffsetValueType($offsetType), !$hasOffset->yes());
}
} else {
$resultTypes[] = TypeCombinator::intersect($arrayType, new NonEmptyArrayType());
continue;
}
} else {
return null;
}

$arrayType = $valueTypesBuilder->getArray();
if ($arrayType->isIterableAtLeastOnce()->yes()) {
return $arrayType;
$resultTypes[] = $valueTypesBuilder->getArray();
continue;
}

$resultTypes[] = $arrayType;
}

return null;
return $this->create($countFuncCall->getArgs()[0]->value, TypeCombinator::union(...$resultTypes), $context, $scope)->setRootExpr($rootExpr);
}

private function specifyTypesForConstantBinaryExpression(
Expand Down Expand Up @@ -2186,36 +2165,20 @@ public function resolveIdentical(Expr\BinaryOp\Identical $expr, Scope $scope, Ty
);
}

if ($argType instanceof UnionType) {
$narrowed = $this->narrowUnionByArraySize($unwrappedLeftExpr, $argType, $rightType, $context, $scope, $expr);
if ($narrowed !== null) {
return $narrowed;
}
$specifiedTypes = $this->specifyTypesForCountFuncCall($unwrappedLeftExpr, $argType, $rightType, $context, $scope, $expr);
if ($specifiedTypes !== null) {
return $specifiedTypes;
}

if ($context->truthy()) {
if ($argType->isArray()->yes()) {
if (
$argType->isConstantArray()->yes()
&& $rightType->isSuperTypeOf($argType->getArraySize())->no()
) {
return $this->create($unwrappedLeftExpr->getArgs()[0]->value, new NeverType(), $context, $scope)->setRootExpr($expr);
}

$funcTypes = $this->create($unwrappedLeftExpr, $rightType, $context, $scope)->setRootExpr($expr);
$constArray = $this->turnListIntoConstantArray($unwrappedLeftExpr, $argType, $rightType, $scope);
if ($constArray !== null) {
return $funcTypes->unionWith(
$this->create($unwrappedLeftExpr->getArgs()[0]->value, $constArray, $context, $scope)->setRootExpr($expr),
);
} elseif (IntegerRangeType::fromInterval(1, null)->isSuperTypeOf($rightType)->yes()) {
return $funcTypes->unionWith(
$this->create($unwrappedLeftExpr->getArgs()[0]->value, new NonEmptyArrayType(), $context, $scope)->setRootExpr($expr),
);
}

return $funcTypes;
if ($context->truthy() && $argType->isArray()->yes()) {
$funcTypes = $this->create($unwrappedLeftExpr, $rightType, $context, $scope)->setRootExpr($expr);
if (IntegerRangeType::fromInterval(1, null)->isSuperTypeOf($rightType)->yes()) {
return $funcTypes->unionWith(
$this->create($unwrappedLeftExpr->getArgs()[0]->value, new NonEmptyArrayType(), $context, $scope)->setRootExpr($expr),
);
}

return $funcTypes;
}
}

Expand Down
8 changes: 4 additions & 4 deletions tests/PHPStan/Analyser/nsrt/bug-4700.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,10 @@ function(array $array, int $count): void {
if (isset($array['d'])) $a[] = $array['d'];
if (isset($array['e'])) $a[] = $array['e'];
if (count($a) > $count) {
assertType('int<1, 5>', count($a));
assertType('array{0: mixed~null, 1?: mixed~null, 2?: mixed~null, 3?: mixed~null, 4?: mixed~null}', $a);
assertType('int<2, 5>', count($a));
assertType('list{0: mixed~null, 1: mixed~null, 2?: mixed~null, 3?: mixed~null, 4?: mixed~null}', $a);
} else {
assertType('0', count($a));
assertType('array{}', $a);
assertType('int<0, 5>', count($a)); // Could be int<0, 1>
assertType('array{}|array{0: mixed~null, 1?: mixed~null, 2?: mixed~null, 3?: mixed~null, 4?: mixed~null}', $a); // Could be array{}|array{0: mixed~null}
}
};
2 changes: 1 addition & 1 deletion tests/PHPStan/Analyser/nsrt/bug11480.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ public function intUnionCount(): void
if (count($x) >= $count) {
assertType("array{'xy'}|array{0: 'ab', 1?: 'xy'}", $x);
} else {
assertType("array{}|array{'xy'}|array{0: 'ab', 1?: 'xy'}", $x);
assertType("array{}", $x);
}
assertType("array{}|array{'xy'}|array{0: 'ab', 1?: 'xy'}", $x);
}
Expand Down
23 changes: 23 additions & 0 deletions tests/PHPStan/Analyser/nsrt/count-type.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,29 @@ public function doFooBar(
}
}

/** @param array{0: string, 1?: string} $arr */
public function doBar(array $arr): void
{
if (count($arr) <= 1) {
assertType('1', count($arr));
return;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about adding assertType('array{string}', $arr); ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point, I already forgot. but quickly looked again and apparently it's currently

assertType('array{0: string, 1?: string}', $arr);

it ends up using a falsey scope here, because count($arr) <= 1 is transformed to 2 < count($arr) and then something in the loop still skips creating a result type because it can't fully deal with falsey scopes unfortunately. If I remove that continue, it would basically come up with "cannot be array{string, string}" which should make it then array{string} by some falsey scope filtering or so irrelevant of the new code. but that does also not work :/ and many tests for (list) counting start to fail :/ would be great to further improve this, but I was kind of stuck there..

}

assertType('2', count($arr));
assertType('array{string, string}', $arr);
}

/** @param array{0: string, 1?: string} $arr */
public function doBaz(array $arr): void
{
if (count($arr) > 1) {
assertType('2', count($arr));
assertType('array{string, string}', $arr);
}

assertType('1|2', count($arr));
}

}

/**
Expand Down
5 changes: 5 additions & 0 deletions tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -545,6 +545,11 @@ public function testBug3608(): void
$this->analyse([__DIR__ . '/data/bug-3608.php'], []);
}

public function testBug3631(): void
{
$this->analyse([__DIR__ . '/data/bug-3631.php'], []);
}

public function testBug3920(): void
{
$this->analyse([__DIR__ . '/data/bug-3920.php'], []);
Expand Down
27 changes: 27 additions & 0 deletions tests/PHPStan/Rules/Functions/data/bug-3631.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
<?php declare(strict_types = 1);

namespace Bug3631;

/**
* @return array<string>
*/
function someFunc(bool $flag): array
{
$ids = [
['fa', 'foo', 'baz']
];

if ($flag) {
$ids[] = ['foo', 'bar', 'baz'];

}

if (count($ids) > 1) {
return array_intersect(...$ids);
}

return $ids[0];
}

var_dump(someFunc(true));
var_dump(someFunc(false));
Loading