Skip to content

Commit

Permalink
Merge pull request #7511 from orklah/literal-inequality
Browse files Browse the repository at this point in the history 
improve literal inequality with ranges and rework GreaterThan/LessThan assertions
  • Loading branch information
@orklah
orklah committed Jan 28, 2022
2 parents 4609bc4 + 66ccf93 commit 048025b
Show file tree
Hide file tree
Showing 11 changed files with 241 additions and 87 deletions.
71 changes: 32 additions & 39 deletions src/Psalm/Internal/Analyzer/Statements/Expression/AssertionFinder.php
View file
Expand Up @@ -51,9 +51,11 @@
use Psalm\Storage\Assertion\IsCountable;
use Psalm\Storage\Assertion\IsEqualIsset;
use Psalm\Storage\Assertion\IsGreaterThan;
use Psalm\Storage\Assertion\IsGreaterThanOrEqualTo;
use Psalm\Storage\Assertion\IsIdentical;
use Psalm\Storage\Assertion\IsIsset;
use Psalm\Storage\Assertion\IsLessThan;
use Psalm\Storage\Assertion\IsLessThanOrEqualTo;
use Psalm\Storage\Assertion\IsLooselyEqual;
use Psalm\Storage\Assertion\IsNotIdentical;
use Psalm\Storage\Assertion\IsNotLooselyEqual;
Expand Down Expand Up @@ -1644,8 +1646,7 @@ protected static function hasCountEqualityCheck(
protected static function hasSuperiorNumberCheck(
FileSource $source,
PhpParser\Node\Expr\BinaryOp $conditional,
?int &$literal_value_comparison,
bool &$isset_assert
?int &$literal_value_comparison
) {
$right_assignment = false;
$value_right = null;
Expand All @@ -1666,10 +1667,7 @@ protected static function hasSuperiorNumberCheck(
$value_right = $conditional->right->expr->value;
}
if ($right_assignment === true && $value_right !== null) {
$isset_assert = $value_right === 0 && $conditional instanceof Greater;

$literal_value_comparison = $value_right +
($conditional instanceof PhpParser\Node\Expr\BinaryOp\Greater ? 1 : 0);
$literal_value_comparison = $value_right;

return self::ASSIGNMENT_TO_RIGHT;
}
Expand All @@ -1693,10 +1691,7 @@ protected static function hasSuperiorNumberCheck(
$value_left = $conditional->left->expr->value;
}
if ($left_assignment === true && $value_left !== null) {
$isset_assert = $value_left === 0 && $conditional instanceof Greater;

$literal_value_comparison = $value_left +
($conditional instanceof PhpParser\Node\Expr\BinaryOp\Greater ? -1 : 0);
$literal_value_comparison = $value_left;

return self::ASSIGNMENT_TO_LEFT;
}
Expand All @@ -1711,8 +1706,7 @@ protected static function hasSuperiorNumberCheck(
protected static function hasInferiorNumberCheck(
FileSource $source,
PhpParser\Node\Expr\BinaryOp $conditional,
?int &$literal_value_comparison,
bool &$isset_assert
?int &$literal_value_comparison
) {
$right_assignment = false;
$value_right = null;
Expand All @@ -1733,10 +1727,8 @@ protected static function hasInferiorNumberCheck(
$value_right = $conditional->right->expr->value;
}
if ($right_assignment === true && $value_right !== null) {
$isset_assert = $value_right === 0 && $conditional instanceof Smaller;
$literal_value_comparison = $value_right;

$literal_value_comparison = $value_right +
($conditional instanceof PhpParser\Node\Expr\BinaryOp\Smaller ? -1 : 0);
return self::ASSIGNMENT_TO_RIGHT;
}

Expand All @@ -1759,10 +1751,7 @@ protected static function hasInferiorNumberCheck(
$value_left = $conditional->left->expr->value;
}
if ($left_assignment === true && $value_left !== null) {
$isset_assert = $value_left === 0 && $conditional instanceof Smaller;

$literal_value_comparison = $value_left +
($conditional instanceof PhpParser\Node\Expr\BinaryOp\Smaller ? 1 : 0);
$literal_value_comparison = $value_left;

return self::ASSIGNMENT_TO_LEFT;
}
Expand Down Expand Up @@ -3788,13 +3777,11 @@ private static function getGreaterAssertions(
$count_equality_position = self::hasNonEmptyCountEqualityCheck($conditional, $min_count);
$max_count = null;
$count_inequality_position = self::hasLessThanCountEqualityCheck($conditional, $max_count);
$isset_assert = false;
$superior_value_comparison = null;
$superior_value_position = self::hasSuperiorNumberCheck(
$source,
$conditional,
$superior_value_comparison,
$isset_assert
$superior_value_comparison
);

if ($count_equality_position) {
Expand Down Expand Up @@ -3851,7 +3838,7 @@ private static function getGreaterAssertions(
return $if_types ? [$if_types] : [];
}

if ($superior_value_position) {
if ($superior_value_position && $superior_value_comparison !== null) {
if ($superior_value_position === self::ASSIGNMENT_TO_RIGHT) {
$var_name = ExpressionIdentifier::getArrayVarId(
$conditional->left,
Expand All @@ -3868,13 +3855,17 @@ private static function getGreaterAssertions(

if ($var_name !== null) {
if ($superior_value_position === self::ASSIGNMENT_TO_RIGHT) {
$if_types[$var_name] = [[new IsGreaterThan($superior_value_comparison)]];
if ($conditional instanceof GreaterOrEqual) {
$if_types[$var_name] = [[new IsGreaterThanOrEqualTo($superior_value_comparison)]];
} else {
$if_types[$var_name] = [[new IsGreaterThan($superior_value_comparison)]];
}
} else {
$if_types[$var_name] = [[new IsLessThan($superior_value_comparison)]];
}

if ($isset_assert) {
$if_types[$var_name][] = [new IsEqualIsset()];
if ($conditional instanceof GreaterOrEqual) {
$if_types[$var_name] = [[new IsLessThanOrEqualTo($superior_value_comparison)]];
} else {
$if_types[$var_name] = [[new IsLessThan($superior_value_comparison)]];
}
}
}

Expand All @@ -3898,13 +3889,11 @@ private static function getSmallerAssertions(
$count_equality_position = self::hasNonEmptyCountEqualityCheck($conditional, $min_count);
$max_count = null;
$count_inequality_position = self::hasLessThanCountEqualityCheck($conditional, $max_count);
$isset_assert = false;
$inferior_value_comparison = null;
$inferior_value_position = self::hasInferiorNumberCheck(
$source,
$conditional,
$inferior_value_comparison,
$isset_assert
$inferior_value_comparison
);

if ($count_equality_position) {
Expand Down Expand Up @@ -3973,15 +3962,19 @@ private static function getSmallerAssertions(
}


if ($var_name !== null) {
if ($var_name !== null && $inferior_value_comparison !== null) {
if ($inferior_value_position === self::ASSIGNMENT_TO_RIGHT) {
$if_types[$var_name] = [[new IsLessThan($inferior_value_comparison)]];
if ($conditional instanceof SmallerOrEqual) {
$if_types[$var_name] = [[new IsLessThanOrEqualTo($inferior_value_comparison)]];
} else {
$if_types[$var_name] = [[new IsLessThan($inferior_value_comparison)]];
}
} else {
$if_types[$var_name] = [[new IsGreaterThan($inferior_value_comparison)]];
}

if ($isset_assert) {
$if_types[$var_name][] = [new IsEqualIsset()];
if ($conditional instanceof SmallerOrEqual) {
$if_types[$var_name] = [[new IsGreaterThanOrEqualTo($inferior_value_comparison)]];
} else {
$if_types[$var_name] = [[new IsGreaterThan($inferior_value_comparison)]];
}
}
}

Expand Down
2 changes: 0 additions & 2 deletions src/Psalm/Internal/Analyzer/Statements/Expression/Call/ArrayFunctionArgumentsAnalyzer.php
View file
Expand Up @@ -547,7 +547,6 @@ public static function handleByRefArrayAdjustment(
]
);
} else {
/** @psalm-suppress InvalidPropertyAssignmentValue */
$array_atomic_type->count--;
}
} else {
Expand All @@ -565,7 +564,6 @@ public static function handleByRefArrayAdjustment(
]
);
} else {
/** @psalm-suppress InvalidPropertyAssignmentValue */
$array_atomic_type->count--;
}
} else {
Expand Down
38 changes: 38 additions & 0 deletions src/Psalm/Internal/Type/NegatedAssertionReconciler.php
View file
Expand Up @@ -328,6 +328,44 @@ private static function handleLiteralNegatedEquality(
$did_remove_type = true;
}
}

$existing_range_types = $existing_var_type->getRangeInts();

if ($existing_range_types) {
foreach ($existing_range_types as $int_key => $literal_type) {
if ($literal_type->contains($assertion_type->value)) {
$did_remove_type = true;
$existing_var_type->removeType($int_key);
if ($literal_type->min_bound === null
|| $literal_type->min_bound <= $assertion_type->value - 1
) {
$existing_var_type->addType(new Type\Atomic\TIntRange(
$literal_type->min_bound,
$assertion_type->value - 1
));
}
if ($literal_type->max_bound === null
|| $literal_type->max_bound >= $assertion_type->value + 1
) {
$existing_var_type->addType(new Type\Atomic\TIntRange(
$assertion_type->value + 1,
$literal_type->max_bound
));
}
}
}
}

if (isset($existing_var_type->getAtomicTypes()['int'])
&& get_class($existing_var_type->getAtomicTypes()['int']) === Type\Atomic\TInt::class
) {
$did_remove_type = true;
//this may be used to generate a range containing any int except the one that was asserted against
//but this is failing some tests
/*$existing_var_type->removeType('int');
$existing_var_type->addType(new Type\Atomic\TIntRange(null, $assertion_type->value - 1));
$existing_var_type->addType(new Type\Atomic\TIntRange($assertion_type->value + 1, null));*/
}
} else {
$scalar_var_type = clone $assertion_type;
}
Expand Down
32 changes: 18 additions & 14 deletions src/Psalm/Internal/Type/SimpleAssertionReconciler.php
View file
Expand Up @@ -153,7 +153,7 @@ public static function reconcile(
}

if ($assertion instanceof IsGreaterThan) {
return self::reconcileSuperiorTo(
return self::reconcileIsGreaterThan(
$assertion,
$existing_var_type,
$inside_loop,
Expand All @@ -166,7 +166,7 @@ public static function reconcile(
}

if ($assertion instanceof IsLessThan) {
return self::reconcileInferiorTo(
return self::reconcileIsLessThan(
$assertion,
$existing_var_type,
$inside_loop,
Expand Down Expand Up @@ -1612,7 +1612,7 @@ private static function reconcileHasArrayKey(
/**
* @param string[] $suppressed_issues
*/
private static function reconcileSuperiorTo(
private static function reconcileIsGreaterThan(
IsGreaterThan $assertion,
Union $existing_var_type,
bool $inside_loop,
Expand All @@ -1622,19 +1622,21 @@ private static function reconcileSuperiorTo(
?CodeLocation $code_location,
array $suppressed_issues
): Union {
$assertion_value = $assertion->value;
//we add 1 from the assertion value because we're on a strict operator
$assertion_value = $assertion->value + 1;

$did_remove_type = false;

if ($existing_var_type->hasType('null') && $assertion->doesFilterNull()) {
$did_remove_type = true;
$existing_var_type->removeType('null');
}

foreach ($existing_var_type->getAtomicTypes() as $atomic_type) {
if ($inside_loop) {
continue;
}

if ($assertion_value === null) {
continue;
}

if ($atomic_type instanceof TIntRange) {
if ($atomic_type->contains($assertion_value)) {
// if the range contains the assertion, the range must be adapted
Expand Down Expand Up @@ -1715,7 +1717,7 @@ private static function reconcileSuperiorTo(
/**
* @param string[] $suppressed_issues
*/
private static function reconcileInferiorTo(
private static function reconcileIsLessThan(
IsLessThan $assertion,
Union $existing_var_type,
bool $inside_loop,
Expand All @@ -1725,19 +1727,21 @@ private static function reconcileInferiorTo(
?CodeLocation $code_location,
array $suppressed_issues
): Union {
$assertion_value = $assertion->value;
//we remove 1 from the assertion value because we're on a strict operator
$assertion_value = $assertion->value - 1;

$did_remove_type = false;

if ($existing_var_type->hasType('null') && $assertion->doesFilterNull()) {
$did_remove_type = true;
$existing_var_type->removeType('null');
}

foreach ($existing_var_type->getAtomicTypes() as $atomic_type) {
if ($inside_loop) {
continue;
}

if ($assertion_value === null) {
continue;
}

if ($atomic_type instanceof TIntRange) {
if ($atomic_type->contains($assertion_value)) {
// if the range contains the assertion, the range must be adapted
Expand Down
50 changes: 26 additions & 24 deletions src/Psalm/Internal/Type/SimpleNegatedAssertionReconciler.php
View file
Expand Up @@ -1653,22 +1653,23 @@ private static function reconcileResource(
*/
private static function reconcileIsLessThanOrEqualTo(
IsLessThanOrEqualTo $assertion,
Union $existing_var_type,
bool $inside_loop,
string $old_var_type_string,
?string $var_id,
bool $negated,
?CodeLocation $code_location,
array $suppressed_issues
Union $existing_var_type,
bool $inside_loop,
string $old_var_type_string,
?string $var_id,
bool $negated,
?CodeLocation $code_location,
array $suppressed_issues
): Union {
if ($assertion->value === null) {
return $existing_var_type;
}

$assertion_value = $assertion->value - 1;
$assertion_value = $assertion->value;

$did_remove_type = false;

if ($existing_var_type->hasType('null') && $assertion->doesFilterNull()) {
$did_remove_type = true;
$existing_var_type->removeType('null');
}

foreach ($existing_var_type->getAtomicTypes() as $atomic_type) {
if ($inside_loop) {
continue;
Expand Down Expand Up @@ -1756,22 +1757,23 @@ private static function reconcileIsLessThanOrEqualTo(
*/
private static function reconcileIsGreaterThanOrEqualTo(
IsGreaterThanOrEqualTo $assertion,
Union $existing_var_type,
bool $inside_loop,
string $old_var_type_string,
?string $var_id,
bool $negated,
?CodeLocation $code_location,
array $suppressed_issues
Union $existing_var_type,
bool $inside_loop,
string $old_var_type_string,
?string $var_id,
bool $negated,
?CodeLocation $code_location,
array $suppressed_issues
): Union {
if ($assertion->value === null) {
return $existing_var_type;
}

$assertion_value = $assertion->value + 1;
$assertion_value = $assertion->value;

$did_remove_type = false;

if ($existing_var_type->hasType('null') && $assertion->doesFilterNull()) {
$did_remove_type = true;
$existing_var_type->removeType('null');
}

foreach ($existing_var_type->getAtomicTypes() as $atomic_type) {
if ($inside_loop) {
continue;
Expand Down

0 comments on commit 048025b

Please sign in to comment.