Merge pull request #7261 from vimeo/muglug-improve-negated-reconcilia…

…tion-logic Improve negated reconciliation logic
vimeo · Jan 1, 2022 · 6c176bb · 6c176bb
2 parents 2a6f122 + 9663dc5
commit 6c176bb
Show file tree

Hide file tree

Showing 4 changed files with 116 additions and 123 deletions.
diff --git a/psalm-baseline.xml b/psalm-baseline.xml
@@ -1,5 +1,5 @@
 <?xml version="1.0" encoding="UTF-8"?>
-<files psalm-version="dev-master@76bb8bc65567d2bc600fd4925c812589917b6ab1">
+<files psalm-version="dev-master@16a61a758e44158c19c779c8b0a1c2143e40d83e">
   <file src="examples/TemplateChecker.php">
     <PossiblyUndefinedIntArrayOffset occurrences="2">
       <code>$comment_block-&gt;tags['variablesfrom'][0]</code>
@@ -186,8 +186,7 @@
     </DeprecatedMethod>
   </file>
   <file src="src/Psalm/Internal/Analyzer/Statements/Expression/Call/MethodCallAnalyzer.php">
-    <PossiblyUndefinedIntArrayOffset occurrences="7">
-      <code>$result-&gt;existent_method_ids[0]</code>
+    <PossiblyUndefinedIntArrayOffset occurrences="6">
       <code>$result-&gt;invalid_method_call_types[0]</code>
       <code>$result-&gt;non_existent_class_method_ids[0]</code>
       <code>$result-&gt;non_existent_class_method_ids[0]</code>
@@ -399,13 +398,6 @@
       <code>new TEmpty()</code>
     </DeprecatedClass>
   </file>
-  <file src="src/Psalm/Internal/Type/NegatedAssertionReconciler.php">
-    <DeprecatedMethod occurrences="3">
-      <code>Type::getEmpty()</code>
-      <code>Type::getEmpty()</code>
-      <code>Type::getEmpty()</code>
-    </DeprecatedMethod>
-  </file>
   <file src="src/Psalm/Internal/Type/SimpleAssertionReconciler.php">
     <DeprecatedClass occurrences="2">
       <code>new TEmpty()</code>
@@ -432,7 +424,10 @@
       <code>new TEmpty</code>
       <code>new TEmpty</code>
     </DeprecatedClass>
-    <DeprecatedMethod occurrences="1">
+    <DeprecatedMethod occurrences="4">
+      <code>Type::getEmpty()</code>
+      <code>Type::getEmpty()</code>
+      <code>Type::getEmpty()</code>
       <code>Type::getEmpty()</code>
     </DeprecatedMethod>
   </file>

diff --git a/src/Psalm/Internal/Type/NegatedAssertionReconciler.php b/src/Psalm/Internal/Type/NegatedAssertionReconciler.php
@@ -7,10 +7,6 @@
 use Psalm\Internal\Analyzer\TraitAnalyzer;
 use Psalm\Internal\Type\Comparator\AtomicTypeComparator;
 use Psalm\Internal\Type\Comparator\UnionTypeComparator;
-use Psalm\Issue\DocblockTypeContradiction;
-use Psalm\Issue\RedundantPropertyInitializationCheck;
-use Psalm\Issue\TypeDoesNotContainType;
-use Psalm\IssueBuffer;
 use Psalm\Type;
 use Psalm\Type\Atomic\TArray;
 use Psalm\Type\Atomic\TEmptyMixed;
@@ -20,7 +16,6 @@
 use Psalm\Type\Atomic\TInt;
 use Psalm\Type\Atomic\TIterable;
 use Psalm\Type\Atomic\TNamedObject;
-use Psalm\Type\Atomic\TNonEmptyMixed;
 use Psalm\Type\Atomic\TNonEmptyString;
 use Psalm\Type\Atomic\TString;
 use Psalm\Type\Atomic\TTrue;
@@ -82,113 +77,6 @@ public static function reconcile(
             return $existing_var_type;
         }
 
