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

Improve performance again by dumbing down nested arrays #2077

Merged
merged 1 commit into from Dec 20, 2022
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
9 changes: 8 additions & 1 deletion src/Type/Constant/ConstantArrayType.php
Expand Up @@ -300,7 +300,14 @@ public function accepts(Type $type, bool $strictTypes): TrinaryLogic
$result = $result->and($acceptsValue);
}

return $result->and($type->isArray());
$result = $result->and($type->isArray());
if ($type->isOversizedArray()->yes()) {
if (!$result->no()) {
return TrinaryLogic::createYes();
}
}

return $result;
}

public function isSuperTypeOf(Type $type): TrinaryLogic
Expand Down
1 change: 0 additions & 1 deletion src/Type/Constant/OversizedArrayBuilder.php
Expand Up @@ -77,7 +77,6 @@ public function build(Array_ $expr, callable $getTypeCallback): Type
$isList = false;
$nextAutoIndex = $itemKeyType->getValue() + 1;
} else {
$itemKeyType = new ConstantIntegerType($nextAutoIndex);
$nextAutoIndex++;
}
} else {
Expand Down
88 changes: 86 additions & 2 deletions src/Type/TypeCombinator.php
Expand Up @@ -623,17 +623,101 @@ private static function processArrayTypes(array $arrayTypes): array
return [
self::intersect(new ArrayType(
self::union(...$keyTypesForGeneralArray),
self::union(...$valueTypesForGeneralArray),
self::union(...self::optimizeConstantArrays($valueTypesForGeneralArray)),
), ...$accessoryTypes),
];
}

$reducedArrayTypes = self::reduceArrays($arrayTypes);

return array_map(
static fn (Type $arrayType) => self::intersect($arrayType, ...$accessoryTypes),
self::reduceArrays($arrayTypes),
self::optimizeConstantArrays($reducedArrayTypes),
);
}

/**
* @param Type[] $types
* @return Type[]
*/
private static function optimizeConstantArrays(array $types): array
{
$constantArrayValuesCount = self::countConstantArrayValueTypes($types);

if ($constantArrayValuesCount > ConstantArrayTypeBuilder::ARRAY_COUNT_LIMIT) {
$results = [];
foreach ($types as $type) {
$results[] = TypeTraverser::map($type, static function (Type $type, callable $traverse): Type {
if ($type instanceof ConstantArrayType) {
if ($type->isIterableAtLeastOnce()->no()) {
return $type;
}

$isList = true;
$valueTypes = [];
$keyTypes = [];
$nextAutoIndex = 0;
foreach ($type->getKeyTypes() as $i => $innerKeyType) {
if (!$innerKeyType instanceof ConstantIntegerType) {
$isList = false;
} elseif ($innerKeyType->getValue() !== $nextAutoIndex) {
$isList = false;
$nextAutoIndex = $innerKeyType->getValue() + 1;
} else {
$nextAutoIndex++;
}

$generalizedKeyType = $innerKeyType->generalize(GeneralizePrecision::moreSpecific());
$keyTypes[$generalizedKeyType->describe(VerbosityLevel::precise())] = $generalizedKeyType;

$innerValueType = $type->getValueTypes()[$i];
$generalizedValueType = $traverse($innerValueType);
$valueTypes[$generalizedValueType->describe(VerbosityLevel::precise())] = $generalizedValueType;
}

$keyType = TypeCombinator::union(...array_values($keyTypes));
$valueType = TypeCombinator::union(...array_values($valueTypes));

$arrayType = new ArrayType($keyType, $valueType);
if ($isList) {
$arrayType = AccessoryArrayListType::intersectWith($arrayType);
}

return TypeCombinator::intersect($arrayType, new NonEmptyArrayType(), new OversizedArrayType());
}

return $traverse($type);
});
}

return $results;
}

return $types;
}

