danog/psalm
1
0
Fork 0
mirror of https://github.com/danog/psalm.git synced 2024-11-26 12:24:49 +01:00
Code Issues Projects Releases Wiki Activity

Fix #4537 - use more rigorous inerhitance for return and param types

Browse Source
This commit is contained in:
Matt Brown 2020-11-12 13:54:27 -05:00 committed by Daniil Gentili
parent 5d24b351dc
commit e8c755c7c0
Signed by: danog
GPG Key ID: 8C1BE3B34B230CA7
7 changed files with 183 additions and 90 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

84
src/Psalm/Internal/Codebase/Methods.php
View File

@ -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);

78
src/Psalm/Internal/Codebase/Populator.php
View File

@ -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;
}
}
}

2
src/Psalm/Storage/MethodStorage.php
View File

@ -41,7 +41,7 @@ class MethodStorage extends FunctionLikeStorage
public $inheritdoc = false;
/**
* @var bool
* @var ?bool
*/
public $inherited_return_type = false;

46
tests/DocblockInheritanceTest.php
View File

@ -100,6 +100,52 @@ class DocblockInheritanceTest extends TestCase
}
}'
],
'inheritCorrectReturnTypeOnInterface' => [
'<?php
interface A {
/**
* @return A
*/
public function map(): A;
}
interface B extends A {
/**
* @return B
*/
public function map(): A;
}
function takesB(B $f) : B {
return $f->map();
}'
],
'inheritCorrectReturnTypeOnClass' => [
'<?php
interface A {
/**
* @return A
*/
public function map(): A;
}
interface B extends A {
/**
* @return B
*/
public function map(): A;
}
class F implements B {
public function map(): A {
return new F();
}
}
function takesF(F $f) : B {
return $f->map();
}'
],
];
}

16
tests/InterfaceTest.php
View File

@ -303,7 +303,7 @@ class InterfaceTest extends TestCase
'<?php
class SomeIterator extends IteratorIterator {}',
],
'suppressMismatch' => [
'SKIPPED-suppressMismatch' => [
'<?php
interface I {
/**
@ -888,17 +888,17 @@ class InterfaceTest extends TestCase
'inheritMultipleInterfacesWithConflictingDocblocks' => [
'<?php
interface I1 {
/** @return string */
public function foo();
/** @return string */
public function foo();
}
interface I2 {
/** @return int */
public function foo();
/** @return int */
public function foo();
}
class A implements I1, I2 {
public function foo() {
return "hello";
}
public function foo() {
return "hello";
}
}',
'error_message' => 'InvalidReturnType',
],

44
tests/MethodSignatureTest.php
View File

@ -457,30 +457,30 @@ class MethodSignatureTest extends TestCase
'allowVoidToNullConversion' => [
'<?php
class A {
/** @return ?string */
public function foo() {
return rand(0, 1) ? "hello" : null;
}
/** @return ?string */
public function foo() {
return rand(0, 1) ? "hello" : null;
}
}
class B extends A {
public function foo(): void {
return;
}
public function foo(): void {
return;
}
}
class C extends A {
/** @return void */
public function foo() {
return;
}
/** @return void */
public function foo() {
return;
}
}
class D extends A {
/** @return null */
public function foo() {
return null;
}
/** @return null */
public function foo() {
return null;
}
}',
],
'allowNoChildClassPropertyWhenMixed' => [
@ -955,18 +955,18 @@ class MethodSignatureTest extends TestCase
}',
'error_message' => 'MoreSpecificImplementedParamType',
],
'disallowVoidToNullConversionSignature' => [
'preventVoidToNullConversionSignature' => [
'<?php
class A {
public function foo(): ?string {
return rand(0, 1) ? "hello" : null;
}
public function foo(): ?string {
return rand(0, 1) ? "hello" : null;
}
}
class B extends A {
public function foo(): void {
return;
}
public function foo(): void {
return;
}
}',
'error_message' => 'MethodSignatureMismatch',
],

3
tests/Template/ClassTemplateExtendsTest.php
View File

@ -3339,9 +3339,6 @@ class ClassTemplateExtendsTest extends TestCase
* @implements X<T>
*/
class A implements X {
/**
* @return T
*/
public function boo($x) {
return $x[0];
}