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

Fix ArrayColumnFunctionReturnTypeExtension producing array key types #1033

Merged
merged 2 commits into from Mar 2, 2022
Merged

Fix ArrayColumnFunctionReturnTypeExtension producing array key types #1033

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7ace968
Improve ArrayType::castToArrayKeyType() for booleans
jlherren Feb 22, 2022
06dcd54
Fix ArrayColumnFunctionReturnTypeExtension producing array key types
jlherren Feb 22, 2022
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
6 changes: 5 additions & 1 deletion src/Type/ArrayType.php
View file
Open in desktop
Expand Up @@ -342,7 +342,11 @@ public static function castToArrayKeyType(Type $offsetType): Type
return $offsetType;
}

if ($offsetType instanceof FloatType || $offsetType instanceof BooleanType || $offsetType->isNumericString()->yes()) {
if ($offsetType instanceof BooleanType) {
return new UnionType([new ConstantIntegerType(0), new ConstantIntegerType(1)]);
}

if ($offsetType instanceof FloatType || $offsetType->isNumericString()->yes()) {
return new IntegerType();
}

Expand Down
20 changes: 19 additions & 1 deletion src/Type/Php/ArrayColumnFunctionReturnTypeExtension.php
View file
Open in desktop
Expand Up @@ -90,7 +90,7 @@ private function handleAnyArray(Type $arrayType, Type $columnType, ?Type $indexT
$returnKeyType = new IntegerType();
}

$returnType = new ArrayType($returnKeyType, $returnValueType);
$returnType = new ArrayType($this->castToArrayKeyType($returnKeyType), $returnValueType);

if ($iterableAtLeastOnce->yes()) {
$returnType = TypeCombinator::intersect($returnType, new NonEmptyArrayType());
Expand Down Expand Up @@ -125,6 +125,9 @@ private function handleConstantArray(ConstantArrayType $arrayType, Type $columnT
$keyType = null;
}

if ($keyType !== null) {
$keyType = $this->castToArrayKeyType($keyType);
}
$builder->setOffsetValueType($keyType, $valueType);
}

Expand Down Expand Up @@ -180,4 +183,19 @@ private function getOffsetOrProperty(Type $type, Type $offsetOrProperty, Scope $
return TypeCombinator::union(...$returnTypes);
}

private function castToArrayKeyType(Type $type): Type
{
$isArray = $type->isArray();
if ($isArray->yes()) {
return new IntegerType();
Copy link
Member

Choose a reason for hiding this comment

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

I don't get it. Array gets cast to integer?

Copy link

Choose a reason for hiding this comment

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

See https://sandbox.onlinephpfunctions.com/code/381d0182b85cc698b02b18b82cd9dae0da3f7a00
I intentionally used 'foo' instead of 'type' in the array_column call in this example. So this seems to be the behaviour.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes exactly. In PHP < 8, whenever a value cannot be used as an array key (either because it is an array, or the value is missing entirely), the next available integer array key is used (as if you'd do $arr[] = $value). In PHP >= 8, using an array value as key causes a fatal error, which of course is an opportunity for PHPStan to detect in the future.

Copy link
Member

Choose a reason for hiding this comment

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

FYI I miss an if for this behaviour in this extension. Feel free to add in a future PR :)

}
if ($isArray->no()) {
return ArrayType::castToArrayKeyType($type);
}
return TypeCombinator::union(
ArrayType::castToArrayKeyType(TypeCombinator::remove($type, new ArrayType(new MixedType(), new MixedType()))),
new IntegerType(),
);
}

}
1 change: 1 addition & 0 deletions tests/PHPStan/Analyser/NodeScopeResolverTest.php
View file
Open in desktop
Expand Up @@ -750,6 +750,7 @@ public function dataFileAsserts(): iterable

yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-6699.php');
yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-6715.php');
yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-6682.php');
}

