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

Improve TypeCombinator::reduceArrays() perf with retained type compleness #1741

Merged
merged 1 commit into from Sep 23, 2022
Merged

Improve TypeCombinator::reduceArrays() perf with retained type compleness #1741

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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
95 changes: 39 additions & 56 deletions src/Type/TypeCombinator.php
View file
Open in desktop
Expand Up @@ -637,96 +637,79 @@ private static function processArrayTypes(array $arrayTypes, array $accessoryTyp
return [
self::intersect(new ArrayType(
self::union(...$keyTypesForGeneralArray),
self::union(...self::optimizeConstantArrays($valueTypesForGeneralArray)),
self::union(...$valueTypesForGeneralArray),
), ...$accessoryTypes),
];
}

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

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

/**
* @param Type[] $types
* @param Type[] $constantArrays
* @return Type[]
*/
private static function optimizeConstantArrays(array $types): array
private static function reduceArrays(array $constantArrays): 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) {
return $type->generalize(GeneralizePrecision::moreSpecific());
}
$newArrays = [];
$arraysToProcess = [];
$emptyArray = null;
foreach ($constantArrays as $constantArray) {
if (!$constantArray instanceof ConstantArrayType) {
$newArrays[] = $constantArray;
continue;
}

return $traverse($type);
});
if ($constantArray->isEmpty()) {
$emptyArray = $constantArray;
continue;
}

return $results;
$arraysToProcess[] = $constantArray;
}

return $types;
}
if ($emptyArray !== null) {
$newArrays[] = $emptyArray;
}

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

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

return $traverse($type);
});
foreach ($arraysToProcessPerKey as $arrays) {
for ($i = 0, $arraysCount = count($arrays); $i < $arraysCount - 1; $i++) {
for ($j = $i + 1; $j < $arraysCount; $j++) {
$eligibleCombinations[$arrays[$i]][$arrays[$j]] = $arrays[$j];
}
}
}
rvanvelzen marked this conversation as resolved.
Show resolved Hide resolved
return $constantArrayValuesCount;
}