-        if (!$is_equality) {
-            if ($assertion === 'isset') {
-                if ($existing_var_type->possibly_undefined) {
-                    return Type::getEmpty();
-                }
-
-                if (!$existing_var_type->isNullable()
-                    && $key
-                    && strpos($key, '[') === false
-                ) {
-                    foreach ($existing_var_type->getAtomicTypes() as $atomic) {
-                        if (!$existing_var_type->hasMixed()
-                            || $atomic instanceof TNonEmptyMixed
-                        ) {
-                            $failed_reconciliation = Reconciler::RECONCILIATION_EMPTY;
-
-                            if ($code_location) {
-                                if ($existing_var_type->from_static_property) {
-                                    IssueBuffer::maybeAdd(
-                                        new RedundantPropertyInitializationCheck(
-                                            'Static property ' . $key . ' with type '
-                                                . $existing_var_type
-                                                . ' has unexpected isset check — should it be nullable?',
-                                            $code_location
-                                        ),
-                                        $suppressed_issues
-                                    );
-                                } elseif ($existing_var_type->from_property) {
-                                    IssueBuffer::maybeAdd(
-                                        new RedundantPropertyInitializationCheck(
-                                            'Property ' . $key . ' with type '
-                                                . $existing_var_type . ' should already be set in the constructor',
-                                            $code_location
-                                        ),
-                                        $suppressed_issues
-                                    );
-                                } elseif ($existing_var_type->from_docblock) {
-                                    IssueBuffer::maybeAdd(
-                                        new DocblockTypeContradiction(
-                                            'Cannot resolve types for ' . $key . ' with docblock-defined type '
-                                                . $existing_var_type . ' and !isset assertion',
-                                            $code_location,
-                                            null
-                                        ),
-                                        $suppressed_issues
-                                    );
-                                } else {
-                                    IssueBuffer::maybeAdd(
-                                        new TypeDoesNotContainType(
-                                            'Cannot resolve types for ' . $key . ' with type '
-                                                . $existing_var_type . ' and !isset assertion',
-                                            $code_location,
-                                            null
-                                        ),
-                                        $suppressed_issues
-                                    );
-                                }
-                            }
-
-                            return $existing_var_type->from_docblock
-                                ? Type::getNull()
-                                : Type::getEmpty();
-                        }
-                    }
-                }
-
-                return Type::getNull();
-            }
-
-            if ($assertion === 'array-key-exists') {
-                return Type::getEmpty();
-            }
-
-            if (strpos($assertion, 'in-array-') === 0) {
-                $assertion = substr($assertion, 9);
-                $new_var_type = Type::parseString($assertion);
-
-                $intersection = Type::intersectUnionTypes(
-                    $new_var_type,
-                    $existing_var_type,
-                    $statements_analyzer->getCodebase()
-                );
-
-                if ($intersection === null) {
-                    if ($key && $code_location) {
-                        self::triggerIssueForImpossible(
-                            $existing_var_type,
-                            $existing_var_type->getId(),
-                            $key,
-                            '!' . $assertion,
-                            true,
-                            $negated,
-                            $code_location,
-                            $suppressed_issues
-                        );
-                    }
-
-                    $failed_reconciliation = Reconciler::RECONCILIATION_EMPTY;
-                }
-
-                return $existing_var_type;
-            }
-
-            if (strpos($assertion, 'has-array-key-') === 0) {
-                return $existing_var_type;
-            }
-        }
 
         $existing_var_atomic_types = $existing_var_type->getAtomicTypes();
 
