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 union assertions #8324

Merged
merged 13 commits into from Jul 28, 2022
Merged

Fix union assertions #8324

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
18106af
Refactor
danog Jul 27, 2022
09fb879
Fix #8322, #8319
danog Jul 27, 2022
37274b3
Add temporary failing test
danog Jul 27, 2022
e195891
Fix unit test
danog Jul 27, 2022
c2a8d1b
cs
danog Jul 27, 2022
c409675
Fix intersection of final classes
danog Jul 27, 2022
82e8c0d
Disable intersection of unrelated classes
danog Jul 27, 2022
1a4d68e
Make private
danog Jul 27, 2022
68f3f24
Fix
danog Jul 27, 2022
6ce32a4
Fix
danog Jul 27, 2022
abe5887
Fix
danog Jul 27, 2022
33d8f3a
Fix unit tests
danog Jul 28, 2022
7a7a7f6
cs-fix
danog Jul 28, 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
149 changes: 37 additions & 112 deletions src/Psalm/Internal/Analyzer/Statements/Expression/CallAnalyzer.php
View file
Open in desktop
Expand Up @@ -753,20 +753,20 @@ public static function applyAssertionsToContext(
$orred_rules = [];

foreach ($var_possibilities->rule as $assertion_rule) {
$assertion_type = $assertion_rule->getAtomicType();
$assertion_type_atomic = $assertion_rule->getAtomicType();

if ($assertion_type) {
$union = new Union([clone $assertion_type]);
if ($assertion_type_atomic) {
$assertion_type = new Union([clone $assertion_type_atomic]);
TemplateInferredTypeReplacer::replace(
$union,
$assertion_type,
$template_result,
$codebase
);

if ($union->isSingle()) {
foreach ($union->getAtomicTypes() as $atomic_type) {
if ($assertion_type instanceof TTemplateParam
&& $assertion_type->as->getId() === $atomic_type->getId()
if (count($assertion_type->getAtomicTypes()) === 1) {
foreach ($assertion_type->getAtomicTypes() as $atomic_type) {
if ($assertion_type_atomic instanceof TTemplateParam
&& $assertion_type_atomic->as->getId() === $atomic_type->getId()
) {
continue;
}
Expand All @@ -775,43 +775,46 @@ public static function applyAssertionsToContext(
$assertion_rule->setAtomicType($atomic_type);
$orred_rules[] = $assertion_rule;
}
} elseif (isset($context->vars_in_scope[$var_possibilities->var_id])) {
$other_type = $context->vars_in_scope[$var_possibilities->var_id];
} elseif (isset($context->vars_in_scope[$assertion_var_id])) {
$asserted_type = $context->vars_in_scope[$assertion_var_id];
if ($assertion_rule instanceof IsIdentical) {
$intersection = Type::intersectUnionTypes($assertion_type, $asserted_type, $codebase);

if ($assertion_rule instanceof IsIdentical
|| $assertion_rule instanceof IsType
) {
if ($intersection === null) {
IssueBuffer::maybeAdd(
new TypeDoesNotContainType(
$asserted_type->getId() . ' is not contained by '
. $assertion_type->getId(),
new CodeLocation($statements_analyzer->getSource(), $expr),
$asserted_type->getId() . ' ' . $assertion_type->getId()
),
$statements_analyzer->getSuppressedIssues()
);
$intersection = Type::getNever();
} elseif ($intersection->getId(true) === $asserted_type->getId(true)) {
continue;
}
foreach ($intersection->getAtomicTypes() as $atomic_type) {
$orred_rules[] = new IsIdentical($atomic_type);
}
} elseif ($assertion_rule instanceof IsType) {
if (!UnionTypeComparator::canExpressionTypesBeIdentical(
$codebase,
$union,
$context->vars_in_scope[$var_possibilities->var_id]
$assertion_type,
$asserted_type
)) {
IssueBuffer::maybeAdd(
new TypeDoesNotContainType(
$union->getId() . ' cannot be identical to ' . $other_type->getId(),
$asserted_type->getId() . ' is not contained by '
. $assertion_type->getId(),
new CodeLocation($statements_analyzer->getSource(), $expr),
$union->getId() . ' ' . $other_type->getId()
$asserted_type->getId() . ' ' . $assertion_type->getId()
),
$statements_analyzer->getSuppressedIssues()
);
}
}
} elseif (isset($context->vars_in_scope[$assertion_var_id])) {
$other_type = $context->vars_in_scope[$assertion_var_id];
$union = self::createUnionIntersectionFromOldType($union, $other_type);

if ($union !== null) {
foreach ($union->getAtomicTypes() as $atomic_type) {
if ($assertion_type instanceof TTemplateParam
&& $assertion_type->as->getId() === $atomic_type->getId()
) {
continue;
}

$assertion_rule = clone $assertion_rule;
$assertion_rule->setAtomicType($atomic_type);
$orred_rules[] = $assertion_rule;
}
} else {
// Ignore negations and loose assertions with union types
}
}
} else {
Expand Down Expand Up @@ -1135,82 +1138,4 @@ public static function checkTemplateResult(
}
}
}

/**
* This method should detect if the new type narrows down the old type.
*/
private static function isNewTypeNarrowingDownOldType(Union $old_type, Union $new_type): bool
{
if ($new_type->isSingle()) {
return true;
}

// non-mixed is always better than mixed
if ($old_type->isMixed() && !$new_type->hasMixed()) {
return true;
}

// non-nullable is always better than nullable
if ($old_type->isNullable() && !$new_type->isNullable()) {
return true;
}

// Do not hassle around with non-single old types if they are not nullable
if (!$old_type->isSingle()) {
return false;
}

// Do not hassle around with single literals as they supposed to be more accurate than any new type assertion
if ($old_type->isSingleFloatLiteral()
|| $old_type->isSingleIntLiteral()
|| $old_type->isSingleStringLiteral()
) {
return false;
}

// Literals should always replace non-literals
if (($old_type->isString() && $new_type->allStringLiterals())
|| ($old_type->isInt() && $new_type->allIntLiterals())
|| ($old_type->isFloat() && $new_type->allFloatLiterals())
) {
return true;
}

return false;
}

/**
* This method should kick all literals within `new_type` which are not part of the already known `old_type`.
* So lets say we already know that the old type is one of "a", "b" or "c".
* If another assertion takes place to determine if the value is either "a", "c" or "d", we can kick "d" as that
* won't be possible.
*/
private static function createUnionIntersectionFromOldType(Union $new_type, Union $old_type): ?Union
{
if (!self::isNewTypeNarrowingDownOldType($old_type, $new_type)) {
return null;
}

if (!$new_type->allLiterals() || !$old_type->allLiterals()) {
return $new_type;
}

$equal_atomic_types = [];

foreach ($new_type->getAtomicTypes() as $new_atomic_type) {
foreach ($old_type->getAtomicTypes() as $old_atomic_type) {
if (!$new_atomic_type->equals($old_atomic_type, false)) {
continue;
}

$equal_atomic_types[] = $new_atomic_type;
}
}

if ($equal_atomic_types === []) {
return null;
}

return new Union($equal_atomic_types);
}
}
51 changes: 43 additions & 8 deletions src/Psalm/Type.php
View file
Open in desktop
Expand Up @@ -19,6 +19,7 @@
use Psalm\Type\Atomic\TFalse;
use Psalm\Type\Atomic\TFloat;
use Psalm\Type\Atomic\TInt;
use Psalm\Type\Atomic\TIntRange;
use Psalm\Type\Atomic\TIterable;
use Psalm\Type\Atomic\TList;
use Psalm\Type\Atomic\TLiteralClassString;
Expand Down Expand Up @@ -691,6 +692,23 @@ private static function intersectAtomicTypes(
$intersection_performed = true;
}
}
if ($type_1_atomic instanceof TInt && $type_2_atomic instanceof TInt) {
$int_intersection = TIntRange::intersectIntRanges(
TIntRange::convertToIntRange($type_1_atomic),
TIntRange::convertToIntRange($type_2_atomic)
);
if ($int_intersection
&& ($int_intersection->min_bound !== null || $int_intersection->max_bound !== null)
) {
$intersection_performed = true;
if ($int_intersection->min_bound !== null
&& $int_intersection->min_bound === $int_intersection->max_bound
) {
return new TLiteralInt($int_intersection->min_bound);
}
return $int_intersection;
}
}

if (null === $intersection_atomic) {
if (AtomicTypeComparator::isContainedBy(
Expand Down Expand Up @@ -719,9 +737,19 @@ private static function intersectAtomicTypes(
}
}

if (static::mayHaveIntersection($type_1_atomic)
&& static::mayHaveIntersection($type_2_atomic)
if (self::mayHaveIntersection($type_1_atomic, $codebase)
&& self::mayHaveIntersection($type_2_atomic, $codebase)
) {
/** @psalm-suppress TypeDoesNotContainType */
if ($type_1_atomic instanceof TNamedObject && $type_2_atomic instanceof TNamedObject) {
$first = $codebase->classlike_storage_provider->get($type_1_atomic->value);
$second = $codebase->classlike_storage_provider->get($type_2_atomic->value);
$first_is_class = !$first->is_interface && !$first->is_trait;
$second_is_class = !$second->is_interface && !$second->is_trait;
if ($first_is_class && $second_is_class) {
return $intersection_atomic;
}
}
if ($intersection_atomic === null && $wider_type === null) {
$intersection_atomic = clone $type_1_atomic;
$wider_type = $type_2_atomic;
Expand All @@ -733,8 +761,8 @@ private static function intersectAtomicTypes(
.' Did you forget to assign one of the variables?'
);
}
if (!static::mayHaveIntersection($intersection_atomic)
|| !static::mayHaveIntersection($wider_type)
if (!self::mayHaveIntersection($intersection_atomic, $codebase)
|| !self::mayHaveIntersection($wider_type, $codebase)
) {
throw new LogicException(
'$intersection_atomic and $wider_type should be both support intersection.'
Expand Down Expand Up @@ -769,12 +797,19 @@ private static function intersectAtomicTypes(
/**
* @psalm-assert-if-true TIterable|TNamedObject|TTemplateParam|TObjectWithProperties $type
danog marked this conversation as resolved.
Show resolved Hide resolved
*/
private static function mayHaveIntersection(Atomic $type): bool
private static function mayHaveIntersection(Atomic $type, Codebase $codebase): bool
{
return $type instanceof TIterable
|| $type instanceof TNamedObject
if ($type instanceof TIterable
|| $type instanceof TTemplateParam
|| $type instanceof TObjectWithProperties;
|| $type instanceof TObjectWithProperties
) {
return true;
}
if (!$type instanceof TNamedObject) {
return false;
}
$storage = $codebase->classlike_storage_provider->get($type->value);
return !$storage->final;
}

private static function hasIntersection(Atomic $type): bool
Expand Down
13 changes: 0 additions & 13 deletions src/Psalm/Type/Atomic/TLiteralFloat.php
View file
Open in desktop
Expand Up @@ -2,10 +2,6 @@

namespace Psalm\Type\Atomic;

use Psalm\Type\Atomic;

use function get_class;

/**
* Denotes a floating point value where the exact numeric value is known.
*/
Expand Down Expand Up @@ -45,13 +41,4 @@ public function toNamespacedString(
): string {
return 'float';
}

public function equals(Atomic $other_type, bool $ensure_source_equality): bool
{
if (get_class($other_type) !== static::class) {
return false;
}

return $this->value === $other_type->value;
}
}
19 changes: 0 additions & 19 deletions src/Psalm/Type/Atomic/TLiteralInt.php
View file
Open in desktop
Expand Up @@ -2,10 +2,6 @@

namespace Psalm\Type\Atomic;

use Psalm\Type\Atomic;

use function get_class;

/**
* Denotes an integer value where the exact numeric value is known.
*/
Expand Down Expand Up @@ -50,19 +46,4 @@ public function toNamespacedString(
): string {
return $use_phpdoc_format ? 'int' : (string) $this->value;
}

public function equals(Atomic $other_type, bool $ensure_source_equality): bool
{
if (get_class($other_type) !== static::class) {
return false;
}

if (($this->from_docblock && $ensure_source_equality)
|| ($other_type->from_docblock && $ensure_source_equality)
) {
return false;
}

return $this->value === $other_type->value;
}
}
18 changes: 0 additions & 18 deletions src/Psalm/Type/Atomic/TLiteralString.php
View file
Open in desktop
Expand Up @@ -2,10 +2,7 @@

namespace Psalm\Type\Atomic;

use Psalm\Type\Atomic;

use function addcslashes;
use function get_class;
use function mb_strlen;
use function mb_substr;

Expand Down Expand Up @@ -58,19 +55,4 @@ public function toNamespacedString(
): string {
return $use_phpdoc_format ? 'string' : "'" . $this->value . "'";
}

public function equals(Atomic $other_type, bool $ensure_source_equality): bool
{
if (get_class($other_type) !== static::class) {
return false;
}

if (($this->from_docblock && $ensure_source_equality)
|| ($other_type->from_docblock && $ensure_source_equality)
) {
return false;
}

return $this->value === $other_type->value;
}
}
3 changes: 3 additions & 0 deletions src/Psalm/Type/Union.php
View file
Open in desktop
Expand Up @@ -1268,6 +1268,9 @@ public function allIntLiterals(): bool
return true;
}

/**
* @psalm-suppress PossiblyUnusedMethod Public API
*/
public function allFloatLiterals(): bool
{
foreach ($this->types as $atomic_key_type) {
Expand Down