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

Use isArray, isConstantArray instead of instanceof in TypeCombinartor::union #2118

Merged
merged 9 commits into from Dec 19, 2022
Merged

Use isArray, isConstantArray instead of instanceof in TypeCombinartor::union #2118

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
46ee018
move IterableType union logic to compareTypesInUnion
rajyan Dec 13, 2022
3558d3d
AccessoryTypes themselves are not isArray->yes
rajyan Dec 16, 2022
3e94bd8
refactor union of array types by using isArray, isConstantArray
rajyan Dec 16, 2022
e734fb4
cs-fix
rajyan Dec 16, 2022
116a2be
Revert "AccessoryTypes themselves are not isArray->yes"
rajyan Dec 17, 2022
7581b9e
no need to union AccessoryTypes because classified by `$innerType->de…
rajyan Dec 17, 2022
b4719b8
micro optimization
rajyan Dec 17, 2022
b611f52
add failing tests for wrong union result type
rajyan Dec 18, 2022
d0626fb
fix to use isConstantArray in `reduceArrays` too
rajyan Dec 19, 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
208 changes: 94 additions & 114 deletions src/Type/TypeCombinator.php
View file
Open in desktop
Expand Up @@ -20,9 +20,7 @@
use PHPStan\Type\Generic\TemplateType;
use PHPStan\Type\Generic\TemplateTypeFactory;
use PHPStan\Type\Generic\TemplateUnionType;
use function array_intersect_key;
use function array_key_exists;
use function array_keys;
use function array_map;
use function array_merge;
use function array_slice;
Expand Down Expand Up @@ -143,7 +141,6 @@ public static function union(Type ...$types): Type
}

$arrayTypes = [];
$arrayAccessoryTypes = [];
$scalarTypes = [];
$hasGenericScalarTypes = [];
for ($i = 0; $i < $typesCount; $i++) {
Expand All @@ -165,106 +162,26 @@ public static function union(Type ...$types): Type
if ($types[$i] instanceof StringType && !$types[$i] instanceof ClassStringType) {
$hasGenericScalarTypes[ConstantStringType::class] = true;
}
if ($types[$i] instanceof IntersectionType) {
$intermediateArrayType = null;
$intermediateAccessoryTypes = [];
foreach ($types[$i]->getTypes() as $innerType) {
if ($innerType instanceof TemplateType) {
continue 2;
}
if ($innerType instanceof ArrayType) {
$intermediateArrayType = $innerType;
continue;
}
if ($innerType instanceof AccessoryType || $innerType instanceof CallableType) {
if ($innerType instanceof HasOffsetValueType) {
$intermediateAccessoryTypes[sprintf('hasOffsetValue(%s)', $innerType->getOffsetType()->describe(VerbosityLevel::cache()))][] = $innerType;
continue;
}

$intermediateAccessoryTypes[$innerType->describe(VerbosityLevel::cache())][] = $innerType;
continue;
}
}

if ($intermediateArrayType !== null) {
$arrayTypes[] = $intermediateArrayType;
$arrayAccessoryTypes[] = $intermediateAccessoryTypes;
unset($types[$i]);
continue;
}
}
if (!$types[$i] instanceof ArrayType) {
if (!$types[$i]->isArray()->yes()) {
continue;
}

$arrayTypes[] = $types[$i];

$intermediateAccessoryTypes = [];
if ($types[$i]->isIterableAtLeastOnce()->yes()) {
$nonEmpty = new NonEmptyArrayType();
$intermediateAccessoryTypes[$nonEmpty->describe(VerbosityLevel::cache())] = [$nonEmpty];
}
if ($types[$i]->isList()->yes() && AccessoryArrayListType::isListTypeEnabled()) {
$list = new AccessoryArrayListType();
$intermediateAccessoryTypes[$list->describe(VerbosityLevel::cache())] = [$list];
}
$arrayAccessoryTypes[] = $intermediateAccessoryTypes;
unset($types[$i]);
}

foreach ($scalarTypes as $classType => $scalarTypeItems) {
$scalarTypes[$classType] = array_values($scalarTypeItems);
}

/** @var ArrayType[] $arrayTypes */
$arrayTypes = $arrayTypes;

$commonArrayAccessoryTypesKeys = [];
if (count($arrayAccessoryTypes) > 1) {
$commonArrayAccessoryTypesKeys = array_keys(array_intersect_key(...$arrayAccessoryTypes));
} elseif (count($arrayAccessoryTypes) > 0) {
$commonArrayAccessoryTypesKeys = array_keys($arrayAccessoryTypes[0]);
}

$arrayAccessoryTypesToProcess = [];
foreach ($commonArrayAccessoryTypesKeys as $commonKey) {
$typesToUnion = [];
foreach ($arrayAccessoryTypes as $array) {
foreach ($array[$commonKey] as $arrayAccessoryType) {
$typesToUnion[] = $arrayAccessoryType;
}
}
$arrayAccessoryTypesToProcess[] = self::union(...$typesToUnion);
}
Comment on lines -221 to -239
Copy link
Contributor Author

@rajyan rajyan Dec 19, 2022 •
edited

Choose a reason for hiding this comment

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

this logic is replaced by count($arrayType) === count($accessoryType[$key]) in
https://github.com/phpstan/phpstan-src/pull/2118/files#diff-fef252fe8071200a31b3d282d1442135d62297098c9b20fd731f3888398ee973R549-R562

also, there is no need to union accessoryTypes except HasOffsetValueType, because the types are all the same.


$types = array_values(
array_merge(
$types,
self::processArrayTypes($arrayTypes, $arrayAccessoryTypesToProcess),
self::processArrayTypes($arrayTypes),
),
);
$typesCount = count($types);

// simplify string[] | int[] to (string|int)[]
for ($i = 0; $i < $typesCount; $i++) {
if (! $types[$i] instanceof IterableType) {
continue;
}

for ($j = $i + 1; $j < $typesCount; $j++) {
if ($types[$j] instanceof IterableType) {
$types[$i] = new IterableType(
self::union($types[$i]->getIterableKeyType(), $types[$j]->getIterableKeyType()),
self::union($types[$i]->getIterableValueType(), $types[$j]->getIterableValueType()),
);
array_splice($types, $j, 1);
$typesCount--;
continue 2;
}
}
}

foreach ($scalarTypes as $classType => $scalarTypeItems) {
if (isset($hasGenericScalarTypes[$classType])) {
unset($scalarTypes[$classType]);
Expand Down Expand Up @@ -405,6 +322,17 @@ private static function compareTypesInUnion(Type $a, Type $b): ?array
return null;
}

// simplify string[] | int[] to (string|int)[]
if ($a instanceof IterableType && $b instanceof IterableType) {
return [
new IterableType(
self::union($a->getIterableKeyType(), $b->getIterableKeyType()),
self::union($a->getIterableValueType(), $b->getIterableValueType()),
),
null,
];
}

if ($a instanceof SubtractableType) {
$typeWithoutSubtractedTypeA = $a->getTypeWithoutSubtractedType();
if ($typeWithoutSubtractedTypeA instanceof MixedType && $b instanceof MixedType) {
Expand Down Expand Up @@ -572,33 +500,80 @@ private static function intersectWithSubtractedType(
}

/**
* @param ArrayType[] $arrayTypes
* @param Type[] $accessoryTypes
* @param Type[] $arrayTypes
* @return Type[]
*/
private static function processArrayTypes(array $arrayTypes, array $accessoryTypes): array
private static function processArrayAccessoryTypes(array $arrayTypes): array
{
foreach ($arrayTypes as $arrayType) {
if (!$arrayType instanceof ConstantArrayType) {
continue;
$accessoryTypes = [];
foreach ($arrayTypes as $i => $arrayType) {
if ($arrayType instanceof IntersectionType) {
foreach ($arrayType->getTypes() as $innerType) {
if ($innerType instanceof TemplateType) {
break;
}
if (!($innerType instanceof AccessoryType) && !($innerType instanceof CallableType)) {
continue;
}
if ($innerType instanceof HasOffsetValueType) {
$accessoryTypes[sprintf('hasOffsetValue(%s)', $innerType->getOffsetType()->describe(VerbosityLevel::cache()))][$i] = $innerType;
continue;
}

$accessoryTypes[$innerType->describe(VerbosityLevel::cache())][$i] = $innerType;
}
}
if (count($arrayType->getKeyTypes()) > 0) {

if (!$arrayType->isConstantArray()->yes()) {
continue;
}
$constantArrays = $arrayType->getConstantArrays();

foreach ($constantArrays as $constantArray) {
if ($constantArray->isList()->yes() && AccessoryArrayListType::isListTypeEnabled()) {
$list = new AccessoryArrayListType();
$accessoryTypes[$list->describe(VerbosityLevel::cache())][$i] = $list;
}

foreach ($accessoryTypes as $i => $accessoryType) {
if (!$accessoryType instanceof NonEmptyArrayType) {
if (!$constantArray->isIterableAtLeastOnce()->yes()) {
continue;
}

unset($accessoryTypes[$i]);
break 2;
$nonEmpty = new NonEmptyArrayType();
$accessoryTypes[$nonEmpty->describe(VerbosityLevel::cache())][$i] = $nonEmpty;
}
}

$commonAccessoryTypes = [];
$arrayTypeCount = count($arrayTypes);
foreach ($accessoryTypes as $accessoryType) {
if (count($accessoryType) !== $arrayTypeCount) {
continue;
}

if ($accessoryType[0] instanceof HasOffsetValueType) {
$commonAccessoryTypes[] = self::union(...$accessoryType);
continue;
}

$commonAccessoryTypes[] = $accessoryType[0];
}

return $commonAccessoryTypes;
}

/**
* @param Type[] $arrayTypes
* @return Type[]
*/
private static function processArrayTypes(array $arrayTypes): array
{
if ($arrayTypes === []) {
return [];
}

$accessoryTypes = self::processArrayAccessoryTypes($arrayTypes);

if (count($arrayTypes) === 1) {
return [
self::intersect(...$arrayTypes, ...$accessoryTypes),
Expand All @@ -614,27 +589,32 @@ private static function processArrayTypes(array $arrayTypes, array $accessoryTyp
$nextConstantKeyTypeIndex = 1;

foreach ($arrayTypes as $arrayType) {
if (!$arrayType instanceof ConstantArrayType || $generalArrayOccurred) {
$keyTypesForGeneralArray[] = $arrayType->getKeyType();
$valueTypesForGeneralArray[] = $arrayType->getItemType();
$generalArrayOccurred = true;
if ($generalArrayOccurred || !$arrayType->isConstantArray()->yes()) {
foreach ($arrayType->getArrays() as $type) {
$keyTypesForGeneralArray[] = $type->getKeyType();
$valueTypesForGeneralArray[] = $type->getItemType();
$generalArrayOccurred = true;
}
continue;
}

foreach ($arrayType->getKeyTypes() as $i => $keyType) {
$keyTypesForGeneralArray[] = $keyType;
$valueTypesForGeneralArray[] = $arrayType->getValueTypes()[$i];
$constantArrays = $arrayType->getConstantArrays();
foreach ($constantArrays as $constantArray) {
foreach ($constantArray->getKeyTypes() as $i => $keyType) {
$keyTypesForGeneralArray[] = $keyType;
$valueTypesForGeneralArray[] = $constantArray->getValueTypes()[$i];

$keyTypeValue = $keyType->getValue();
if (array_key_exists($keyTypeValue, $constantKeyTypesNumbered)) {
continue;
}
$keyTypeValue = $keyType->getValue();
if (array_key_exists($keyTypeValue, $constantKeyTypesNumbered)) {
continue;
}

$constantKeyTypesNumbered[$keyTypeValue] = $nextConstantKeyTypeIndex;
$nextConstantKeyTypeIndex *= 2;
if (!is_int($nextConstantKeyTypeIndex)) {
$generalArrayOccurred = true;
continue;
$constantKeyTypesNumbered[$keyTypeValue] = $nextConstantKeyTypeIndex;
$nextConstantKeyTypeIndex *= 2;
if (!is_int($nextConstantKeyTypeIndex)) {
$generalArrayOccurred = true;
continue 2;
}
}
}
}
Expand Down Expand Up @@ -664,7 +644,7 @@ private static function reduceArrays(array $constantArrays): array
$arraysToProcess = [];
$emptyArray = null;
foreach ($constantArrays as $constantArray) {
if (!$constantArray instanceof ConstantArrayType) {
if (!$constantArray->isConstantArray()->yes()) {
$newArrays[] = $constantArray;
continue;
}
Expand All @@ -674,7 +654,7 @@ private static function reduceArrays(array $constantArrays): array
continue;
}

$arraysToProcess[] = $constantArray;
$arraysToProcess = array_merge($arraysToProcess, $constantArray->getConstantArrays());
}

if ($emptyArray !== null) {
Expand Down
1 change: 1 addition & 0 deletions tests/PHPStan/Analyser/NodeScopeResolverTest.php
View file
Open in desktop
Expand Up @@ -1143,6 +1143,7 @@ public function dataFileAsserts(): iterable
yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-4565.php');
yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-3789.php');
yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-8520.php');
yield from $this->gatherAssertTypes(__DIR__ . '/data/filter-var-dynamic-return-type-extension-regression.php');
}

/**
Expand Down
60 changes: 60 additions & 0 deletions tests/PHPStan/Analyser/data/filter-var-dynamic-return-type-extension-regression.php
View file
Open in desktop
@@ -0,0 +1,60 @@
<?php

namespace FilterVarDynamicReturnTypeExtensionRegression;

use PHPStan\Type\ConstantScalarType;
use PHPStan\Type\MixedType;
use PHPStan\Type\Type;
use PHPStan\Type\TypeCombinator;
use function PHPStan\Testing\assertType;

class Test {

/**
* @return array{default: Type, range?: Type}
*/
public function getOtherTypes()
{

}

public function determineExactType(): ?Type
{

}

public function test()
{

$exactType = $this->determineExactType();
$type = $exactType ?? new MixedType();
$otherTypes = $this->getOtherTypes();

assertType('array{default: PHPStan\Type\Type, range?: PHPStan\Type\Type}', $otherTypes);
if (isset($otherTypes['range'])) {
assertType('array{default: PHPStan\Type\Type, range: PHPStan\Type\Type}', $otherTypes);
if ($type instanceof ConstantScalarType) {
if ($otherTypes['range']->isSuperTypeOf($type)->no()) {
$type = $otherTypes['default'];
}
assertType('array{default: PHPStan\Type\Type, range: PHPStan\Type\Type}', $otherTypes);
unset($otherTypes['default']);
assertType('array{range: PHPStan\Type\Type}', $otherTypes);
} else {
$type = $otherTypes['range'];
assertType('array{default: PHPStan\Type\Type, range: PHPStan\Type\Type}', $otherTypes);
}
assertType('array{default?: PHPStan\Type\Type, range: PHPStan\Type\Type}', $otherTypes);
}
assertType('array{default?: PHPStan\Type\Type, range?: PHPStan\Type\Type}&non-empty-array', $otherTypes);
if ($exactType !== null) {
assertType('array{default?: PHPStan\Type\Type, range?: PHPStan\Type\Type}&non-empty-array', $otherTypes);
unset($otherTypes['default']);
assertType('array{range?: PHPStan\Type\Type}', $otherTypes);
}
assertType('array{default?: PHPStan\Type\Type, range?: PHPStan\Type\Type}', $otherTypes);
if (isset($otherTypes['default']) && $otherTypes['default']->isSuperTypeOf($type)->no()) {
$type = TypeCombinator::union($type, $otherTypes['default']);
}
}
}