/**
* @param Type[] $constantArrays
* @return Type[]
*/
private static function reduceArrays(array $constantArrays): array
{
$newArrays = [];
$arraysToProcess = [];
foreach ($constantArrays as $constantArray) {
if (!$constantArray instanceof ConstantArrayType) {
$newArrays[] = $constantArray;
foreach ($eligibleCombinations as $i => $other) {
if (!array_key_exists($i, $arraysToProcess)) {
continue;
}

$arraysToProcess[] = $constantArray;
}
foreach ($other as $j) {
if (!array_key_exists($j, $arraysToProcess)) {
continue;
}

for ($i = 0, $arraysToProcessCount = count($arraysToProcess); $i < $arraysToProcessCount; $i++) {
for ($j = $i + 1; $j < $arraysToProcessCount; $j++) {
if ($arraysToProcess[$j]->isKeysSupersetOf($arraysToProcess[$i])) {
$arraysToProcess[$j] = $arraysToProcess[$j]->mergeWith($arraysToProcess[$i]);
array_splice($arraysToProcess, $i--, 1);
$arraysToProcessCount--;
unset($arraysToProcess[$i]);
continue 2;

} elseif ($arraysToProcess[$i]->isKeysSupersetOf($arraysToProcess[$j])) {
$arraysToProcess[$i] = $arraysToProcess[$i]->mergeWith($arraysToProcess[$j]);
array_splice($arraysToProcess, $j--, 1);
$arraysToProcessCount--;
unset($arraysToProcess[$j]);
continue 1;
}
}
Expand Down
14 changes: 13 additions & 1 deletion tests/PHPStan/Analyser/AnalyserIntegrationTest.php
View file
Open in desktop
Expand Up @@ -931,7 +931,19 @@ public function testBug7581(): void
public function testBug7903(): void
{
$errors = $this->runAnalyse(__DIR__ . '/data/bug-7903.php');
$this->assertNoErrors($errors);
$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());
}

public function testBug7901(): void
Expand Down
1 change: 1 addition & 0 deletions tests/PHPStan/Analyser/NodeScopeResolverTest.php
View file
Open in desktop
Expand Up @@ -1041,6 +1041,7 @@ public function dataFileAsserts(): iterable
yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-7987.php');
yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-7963-three.php');
yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-8017.php');
yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-8004.php');
}

/**
Expand Down
114 changes: 114 additions & 0 deletions tests/PHPStan/Analyser/data/bug-8004.php
View file
Open in desktop
@@ -0,0 +1,114 @@
<?php

namespace Bug8004;

use function PHPStan\Testing\assertType;

class Importer
{
/**
* @param array{questions: array<int, array{question: string|null, answer_1: string|\DateTimeInterface|null, answer_2: string|\DateTimeInterface|null, answer_3: string|\DateTimeInterface|null, answer_4: string|\DateTimeInterface|null, right_answer: int|float|string|\DateTimeInterface|null, right_answer_explanation: string|null}>} $importQuiz
*
* @return list<array{line: int, type: QuizImportErrorType::*, value: int|float|string|\DateTimeInterface|null}>
*/
public function getErrorsOnInvalidQuestions(array $importQuiz, int $key): array
{
$errors = [];

foreach ($importQuiz['questions'] as $index => $question) {
if (empty($question['question']) && empty($question['answer_1']) && empty($question['answer_2']) && empty($question['answer_3']) && empty($question['answer_4']) && empty($question['right_answer']) && empty($question['right_answer_explanation'])) {
continue;
}

if (empty($question['question'])) {
$errors[] = ['line' => $key, 'type' => QuizImportErrorType::EMPTY_QUESTION, 'value' => $index + 1];
} elseif (255 < mb_strlen($question['question'])) {
$errors[] = ['line' => $key, 'type' => QuizImportErrorType::INVALID_QUESTION_TOO_LONG, 'value' => $index + 1];
}

if (null === $question['answer_1'] || '' === $question['answer_1'] || null === $question['answer_2'] || '' === $question['answer_2']) {
$errors[] = ['line' => $key, 'type' => QuizImportErrorType::EMPTY_ANSWER, 'value' => $index + 1];
}

if (null !== $question['answer_1'] && '' !== $question['answer_1']) {
if (\is_string($question['answer_1']) && 150 < mb_strlen($question['answer_1'])) {
$errors[] = ['line' => $key, 'type' => QuizImportErrorType::INVALID_ANSWER_1_TOO_LONG, 'value' => $index + 1];
} elseif ($question['answer_1'] instanceof \DateTimeInterface) {
$errors[] = ['line' => $key, 'type' => QuizImportErrorType::INVALID_ANSWER_1_TYPE, 'value' => $index + 1];
}
}

if (null !== $question['answer_2'] && '' !== $question['answer_2']) {
if (\is_string($question['answer_2']) && 150 < mb_strlen($question['answer_2'])) {
$errors[] = ['line' => $key, 'type' => QuizImportErrorType::INVALID_ANSWER_2_TOO_LONG, 'value' => $index + 1];
} elseif ($question['answer_2'] instanceof \DateTimeInterface) {
$errors[] = ['line' => $key, 'type' => QuizImportErrorType::INVALID_ANSWER_2_TYPE, 'value' => $index + 1];
}
}

$hasQuestion3 = isset($question['answer_3']) && null !== $question['answer_3'] && '' !== $question['answer_3'];

if ($hasQuestion3) {
if (\is_string($question['answer_3']) && 150 < mb_strlen($question['answer_3'])) {
$errors[] = ['line' => $key, 'type' => QuizImportErrorType::INVALID_ANSWER_3_TOO_LONG, 'value' => $index + 1];
} elseif ($question['answer_3'] instanceof \DateTimeInterface) {
$errors[] = ['line' => $key, 'type' => QuizImportErrorType::INVALID_ANSWER_3_TYPE, 'value' => $index + 1];
}
}

$hasQuestion4 = isset($question['answer_4']) && null !== $question['answer_4'] && '' !== $question['answer_4'];

if ($hasQuestion4) {
if (\is_string($question['answer_4']) && 150 < mb_strlen($question['answer_4'])) {
$errors[] = ['line' => $key, 'type' => QuizImportErrorType::INVALID_ANSWER_4_TOO_LONG, 'value' => $index + 1];
} elseif ($question['answer_4'] instanceof \DateTimeInterface) {
$errors[] = ['line' => $key, 'type' => QuizImportErrorType::INVALID_ANSWER_4_TYPE, 'value' => $index + 1];
}
}

$answerCount = 2 + ($hasQuestion3 ? 1 : 0) + ($hasQuestion4 ? 1 : 0);

if (empty($question['right_answer']) || !is_numeric($question['right_answer']) || $question['right_answer'] <= 0 || (int) $question['right_answer'] > $answerCount) {
$errors[] = ['line' => $key, 'type' => QuizImportErrorType::INVALID_RIGHT_ANSWER, 'value' => $index + 1];
}
}

assertType("array<int, 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);

return $errors;
}
}

class QuizImportErrorType
{
public const EMPTY_ANSWER = 'empty_answer';
public const EMPTY_BONUS_DURATION = 'empty_bonus_duration';
public const EMPTY_BONUS_POINTS = 'empty_bonus_points';
public const EMPTY_COLLECTION = 'empty_collection';
public const EMPTY_INTRODUCTION = 'empty_introduction';
public const EMPTY_QUESTION = 'empty_question';
public const EMPTY_TITLE = 'empty_title';
public const INVALID_ANSWER_1_TOO_LONG = 'invalid_answer_1_too_long';
public const INVALID_ANSWER_2_TOO_LONG = 'invalid_answer_2_too_long';
public const INVALID_ANSWER_3_TOO_LONG = 'invalid_answer_3_too_long';
public const INVALID_ANSWER_4_TOO_LONG = 'invalid_answer_4_too_long';
public const INVALID_ANSWER_1_TYPE = 'invalid_answer_1_type';
public const INVALID_ANSWER_2_TYPE = 'invalid_answer_2_type';
public const INVALID_ANSWER_3_TYPE = 'invalid_answer_3_type';
public const INVALID_ANSWER_4_TYPE = 'invalid_answer_4_type';
public const INVALID_AUTHOR = 'invalid_author';
public const INVALID_BONUS_DURATION = 'invalid_bonus_duration';
public const INVALID_BONUS_POINTS = 'invalid_bonus_points';
public const INVALID_CLOSING_DATE = 'invalid_closing_date';
public const INVALID_COLLECTION = 'invalid_collection';
public const INVALID_NEWS_FEED = 'invalid_news_feed';
public const INVALID_PARTICIPATION_POINTS = 'invalid_participation_points';
public const INVALID_PERFECT_POINTS = 'invalid_perfect_points';
public const INVALID_PUBLICATION_DATE = 'invalid_publication_date';
public const INVALID_QUESTION_TOO_LONG = 'invalid_question_too_long';
public const INVALID_RESPONSE_TIME = 'invalid_response_time';
public const INVALID_RIGHT_ANSWER = 'invalid_right_answer';
public const INVALID_RIGHT_ANSWER_POINTS = 'invalid_right_answer_points';
public const INVALID_TITLE_TOO_LONG = 'invalid_title_too_long';
public const INVALID_WRONG_ANSWER_POINTS = 'invalid_wrong_answer_points';
}