Skip to content

Commit

Permalink
Merge pull request #8324 from Nicelocal/fix_union_assertions
Browse files Browse the repository at this point in the history 
Fix union assertions
  • Loading branch information
@orklah
orklah committed Jul 28, 2022
2 parents 63b389f + 7a7a7f6 commit 7c4228f
Show file tree
Hide file tree
Showing 11 changed files with 166 additions and 174 deletions.
149 changes: 37 additions & 112 deletions src/Psalm/Internal/Analyzer/Statements/Expression/CallAnalyzer.php
View file
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
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
*/
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
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
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
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
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

0 comments on commit 7c4228f

Please sign in to comment.