From af28687708740335d3add1ac3ca421672dc3a2a0 Mon Sep 17 00:00:00 2001 From: orklah Date: Fri, 7 Jan 2022 19:38:15 +0100 Subject: [PATCH] fix reconciliation when the assertions is not part of the existing range and add tests --- .../Type/SimpleAssertionReconciler.php | 14 +++++++++ .../Type/SimpleNegatedAssertionReconciler.php | 14 +++++++++ src/Psalm/Type/Atomic/TIntRange.php | 16 ++++++++++ tests/IntRangeTest.php | 30 +++++++++++++++++++ 4 files changed, 74 insertions(+) diff --git a/src/Psalm/Internal/Type/SimpleAssertionReconciler.php b/src/Psalm/Internal/Type/SimpleAssertionReconciler.php index c90eeeeb5..9865e5d4b 100644 --- a/src/Psalm/Internal/Type/SimpleAssertionReconciler.php +++ b/src/Psalm/Internal/Type/SimpleAssertionReconciler.php @@ -1653,6 +1653,7 @@ class SimpleAssertionReconciler extends Reconciler if ($atomic_type instanceof TIntRange) { if ($atomic_type->contains($assertion_value)) { + // if the range contains the assertion, the range must be adapted $did_remove_type = true; $existing_var_type->removeType($atomic_type->getKey()); if ($atomic_type->min_bound === null) { @@ -1664,6 +1665,12 @@ class SimpleAssertionReconciler extends Reconciler ); } $existing_var_type->addType($atomic_type); + } elseif ($atomic_type->isLesserThan($assertion_value)) { + // if the range is lesser than the assertion, the type must be removed + $did_remove_type = true; + $existing_var_type->removeType($atomic_type->getKey()); + } elseif ($atomic_type->isGreaterThan($assertion_value)) { + // if the range is greater than the assertion, the check is redundant } } elseif ($atomic_type instanceof TLiteralInt) { if ($atomic_type->value < $assertion_value) { @@ -1751,6 +1758,7 @@ class SimpleAssertionReconciler extends Reconciler if ($atomic_type instanceof TIntRange) { if ($atomic_type->contains($assertion_value)) { + // if the range contains the assertion, the range must be adapted $did_remove_type = true; $existing_var_type->removeType($atomic_type->getKey()); if ($atomic_type->max_bound === null) { @@ -1759,6 +1767,12 @@ class SimpleAssertionReconciler extends Reconciler $atomic_type->max_bound = min($atomic_type->max_bound, $assertion_value); } $existing_var_type->addType($atomic_type); + } elseif ($atomic_type->isLesserThan($assertion_value)) { + // if the range is lesser than the assertion, the check is redundant + } elseif ($atomic_type->isGreaterThan($assertion_value)) { + // if the range is greater than the assertion, the type must be removed + $did_remove_type = true; + $existing_var_type->removeType($atomic_type->getKey()); } } elseif ($atomic_type instanceof TLiteralInt) { if ($atomic_type->value > $assertion_value) { diff --git a/src/Psalm/Internal/Type/SimpleNegatedAssertionReconciler.php b/src/Psalm/Internal/Type/SimpleNegatedAssertionReconciler.php index 63d77ce58..5fdc2508a 100644 --- a/src/Psalm/Internal/Type/SimpleNegatedAssertionReconciler.php +++ b/src/Psalm/Internal/Type/SimpleNegatedAssertionReconciler.php @@ -1638,6 +1638,7 @@ class SimpleNegatedAssertionReconciler extends Reconciler if ($atomic_type instanceof TIntRange) { if ($atomic_type->contains($assertion_value)) { + // if the range contains the assertion, the range must be adapted $did_remove_type = true; $existing_var_type->removeType($atomic_type->getKey()); if ($atomic_type->max_bound === null) { @@ -1649,6 +1650,12 @@ class SimpleNegatedAssertionReconciler extends Reconciler ); } $existing_var_type->addType($atomic_type); + } elseif ($atomic_type->isLesserThan($assertion_value)) { + // if the range is lesser than the assertion, the check is redundant + } elseif ($atomic_type->isGreaterThan($assertion_value)) { + // if the range is greater than the assertion, the type must be removed + $did_remove_type = true; + $existing_var_type->removeType($atomic_type->getKey()); } } elseif ($atomic_type instanceof TLiteralInt) { if ($atomic_type->value > $assertion_value) { @@ -1736,6 +1743,7 @@ class SimpleNegatedAssertionReconciler extends Reconciler if ($atomic_type instanceof TIntRange) { if ($atomic_type->contains($assertion_value)) { + // if the range contains the assertion, the range must be adapted $did_remove_type = true; $existing_var_type->removeType($atomic_type->getKey()); if ($atomic_type->min_bound === null) { @@ -1744,6 +1752,12 @@ class SimpleNegatedAssertionReconciler extends Reconciler $atomic_type->min_bound = max($atomic_type->min_bound, $assertion_value); } $existing_var_type->addType($atomic_type); + } elseif ($atomic_type->isLesserThan($assertion_value)) { + // if the range is lesser than the assertion, the type must be removed + $did_remove_type = true; + $existing_var_type->removeType($atomic_type->getKey()); + } elseif ($atomic_type->isGreaterThan($assertion_value)) { + // if the range is greater than the assertion, the check is redundant } } elseif ($atomic_type instanceof TLiteralInt) { if ($atomic_type->value < $assertion_value) { diff --git a/src/Psalm/Type/Atomic/TIntRange.php b/src/Psalm/Type/Atomic/TIntRange.php index 5571823f2..5b2adcddf 100644 --- a/src/Psalm/Type/Atomic/TIntRange.php +++ b/src/Psalm/Type/Atomic/TIntRange.php @@ -86,6 +86,22 @@ class TIntRange extends TInt ($this->min_bound <= $i && $this->max_bound >= $i); } + /** + * Returns true if every part of the Range is lesser than the given value + */ + public function isLesserThan(int $i): bool + { + return $this->max_bound !== null && $this->max_bound < $i; + } + + /** + * Returns true if every part of the Range is greater than the given value + */ + public function isGreaterThan(int $i): bool + { + return $this->min_bound !== null && $this->min_bound > $i; + } + public static function getNewLowestBound(?int $bound1, ?int $bound2): ?int { if ($bound1 === null || $bound2 === null) { diff --git a/tests/IntRangeTest.php b/tests/IntRangeTest.php index 5b7cf913e..f5eb50bb8 100644 --- a/tests/IntRangeTest.php +++ b/tests/IntRangeTest.php @@ -702,6 +702,36 @@ class IntRangeTest extends TestCase }', 'error_message' => 'InvalidReturnType', ], + 'assertOutOfRange' => [ + ' $a + */ + function scope(int $a): void{ + assert($a === 0); + }', + 'error_message' => 'DocblockTypeContradiction', + ], + 'assertRedundantInferior' => [ + ' $a + */ + function scope(int $a): void{ + assert($a < 10); + }', + 'error_message' => 'RedundantConditionGivenDocblockType', + ], + 'assertImpossibleInferior' => [ + ' $a + */ + function scope(int $a): void{ + assert($a < 4); + }', + 'error_message' => 'DocblockTypeContradiction', + ], ]; } }