From d8bea796439a25376867fd1dccf1f065b425e5eb Mon Sep 17 00:00:00 2001 From: Ivan Sidorov Date: Tue, 26 Mar 2024 08:37:21 +0000 Subject: [PATCH 01/20] Part 1. Fail tests of named mixins with object methods Problems: ``` 1) Psalm\Tests\MixinsDeepTest::testValidCode with data set "NamedMixinsWithoutT_WithObjectMethods" Psalm\Exception\CodeException: MixedAssignment - src/somefile.php:22:1 - Unable to determine the type that $a is being assigned to 2) Psalm\Tests\MixinsDeepTest::testValidCode with data set "NamedMixinsWithT_WithObjectMethods" Psalm\Exception\CodeException: MixedAssignment - src/somefile.php:37:1 - Unable to determine the type that $a is being assigned to ``` --- tests/MixinsDeepTest.php | 93 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 93 insertions(+) create mode 100644 tests/MixinsDeepTest.php diff --git a/tests/MixinsDeepTest.php b/tests/MixinsDeepTest.php new file mode 100644 index 00000000000..61f2d76fa3a --- /dev/null +++ b/tests/MixinsDeepTest.php @@ -0,0 +1,93 @@ + [ + 'code' => <<<'PHP' + getString(); + $b = $baz->getInt(); + PHP, + 'assertions' => [ + '$a' => 'string', + '$b' => 'int', + ], + ], + 'NamedMixinsWithT_WithObjectMethods' => [ + 'code' => <<<'PHP' + + */ + abstract class Bar { + /** + * @return T2 + */ + abstract public function getInt(); + + public function __call(string $name, array $arguments) {} + } + + /** + * @template T1 + * @template T2 + * @mixin Bar + */ + class Baz { + public function __call(string $name, array $arguments) {} + } + + /** @var Baz */ + $baz = new Baz(); + $a = $baz->getString(); + $b = $baz->getInt(); + PHP, + 'assertions' => [ + '$a' => 'string', + '$b' => 'int', + ], + ], + ]; + } +} From a1992a7af954942ec6b5a03a20353fc513bc3c02 Mon Sep 17 00:00:00 2001 From: Ivan Sidorov Date: Sun, 4 Feb 2024 20:54:05 +0000 Subject: [PATCH 02/20] Part 1. Moving code into the handleMixins method Creating the `handleMixins` method is necessary so that it can be used in the future for recursive calls from the `handleTemplatedMixins` and `handleRegularMixins` methods. A recursive call to this method will allow us to search the depth of the mixins. --- .../Call/Method/AtomicMethodCallAnalyzer.php | 117 ++++++++++++------ 1 file changed, 80 insertions(+), 37 deletions(-) diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/Method/AtomicMethodCallAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/Method/AtomicMethodCallAnalyzer.php index 62903d0af76..1bcc8cce36b 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/Method/AtomicMethodCallAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/Method/AtomicMethodCallAnalyzer.php @@ -295,45 +295,22 @@ public static function analyze( } } - $naive_method_exists = false; - // @mixin attributes are an absolute pain! Lots of complexity here, // as they can redefine the called class, method id etc. - if ($class_storage->templatedMixins - && $lhs_type_part instanceof TGenericObject - && $class_storage->template_types - ) { - [$lhs_type_part, $class_storage, $naive_method_exists, $method_id, $fq_class_name] - = self::handleTemplatedMixins( - $class_storage, - $lhs_type_part, - $method_name_lc, - $codebase, - $context, - $method_id, - $source, - $stmt, - $statements_analyzer, - $fq_class_name, - ); - } elseif ($class_storage->mixin_declaring_fqcln - && $class_storage->namedMixins - ) { - [$lhs_type_part, $class_storage, $naive_method_exists, $method_id, $fq_class_name] - = self::handleRegularMixins( - $class_storage, - $lhs_type_part, - $method_name_lc, - $codebase, - $context, - $method_id, - $source, - $stmt, - $statements_analyzer, - $fq_class_name, - $lhs_var_id, - ); - } + [$lhs_type_part, $class_storage, $naive_method_exists, $method_id, $fq_class_name] + = self::handleMixins( + $class_storage, + $lhs_type_part, + $method_name_lc, + $codebase, + $context, + $method_id, + $source, + $stmt, + $statements_analyzer, + $fq_class_name, + $lhs_var_id, + ); } $all_intersection_return_type = null; @@ -723,6 +700,72 @@ private static function handleInvalidClass( } } + /** + * @param lowercase-string $method_name_lc + * @return array{TNamedObject, ClassLikeStorage, bool, MethodIdentifier, string} + */ + private static function handleMixins( + ClassLikeStorage $class_storage, + TNamedObject $lhs_type_part, + string $method_name_lc, + Codebase $codebase, + Context $context, + MethodIdentifier $method_id, + StatementsSource $source, + PhpParser\Node\Expr\MethodCall $stmt, + StatementsAnalyzer $statements_analyzer, + string $fq_class_name, + ?string $lhs_var_id + ): array { + $naive_method_exists = false; + + // @mixin attributes are an absolute pain! Lots of complexity here, + // as they can redefine the called class, method id etc. + if ($class_storage->templatedMixins + && $lhs_type_part instanceof TGenericObject + && $class_storage->template_types + ) { + [$lhs_type_part, $class_storage, $naive_method_exists, $method_id, $fq_class_name] + = self::handleTemplatedMixins( + $class_storage, + $lhs_type_part, + $method_name_lc, + $codebase, + $context, + $method_id, + $source, + $stmt, + $statements_analyzer, + $fq_class_name, + ); + } elseif ($class_storage->mixin_declaring_fqcln + && $class_storage->namedMixins + ) { + [$lhs_type_part, $class_storage, $naive_method_exists, $method_id, $fq_class_name] + = self::handleRegularMixins( + $class_storage, + $lhs_type_part, + $method_name_lc, + $codebase, + $context, + $method_id, + $source, + $stmt, + $statements_analyzer, + $fq_class_name, + $lhs_var_id, + ); + } + + return [ + $lhs_type_part, + $class_storage, + $naive_method_exists, + $method_id, + $fq_class_name, + ]; + } + /** * @param lowercase-string $method_name_lc * @return array{TNamedObject, ClassLikeStorage, bool, MethodIdentifier, string} From 8cf6dbfb9f930aa29b9f16e38c3fb544dda13cbc Mon Sep 17 00:00:00 2001 From: Ivan Sidorov Date: Sun, 4 Feb 2024 23:42:52 +0000 Subject: [PATCH 03/20] Part 1. Add variable method_exists Deep processing of mixins is planned to be done through recursive calls to the `handleMixins` method. To realise this, we need this method to return information that it has managed to find the desired method. Therefore, the `method_exists` variable was declared to make it clear that the method was found. The existing `naive_method_exists` variable is not suitable for us, because it is not set to `true` if the method is magic (pseudo). --- .../Call/Method/AtomicMethodCallAnalyzer.php | 22 +++++++++++++------ 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/Method/AtomicMethodCallAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/Method/AtomicMethodCallAnalyzer.php index 1bcc8cce36b..40b911459a2 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/Method/AtomicMethodCallAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/Method/AtomicMethodCallAnalyzer.php @@ -297,7 +297,7 @@ public static function analyze( // @mixin attributes are an absolute pain! Lots of complexity here, // as they can redefine the called class, method id etc. - [$lhs_type_part, $class_storage, $naive_method_exists, $method_id, $fq_class_name] + [$lhs_type_part, $class_storage, , $naive_method_exists, $method_id, $fq_class_name] = self::handleMixins( $class_storage, $lhs_type_part, @@ -702,7 +702,7 @@ private static function handleInvalidClass( /** * @param lowercase-string $method_name_lc - * @return array{TNamedObject, ClassLikeStorage, bool, MethodIdentifier, string} + * @return array{TNamedObject, ClassLikeStorage, bool, bool, MethodIdentifier, string} */ private static function handleMixins( ClassLikeStorage $class_storage, @@ -717,6 +717,7 @@ private static function handleMixins( string $fq_class_name, ?string $lhs_var_id ): array { + $method_exists = false; $naive_method_exists = false; // @mixin attributes are an absolute pain! Lots of complexity here, @@ -725,7 +726,7 @@ private static function handleMixins( && $lhs_type_part instanceof TGenericObject && $class_storage->template_types ) { - [$lhs_type_part, $class_storage, $naive_method_exists, $method_id, $fq_class_name] + [$lhs_type_part, $class_storage, $method_exists, $naive_method_exists, $method_id, $fq_class_name] = self::handleTemplatedMixins( $class_storage, $lhs_type_part, @@ -741,7 +742,7 @@ private static function handleMixins( } elseif ($class_storage->mixin_declaring_fqcln && $class_storage->namedMixins ) { - [$lhs_type_part, $class_storage, $naive_method_exists, $method_id, $fq_class_name] + [$lhs_type_part, $class_storage, $method_exists, $naive_method_exists, $method_id, $fq_class_name] = self::handleRegularMixins( $class_storage, $lhs_type_part, @@ -760,6 +761,7 @@ private static function handleMixins( return [ $lhs_type_part, $class_storage, + $method_exists, $naive_method_exists, $method_id, $fq_class_name, @@ -768,7 +770,7 @@ private static function handleMixins( /** * @param lowercase-string $method_name_lc - * @return array{TNamedObject, ClassLikeStorage, bool, MethodIdentifier, string} + * @return array{TNamedObject, ClassLikeStorage, bool, bool, MethodIdentifier, string} */ private static function handleTemplatedMixins( ClassLikeStorage $class_storage, @@ -782,6 +784,7 @@ private static function handleTemplatedMixins( StatementsAnalyzer $statements_analyzer, string $fq_class_name ): array { + $method_exists = false; $naive_method_exists = false; if ($class_storage->templatedMixins @@ -832,12 +835,13 @@ private static function handleTemplatedMixins( )) { $lhs_type_part = $lhs_type_part_new; $class_storage = $mixin_class_storage; - + $method_exists = true; $naive_method_exists = true; $method_id = $new_method_id; } elseif (isset($mixin_class_storage->pseudo_methods[$method_name_lc])) { $lhs_type_part = $lhs_type_part_new; $class_storage = $mixin_class_storage; + $method_exists = true; $method_id = $new_method_id; } } @@ -849,6 +853,7 @@ private static function handleTemplatedMixins( return [ $lhs_type_part, $class_storage, + $method_exists, $naive_method_exists, $method_id, $fq_class_name, @@ -857,7 +862,7 @@ private static function handleTemplatedMixins( /** * @param lowercase-string $method_name_lc - * @return array{TNamedObject, ClassLikeStorage, bool, MethodIdentifier, string} + * @return array{TNamedObject, ClassLikeStorage, bool, bool, MethodIdentifier, string} */ private static function handleRegularMixins( ClassLikeStorage $class_storage, @@ -872,6 +877,7 @@ private static function handleRegularMixins( string $fq_class_name, ?string $lhs_var_id ): array { + $method_exists = false; $naive_method_exists = false; foreach ($class_storage->namedMixins as $mixin) { @@ -938,6 +944,7 @@ private static function handleRegularMixins( $fq_class_name = $mixin_class_storage->name; $mixin_class_storage->mixin_declaring_fqcln = $class_storage->mixin_declaring_fqcln; $class_storage = $mixin_class_storage; + $method_exists = true; $naive_method_exists = true; $method_id = $new_method_id; } @@ -946,6 +953,7 @@ private static function handleRegularMixins( return [ $lhs_type_part, $class_storage, + $method_exists, $naive_method_exists, $method_id, $fq_class_name, From 0e665e5857ca1a6403e105c3bc4bcc2696c1721e Mon Sep 17 00:00:00 2001 From: Ivan Sidorov Date: Sun, 4 Feb 2024 22:08:55 +0000 Subject: [PATCH 04/20] Part 1. Extracting variables in the handleRegularMixins method Extracting variables defined in the "if" statement block to the upper lines. All this is necessary to use these variables for further searching at the next deeper level of mixins, passing them to the `handleMixins` method, performing recursion in this way. Extracting code from: ``` if ($codebase->methods->methodExists(...)) { // here extracted } ``` --- .../Call/Method/AtomicMethodCallAnalyzer.php | 86 ++++++++++--------- 1 file changed, 47 insertions(+), 39 deletions(-) diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/Method/AtomicMethodCallAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/Method/AtomicMethodCallAnalyzer.php index 40b911459a2..16dc782077c 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/Method/AtomicMethodCallAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/Method/AtomicMethodCallAnalyzer.php @@ -884,6 +884,47 @@ private static function handleRegularMixins( if (!$class_storage->mixin_declaring_fqcln) { continue; } + if (!$codebase->classlike_storage_provider->has($mixin->value)) { + continue; + } + + $mixin_declaring_class_storage = $codebase->classlike_storage_provider->get( + $class_storage->mixin_declaring_fqcln, + ); + + $mixin_class_template_params = ClassTemplateParamCollector::collect( + $codebase, + $mixin_declaring_class_storage, + $codebase->classlike_storage_provider->get($fq_class_name), + null, + $lhs_type_part, + $lhs_var_id === '$this', + ); + + $mixin_lhs_type_part = $mixin->replaceTemplateTypesWithArgTypes( + new TemplateResult([], $mixin_class_template_params ?: []), + $codebase, + ); + + $lhs_type_expanded = TypeExpander::expandUnion( + $codebase, + new Union([$mixin_lhs_type_part]), + $mixin_declaring_class_storage->name, + $fq_class_name, + $class_storage->parent_class, + true, + false, + $class_storage->final, + ); + + $_lhs_type_part = $lhs_type_expanded->getSingleAtomic(); + if ($_lhs_type_part instanceof TNamedObject) { + $mixin_lhs_type_part = $_lhs_type_part; + } + + $mixin_class_storage = $codebase->classlike_storage_provider->get($mixin->value); + + $mixin_fq_class_name = $mixin_class_storage->name; $new_method_id = new MethodIdentifier( $mixin->value, @@ -904,48 +945,15 @@ private static function handleRegularMixins( true, $context->insideUse(), )) { - $mixin_declaring_class_storage = $codebase->classlike_storage_provider->get( - $class_storage->mixin_declaring_fqcln, - ); - - $mixin_class_template_params = ClassTemplateParamCollector::collect( - $codebase, - $mixin_declaring_class_storage, - $codebase->classlike_storage_provider->get($fq_class_name), - null, - $lhs_type_part, - $lhs_var_id === '$this', - ); - - $lhs_type_part = $mixin->replaceTemplateTypesWithArgTypes( - new TemplateResult([], $mixin_class_template_params ?: []), - $codebase, - ); - - $lhs_type_expanded = TypeExpander::expandUnion( - $codebase, - new Union([$lhs_type_part]), - $mixin_declaring_class_storage->name, - $fq_class_name, - $class_storage->parent_class, - true, - false, - $class_storage->final, - ); - - $new_lhs_type_part = $lhs_type_expanded->getSingleAtomic(); - - if ($new_lhs_type_part instanceof TNamedObject) { - $lhs_type_part = $new_lhs_type_part; - } - - $mixin_class_storage = $codebase->classlike_storage_provider->get($mixin->value); + $method_exists = true; + $naive_method_exists = true; + } - $fq_class_name = $mixin_class_storage->name; + if ($method_exists) { + $lhs_type_part = $mixin_lhs_type_part; + $fq_class_name = $mixin_fq_class_name; $mixin_class_storage->mixin_declaring_fqcln = $class_storage->mixin_declaring_fqcln; $class_storage = $mixin_class_storage; - $method_exists = true; - $naive_method_exists = true; $method_id = $new_method_id; } } From 72817c1b817e283812cf3862712b1a2fe21b8c02 Mon Sep 17 00:00:00 2001 From: Ivan Sidorov Date: Sun, 4 Feb 2024 22:18:10 +0000 Subject: [PATCH 05/20] Part 1. Resolve tests of named mixins with object methods --- .../Call/Method/AtomicMethodCallAnalyzer.php | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/Method/AtomicMethodCallAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/Method/AtomicMethodCallAnalyzer.php index 16dc782077c..00f921e9bd5 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/Method/AtomicMethodCallAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/Method/AtomicMethodCallAnalyzer.php @@ -949,12 +949,36 @@ private static function handleRegularMixins( $naive_method_exists = true; } + if (!$method_exists) { + [ + $mixin_lhs_type_part, + $mixin_class_storage, + $method_exists, + $naive_method_exists, + $new_method_id, + $mixin_fq_class_name, + ] = self::handleMixins( + $mixin_class_storage, + $mixin_lhs_type_part, + $method_name_lc, + $codebase, + $context, + $new_method_id, + $source, + $stmt, + $statements_analyzer, + $mixin_fq_class_name, + $lhs_var_id, + ); + } + if ($method_exists) { $lhs_type_part = $mixin_lhs_type_part; $fq_class_name = $mixin_fq_class_name; $mixin_class_storage->mixin_declaring_fqcln = $class_storage->mixin_declaring_fqcln; $class_storage = $mixin_class_storage; $method_id = $new_method_id; + break; } } From bfd29377396ac0d3179c69409f4713a15ab355be Mon Sep 17 00:00:00 2001 From: Ivan Sidorov Date: Tue, 26 Mar 2024 08:39:29 +0000 Subject: [PATCH 06/20] Part 2. Fail tests of templated mixins with object methods Problems: ``` 1) Psalm\Tests\MixinsDeepTest::testValidCode with data set "TemplatedMixins_WithObjectMethods" Psalm\Exception\CodeException: MixedAssignment - src/somefile.php:25:1 - Unable to determine the type that $a is being assigned to ``` --- tests/MixinsDeepTest.php | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/tests/MixinsDeepTest.php b/tests/MixinsDeepTest.php index 61f2d76fa3a..fee2062ae76 100644 --- a/tests/MixinsDeepTest.php +++ b/tests/MixinsDeepTest.php @@ -88,6 +88,40 @@ public function __call(string $name, array $arguments) {} '$b' => 'int', ], ], + 'TemplatedMixins_WithObjectMethods' => [ + 'code' => <<<'PHP' + > */ + $baz = new Baz(); + $a = $baz->getString(); + $b = $baz->getInt(); + PHP, + 'assertions' => [ + '$a' => 'string', + '$b' => 'int', + ], + ], ]; } } From fecae81810741212eda824c1b5b21ec4cdfc2a50 Mon Sep 17 00:00:00 2001 From: Ivan Sidorov Date: Sun, 4 Feb 2024 23:10:01 +0000 Subject: [PATCH 07/20] Part 2. Refactoring the handleTemplatedMixins method Extracting code from: ``` if ($codebase->methods->methodExists(...)) { // here extracted } elseif (...) { // here extracted } ``` --- .../Call/Method/AtomicMethodCallAnalyzer.php | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/Method/AtomicMethodCallAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/Method/AtomicMethodCallAnalyzer.php index 00f921e9bd5..ff17e456c28 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/Method/AtomicMethodCallAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/Method/AtomicMethodCallAnalyzer.php @@ -833,15 +833,17 @@ private static function handleTemplatedMixins( true, $context->insideUse(), )) { - $lhs_type_part = $lhs_type_part_new; - $class_storage = $mixin_class_storage; $method_exists = true; $naive_method_exists = true; - $method_id = $new_method_id; - } elseif (isset($mixin_class_storage->pseudo_methods[$method_name_lc])) { + } + + if (!$method_exists) { + $method_exists = isset($mixin_class_storage->pseudo_methods[$method_name_lc]); + } + + if ($method_exists) { $lhs_type_part = $lhs_type_part_new; $class_storage = $mixin_class_storage; - $method_exists = true; $method_id = $new_method_id; } } From 229c072c6db6d356cde6bd3b6155e510d65d783f Mon Sep 17 00:00:00 2001 From: Ivan Sidorov Date: Mon, 5 Feb 2024 09:32:24 +0000 Subject: [PATCH 08/20] Part 2. Overriding the fq_class_name variable Comparing the `handleTemplatedMixins` and `handleRegularMixins` methods, we can notice that in the `handleRegularMixins` method the `fq_class_name` variable is overridden if the sought method is found in the mixin. Whereas in the `handleTemplatedMixins` method this does not happen. And I think therein lies the error. Since the `class_storage` variable can be overridden in the `handleTemplatedMixins` method, the `fq_class_name` variable should also be overridden. --- .../Expression/Call/Method/AtomicMethodCallAnalyzer.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/Method/AtomicMethodCallAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/Method/AtomicMethodCallAnalyzer.php index ff17e456c28..3993c54457d 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/Method/AtomicMethodCallAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/Method/AtomicMethodCallAnalyzer.php @@ -819,6 +819,8 @@ private static function handleTemplatedMixins( $lhs_type_part_new->value, ); + $mixin_fq_class_name = $mixin_class_storage->name; + if ($codebase->methods->methodExists( $new_method_id, $context->calling_method_id, @@ -845,6 +847,7 @@ private static function handleTemplatedMixins( $lhs_type_part = $lhs_type_part_new; $class_storage = $mixin_class_storage; $method_id = $new_method_id; + $fq_class_name = $mixin_fq_class_name; } } } From 95e2e5400f828bb2e2e547bacb8c085f5c08b117 Mon Sep 17 00:00:00 2001 From: Ivan Sidorov Date: Mon, 5 Feb 2024 09:34:10 +0000 Subject: [PATCH 09/20] Part 2. Resolve tests of templated mixins with object methods --- .../Call/Method/AtomicMethodCallAnalyzer.php | 28 ++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/Method/AtomicMethodCallAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/Method/AtomicMethodCallAnalyzer.php index 3993c54457d..41d9dafd1e4 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/Method/AtomicMethodCallAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/Method/AtomicMethodCallAnalyzer.php @@ -738,6 +738,7 @@ private static function handleMixins( $stmt, $statements_analyzer, $fq_class_name, + $lhs_var_id, ); } elseif ($class_storage->mixin_declaring_fqcln && $class_storage->namedMixins @@ -782,7 +783,8 @@ private static function handleTemplatedMixins( StatementsSource $source, PhpParser\Node\Expr\MethodCall $stmt, StatementsAnalyzer $statements_analyzer, - string $fq_class_name + string $fq_class_name, + ?string $lhs_var_id ): array { $method_exists = false; $naive_method_exists = false; @@ -843,11 +845,35 @@ private static function handleTemplatedMixins( $method_exists = isset($mixin_class_storage->pseudo_methods[$method_name_lc]); } + if (!$method_exists) { + [ + $lhs_type_part_new, + $mixin_class_storage, + $method_exists, + $naive_method_exists, + $new_method_id, + $mixin_fq_class_name, + ] = self::handleMixins( + $mixin_class_storage, + $lhs_type_part_new, + $method_name_lc, + $codebase, + $context, + $new_method_id, + $source, + $stmt, + $statements_analyzer, + $mixin_fq_class_name, + $lhs_var_id, + ); + } + if ($method_exists) { $lhs_type_part = $lhs_type_part_new; $class_storage = $mixin_class_storage; $method_id = $new_method_id; $fq_class_name = $mixin_fq_class_name; + break; } } } From cd7129504bd729adb32df982dc675dd343a5ef2c Mon Sep 17 00:00:00 2001 From: Ivan Sidorov Date: Tue, 26 Mar 2024 08:40:47 +0000 Subject: [PATCH 10/20] Part 3. Regression tests of combined mixins with object methods --- tests/MixinsDeepTest.php | 106 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 106 insertions(+) diff --git a/tests/MixinsDeepTest.php b/tests/MixinsDeepTest.php index fee2062ae76..8a1706d9c5a 100644 --- a/tests/MixinsDeepTest.php +++ b/tests/MixinsDeepTest.php @@ -122,6 +122,112 @@ public function __call(string $name, array $arguments) {} '$b' => 'int', ], ], + 'CombineNamedAndTemplatedMixins_WithObjectMethods' => [ + 'code' => <<<'PHP' + + */ + class Baz { + public function __call(string $name, array $arguments) {} + } + + /** @var Baz */ + $baz = new Baz(); + $a = $baz->getString(); + $b = $baz->getInt(); + PHP, + 'assertions' => [ + '$a' => 'string', + '$b' => 'int', + ], + ], + 'CombineTemplatedAndNamedMixinsWithoutT_WithObjectMethods' => [ + 'code' => <<<'PHP' + $baz */ + $baz = new Baz(); + $a = $baz->getString(); + $b = $baz->getInt(); + PHP, + 'assertions' => [ + '$a' => 'string', + '$b' => 'int', + ], + ], + 'CombineTemplatedAndNamedMixinsWithT_WithObjectMethods' => [ + 'code' => <<<'PHP' + + */ + abstract class Bar { + abstract public function getInt(): int; + public function __call(string $name, array $arguments) {} + } + + /** + * @template T + * @mixin T + */ + class Baz { + public function __call(string $name, array $arguments) {} + } + + /** @var Baz $baz */ + $baz = new Baz(); + $a = $baz->getString(); + $b = $baz->getInt(); + PHP, + 'assertions' => [ + '$a' => 'string', + '$b' => 'int', + ], + ], ]; } } From c5ea30eabe4916e4f5bce6d8e14e30744703d7d2 Mon Sep 17 00:00:00 2001 From: Ivan Sidorov Date: Thu, 15 Feb 2024 18:03:32 +0000 Subject: [PATCH 11/20] Part 4. Fail tests of named mixins with static methods Problems: ``` 1) Psalm\Tests\MixinsDeepTest::testValidCode with data set "NamedMixinsWithoutT_WithStaticMethods" Psalm\Exception\CodeException: MixedAssignment - src/somefile.php:25:1 - Unable to determine the type that $a is being assigned to 2) Psalm\Tests\MixinsDeepTest::testValidCode with data set "NamedMixinsWithT_WithStaticMethods" Psalm\Exception\CodeException: MixedAssignment - src/somefile.php:39:1 - Unable to determine the type that $a is being assigned to ``` --- tests/MixinsDeepTest.php | 84 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 84 insertions(+) diff --git a/tests/MixinsDeepTest.php b/tests/MixinsDeepTest.php index 8a1706d9c5a..3cac5950f1b 100644 --- a/tests/MixinsDeepTest.php +++ b/tests/MixinsDeepTest.php @@ -42,6 +42,41 @@ public function __call(string $name, array $arguments) {} '$b' => 'int', ], ], + 'NamedMixinsWithoutT_WithStaticMethods' => [ + 'code' => <<<'PHP' + [ + '$a' => 'string', + '$b' => 'int', + ], + 'ignored_issues' => ['InvalidReturnType'], + ], 'NamedMixinsWithT_WithObjectMethods' => [ 'code' => <<<'PHP' 'int', ], ], + 'NamedMixinsWithT_WithStaticMethods' => [ + 'code' => <<<'PHP' + + */ + abstract class Bar { + /** + * @return T2 + */ + public static function getInt() {} + + public static function __callStatic(string $name, array $arguments) {} + } + + /** + * @template T1 + * @template T2 + * @mixin Bar + */ + class Baz { + public static function __callStatic(string $name, array $arguments) {} + } + + /** @mixin Baz */ + class Bat { + public static function __callStatic(string $name, array $arguments) {} + } + $a = Bat::getString(); + $b = Bat::getInt(); + PHP, + 'assertions' => [ + '$a' => 'string', + '$b' => 'int', + ], + 'ignored_issues' => ['InvalidReturnType'], + ], 'TemplatedMixins_WithObjectMethods' => [ 'code' => <<<'PHP' Date: Thu, 15 Feb 2024 18:42:44 +0000 Subject: [PATCH 12/20] Part 4. Moving code into the handleRegularMixins method Creating this method is necessary to be able to make recursive calls using this method in the next step. --- .../StaticMethod/AtomicStaticCallAnalyzer.php | 207 ++++++++++-------- 1 file changed, 118 insertions(+), 89 deletions(-) diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticMethod/AtomicStaticCallAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticMethod/AtomicStaticCallAnalyzer.php index c9dce4d1460..78fc8bfba96 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticMethod/AtomicStaticCallAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticMethod/AtomicStaticCallAnalyzer.php @@ -384,97 +384,29 @@ private static function handleNamedCall( $args = $stmt->isFirstClassCallable() ? [] : $stmt->getArgs(); - if (!$naive_method_exists - && $class_storage->mixin_declaring_fqcln - && $class_storage->namedMixins - ) { - foreach ($class_storage->namedMixins as $mixin) { - $new_method_id = new MethodIdentifier( - $mixin->value, - $method_name_lc, - ); + if (!$naive_method_exists) { + $new_lhs_type = self::handleRegularMixins( + $class_storage, + $lhs_type_part, + $method_name_lc, + $codebase, + $context, + $stmt_name, + $statements_analyzer, + $fq_class_name, + ); + if ($new_lhs_type !== null) { + $mixin_context = clone $context; + $mixin_context->vars_in_scope['$__tmp_mixin_var__'] = $new_lhs_type; - if ($codebase->methods->methodExists( - $new_method_id, - $context->calling_method_id, - $codebase->collect_locations - ? new CodeLocation($statements_analyzer, $stmt_name) - : null, - !$context->collect_initializations - && !$context->collect_mutations - ? $statements_analyzer - : null, - $statements_analyzer->getFilePath(), + return self::forwardCallToInstanceMethod( + $statements_analyzer, + $stmt, + $stmt_name, + $mixin_context, + '__tmp_mixin_var__', true, - $context->insideUse(), - )) { - $mixin_candidates = []; - foreach ($class_storage->templatedMixins as $mixin_candidate) { - $mixin_candidates[] = $mixin_candidate; - } - - foreach ($class_storage->namedMixins as $mixin_candidate) { - $mixin_candidates[] = $mixin_candidate; - } - - $mixin_candidates_no_generic = array_filter( - $mixin_candidates, - static fn(Atomic $check): bool => !($check instanceof TGenericObject), - ); - - // $mixin_candidates_no_generic will only be empty when there are TGenericObject entries. - // In that case, Union will be initialized with an empty array but - // replaced with non-empty types in the following loop. - /** @psalm-suppress ArgumentTypeCoercion */ - $mixin_candidate_type = new Union($mixin_candidates_no_generic); - - foreach ($mixin_candidates as $tGenericMixin) { - if (!($tGenericMixin instanceof TGenericObject)) { - continue; - } - - $mixin_declaring_class_storage = $codebase->classlike_storage_provider->get( - $class_storage->mixin_declaring_fqcln, - ); - - $new_mixin_candidate_type = AtomicPropertyFetchAnalyzer::localizePropertyType( - $codebase, - new Union([$lhs_type_part]), - $tGenericMixin, - $class_storage, - $mixin_declaring_class_storage, - )->getBuilder(); - - foreach ($mixin_candidate_type->getAtomicTypes() as $type) { - $new_mixin_candidate_type->addType($type); - } - - $mixin_candidate_type = $new_mixin_candidate_type->freeze(); - } - - $new_lhs_type = TypeExpander::expandUnion( - $codebase, - $mixin_candidate_type, - $fq_class_name, - $fq_class_name, - $class_storage->parent_class, - true, - false, - $class_storage->final, - ); - - $mixin_context = clone $context; - $mixin_context->vars_in_scope['$__tmp_mixin_var__'] = $new_lhs_type; - - return self::forwardCallToInstanceMethod( - $statements_analyzer, - $stmt, - $stmt_name, - $mixin_context, - '__tmp_mixin_var__', - true, - ); - } + ); } } @@ -991,6 +923,103 @@ private static function checkPseudoMethod( return null; } + /** + * @param lowercase-string $method_name_lc + */ + private static function handleRegularMixins( + ClassLikeStorage $class_storage, + Atomic $lhs_type_part, + string $method_name_lc, + Codebase $codebase, + Context $context, + PhpParser\Node\Identifier $stmt_name, + StatementsAnalyzer $statements_analyzer, + string $fq_class_name + ): ?Union { + if ($class_storage->mixin_declaring_fqcln === null) { + return null; + } + foreach ($class_storage->namedMixins as $mixin) { + $new_method_id = new MethodIdentifier( + $mixin->value, + $method_name_lc, + ); + + if ($codebase->methods->methodExists( + $new_method_id, + $context->calling_method_id, + $codebase->collect_locations + ? new CodeLocation($statements_analyzer, $stmt_name) + : null, + !$context->collect_initializations + && !$context->collect_mutations + ? $statements_analyzer + : null, + $statements_analyzer->getFilePath(), + true, + $context->insideUse(), + )) { + $mixin_candidates = []; + foreach ($class_storage->templatedMixins as $mixin_candidate) { + $mixin_candidates[] = $mixin_candidate; + } + + foreach ($class_storage->namedMixins as $mixin_candidate) { + $mixin_candidates[] = $mixin_candidate; + } + + $mixin_candidates_no_generic = array_filter( + $mixin_candidates, + static fn(Atomic $check): bool => !($check instanceof TGenericObject), + ); + + // $mixin_candidates_no_generic will only be empty when there are TGenericObject entries. + // In that case, Union will be initialized with an empty array but + // replaced with non-empty types in the following loop. + /** @psalm-suppress ArgumentTypeCoercion */ + $mixin_candidate_type = new Union($mixin_candidates_no_generic); + + foreach ($mixin_candidates as $tGenericMixin) { + if (!($tGenericMixin instanceof TGenericObject)) { + continue; + } + + $mixin_declaring_class_storage = $codebase->classlike_storage_provider->get( + $class_storage->mixin_declaring_fqcln, + ); + + $new_mixin_candidate_type = AtomicPropertyFetchAnalyzer::localizePropertyType( + $codebase, + new Union([$lhs_type_part]), + $tGenericMixin, + $class_storage, + $mixin_declaring_class_storage, + )->getBuilder(); + + foreach ($mixin_candidate_type->getAtomicTypes() as $type) { + $new_mixin_candidate_type->addType($type); + } + + $mixin_candidate_type = $new_mixin_candidate_type->freeze(); + } + + $new_lhs_type = TypeExpander::expandUnion( + $codebase, + $mixin_candidate_type, + $fq_class_name, + $fq_class_name, + $class_storage->parent_class, + true, + false, + $class_storage->final, + ); + + return $new_lhs_type; + } + } + return null; + } + public static function handleNonObjectCall( StatementsAnalyzer $statements_analyzer, PhpParser\Node\Expr\StaticCall $stmt, From 0d0a8b50096445d694584dc4ff03e4fb042f65ba Mon Sep 17 00:00:00 2001 From: Ivan Sidorov Date: Fri, 16 Feb 2024 17:59:51 +0000 Subject: [PATCH 13/20] Part 4. Resolve tests of named mixins with static methods Added recursive call --- .../StaticMethod/AtomicStaticCallAnalyzer.php | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticMethod/AtomicStaticCallAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticMethod/AtomicStaticCallAnalyzer.php index 78fc8bfba96..094602ff91b 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticMethod/AtomicStaticCallAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticMethod/AtomicStaticCallAnalyzer.php @@ -1017,6 +1017,26 @@ private static function handleRegularMixins( return $new_lhs_type; } } + foreach ($class_storage->namedMixins as $mixin) { + if (!$codebase->classlike_storage_provider->has($mixin->value)) { + continue; + } + $mixin_class_storage = $codebase->classlike_storage_provider->get($mixin->value); + $mixin_fq_class_name = $mixin_class_storage->name; + $new_lhs_type = self::handleRegularMixins( + $mixin_class_storage, + $lhs_type_part, + $method_name_lc, + $codebase, + $context, + $stmt_name, + $statements_analyzer, + $mixin_fq_class_name, + ); + if ($new_lhs_type) { + return $new_lhs_type; + } + } return null; } From 08b50d69a0f982be75b131e69241e0a9dc65fbbd Mon Sep 17 00:00:00 2001 From: Ivan Sidorov Date: Sat, 17 Feb 2024 19:20:03 +0000 Subject: [PATCH 14/20] Part 5. Fail tests of mixins with properties Problems: ``` 1) Psalm\Tests\MixinsDeepTest::testValidCode with data set "NamedMixinsWithoutT_WithObjectProperties" Psalm\Exception\CodeException: MixedAssignment - src/somefile.php:23:1 - Unable to determine the type that $a is being assigned to ``` --- tests/MixinsDeepTest.php | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/tests/MixinsDeepTest.php b/tests/MixinsDeepTest.php index 3cac5950f1b..2e382f2bf31 100644 --- a/tests/MixinsDeepTest.php +++ b/tests/MixinsDeepTest.php @@ -77,6 +77,38 @@ public static function __callStatic(string $name, array $arguments) {} ], 'ignored_issues' => ['InvalidReturnType'], ], + 'NamedMixinsWithoutT_WithObjectProperties' => [ + 'code' => <<<'PHP' + propString; + $b = $baz->propInt; + PHP, + 'assertions' => [ + '$a' => 'string', + '$b' => 'int', + ], + ], 'NamedMixinsWithT_WithObjectMethods' => [ 'code' => <<<'PHP' Date: Sat, 17 Feb 2024 19:53:04 +0000 Subject: [PATCH 15/20] Part 5. Move code to method handleRegularMixins --- .../Fetch/AtomicPropertyFetchAnalyzer.php | 102 ++++++++++++------ 1 file changed, 70 insertions(+), 32 deletions(-) diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/AtomicPropertyFetchAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/AtomicPropertyFetchAnalyzer.php index 6ad6983b76e..d34a23db352 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/AtomicPropertyFetchAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/AtomicPropertyFetchAnalyzer.php @@ -265,38 +265,19 @@ public static function analyze( if (!$naive_property_exists) { if ($class_storage->namedMixins) { - foreach ($class_storage->namedMixins as $mixin) { - $new_property_id = $mixin->value . '::$' . $prop_name; - - try { - $new_class_storage = $codebase->classlike_storage_provider->get($mixin->value); - } catch (InvalidArgumentException $e) { - $new_class_storage = null; - } - - if ($new_class_storage - && ($codebase->properties->propertyExists( - $new_property_id, - !$in_assignment, - $statements_analyzer, - $context, - $codebase->collect_locations - ? new CodeLocation($statements_analyzer->getSource(), $stmt) - : null, - ) - || isset($new_class_storage->pseudo_property_get_types['$' . $prop_name])) - ) { - $fq_class_name = $mixin->value; - $lhs_type_part = $mixin; - $class_storage = $new_class_storage; - - if (!isset($new_class_storage->pseudo_property_get_types['$' . $prop_name])) { - $naive_property_exists = true; - } - - $property_id = $new_property_id; - } - } + [$lhs_type_part, $class_storage, $naive_property_exists, $property_id, $fq_class_name] + = self::handleRegularMixins( + $class_storage, + $lhs_type_part, + $prop_name, + $in_assignment, + $codebase, + $context, + $property_id, + $stmt, + $statements_analyzer, + $fq_class_name, + ); } elseif ($intersection_types !== [] && !$class_storage->final) { foreach ($intersection_types as $intersection_type) { self::analyze( @@ -1277,6 +1258,63 @@ private static function handleNonExistentProperty( ); } + /** + * @return array{TNamedObject, ClassLikeStorage, bool, bool, string, string} + */ + private static function handleRegularMixins( + ClassLikeStorage $class_storage, + TNamedObject $lhs_type_part, + string $prop_name, + bool $in_assignment, + Codebase $codebase, + Context $context, + string $property_id, + PhpParser\Node\Expr\PropertyFetch $stmt, + StatementsAnalyzer $statements_analyzer, + string $fq_class_name + ): array { + $naive_property_exists = false; + foreach ($class_storage->namedMixins as $mixin) { + $new_property_id = $mixin->value . '::$' . $prop_name; + + try { + $new_class_storage = $codebase->classlike_storage_provider->get($mixin->value); + } catch (InvalidArgumentException $e) { + $new_class_storage = null; + } + + if ($new_class_storage + && ($codebase->properties->propertyExists( + $new_property_id, + !$in_assignment, + $statements_analyzer, + $context, + $codebase->collect_locations + ? new CodeLocation($statements_analyzer->getSource(), $stmt) + : null, + ) + || isset($new_class_storage->pseudo_property_get_types['$' . $prop_name])) + ) { + $fq_class_name = $mixin->value; + $lhs_type_part = $mixin; + $class_storage = $new_class_storage; + + if (!isset($new_class_storage->pseudo_property_get_types['$' . $prop_name])) { + $naive_property_exists = true; + } + + $property_id = $new_property_id; + } + } + return [ + $lhs_type_part, + $class_storage, + $naive_property_exists, + $property_id, + $fq_class_name, + ]; + } + private static function getClassPropertyType( StatementsAnalyzer $statements_analyzer, Codebase $codebase, From 0ce84183016b32095ebfbf3a3b6de6d9ad343713 Mon Sep 17 00:00:00 2001 From: Ivan Sidorov Date: Mon, 19 Feb 2024 15:47:45 +0000 Subject: [PATCH 16/20] Part 5. Resolve test of mixins with properties Added recurcive call --- .../Fetch/AtomicPropertyFetchAnalyzer.php | 44 ++++++++++++++++++- 1 file changed, 43 insertions(+), 1 deletion(-) diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/AtomicPropertyFetchAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/AtomicPropertyFetchAnalyzer.php index d34a23db352..e2190597bed 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/AtomicPropertyFetchAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/AtomicPropertyFetchAnalyzer.php @@ -265,7 +265,7 @@ public static function analyze( if (!$naive_property_exists) { if ($class_storage->namedMixins) { - [$lhs_type_part, $class_storage, $naive_property_exists, $property_id, $fq_class_name] + [$lhs_type_part, $class_storage, , $naive_property_exists, $property_id, $fq_class_name] = self::handleRegularMixins( $class_storage, $lhs_type_part, @@ -1273,6 +1273,7 @@ private static function handleRegularMixins( StatementsAnalyzer $statements_analyzer, string $fq_class_name ): array { + $property_exists = false; $naive_property_exists = false; foreach ($class_storage->namedMixins as $mixin) { $new_property_id = $mixin->value . '::$' . $prop_name; @@ -1299,6 +1300,7 @@ private static function handleRegularMixins( $lhs_type_part = $mixin; $class_storage = $new_class_storage; + $property_exists = true; if (!isset($new_class_storage->pseudo_property_get_types['$' . $prop_name])) { $naive_property_exists = true; } @@ -1306,9 +1308,49 @@ private static function handleRegularMixins( $property_id = $new_property_id; } } + if (!$property_exists) { + foreach ($class_storage->namedMixins as $mixin) { + try { + $mixin_class_storage = $codebase->classlike_storage_provider->get($mixin->value); + } catch (InvalidArgumentException $e) { + continue; + } + $mixin_lhs_type_part = $mixin; + $mixin_fq_class_name = $mixin_class_storage->name; + + [ + $new_lhs_type_part, + $new_class_storage, + $property_exists, + $naive_property_exists, + $new_property_id, + $new_fq_class_name, + ] = self::handleRegularMixins( + $mixin_class_storage, + $mixin_lhs_type_part, + $prop_name, + $in_assignment, + $codebase, + $context, + $property_id, + $stmt, + $statements_analyzer, + $mixin_fq_class_name, + ); + + if ($property_exists) { + $fq_class_name = $new_fq_class_name; + $lhs_type_part = $new_lhs_type_part; + $class_storage = $new_class_storage; + $property_id = $new_property_id; + break; + } + } + } return [ $lhs_type_part, $class_storage, + $property_exists, $naive_property_exists, $property_id, $fq_class_name, From 635d38eccca4c048cec36fc7c4b5e42bffd4d92a Mon Sep 17 00:00:00 2001 From: Ivan Sidorov Date: Wed, 27 Mar 2024 07:07:52 +0000 Subject: [PATCH 17/20] Part 6. Fix mixin collisions on object methods --- .../Call/Method/AtomicMethodCallAnalyzer.php | 25 ++++++++-- tests/MixinsDeepTest.php | 50 +++++++++++++++++++ 2 files changed, 70 insertions(+), 5 deletions(-) diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/Method/AtomicMethodCallAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/Method/AtomicMethodCallAnalyzer.php index 41d9dafd1e4..60bac79ce80 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/Method/AtomicMethodCallAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/Method/AtomicMethodCallAnalyzer.php @@ -50,6 +50,7 @@ use function array_values; use function count; use function get_class; +use function in_array; use function reset; use function strtolower; @@ -702,6 +703,7 @@ private static function handleInvalidClass( /** * @param lowercase-string $method_name_lc + * @param string[] $ignore_mixins * @return array{TNamedObject, ClassLikeStorage, bool, bool, MethodIdentifier, string} */ private static function handleMixins( @@ -715,7 +717,8 @@ private static function handleMixins( PhpParser\Node\Expr\MethodCall $stmt, StatementsAnalyzer $statements_analyzer, string $fq_class_name, - ?string $lhs_var_id + ?string $lhs_var_id, + array $ignore_mixins = [] ): array { $method_exists = false; $naive_method_exists = false; @@ -739,6 +742,7 @@ private static function handleMixins( $statements_analyzer, $fq_class_name, $lhs_var_id, + $ignore_mixins, ); } elseif ($class_storage->mixin_declaring_fqcln && $class_storage->namedMixins @@ -756,6 +760,7 @@ private static function handleMixins( $statements_analyzer, $fq_class_name, $lhs_var_id, + $ignore_mixins, ); } @@ -771,6 +776,7 @@ private static function handleMixins( /** * @param lowercase-string $method_name_lc + * @param string[] $ignore_mixins * @return array{TNamedObject, ClassLikeStorage, bool, bool, MethodIdentifier, string} */ private static function handleTemplatedMixins( @@ -784,11 +790,14 @@ private static function handleTemplatedMixins( PhpParser\Node\Expr\MethodCall $stmt, StatementsAnalyzer $statements_analyzer, string $fq_class_name, - ?string $lhs_var_id + ?string $lhs_var_id, + array $ignore_mixins ): array { $method_exists = false; $naive_method_exists = false; + $ignore_mixins[] = $fq_class_name; + if ($class_storage->templatedMixins && $lhs_type_part instanceof TGenericObject && $class_storage->template_types @@ -845,7 +854,7 @@ private static function handleTemplatedMixins( $method_exists = isset($mixin_class_storage->pseudo_methods[$method_name_lc]); } - if (!$method_exists) { + if (!$method_exists && !in_array($mixin_fq_class_name, $ignore_mixins)) { [ $lhs_type_part_new, $mixin_class_storage, @@ -865,6 +874,7 @@ private static function handleTemplatedMixins( $statements_analyzer, $mixin_fq_class_name, $lhs_var_id, + $ignore_mixins, ); } @@ -893,6 +903,7 @@ private static function handleTemplatedMixins( /** * @param lowercase-string $method_name_lc + * @param string[] $ignore_mixins * @return array{TNamedObject, ClassLikeStorage, bool, bool, MethodIdentifier, string} */ private static function handleRegularMixins( @@ -906,11 +917,14 @@ private static function handleRegularMixins( PhpParser\Node\Expr\MethodCall $stmt, StatementsAnalyzer $statements_analyzer, string $fq_class_name, - ?string $lhs_var_id + ?string $lhs_var_id, + array $ignore_mixins ): array { $method_exists = false; $naive_method_exists = false; + $ignore_mixins[] = $fq_class_name; + foreach ($class_storage->namedMixins as $mixin) { if (!$class_storage->mixin_declaring_fqcln) { continue; @@ -980,7 +994,7 @@ private static function handleRegularMixins( $naive_method_exists = true; } - if (!$method_exists) { + if (!$method_exists && !in_array($mixin_fq_class_name, $ignore_mixins)) { [ $mixin_lhs_type_part, $mixin_class_storage, @@ -1000,6 +1014,7 @@ private static function handleRegularMixins( $statements_analyzer, $mixin_fq_class_name, $lhs_var_id, + $ignore_mixins, ); } diff --git a/tests/MixinsDeepTest.php b/tests/MixinsDeepTest.php index 2e382f2bf31..b6d24dcb159 100644 --- a/tests/MixinsDeepTest.php +++ b/tests/MixinsDeepTest.php @@ -344,6 +344,56 @@ public function __call(string $name, array $arguments) {} '$b' => 'int', ], ], + 'LowMixinCollision_WithObjectMethods' => [ + 'code' => <<<'PHP' + notExistsMethod(); + PHP, + 'assertions' => [ + '$a' => 'mixed', + ], + 'ignored_issues' => ['MixedAssignment'], + ], + 'DeepMixinCollision_WithObjectMethods' => [ + 'code' => <<<'PHP' + notExistsMethod(); + PHP, + 'assertions' => [ + '$a' => 'mixed', + ], + 'ignored_issues' => ['MixedAssignment'], + ], ]; } } From 83b437af53f2489a335e0161cff46b78213dabe2 Mon Sep 17 00:00:00 2001 From: Ivan Sidorov Date: Wed, 27 Mar 2024 07:28:44 +0000 Subject: [PATCH 18/20] Part 6. Fix mixin collisions on static methods --- .../StaticMethod/AtomicStaticCallAnalyzer.php | 11 ++++- tests/MixinsDeepTest.php | 48 +++++++++++++++++++ 2 files changed, 58 insertions(+), 1 deletion(-) diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticMethod/AtomicStaticCallAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticMethod/AtomicStaticCallAnalyzer.php index 094602ff91b..d571f83f3d3 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticMethod/AtomicStaticCallAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticMethod/AtomicStaticCallAnalyzer.php @@ -925,6 +925,7 @@ private static function checkPseudoMethod( /** * @param lowercase-string $method_name_lc + * @param string[] $ignore_mixins */ private static function handleRegularMixins( ClassLikeStorage $class_storage, @@ -934,11 +935,15 @@ private static function handleRegularMixins( Context $context, PhpParser\Node\Identifier $stmt_name, StatementsAnalyzer $statements_analyzer, - string $fq_class_name + string $fq_class_name, + array $ignore_mixins = [] ): ?Union { if ($class_storage->mixin_declaring_fqcln === null) { return null; } + + $ignore_mixins[] = $fq_class_name; + foreach ($class_storage->namedMixins as $mixin) { $new_method_id = new MethodIdentifier( $mixin->value, @@ -1023,6 +1028,9 @@ private static function handleRegularMixins( } $mixin_class_storage = $codebase->classlike_storage_provider->get($mixin->value); $mixin_fq_class_name = $mixin_class_storage->name; + if (in_array($mixin_fq_class_name, $ignore_mixins)) { + continue; + } $new_lhs_type = self::handleRegularMixins( $mixin_class_storage, $lhs_type_part, @@ -1032,6 +1040,7 @@ private static function handleRegularMixins( $stmt_name, $statements_analyzer, $mixin_fq_class_name, + $ignore_mixins, ); if ($new_lhs_type) { return $new_lhs_type; diff --git a/tests/MixinsDeepTest.php b/tests/MixinsDeepTest.php index b6d24dcb159..98c48ddb50b 100644 --- a/tests/MixinsDeepTest.php +++ b/tests/MixinsDeepTest.php @@ -394,6 +394,54 @@ public function __call(string $name, array $arguments) {} ], 'ignored_issues' => ['MixedAssignment'], ], + 'LowMixinCollision_WithStaticMethods' => [ + 'code' => <<<'PHP' + [ + '$a' => 'mixed', + ], + 'ignored_issues' => ['MixedAssignment'], + ], + 'DeepMixinCollision_WithStaticMethods' => [ + 'code' => <<<'PHP' + [ + '$a' => 'mixed', + ], + 'ignored_issues' => ['MixedAssignment'], + ], ]; } } From b206f1a523cb83e488bc099566462a43b4971fe8 Mon Sep 17 00:00:00 2001 From: Ivan Sidorov Date: Wed, 27 Mar 2024 07:41:22 +0000 Subject: [PATCH 19/20] Part 6. Fix mixin collisions on properties --- .../Fetch/AtomicPropertyFetchAnalyzer.php | 11 +++- tests/MixinsDeepTest.php | 50 +++++++++++++++++++ 2 files changed, 60 insertions(+), 1 deletion(-) diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/AtomicPropertyFetchAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/AtomicPropertyFetchAnalyzer.php index e2190597bed..b788567553b 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/AtomicPropertyFetchAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/AtomicPropertyFetchAnalyzer.php @@ -1259,6 +1259,7 @@ private static function handleNonExistentProperty( } /** + * @param string[] $ignore_mixins * @return array{TNamedObject, ClassLikeStorage, bool, bool, string, string} */ private static function handleRegularMixins( @@ -1271,10 +1272,14 @@ private static function handleRegularMixins( string $property_id, PhpParser\Node\Expr\PropertyFetch $stmt, StatementsAnalyzer $statements_analyzer, - string $fq_class_name + string $fq_class_name, + array $ignore_mixins = [] ): array { $property_exists = false; $naive_property_exists = false; + + $ignore_mixins[] = $fq_class_name; + foreach ($class_storage->namedMixins as $mixin) { $new_property_id = $mixin->value . '::$' . $prop_name; @@ -1317,6 +1322,9 @@ private static function handleRegularMixins( } $mixin_lhs_type_part = $mixin; $mixin_fq_class_name = $mixin_class_storage->name; + if (in_array($mixin_fq_class_name, $ignore_mixins)) { + continue; + } [ $new_lhs_type_part, @@ -1336,6 +1344,7 @@ private static function handleRegularMixins( $stmt, $statements_analyzer, $mixin_fq_class_name, + $ignore_mixins, ); if ($property_exists) { diff --git a/tests/MixinsDeepTest.php b/tests/MixinsDeepTest.php index 98c48ddb50b..55961d10171 100644 --- a/tests/MixinsDeepTest.php +++ b/tests/MixinsDeepTest.php @@ -442,6 +442,56 @@ public static function __callStatic(string $name, array $arguments) {} ], 'ignored_issues' => ['MixedAssignment'], ], + 'LowMixinCollision_WithProperties' => [ + 'code' => <<<'PHP' + notExistsProp; + PHP, + 'assertions' => [ + '$a' => 'mixed', + ], + 'ignored_issues' => ['MixedAssignment'], + ], + 'DeepMixinCollision_WithProperties' => [ + 'code' => <<<'PHP' + notExistsProp; + PHP, + 'assertions' => [ + '$a' => 'mixed', + ], + 'ignored_issues' => ['MixedAssignment'], + ], ]; } } From 021c77bbc92c5d25fd698cc0d716a6ca01aaf917 Mon Sep 17 00:00:00 2001 From: Ivan Sidorov Date: Thu, 12 Sep 2024 04:41:09 +0000 Subject: [PATCH 20/20] Part 7. Success regression tests with UndefinedMixinClass --- tests/MixinAnnotationTest.php | 9 ++ tests/MixinsDeepTest.php | 151 ++++++++++++++++++++++++++++++++++ 2 files changed, 160 insertions(+) diff --git a/tests/MixinAnnotationTest.php b/tests/MixinAnnotationTest.php index 3c0c11b26bb..41a03937f49 100644 --- a/tests/MixinAnnotationTest.php +++ b/tests/MixinAnnotationTest.php @@ -603,12 +603,14 @@ public function providerInvalidCodeParse(): iterable { return [ 'undefinedMixinClass' => [ + // Similar test in MixinsDeepTest.php 'code' => ' 'UndefinedDocblockClass', ], 'undefinedMixinClassWithPropertyFetch' => [ + // Similar test in MixinsDeepTest.php 'code' => ' 'UndefinedPropertyFetch', ], 'undefinedMixinClassWithPropertyFetch_WithMagicMethod' => [ + // Similar test in MixinsDeepTest.php 'code' => ' 'UndefinedMagicPropertyFetch', ], 'undefinedMixinClassWithPropertyAssignment' => [ + // Similar test in MixinsDeepTest.php 'code' => ' 'UndefinedPropertyAssignment', ], 'undefinedMixinClassWithPropertyAssignment_WithMagicMethod' => [ + // Similar test in MixinsDeepTest.php 'code' => ' 'UndefinedMagicPropertyAssignment', ], 'undefinedMixinClassWithMethodCall' => [ + // Similar test in MixinsDeepTest.php 'code' => ' 'UndefinedMethod', ], 'undefinedMixinClassWithMethodCall_WithMagicMethod' => [ + // Similar test in MixinsDeepTest.php 'code' => ' 'UndefinedMagicMethod', ], 'undefinedMixinClassWithStaticMethodCall' => [ + // Similar test in MixinsDeepTest.php 'code' => ' 'UndefinedMethod', ], 'undefinedMixinClassWithStaticMethodCall_WithMagicMethod' => [ + // Similar test in MixinsDeepTest.php 'code' => ' [ + // Similar test in MixinAnnotationTest.php + 'code' => ' 'UndefinedDocblockClass', + ], + 'undefinedMixinClassWithPropertyFetch' => [ + // Similar test in MixinAnnotationTest.php + 'code' => 'foo;', + 'error_message' => 'UndefinedPropertyFetch', + ], + 'undefinedMixinClassWithPropertyFetch_WithMagicMethod' => [ + // Similar test in MixinAnnotationTest.php + 'code' => 'foo;', + 'error_message' => 'UndefinedMagicPropertyFetch', + ], + 'undefinedMixinClassWithPropertyAssignment' => [ + // Similar test in MixinAnnotationTest.php + 'code' => 'foo = "bar";', + 'error_message' => 'UndefinedPropertyAssignment', + ], + 'undefinedMixinClassWithPropertyAssignment_WithMagicMethod' => [ + // Similar test in MixinAnnotationTest.php + 'code' => 'foo = "bar";', + 'error_message' => 'UndefinedMagicPropertyAssignment', + ], + 'undefinedMixinClassWithMethodCall' => [ + // Similar test in MixinAnnotationTest.php + 'code' => 'foo();', + 'error_message' => 'UndefinedMethod', + ], + 'undefinedMixinClassWithMethodCall_WithMagicMethod' => [ + // Similar test in MixinAnnotationTest.php + 'code' => 'foo();', + 'error_message' => 'UndefinedMagicMethod', + ], + 'undefinedMixinClassWithStaticMethodCall' => [ + // Similar test in MixinAnnotationTest.php + 'code' => ' 'UndefinedMethod', + ], + 'undefinedMixinClassWithStaticMethodCall_WithMagicMethod' => [ + // Similar test in MixinAnnotationTest.php + 'code' => ' 'UndefinedMagicMethod', + ], + ]; + } }