Skip to content

Narrow array on count(), string on strlen() with positive-int #3373

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

Closed
wants to merge 5 commits into from
+169 −107
Closed

Narrow array on count(), string on strlen() with positive-int #3373

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5769d82
Narrow array on count(), string on strlen() with positive-int
staabm Aug 30, 2024
b89a636
Update TypeSpecifier.php
staabm Aug 30, 2024
ab465fb
Update TypeSpecifier.php
staabm Aug 30, 2024
4d82fd1
Update TypeSpecifier.php
staabm Aug 30, 2024
517f74f
fix count(mixed), count(Countable)
staabm Aug 30, 2024
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
215 changes: 114 additions & 101 deletions src/Analyser/TypeSpecifier.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
use PHPStan\Type\ArrayType;
use PHPStan\Type\BooleanType;
use PHPStan\Type\ConditionalTypeForParameter;
use PHPStan\Type\Constant\ConstantArrayType;
use PHPStan\Type\Constant\ConstantArrayTypeBuilder;
use PHPStan\Type\Constant\ConstantBooleanType;
use PHPStan\Type\Constant\ConstantIntegerType;
Expand Down Expand Up @@ -1037,7 +1038,7 @@ private function turnListIntoConstantArray(FuncCall $countFuncCall, Type $type,
$offsetType = new ConstantIntegerType($i);
$valueTypesBuilder->setOffsetValueType($offsetType, $type->getOffsetValueType($offsetType), true);
}
} else {
} elseif ($type->isConstantArray()->yes()) {
Copy link
Contributor Author

@staabm staabm Aug 30, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

compensated the now passed in maybe unbound IntegerRangeTypes

for ($i = $sizeType->getMin();; $i++) {
$offsetType = new ConstantIntegerType($i);
$hasOffset = $type->hasOffsetValueType($offsetType);
Expand All @@ -1048,25 +1049,25 @@ private function turnListIntoConstantArray(FuncCall $countFuncCall, Type $type,
}

}
return $valueTypesBuilder->getArray();

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

return null;
}

private function specifyTypesForConstantBinaryExpression(
Expr $exprNode,
Type $constantType,
ConstantScalarType $constantType,
TypeSpecifierContext $context,
Scope $scope,
?Expr $rootExpr,
): ?SpecifiedTypes
{
$scalarValues = $constantType->getConstantScalarValues();
if (count($scalarValues) !== 1) {
return null;
}
$constValue = $scalarValues[0];
$constValue = $constantType->getValue();

if (!$context->null() && $constValue === false) {
$types = $this->create($exprNode, $constantType, $context, false, $scope, $rootExpr);
Expand Down Expand Up @@ -1096,103 +1097,18 @@ private function specifyTypesForConstantBinaryExpression(
));
}

if (
Copy link
Contributor Author

Choose a reason for hiding this comment

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

moved this 2 if-blocks out of specifyTypesForConstantBinaryExpression as we no longer check for a constant type operand, but do a more general isInteger based handling, which includes unbound IntegerRangeType.

!$context->null()
&& $exprNode instanceof FuncCall
&& count($exprNode->getArgs()) >= 1
&& $exprNode->name instanceof Name
&& in_array(strtolower((string) $exprNode->name), ['count', 'sizeof'], true)
&& $constantType instanceof ConstantIntegerType
) {
if ($constantType->getValue() < 0) {
return $this->create($exprNode->getArgs()[0]->value, new NeverType(), $context, false, $scope, $rootExpr);
}

$argType = $scope->getType($exprNode->getArgs()[0]->value);

if ($argType instanceof UnionType) {
$narrowed = $this->narrowUnionByArraySize($exprNode, $argType, $constantType, $context, $scope, $rootExpr);
if ($narrowed !== null) {
return $narrowed;
}
}

if ($context->truthy() || $constantType->getValue() === 0) {
$newContext = $context;
if ($constantType->getValue() === 0) {
$newContext = $newContext->negate();
}

if ($argType->isArray()->yes()) {
if (
$context->truthy()
&& $argType->isConstantArray()->yes()
&& $constantType->isSuperTypeOf($argType->getArraySize())->no()
) {
return $this->create($exprNode->getArgs()[0]->value, new NeverType(), $context, false, $scope, $rootExpr);
}

$funcTypes = $this->create($exprNode, $constantType, $context, false, $scope, $rootExpr);
$constArray = $this->turnListIntoConstantArray($exprNode, $argType, $constantType, $scope);
if ($context->truthy() && $constArray !== null) {
$valueTypes = $this->create($exprNode->getArgs()[0]->value, $constArray, $context, false, $scope, $rootExpr);
} else {
$valueTypes = $this->create($exprNode->getArgs()[0]->value, new NonEmptyArrayType(), $newContext, false, $scope, $rootExpr);
}
return $funcTypes->unionWith($valueTypes);
}
}
}

