diff --git a/src/Psalm/Internal/Algebra.php b/src/Psalm/Internal/Algebra.php index 0f581fc850f..3574841eaa0 100644 --- a/src/Psalm/Internal/Algebra.php +++ b/src/Psalm/Internal/Algebra.php @@ -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; @@ -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 $clauses * @param array $cond_referenced_var_ids * @param array>> $active_truths @@ -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 cannot be array + 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; } diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/AssertionFinder.php b/src/Psalm/Internal/Analyzer/Statements/Expression/AssertionFinder.php index 3396deae0e1..fdfa804bc29 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/AssertionFinder.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/AssertionFinder.php @@ -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>>> */ @@ -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)) @@ -2373,6 +2404,7 @@ private static function getTrueInequalityAssertions( } } + /** @psalm-suppress LessSpecificReturnStatement */ return $if_types; } @@ -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>>> */ @@ -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)) @@ -3066,6 +3132,7 @@ private static function getFalseEqualityAssertions( } } + /** @psalm-suppress LessSpecificReturnStatement */ return $if_types; } diff --git a/tests/AssertAnnotationTest.php b/tests/AssertAnnotationTest.php index 249051b1174..e8fa215d212 100644 --- a/tests/AssertAnnotationTest.php +++ b/tests/AssertAnnotationTest.php @@ -2511,6 +2511,348 @@ function testLeftMethodTernary(Either $either): void $either->isLeft() ? testLeft($either) : testRight($either); }', ], + 'assertArrayListIfTrueFalseCompareTrue' => [ + 'code' => '|list $arg + * @return bool + * + * @psalm-assert-if-false array $arg + * @psalm-assert-if-true list $arg + */ + function is_array_or_list($arg) { + // should be array_is_list($arg), but tests run in non-PHP 8 environment + if (array_values($arg) === $arg) { + return true; + } + return false; + } + /** + * @param list $arg + * @return void + */ + function takesAList($arg) {} + /** + * @param array $arg + * @return void + */ + function takesAnArray($arg) {} + /** + * @var array|list $foo + */ + $foo; + if (is_array_or_list($foo) === true) { + takesAList($foo); + } else { + takesAnArray($foo); + }', + ], + 'assertArrayListIfTrueFalseCompareTruthy' => [ + 'code' => '|list $arg + * @return bool + * + * @psalm-assert-if-false array $arg + * @psalm-assert-if-true list $arg + */ + function is_array_or_list($arg) { + // should be array_is_list($arg), but tests run in non-PHP 8 environment + if (array_values($arg) === $arg) { + return true; + } + return false; + } + /** + * @param list $arg + * @return void + */ + function takesAList($arg) {} + /** + * @param array $arg + * @return void + */ + function takesAnArray($arg) {} + /** + * @var array|list $foo + */ + $foo; + if (is_array_or_list($foo)) { + takesAList($foo); + } else { + takesAnArray($foo); + }', + ], + 'assertArrayListIfTrueFalseCompareNotTrue' => [ + 'code' => '|list $arg + * @return bool + * + * @psalm-assert-if-false array $arg + * @psalm-assert-if-true list $arg + */ + function is_array_or_list($arg) { + if (array_values($arg) === $arg) { + return true; + } + return false; + } + /** + * @param list $arg + * @return void + */ + function takesAList($arg) {} + /** + * @param array $arg + * @return void + */ + function takesAnArray($arg) {} + /** + * @var array|list $foo + */ + $foo; + if (is_array_or_list($foo) !== true) { + takesAnArray($foo); + } else { + takesAList($foo); + }', + ], + 'assertArrayListIfTrueFalseCompareFalse' => [ + 'code' => '|list $arg + * @return bool + * + * @psalm-assert-if-false array $arg + * @psalm-assert-if-true list $arg + */ + function is_array_or_list($arg) { + if (array_values($arg) === $arg) { + return true; + } + return false; + } + /** + * @param list $arg + * @return void + */ + function takesAList($arg) {} + /** + * @param array $arg + * @return void + */ + function takesAnArray($arg) {} + /** + * @var array|list $foo + */ + $foo; + if (is_array_or_list($foo) === false) { + takesAnArray($foo); + } else { + takesAList($foo); + }', + ], + 'assertArrayListIfTrueFalseCompareNotFalse' => [ + 'code' => '|list $arg + * @return bool + * + * @psalm-assert-if-false array $arg + * @psalm-assert-if-true list $arg + */ + function is_array_or_list($arg) { + if (array_values($arg) === $arg) { + return true; + } + return false; + } + /** + * @param list $arg + * @return void + */ + function takesAList($arg) {} + /** + * @param array $arg + * @return void + */ + function takesAnArray($arg) {} + /** + * @var array|list $foo + */ + $foo; + if (is_array_or_list($foo) !== false) { + takesAList($foo); + } else { + takesAnArray($foo); + }', + ], + 'assertArrayListIfTrueFalseCompareFalsy' => [ + 'code' => '|list $arg + * @return bool + * + * @psalm-assert-if-false array $arg + * @psalm-assert-if-true list $arg + */ + function is_array_or_list($arg) { + if (array_values($arg) === $arg) { + return true; + } + return false; + } + /** + * @param list $arg + * @return void + */ + function takesAList($arg) {} + /** + * @param array $arg + * @return void + */ + function takesAnArray($arg) {} + /** + * @var array|list $foo + */ + $foo; + if (!is_array_or_list($foo)) { + takesAnArray($foo); + } else { + takesAList($foo); + }', + ], + 'assertArrayArrayIfTrueFalseCompareFalsy' => [ + 'code' => '|array $arg + * @return bool + * + * @psalm-suppress InvalidReturnType + * + * @psalm-assert-if-false array $arg + * @psalm-assert-if-true array $arg + */ + function is_array_a_or_b($arg) {} + /** + * @param array $arg + * @return void + */ + function takesAnArrayA($arg) {} + /** + * @param array $arg + * @return void + */ + function takesAnArrayB($arg) {} + /** + * @var array|array $foo + */ + $foo; + if (!is_array_a_or_b($foo)) { + takesAnArrayA($foo); + } else { + takesAnArrayB($foo); + }', + ], + 'assertListListIfTrueFalseCompareFalsy' => [ + 'code' => '|list $arg + * @return bool + * + * @psalm-suppress InvalidReturnType + * + * @psalm-assert-if-false list $arg + * @psalm-assert-if-true list $arg + */ + function is_list_string_or_int($arg) {} + /** + * @param list $arg + * @return void + */ + function takesAListString($arg) {} + /** + * @param list $arg + * @return void + */ + function takesAListInt($arg) {} + /** + * @var list|list $foo + */ + $foo; + if (!is_list_string_or_int($foo)) { + takesAListString($foo); + } else { + takesAListInt($foo); + }', + ], + 'assertKeyedArrayKeyedArrayIfTrueFalseCompareFalsy' => [ + 'code' => ' [ + 'code' => '