Do not consider isset checks on static properties as redundant (#5525)

* Do not consider isset checks on static properties as redundant Unlike normal properties, static properties do not have a prescribed initialization sequence, so they can always be uninitialized (or unset). Thus `isset()` checks on them are never redundant. Fixes #5489 * Fix issue with nullable is_static
vimeo · Apr 25, 2021 · 933822e · 933822e
1 parent 85a0ef0
commit 933822e
Show file tree

Hide file tree

Showing 7 changed files with 45 additions and 0 deletions.
diff --git a/src/Psalm/Internal/Analyzer/ClassAnalyzer.php b/src/Psalm/Internal/Analyzer/ClassAnalyzer.php
@@ -802,13 +802,15 @@ public static function addContextProperties(
                 ) {
                     $property_type->initialized = false;
                     $property_type->from_property = true;
+                    $property_type->from_static_property = $property_storage->is_static === true;
                 }
             } else {
                 $property_type = Type::getMixed();
 
                 if (!$property_storage->has_default && !$property_storage->is_promoted) {
                     $property_type->initialized = false;
                     $property_type->from_property = true;
+                    $property_type->from_static_property = $property_storage->is_static === true;
                 }
             }
 

diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/AssignmentAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/AssignmentAnalyzer.php
@@ -255,6 +255,7 @@ public static function analyze(
             if ($assign_value_type) {
                 $assign_value_type = clone $assign_value_type;
                 $assign_value_type->from_property = false;
+                $assign_value_type->from_static_property = false;
                 $assign_value_type->ignore_isset = false;
             } else {
                 $assign_value_type = Type::getMixed();

diff --git a/src/Psalm/Internal/Type/NegatedAssertionReconciler.php b/src/Psalm/Internal/Type/NegatedAssertionReconciler.php
@@ -82,6 +82,7 @@ public static function reconcile(
                     && $key
                     && strpos($key, '[') === false
                     && $key !== '$_SESSION'
+                    && !$existing_var_type->from_static_property
                 ) {
                     foreach ($existing_var_type->getAtomicTypes() as $atomic) {
                         if (!$existing_var_type->hasMixed()

diff --git a/src/Psalm/Internal/Type/SimpleAssertionReconciler.php b/src/Psalm/Internal/Type/SimpleAssertionReconciler.php
@@ -434,6 +434,7 @@ private static function reconcileIsset(
             && (!$did_remove_type || empty($existing_var_type->getAtomicTypes()))
             && $key
             && $code_location
+            && !$existing_var_type->from_static_property
         ) {
             self::triggerIssueForImpossible(
                 $existing_var_type,
@@ -458,6 +459,7 @@ private static function reconcileIsset(
         }
 
         $existing_var_type->from_property = false;
+        $existing_var_type->from_static_property = false;
         $existing_var_type->possibly_undefined = false;
         $existing_var_type->possibly_undefined_from_try = false;
         $existing_var_type->ignore_isset = false;

diff --git a/src/Psalm/Internal/Type/TypeExpander.php b/src/Psalm/Internal/Type/TypeExpander.php
@@ -78,6 +78,7 @@ public static function expandUnion(
         $fleshed_out_type->by_ref = $return_type->by_ref;
         $fleshed_out_type->initialized = $return_type->initialized;
         $fleshed_out_type->from_property = $return_type->from_property;
+        $fleshed_out_type->from_static_property = $return_type->from_static_property;
         $fleshed_out_type->had_template = $return_type->had_template;
         $fleshed_out_type->parent_nodes = $return_type->parent_nodes;
 

diff --git a/src/Psalm/Type/Union.php b/src/Psalm/Type/Union.php
@@ -56,6 +56,16 @@ class Union implements TypeNode
      */
     public $from_property = false;
 
+    /**
+     * Whether the type originated from *static* property
+     *
+     * Unlike non-static properties, static properties have no prescribed place
+     * like __construct() to be initialized in
+     *
+     * @var bool
+     */
+    public $from_static_property = false;
+
     /**
      * Whether the property that this type has been derived from has been initialized in a constructor
      *

diff --git a/tests/TypeReconciliation/IssetTest.php b/tests/TypeReconciliation/IssetTest.php
@@ -1006,6 +1006,34 @@ function foo(array $test) : void {
                         echo $test[0];
                     }'
             ],
+            'issetOnStaticProperty' => [
+                '<?php
+                    class Singleton {
+                        private static self $instance;
+                        public function getInstance(): self {
+                            if (isset(self::$instance)) {
+                                return self::$instance;
+                            }
+                            return self::$instance = new self();
+                        }
+                        private function __construct() {}
+                    }
+                ',
+            ],
+            'negatedIssetOnStaticProperty' => [
+                '<?php
+                    class Singleton {
+                        private static self $instance;
+                        public function getInstance(): self {
+                            if (!isset(self::$instance)) {
+                                self::$instance = new self();
+                            }
+                            return self::$instance;
+                        }
+                        private function __construct() {}
+                    }
+                ',
+            ],
         ];
     }