Fix vimeo#4374 - prevent paradox and allow Psalm to understand more a…

…ssignments in conditionals
danog · Jan 29, 2021 · 7df404b · 7df404b
1 parent 26352d0
commit 7df404b
Show file tree

Hide file tree

Showing 3 changed files with 59 additions and 25 deletions.
diff --git a/dictionaries/CallMap.php b/dictionaries/CallMap.php
@@ -476,7 +476,7 @@
 'asin' => ['float', 'number'=>'float'],
 'asinh' => ['float', 'number'=>'float'],
 'asort' => ['bool', '&rw_array'=>'array', 'sort_flags='=>'int'],
-'assert' => ['bool', 'assertion'=>'string|bool', 'description='=>'string|Throwable|null'],
+'assert' => ['bool', 'assertion'=>'string|bool|int', 'description='=>'string|Throwable|null'],
 'assert_options' => ['mixed|false', 'what'=>'int', 'value='=>'mixed'],
 'ast\get_kind_name' => ['string', 'kind'=>'int'],
 'ast\get_metadata' => ['array<int,ast\Metadata>'],

diff --git a/src/Psalm/Internal/Analyzer/Statements/Block/IfAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Block/IfAnalyzer.php
@@ -79,10 +79,14 @@ public static function analyze(
         $if_scope = new IfScope();
 
         // We need to clone the original context for later use if we're exiting in this if conditional
-        if (!$stmt->else && !$stmt->elseifs && $stmt->cond instanceof PhpParser\Node\Expr\BinaryOp\BooleanOr) {
+        if (!$stmt->else && !$stmt->elseifs
+            && ($stmt->cond instanceof PhpParser\Node\Expr\BinaryOp
+                || ($stmt->cond instanceof PhpParser\Node\Expr\BooleanNot
+                    && $stmt->cond->expr instanceof PhpParser\Node\Expr\BinaryOp))
+        ) {
             $final_actions = ScopeAnalyzer::getControlActions(
                 $stmt->stmts,
-                $statements_analyzer->node_data,
+                null,
                 $codebase->config->exit_functions,
                 $context->break_types
             );
@@ -881,7 +885,6 @@ protected static function analyzeIfBlock(
         if ($has_leaving_statements && !$has_break_statement && !$stmt->else && !$stmt->elseifs) {
             // If we're assigning inside
             if ($if_conditional_scope->cond_assigned_var_ids
-                && $stmt->cond instanceof PhpParser\Node\Expr\BinaryOp\BooleanOr
                 && $if_scope->mic_drop_context
             ) {
                 self::addConditionallyAssignedVarsToContext(
@@ -1866,23 +1869,23 @@ public static function addConditionallyAssignedVarsToContext(
         $old_node_data = $statements_analyzer->node_data;
         $statements_analyzer->node_data = clone $old_node_data;
 
-        $suppressed_issues = $statements_analyzer->getSuppressedIssues();
-
-        if (!in_array('RedundantCondition', $suppressed_issues, true)) {
-            $statements_analyzer->addSuppressedIssues(['RedundantCondition']);
-        }
-        if (!in_array('RedundantConditionGivenDocblockType', $suppressed_issues, true)) {
-            $statements_analyzer->addSuppressedIssues(['RedundantConditionGivenDocblockType']);
-        }
-        if (!in_array('TypeDoesNotContainType', $suppressed_issues, true)) {
-            $statements_analyzer->addSuppressedIssues(['TypeDoesNotContainType']);
-        }
+        IssueBuffer::startRecording();
 
         foreach ($exprs as $expr) {
+            if ($expr instanceof PhpParser\Node\Expr\BinaryOp\BooleanAnd) {
+                $fake_not = new PhpParser\Node\Expr\BinaryOp\BooleanOr(
+                    self::negateExpr($expr->left),
+                    self::negateExpr($expr->right),
+                    $expr->getAttributes()
+                );
+            } else {
+                $fake_not = self::negateExpr($expr);
+            }
+
             $fake_negated_expr = new PhpParser\Node\Expr\FuncCall(
                 new PhpParser\Node\Name\FullyQualified('assert'),
                 [new PhpParser\Node\Arg(
-                    new PhpParser\Node\Expr\BooleanNot($expr, $expr->getAttributes()),
+                    $fake_not,
                     false,
                     false,
                     $expr->getAttributes()
@@ -1901,15 +1904,8 @@ public static function addConditionallyAssignedVarsToContext(
             $mic_drop_context->inside_negation = !$mic_drop_context->inside_negation;
         }
 
-        if (!in_array('RedundantCondition', $suppressed_issues, true)) {
-            $statements_analyzer->removeSuppressedIssues(['RedundantCondition']);
-        }
-        if (!in_array('RedundantConditionGivenDocblockType', $suppressed_issues, true)) {
-            $statements_analyzer->removeSuppressedIssues(['RedundantConditionGivenDocblockType']);
-        }
-        if (!in_array('TypeDoesNotContainType', $suppressed_issues, true)) {
-            $statements_analyzer->removeSuppressedIssues(['TypeDoesNotContainType']);
-        }
+        IssueBuffer::clearRecordingLevel();
+        IssueBuffer::stopRecording();
 
         $statements_analyzer->node_data = $old_node_data;
 
@@ -1919,4 +1915,13 @@ public static function addConditionallyAssignedVarsToContext(
             }
         }
     }
+
+    private static function negateExpr(PhpParser\Node\Expr $expr) : PhpParser\Node\Expr
+    {
+        if ($expr instanceof PhpParser\Node\Expr\BooleanNot) {
+            return $expr->expr;
+        }
+
+        return new PhpParser\Node\Expr\BooleanNot($expr, $expr->getAttributes());
+    }
 }
diff --git a/tests/TypeReconciliation/ConditionalTest.php b/tests/TypeReconciliation/ConditionalTest.php
@@ -2942,6 +2942,22 @@ function bar(?U $a, ?U $b) : void {
                         ) {}
                     }'
             ],
+            'assertWithAssignmentInOr' => [
+                'function test(int $x = null): int {
+                    \assert($x || ($x = rand(0, 10)));
+                    return $x;
+                }'
+            ],
+            'noParadoxicalConditionAfterTwoAssignments' => [
+                '<?php
+                    function foo(string $str): ?int {
+                        if (rand(0, 1) || (!($pos = strpos($str, "a")) && !($pos = strpos($str, "b")))) {
+                            return null;
+                        }
+
+                        return $pos;
+                    }'
+            ],
         ];
     }
 
@@ -3322,6 +3338,19 @@ function foo(Exception $e) : void {
                     }',
                 'error_message' => 'TypeDoesNotContainType',
             ],
+            'assignmentInBranchOfAnd' => [
+                '<?php
+                    function foo(string $str): ?int {
+                        $pos = 5;
+
+                        if (rand(0, 1) && !($pos = $str)) {
+                            return null;
+                        }
+
+                        return $pos;
+                    }',
+                'error_message' => 'InvalidReturnStatement',
+            ],
         ];
     }
 }