Fix #3631 - better treatment for assignments in complex conditionals

vimeo · Jun 24, 2020 · 7a7cd91 · 7a7cd91
1 parent 9aa0aca
commit 7a7cd91
Show file tree

Hide file tree

Showing 6 changed files with 86 additions and 18 deletions.
diff --git a/src/Psalm/Internal/Analyzer/FunctionAnalyzer.php b/src/Psalm/Internal/Analyzer/FunctionAnalyzer.php
@@ -193,8 +193,7 @@ public static function getReturnTypeFromCallMapWithArgs(
                                     return clone $array_type->type_param;
                                 }
                             } elseif ($first_arg_type->hasScalarType()
-                                && isset($call_args[1])
-                                && ($second_arg = $call_args[1]->value)
+                                && ($second_arg = ($call_args[1]->value ?? null))
                                 && ($second_arg_type = $statements_analyzer->node_data->getType($second_arg))
                                 && $second_arg_type->hasScalarType()
                             ) {

diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/AssertionFinder.php b/src/Psalm/Internal/Analyzer/Statements/Expression/AssertionFinder.php
@@ -123,6 +123,33 @@ public static function scrapeAssertions(
             return $if_types;
         }
 
+        if ($conditional instanceof PhpParser\Node\Expr\Assign) {
+            $var_name = ExpressionIdentifier::getArrayVarId(
+                $conditional->var,
+                $this_class_name,
+                $source
+            );
+
+            $candidate_if_types = self::scrapeAssertions(
+                $conditional->expr,
+                $this_class_name,
+                $source,
+                $codebase,
+                $inside_negation,
+                $cache
+            );
+
+            if ($var_name) {
+                if ($candidate_if_types) {
+                    $if_types[$var_name] = [['>' . \json_encode($candidate_if_types)]];
+                } else {
+                    $if_types[$var_name] = [['!falsy']];
+                }
+            }
+
+            return $if_types;
+        }
+
         $var_name = ExpressionIdentifier::getArrayVarId(
             $conditional,
             $this_class_name,
@@ -139,20 +166,6 @@ public static function scrapeAssertions(
             }
         }
 
-        if ($conditional instanceof PhpParser\Node\Expr\Assign) {
-            $var_name = ExpressionIdentifier::getArrayVarId(
-                $conditional->var,
-                $this_class_name,
-                $source
-            );
-
-            if ($var_name) {
-                $if_types[$var_name] = [['!falsy']];
-            }
-
-            return $if_types;
-        }
-
         if ($conditional instanceof PhpParser\Node\Expr\BooleanNot) {
             $expr_assertions = null;
 

diff --git a/...Psalm/Internal/Analyzer/Statements/Expression/Call/Method/MethodCallReturnTypeFetcher.php b/...Psalm/Internal/Analyzer/Statements/Expression/Call/Method/MethodCallReturnTypeFetcher.php
@@ -85,8 +85,7 @@ public static function fetch(
 
         if (InternalCallMapHandler::inCallMap((string) $call_map_id)) {
             if (($template_result->upper_bounds || $class_storage->stubbed)
-                && isset($class_storage->methods[$method_id->method_name])
-                && ($method_storage = $class_storage->methods[$method_id->method_name])
+                && ($method_storage = ($class_storage->methods[$method_id->method_name] ?? null))
                 && $method_storage->return_type
             ) {
                 $return_type_candidate = clone $method_storage->return_type;

diff --git a/src/Psalm/Internal/Type/AssertionReconciler.php b/src/Psalm/Internal/Type/AssertionReconciler.php
@@ -81,6 +81,11 @@ public static function reconcile(
             $is_equality = true;
         }
 
+        if ($assertion[0] === '>') {
+            $assertion = 'falsy';
+            $is_negation = true;
+        }
+
         if ($existing_var_type === null
             && is_string($key)
             && VariableFetchAnalyzer::isSuperGlobal($key)

diff --git a/src/Psalm/Type/Reconciler.php b/src/Psalm/Type/Reconciler.php
@@ -265,6 +265,25 @@ public static function reconcileKeyedTypes(
                 $orred_type = null;
 
                 foreach ($new_type_part_parts as $new_type_part_part) {
+                    if ($new_type_part_part[0] === '>') {
+                        /** @var array<string, array<array<string>>> */
+                        $data = \json_decode(substr($new_type_part_part, 1), true);
+
+                        $existing_types = self::reconcileKeyedTypes(
+                            $data,
+                            $data,
+                            $existing_types,
+                            $changed_var_ids,
+                            $referenced_var_ids,
+                            $statements_analyzer,
+                            $template_type_map,
+                            $inside_loop,
+                            $code_location
+                        );
+
+                        $new_type_part_part = '!falsy';
+                    }
+
                     $result_type_candidate = AssertionReconciler::reconcile(
                         $new_type_part_part,
                         $result_type ? clone $result_type : null,

diff --git a/tests/TypeReconciliation/ConditionalTest.php b/tests/TypeReconciliation/ConditionalTest.php
@@ -2793,6 +2793,39 @@ public static function compare(A $other_type) : AChild {
 
                             return $other_type;
                         }
+                    }',
+            ],
+            'applyTruthyAssertionsToRightHandSideOfAssignment' => [
+                '<?php
+                    function takesAString(string $name): void {}
+
+                    function randomReturn(): ?string {
+                        return rand(1,2) === 1 ? "foo" : null;
+                    }
+
+                    $name = randomReturn();
+
+                    if ($foo = ($name !== null)) {
+                        takesAString($name);
+                    }'
+            ],
+            'maintainTruthinessInsideAssignment' => [
+                '<?php
+                    class C {
+                        public function foo() : void {}
+                    }
+
+                    class B {
+                        public ?C $c = null;
+                    }
+
+                    function updateBackgroundClip(?B $b): void {
+                        if (!$b || !($a = $b->c)) {
+                            // do something
+                        } else {
+                            /** @psalm-suppress MixedMethodCall */
+                            $a->foo();
+                        }
                     }'
             ],
         ];