From f6f7c20f36162395e231390aa212d4a052453ffe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maximilian=20B=C3=B6sing?= <2189546+boesing@users.noreply.github.com> Date: Sun, 10 Sep 2023 15:21:14 +0200 Subject: [PATCH 1/4] qa: extend inherited assertion test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Maximilian Bösing <2189546+boesing@users.noreply.github.com> --- tests/AssertAnnotationTest.php | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/tests/AssertAnnotationTest.php b/tests/AssertAnnotationTest.php index 7bba9297f2c..f64101d63dd 100644 --- a/tests/AssertAnnotationTest.php +++ b/tests/AssertAnnotationTest.php @@ -2942,7 +2942,8 @@ public function validate(mixed $value): void } namespace Namespace2 { - use Namespace1\AbstractSingleInstancePluginManager; + use InvalidArgumentException;use Namespace1\AbstractSingleInstancePluginManager; + use Namespace1\AbstractPluginManager; use stdClass; /** @template-extends AbstractSingleInstancePluginManager */ @@ -2951,6 +2952,14 @@ final class Qoo extends AbstractSingleInstancePluginManager /** @var class-string */ protected string $instanceOf = stdClass::class; } + + /** @template-extends AbstractPluginManager */ + final class Ooq extends AbstractPluginManager + { + public function validate(mixed $value): void + { + } + } } namespace { @@ -2959,10 +2968,16 @@ final class Qoo extends AbstractSingleInstancePluginManager /** @var mixed $object */ $object = null; $baz->validate($object); + + $ooq = new \Namespace2\Ooq(function (): void {}); + /** @var mixed $callable */ + $callable = null; + $ooq->validate($callable); } ', 'assertions' => [ '$object===' => 'stdClass', + '$callable===' => 'callable', ], 'ignored_issues' => [], 'php_version' => '8.1', From ad463f38e9e0ef26b0e4a46148612bf662f18ba5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maximilian=20B=C3=B6sing?= <2189546+boesing@users.noreply.github.com> Date: Sun, 10 Sep 2023 15:22:21 +0200 Subject: [PATCH 2/4] bugfix: always gather class templates MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit No clue why there are conditions on when templates are allowed to get picked up. I've removed this check which actually solves a problem in inherited assertions. Signed-off-by: Maximilian Bösing <2189546+boesing@users.noreply.github.com> --- .../Statements/Expression/Call/ClassTemplateParamCollector.php | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/ClassTemplateParamCollector.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/ClassTemplateParamCollector.php index 6802fa495db..c3bbd2995b7 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/ClassTemplateParamCollector.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/ClassTemplateParamCollector.php @@ -48,9 +48,6 @@ public static function collect( if ($static_class_storage->template_extended_params && $method_name && !empty($non_trait_class_storage->overridden_method_ids[$method_name]) - && isset($class_storage->methods[$method_name]) - && (!isset($non_trait_class_storage->methods[$method_name]->return_type) - || $class_storage->methods[$method_name]->inherited_return_type) ) { foreach ($non_trait_class_storage->overridden_method_ids[$method_name] as $overridden_method_id) { $overridden_storage = $codebase->methods->getStorage($overridden_method_id); From e3920e6f4dd7d3707b6f16c0a571a047005eccba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maximilian=20B=C3=B6sing?= <2189546+boesing@users.noreply.github.com> Date: Sun, 10 Sep 2023 15:22:36 +0200 Subject: [PATCH 3/4] qa: remove unnecessary type juggling regarding inherited assertions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Maximilian Bösing <2189546+boesing@users.noreply.github.com> --- .../AssertionsFromInheritanceResolver.php | 77 +------------------ 1 file changed, 1 insertion(+), 76 deletions(-) diff --git a/src/Psalm/Internal/Codebase/AssertionsFromInheritanceResolver.php b/src/Psalm/Internal/Codebase/AssertionsFromInheritanceResolver.php index 489c88f3644..aa42e2a8942 100644 --- a/src/Psalm/Internal/Codebase/AssertionsFromInheritanceResolver.php +++ b/src/Psalm/Internal/Codebase/AssertionsFromInheritanceResolver.php @@ -5,17 +5,13 @@ namespace Psalm\Internal\Codebase; use Psalm\Codebase; -use Psalm\Storage\Assertion\IsType; use Psalm\Storage\ClassLikeStorage; use Psalm\Storage\MethodStorage; use Psalm\Storage\Possibilities; -use Psalm\Type\Atomic\TTemplateParam; use function array_filter; -use function array_map; use function array_merge; use function array_values; -use function reset; use function strtolower; /** @@ -61,80 +57,9 @@ public function resolve( * Since the inheritance does not provide its own assertions, we have to detect those * from inherited classes */ - $assertions += array_map( - fn(Possibilities $possibilities) => $this->modifyAssertionsForInheritance( - $possibilities, - $this->codebase, - $called_class, - $inherited_classes_and_interfaces, - ), - $potential_assertion_providing_method_storage->assertions, - ); + $assertions += $potential_assertion_providing_method_storage->assertions; } return $assertions; } - - /** - * In case the called class is either implementing or extending a class/interface which does also has the - * template we are searching for, we assume that the called method has the same assertions. - * - * @param list $potential_assertion_providing_classes - */ - private function modifyAssertionsForInheritance( - Possibilities $possibilities, - Codebase $codebase, - ClassLikeStorage $called_class, - array $potential_assertion_providing_classes - ): Possibilities { - $replacement = new Possibilities($possibilities->var_id, []); - $extended_params = $called_class->template_extended_params; - foreach ($possibilities->rule as $assertion) { - if (!$assertion instanceof IsType - || !$assertion->type instanceof TTemplateParam) { - $replacement->rule[] = $assertion; - continue; - } - - /** Called class does not extend the template parameter */ - $extended_templates = $called_class->template_extended_params; - if (!isset($extended_templates[$assertion->type->defining_class][$assertion->type->param_name])) { - $replacement->rule[] = $assertion; - continue; - } - - foreach ($potential_assertion_providing_classes as $potential_assertion_providing_class) { - if (!isset($extended_params[$potential_assertion_providing_class][$assertion->type->param_name])) { - continue; - } - - if (!$codebase->classlike_storage_provider->has($potential_assertion_providing_class)) { - continue; - } - - $potential_assertion_providing_classlike_storage = $codebase->classlike_storage_provider->get( - $potential_assertion_providing_class, - ); - if (!isset( - $potential_assertion_providing_classlike_storage->template_types[$assertion->type->param_name], - )) { - continue; - } - - $replacement->rule[] = new IsType(new TTemplateParam( - $assertion->type->param_name, - reset( - $potential_assertion_providing_classlike_storage->template_types[$assertion->type->param_name], - ), - $potential_assertion_providing_class, - )); - - continue 2; - } - - $replacement->rule[] = $assertion; - } - - return $replacement; - } } From 56b719b1e65ebeb4b8df5a02a61eb322874d54ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maximilian=20B=C3=B6sing?= <2189546+boesing@users.noreply.github.com> Date: Sun, 10 Sep 2023 16:11:22 +0200 Subject: [PATCH 4/4] qa: remove unnecessary lines from tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Maximilian Bösing <2189546+boesing@users.noreply.github.com> --- tests/AssertAnnotationTest.php | 33 ++------------------------------- 1 file changed, 2 insertions(+), 31 deletions(-) diff --git a/tests/AssertAnnotationTest.php b/tests/AssertAnnotationTest.php index f64101d63dd..650edd663ed 100644 --- a/tests/AssertAnnotationTest.php +++ b/tests/AssertAnnotationTest.php @@ -2890,9 +2890,6 @@ public static function doAssert($value): void /** @template InstanceType */ interface PluginManagerInterface { - /** @return InstanceType */ - public function get(): mixed; - /** @psalm-assert InstanceType $value */ public function validate(mixed $value): void; } @@ -2903,15 +2900,6 @@ public function validate(mixed $value): void; */ abstract class AbstractPluginManager implements PluginManagerInterface { - /** @param InstanceType $value */ - public function __construct(private readonly mixed $value) - {} - - /** {@inheritDoc} */ - public function get(): mixed - { - return $this->value; - } } /** @@ -2920,21 +2908,6 @@ public function get(): mixed */ abstract class AbstractSingleInstancePluginManager extends AbstractPluginManager { - /** - * An object type that the created instance must be instanced of - * - * @var class-string - */ - protected string $instanceOf; - - /** {@inheritDoc} */ - public function get(): object - { - return parent::get(); - } - - - /** {@inheritDoc} */ public function validate(mixed $value): void { } @@ -2949,8 +2922,6 @@ public function validate(mixed $value): void /** @template-extends AbstractSingleInstancePluginManager */ final class Qoo extends AbstractSingleInstancePluginManager { - /** @var class-string */ - protected string $instanceOf = stdClass::class; } /** @template-extends AbstractPluginManager */ @@ -2963,13 +2934,13 @@ public function validate(mixed $value): void } namespace { - $baz = new \Namespace2\Qoo(new stdClass); + $baz = new \Namespace2\Qoo(); /** @var mixed $object */ $object = null; $baz->validate($object); - $ooq = new \Namespace2\Ooq(function (): void {}); + $ooq = new \Namespace2\Ooq(); /** @var mixed $callable */ $callable = null; $ooq->validate($callable);