if (
!$context->null()
&& $exprNode instanceof FuncCall
&& count($exprNode->getArgs()) === 1
&& $exprNode->name instanceof Name
&& in_array(strtolower((string) $exprNode->name), ['strlen', 'mb_strlen'], true)
&& $constantType instanceof ConstantIntegerType
) {
if ($constantType->getValue() < 0) {
return $this->create($exprNode->getArgs()[0]->value, new NeverType(), $context, false, $scope, $rootExpr);
}

if ($context->truthy() || $constantType->getValue() === 0) {
$newContext = $context;
if ($constantType->getValue() === 0) {
$newContext = $newContext->negate();
}
$argType = $scope->getType($exprNode->getArgs()[0]->value);
if ($argType->isString()->yes()) {
$funcTypes = $this->create($exprNode, $constantType, $context, false, $scope, $rootExpr);

$accessory = new AccessoryNonEmptyStringType();
if ($constantType->getValue() >= 2) {
$accessory = new AccessoryNonFalsyStringType();
}
$valueTypes = $this->create($exprNode->getArgs()[0]->value, $accessory, $newContext, false, $scope, $rootExpr);

return $funcTypes->unionWith($valueTypes);
}
}

}

return null;
}

private function specifyTypesForConstantStringBinaryExpression(
Expr $exprNode,
Type $constantType,
ConstantStringType $constantType,
TypeSpecifierContext $context,
Scope $scope,
?Expr $rootExpr,
): ?SpecifiedTypes
{
$scalarValues = $constantType->getConstantScalarValues();
if (count($scalarValues) !== 1 || !is_string($scalarValues[0])) {
return null;
}
$constantStringValue = $scalarValues[0];
$constantStringValue = $constantType->getValue();

if (
$context->truthy()
Expand Down Expand Up @@ -2165,14 +2081,111 @@ public function resolveIdentical(Expr\BinaryOp\Identical $expr, Scope $scope, Ty
}
}

if ($rightType->isInteger()->yes() || $rightType->isString()->yes()) {
$types = null;
foreach ($rightType->getFiniteTypes() as $finiteType) {
if ($finiteType->isString()->yes()) {
$specifiedType = $this->specifyTypesForConstantStringBinaryExpression($unwrappedLeftExpr, $finiteType, $context, $scope, $rootExpr);
if (
!$context->null()
&& $unwrappedLeftExpr instanceof FuncCall
&& count($unwrappedLeftExpr->getArgs()) >= 1
&& $unwrappedLeftExpr->name instanceof Name
&& in_array(strtolower((string) $unwrappedLeftExpr->name), ['count', 'sizeof'], true)
&& $rightType->isInteger()->yes()
) {
if (IntegerRangeType::fromInterval(null, -1)->isSuperTypeOf($rightType)->yes()) {
return $this->create($unwrappedLeftExpr->getArgs()[0]->value, new NeverType(), $context, false, $scope, $rootExpr);
}

$argType = $scope->getType($unwrappedLeftExpr->getArgs()[0]->value);
$isZero = (new ConstantIntegerType(0))->isSuperTypeOf($rightType);
if ($isZero->yes()) {
$funcTypes = $this->create($unwrappedLeftExpr, $rightType, $context, false, $scope, $rootExpr);

if ($context->truthy() && !$argType->isArray()->yes()) {
$newArgType = new UnionType([
new ObjectType(Countable::class),
new ConstantArrayType([], []),
]);
} else {
$specifiedType = $this->specifyTypesForConstantBinaryExpression($unwrappedLeftExpr, $finiteType, $context, $scope, $rootExpr);
$newArgType = new ConstantArrayType([], []);
}

return $funcTypes->unionWith(
$this->create($unwrappedLeftExpr->getArgs()[0]->value, $newArgType, $context, false, $scope, $rootExpr),
);
}

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

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, false, $scope, $rootExpr);
}

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

return $funcTypes;
}
}
}

