Improve error message for #5489

vimeo · Apr 25, 2021 · 4f5c205 · 4f5c205
1 parent 933822e
commit 4f5c205
Show file tree

Hide file tree

Showing 4 changed files with 62 additions and 40 deletions.
diff --git a/src/Psalm/Internal/Type/NegatedAssertionReconciler.php b/src/Psalm/Internal/Type/NegatedAssertionReconciler.php
@@ -82,7 +82,6 @@ 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()
@@ -91,7 +90,18 @@ public static function reconcile(
                             $failed_reconciliation = 2;
 
                             if ($code_location) {
-                                if ($existing_var_type->from_property) {
+                                if ($existing_var_type->from_static_property) {
+                                    if (IssueBuffer::accepts(
+                                        new RedundantPropertyInitializationCheck(
+                                            'Static property type ' . $key . ' with type '
+                                                . $existing_var_type . ' has unexpected isset check — should it be nullable?',
+                                            $code_location
+                                        ),
+                                        $suppressed_issues
+                                    )) {
+                                        // fall through
+                                    }
+                                } elseif ($existing_var_type->from_property) {
                                     if (IssueBuffer::accepts(
                                         new RedundantPropertyInitializationCheck(
                                             'Property type ' . $key . ' with type '

diff --git a/src/Psalm/Internal/Type/SimpleAssertionReconciler.php b/src/Psalm/Internal/Type/SimpleAssertionReconciler.php
@@ -434,7 +434,6 @@ 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,

diff --git a/src/Psalm/Type/Reconciler.php b/src/Psalm/Type/Reconciler.php
@@ -891,15 +891,28 @@ protected static function triggerIssueForImpossible(
 
         if ($redundant) {
             if ($existing_var_type->from_property && $assertion === 'isset') {
-                if (IssueBuffer::accepts(
-                    new RedundantPropertyInitializationCheck(
-                        'Property type ' . $key . ' with type '
-                            . $old_var_type_string . ' should already be set in the constructor',
-                        $code_location
-                    ),
-                    $suppressed_issues
-                )) {
-                    // fall through
+                if ($existing_var_type->from_static_property) {
+                    if (IssueBuffer::accepts(
+                        new RedundantPropertyInitializationCheck(
+                            'Static property type ' . $key . ' with type '
+                                . $old_var_type_string . ' has unexpected isset check — should it be nullable?',
+                            $code_location
+                        ),
+                        $suppressed_issues
+                    )) {
+                        // fall through
+                    }
+                } else {
+                    if (IssueBuffer::accepts(
+                        new RedundantPropertyInitializationCheck(
+                            'Property type ' . $key . ' with type '
+                                . $old_var_type_string . ' should already be set in the constructor',
+                            $code_location
+                        ),
+                        $suppressed_issues
+                    )) {
+                        // fall through
+                    }
                 }
             } elseif ($from_docblock) {
                 if (IssueBuffer::accepts(

diff --git a/tests/TypeReconciliation/IssetTest.php b/tests/TypeReconciliation/IssetTest.php
@@ -1006,34 +1006,6 @@ 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() {}
-                    }
-                ',
-            ],
         ];
     }
 
@@ -1144,6 +1116,34 @@ function foo() : void {
                     }',
                 'error_message' => 'PossiblyInvalidArrayAccess',
             ],
+            '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() {}
+                    }',
+                'error_message' => 'RedundantPropertyInitializationCheck',
+            ],
+            '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() {}
+                    }',
+                'error_message' => 'RedundantPropertyInitializationCheck',
+            ],
         ];
     }
 }