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.

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

Fix constant-string handling in union-types #2134

Merged
merged 11 commits into from Jan 2, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 8 additions & 0 deletions phpstan-baseline.neon
Expand Up @@ -15,6 +15,14 @@ parameters:
count: 1
path: src/Analyser/LazyInternalScopeFactory.php

-
message: """
#^Call to deprecated method getAnyArrays\\(\\) of class PHPStan\\\\Type\\\\TypeUtils\\:
Use PHPStan\\\\Type\\\\Type\\:\\:getArrays\\(\\) instead\\.$#
"""
count: 2
path: src/Analyser/MutatingScope.php

-
message: """
#^Call to deprecated method getTypeFromValue\\(\\) of class PHPStan\\\\Type\\\\ConstantTypeHelper\\:
Expand Down
42 changes: 19 additions & 23 deletions src/Analyser/MutatingScope.php
Expand Up @@ -4078,7 +4078,7 @@ public function processClosureScope(
$prevVariableType = $prevScope->getVariableType($variableName);
if (!$variableType->equals($prevVariableType)) {
$variableType = TypeCombinator::union($variableType, $prevVariableType);
$variableType = self::generalizeType($variableType, $prevVariableType);
$variableType = self::generalizeType($variableType, $prevVariableType, 0);
}
}

Expand Down Expand Up @@ -4200,15 +4200,15 @@ private function generalizeVariableTypeHolders(

$variableTypeHolders[$variableExprString] = new ExpressionTypeHolder(
$variableTypeHolder->getExpr(),
self::generalizeType($variableTypeHolder->getType(), $otherVariableTypeHolders[$variableExprString]->getType()),
self::generalizeType($variableTypeHolder->getType(), $otherVariableTypeHolders[$variableExprString]->getType(), 0),
Copy link
Contributor Author

@staabm staabm Dec 31, 2022

Choose a reason for hiding this comment

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

I printed the types after generalization at this line in this PR vs. 1.9.x-dev at 9128321

most types are identical. the only different are:

- string(71) "non-empty-array<literal-string&non-falsy-string, mixed>&oversized-array"
+ string(206) "non-empty-array<literal-string&non-falsy-string, non-empty-array<int|string, 0|(non-empty-array<int|string, 0|(non-empty-array<int|string, *ERROR*>&oversized-array)|float>&oversized-array)>>&oversized-array"

so after this PR we no longer get MixedType and therefore we get the errors

$variableTypeHolder->getCertainty(),
);
}

return $variableTypeHolders;
}

private static function generalizeType(Type $a, Type $b): Type
private static function generalizeType(Type $a, Type $b, int $depth): Type
{
if ($a->equals($b)) {
return $a;
Expand Down Expand Up @@ -4301,6 +4301,7 @@ private static function generalizeType(Type $a, Type $b): Type
self::generalizeType(
$constantArraysA->getOffsetValueType($keyType),
$constantArraysB->getOffsetValueType($keyType),
$depth + 1,
),
!$constantArraysA->hasOffsetValueType($keyType)->and($constantArraysB->hasOffsetValueType($keyType))->negate()->no(),
);
Expand All @@ -4309,8 +4310,8 @@ private static function generalizeType(Type $a, Type $b): Type
$resultTypes[] = $resultArrayBuilder->getArray();
} else {
$resultType = new ArrayType(
TypeCombinator::union(self::generalizeType($constantArraysA->getIterableKeyType(), $constantArraysB->getIterableKeyType())),
TypeCombinator::union(self::generalizeType($constantArraysA->getIterableValueType(), $constantArraysB->getIterableValueType())),
TypeCombinator::union(self::generalizeType($constantArraysA->getIterableKeyType(), $constantArraysB->getIterableKeyType(), $depth + 1)),
TypeCombinator::union(self::generalizeType($constantArraysA->getIterableValueType(), $constantArraysB->getIterableValueType(), $depth + 1)),
);
if ($constantArraysA->isIterableAtLeastOnce()->yes() && $constantArraysB->isIterableAtLeastOnce()->yes()) {
$resultType = TypeCombinator::intersect($resultType, new NonEmptyArrayType());
Expand All @@ -4334,16 +4335,14 @@ private static function generalizeType(Type $a, Type $b): Type

$aValueType = $generalArraysA->getIterableValueType();
$bValueType = $generalArraysB->getIterableValueType();
$aArrays = $aValueType->getArrays();
$bArrays = $bValueType->getArrays();
if (
count($aArrays) === 1
&& $aArrays[0]->isConstantArray()->no()
&& count($bArrays) === 1
&& $bArrays[0]->isConstantArray()->no()
$aValueType->isArray()->yes()
&& $aValueType->isConstantArray()->no()
&& $bValueType->isArray()->yes()
&& $bValueType->isConstantArray()->no()
) {
$aDepth = self::getArrayDepth($aArrays[0]);
$bDepth = self::getArrayDepth($bArrays[0]);
$aDepth = self::getArrayDepth($aValueType) + $depth;
$bDepth = self::getArrayDepth($bValueType) + $depth;
if (
($aDepth > 2 || $bDepth > 2)
&& abs($aDepth - $bDepth) > 0
Expand All @@ -4354,8 +4353,8 @@ private static function generalizeType(Type $a, Type $b): Type
}

$resultType = new ArrayType(
TypeCombinator::union(self::generalizeType($generalArraysA->getIterableKeyType(), $generalArraysB->getIterableKeyType())),
TypeCombinator::union(self::generalizeType($aValueType, $bValueType)),
TypeCombinator::union(self::generalizeType($generalArraysA->getIterableKeyType(), $generalArraysB->getIterableKeyType(), $depth + 1)),
TypeCombinator::union(self::generalizeType($aValueType, $bValueType, $depth + 1)),
);
if ($generalArraysA->isIterableAtLeastOnce()->yes() && $generalArraysB->isIterableAtLeastOnce()->yes()) {
$resultType = TypeCombinator::intersect($resultType, new NonEmptyArrayType());
Expand Down Expand Up @@ -4514,17 +4513,14 @@ private static function generalizeType(Type $a, Type $b): Type
);
}

private static function getArrayDepth(ArrayType $type): int
private static function getArrayDepth(Type $type): int
{
$depth = 0;
while ($type instanceof ArrayType) {
$arrays = TypeUtils::getAnyArrays($type);
while (count($arrays) > 0) {
$temp = $type->getIterableValueType();
$arrays = $temp->getArrays();
if (count($arrays) === 1) {
$type = $arrays[0];
} else {
$type = $temp;
}
$type = $temp;
$arrays = TypeUtils::getAnyArrays($type);
Copy link
Contributor Author

@staabm staabm Dec 31, 2022

Choose a reason for hiding this comment

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

I think our problem is that getAnyArrays does not return arrays for the types in question because we are working with a union of intersection types

grafik

and TypeUtils::getAnyArrays does not recursivly check the intersections within unions

$depth++;
}

Expand Down
39 changes: 32 additions & 7 deletions src/Type/IntersectionType.php
Expand Up @@ -93,27 +93,52 @@ public function inferTemplateTypesOn(Type $templateType): TemplateTypeMap
return $types;
}

/**
* @return string[]
*/
public function getReferencedClasses(): array
{
return UnionTypeHelper::getReferencedClasses($this->types);
$classes = [];
foreach ($this->types as $type) {
foreach ($type->getReferencedClasses() as $className) {
$classes[] = $className;
}
}

return $classes;
}

public function getArrays(): array
{
return UnionTypeHelper::getArrays($this->getTypes());
$arrays = [];
foreach ($this->types as $type) {
foreach ($type->getArrays() as $array) {
$arrays[] = $array;
}
}

return $arrays;
}

public function getConstantArrays(): array
{
return UnionTypeHelper::getConstantArrays($this->getTypes());
$constantArrays = [];
foreach ($this->types as $type) {
foreach ($type->getConstantArrays() as $constantArray) {
$constantArrays[] = $constantArray;
}
}

return $constantArrays;
}

public function getConstantStrings(): array
{
return UnionTypeHelper::getConstantStrings($this->getTypes());
$strings = [];
foreach ($this->types as $type) {
foreach ($type->getConstantStrings() as $string) {
$strings[] = $string;
}
}

return $strings;
}

public function accepts(Type $otherType, bool $strictTypes): TrinaryLogic
Expand Down
8 changes: 6 additions & 2 deletions src/Type/TypeUtils.php
Expand Up @@ -218,15 +218,19 @@ private static function map(
if ($type instanceof UnionType) {
$matchingTypes = [];
foreach ($type->getTypes() as $innerType) {
if (!$innerType instanceof $typeClass) {
$matchingInner = self::map($typeClass, $innerType, $inspectIntersections, $stopOnUnmatched);

if ($matchingInner === []) {
if ($stopOnUnmatched) {
return [];
}

continue;
}

$matchingTypes[] = $innerType;
foreach ($matchingInner as $innerMapped) {
$matchingTypes[] = $innerMapped;
}
}

return $matchingTypes;
Expand Down
53 changes: 49 additions & 4 deletions src/Type/UnionType.php
Expand Up @@ -102,22 +102,67 @@ private function getSortedTypes(): array
*/
public function getReferencedClasses(): array
{
return UnionTypeHelper::getReferencedClasses($this->getTypes());
$classes = [];
foreach ($this->types as $type) {
foreach ($type->getReferencedClasses() as $className) {
$classes[] = $className;
}
}

return $classes;
}

public function getArrays(): array
{
return UnionTypeHelper::getArrays($this->getTypes());
$arrays = [];
foreach ($this->types as $type) {
$innerTypeArrays = $type->getArrays();
if ($innerTypeArrays === []) {
return [];
}

foreach ($innerTypeArrays as $array) {
$arrays[] = $array;
staabm marked this conversation as resolved.
Show resolved Hide resolved
}
}

return $arrays;
}

public function getConstantArrays(): array
{
return UnionTypeHelper::getConstantArrays($this->getTypes());
$constantArrays = [];
foreach ($this->types as $type) {
$typeAsConstantArrays = $type->getConstantArrays();

if ($typeAsConstantArrays === []) {
return [];
}

foreach ($typeAsConstantArrays as $constantArray) {
$constantArrays[] = $constantArray;
}
}

return $constantArrays;
}

public function getConstantStrings(): array
{
return UnionTypeHelper::getConstantStrings($this->getTypes());
$strings = [];
foreach ($this->types as $type) {
$constantStrings = $type->getConstantStrings();

if ($constantStrings === []) {
return [];
}

foreach ($constantStrings as $string) {
$strings[] = $string;
}
}

return $strings;
}

public function accepts(Type $type, bool $strictTypes): TrinaryLogic
Expand Down
58 changes: 0 additions & 58 deletions src/Type/UnionTypeHelper.php
Expand Up @@ -3,12 +3,10 @@
namespace PHPStan\Type;

use PHPStan\Type\Accessory\AccessoryType;
use PHPStan\Type\Constant\ConstantArrayType;
use PHPStan\Type\Constant\ConstantBooleanType;
use PHPStan\Type\Constant\ConstantFloatType;
use PHPStan\Type\Constant\ConstantIntegerType;
use PHPStan\Type\Constant\ConstantStringType;
use function array_merge;
use function count;
use function strcasecmp;
use function usort;
Expand All @@ -17,62 +15,6 @@
class UnionTypeHelper
{

/**
* @param Type[] $types
* @return string[]
*/
public static function getReferencedClasses(array $types): array
{
$referencedClasses = [];
foreach ($types as $type) {
$referencedClasses[] = $type->getReferencedClasses();
}

return array_merge(...$referencedClasses);
}

/**
* @param Type[] $types
* @return list<ArrayType>
*/
public static function getArrays(array $types): array
{
$arrays = [];
foreach ($types as $type) {
$arrays[] = $type->getArrays();
}

return array_merge(...$arrays);
}

/**
* @param Type[] $types
* @return list<ConstantArrayType>
*/
public static function getConstantArrays(array $types): array
{
$constantArrays = [];
foreach ($types as $type) {
$constantArrays[] = $type->getConstantArrays();
}

return array_merge(...$constantArrays);
}

/**
* @param Type[] $types
* @return list<ConstantStringType>
*/
public static function getConstantStrings(array $types): array
{
$strings = [];
foreach ($types as $type) {
$strings[] = $type->getConstantStrings();
}

return array_merge(...$strings);
}

/**
* @param Type[] $types
* @return Type[]
Expand Down
1 change: 1 addition & 0 deletions tests/PHPStan/Analyser/NodeScopeResolverTest.php
Expand Up @@ -1154,6 +1154,7 @@ public function dataFileAsserts(): iterable
yield from $this->gatherAssertTypes(__DIR__ . '/data/pathinfo-php8.php');
}
yield from $this->gatherAssertTypes(__DIR__ . '/data/pathinfo.php');
yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-8568.php');
}

/**
Expand Down
26 changes: 26 additions & 0 deletions tests/PHPStan/Analyser/data/bug-8568.php
@@ -0,0 +1,26 @@
<?php declare(strict_types = 1);

namespace Bug8568;

use function PHPStan\Testing\assertType;

class HelloWorld
{
public function sayHello(): void
{
assertType('non-falsy-string', 'a' . $this->get());
}

public function get(): ?int
{
return rand() ? 5 : null;
}

/**
* @param numeric-string $numericS
*/
public function intersections($numericS): void {
assertType('non-falsy-string', 'a'. $numericS);
assertType('numeric-string', (string) $numericS);
}
}
Expand Up @@ -122,4 +122,10 @@ public function testBug8076(): void
$this->analyse([__DIR__ . '/data/bug-8076.php'], []);
}

public function testBug8562(): void
{
$this->treatPhpDocTypesAsCertain = true;
$this->analyse([__DIR__ . '/data/bug-8562.php'], []);
}

}