if (
!$context->null()
&& $unwrappedLeftExpr instanceof FuncCall
&& count($unwrappedLeftExpr->getArgs()) === 1
&& $unwrappedLeftExpr->name instanceof Name
&& in_array(strtolower((string) $unwrappedLeftExpr->name), ['strlen', 'mb_strlen'], true)
&& $rightType->isInteger()->yes()
) {
if (IntegerRangeType::fromInterval(null, -1)->isSuperTypeOf($rightType)->yes()) {
return $this->create($unwrappedLeftExpr->getArgs()[0]->value, new NeverType(), $context, false, $scope, $rootExpr);
}

$isZero = (new ConstantIntegerType(0))->isSuperTypeOf($rightType);
if ($isZero->yes()) {
$funcTypes = $this->create($unwrappedLeftExpr, $rightType, $context, false, $scope, $rootExpr);
return $funcTypes->unionWith(
$this->create($unwrappedLeftExpr->getArgs()[0]->value, new ConstantStringType(''), $context, false, $scope, $rootExpr),
);
}

if ($context->truthy() && IntegerRangeType::fromInterval(1, null)->isSuperTypeOf($rightType)->yes()) {
$argType = $scope->getType($unwrappedLeftExpr->getArgs()[0]->value);
if ($argType->isString()->yes()) {
$funcTypes = $this->create($unwrappedLeftExpr, $rightType, $context, false, $scope, $rootExpr);

$accessory = new AccessoryNonEmptyStringType();
if (IntegerRangeType::fromInterval(2, null)->isSuperTypeOf($rightType)->yes()) {
$accessory = new AccessoryNonFalsyStringType();
}
$valueTypes = $this->create($unwrappedLeftExpr->getArgs()[0]->value, $accessory, $context, false, $scope, $rootExpr);

return $funcTypes->unionWith($valueTypes);
}
}
}

if ($rightType->isString()->yes()) {
$types = null;
foreach ($rightType->getConstantStrings() as $constantString) {
$specifiedType = $this->specifyTypesForConstantStringBinaryExpression($unwrappedLeftExpr, $constantString, $context, $scope, $rootExpr);

if ($specifiedType === null) {
continue;
}
Expand Down
2 changes: 1 addition & 1 deletion tests/PHPStan/Analyser/TypeSpecifierTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1283,7 +1283,7 @@ public function dataCondition(): iterable
),
),
[
'$foo' => 'non-empty-string',
'$foo' => "string & ~''",
'strlen($foo)' => '~0',
],
[
Expand Down
2 changes: 1 addition & 1 deletion tests/PHPStan/Analyser/nsrt/bug-3993.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ public function doFoo($arguments)
return;
}

assertType('mixed~null', $arguments);
assertType('mixed~array{}|null', $arguments);

array_shift($arguments);

Expand Down
25 changes: 23 additions & 2 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 @@ -44,12 +44,12 @@ public function doFooBar(
if (count($arr) == $maybeZero) {
assertType('array', $arr);
} else {
assertType('non-empty-array', $arr);
assertType('array', $arr);
Copy link
Contributor Author

@staabm staabm Aug 30, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I think I did a mistake here in previous PR, as the array can still be empty in the else branch

https://3v4l.org/409Jp

}
if (count($arr) === $maybeZero) {
assertType('array', $arr);
} else {
assertType('non-empty-array', $arr);
assertType('array', $arr);
}

if (count($arr) == $negative) {
Expand All @@ -65,3 +65,24 @@ public function doFooBar(
}

}

/**
* @param \ArrayObject<int, mixed> $obj
*/
function(\ArrayObject $obj): void {
if (count($obj) === 0) {
assertType('ArrayObject', $obj);
return;
}

assertType('ArrayObject', $obj);
};

function($mixed): void {
if (count($mixed) === 0) {
assertType('array{}|Countable', $mixed);
return;
}

assertType('mixed~array{}', $mixed);
};
32 changes: 30 additions & 2 deletions tests/PHPStan/Analyser/nsrt/strlen-int-range.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,10 +69,10 @@ function doFoo(string $s, $zeroToThree, $twoOrThree, $twoOrMore, int $maxThree,
}

if (strlen($s) == $oneOrMore) {
assertType('string', $s); // could be non-empty-string
assertType('non-empty-string', $s);
}
if (strlen($s) === $oneOrMore) {
assertType('string', $s); // could be non-empty-string
assertType('non-empty-string', $s);
}

if (strlen($s) == $tenOrEleven) {
Expand Down Expand Up @@ -113,3 +113,31 @@ function doFoo(string $s, $zeroToThree, $twoOrThree, $twoOrMore, int $maxThree,
assertType('string', $s);
}
}

/**
* @param int<1, max> $oneOrMore
* @param int<2, max> $twoOrMore
*/
function doFooBar(string $s, array $arr, int $oneOrMore, int $twoOrMore): void
{
if (strlen($s) == $oneOrMore) {
assertType('non-empty-string', $s);
}
if (strlen($s) === $oneOrMore) {
assertType('non-empty-string', $s);
}

if (strlen($s) == $twoOrMore) {
assertType('non-falsy-string', $s);
}
if (strlen($s) === $twoOrMore) {
assertType('non-falsy-string', $s);
}

if (count($arr) == $oneOrMore) {
assertType('non-empty-array', $arr);
}
if (count($arr) === $oneOrMore) {
assertType('non-empty-array', $arr);
}
}
Loading