/**
Expand Down
30 changes: 26 additions & 4 deletions tests/PHPStan/Analyser/data/array-column.php
View file
Open in desktop
Expand Up @@ -20,6 +20,17 @@ function testArrays(array $array): void
assertType('array{}', array_column($array, 'column'));
assertType('array{}', array_column($array, 'column', 'key'));
assertType('array{}', array_column($array, null, 'key'));

/** @var array<int, array<string, float>> $array */
assertType('array<int, float>', array_column($array, 'column', 'key'));
/** @var array<int, array<string, bool>> $array */
assertType('array<int, bool>', array_column($array, 'column', 'key'));
/** @var array<int, array<string, true>> $array */
assertType('array<int, true>', array_column($array, 'column', 'key'));
/** @var array<int, array<string, null>> $array */
assertType('array<\'\'|int, null>', array_column($array, 'column', 'key'));
/** @var array<int, array<string, array>> $array */
assertType('array<int, array>', array_column($array, 'column', 'key'));
}

function testConstantArrays(array $array): void
Expand Down Expand Up @@ -54,6 +65,17 @@ function testConstantArrays(array $array): void
assertType('non-empty-array<int, string>', array_column($array, 'column'));
assertType('non-empty-array<string, string>', array_column($array, 'column', 'key'));
assertType('non-empty-array<string, array{column: string, key: string}>', array_column($array, null, 'key'));

/** @var array<int, array{column: string, key: float}> $array */
assertType('array<int, string>', array_column($array, 'column', 'key'));
/** @var array<int, array{column: string, key: bool}> $array */
assertType('array<0|1, string>', array_column($array, 'column', 'key'));
/** @var array<int, array{column: string, key: true}> $array */
assertType('array<1, string>', array_column($array, 'column', 'key'));
/** @var array<int, array{column: string, key: null}> $array */
assertType('array<\'\', string>', array_column($array, 'column', 'key'));
/** @var array<int, array{column: string, key: array}> $array */
assertType('array<int, string>', array_column($array, 'column', 'key'));
}

function testImprecise(array $array): void {
Expand Down Expand Up @@ -84,17 +106,17 @@ function testObjects(array $array): void {
assertType('array<string, DOMElement>', array_column($array, null, 'tagName'));
assertType('array<int, mixed>', array_column($array, 'foo'));
assertType('array<string, mixed>', array_column($array, 'foo', 'tagName'));
assertType('array<string>', array_column($array, 'nodeName', 'foo'));
assertType('array<DOMElement>', array_column($array, null, 'foo'));
assertType('array<int|string, string>', array_column($array, 'nodeName', 'foo'));
assertType('array<int|string, DOMElement>', array_column($array, null, 'foo'));

/** @var non-empty-array<int, DOMElement> $array */
assertType('non-empty-array<int, string>', array_column($array, 'nodeName'));
assertType('non-empty-array<string, string>', array_column($array, 'nodeName', 'tagName'));
assertType('non-empty-array<string, DOMElement>', array_column($array, null, 'tagName'));
assertType('array<int, mixed>', array_column($array, 'foo'));
assertType('array<string, mixed>', array_column($array, 'foo', 'tagName'));
assertType('non-empty-array<string>', array_column($array, 'nodeName', 'foo'));
assertType('non-empty-array<DOMElement>', array_column($array, null, 'foo'));
assertType('non-empty-array<int|string, string>', array_column($array, 'nodeName', 'foo'));
assertType('non-empty-array<int|string, DOMElement>', array_column($array, null, 'foo'));

/** @var array{DOMElement} $array */
assertType('array{string}', array_column($array, 'nodeName'));
Expand Down
19 changes: 19 additions & 0 deletions tests/PHPStan/Analyser/data/bug-6682.php
View file
Open in desktop
@@ -0,0 +1,19 @@
<?php

declare(strict_types = 1);

namespace Bug6682;

use function PHPStan\Testing\assertType;

class Example
{
/**
* @param array<int, array<string, string|array<string,string>|null>> $data
*/
public function __construct(array $data)
{
$x = array_column($data, null, 'type');
assertType('array<int|string, array<string, array<string, string>|string|null>>', $x);
}
}
17 changes: 17 additions & 0 deletions tests/PHPStan/Analyser/data/constant-array-type-set.php
View file
Open in desktop
Expand Up @@ -97,4 +97,21 @@ public function doBar5(int $offset): void
assertType('non-empty-array<int<0, 4>, bool>', $a);
}

public function doBar6(bool $offset): void
{
$a = [false, false, false];
$a[$offset] = true;
assertType('array{bool, bool, false}', $a);
}

/**
* @param true $offset
*/
public function doBar7(bool $offset): void
{
$a = [false, false, false];
$a[$offset] = true;
assertType('array{false, true, false}', $a);
}

}