Fix #4524 - do better template param inheritance

vimeo · Nov 11, 2020 · a8d7248 · a8d7248
1 parent 5ad1e80
commit a8d7248
Show file tree

Hide file tree

Showing 3 changed files with 93 additions and 62 deletions.
diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/NewAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/NewAnalyzer.php
@@ -741,52 +741,4 @@ function ($assertion) use ($generic_params) {
 
         return true;
     }
-
-    /**
-     * @param  array<string, array<int|string, Type\Union>>  $template_type_extends
-     * @param  array<string, array<string, array{Type\Union}>>  $found_generic_params
-     */
-    private static function getGenericParamForOffset(
-        string $fq_class_name,
-        string $template_name,
-        array $template_type_extends,
-        array $found_generic_params,
-        bool $mapped = false
-    ): Type\Union {
-        if (isset($found_generic_params[$template_name][$fq_class_name])) {
-            if (!$mapped && isset($template_type_extends[$fq_class_name][$template_name])) {
-                foreach ($template_type_extends[$fq_class_name][$template_name]->getAtomicTypes() as $t) {
-                    if ($t instanceof Type\Atomic\TTemplateParam) {
-                        if ($t->param_name !== $template_name) {
-                            return $t->as;
-                        }
-                    }
-                }
-            }
-
-            return $found_generic_params[$template_name][$fq_class_name][0];
-        }
-
-        foreach ($template_type_extends as $type_map) {
-            foreach ($type_map as $extended_template_name => $extended_type) {
-                foreach ($extended_type->getAtomicTypes() as $extended_atomic_type) {
-                    if (is_string($extended_template_name)
-                        && $extended_atomic_type instanceof Type\Atomic\TTemplateParam
-                        && $extended_atomic_type->param_name === $template_name
-                        && $extended_template_name !== $template_name
-                    ) {
-                        return self::getGenericParamForOffset(
-                            $fq_class_name,
-                            $extended_template_name,
-                            $template_type_extends,
-                            $found_generic_params,
-                            true
-                        );
-                    }
-                }
-            }
-        }
-
-        return Type::getMixed();
-    }
 }
diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/CallAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/CallAnalyzer.php
@@ -362,20 +362,13 @@ public static function getTemplateTypesForCall(
                             $output_type = null;
 
                             foreach ($type->getAtomicTypes() as $atomic_type) {
-                                if ($atomic_type instanceof Type\Atomic\TTemplateParam
-                                    && isset(
-                                        $calling_class_storage
-                                            ->template_type_extends
-                                                [$atomic_type->defining_class]
-                                                [$atomic_type->param_name]
-                                    )
-                                ) {
-                                    $output_type_candidate = $calling_class_storage
-                                        ->template_type_extends
-                                            [$atomic_type->defining_class]
-                                            [$atomic_type->param_name];
-                                } elseif ($atomic_type instanceof Type\Atomic\TTemplateParam) {
-                                    $output_type_candidate = $atomic_type->as;
+                                if ($atomic_type instanceof Type\Atomic\TTemplateParam) {
+                                    $output_type_candidate = self::getGenericParamForOffset(
+                                        $atomic_type->defining_class,
+                                        $atomic_type->param_name,
+                                        $calling_class_storage->template_type_extends,
+                                        $template_types
+                                    );
                                 } else {
                                     $output_type_candidate = new Type\Union([$atomic_type]);
                                 }
@@ -421,6 +414,54 @@ public static function getTemplateTypesForCall(
         return $template_types;
     }
 
+    /**
+     * @param  array<string, array<int|string, Type\Union>>  $template_type_extends
+     * @param  array<string, array<string, array{Type\Union}>>  $found_generic_params
+     */
+    public static function getGenericParamForOffset(
+        string $fq_class_name,
+        string $template_name,
+        array $template_type_extends,
+        array $found_generic_params,
+        bool $mapped = false
+    ): Type\Union {
+        if (isset($found_generic_params[$template_name][$fq_class_name])) {
+            if (!$mapped && isset($template_type_extends[$fq_class_name][$template_name])) {
+                foreach ($template_type_extends[$fq_class_name][$template_name]->getAtomicTypes() as $t) {
+                    if ($t instanceof Type\Atomic\TTemplateParam) {
+                        if ($t->param_name !== $template_name) {
+                            return $t->as;
+                        }
+                    }
+                }
+            }
+
+            return $found_generic_params[$template_name][$fq_class_name][0];
+        }
+
+        foreach ($template_type_extends as $type_map) {
+            foreach ($type_map as $extended_template_name => $extended_type) {
+                foreach ($extended_type->getAtomicTypes() as $extended_atomic_type) {
+                    if (is_string($extended_template_name)
+                        && $extended_atomic_type instanceof Type\Atomic\TTemplateParam
+                        && $extended_atomic_type->param_name === $template_name
+                        && $extended_template_name !== $template_name
+                    ) {
+                        return self::getGenericParamForOffset(
+                            $fq_class_name,
+                            $extended_template_name,
+                            $template_type_extends,
+                            $found_generic_params,
+                            true
+                        );
+                    }
+                }
+            }
+        }
+
+        return Type::getMixed();
+    }
+
     /**
      * @param  PhpParser\Node\Scalar\String_|PhpParser\Node\Expr\Array_|PhpParser\Node\Expr\BinaryOp\Concat
      *         $callable_arg

diff --git a/tests/Template/ClassTemplateExtendsTest.php b/tests/Template/ClassTemplateExtendsTest.php
@@ -5816,6 +5816,44 @@ class TeacherModel extends BaseModel {}
                     class StudentRepository extends BaseRepository {}',
                 'error_message' => 'InvalidTemplateParam'
             ],
+            'detectIssueInDoublyInheritedMethod' => [
+                '<?php
+                    class Foo {}
+                    class FooChild extends Foo {}
+
+                    /**
+                     * @template T0
+                     */
+                    interface A {
+                        /**
+                         * @template U
+                         * @param callable(T0): U $func
+                         * @return U
+                         */
+                        function test(callable $func);
+                    }
+
+                    /**
+                     * @template T1
+                     * @template-extends A<T1>
+                     */
+                    interface B extends A {}
+
+                    /**
+                     * @template T2
+                     * @template-extends B<T2>
+                     */
+                    interface C extends B {}
+
+                    /**
+                     * @param C<Foo> $c
+                     */
+                    function second(C $c) : void {
+                        $f = function (FooChild $foo) : FooChild { return $foo; };
+                        $c->test($f);
+                    }',
+                'error_message' => 'ArgumentTypeCoercion'
+            ],
         ];
     }
 }