Skip to content

Commit

Permalink
Merge pull request #9853 from kkmuffme/multiple-psalm-assert-if-for-a…
Browse files Browse the repository at this point in the history
…rray-list

Allow if/false assert for same variable to allow array/list distinction
  • Loading branch information
orklah committed Jun 4, 2023
2 parents a82e7fc + fa644cb commit c158605
Show file tree
Hide file tree
Showing 3 changed files with 486 additions and 6 deletions.
71 changes: 71 additions & 0 deletions src/Psalm/Internal/Algebra.php
Expand Up @@ -4,6 +4,9 @@

use Psalm\Exception\ComplicatedExpressionException;
use Psalm\Storage\Assertion;
use Psalm\Type\Atomic\TArray;
use Psalm\Type\Atomic\TKeyedArray;
use Psalm\Type\Atomic\TList;
use UnexpectedValueException;

use function array_filter;
Expand Down Expand Up @@ -322,6 +325,8 @@ public static function simplifyCNF(array $clauses): array
/**
* Look for clauses with only one possible value
*
* doesn't infer the "unset" correctly
* @psalm-suppress MoreSpecificReturnType
* @param list<Clause> $clauses
* @param array<string, bool> $cond_referenced_var_ids
* @param array<string, array<int, array<int, Assertion>>> $active_truths
Expand Down Expand Up @@ -388,6 +393,72 @@ public static function getTruthsFromFormula(
}
}

foreach ($truths as $var => $anded_types) {
$has_list_or_array = false;
foreach ($anded_types as $orred_types) {
foreach ($orred_types as $assertion) {
if ($assertion->isNegation()) {
continue;
}

if (!isset($assertion->type)) {
continue;
}

if ($assertion->type instanceof TList
|| $assertion->type instanceof TArray
|| $assertion->type instanceof TKeyedArray) {
$has_list_or_array = true;
// list/array are collapsed, therefore there can only be 1 and we can abort
// otherwise we would have to remove them all individually
// e.g. array<string, string> cannot be array<int, float>
break 2;
}
}
}

if ($has_list_or_array === false) {
continue;
}

foreach ($anded_types as $key => $orred_types) {
foreach ($orred_types as $index => $assertion) {
// we only need to check negations
// due to type collapsing, any negations for arrays are irrelevant
if (!$assertion->isNegation()) {
continue;
}

if (!isset($assertion->type)) {
continue;
}

if ($assertion->type instanceof TList
|| $assertion->type instanceof TArray
|| $assertion->type instanceof TKeyedArray) {
unset($truths[$var][$key][$index]);
}
}

/**
* doesn't infer the "unset" correctly
*
* @psalm-suppress DocblockTypeContradiction
*/
if ($truths[$var][$key] === []) {
unset($truths[$var][$key]);
} else {
/**
* doesn't infer the "unset" correctly
*
* @psalm-suppress RedundantFunctionCallGivenDocblockType
*/
$truths[$var][$key] = array_values($truths[$var][$key]);
}
}
}

/** @psalm-suppress LessSpecificReturnStatement */
return $truths;
}

Expand Down
Expand Up @@ -2254,6 +2254,7 @@ private static function getFalseInequalityAssertions(
}

/**
* @psalm-suppress MoreSpecificReturnType
* @param PhpParser\Node\Expr\BinaryOp\NotIdentical|PhpParser\Node\Expr\BinaryOp\NotEqual $conditional
* @return list<non-empty-array<string, non-empty-list<non-empty-list<Assertion>>>>
*/
Expand Down Expand Up @@ -2325,15 +2326,45 @@ private static function getTrueInequalityAssertions(
}

if (count($notif_types) === 1) {
$notif_types = $notif_types[0];
$notif_type = $notif_types[0];

if (count($notif_types) === 1) {
$if_types = Algebra::negateTypes($notif_types);
if (count($notif_type) === 1) {
$if_types = Algebra::negateTypes($notif_type);
}
}

$if_types = $if_types ? [$if_types] : [];

if ($if_types === [] && count($notif_types) === 2) {
$check_var_assertion = null;
$check_var = null;
foreach ($notif_types as $notif_type) {
foreach ($notif_type as $var => $assertions) {
if (count($assertions) !== 1 || count($assertions[0]) !== 1) {
$if_types = [];
break 2;
}

$is_not_assertion = $assertions[0][0] instanceof IsNotType ? true : false;
if (!isset($check_var)) {
$check_var_assertion = $is_not_assertion;
$check_var = $var;
continue;
}

// only if we have 1 IsType and 1 IsNotType assertion for same variable
if ($check_var !== $var
|| !isset($check_var_assertion)
|| $check_var_assertion === $is_not_assertion) {
$if_types = [];
break 2;
}
}

$if_types[] = Algebra::negateTypes($notif_type);
}
}

if ($codebase
&& $source instanceof StatementsAnalyzer
&& ($var_type = $source->node_data->getType($base_conditional))
Expand Down Expand Up @@ -2373,6 +2404,7 @@ private static function getTrueInequalityAssertions(
}
}

/** @psalm-suppress LessSpecificReturnStatement */
return $if_types;
}

Expand Down Expand Up @@ -2951,6 +2983,7 @@ private static function getTrueEqualityAssertions(
}

/**
* @psalm-suppress MoreSpecificReturnType
* @param PhpParser\Node\Expr\BinaryOp\Identical|PhpParser\Node\Expr\BinaryOp\Equal $conditional
* @return list<non-empty-array<string, non-empty-list<non-empty-list<Assertion>>>>
*/
Expand Down Expand Up @@ -3022,15 +3055,48 @@ private static function getFalseEqualityAssertions(
}

if (count($notif_types) === 1) {
$notif_types = $notif_types[0];
$notif_type = $notif_types[0];

if (count($notif_types) === 1) {
$if_types = Algebra::negateTypes($notif_types);
if (count($notif_type) === 1) {
$if_types = Algebra::negateTypes($notif_type);
}
}

$if_types = $if_types ? [$if_types] : [];

// @psalm-assert-if-true and @psalm-assert-if-false for same variable in same function, e.g. array/list cases
// @todo optionally extend this to arbitrary number of assert-if cases of multiple variables in the function
// same code above too
if ($if_types === [] && count($notif_types) === 2) {
$check_var_assertion = null;
$check_var = null;
foreach ($notif_types as $notif_type) {
foreach ($notif_type as $var => $assertions) {
if (count($assertions) !== 1 || count($assertions[0]) !== 1) {
$if_types = [];
break 2;
}

$is_not_assertion = $assertions[0][0] instanceof IsNotType ? true : false;
if (!isset($check_var)) {
$check_var_assertion = $is_not_assertion;
$check_var = $var;
continue;
}

// only if we have 1 IsType and 1 IsNotType assertion for same variable
if ($check_var !== $var
|| !isset($check_var_assertion)
|| $check_var_assertion === $is_not_assertion) {
$if_types = [];
break 2;
}
}

$if_types[] = Algebra::negateTypes($notif_type);
}
}

if ($codebase
&& $source instanceof StatementsAnalyzer
&& ($var_type = $source->node_data->getType($base_conditional))
Expand Down Expand Up @@ -3066,6 +3132,7 @@ private static function getFalseEqualityAssertions(
}
}

/** @psalm-suppress LessSpecificReturnStatement */
return $if_types;
}

Expand Down

0 comments on commit c158605

Please sign in to comment.