@@ -200,6 +88,7 @@ public static function reconcile(
             $existing_var_type->addType(new TFalse);
         } else {
             $simple_negated_type = SimpleNegatedAssertionReconciler::reconcile(
+                $statements_analyzer->getCodebase(),
                 $assertion,
                 $existing_var_type,
                 $key,

diff --git a/src/Psalm/Internal/Type/SimpleNegatedAssertionReconciler.php b/src/Psalm/Internal/Type/SimpleNegatedAssertionReconciler.php
@@ -3,7 +3,12 @@
 namespace Psalm\Internal\Type;
 
 use Psalm\CodeLocation;
+use Psalm\Codebase;
 use Psalm\Internal\Codebase\InternalCallMapHandler;
+use Psalm\Issue\DocblockTypeContradiction;
+use Psalm\Issue\RedundantPropertyInitializationCheck;
+use Psalm\Issue\TypeDoesNotContainType;
+use Psalm\IssueBuffer;
 use Psalm\Type;
 use Psalm\Type\Atomic\Scalar;
 use Psalm\Type\Atomic\TArray;
@@ -57,6 +62,7 @@ class SimpleNegatedAssertionReconciler extends Reconciler
      * @param  0|1|2      $failed_reconciliation
      */
     public static function reconcile(
+        Codebase $codebase,
         string $assertion,
         Union $existing_var_type,
         ?string $key = null,
@@ -67,6 +73,108 @@ public static function reconcile(
         bool $is_equality = false,
         bool $inside_loop = false
     ): ?Union {
+        if ($assertion === 'isset') {
+            if ($existing_var_type->possibly_undefined) {
+                return Type::getEmpty();
+            }
+
+            if (!$existing_var_type->isNullable()
+                && $key
+                && strpos($key, '[') === false
+            ) {
+                foreach ($existing_var_type->getAtomicTypes() as $atomic) {
+                    if (!$existing_var_type->hasMixed()
+                        || $atomic instanceof TNonEmptyMixed
+                    ) {
+                        $failed_reconciliation = Reconciler::RECONCILIATION_EMPTY;
+
+                        if ($code_location) {
+                            if ($existing_var_type->from_static_property) {
+                                IssueBuffer::maybeAdd(
+                                    new RedundantPropertyInitializationCheck(
+                                        'Static property ' . $key . ' with type '
+                                            . $existing_var_type
+                                            . ' has unexpected isset check — should it be nullable?',
+                                        $code_location
+                                    ),
+                                    $suppressed_issues
+                                );
+                            } elseif ($existing_var_type->from_property) {
+                                IssueBuffer::maybeAdd(
+                                    new RedundantPropertyInitializationCheck(
+                                        'Property ' . $key . ' with type '
+                                            . $existing_var_type . ' should already be set in the constructor',
+                                        $code_location
+                                    ),
+                                    $suppressed_issues
+                                );
+                            } elseif ($existing_var_type->from_docblock) {
+                                IssueBuffer::maybeAdd(
+                                    new DocblockTypeContradiction(
+                                        'Cannot resolve types for ' . $key . ' with docblock-defined type '
+                                            . $existing_var_type . ' and !isset assertion',
+                                        $code_location,
+                                        null
+                                    ),
+                                    $suppressed_issues
+                                );
+                            } else {
+                                IssueBuffer::maybeAdd(
+                                    new TypeDoesNotContainType(
+                                        'Cannot resolve types for ' . $key . ' with type '
+                                            . $existing_var_type . ' and !isset assertion',
+                                        $code_location,
+                                        null
+                                    ),
+                                    $suppressed_issues
+                                );
+                            }
+                        }
+
+                        return $existing_var_type->from_docblock
+                            ? Type::getNull()
+                            : Type::getEmpty();
+                    }
+                }
+            }
+
+            return Type::getNull();
+        }
+
+        if ($assertion === 'array-key-exists') {
+            return Type::getEmpty();
+        }
+
+        if (strpos($assertion, 'in-array-') === 0) {
+            $assertion = substr($assertion, 9);
+            $new_var_type = Type::parseString($assertion);
+
+            $intersection = Type::intersectUnionTypes(
+                $new_var_type,
+                $existing_var_type,
+                $codebase
+            );
+
+            if ($intersection === null) {
+                if ($key && $code_location) {
+                    self::triggerIssueForImpossible(
+                        $existing_var_type,
+                        $existing_var_type->getId(),
+                        $key,
+                        '!' . $assertion,
+                        true,
+                        $negated,
+                        $code_location,
+                        $suppressed_issues
+                    );
+                }
+
+                $failed_reconciliation = Reconciler::RECONCILIATION_EMPTY;
+            }
+
+            return $existing_var_type;
+        }
+
         if ($assertion === 'object' && !$existing_var_type->hasMixed()) {
             return self::reconcileObject(
                 $existing_var_type,

diff --git a/src/Psalm/Internal/Type/TypeExpander.php b/src/Psalm/Internal/Type/TypeExpander.php
@@ -811,6 +811,7 @@ private static function expandConditional(
                 );
 
                 $else_conditional_return_type = SimpleNegatedAssertionReconciler::reconcile(
+                    $codebase,
                     $assertion,
                     $else_conditional_return_type
                 );