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 literal inequality with ranges and rework GreaterThan/LessThan assertions #7511

Merged
merged 2 commits into from Jan 28, 2022
Merged

improve literal inequality with ranges and rework GreaterThan/LessThan assertions #7511

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3b73654
handle literal inequality with ints
orklah Jan 24, 2022
66ccf93
fix CS
orklah Jan 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
71 changes: 32 additions & 39 deletions src/Psalm/Internal/Analyzer/Statements/Expression/AssertionFinder.php
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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