From 35c167060295ccad1a43d1ea0cded7b176d3a912 Mon Sep 17 00:00:00 2001 From: Brown Date: Thu, 27 Feb 2020 18:42:15 -0500 Subject: [PATCH] Fix #2875 - treat intersections more similarly than before --- .../Internal/Analyzer/MethodAnalyzer.php | 73 ++++++++------ .../Analyzer/Statements/ReturnAnalyzer.php | 2 +- src/Psalm/Internal/Analyzer/TypeAnalyzer.php | 98 ++++++++++++------- .../Internal/Type/AssertionReconciler.php | 19 ++-- src/Psalm/Type/Atomic/CallableTrait.php | 2 +- src/Psalm/Type/Atomic/TGenericObject.php | 5 +- src/Psalm/Type/Atomic/TIterable.php | 6 +- src/Psalm/Type/Atomic/TNamedObject.php | 4 +- src/Psalm/Type/Atomic/TTemplateParam.php | 4 +- tests/JsonOutputTest.php | 4 +- tests/Template/ClassTemplateExtendsTest.php | 1 + tests/Template/ClassTemplateTest.php | 1 + tests/Template/FunctionTemplateTest.php | 28 ++++++ 13 files changed, 167 insertions(+), 80 deletions(-) diff --git a/src/Psalm/Internal/Analyzer/MethodAnalyzer.php b/src/Psalm/Internal/Analyzer/MethodAnalyzer.php index 4c8921e80..242f07c02 100644 --- a/src/Psalm/Internal/Analyzer/MethodAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/MethodAnalyzer.php @@ -6,6 +6,7 @@ use Psalm\Codebase; use Psalm\CodeLocation; use Psalm\Context; use Psalm\Internal\Analyzer\Statements\ExpressionAnalyzer; +use Psalm\Internal\Type\UnionTemplateHandler; use Psalm\Issue\DeprecatedMethod; use Psalm\Issue\ImplementedParamTypeMismatch; use Psalm\Issue\ImplementedReturnTypeMismatch; @@ -731,38 +732,28 @@ class MethodAnalyzer extends FunctionLikeAnalyzer ); } - $guide_trait_name = null; + if ($implementer_classlike_storage->is_trait) { + $implementer_called_class_storage = $codebase->classlike_storage_provider->get( + $implementer_called_class_name + ); - if ($guide_classlike_storage === $implementer_classlike_storage) { - $guide_trait_name = $implementer_method_storage->defining_fqcln; - } + if (isset( + $implementer_called_class_storage->template_type_extends[$implementer_classlike_storage->name] + )) { + self::transformTemplates( + $implementer_called_class_storage->template_type_extends, + $implementer_classlike_storage->name, + $implementer_method_storage_return_type, + $codebase + ); - if ($guide_trait_name - && isset($implementer_classlike_storage->template_type_extends[$guide_trait_name]) - ) { - $map = $implementer_classlike_storage->template_type_extends[$guide_trait_name]; - - $template_types = []; - - foreach ($map as $key => $type) { - if (is_string($key) && $implementer_method_storage->defining_fqcln) { - $template_types[$key][$implementer_method_storage->defining_fqcln] = [ - $type, - ]; - } + self::transformTemplates( + $implementer_called_class_storage->template_type_extends, + $guide_class_name, + $guide_method_storage_return_type, + $codebase + ); } - - $template_result = new \Psalm\Internal\Type\TemplateResult($template_types, []); - - $implementer_method_storage_return_type->replaceTemplateTypesWithArgTypes( - $template_result->template_types, - $codebase - ); - - $guide_method_storage_return_type->replaceTemplateTypesWithArgTypes( - $template_result->template_types, - $codebase - ); } // treat void as null when comparing against docblock implementer @@ -1017,6 +1008,30 @@ class MethodAnalyzer extends FunctionLikeAnalyzer ); } + if ($implementer_classlike_storage->is_trait) { + $implementer_called_class_storage = $codebase->classlike_storage_provider->get( + $implementer_called_class_name + ); + + if (isset( + $implementer_called_class_storage->template_type_extends[$implementer_classlike_storage->name] + )) { + self::transformTemplates( + $implementer_called_class_storage->template_type_extends, + $implementer_classlike_storage->name, + $implementer_method_storage_param_type, + $codebase + ); + + self::transformTemplates( + $implementer_called_class_storage->template_type_extends, + $guide_class_name, + $guide_method_storage_param_type, + $codebase + ); + } + } + $union_comparison_results = new TypeComparisonResult(); if (!TypeAnalyzer::isContainedBy( diff --git a/src/Psalm/Internal/Analyzer/Statements/ReturnAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/ReturnAnalyzer.php index 04f00f5ec..cde0b065c 100644 --- a/src/Psalm/Internal/Analyzer/Statements/ReturnAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/ReturnAnalyzer.php @@ -364,7 +364,7 @@ class ReturnAnalyzer } else { if (IssueBuffer::accepts( new InvalidReturnStatement( - 'The type \'' . $stmt_type->getId() + 'The inferred type \'' . $stmt_type->getId() . '\' does not match the declared return ' . 'type \'' . $local_return_type->getId() . '\' for ' . $cased_method_id, new CodeLocation($source, $stmt->expr) diff --git a/src/Psalm/Internal/Analyzer/TypeAnalyzer.php b/src/Psalm/Internal/Analyzer/TypeAnalyzer.php index ce86125a8..329cbf636 100644 --- a/src/Psalm/Internal/Analyzer/TypeAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/TypeAnalyzer.php @@ -500,7 +500,7 @@ class TypeAnalyzer $allow_interface_equality ) { $intersection_input_types = $input_type_part->extra_types ?: []; - $intersection_input_types[] = $input_type_part; + $intersection_input_types[$input_type_part->getKey(false)] = $input_type_part; if ($input_type_part instanceof TTemplateParam) { foreach ($input_type_part->as->getAtomicTypes() as $g) { @@ -514,7 +514,7 @@ class TypeAnalyzer } $intersection_container_types = $container_type_part->extra_types ?: []; - $intersection_container_types[] = $container_type_part; + $intersection_container_types[$container_type_part->getKey(false)] = $container_type_part; if ($container_type_part instanceof TTemplateParam) { foreach ($container_type_part->as->getAtomicTypes() as $g) { @@ -527,12 +527,32 @@ class TypeAnalyzer } } - foreach ($intersection_container_types as $intersection_container_type) { + foreach ($intersection_container_types as $container_type_key => $intersection_container_type) { if ($intersection_container_type instanceof TIterable) { $intersection_container_type_lower = 'iterable'; } elseif ($intersection_container_type instanceof TObjectWithProperties) { $intersection_container_type_lower = 'object'; } elseif ($intersection_container_type instanceof TTemplateParam) { + if (!$allow_interface_equality) { + if (isset($intersection_input_types[$container_type_key])) { + continue; + } + + if (\substr($intersection_container_type->defining_class, 0, 3) === 'fn-') { + foreach ($intersection_input_types as $intersection_input_type) { + if ($intersection_input_type instanceof TTemplateParam + && \substr($intersection_input_type->defining_class, 0, 3) === 'fn-' + && $intersection_input_type->defining_class + !== $intersection_container_type->defining_class + ) { + continue 2; + } + } + } + + return false; + } + if ($intersection_container_type->as->isMixed()) { continue; } @@ -544,6 +564,10 @@ class TypeAnalyzer continue; } + if ($g instanceof TObject) { + continue 2; + } + if (!$g instanceof TNamedObject) { continue 2; } @@ -597,35 +621,35 @@ class TypeAnalyzer ); } + if ($intersection_container_type instanceof TTemplateParam + && $intersection_input_type instanceof TTemplateParam + ) { + if ($intersection_container_type->param_name !== $intersection_input_type->param_name + || ((string)$intersection_container_type->defining_class + !== (string)$intersection_input_type->defining_class + && \substr($intersection_input_type->defining_class, 0, 3) !== 'fn-' + && \substr($intersection_container_type->defining_class, 0, 3) !== 'fn-') + ) { + if (\substr($intersection_input_type->defining_class, 0, 3) !== 'fn-') { + $input_class_storage = $codebase->classlike_storage_provider->get( + $intersection_input_type->defining_class + ); + + if (isset($input_class_storage->template_type_extends + [$intersection_container_type->defining_class] + [$intersection_container_type->param_name]) + ) { + continue; + } + } + + return false; + } + } + if (!$intersection_container_type instanceof TTemplateParam || $intersection_input_type instanceof TTemplateParam ) { - if ($intersection_container_type instanceof TTemplateParam - && $intersection_input_type instanceof TTemplateParam - ) { - if ($intersection_container_type->param_name !== $intersection_input_type->param_name - || ((string)$intersection_container_type->defining_class - !== (string)$intersection_input_type->defining_class - && \substr($intersection_input_type->defining_class, 0, 3) !== 'fn-' - && \substr($intersection_container_type->defining_class, 0, 3) !== 'fn-') - ) { - if (\substr($intersection_input_type->defining_class, 0, 3) !== 'fn-') { - $input_class_storage = $codebase->classlike_storage_provider->get( - $intersection_input_type->defining_class - ); - - if (isset($input_class_storage->template_type_extends - [$intersection_container_type->defining_class] - [$intersection_container_type->param_name]) - ) { - continue; - } - } - - return false; - } - } - if ($intersection_container_type_lower === $intersection_input_type_lower) { continue 2; } @@ -691,12 +715,18 @@ class TypeAnalyzer bool $allow_float_int_equality = true, ?TypeComparisonResult $atomic_comparison_result = null ) : bool { - if ($container_type_part instanceof TTemplateParam && $input_type_part instanceof TTemplateParam) { + if (($container_type_part instanceof TTemplateParam + || ($container_type_part instanceof TNamedObject + && isset($container_type_part->extra_types))) + && ($input_type_part instanceof TTemplateParam + || ($input_type_part instanceof TNamedObject + && isset($input_type_part->extra_types))) + ) { return self::isObjectContainedByObject( $codebase, - $container_type_part, $input_type_part, - true + $container_type_part, + $allow_interface_equality ); } @@ -826,11 +856,11 @@ class TypeAnalyzer && $container_type_part instanceof Type\Atomic\TFn) || (($input_type_part instanceof TNamedObject || ($input_type_part instanceof TTemplateParam - && $input_type_part->as->hasNamedObjectType()) + && $input_type_part->as->hasObjectType()) || $input_type_part instanceof TIterable) && ($container_type_part instanceof TNamedObject || ($container_type_part instanceof TTemplateParam - && $container_type_part->isNamedObjectType()) + && $container_type_part->isObjectType()) || $container_type_part instanceof TIterable) && self::isObjectContainedByObject( $codebase, diff --git a/src/Psalm/Internal/Type/AssertionReconciler.php b/src/Psalm/Internal/Type/AssertionReconciler.php index e364271e2..854940f1b 100644 --- a/src/Psalm/Internal/Type/AssertionReconciler.php +++ b/src/Psalm/Internal/Type/AssertionReconciler.php @@ -689,16 +689,23 @@ class AssertionReconciler extends \Psalm\Type\Reconciler && $new_type_part->as->isSingle() ) { $new_as_atomic = \array_values($new_type_part->as->getAtomicTypes())[0]; + $acceptable_atomic_types = []; foreach ($existing_var_type->getAtomicTypes() as $existing_var_type_part) { - if (TypeAnalyzer::isAtomicContainedBy( - $codebase, - $existing_var_type_part, - $new_as_atomic - )) { + if ($existing_var_type_part instanceof TNamedObject + || $existing_var_type_part instanceof TTemplateParam + ) { + $new_type_part->addIntersectionType($existing_var_type_part); $acceptable_atomic_types[] = clone $existing_var_type_part; - continue; + } else { + if (TypeAnalyzer::isAtomicContainedBy( + $codebase, + $existing_var_type_part, + $new_as_atomic + )) { + $acceptable_atomic_types[] = clone $existing_var_type_part; + } } } diff --git a/src/Psalm/Type/Atomic/CallableTrait.php b/src/Psalm/Type/Atomic/CallableTrait.php index 6fd8ef6a0..41a1970f3 100644 --- a/src/Psalm/Type/Atomic/CallableTrait.php +++ b/src/Psalm/Type/Atomic/CallableTrait.php @@ -64,7 +64,7 @@ trait CallableTrait /** * @return string */ - public function getKey() + public function getKey(bool $include_extra = true) { return $this->__toString(); } diff --git a/src/Psalm/Type/Atomic/TGenericObject.php b/src/Psalm/Type/Atomic/TGenericObject.php index 48a870b17..f9f1ad65d 100644 --- a/src/Psalm/Type/Atomic/TGenericObject.php +++ b/src/Psalm/Type/Atomic/TGenericObject.php @@ -30,16 +30,17 @@ class TGenericObject extends TNamedObject /** * @return string */ - public function getKey() + public function getKey(bool $include_extra = true) { $s = ''; + foreach ($this->type_params as $type_param) { $s .= $type_param->getKey() . ', '; } $extra_types = ''; - if ($this->extra_types) { + if ($include_extra && $this->extra_types) { $extra_types = '&' . implode('&', $this->extra_types); } diff --git a/src/Psalm/Type/Atomic/TIterable.php b/src/Psalm/Type/Atomic/TIterable.php index f1b7035ca..65248a465 100644 --- a/src/Psalm/Type/Atomic/TIterable.php +++ b/src/Psalm/Type/Atomic/TIterable.php @@ -39,8 +39,12 @@ class TIterable extends Atomic /** * @return string */ - public function getKey() + public function getKey(bool $include_extra = true) { + if ($include_extra && $this->extra_types) { + // do nothing + } + return 'iterable'; } diff --git a/src/Psalm/Type/Atomic/TNamedObject.php b/src/Psalm/Type/Atomic/TNamedObject.php index 810214a54..a2b802133 100644 --- a/src/Psalm/Type/Atomic/TNamedObject.php +++ b/src/Psalm/Type/Atomic/TNamedObject.php @@ -40,9 +40,9 @@ class TNamedObject extends Atomic /** * @return string */ - public function getKey() + public function getKey(bool $include_extra = true) { - if ($this->extra_types) { + if ($include_extra && $this->extra_types) { return $this->value . '&' . implode('&', $this->extra_types); } diff --git a/src/Psalm/Type/Atomic/TTemplateParam.php b/src/Psalm/Type/Atomic/TTemplateParam.php index adb7bd556..b54bc9776 100644 --- a/src/Psalm/Type/Atomic/TTemplateParam.php +++ b/src/Psalm/Type/Atomic/TTemplateParam.php @@ -48,9 +48,9 @@ class TTemplateParam extends \Psalm\Type\Atomic /** * @return string */ - public function getKey() + public function getKey(bool $include_extra = true) { - if ($this->extra_types) { + if ($include_extra && $this->extra_types) { return $this->param_name . ':' . $this->defining_class . '&' . implode('&', $this->extra_types); } diff --git a/tests/JsonOutputTest.php b/tests/JsonOutputTest.php index df0d11ae0..31bc118ca 100644 --- a/tests/JsonOutputTest.php +++ b/tests/JsonOutputTest.php @@ -209,7 +209,7 @@ echo $a;'; function fooFoo(int $a): string { return $a + 1; }', - 'message' => "The type 'int' does not match the declared return type 'string' for fooFoo", + 'message' => "The inferred type 'int' does not match the declared return type 'string' for fooFoo", 'line' => 3, 'error' => '$a + 1', ], @@ -248,7 +248,7 @@ echo $a;'; function fooFoo() { return "hello"; }', - 'message' => "The type 'string(hello)' does not match the declared return type 'int' for fooFoo", + 'message' => "The inferred type 'string(hello)' does not match the declared return type 'int' for fooFoo", 'line' => 6, 'error' => '"hello"', ], diff --git a/tests/Template/ClassTemplateExtendsTest.php b/tests/Template/ClassTemplateExtendsTest.php index 2f0cfa170..74f73a1bc 100644 --- a/tests/Template/ClassTemplateExtendsTest.php +++ b/tests/Template/ClassTemplateExtendsTest.php @@ -1147,6 +1147,7 @@ class ClassTemplateExtendsTest extends TestCase } /** * @template T + * @implements Functor */ class Box implements Functor { diff --git a/tests/Template/ClassTemplateTest.php b/tests/Template/ClassTemplateTest.php index fc27e3721..71d14b543 100644 --- a/tests/Template/ClassTemplateTest.php +++ b/tests/Template/ClassTemplateTest.php @@ -1569,6 +1569,7 @@ class ClassTemplateTest extends TestCase */ function makeConcrete(string $className) : object { + /** @var T&I */ return new class() extends C implements I { public function getMe() { return $this; diff --git a/tests/Template/FunctionTemplateTest.php b/tests/Template/FunctionTemplateTest.php index 951efc563..53e15d284 100644 --- a/tests/Template/FunctionTemplateTest.php +++ b/tests/Template/FunctionTemplateTest.php @@ -1460,6 +1460,34 @@ class FunctionTemplateTest extends TestCase }', 'error_message' => 'InvalidReturnStatement', ], + 'returnIntersectionWhenTemplateIsExpectedForward' => [ + ' 'InvalidReturnStatement', + ], + 'returnIntersectionWhenTemplateIsExpectedBackward' => [ + ' 'InvalidReturnStatement', + ], ]; } }