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

Jump to bottom

phpstan-assert with recursive count() results in type loss #2812

Merged
merged 1 commit into from
Jun 4, 2024
Merged

phpstan-assert with recursive count() results in type loss #2812

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
59 changes: 55 additions & 4 deletions src/Analyser/TypeSpecifier.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace PHPStan\Analyser;

use Countable;
use PhpParser\Node;
use PhpParser\Node\Expr;
use PhpParser\Node\Expr\ArrayDimFetch;
Expand Down Expand Up @@ -79,6 +80,7 @@
use function is_string;
use function strtolower;
use function substr;
use const COUNT_NORMAL;

class TypeSpecifier
{
Expand Down Expand Up @@ -208,7 +210,7 @@ public function specifyTypesInCondition(

if (
$expr->left instanceof FuncCall
&& count($expr->left->getArgs()) === 1
&& count($expr->left->getArgs()) >= 1
&& $expr->left->name instanceof Name
&& in_array(strtolower((string) $expr->left->name), ['count', 'sizeof', 'strlen', 'mb_strlen'], true)
&& (
Expand Down Expand Up @@ -237,7 +239,7 @@ public function specifyTypesInCondition(
if (
!$context->null()
&& $expr->right instanceof FuncCall
&& count($expr->right->getArgs()) === 1
&& count($expr->right->getArgs()) >= 1
staabm marked this conversation as resolved.
Show resolved Hide resolved
&& $expr->right->name instanceof Name
&& in_array(strtolower((string) $expr->right->name), ['count', 'sizeof'], true)
&& $leftType->isInteger()->yes()
Expand All @@ -247,6 +249,39 @@ public function specifyTypesInCondition(
|| ($context->false() && (new ConstantIntegerType(1 - $offset))->isSuperTypeOf($leftType)->yes())
) {
$argType = $scope->getType($expr->right->getArgs()[0]->value);

if ($context->truthy() && $argType->isArray()->maybe()) {
$countables = [];
if ($argType instanceof UnionType) {
$countableInterface = new ObjectType(Countable::class);
foreach ($argType->getTypes() as $innerType) {
if (
$innerType->isArray()->yes()
) {
$innerType = TypeCombinator::intersect(new NonEmptyArrayType(), $innerType);
if ($innerType->isList()->yes()) {
$innerType = AccessoryArrayListType::intersectWith($innerType);
}
$countables[] = $innerType;
}

if (
!$countableInterface->isSuperTypeOf($innerType)->yes()
) {
continue;
}

$countables[] = $innerType;
}
}

if (count($countables) > 0) {
$countableType = TypeCombinator::union(...$countables);

return $this->create($expr->right->getArgs()[0]->value, $countableType, $context, false, $scope, $rootExpr);
}
}

if ($argType->isArray()->yes()) {
$newType = new NonEmptyArrayType();
if ($context->true() && $argType->isList()->yes()) {
Expand Down Expand Up @@ -944,7 +979,7 @@ private function specifyTypesForConstantBinaryExpression(
if (
!$context->null()
&& $exprNode instanceof FuncCall
&& count($exprNode->getArgs()) === 1
&& count($exprNode->getArgs()) >= 1
&& $exprNode->name instanceof Name
&& in_array(strtolower((string) $exprNode->name), ['count', 'sizeof'], true)
&& $constantType instanceof ConstantIntegerType
Expand All @@ -954,10 +989,26 @@ private function specifyTypesForConstantBinaryExpression(
if ($constantType->getValue() === 0) {
$newContext = $newContext->negate();
}

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

if ($argType->isArray()->yes()) {
if (count($exprNode->getArgs()) === 1) {
$isNormalCount = true;
} else {
$mode = $scope->getType($exprNode->getArgs()[1]->value);
if (!$mode->isInteger()->yes()) {
return new SpecifiedTypes();
}

$isNormalCount = (new ConstantIntegerType(COUNT_NORMAL))->isSuperTypeOf($mode)->yes();
if (!$isNormalCount) {
$isNormalCount = $argType->getIterableValueType()->isArray()->no();
Copy link
Member

Choose a reason for hiding this comment

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

I think this still misses some scenarios.

We should have different results for $isNormalCount yes/maybe/no.
And also different results for $argType->getIterableValueType()->isArray() yes/maybe/no.

If you create a matrix:

no + no
no + maybe
no + yes
maybe + no
maybe + maybe
maybe + yes
yes + no
yes + maybe
yes + yes

You'll see that some scenarios aren't covered correctly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks, I added tests covering this testmatrix in

tests/PHPStan/Analyser/data/count-maybe.php

}
}

$funcTypes = $this->create($exprNode, $constantType, $context, false, $scope, $rootExpr);
if ($argType->isList()->yes() && $context->truthy() && $constantType->getValue() < ConstantArrayTypeBuilder::ARRAY_COUNT_LIMIT) {
if ($isNormalCount && $argType->isList()->yes() && $context->truthy() && $constantType->getValue() < ConstantArrayTypeBuilder::ARRAY_COUNT_LIMIT) {
$valueTypesBuilder = ConstantArrayTypeBuilder::createEmpty();
$itemType = $argType->getIterableValueType();
for ($i = 0; $i < $constantType->getValue(); $i++) {
Expand Down
1 change: 1 addition & 0 deletions tests/PHPStan/Analyser/NodeScopeResolverTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,7 @@ public function dataFileAsserts(): iterable
if (PHP_VERSION_ID >= 80000) {
yield from $this->gatherAssertTypes(__DIR__ . '/data/minmax-php8.php');
}
yield from $this->gatherAssertTypes(__DIR__ . '/data/count-maybe.php');
yield from $this->gatherAssertTypes(__DIR__ . '/data/classPhpDocs.php');
yield from $this->gatherAssertTypes(__DIR__ . '/data/non-empty-array-key-type.php');
yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-3133.php');
Expand Down
49 changes: 48 additions & 1 deletion tests/PHPStan/Analyser/data/bug-10264.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,64 @@ function doFoo() {
assertType('list<Bug10264\A>', $list);
}

function doFoo2() {
/** @var list<A> $list */
$list = [];

assertType('list<Bug10264\A>', $list);

assert((count($list, COUNT_NORMAL) <= 1) === true);
assertType('list<Bug10264\A>', $list);
}

/** @param list<int> $c */
public function sayHello(array $c): void
{
assertType('list<int>', $c);
if (count($c) > 0) {
$c = array_map(fn () => new stdClass(), $c);
$c = array_map(fn() => new stdClass(), $c);
assertType('non-empty-list<stdClass>', $c);
} else {
assertType('array{}', $c);
}

assertType('list<stdClass>', $c);
}

function doBar() {
/** @var list<A> $list */
$list = [];

assertType('list<Bug10264\A>', $list);

assert((count($list, COUNT_RECURSIVE) <= 1) === true);
assertType('list<Bug10264\A>', $list);
}

function doIf():void {
/** @var list<A> $list */
$list = [];

assertType('list<Bug10264\A>', $list);

if( count($list, COUNT_RECURSIVE) >= 1) {
assertType('non-empty-list<Bug10264\A>', $list);
} else {
assertType('array{}', $list);
}
}

function countModeInt(int $i):void {
/** @var list<A> $list */
$list = [];

assertType('list<Bug10264\A>', $list);

if( count($list, $i) >= 1) {
assertType('non-empty-list<Bug10264\A>', $list);
} else {
assertType('array{}', $list);
}
}

}
192 changes: 192 additions & 0 deletions tests/PHPStan/Analyser/data/count-maybe.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,192 @@
<?php

namespace CountMaybe;

use Countable;
use function PHPStan\Testing\assertType;

function doBar1(float $notCountable, int $mode): void
{
if (count($notCountable, $mode) > 0) {
assertType('float', $notCountable);
} else {
assertType('float', $notCountable);
}
assertType('float', $notCountable);
}

/**
* @param array|int $maybeMode
*/
function doBar2(float $notCountable, $maybeMode): void
{
if (count($notCountable, $maybeMode) > 0) {
assertType('float', $notCountable);
} else {
assertType('float', $notCountable);
}
assertType('float', $notCountable);
}

function doBar3(float $notCountable, float $invalidMode): void
{
if (count($notCountable, $invalidMode) > 0) {
assertType('float', $notCountable);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

should we resolve to a *ERROR* on invalid mode? I was not sure, as we already have a error at the count level because of the invalid argument type...?

} else {
assertType('float', $notCountable);
}
assertType('float', $notCountable);
}

/**
* @param float|int[] $maybeCountable
*/
function doFoo1($maybeCountable, int $mode): void
{
if (count($maybeCountable, $mode) > 0) {
assertType('non-empty-array<int>', $maybeCountable);
} else {
assertType('array<int>|float', $maybeCountable);
}
assertType('array<int>|float', $maybeCountable);
}

/**
* @param float|int[] $maybeCountable
* @param array|int $maybeMode
*/
function doFoo2($maybeCountable, $maybeMode): void
{
if (count($maybeCountable, $maybeMode) > 0) {
assertType('non-empty-array<int>', $maybeCountable);
} else {
assertType('array<int>|float', $maybeCountable);
}
assertType('array<int>|float', $maybeCountable);
}

/**
* @param float|int[] $maybeCountable
*/
function doFoo3($maybeCountable, float $invalidMode): void
{
if (count($maybeCountable, $invalidMode) > 0) {
assertType('non-empty-array<int>', $maybeCountable);
} else {
assertType('array<int>|float', $maybeCountable);
}
assertType('array<int>|float', $maybeCountable);
}

/**
* @param float|list<int> $maybeCountable
*/
function doFoo4($maybeCountable, int $mode): void
{
if (count($maybeCountable, $mode) > 0) {
assertType('non-empty-list<int>', $maybeCountable);
} else {
assertType('list<int>|float', $maybeCountable);
}
assertType('list<int>|float', $maybeCountable);
}

/**
* @param float|list<int> $maybeCountable
* @param array|int $maybeMode
*/
function doFoo5($maybeCountable, $maybeMode): void
{
if (count($maybeCountable, $maybeMode) > 0) {
assertType('non-empty-list<int>', $maybeCountable);
} else {
assertType('list<int>|float', $maybeCountable);
}
assertType('list<int>|float', $maybeCountable);
}

/**
* @param float|list<int> $maybeCountable
*/
function doFoo6($maybeCountable, float $invalidMode): void
{
if (count($maybeCountable, $invalidMode) > 0) {
assertType('non-empty-list<int>', $maybeCountable);
} else {
assertType('list<int>|float', $maybeCountable);
}
assertType('list<int>|float', $maybeCountable);
}

/**
* @param float|list<int>|Countable $maybeCountable
*/
function doFoo7($maybeCountable, int $mode): void
{
if (count($maybeCountable, $mode) > 0) {
assertType('non-empty-list<int>|Countable', $maybeCountable);
} else {
assertType('list<int>|Countable|float', $maybeCountable);
}
assertType('list<int>|Countable|float', $maybeCountable);
}

/**
* @param float|list<int>|Countable $maybeCountable
* @param array|int $maybeMode
*/
function doFoo8($maybeCountable, $maybeMode): void
{
if (count($maybeCountable, $maybeMode) > 0) {
assertType('non-empty-list<int>|Countable', $maybeCountable);
} else {
assertType('list<int>|Countable|float', $maybeCountable);
}
assertType('list<int>|Countable|float', $maybeCountable);
}

/**
* @param float|list<int>|Countable $maybeCountable
*/
function doFoo9($maybeCountable, float $invalidMode): void
{
if (count($maybeCountable, $invalidMode) > 0) {
assertType('non-empty-list<int>|Countable', $maybeCountable);
} else {
assertType('list<int>|Countable|float', $maybeCountable);
}
assertType('list<int>|Countable|float', $maybeCountable);
}

function doFooBar1(array $countable, int $mode): void
{
if (count($countable, $mode) > 0) {
assertType('non-empty-array', $countable);
} else {
assertType('array{}', $countable);
}
assertType('array', $countable);
}

/**
* @param array|int $maybeMode
*/
function doFooBar2(array $countable, $maybeMode): void
{
if (count($countable, $maybeMode) > 0) {
assertType('non-empty-array', $countable);
} else {
assertType('array{}', $countable);
}
assertType('array', $countable);
}

function doFooBar3(array $countable, float $invalidMode): void
{
if (count($countable, $invalidMode) > 0) {
assertType('non-empty-array', $countable);
} else {
assertType('array{}', $countable);
}
assertType('array', $countable);
}
Loading
Loading