From 8bfb0412e72dbca23917b48a36511bded23a61ff Mon Sep 17 00:00:00 2001 From: Matt Brown Date: Sat, 5 Dec 2020 11:58:55 -0500 Subject: [PATCH] =?UTF-8?q?Fix=20#4782=20-=20don=E2=80=99t=20replace=20clo?= =?UTF-8?q?sure=20types=20with=20upper=20bounds=20when=20replacing=20class?= =?UTF-8?q?=20param=20types?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../Expression/Call/ArgumentAnalyzer.php | 19 +++++++-- src/Psalm/Internal/Type/TemplateResult.php | 7 ++++ .../Type/TemplateStandinTypeReplacer.php | 10 +++-- tests/Template/ClassTemplateTest.php | 42 ++++++++++++++----- 4 files changed, 61 insertions(+), 17 deletions(-) diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/ArgumentAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/ArgumentAnalyzer.php index 3ab280d42..aceea26be 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/ArgumentAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/ArgumentAnalyzer.php @@ -257,15 +257,26 @@ class ArgumentAnalyzer ); if ($class_generic_params) { - $empty_generic_params = []; + // here we're replacing the param types and arg types with the bound + // class template params. + // + // For example, if we're operating on a class Foo with params TKey and TValue, + // and we're calling a method "add(TKey $key, TValue $value)" on an instance + // of that class where we know that TKey is int and TValue is string, then we + // want to replace the substitute the expected values so it's as if we were actually + // calling "add(int $key, string $value)" + $readonly_template_result = new TemplateResult($class_generic_params, []); - $empty_template_result = new TemplateResult($class_generic_params, $empty_generic_params); + // This flag ensures that the template results will never be written to + // It also supercedes the `$add_upper_bounds` flag so that closure params + // don’t get overwritten + $readonly_template_result->readonly = true; $arg_value_type = $statements_analyzer->node_data->getType($arg->value); $param_type = TemplateStandinTypeReplacer::replace( $param_type, - $empty_template_result, + $readonly_template_result, $codebase, $statements_analyzer, $arg_value_type, @@ -275,7 +286,7 @@ class ArgumentAnalyzer $arg_type = TemplateStandinTypeReplacer::replace( $arg_type, - $empty_template_result, + $readonly_template_result, $codebase, $statements_analyzer, $arg_value_type, diff --git a/src/Psalm/Internal/Type/TemplateResult.php b/src/Psalm/Internal/Type/TemplateResult.php index d327c7837..8806fc6ad 100644 --- a/src/Psalm/Internal/Type/TemplateResult.php +++ b/src/Psalm/Internal/Type/TemplateResult.php @@ -22,6 +22,13 @@ class TemplateResult */ public $lower_bounds = []; + /** + * If set to true then we shouldn't update the template bounds + * + * @var bool + */ + public $readonly = false; + /** * @var list */ diff --git a/src/Psalm/Internal/Type/TemplateStandinTypeReplacer.php b/src/Psalm/Internal/Type/TemplateStandinTypeReplacer.php index 385b0df62..518067b31 100644 --- a/src/Psalm/Internal/Type/TemplateStandinTypeReplacer.php +++ b/src/Psalm/Internal/Type/TemplateStandinTypeReplacer.php @@ -619,6 +619,7 @@ class TemplateStandinTypeReplacer ); if ($input_type + && !$template_result->readonly && ( $atomic_type->as->isMixed() || !$codebase @@ -686,20 +687,23 @@ class TemplateStandinTypeReplacer ); } - foreach ($atomic_types as $atomic_type) { + foreach ($atomic_types as &$atomic_type) { if ($atomic_type instanceof Atomic\TNamedObject || $atomic_type instanceof Atomic\TTemplateParam || $atomic_type instanceof Atomic\TIterable || $atomic_type instanceof Atomic\TObjectWithProperties ) { $atomic_type->extra_types = $extra_types; + } elseif ($atomic_type instanceof Atomic\TObject && $extra_types) { + $atomic_type = \reset($extra_types); + $atomic_type->extra_types = \array_slice($extra_types, 1); } } return $atomic_types; } - if ($add_upper_bound && $input_type) { + if ($add_upper_bound && $input_type && !$template_result->readonly) { $matching_input_keys = []; if ($codebase @@ -782,7 +786,7 @@ class TemplateStandinTypeReplacer $atomic_types[] = $class_string; - if ($input_type) { + if ($input_type && !$template_result->readonly) { $valid_input_atomic_types = []; foreach ($input_type->getAtomicTypes() as $input_atomic_type) { diff --git a/tests/Template/ClassTemplateTest.php b/tests/Template/ClassTemplateTest.php index 74e7d51a1..f8294670e 100644 --- a/tests/Template/ClassTemplateTest.php +++ b/tests/Template/ClassTemplateTest.php @@ -2394,21 +2394,18 @@ class ClassTemplateTest extends TestCase 'intersectOnTOfObject' => [ 'setMethodPrefixInterceptor( - function(AccessInterceptorInterface $i) : string { + function foo(A $i) : void { + $i->setClosure( + function(A $i) : string { return "hello"; } ); @@ -3801,6 +3798,31 @@ class ClassTemplateTest extends TestCase false, '8.0' ], + 'bindClosureParamAccurately' => [ + ' + */ + public function map(Closure $func); + + } + + /** + * @param Collection $c + */ + function f(Collection $c): void { + $fn = function(int $_p): bool { return true; }; + $c->map($fn); + }', + 'error_message' => 'InvalidScalarArgument', + ], ]; } }