From af5c191e7b446fb3f7f45a9d8f8c1df4e0ce9099 Mon Sep 17 00:00:00 2001 From: AndrolGenhald Date: Tue, 7 Jun 2022 19:08:55 -0500 Subject: [PATCH 1/2] Fix generic object comparison to use template constraint as default (fixes #8068). --- .../Fetch/AtomicPropertyFetchAnalyzer.php | 32 ++++----------- .../Type/Comparator/GenericTypeComparator.php | 40 +++---------------- .../Type/TemplateStandinTypeReplacer.php | 20 +++++++++- src/Psalm/Storage/ClassLikeStorage.php | 18 +++++++++ tests/Template/ClassTemplateExtendsTest.php | 2 +- .../FunctionClassStringTemplateTest.php | 27 +++++++++++++ 6 files changed, 76 insertions(+), 63 deletions(-) diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/AtomicPropertyFetchAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/AtomicPropertyFetchAnalyzer.php index 0011896ea2e..afa84a3d7e4 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/AtomicPropertyFetchAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/AtomicPropertyFetchAnalyzer.php @@ -63,7 +63,7 @@ use function array_keys; use function array_search; -use function array_values; +use function count; use function in_array; use function is_int; use function is_string; @@ -570,15 +570,9 @@ private static function propertyFetchCanBeAnalyzed( $class_storage->parent_class ); - if ($class_storage->template_types) { + if (count($template_types = $class_storage->getClassTemplateTypes()) !== 0) { if (!$lhs_type_part instanceof TGenericObject) { - $type_params = []; - - foreach ($class_storage->template_types as $type_map) { - $type_params[] = clone array_values($type_map)[0]; - } - - $lhs_type_part = new TGenericObject($lhs_type_part->value, $type_params); + $lhs_type_part = new TGenericObject($lhs_type_part->value, $template_types); } $stmt_type = self::localizePropertyType( @@ -1100,15 +1094,9 @@ private static function handleNonExistentProperty( ) { $stmt_type = clone $class_storage->pseudo_property_get_types['$' . $prop_name]; - if ($class_storage->template_types) { + if (count($template_types = $class_storage->getClassTemplateTypes()) !== 0) { if (!$lhs_type_part instanceof TGenericObject) { - $type_params = []; - - foreach ($class_storage->template_types as $type_map) { - $type_params[] = clone array_values($type_map)[0]; - } - - $lhs_type_part = new TGenericObject($lhs_type_part->value, $type_params); + $lhs_type_part = new TGenericObject($lhs_type_part->value, $template_types); } $stmt_type = self::localizePropertyType( @@ -1200,15 +1188,9 @@ private static function getClassPropertyType( $declaring_class_storage->parent_class ); - if ($declaring_class_storage->template_types) { + if (count($template_types = $declaring_class_storage->getClassTemplateTypes()) !== 0) { if (!$lhs_type_part instanceof TGenericObject) { - $type_params = []; - - foreach ($declaring_class_storage->template_types as $type_map) { - $type_params[] = clone array_values($type_map)[0]; - } - - $lhs_type_part = new TGenericObject($lhs_type_part->value, $type_params); + $lhs_type_part = new TGenericObject($lhs_type_part->value, $template_types); } $class_property_type = self::localizePropertyType( diff --git a/src/Psalm/Internal/Type/Comparator/GenericTypeComparator.php b/src/Psalm/Internal/Type/Comparator/GenericTypeComparator.php index dd7e648c3fd..8cd529c66da 100644 --- a/src/Psalm/Internal/Type/Comparator/GenericTypeComparator.php +++ b/src/Psalm/Internal/Type/Comparator/GenericTypeComparator.php @@ -4,16 +4,11 @@ use Psalm\Codebase; use Psalm\Internal\Type\TemplateStandinTypeReplacer; -use Psalm\Type; use Psalm\Type\Atomic; use Psalm\Type\Atomic\TGenericObject; use Psalm\Type\Atomic\TIterable; use Psalm\Type\Atomic\TNamedObject; -use function array_fill; -use function array_values; -use function count; - /** * @internal */ @@ -44,38 +39,13 @@ public static function isContainedBy( $container_was_iterable = true; } - if (!$input_type_part instanceof TGenericObject && !$input_type_part instanceof TIterable) { - if ($input_type_part instanceof TNamedObject - && $codebase->classExists($input_type_part->value) - ) { - $class_storage = $codebase->classlike_storage_provider->get($input_type_part->value); - - $container_class = $container_type_part->value; - - // attempt to transform it - if (!empty($class_storage->template_extended_params[$container_class])) { - $input_type_part = new TGenericObject( - $input_type_part->value, - array_values($class_storage->template_extended_params[$container_class]) - ); - } + if (!$input_type_part instanceof TNamedObject && !$input_type_part instanceof TIterable) { + if ($atomic_comparison_result) { + $atomic_comparison_result->type_coerced = true; + $atomic_comparison_result->type_coerced_from_mixed = true; } - if (!$input_type_part instanceof TGenericObject) { - if ($input_type_part instanceof TNamedObject) { - $input_type_part = new TGenericObject( - $input_type_part->value, - array_fill(0, count($container_type_part->type_params), Type::getMixed()) - ); - } else { - if ($atomic_comparison_result) { - $atomic_comparison_result->type_coerced = true; - $atomic_comparison_result->type_coerced_from_mixed = true; - } - - return false; - } - } + return false; } $container_type_params_covariant = []; diff --git a/src/Psalm/Internal/Type/TemplateStandinTypeReplacer.php b/src/Psalm/Internal/Type/TemplateStandinTypeReplacer.php index 3791dc03e4a..7d6dbf6cee3 100644 --- a/src/Psalm/Internal/Type/TemplateStandinTypeReplacer.php +++ b/src/Psalm/Internal/Type/TemplateStandinTypeReplacer.php @@ -33,6 +33,7 @@ use Psalm\Type\Union; use Throwable; +use function array_fill; use function array_keys; use function array_merge; use function array_search; @@ -42,6 +43,7 @@ use function in_array; use function reset; use function strpos; +use function strtolower; use function substr; use function usort; @@ -1124,7 +1126,7 @@ function (TemplateBound $bound_a, TemplateBound $bound_b) { } /** - * @param TGenericObject|TIterable $input_type_part + * @param TGenericObject|TNamedObject|TIterable $input_type_part * @param TGenericObject|TIterable $container_type_part * @return list */ @@ -1134,7 +1136,21 @@ public static function getMappedGenericTypeParams( Atomic $container_type_part, ?array &$container_type_params_covariant = null ): array { - $input_type_params = $input_type_part->type_params; + if ($input_type_part instanceof TGenericObject || $input_type_part instanceof TIterable) { + $input_type_params = $input_type_part->type_params; + } else { + $class_storage = $codebase->classlike_storage_provider->get($input_type_part->value); + + $container_class = $container_type_part->value; + + if (strtolower($input_type_part->value) === strtolower($container_type_part->value)) { + $input_type_params = $class_storage->getClassTemplateTypes(); + } elseif (!empty($class_storage->template_extended_params[$container_class])) { + $input_type_params = array_values($class_storage->template_extended_params[$container_class]); + } else { + $input_type_params = array_fill(0, count($class_storage->template_types ?? []), Type::getMixed()); + } + } try { $input_class_storage = $codebase->classlike_storage_provider->get($input_type_part->value); diff --git a/src/Psalm/Storage/ClassLikeStorage.php b/src/Psalm/Storage/ClassLikeStorage.php index 1b33c69acb2..01b36968975 100644 --- a/src/Psalm/Storage/ClassLikeStorage.php +++ b/src/Psalm/Storage/ClassLikeStorage.php @@ -11,6 +11,8 @@ use Psalm\Type\Atomic\TTemplateParam; use Psalm\Type\Union; +use function array_values; + class ClassLikeStorage implements HasAttributesInterface { use CustomMetadataTrait; @@ -470,4 +472,20 @@ public function getAttributeStorages(): array { return $this->attributes; } + + /** + * Get the template constraint types for the class. + * + * @return list + */ + public function getClassTemplateTypes(): array + { + $type_params = []; + + foreach ($this->template_types ?? [] as $type_map) { + $type_params[] = clone array_values($type_map)[0]; + } + + return $type_params; + } } diff --git a/tests/Template/ClassTemplateExtendsTest.php b/tests/Template/ClassTemplateExtendsTest.php index 8fcba0c578b..c8c0fa4e532 100644 --- a/tests/Template/ClassTemplateExtendsTest.php +++ b/tests/Template/ClassTemplateExtendsTest.php @@ -5342,7 +5342,7 @@ public function __construct(DoStuff $stuff) {} } new Foo(new DoStuffX());', - 'error_message' => 'MixedArgumentTypeCoercion' + 'error_message' => 'TooManyTemplateParams' ], 'concreteDefinesSignatureTypesDifferent' => [ ' [ + 'code' => ' $_fooClass */ + function bar(string $_fooClass): void {} + + bar(Foo::class); + ', + ], + 'classStringWithGenericChildSatisfiesGenericParentWithDifferentConstraint' => [ + 'code' => ' + */ + class Bar extends Foo {} + + /** @param class-string $_fooClass */ + function bar(string $_fooClass): void {} + + bar(Bar::class); + ', + ], ]; } From 271dce8b6ce48b45001b1bc1385a731f6ba15800 Mon Sep 17 00:00:00 2001 From: AndrolGenhald Date: Tue, 7 Jun 2022 19:16:53 -0500 Subject: [PATCH 2/2] Fix test format after cherry-pick from master. --- tests/Template/FunctionClassStringTemplateTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/Template/FunctionClassStringTemplateTest.php b/tests/Template/FunctionClassStringTemplateTest.php index dcbff55f85a..57df39694a4 100644 --- a/tests/Template/FunctionClassStringTemplateTest.php +++ b/tests/Template/FunctionClassStringTemplateTest.php @@ -701,7 +701,7 @@ function (Bar $bar) use ($class): bool { interface Bar {}' ], 'classStringSatisfiesTemplateWithConstraint' => [ - 'code' => ' [ - 'code' => '