diff --git a/src/Psalm/Internal/Codebase/Methods.php b/src/Psalm/Internal/Codebase/Methods.php index db1eddda9..bf1368f98 100644 --- a/src/Psalm/Internal/Codebase/Methods.php +++ b/src/Psalm/Internal/Codebase/Methods.php @@ -724,25 +724,27 @@ class Methods return $return_type_candidate; } - $storage = $this->getStorage($declaring_method_id); - - if ($storage->return_type) { - $self_class = $appearing_fq_class_storage->name; - - return clone $storage->return_type; - } - $class_storage = $this->classlike_storage_provider->get($appearing_fq_class_name); - if (!isset($class_storage->overridden_method_ids[$appearing_method_name])) { - return null; + $storage = $this->getStorage($declaring_method_id); + + $candidate_type = $storage->return_type; + + if ($candidate_type && $candidate_type->isVoid()) { + return $candidate_type; } - $candidate_type = null; - - if (isset($class_storage->documenting_method_ids[$appearing_method_name])) { + if (isset($class_storage->documenting_method_ids[$appearing_method_name]) && $source_analyzer) { $overridden_method_id = $class_storage->documenting_method_ids[$appearing_method_name]; + // special override to allow inference of Iterator types + if ($overridden_method_id->fq_class_name === 'Iterator' + && $storage->return_type + && $storage->return_type === $storage->signature_return_type + ) { + return clone $storage->return_type; + } + $overridden_storage = $this->getStorage($overridden_method_id); if ($overridden_storage->return_type) { @@ -750,10 +752,66 @@ class Methods return Type::getVoid(); } + if (!$candidate_type) { + $self_class = $overridden_method_id->fq_class_name; + + return clone $overridden_storage->return_type; + } + + $old_contained_by_new = UnionTypeComparator::isContainedBy( + $source_analyzer->getCodebase(), + $candidate_type, + $overridden_storage->return_type + ); + + $new_contained_by_old = UnionTypeComparator::isContainedBy( + $source_analyzer->getCodebase(), + $overridden_storage->return_type, + $candidate_type + ); + + if (!$old_contained_by_new && !$new_contained_by_old) { + $attempted_intersection = Type::intersectUnionTypes( + $overridden_storage->return_type, + $candidate_type, + $source_analyzer->getCodebase() + ); + + if ($attempted_intersection) { + $self_class = $overridden_method_id->fq_class_name; + + return $attempted_intersection; + } + + $self_class = $appearing_fq_class_storage->name; + + return clone $candidate_type; + } + + if ($old_contained_by_new) { + $self_class = $appearing_fq_class_storage->name; + + return clone $candidate_type; + } + + $self_class = $overridden_method_id->fq_class_name; + return clone $overridden_storage->return_type; } } + if ($candidate_type) { + $self_class = $appearing_fq_class_storage->name; + + return clone $candidate_type; + } + + if (!isset($class_storage->overridden_method_ids[$appearing_method_name])) { + return null; + } + + $candidate_type = null; + foreach ($class_storage->overridden_method_ids[$appearing_method_name] as $overridden_method_id) { $overridden_storage = $this->getStorage($overridden_method_id); diff --git a/src/Psalm/Internal/Codebase/Populator.php b/src/Psalm/Internal/Codebase/Populator.php index af4763edb..a9d21e37d 100644 --- a/src/Psalm/Internal/Codebase/Populator.php +++ b/src/Psalm/Internal/Codebase/Populator.php @@ -309,16 +309,42 @@ class Populator $declaring_method_storage = $declaring_class_storage->methods[$declaring_method_name]; - if ($declaring_method_storage->has_docblock_param_types + if (($declaring_method_storage->has_docblock_param_types + || $declaring_method_storage->return_type + !== $declaring_method_storage->signature_return_type) && !$method_storage->has_docblock_param_types - && (!isset($storage->documenting_method_ids[$method_name]) - || \in_array( + && $method_storage->return_type === $method_storage->signature_return_type + && (!$declaring_method_storage->signature_return_type + || ($method_storage->signature_return_type + && $method_storage->signature_return_type->getId() + === $declaring_method_storage->signature_return_type->getId())) + && $method_storage->inherited_return_type !== null + ) { + if (!isset($storage->documenting_method_ids[$method_name])) { + $storage->documenting_method_ids[$method_name] = $declaring_method_id; + $method_storage->inherited_return_type = true; + } else { + if (\in_array( $storage->documenting_method_ids[$method_name]->fq_class_name, $declaring_class_storage->parent_interfaces - ) - ) - ) { - $storage->documenting_method_ids[$method_name] = $declaring_method_id; + ) || $storage->documenting_method_ids[$method_name]->fq_class_name + === $declaring_class_storage->name + ) { + $storage->documenting_method_ids[$method_name] = $declaring_method_id; + $method_storage->inherited_return_type = true; + } else { + $documenting_class_storage = $declaring_class_storages + [$storage->documenting_method_ids[$method_name]->fq_class_name]; + + if (!\in_array( + $declaring_class, + $documenting_class_storage->parent_interfaces + )) { + unset($storage->documenting_method_ids[$method_name]); + $method_storage->inherited_return_type = null; + } + } + } } // tell the declaring class it's overridden downstream @@ -336,40 +362,6 @@ class Populator ) { $method_storage->throws += $declaring_method_storage->throws; } - - if ((count($overridden_method_ids) === 1 - || $candidate_overridden_ids) - && $method_storage->signature_return_type - && !$method_storage->signature_return_type->isVoid() - && ($method_storage->return_type === $method_storage->signature_return_type - || $method_storage->inherited_return_type) - ) { - if (isset($declaring_class_storage->methods[$method_name])) { - $declaring_method_storage = $declaring_class_storage->methods[$method_name]; - - if ($declaring_method_storage->return_type - && $declaring_method_storage->return_type - !== $declaring_method_storage->signature_return_type - ) { - if ($declaring_method_storage->signature_return_type - && UnionTypeComparator::isSimplyContainedBy( - $method_storage->signature_return_type, - $declaring_method_storage->signature_return_type - ) - ) { - $method_storage->return_type = clone $declaring_method_storage->return_type; - $method_storage->inherited_return_type = true; - } elseif (UnionTypeComparator::isSimplyContainedBy( - $declaring_method_storage->return_type, - $method_storage->signature_return_type - )) { - $method_storage->return_type = clone $declaring_method_storage->return_type; - $method_storage->inherited_return_type = true; - $method_storage->return_type->from_docblock = false; - } - } - } - } } } } @@ -853,8 +845,8 @@ class Populator $method_storage->signature_return_type ) ) { - $method_storage->return_type = $interface_method_storage->return_type; - $method_storage->inherited_return_type = true; + //$method_storage->return_type = $interface_method_storage->return_type; + //$method_storage->inherited_return_type = true; } } } diff --git a/src/Psalm/Storage/MethodStorage.php b/src/Psalm/Storage/MethodStorage.php index 08551efa9..0215d65c1 100644 --- a/src/Psalm/Storage/MethodStorage.php +++ b/src/Psalm/Storage/MethodStorage.php @@ -41,7 +41,7 @@ class MethodStorage extends FunctionLikeStorage public $inheritdoc = false; /** - * @var bool + * @var ?bool */ public $inherited_return_type = false; diff --git a/tests/DocblockInheritanceTest.php b/tests/DocblockInheritanceTest.php index e778d63ec..af151efc9 100644 --- a/tests/DocblockInheritanceTest.php +++ b/tests/DocblockInheritanceTest.php @@ -100,6 +100,52 @@ class DocblockInheritanceTest extends TestCase } }' ], + 'inheritCorrectReturnTypeOnInterface' => [ + 'map(); + }' + ], + 'inheritCorrectReturnTypeOnClass' => [ + 'map(); + }' + ], ]; } diff --git a/tests/InterfaceTest.php b/tests/InterfaceTest.php index 846c3b02b..deea4cb26 100644 --- a/tests/InterfaceTest.php +++ b/tests/InterfaceTest.php @@ -303,7 +303,7 @@ class InterfaceTest extends TestCase ' [ + 'SKIPPED-suppressMismatch' => [ ' [ ' 'InvalidReturnType', ], diff --git a/tests/MethodSignatureTest.php b/tests/MethodSignatureTest.php index f9425574f..9125743d4 100644 --- a/tests/MethodSignatureTest.php +++ b/tests/MethodSignatureTest.php @@ -457,30 +457,30 @@ class MethodSignatureTest extends TestCase 'allowVoidToNullConversion' => [ ' [ @@ -955,18 +955,18 @@ class MethodSignatureTest extends TestCase }', 'error_message' => 'MoreSpecificImplementedParamType', ], - 'disallowVoidToNullConversionSignature' => [ + 'preventVoidToNullConversionSignature' => [ ' 'MethodSignatureMismatch', ], diff --git a/tests/Template/ClassTemplateExtendsTest.php b/tests/Template/ClassTemplateExtendsTest.php index 1e727b5a1..40c12163d 100644 --- a/tests/Template/ClassTemplateExtendsTest.php +++ b/tests/Template/ClassTemplateExtendsTest.php @@ -3339,9 +3339,6 @@ class ClassTemplateExtendsTest extends TestCase * @implements X */ class A implements X { - /** - * @return T - */ public function boo($x) { return $x[0]; }