/**
* @param Type[] $types
*/
private static function countConstantArrayValueTypes(array $types): int
{
$constantArrayValuesCount = 0;
foreach ($types as $type) {
if ($type instanceof ConstantArrayType) {
$constantArrayValuesCount += count($type->getValueTypes());
}

TypeTraverser::map($type, static function (Type $type, callable $traverse) use (&$constantArrayValuesCount): Type {
if ($type instanceof ConstantArrayType) {
$constantArrayValuesCount += count($type->getValueTypes());
}

return $traverse($type);
});
}
return $constantArrayValuesCount;
}

/**
* @param Type[] $constantArrays
* @return Type[]
Expand Down
20 changes: 7 additions & 13 deletions tests/PHPStan/Analyser/AnalyserIntegrationTest.php
Expand Up @@ -931,19 +931,7 @@ public function testBug7581(): void
public function testBug7903(): void
{
$errors = $this->runAnalyse(__DIR__ . '/data/bug-7903.php');
$this->assertCount(6, $errors);
$this->assertSame('Comparison operation ">" between 0 and 0 is always false.', $errors[0]->getMessage());
$this->assertSame(212, $errors[0]->getLine());
$this->assertSame('Comparison operation ">" between 0 and 0 is always false.', $errors[1]->getMessage());
$this->assertSame(213, $errors[1]->getLine());
$this->assertSame('Comparison operation ">" between 0 and 0 is always false.', $errors[2]->getMessage());
$this->assertSame(214, $errors[2]->getLine());
$this->assertSame('Comparison operation ">" between 0 and 0 is always false.', $errors[3]->getMessage());
$this->assertSame(215, $errors[3]->getLine());
$this->assertSame('Comparison operation ">" between 0 and 0 is always false.', $errors[4]->getMessage());
$this->assertSame(229, $errors[4]->getLine());
$this->assertSame('Comparison operation ">" between 0 and 0 is always false.', $errors[5]->getMessage());
$this->assertSame(230, $errors[5]->getLine());
$this->assertNoErrors($errors);
}

public function testBug7901(): void
Expand Down Expand Up @@ -1097,6 +1085,12 @@ public function testBug8215(): void
$this->assertNoErrors($errors);
}

public function testBug8146a(): void
{
$errors = $this->runAnalyse(__DIR__ . '/data/bug-8146a.php');
$this->assertNoErrors($errors);
}

/**
* @param string[]|null $allAnalysedFiles
* @return Error[]
Expand Down
2 changes: 1 addition & 1 deletion tests/PHPStan/Analyser/data/bug-8004.php
Expand Up @@ -73,7 +73,7 @@ public function getErrorsOnInvalidQuestions(array $importQuiz, int $key): array
}
}

assertType("list<array{line: int, type: 'empty_answer'|'empty_question'|'invalid_answer_1_too_long'|'invalid_answer_1_type'|'invalid_answer_2_too_long'|'invalid_answer_2_type'|'invalid_answer_3_too_long'|'invalid_answer_3_type'|'invalid_answer_4_too_long'|'invalid_answer_4_type'|'invalid_question_too_long'|'invalid_right_answer', value: int}>", $errors);
assertType("list<non-empty-array<literal-string&non-falsy-string, 'empty_answer'|'empty_question'|'invalid_answer_1_too_long'|'invalid_answer_1_type'|'invalid_answer_2_too_long'|'invalid_answer_2_type'|'invalid_answer_3_too_long'|'invalid_answer_3_type'|'invalid_answer_4_too_long'|'invalid_answer_4_type'|'invalid_question_too_long'|'invalid_right_answer'|int>>", $errors);

return $errors;
}
Expand Down
152 changes: 152 additions & 0 deletions tests/PHPStan/Analyser/data/bug-8146a.php
@@ -0,0 +1,152 @@
<?php

declare(strict_types=1);

namespace Bug8146a;

class HelloWorld
{
private SessionInterface $session;
private DataObject $object;

public function __construct(SessionInterface $session, DataObject $object)
{
$this->session = $session;
$this->object = $object;
}

public function sayHello(): void
{
$changeLog = [];

$firstname = $this->session->get('firstname');
if ($firstname !== $this->object->getFirstname()) {
$changelog['firstname_old'] = $this->object->getFirstname();
$changelog['firstname_new'] = $firstname;
}

$lastname = $this->session->get('lastname');
if ($lastname !== $this->object->getLastname()) {
$changelog['lastname_old'] = $this->object->getLastname();
$changelog['lastname_new'] = $lastname;
}

$street = $this->session->get('street');
if ($street !== $this->object->getStreet()) {
$changelog['street_old'] = $this->object->getStreet();
$changelog['street_new'] = $street;
}

$zip = $this->session->get('zip');
if ($zip !== $this->object->getZip()) {
$changelog['zip_old'] = $this->object->getZip();
$changelog['zip_new'] = $zip;
}

$city = $this->session->get('city');
if ($city !== $this->object->getCity()) {
$changelog['city_old'] = $this->object->getCity();
$changelog['city_new'] = $city;
}

$phonenumber = $this->session->get('phonenumber');
if ($phonenumber !== $this->object->getPhonenumber()) {
$changelog['phonenumber_old'] = $this->object->getPhonenumber();
$changelog['phonenumber_new'] = $phonenumber;
}

$email = $this->session->get('email');
if ($email !== $this->object->getEmail()) {
$changelog['email_old'] = $this->object->getEmail();
$changelog['email_new'] = $email;
}

$deliveryFirstname = $this->session->get('deliveryFirstname');
if ($deliveryFirstname !== $this->object->getDeliveryFirstname()) {
$changelog['deliveryFirstname_old'] = $this->object->getDeliveryFirstname();
$changelog['deliveryFirstname_new'] = $deliveryFirstname;
}

$deliveryLastname = $this->session->get('deliveryLastname');
if ($deliveryLastname !== $this->object->getDeliveryLastname()) {
$changelog['deliveryLastname_old'] = $this->object->getDeliveryLastname();
$changelog['deliveryLastname_new'] = $deliveryLastname;
}
$deliveryStreet = $this->session->get('deliveryStreet');
if ($deliveryStreet !== $this->object->getDeliveryStreet()) {
$changelog['deliveryStreet_old'] = $this->object->getDeliveryStreet();
$changelog['deliveryStreet_new'] = $deliveryStreet;
}
$deliveryZip = $this->session->get('deliveryZip');
if ($deliveryZip !== $this->object->getDeliveryZip()) {
$changelog['deliveryZip_old'] = $this->object->getDeliveryZip();
$changelog['deliveryZip_new'] = $deliveryZip;
}
$deliveryCity = $this->session->get('deliveryCity');
if ($deliveryCity !== $this->object->getDeliveryCity()) {
$changelog['deliveryCity_old'] = $this->object->getDeliveryCity();
$changelog['deliveryCity_new'] = $deliveryCity;
}

}
}

interface SessionInterface
{
/**
* @return mixed
*/
public function get(string $key);
}

interface DataObject
{
/**
* @return string|null
*/
public function getFirstname();
/**
* @return string|null
*/
public function getLastname();
/**
* @return string|null
*/
public function getStreet();
/**
* @return string|null
*/
public function getZip();
/**
* @return string|null
*/
public function getCity();
/**
* @return string|null
*/
public function getPhonenumber();
/**
* @return string|null
*/
public function getEmail();
/**
* @return string|null
*/
public function getDeliveryFirstname();
/**
* @return string|null
*/
public function getDeliveryLastname();
/**
* @return string|null
*/
public function getDeliveryStreet();
/**
* @return string|null
*/
public function getDeliveryZip();
/**
* @return string|null
*/
public function getDeliveryCity();
}