Fix #847 - only warn about LessSpecificReturnType when method is not overridden

src/Psalm/Checker/FunctionLike/ReturnTypeChecker.php

@@ -11,6 +11,7 @@ use Psalm\Checker\InterfaceChecker;
 use Psalm\Checker\ProjectChecker;
 use Psalm\Checker\ScopeChecker;
 use Psalm\Checker\Statements\ExpressionChecker;
+use Psalm\Checker\StatementsChecker;
 use Psalm\Checker\TypeChecker;
 use Psalm\CodeLocation;
 use Psalm\Context;
@@ -29,6 +30,7 @@ use Psalm\Issue\MoreSpecificReturnType;
 use Psalm\IssueBuffer;
 use Psalm\StatementsSource;
 use Psalm\Storage\FunctionLikeStorage;
+use Psalm\Storage\MethodStorage;
 use Psalm\Type;
 use Psalm\Type\TypeCombination;
 
@@ -429,15 +431,34 @@ class ReturnTypeChecker
             } elseif ((!$inferred_return_type->isNullable() && $declared_return_type->isNullable())
                 || (!$inferred_return_type->isFalsable() && $declared_return_type->isFalsable())
             ) {
-                if (IssueBuffer::accepts(
-                    new LessSpecificReturnType(
-                        'The inferred return type \'' . $inferred_return_type . '\' for ' . $cased_method_id .
-                            ' is more specific than the declared return type \'' . $declared_return_type . '\'',
-                        $return_type_location
-                    ),
-                    $suppressed_issues
-                )) {
-                    return false;
+                if ($function instanceof Function_
+                    || $function instanceof Closure
+                    || $function->isPrivate()
+                ) {
+                    $check_for_less_specific_type = true;
+                } elseif ($source instanceof StatementsChecker) {
+                    $method_storage = $function_like_checker->getFunctionLikeStorage($source);
+
+                    if ($method_storage instanceof MethodStorage) {
+                        $check_for_less_specific_type = !$method_storage->overridden_somewhere;
+                    } else {
+                        $check_for_less_specific_type = false;
+                    }
+                } else {
+                    $check_for_less_specific_type = false;
+                }
+
+                if ($check_for_less_specific_type) {
+                    if (IssueBuffer::accepts(
+                        new LessSpecificReturnType(
+                            'The inferred return type \'' . $inferred_return_type . '\' for ' . $cased_method_id .
+                                ' is more specific than the declared return type \'' . $declared_return_type . '\'',
+                            $return_type_location
+                        ),
+                        $suppressed_issues
+                    )) {
+                        return false;
+                    }
                 }
             }
 

src/Psalm/Codebase/Populator.php

@@ -576,6 +576,10 @@ class Populator
         foreach ($parent_storage->inheritable_method_ids as $method_name => $declaring_method_id) {
             if (!$parent_storage->is_trait) {
                 $storage->overridden_method_ids[$method_name][] = $declaring_method_id;
+
+                if (isset($storage->methods[$method_name])) {
+                    $storage->methods[$method_name]->overridden_somewhere = true;
+                }
             }
 
             $aliased_method_names = [$method_name];
@@ -614,6 +618,7 @@ class Populator
 
                     // tell the declaring class it's overridden downstream
                     $declaring_class_storage->methods[strtolower($declaring_method_name)]->overridden_downstream = true;
+                    $declaring_class_storage->methods[strtolower($declaring_method_name)]->overridden_somewhere = true;
                 }
             }
         }

src/Psalm/Storage/MethodStorage.php

@@ -39,4 +39,9 @@ class MethodStorage extends FunctionLikeStorage
      * @var bool
      */
     public $overridden_downstream = false;
+
+    /**
+     * @var bool
+     */
+    public $overridden_somewhere = false;
 }

tests/PropertyTypeTest.php

@@ -113,6 +113,39 @@ class PropertyTypeTest extends TestCase
         $this->assertFalse(isset($context->vars_in_scope['$a->parent']));
     }
 
+    /**
+     * @return void
+     */
+    public function testRemoveClauseAfterReassignment()
+    {
+        Config::getInstance()->remember_property_assignments_after_call = false;
+
+        $this->addFile(
+            'somefile.php',
+            '<?php
+                class Test {
+                    /** @var ?bool */
+                    private $foo;
+
+                    public function run(): void {
+                        $this->foo = false;
+                        $this->bar();
+                        if ($this->foo === true) {}
+                    }
+
+                    private function bar(): void {
+                        if (mt_rand(0, 1)) {
+                            $this->foo = true;
+                        }
+                    }
+                }'
+        );
+
+        $context = new Context();
+
+        $this->analyzeFile('somefile.php', $context);
+    }
+
     /**
      * @return array
      */
@@ -928,6 +961,72 @@ class PropertyTypeTest extends TestCase
                     '$d->e' => 'mixed',
                 ],
             ],
+            'allowLessSpecificReturnTypeForOverriddenMethod' => [
+                '<?php
+                    class A {
+                        public function aa(): ?string {
+                            return "bar";
+                        }
+                    }
+
+                    class B extends A {
+                        public static function aa(): ?string {
+                            return rand(0, 1) ? "bar" : null;
+                        }
+                    }
+
+                    class C extends A {
+                        public static function aa(): ?string {
+                            return "bar";
+                        }
+                    }'
+            ],
+            'allowLessSpecificReturnTypeForInterfaceMethod' => [
+                '<?php
+                    interface Foo {
+                        public static function foo(): ?string;
+                    }
+
+                    class Bar implements Foo {
+                        public static function foo(): ?string
+                        {
+                            return "bar";
+                        }
+                    }
+
+                    class Baz implements Foo {
+                        /**
+                         * @return string $baz
+                         */
+                        public static function foo(): ?string
+                        {
+                            return "baz";
+                        }
+                    }
+
+                    class Bax implements Foo {
+                        /**
+                         * @return null|string $baz
+                         */
+                        public static function foo(): ?string
+                        {
+                            return "bax";
+                        }
+                    }
+
+                    class Baw implements Foo {
+                        /**
+                         * @return null|string $baz
+                         */
+                        public static function foo(): ?string
+                        {
+                            /** @var null|string $val */
+                            $val = "baw";
+
+                            return $val;
+                        }
+                    }',
+            ],
         ];
     }