From 4c95d32c26cb7813583ac02cd058d8917808847a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Nowak?= Date: Tue, 13 Aug 2024 11:21:56 +0200 Subject: [PATCH 01/14] base version of handling query result --- extension.neon | 12 ++ src/Symfony/Message.php | 35 +++++ src/Symfony/MessageMap.php | 31 +++++ src/Symfony/MessageMapFactory.php | 124 ++++++++++++++++++ src/Symfony/Service.php | 11 +- src/Symfony/ServiceDefinition.php | 1 + src/Symfony/ServiceTag.php | 28 ++++ src/Symfony/ServiceTagDefinition.php | 10 ++ src/Symfony/XmlServiceMapFactory.php | 12 +- ...rHandleTraitDynamicReturnTypeExtension.php | 82 ++++++++++++ tests/Type/Symfony/ExtensionTest.php | 1 + tests/Type/Symfony/container.xml | 6 + .../Symfony/data/messenger_handle_trait.php | 67 ++++++++++ 13 files changed, 418 insertions(+), 2 deletions(-) create mode 100644 src/Symfony/Message.php create mode 100644 src/Symfony/MessageMap.php create mode 100644 src/Symfony/MessageMapFactory.php create mode 100644 src/Symfony/ServiceTag.php create mode 100644 src/Symfony/ServiceTagDefinition.php create mode 100644 src/Type/Symfony/MessengerHandleTraitDynamicReturnTypeExtension.php create mode 100644 tests/Type/Symfony/data/messenger_handle_trait.php diff --git a/extension.neon b/extension.neon index 4868bc2d..f6b8282e 100644 --- a/extension.neon +++ b/extension.neon @@ -140,6 +140,13 @@ services: - factory: @symfony.parameterMapFactory::create() + # message map + symfony.messageMapFactory: + class: PHPStan\Symfony\MessageMapFactory + factory: PHPStan\Symfony\MessageMapFactory + - + factory: @symfony.messageMapFactory::create() + # ControllerTrait::get()/has() return type - factory: PHPStan\Type\Symfony\ServiceDynamicReturnTypeExtension(Symfony\Component\DependencyInjection\ContainerInterface) @@ -203,6 +210,11 @@ services: factory: PHPStan\Type\Symfony\EnvelopeReturnTypeExtension tags: [phpstan.broker.dynamicMethodReturnTypeExtension] + # Messenger HandleTrait::handle() return type + - + factory: PHPStan\Type\Symfony\MessengerHandleTraitDynamicReturnTypeExtension + tags: [phpstan.broker.dynamicMethodReturnTypeExtension] + # InputInterface::getArgument() return type - factory: PHPStan\Type\Symfony\InputInterfaceGetArgumentDynamicReturnTypeExtension diff --git a/src/Symfony/Message.php b/src/Symfony/Message.php new file mode 100644 index 00000000..c53958af --- /dev/null +++ b/src/Symfony/Message.php @@ -0,0 +1,35 @@ +class = $class; + $this->returnTypes = $returnTypes; + } + + public function getClass(): string + { + return $this->class; + } + + public function getReturnTypes(): array + { + return $this->returnTypes; + } + + public function countReturnTypes(): int + { + return count($this->returnTypes); + } +} diff --git a/src/Symfony/MessageMap.php b/src/Symfony/MessageMap.php new file mode 100644 index 00000000..eaeaadee --- /dev/null +++ b/src/Symfony/MessageMap.php @@ -0,0 +1,31 @@ +messages[$message->getClass()] = $message; + } + } + + public function getMessageForClass(string $class): ?Message + { + return $this->messages[$class] ?? null; + } + + public function hasMessageForClass(string $class): bool + { + return array_key_exists($class, $this->messages); + } +} diff --git a/src/Symfony/MessageMapFactory.php b/src/Symfony/MessageMapFactory.php new file mode 100644 index 00000000..e774c34b --- /dev/null +++ b/src/Symfony/MessageMapFactory.php @@ -0,0 +1,124 @@ +serviceMap = $symfonyServiceMap; + $this->reflectionProvider = $reflectionProvider; + } + + public function create(): MessageMap + { + $returnTypesMap = []; + + foreach ($this->serviceMap->getServices() as $service) { + $serviceClass = $service->getClass(); + + // todo handle abstract/parent services somehow? + if (is_null($serviceClass)) { + continue; + } + + foreach ($service->getTags() as $tag) { + // todo could there be more tags with the same name for the same service? + // todo check if message handler tag name is constant or configurable + if ($tag->getName() !== 'messenger.message_handler') { + continue; + } + + $tagAttributes = $tag->getAttributes(); + $reflectionClass = $this->reflectionProvider->getClass($serviceClass); + + if (isset($tagAttributes['handles'])) { + $handles = isset($tag['method']) ? [$tag['handles'] => $tag['method']] : [$tag['handles']]; + } else { + $handles = $this->guessHandledMessages($reflectionClass); + } + + foreach ($handles as $messageClassName => $options) { + if (\is_int($messageClassName) && \is_string($options)) { + $messageClassName = $options; + $options = []; + } + + $options['method'] = $options['method'] ?? '__invoke'; + + $methodReflection = $reflectionClass->getNativeMethod($options['method']); + $variant = ParametersAcceptorSelector::selectSingle($methodReflection->getVariants()); + + $returnTypesMap[$messageClassName][] = $variant->getReturnType(); + } + } + } + + $messages = []; + foreach ($returnTypesMap as $messageClassName => $returnTypes) { + $messages[] = new Message($messageClassName, $returnTypes); + } + + return new MessageMap($messages); + } + + private function guessHandledMessages(ClassReflection $reflectionClass): iterable + { + if ($reflectionClass->implementsInterface(MessageSubscriberInterface::class)) { + // todo handle different return formats + return $reflectionClass->getName()::getHandledMessages(); + } + + // todo handle if doesn't exists + $methodReflection = $reflectionClass->getNativeMethod('__invoke'); + + $variant = ParametersAcceptorSelector::selectSingle($methodReflection->getVariants()); + $parameters = $variant->getParameters(); + + if (1 !== count($parameters)) { + // todo handle error + throw new \RuntimeException('invalid handler'); + } + + $type = $parameters[0]->getType(); + + if ($type instanceof UnionType) { + $types = []; + foreach ($type->getTypes() as $type) { + if (!$type instanceof ObjectType) { + // todo handle error + throw new \RuntimeException('invalid handler'); + } + + $types[] = $type->getClassName(); + } + + if ($types) { + return $types; + } + + // todo handle error + throw new \RuntimeException('invalid handler'); + } + + if (!$type instanceof ObjectType) { + throw new \RuntimeException('invalid handler'); + } + + return [$type->getClassName()]; + } +} diff --git a/src/Symfony/Service.php b/src/Symfony/Service.php index c31324f5..5d72c3e4 100644 --- a/src/Symfony/Service.php +++ b/src/Symfony/Service.php @@ -20,12 +20,16 @@ final class Service implements ServiceDefinition /** @var string|null */ private $alias; + /** @var array */ + private $tags; + public function __construct( string $id, ?string $class, bool $public, bool $synthetic, - ?string $alias + ?string $alias, + array $tags = [] ) { $this->id = $id; @@ -33,6 +37,7 @@ public function __construct( $this->public = $public; $this->synthetic = $synthetic; $this->alias = $alias; + $this->tags = $tags; } public function getId(): string @@ -60,4 +65,8 @@ public function getAlias(): ?string return $this->alias; } + public function getTags(): array + { + return $this->tags; + } } diff --git a/src/Symfony/ServiceDefinition.php b/src/Symfony/ServiceDefinition.php index 6df34cba..1b7cea55 100644 --- a/src/Symfony/ServiceDefinition.php +++ b/src/Symfony/ServiceDefinition.php @@ -18,4 +18,5 @@ public function isSynthetic(): bool; public function getAlias(): ?string; + public function getTags(): array; } diff --git a/src/Symfony/ServiceTag.php b/src/Symfony/ServiceTag.php new file mode 100644 index 00000000..880a72cd --- /dev/null +++ b/src/Symfony/ServiceTag.php @@ -0,0 +1,28 @@ +name = $name; + $this->attributes = $attributes; + } + + public function getName(): string + { + return $this->name; + } + + public function getAttributes(): array + { + return $this->attributes; + } +} diff --git a/src/Symfony/ServiceTagDefinition.php b/src/Symfony/ServiceTagDefinition.php new file mode 100644 index 00000000..f2154db4 --- /dev/null +++ b/src/Symfony/ServiceTagDefinition.php @@ -0,0 +1,10 @@ +tag as $tag) { + $tagAttrs = ((array) $tag->attributes())['@attributes'] ?? []; + $tagName = $tagAttrs['name']; + unset($tagAttrs['name']); + + $serviceTags[] = new ServiceTag($tagName, $tagAttrs); + } + $service = new Service( $this->cleanServiceId((string) $attrs->id), isset($attrs->class) ? (string) $attrs->class : null, isset($attrs->public) && (string) $attrs->public === 'true', isset($attrs->synthetic) && (string) $attrs->synthetic === 'true', - isset($attrs->alias) ? $this->cleanServiceId((string) $attrs->alias) : null + isset($attrs->alias) ? $this->cleanServiceId((string) $attrs->alias) : null, + $serviceTags ); if ($service->getAlias() !== null) { diff --git a/src/Type/Symfony/MessengerHandleTraitDynamicReturnTypeExtension.php b/src/Type/Symfony/MessengerHandleTraitDynamicReturnTypeExtension.php new file mode 100644 index 00000000..6462a14c --- /dev/null +++ b/src/Type/Symfony/MessengerHandleTraitDynamicReturnTypeExtension.php @@ -0,0 +1,82 @@ +messageMapFactory = $symfonyMessageMapFactory; + } + + public function getClass(): string + { + // todo traits are not supported yet in phpstan for this extension (https://github.com/phpstan/phpstan/issues/5761) + // return HandleTrait::class; + + // todo or make it configurable with passing concrete classes names to extension config + // todo or use reflection somehow to get all classes that use HandleTrait and configure it dynamically + + // todo temporarily hardcoded test class here + return HandleTraitClass::class; + } + + public function isMethodSupported(MethodReflection $methodReflection): bool + { + // todo additional reflection checker that it comes only from trait? + return $methodReflection->getName() === 'handle'; + } + + public function getTypeFromMethodCall(MethodReflection $methodReflection, MethodCall $methodCall, Scope $scope): ?Type + { + // todo handle different cases: + // - [X] regular message classes + // - [ ] interfaces for message classes + // - [ ] many handlers for one message? it would throw exception in HandleTrait anyway + // - [x] many messages for one handler + // - [partially] cover MessageSubscriberInterface + // - [partially] custom method names for handlers (different than default "__invoke" magic method) + // - [] read SF doc to determine any other cases to covers + + $arg = $methodCall->getArgs()[0]->value; + $argType = $scope->getType($arg); + + if ($argType instanceof ObjectType) { + $messageMap = $this->getMessageMap(); + if ($messageMap->hasMessageForClass($argType->getClassName())) { + $message = $messageMap->getMessageForClass($argType->getClassName()); + + if (1 === $message->countReturnTypes()) { + return $message->getReturnTypes()[0]; + } + } + } + + return null; + } + + private function getMessageMap(): MessageMap + { + if (!$this->messageMap) { + $this->messageMap = $this->messageMapFactory->create(); + } + + return $this->messageMap; + } +} diff --git a/tests/Type/Symfony/ExtensionTest.php b/tests/Type/Symfony/ExtensionTest.php index a076caac..85bbb140 100644 --- a/tests/Type/Symfony/ExtensionTest.php +++ b/tests/Type/Symfony/ExtensionTest.php @@ -14,6 +14,7 @@ class ExtensionTest extends TypeInferenceTestCase /** @return mixed[] */ public function dataFileAsserts(): iterable { + yield from $this->gatherAssertTypes(__DIR__ . '/data/messenger_handle_trait.php'); yield from $this->gatherAssertTypes(__DIR__ . '/data/envelope_all.php'); yield from $this->gatherAssertTypes(__DIR__ . '/data/header_bag_get.php'); yield from $this->gatherAssertTypes(__DIR__ . '/data/response_header_bag_get_cookies.php'); diff --git a/tests/Type/Symfony/container.xml b/tests/Type/Symfony/container.xml index 224c72db..07ddc824 100644 --- a/tests/Type/Symfony/container.xml +++ b/tests/Type/Symfony/container.xml @@ -354,5 +354,11 @@ + + + + + + diff --git a/tests/Type/Symfony/data/messenger_handle_trait.php b/tests/Type/Symfony/data/messenger_handle_trait.php new file mode 100644 index 00000000..7728685e --- /dev/null +++ b/tests/Type/Symfony/data/messenger_handle_trait.php @@ -0,0 +1,67 @@ + ['method' => 'handleInt']; + yield FloatQuery::class => ['method' => 'handleFloat']; + yield StringQuery::class => ['method' => 'handleString']; + } + + public function __invoke(StringQuery $query): string + { + return 'string result'; + } + + public function handleInt(IntQuery $query): int + { + return 0; + } + + public function handleFloat(FloatQuery $query): float + { + return 0.0; + } + + public function handleString(StringQuery $query): string + { + return 'string result'; + } + + // todo add handle method with union return type? +} + +class HandleTraitClass { + use HandleTrait; + + public function __invoke() + { + assertType(RegularQueryResult::class, $this->handle(new RegularQuery())); + assertType('int', $this->handle(new IntQuery())); + assertType('float', $this->handle(new FloatQuery())); + + // HandleTrait will throw exception in fact due to multiple handlers per single query + assertType('mixed', $this->handle(new StringQuery())); + } +} From 59853624c42aaed66e38db7d218ccb599ce3505b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Nowak?= Date: Tue, 13 Aug 2024 11:37:53 +0200 Subject: [PATCH 02/14] coding standards fix --- src/Symfony/Message.php | 60 ++--- src/Symfony/MessageMap.php | 52 ++-- src/Symfony/MessageMapFactory.php | 227 +++++++++--------- src/Symfony/Service.php | 15 +- src/Symfony/ServiceDefinition.php | 3 +- src/Symfony/ServiceTag.php | 46 ++-- src/Symfony/ServiceTagDefinition.php | 8 +- src/Symfony/XmlServiceMapFactory.php | 16 +- ...rHandleTraitDynamicReturnTypeExtension.php | 134 ++++++----- tests/Type/Symfony/ExtensionTest.php | 2 +- 10 files changed, 291 insertions(+), 272 deletions(-) diff --git a/src/Symfony/Message.php b/src/Symfony/Message.php index c53958af..dac5b23c 100644 --- a/src/Symfony/Message.php +++ b/src/Symfony/Message.php @@ -1,35 +1,37 @@ -class = $class; - $this->returnTypes = $returnTypes; - } - - public function getClass(): string - { - return $this->class; - } - - public function getReturnTypes(): array - { - return $this->returnTypes; - } - - public function countReturnTypes(): int - { - return count($this->returnTypes); - } + + /** @var string */ + private $class; + + /** @var array */ + private $returnTypes; + + public function __construct(string $class, array $returnTypes) + { + $this->class = $class; + $this->returnTypes = $returnTypes; + } + + public function getClass(): string + { + return $this->class; + } + + public function getReturnTypes(): array + { + return $this->returnTypes; + } + + public function countReturnTypes(): int + { + return count($this->returnTypes); + } + } diff --git a/src/Symfony/MessageMap.php b/src/Symfony/MessageMap.php index eaeaadee..a96f3521 100644 --- a/src/Symfony/MessageMap.php +++ b/src/Symfony/MessageMap.php @@ -1,31 +1,33 @@ -messages[$message->getClass()] = $message; - } - } - - public function getMessageForClass(string $class): ?Message - { - return $this->messages[$class] ?? null; - } - - public function hasMessageForClass(string $class): bool - { - return array_key_exists($class, $this->messages); - } + + /** @var Message[] */ + private $messages = []; + + /** + * @param Message[] $messages + */ + public function __construct(array $messages) + { + foreach ($messages as $message) { + $this->messages[$message->getClass()] = $message; + } + } + + public function getMessageForClass(string $class): ?Message + { + return $this->messages[$class] ?? null; + } + + public function hasMessageForClass(string $class): bool + { + return array_key_exists($class, $this->messages); + } + } diff --git a/src/Symfony/MessageMapFactory.php b/src/Symfony/MessageMapFactory.php index e774c34b..896f4674 100644 --- a/src/Symfony/MessageMapFactory.php +++ b/src/Symfony/MessageMapFactory.php @@ -1,4 +1,4 @@ -serviceMap = $symfonyServiceMap; - $this->reflectionProvider = $reflectionProvider; - } - - public function create(): MessageMap - { - $returnTypesMap = []; - - foreach ($this->serviceMap->getServices() as $service) { - $serviceClass = $service->getClass(); - - // todo handle abstract/parent services somehow? - if (is_null($serviceClass)) { - continue; - } - - foreach ($service->getTags() as $tag) { - // todo could there be more tags with the same name for the same service? - // todo check if message handler tag name is constant or configurable - if ($tag->getName() !== 'messenger.message_handler') { - continue; - } - - $tagAttributes = $tag->getAttributes(); - $reflectionClass = $this->reflectionProvider->getClass($serviceClass); - - if (isset($tagAttributes['handles'])) { - $handles = isset($tag['method']) ? [$tag['handles'] => $tag['method']] : [$tag['handles']]; - } else { - $handles = $this->guessHandledMessages($reflectionClass); - } - - foreach ($handles as $messageClassName => $options) { - if (\is_int($messageClassName) && \is_string($options)) { - $messageClassName = $options; - $options = []; - } - - $options['method'] = $options['method'] ?? '__invoke'; - - $methodReflection = $reflectionClass->getNativeMethod($options['method']); - $variant = ParametersAcceptorSelector::selectSingle($methodReflection->getVariants()); - - $returnTypesMap[$messageClassName][] = $variant->getReturnType(); - } - } - } - - $messages = []; - foreach ($returnTypesMap as $messageClassName => $returnTypes) { - $messages[] = new Message($messageClassName, $returnTypes); - } - - return new MessageMap($messages); - } - - private function guessHandledMessages(ClassReflection $reflectionClass): iterable - { - if ($reflectionClass->implementsInterface(MessageSubscriberInterface::class)) { - // todo handle different return formats - return $reflectionClass->getName()::getHandledMessages(); - } - - // todo handle if doesn't exists - $methodReflection = $reflectionClass->getNativeMethod('__invoke'); - - $variant = ParametersAcceptorSelector::selectSingle($methodReflection->getVariants()); - $parameters = $variant->getParameters(); - - if (1 !== count($parameters)) { - // todo handle error - throw new \RuntimeException('invalid handler'); - } - - $type = $parameters[0]->getType(); - - if ($type instanceof UnionType) { - $types = []; - foreach ($type->getTypes() as $type) { - if (!$type instanceof ObjectType) { - // todo handle error - throw new \RuntimeException('invalid handler'); - } - - $types[] = $type->getClassName(); - } - - if ($types) { - return $types; - } - - // todo handle error - throw new \RuntimeException('invalid handler'); - } - - if (!$type instanceof ObjectType) { - throw new \RuntimeException('invalid handler'); - } - - return [$type->getClassName()]; - } + + /** @var ReflectionProvider */ + private $reflectionProvider; + + /** @var ServiceMap */ + private $serviceMap; + + public function __construct(ServiceMap $symfonyServiceMap, ReflectionProvider $reflectionProvider) + { + $this->serviceMap = $symfonyServiceMap; + $this->reflectionProvider = $reflectionProvider; + } + + public function create(): MessageMap + { + $returnTypesMap = []; + + foreach ($this->serviceMap->getServices() as $service) { + $serviceClass = $service->getClass(); + + // todo handle abstract/parent services somehow? + if (is_null($serviceClass)) { + continue; + } + + foreach ($service->getTags() as $tag) { + // todo could there be more tags with the same name for the same service? + // todo check if message handler tag name is constant or configurable + if ($tag->getName() !== 'messenger.message_handler') { + continue; + } + + $tagAttributes = $tag->getAttributes(); + $reflectionClass = $this->reflectionProvider->getClass($serviceClass); + + if (isset($tagAttributes['handles'])) { + $handles = isset($tag['method']) ? [$tag['handles'] => $tag['method']] : [$tag['handles']]; + } else { + $handles = $this->guessHandledMessages($reflectionClass); + } + + foreach ($handles as $messageClassName => $options) { + if (is_int($messageClassName) && is_string($options)) { + $messageClassName = $options; + $options = []; + } + + $options['method'] = $options['method'] ?? '__invoke'; + + $methodReflection = $reflectionClass->getNativeMethod($options['method']); + $variant = ParametersAcceptorSelector::selectSingle($methodReflection->getVariants()); + + $returnTypesMap[$messageClassName][] = $variant->getReturnType(); + } + } + } + + $messages = []; + foreach ($returnTypesMap as $messageClassName => $returnTypes) { + $messages[] = new Message($messageClassName, $returnTypes); + } + + return new MessageMap($messages); + } + + private function guessHandledMessages(ClassReflection $reflectionClass): iterable + { + if ($reflectionClass->implementsInterface(MessageSubscriberInterface::class)) { + // todo handle different return formats + return $reflectionClass->getName()::getHandledMessages(); + } + + // todo handle if doesn't exists + $methodReflection = $reflectionClass->getNativeMethod('__invoke'); + + $variant = ParametersAcceptorSelector::selectSingle($methodReflection->getVariants()); + $parameters = $variant->getParameters(); + + if (count($parameters) !== 1) { + // todo handle error + throw new RuntimeException('invalid handler'); + } + + $type = $parameters[0]->getType(); + + if ($type instanceof UnionType) { + $types = []; + foreach ($type->getTypes() as $type) { + if (!$type instanceof ObjectType) { + // todo handle error + throw new RuntimeException('invalid handler'); + } + + $types[] = $type->getClassName(); + } + + if ($types) { + return $types; + } + + // todo handle error + throw new RuntimeException('invalid handler'); + } + + if (!$type instanceof ObjectType) { + throw new RuntimeException('invalid handler'); + } + + return [$type->getClassName()]; + } + } diff --git a/src/Symfony/Service.php b/src/Symfony/Service.php index 5d72c3e4..09152d58 100644 --- a/src/Symfony/Service.php +++ b/src/Symfony/Service.php @@ -20,8 +20,8 @@ final class Service implements ServiceDefinition /** @var string|null */ private $alias; - /** @var array */ - private $tags; + /** @var array */ + private $tags; public function __construct( string $id, @@ -29,7 +29,7 @@ public function __construct( bool $public, bool $synthetic, ?string $alias, - array $tags = [] + array $tags = [] ) { $this->id = $id; @@ -65,8 +65,9 @@ public function getAlias(): ?string return $this->alias; } - public function getTags(): array - { - return $this->tags; - } + public function getTags(): array + { + return $this->tags; + } + } diff --git a/src/Symfony/ServiceDefinition.php b/src/Symfony/ServiceDefinition.php index 1b7cea55..d8c77fb2 100644 --- a/src/Symfony/ServiceDefinition.php +++ b/src/Symfony/ServiceDefinition.php @@ -18,5 +18,6 @@ public function isSynthetic(): bool; public function getAlias(): ?string; - public function getTags(): array; + public function getTags(): array; + } diff --git a/src/Symfony/ServiceTag.php b/src/Symfony/ServiceTag.php index 880a72cd..c02be7c5 100644 --- a/src/Symfony/ServiceTag.php +++ b/src/Symfony/ServiceTag.php @@ -1,28 +1,30 @@ -name = $name; - $this->attributes = $attributes; - } - - public function getName(): string - { - return $this->name; - } - - public function getAttributes(): array - { - return $this->attributes; - } + + /** @var string */ + private $name; + + /** @var array */ + private $attributes; + + public function __construct(string $name, array $attributes = []) + { + $this->name = $name; + $this->attributes = $attributes; + } + + public function getName(): string + { + return $this->name; + } + + public function getAttributes(): array + { + return $this->attributes; + } + } diff --git a/src/Symfony/ServiceTagDefinition.php b/src/Symfony/ServiceTagDefinition.php index f2154db4..19bd84a3 100644 --- a/src/Symfony/ServiceTagDefinition.php +++ b/src/Symfony/ServiceTagDefinition.php @@ -1,10 +1,12 @@ -tag as $tag) { - $tagAttrs = ((array) $tag->attributes())['@attributes'] ?? []; - $tagName = $tagAttrs['name']; - unset($tagAttrs['name']); + $serviceTags = []; + foreach ($def->tag as $tag) { + $tagAttrs = ((array) $tag->attributes())['@attributes'] ?? []; + $tagName = $tagAttrs['name']; + unset($tagAttrs['name']); - $serviceTags[] = new ServiceTag($tagName, $tagAttrs); - } + $serviceTags[] = new ServiceTag($tagName, $tagAttrs); + } $service = new Service( $this->cleanServiceId((string) $attrs->id), @@ -62,7 +62,7 @@ public function create(): ServiceMap isset($attrs->public) && (string) $attrs->public === 'true', isset($attrs->synthetic) && (string) $attrs->synthetic === 'true', isset($attrs->alias) ? $this->cleanServiceId((string) $attrs->alias) : null, - $serviceTags + $serviceTags ); if ($service->getAlias() !== null) { diff --git a/src/Type/Symfony/MessengerHandleTraitDynamicReturnTypeExtension.php b/src/Type/Symfony/MessengerHandleTraitDynamicReturnTypeExtension.php index 6462a14c..a0f06a5a 100644 --- a/src/Type/Symfony/MessengerHandleTraitDynamicReturnTypeExtension.php +++ b/src/Type/Symfony/MessengerHandleTraitDynamicReturnTypeExtension.php @@ -1,4 +1,4 @@ -messageMapFactory = $symfonyMessageMapFactory; - } - - public function getClass(): string - { - // todo traits are not supported yet in phpstan for this extension (https://github.com/phpstan/phpstan/issues/5761) - // return HandleTrait::class; - - // todo or make it configurable with passing concrete classes names to extension config - // todo or use reflection somehow to get all classes that use HandleTrait and configure it dynamically - - // todo temporarily hardcoded test class here - return HandleTraitClass::class; - } - - public function isMethodSupported(MethodReflection $methodReflection): bool - { - // todo additional reflection checker that it comes only from trait? - return $methodReflection->getName() === 'handle'; - } - - public function getTypeFromMethodCall(MethodReflection $methodReflection, MethodCall $methodCall, Scope $scope): ?Type - { - // todo handle different cases: - // - [X] regular message classes - // - [ ] interfaces for message classes - // - [ ] many handlers for one message? it would throw exception in HandleTrait anyway - // - [x] many messages for one handler - // - [partially] cover MessageSubscriberInterface - // - [partially] custom method names for handlers (different than default "__invoke" magic method) - // - [] read SF doc to determine any other cases to covers - - $arg = $methodCall->getArgs()[0]->value; - $argType = $scope->getType($arg); - - if ($argType instanceof ObjectType) { - $messageMap = $this->getMessageMap(); - if ($messageMap->hasMessageForClass($argType->getClassName())) { - $message = $messageMap->getMessageForClass($argType->getClassName()); - - if (1 === $message->countReturnTypes()) { - return $message->getReturnTypes()[0]; - } - } - } - - return null; - } - - private function getMessageMap(): MessageMap - { - if (!$this->messageMap) { - $this->messageMap = $this->messageMapFactory->create(); - } - - return $this->messageMap; - } + + /** @var MessageMapFactory */ + private $messageMapFactory; + + /** @var MessageMap */ + private $messageMap; + + public function __construct(MessageMapFactory $symfonyMessageMapFactory) + { + $this->messageMapFactory = $symfonyMessageMapFactory; + } + + public function getClass(): string + { + // todo traits are not supported yet in phpstan for this extension (https://github.com/phpstan/phpstan/issues/5761) + // return HandleTrait::class; + + // todo or make it configurable with passing concrete classes names to extension config + // todo or use reflection somehow to get all classes that use HandleTrait and configure it dynamically + + // todo temporarily hardcoded test class here + return HandleTraitClass::class; + } + + public function isMethodSupported(MethodReflection $methodReflection): bool + { + // todo additional reflection checker that it comes only from trait? + return $methodReflection->getName() === 'handle'; + } + + public function getTypeFromMethodCall(MethodReflection $methodReflection, MethodCall $methodCall, Scope $scope): ?Type + { + // todo handle different cases: + // - [X] regular message classes + // - [ ] interfaces for message classes + // - [ ] many handlers for one message? it would throw exception in HandleTrait anyway + // - [x] many messages for one handler + // - [partially] cover MessageSubscriberInterface + // - [partially] custom method names for handlers (different than default "__invoke" magic method) + // - [] read SF doc to determine any other cases to covers + + $arg = $methodCall->getArgs()[0]->value; + $argType = $scope->getType($arg); + + if ($argType instanceof ObjectType) { + $messageMap = $this->getMessageMap(); + if ($messageMap->hasMessageForClass($argType->getClassName())) { + $message = $messageMap->getMessageForClass($argType->getClassName()); + + if ($message->countReturnTypes() === 1) { + return $message->getReturnTypes()[0]; + } + } + } + + return null; + } + + private function getMessageMap(): MessageMap + { + if (!$this->messageMap) { + $this->messageMap = $this->messageMapFactory->create(); + } + + return $this->messageMap; + } + } diff --git a/tests/Type/Symfony/ExtensionTest.php b/tests/Type/Symfony/ExtensionTest.php index 85bbb140..40420be0 100644 --- a/tests/Type/Symfony/ExtensionTest.php +++ b/tests/Type/Symfony/ExtensionTest.php @@ -14,7 +14,7 @@ class ExtensionTest extends TypeInferenceTestCase /** @return mixed[] */ public function dataFileAsserts(): iterable { - yield from $this->gatherAssertTypes(__DIR__ . '/data/messenger_handle_trait.php'); + yield from $this->gatherAssertTypes(__DIR__ . '/data/messenger_handle_trait.php'); yield from $this->gatherAssertTypes(__DIR__ . '/data/envelope_all.php'); yield from $this->gatherAssertTypes(__DIR__ . '/data/header_bag_get.php'); yield from $this->gatherAssertTypes(__DIR__ . '/data/response_header_bag_get_cookies.php'); From bd75cb645f4721eb350b8bc60d4f4c99d8ecb527 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Nowak?= Date: Wed, 14 Aug 2024 07:52:15 +0200 Subject: [PATCH 03/14] phpstan fix --- src/Symfony/Message.php | 5 +- src/Symfony/MessageMap.php | 7 --- src/Symfony/MessageMapFactory.php | 53 +++++++------------ src/Symfony/Service.php | 3 +- src/Symfony/ServiceDefinition.php | 1 + src/Symfony/ServiceTag.php | 3 +- src/Symfony/ServiceTagDefinition.php | 1 + ...rHandleTraitDynamicReturnTypeExtension.php | 23 ++++---- 8 files changed, 39 insertions(+), 57 deletions(-) diff --git a/src/Symfony/Message.php b/src/Symfony/Message.php index dac5b23c..8a633a59 100644 --- a/src/Symfony/Message.php +++ b/src/Symfony/Message.php @@ -2,6 +2,7 @@ namespace PHPStan\Symfony; +use PHPStan\Type\Type; use function count; final class Message @@ -10,9 +11,10 @@ final class Message /** @var string */ private $class; - /** @var array */ + /** @var Type[] */ private $returnTypes; + /** @param Type[] $returnTypes */ public function __construct(string $class, array $returnTypes) { $this->class = $class; @@ -24,6 +26,7 @@ public function getClass(): string return $this->class; } + /** @return Type[] */ public function getReturnTypes(): array { return $this->returnTypes; diff --git a/src/Symfony/MessageMap.php b/src/Symfony/MessageMap.php index a96f3521..3ef6fcef 100644 --- a/src/Symfony/MessageMap.php +++ b/src/Symfony/MessageMap.php @@ -2,8 +2,6 @@ namespace PHPStan\Symfony; -use function array_key_exists; - final class MessageMap { @@ -25,9 +23,4 @@ public function getMessageForClass(string $class): ?Message return $this->messages[$class] ?? null; } - public function hasMessageForClass(string $class): bool - { - return array_key_exists($class, $this->messages); - } - } diff --git a/src/Symfony/MessageMapFactory.php b/src/Symfony/MessageMapFactory.php index 896f4674..ce4e23bb 100644 --- a/src/Symfony/MessageMapFactory.php +++ b/src/Symfony/MessageMapFactory.php @@ -5,8 +5,6 @@ use PHPStan\Reflection\ClassReflection; use PHPStan\Reflection\ParametersAcceptorSelector; use PHPStan\Reflection\ReflectionProvider; -use PHPStan\Type\ObjectType; -use PHPStan\Type\UnionType; use RuntimeException; use Symfony\Component\Messenger\Handler\MessageSubscriberInterface; use function count; @@ -18,6 +16,8 @@ final class MessageMapFactory { + private const DEFAULT_HANDLER_METHOD = '__invoke'; + /** @var ReflectionProvider */ private $reflectionProvider; @@ -53,19 +53,13 @@ public function create(): MessageMap $reflectionClass = $this->reflectionProvider->getClass($serviceClass); if (isset($tagAttributes['handles'])) { - $handles = isset($tag['method']) ? [$tag['handles'] => $tag['method']] : [$tag['handles']]; + // todo cover by test case + $handles = [$tagAttributes['handles'] => ['method' => $tagAttributes['method'] ?? self::DEFAULT_HANDLER_METHOD]]; } else { $handles = $this->guessHandledMessages($reflectionClass); } foreach ($handles as $messageClassName => $options) { - if (is_int($messageClassName) && is_string($options)) { - $messageClassName = $options; - $options = []; - } - - $options['method'] = $options['method'] ?? '__invoke'; - $methodReflection = $reflectionClass->getNativeMethod($options['method']); $variant = ParametersAcceptorSelector::selectSingle($methodReflection->getVariants()); @@ -82,15 +76,24 @@ public function create(): MessageMap return new MessageMap($messages); } + /** @return array> */ private function guessHandledMessages(ClassReflection $reflectionClass): iterable { if ($reflectionClass->implementsInterface(MessageSubscriberInterface::class)) { // todo handle different return formats - return $reflectionClass->getName()::getHandledMessages(); + foreach ($reflectionClass->getName()::getHandledMessages() as $index => $value) { + if (is_int($index) && is_string($value)) { + yield $value => ['method' => self::DEFAULT_HANDLER_METHOD]; + } else { + yield $index => $value; + } + } + + return; } // todo handle if doesn't exists - $methodReflection = $reflectionClass->getNativeMethod('__invoke'); + $methodReflection = $reflectionClass->getNativeMethod(self::DEFAULT_HANDLER_METHOD); $variant = ParametersAcceptorSelector::selectSingle($methodReflection->getVariants()); $parameters = $variant->getParameters(); @@ -102,30 +105,10 @@ private function guessHandledMessages(ClassReflection $reflectionClass): iterabl $type = $parameters[0]->getType(); - if ($type instanceof UnionType) { - $types = []; - foreach ($type->getTypes() as $type) { - if (!$type instanceof ObjectType) { - // todo handle error - throw new RuntimeException('invalid handler'); - } - - $types[] = $type->getClassName(); - } - - if ($types) { - return $types; - } - - // todo handle error - throw new RuntimeException('invalid handler'); + // todo many class names? + foreach ($type->getObjectClassNames() as $className) { + yield $className => ['method' => self::DEFAULT_HANDLER_METHOD]; } - - if (!$type instanceof ObjectType) { - throw new RuntimeException('invalid handler'); - } - - return [$type->getClassName()]; } } diff --git a/src/Symfony/Service.php b/src/Symfony/Service.php index 09152d58..1cc465ac 100644 --- a/src/Symfony/Service.php +++ b/src/Symfony/Service.php @@ -20,9 +20,10 @@ final class Service implements ServiceDefinition /** @var string|null */ private $alias; - /** @var array */ + /** @var ServiceTag[] */ private $tags; + /** @param ServiceTag[] $tags */ public function __construct( string $id, ?string $class, diff --git a/src/Symfony/ServiceDefinition.php b/src/Symfony/ServiceDefinition.php index d8c77fb2..3862fa8d 100644 --- a/src/Symfony/ServiceDefinition.php +++ b/src/Symfony/ServiceDefinition.php @@ -18,6 +18,7 @@ public function isSynthetic(): bool; public function getAlias(): ?string; + /** @return ServiceTag[] */ public function getTags(): array; } diff --git a/src/Symfony/ServiceTag.php b/src/Symfony/ServiceTag.php index c02be7c5..a8437fd1 100644 --- a/src/Symfony/ServiceTag.php +++ b/src/Symfony/ServiceTag.php @@ -8,9 +8,10 @@ final class ServiceTag implements ServiceTagDefinition /** @var string */ private $name; - /** @var array */ + /** @var array */ private $attributes; + /** @param array $attributes */ public function __construct(string $name, array $attributes = []) { $this->name = $name; diff --git a/src/Symfony/ServiceTagDefinition.php b/src/Symfony/ServiceTagDefinition.php index 19bd84a3..b0f66d9c 100644 --- a/src/Symfony/ServiceTagDefinition.php +++ b/src/Symfony/ServiceTagDefinition.php @@ -7,6 +7,7 @@ interface ServiceTagDefinition public function getName(): string; + /** @return array */ public function getAttributes(): array; } diff --git a/src/Type/Symfony/MessengerHandleTraitDynamicReturnTypeExtension.php b/src/Type/Symfony/MessengerHandleTraitDynamicReturnTypeExtension.php index a0f06a5a..43be1295 100644 --- a/src/Type/Symfony/MessengerHandleTraitDynamicReturnTypeExtension.php +++ b/src/Type/Symfony/MessengerHandleTraitDynamicReturnTypeExtension.php @@ -9,8 +9,9 @@ use PHPStan\Symfony\MessageMap; use PHPStan\Symfony\MessageMapFactory; use PHPStan\Type\DynamicMethodReturnTypeExtension; -use PHPStan\Type\ObjectType; use PHPStan\Type\Type; +use function count; +use function is_null; final class MessengerHandleTraitDynamicReturnTypeExtension implements DynamicMethodReturnTypeExtension { @@ -18,7 +19,7 @@ final class MessengerHandleTraitDynamicReturnTypeExtension implements DynamicMet /** @var MessageMapFactory */ private $messageMapFactory; - /** @var MessageMap */ + /** @var MessageMap|null */ private $messageMap; public function __construct(MessageMapFactory $symfonyMessageMapFactory) @@ -34,8 +35,7 @@ public function getClass(): string // todo or make it configurable with passing concrete classes names to extension config // todo or use reflection somehow to get all classes that use HandleTrait and configure it dynamically - // todo temporarily hardcoded test class here - return HandleTraitClass::class; + return HandleTraitClass::class; // @phpstan-ignore-line todo temporarily hardcoded test class here } public function isMethodSupported(MethodReflection $methodReflection): bool @@ -56,16 +56,15 @@ public function getTypeFromMethodCall(MethodReflection $methodReflection, Method // - [] read SF doc to determine any other cases to covers $arg = $methodCall->getArgs()[0]->value; - $argType = $scope->getType($arg); + $argClassNames = $scope->getType($arg)->getObjectClassNames(); - if ($argType instanceof ObjectType) { + // todo filter out not handled cases on map creation? + if (count($argClassNames) === 1) { $messageMap = $this->getMessageMap(); - if ($messageMap->hasMessageForClass($argType->getClassName())) { - $message = $messageMap->getMessageForClass($argType->getClassName()); + $message = $messageMap->getMessageForClass($argClassNames[0]); - if ($message->countReturnTypes() === 1) { - return $message->getReturnTypes()[0]; - } + if (!is_null($message) && $message->countReturnTypes() === 1) { + return $message->getReturnTypes()[0]; } } @@ -74,7 +73,7 @@ public function getTypeFromMethodCall(MethodReflection $methodReflection, Method private function getMessageMap(): MessageMap { - if (!$this->messageMap) { + if (is_null($this->messageMap)) { $this->messageMap = $this->messageMapFactory->create(); } From 9768de5b634901d4c05351754cf999cbd408781a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Nowak?= Date: Wed, 14 Aug 2024 07:56:18 +0200 Subject: [PATCH 04/14] phpstan fix 2 --- .../Symfony/MessengerHandleTraitDynamicReturnTypeExtension.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Type/Symfony/MessengerHandleTraitDynamicReturnTypeExtension.php b/src/Type/Symfony/MessengerHandleTraitDynamicReturnTypeExtension.php index 43be1295..cf8d3a9f 100644 --- a/src/Type/Symfony/MessengerHandleTraitDynamicReturnTypeExtension.php +++ b/src/Type/Symfony/MessengerHandleTraitDynamicReturnTypeExtension.php @@ -35,7 +35,8 @@ public function getClass(): string // todo or make it configurable with passing concrete classes names to extension config // todo or use reflection somehow to get all classes that use HandleTrait and configure it dynamically - return HandleTraitClass::class; // @phpstan-ignore-line todo temporarily hardcoded test class here + // todo temporarily hardcoded test class here + return HandleTraitClass::class; } public function isMethodSupported(MethodReflection $methodReflection): bool From 0c8cb342cde55e27cf3b31ce11e690cdb006aea0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Nowak?= Date: Tue, 15 Oct 2024 10:46:54 +0200 Subject: [PATCH 05/14] add /.idea path to .gitignore --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 7de9f3c5..c0c8f534 100644 --- a/.gitignore +++ b/.gitignore @@ -1,5 +1,6 @@ /tests/tmp /build-cs /vendor +/.idea /composer.lock .phpunit.result.cache From 224c478df90a2073df711da4bc9039e6c0f780b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Nowak?= Date: Thu, 17 Oct 2024 11:03:50 +0200 Subject: [PATCH 06/14] use ExpressionTypeResolverExtension instead of DynamicMethodReturnTypeExtension --- extension.neon | 4 +- ...rHandleTraitDynamicReturnTypeExtension.php | 84 ---------------- ...essengerHandleTraitReturnTypeExtension.php | 95 +++++++++++++++++++ 3 files changed, 97 insertions(+), 86 deletions(-) delete mode 100644 src/Type/Symfony/MessengerHandleTraitDynamicReturnTypeExtension.php create mode 100644 src/Type/Symfony/MessengerHandleTraitReturnTypeExtension.php diff --git a/extension.neon b/extension.neon index f6b8282e..cbdfd73d 100644 --- a/extension.neon +++ b/extension.neon @@ -212,8 +212,8 @@ services: # Messenger HandleTrait::handle() return type - - factory: PHPStan\Type\Symfony\MessengerHandleTraitDynamicReturnTypeExtension - tags: [phpstan.broker.dynamicMethodReturnTypeExtension] + class: PHPStan\Type\Symfony\MessengerHandleTraitReturnTypeExtension + tags: [phpstan.broker.expressionTypeResolverExtension] # InputInterface::getArgument() return type - diff --git a/src/Type/Symfony/MessengerHandleTraitDynamicReturnTypeExtension.php b/src/Type/Symfony/MessengerHandleTraitDynamicReturnTypeExtension.php deleted file mode 100644 index cf8d3a9f..00000000 --- a/src/Type/Symfony/MessengerHandleTraitDynamicReturnTypeExtension.php +++ /dev/null @@ -1,84 +0,0 @@ -messageMapFactory = $symfonyMessageMapFactory; - } - - public function getClass(): string - { - // todo traits are not supported yet in phpstan for this extension (https://github.com/phpstan/phpstan/issues/5761) - // return HandleTrait::class; - - // todo or make it configurable with passing concrete classes names to extension config - // todo or use reflection somehow to get all classes that use HandleTrait and configure it dynamically - - // todo temporarily hardcoded test class here - return HandleTraitClass::class; - } - - public function isMethodSupported(MethodReflection $methodReflection): bool - { - // todo additional reflection checker that it comes only from trait? - return $methodReflection->getName() === 'handle'; - } - - public function getTypeFromMethodCall(MethodReflection $methodReflection, MethodCall $methodCall, Scope $scope): ?Type - { - // todo handle different cases: - // - [X] regular message classes - // - [ ] interfaces for message classes - // - [ ] many handlers for one message? it would throw exception in HandleTrait anyway - // - [x] many messages for one handler - // - [partially] cover MessageSubscriberInterface - // - [partially] custom method names for handlers (different than default "__invoke" magic method) - // - [] read SF doc to determine any other cases to covers - - $arg = $methodCall->getArgs()[0]->value; - $argClassNames = $scope->getType($arg)->getObjectClassNames(); - - // todo filter out not handled cases on map creation? - if (count($argClassNames) === 1) { - $messageMap = $this->getMessageMap(); - $message = $messageMap->getMessageForClass($argClassNames[0]); - - if (!is_null($message) && $message->countReturnTypes() === 1) { - return $message->getReturnTypes()[0]; - } - } - - return null; - } - - private function getMessageMap(): MessageMap - { - if (is_null($this->messageMap)) { - $this->messageMap = $this->messageMapFactory->create(); - } - - return $this->messageMap; - } - -} diff --git a/src/Type/Symfony/MessengerHandleTraitReturnTypeExtension.php b/src/Type/Symfony/MessengerHandleTraitReturnTypeExtension.php new file mode 100644 index 00000000..68e66de1 --- /dev/null +++ b/src/Type/Symfony/MessengerHandleTraitReturnTypeExtension.php @@ -0,0 +1,95 @@ +messageMapFactory = $symfonyMessageMapFactory; + } + + public function getType(Expr $expr, Scope $scope): ?Type + { + // todo handle different cases: + // - [X] regular message classes + // - [ ] interfaces for message classes + // - [ ] many handlers for one message? it would throw exception in HandleTrait anyway + // - [x] many messages for one handler + // - [partially] cover MessageSubscriberInterface + // - [partially] custom method names for handlers (different than default "__invoke" magic method) + // - [] read SF doc to determine any other cases to covers + + if ($this->isSupported($expr, $scope)) { + $arg = $expr->getArgs()[0]->value; + $argClassNames = $scope->getType($arg)->getObjectClassNames(); + + // todo filter out not handled cases on map creation? + if (count($argClassNames) === 1) { + $messageMap = $this->getMessageMap(); + $message = $messageMap->getMessageForClass($argClassNames[0]); + + if (!is_null($message) && $message->countReturnTypes() === 1) { + return $message->getReturnTypes()[0]; + } + } + } + + return null; + } + + private function getMessageMap(): MessageMap + { + if (is_null($this->messageMap)) { + $this->messageMap = $this->messageMapFactory->create(); + } + + return $this->messageMap; + } + + /** + * @phpstan-assert-if-true MethodCall $expr + */ + private function isSupported(Expr $expr, Scope $scope): bool + { + if (!($expr instanceof MethodCall) || !($expr->name instanceof Identifier) || $expr->name->name !== self::TRAIT_METHOD_NAME) { + return false; + } + + if (!$scope->isInClass()) { + return false; + } + + try { + $methodReflection = $scope->getClassReflection()->getNativeReflection()->getMethod(self::TRAIT_METHOD_NAME); + $declaringClassReflection = $methodReflection->getBetterReflection()->getDeclaringClass(); + + return $declaringClassReflection->getName() === self::TRAIT_NAME; + } catch (ReflectionException $e) { + return false; + } + } + +} From 9438e5e2d47dbc8156e0a5b299fa0f76870cdbee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Nowak?= Date: Thu, 17 Oct 2024 11:53:07 +0200 Subject: [PATCH 07/14] adjust MessageMap to contain only supported cases --- src/Symfony/Message.php | 40 ------------------- src/Symfony/MessageMap.php | 18 ++++----- src/Symfony/MessageMapFactory.php | 10 +++-- ...essengerHandleTraitReturnTypeExtension.php | 7 ++-- 4 files changed, 19 insertions(+), 56 deletions(-) delete mode 100644 src/Symfony/Message.php diff --git a/src/Symfony/Message.php b/src/Symfony/Message.php deleted file mode 100644 index 8a633a59..00000000 --- a/src/Symfony/Message.php +++ /dev/null @@ -1,40 +0,0 @@ -class = $class; - $this->returnTypes = $returnTypes; - } - - public function getClass(): string - { - return $this->class; - } - - /** @return Type[] */ - public function getReturnTypes(): array - { - return $this->returnTypes; - } - - public function countReturnTypes(): int - { - return count($this->returnTypes); - } - -} diff --git a/src/Symfony/MessageMap.php b/src/Symfony/MessageMap.php index 3ef6fcef..e5d6f307 100644 --- a/src/Symfony/MessageMap.php +++ b/src/Symfony/MessageMap.php @@ -2,25 +2,25 @@ namespace PHPStan\Symfony; +use PHPStan\Type\Type; + final class MessageMap { - /** @var Message[] */ - private $messages = []; + /** @var array */ + private $messageMap; /** - * @param Message[] $messages + * @param array $messageMap */ - public function __construct(array $messages) + public function __construct(array $messageMap) { - foreach ($messages as $message) { - $this->messages[$message->getClass()] = $message; - } + $this->messageMap = $messageMap; } - public function getMessageForClass(string $class): ?Message + public function getTypeForClass(string $class): ?Type { - return $this->messages[$class] ?? null; + return $this->messageMap[$class] ?? null; } } diff --git a/src/Symfony/MessageMapFactory.php b/src/Symfony/MessageMapFactory.php index ce4e23bb..69de50db 100644 --- a/src/Symfony/MessageMapFactory.php +++ b/src/Symfony/MessageMapFactory.php @@ -68,12 +68,16 @@ public function create(): MessageMap } } - $messages = []; + $messageMap = []; foreach ($returnTypesMap as $messageClassName => $returnTypes) { - $messages[] = new Message($messageClassName, $returnTypes); + if (count($returnTypes) !== 1) { + continue; + } + + $messageMap[$messageClassName] = $returnTypes[0]; } - return new MessageMap($messages); + return new MessageMap($messageMap); } /** @return array> */ diff --git a/src/Type/Symfony/MessengerHandleTraitReturnTypeExtension.php b/src/Type/Symfony/MessengerHandleTraitReturnTypeExtension.php index 68e66de1..b583d1d8 100644 --- a/src/Type/Symfony/MessengerHandleTraitReturnTypeExtension.php +++ b/src/Type/Symfony/MessengerHandleTraitReturnTypeExtension.php @@ -46,13 +46,12 @@ public function getType(Expr $expr, Scope $scope): ?Type $arg = $expr->getArgs()[0]->value; $argClassNames = $scope->getType($arg)->getObjectClassNames(); - // todo filter out not handled cases on map creation? if (count($argClassNames) === 1) { $messageMap = $this->getMessageMap(); - $message = $messageMap->getMessageForClass($argClassNames[0]); + $returnType = $messageMap->getTypeForClass($argClassNames[0]); - if (!is_null($message) && $message->countReturnTypes() === 1) { - return $message->getReturnTypes()[0]; + if (!is_null($returnType)) { + return $returnType; } } } From 8a44be85014d3a423277931c814fd36fc6b899f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Nowak?= Date: Thu, 17 Oct 2024 14:06:56 +0200 Subject: [PATCH 08/14] covered more test-cases --- src/Symfony/MessageMapFactory.php | 4 +- tests/Type/Symfony/container.xml | 12 ++++ .../Symfony/data/messenger_handle_trait.php | 58 +++++++++++++++++-- 3 files changed, 66 insertions(+), 8 deletions(-) diff --git a/src/Symfony/MessageMapFactory.php b/src/Symfony/MessageMapFactory.php index 69de50db..edd3a090 100644 --- a/src/Symfony/MessageMapFactory.php +++ b/src/Symfony/MessageMapFactory.php @@ -53,14 +53,13 @@ public function create(): MessageMap $reflectionClass = $this->reflectionProvider->getClass($serviceClass); if (isset($tagAttributes['handles'])) { - // todo cover by test case $handles = [$tagAttributes['handles'] => ['method' => $tagAttributes['method'] ?? self::DEFAULT_HANDLER_METHOD]]; } else { $handles = $this->guessHandledMessages($reflectionClass); } foreach ($handles as $messageClassName => $options) { - $methodReflection = $reflectionClass->getNativeMethod($options['method']); + $methodReflection = $reflectionClass->getNativeMethod($options['method'] ?? self::DEFAULT_HANDLER_METHOD); $variant = ParametersAcceptorSelector::selectSingle($methodReflection->getVariants()); $returnTypesMap[$messageClassName][] = $variant->getReturnType(); @@ -84,7 +83,6 @@ public function create(): MessageMap private function guessHandledMessages(ClassReflection $reflectionClass): iterable { if ($reflectionClass->implementsInterface(MessageSubscriberInterface::class)) { - // todo handle different return formats foreach ($reflectionClass->getName()::getHandledMessages() as $index => $value) { if (is_int($index) && is_string($value)) { yield $value => ['method' => self::DEFAULT_HANDLER_METHOD]; diff --git a/tests/Type/Symfony/container.xml b/tests/Type/Symfony/container.xml index 07ddc824..16d4b7fe 100644 --- a/tests/Type/Symfony/container.xml +++ b/tests/Type/Symfony/container.xml @@ -360,5 +360,17 @@ + + + + + + + + + + + + diff --git a/tests/Type/Symfony/data/messenger_handle_trait.php b/tests/Type/Symfony/data/messenger_handle_trait.php index 7728685e..fb137e0c 100644 --- a/tests/Type/Symfony/data/messenger_handle_trait.php +++ b/tests/Type/Symfony/data/messenger_handle_trait.php @@ -16,6 +16,7 @@ public function __invoke(RegularQuery $query): RegularQueryResult } } +class BooleanQuery {} class StringQuery {} class IntQuery {} class FloatQuery {} @@ -23,15 +24,15 @@ class MultiQueryHandler implements MessageSubscriberInterface { public static function getHandledMessages(): iterable { - yield StringQuery::class; + yield BooleanQuery::class; yield IntQuery::class => ['method' => 'handleInt']; yield FloatQuery::class => ['method' => 'handleFloat']; yield StringQuery::class => ['method' => 'handleString']; } - public function __invoke(StringQuery $query): string + public function __invoke(BooleanQuery $query): bool { - return 'string result'; + return true; } public function handleInt(IntQuery $query): int @@ -52,16 +53,63 @@ public function handleString(StringQuery $query): string // todo add handle method with union return type? } +class TaggedQuery {} +class TaggedResult {} +class TaggedHandler +{ + public function handle(TaggedQuery $query): TaggedResult + { + return new TaggedResult(); + } +} + +class MultiHandlesForInTheSameHandlerQuery {} +class MultiHandlesForInTheSameHandler implements MessageSubscriberInterface +{ + public static function getHandledMessages(): iterable + { + yield MultiHandlesForInTheSameHandlerQuery::class; + yield MultiHandlesForInTheSameHandlerQuery::class => ['priority' => '0']; + } + + public function __invoke(MultiHandlesForInTheSameHandlerQuery $query): bool + { + return true; + } +} + +class MultiHandlersForTheSameMessageQuery {} +class MultiHandlersForTheSameMessageHandler1 +{ + public function __invoke(MultiHandlersForTheSameMessageQuery $query): bool + { + return true; + } +} +class MultiHandlersForTheSameMessageHandler2 +{ + public function __invoke(MultiHandlersForTheSameMessageQuery $query): bool + { + return false; + } +} + class HandleTraitClass { use HandleTrait; public function __invoke() { assertType(RegularQueryResult::class, $this->handle(new RegularQuery())); + + assertType('bool', $this->handle(new BooleanQuery())); assertType('int', $this->handle(new IntQuery())); assertType('float', $this->handle(new FloatQuery())); + assertType('string', $this->handle(new StringQuery())); + + assertType(TaggedResult::class, $this->handle(new TaggedQuery())); - // HandleTrait will throw exception in fact due to multiple handlers per single query - assertType('mixed', $this->handle(new StringQuery())); + // HandleTrait will throw exception in fact due to multiple handle methods/handlers per single query + assertType('mixed', $this->handle(new MultiHandlesForInTheSameHandlerQuery())); + assertType('mixed', $this->handle(new MultiHandlersForTheSameMessageQuery())); } } From 46e0cc8c4caef43d8b17bf0f583075b3ed4ef8e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Nowak?= Date: Thu, 17 Oct 2024 14:47:25 +0200 Subject: [PATCH 09/14] get rid of todo comments in \PHPStan\Symfony\MessageMapFactory::guessHandledMessages --- src/Symfony/MessageMapFactory.php | 30 +++++++++++-------- ...essengerHandleTraitReturnTypeExtension.php | 9 ------ 2 files changed, 18 insertions(+), 21 deletions(-) diff --git a/src/Symfony/MessageMapFactory.php b/src/Symfony/MessageMapFactory.php index edd3a090..3e0bd76c 100644 --- a/src/Symfony/MessageMapFactory.php +++ b/src/Symfony/MessageMapFactory.php @@ -3,16 +3,15 @@ namespace PHPStan\Symfony; use PHPStan\Reflection\ClassReflection; +use PHPStan\Reflection\MissingMethodFromReflectionException; use PHPStan\Reflection\ParametersAcceptorSelector; use PHPStan\Reflection\ReflectionProvider; -use RuntimeException; use Symfony\Component\Messenger\Handler\MessageSubscriberInterface; use function count; use function is_int; use function is_null; use function is_string; -// todo add tests final class MessageMapFactory { @@ -94,23 +93,30 @@ private function guessHandledMessages(ClassReflection $reflectionClass): iterabl return; } - // todo handle if doesn't exists - $methodReflection = $reflectionClass->getNativeMethod(self::DEFAULT_HANDLER_METHOD); + try { + $methodReflection = $reflectionClass->getNativeMethod(self::DEFAULT_HANDLER_METHOD); + } catch (MissingMethodFromReflectionException $e) { + return; + } + + $variants = $methodReflection->getVariants(); + if (count($variants) !== 1) { + return; + } - $variant = ParametersAcceptorSelector::selectSingle($methodReflection->getVariants()); - $parameters = $variant->getParameters(); + $parameters = $variants[0]->getParameters(); if (count($parameters) !== 1) { - // todo handle error - throw new RuntimeException('invalid handler'); + return; } - $type = $parameters[0]->getType(); + $classNames = $parameters[0]->getType()->getObjectClassNames(); - // todo many class names? - foreach ($type->getObjectClassNames() as $className) { - yield $className => ['method' => self::DEFAULT_HANDLER_METHOD]; + if (count($classNames) !== 1) { + return; } + + yield $classNames[0] => ['method' => self::DEFAULT_HANDLER_METHOD]; } } diff --git a/src/Type/Symfony/MessengerHandleTraitReturnTypeExtension.php b/src/Type/Symfony/MessengerHandleTraitReturnTypeExtension.php index b583d1d8..90160980 100644 --- a/src/Type/Symfony/MessengerHandleTraitReturnTypeExtension.php +++ b/src/Type/Symfony/MessengerHandleTraitReturnTypeExtension.php @@ -33,15 +33,6 @@ public function __construct(MessageMapFactory $symfonyMessageMapFactory) public function getType(Expr $expr, Scope $scope): ?Type { - // todo handle different cases: - // - [X] regular message classes - // - [ ] interfaces for message classes - // - [ ] many handlers for one message? it would throw exception in HandleTrait anyway - // - [x] many messages for one handler - // - [partially] cover MessageSubscriberInterface - // - [partially] custom method names for handlers (different than default "__invoke" magic method) - // - [] read SF doc to determine any other cases to covers - if ($this->isSupported($expr, $scope)) { $arg = $expr->getArgs()[0]->value; $argClassNames = $scope->getType($arg)->getObjectClassNames(); From 5cba4ba241b9ce47edb72ca1836dc1808e25ca62 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Nowak?= Date: Thu, 17 Oct 2024 14:59:57 +0200 Subject: [PATCH 10/14] addressed rest of todo comments --- src/Symfony/MessageMapFactory.php | 12 +++++------- tests/Type/Symfony/data/messenger_handle_trait.php | 2 -- 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/src/Symfony/MessageMapFactory.php b/src/Symfony/MessageMapFactory.php index 3e0bd76c..5d3960f1 100644 --- a/src/Symfony/MessageMapFactory.php +++ b/src/Symfony/MessageMapFactory.php @@ -4,7 +4,6 @@ use PHPStan\Reflection\ClassReflection; use PHPStan\Reflection\MissingMethodFromReflectionException; -use PHPStan\Reflection\ParametersAcceptorSelector; use PHPStan\Reflection\ReflectionProvider; use Symfony\Component\Messenger\Handler\MessageSubscriberInterface; use function count; @@ -15,6 +14,7 @@ final class MessageMapFactory { + private const MESSENGER_HANDLER_TAG = 'messenger.message_handler'; private const DEFAULT_HANDLER_METHOD = '__invoke'; /** @var ReflectionProvider */ @@ -36,15 +36,12 @@ public function create(): MessageMap foreach ($this->serviceMap->getServices() as $service) { $serviceClass = $service->getClass(); - // todo handle abstract/parent services somehow? if (is_null($serviceClass)) { continue; } foreach ($service->getTags() as $tag) { - // todo could there be more tags with the same name for the same service? - // todo check if message handler tag name is constant or configurable - if ($tag->getName() !== 'messenger.message_handler') { + if ($tag->getName() !== self::MESSENGER_HANDLER_TAG) { continue; } @@ -59,9 +56,10 @@ public function create(): MessageMap foreach ($handles as $messageClassName => $options) { $methodReflection = $reflectionClass->getNativeMethod($options['method'] ?? self::DEFAULT_HANDLER_METHOD); - $variant = ParametersAcceptorSelector::selectSingle($methodReflection->getVariants()); - $returnTypesMap[$messageClassName][] = $variant->getReturnType(); + foreach ($methodReflection->getVariants() as $variant) { + $returnTypesMap[$messageClassName][] = $variant->getReturnType(); + } } } } diff --git a/tests/Type/Symfony/data/messenger_handle_trait.php b/tests/Type/Symfony/data/messenger_handle_trait.php index fb137e0c..7a86d482 100644 --- a/tests/Type/Symfony/data/messenger_handle_trait.php +++ b/tests/Type/Symfony/data/messenger_handle_trait.php @@ -49,8 +49,6 @@ public function handleString(StringQuery $query): string { return 'string result'; } - - // todo add handle method with union return type? } class TaggedQuery {} From f7bf1e074c040739c8e9ee81c85dc0742608ede5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Nowak?= Date: Fri, 18 Oct 2024 12:44:00 +0200 Subject: [PATCH 11/14] code review 1 --- src/Symfony/MessageMap.php | 7 ++-- src/Symfony/MessageMapFactory.php | 33 +++++++++++++++---- ...essengerHandleTraitReturnTypeExtension.php | 3 +- 3 files changed, 32 insertions(+), 11 deletions(-) diff --git a/src/Symfony/MessageMap.php b/src/Symfony/MessageMap.php index e5d6f307..d3451bbe 100644 --- a/src/Symfony/MessageMap.php +++ b/src/Symfony/MessageMap.php @@ -7,17 +7,16 @@ final class MessageMap { - /** @var array */ + /** @var array */ private $messageMap; - /** - * @param array $messageMap - */ + /** @param array $messageMap */ public function __construct(array $messageMap) { $this->messageMap = $messageMap; } + /** @param class-string $class */ public function getTypeForClass(string $class): ?Type { return $this->messageMap[$class] ?? null; diff --git a/src/Symfony/MessageMapFactory.php b/src/Symfony/MessageMapFactory.php index 5d3960f1..ee443a63 100644 --- a/src/Symfony/MessageMapFactory.php +++ b/src/Symfony/MessageMapFactory.php @@ -5,10 +5,12 @@ use PHPStan\Reflection\ClassReflection; use PHPStan\Reflection\MissingMethodFromReflectionException; use PHPStan\Reflection\ReflectionProvider; +use PHPStan\ShouldNotHappenException; use Symfony\Component\Messenger\Handler\MessageSubscriberInterface; +use function class_exists; use function count; +use function is_array; use function is_int; -use function is_null; use function is_string; final class MessageMapFactory @@ -36,7 +38,7 @@ public function create(): MessageMap foreach ($this->serviceMap->getServices() as $service) { $serviceClass = $service->getClass(); - if (is_null($serviceClass)) { + if ($serviceClass === null) { continue; } @@ -45,6 +47,7 @@ public function create(): MessageMap continue; } + /** @var array{handles?: class-string, method?: string} $tagAttributes */ $tagAttributes = $tag->getAttributes(); $reflectionClass = $this->reflectionProvider->getClass($serviceClass); @@ -76,15 +79,15 @@ public function create(): MessageMap return new MessageMap($messageMap); } - /** @return array> */ + /** @return array> */ private function guessHandledMessages(ClassReflection $reflectionClass): iterable { if ($reflectionClass->implementsInterface(MessageSubscriberInterface::class)) { foreach ($reflectionClass->getName()::getHandledMessages() as $index => $value) { - if (is_int($index) && is_string($value)) { - yield $value => ['method' => self::DEFAULT_HANDLER_METHOD]; - } else { + if (self::containOptions($index, $value)) { yield $index => $value; + } else { + yield $value => ['method' => self::DEFAULT_HANDLER_METHOD]; } } @@ -108,6 +111,7 @@ private function guessHandledMessages(ClassReflection $reflectionClass): iterabl return; } + /** @var class-string[] $classNames */ $classNames = $parameters[0]->getType()->getObjectClassNames(); if (count($classNames) !== 1) { @@ -117,4 +121,21 @@ private function guessHandledMessages(ClassReflection $reflectionClass): iterabl yield $classNames[0] => ['method' => self::DEFAULT_HANDLER_METHOD]; } + /** + * @phpstan-assert-if-true class-string $index + * @phpstan-assert-if-true array $value + * @phpstan-assert-if-false int $index + * @phpstan-assert-if-false class-string $value + */ + private static function containOptions(mixed $index, mixed $value): bool + { + if (is_string($index) && class_exists($index) && is_array($value)) { + return true; + } elseif (is_int($index) && is_string($value) && class_exists($value)) { + return false; + } + + throw new ShouldNotHappenException(); + } + } diff --git a/src/Type/Symfony/MessengerHandleTraitReturnTypeExtension.php b/src/Type/Symfony/MessengerHandleTraitReturnTypeExtension.php index 90160980..2e77dad3 100644 --- a/src/Type/Symfony/MessengerHandleTraitReturnTypeExtension.php +++ b/src/Type/Symfony/MessengerHandleTraitReturnTypeExtension.php @@ -35,6 +35,7 @@ public function getType(Expr $expr, Scope $scope): ?Type { if ($this->isSupported($expr, $scope)) { $arg = $expr->getArgs()[0]->value; + /** @var class-string[] $argClassNames */ $argClassNames = $scope->getType($arg)->getObjectClassNames(); if (count($argClassNames) === 1) { @@ -52,7 +53,7 @@ public function getType(Expr $expr, Scope $scope): ?Type private function getMessageMap(): MessageMap { - if (is_null($this->messageMap)) { + if ($this->messageMap === null) { $this->messageMap = $this->messageMapFactory->create(); } From d62ec03344e9004f61a48d5b16109b4ed32141ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Nowak?= Date: Fri, 18 Oct 2024 12:48:42 +0200 Subject: [PATCH 12/14] code review 2 removed mixed type which doesn't exist in $value * @phpstan-assert-if-false int $index * @phpstan-assert-if-false class-string $value */ - private static function containOptions(mixed $index, mixed $value): bool + private static function containOptions($index, $value): bool { if (is_string($index) && class_exists($index) && is_array($value)) { return true; From 89c7534a1377a5fa2f5fa034d2f2c848a886fe7f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Nowak?= Date: Mon, 23 Dec 2024 09:04:34 +0100 Subject: [PATCH 13/14] code review fixes --- .gitignore | 1 - src/Symfony/MessageMap.php | 5 +- src/Symfony/MessageMapFactory.php | 49 ++++++++++++------- ...essengerHandleTraitReturnTypeExtension.php | 16 +++--- 4 files changed, 42 insertions(+), 29 deletions(-) diff --git a/.gitignore b/.gitignore index c0c8f534..7de9f3c5 100644 --- a/.gitignore +++ b/.gitignore @@ -1,6 +1,5 @@ /tests/tmp /build-cs /vendor -/.idea /composer.lock .phpunit.result.cache diff --git a/src/Symfony/MessageMap.php b/src/Symfony/MessageMap.php index d3451bbe..7523742c 100644 --- a/src/Symfony/MessageMap.php +++ b/src/Symfony/MessageMap.php @@ -7,16 +7,15 @@ final class MessageMap { - /** @var array */ + /** @var array */ private $messageMap; - /** @param array $messageMap */ + /** @param array $messageMap */ public function __construct(array $messageMap) { $this->messageMap = $messageMap; } - /** @param class-string $class */ public function getTypeForClass(string $class): ?Type { return $this->messageMap[$class] ?? null; diff --git a/src/Symfony/MessageMapFactory.php b/src/Symfony/MessageMapFactory.php index 92a2de42..fc0fcc0e 100644 --- a/src/Symfony/MessageMapFactory.php +++ b/src/Symfony/MessageMapFactory.php @@ -3,7 +3,6 @@ namespace PHPStan\Symfony; use PHPStan\Reflection\ClassReflection; -use PHPStan\Reflection\MissingMethodFromReflectionException; use PHPStan\Reflection\ReflectionProvider; use PHPStan\ShouldNotHappenException; use Symfony\Component\Messenger\Handler\MessageSubscriberInterface; @@ -47,9 +46,14 @@ public function create(): MessageMap continue; } + if (!$this->reflectionProvider->hasClass($serviceClass)) { + continue; + } + + $reflectionClass = $this->reflectionProvider->getClass($serviceClass); + /** @var array{handles?: class-string, method?: string} $tagAttributes */ $tagAttributes = $tag->getAttributes(); - $reflectionClass = $this->reflectionProvider->getClass($serviceClass); if (isset($tagAttributes['handles'])) { $handles = [$tagAttributes['handles'] => ['method' => $tagAttributes['method'] ?? self::DEFAULT_HANDLER_METHOD]]; @@ -58,7 +62,13 @@ public function create(): MessageMap } foreach ($handles as $messageClassName => $options) { - $methodReflection = $reflectionClass->getNativeMethod($options['method'] ?? self::DEFAULT_HANDLER_METHOD); + $methodName = $options['method'] ?? self::DEFAULT_HANDLER_METHOD; + + if (!$reflectionClass->hasNativeMethod($methodName)) { + continue; + } + + $methodReflection = $reflectionClass->getNativeMethod($methodName); foreach ($methodReflection->getVariants() as $variant) { $returnTypesMap[$messageClassName][] = $variant->getReturnType(); @@ -79,27 +89,33 @@ public function create(): MessageMap return new MessageMap($messageMap); } - /** @return array> */ + /** @return iterable> */ private function guessHandledMessages(ClassReflection $reflectionClass): iterable { if ($reflectionClass->implementsInterface(MessageSubscriberInterface::class)) { - foreach ($reflectionClass->getName()::getHandledMessages() as $index => $value) { - if (self::containOptions($index, $value)) { - yield $index => $value; - } else { - yield $value => ['method' => self::DEFAULT_HANDLER_METHOD]; + $className = $reflectionClass->getName(); + + foreach ($className::getHandledMessages() as $index => $value) { + try { + if (self::containOptions($index, $value)) { + yield $index => $value; + } else { + yield $value => ['method' => self::DEFAULT_HANDLER_METHOD]; + } + } catch (ShouldNotHappenException $e) { + continue; } } return; } - try { - $methodReflection = $reflectionClass->getNativeMethod(self::DEFAULT_HANDLER_METHOD); - } catch (MissingMethodFromReflectionException $e) { + if (!$reflectionClass->hasNativeMethod(self::DEFAULT_HANDLER_METHOD)) { return; } + $methodReflection = $reflectionClass->getNativeMethod(self::DEFAULT_HANDLER_METHOD); + $variants = $methodReflection->getVariants(); if (count($variants) !== 1) { return; @@ -111,7 +127,6 @@ private function guessHandledMessages(ClassReflection $reflectionClass): iterabl return; } - /** @var class-string[] $classNames */ $classNames = $parameters[0]->getType()->getObjectClassNames(); if (count($classNames) !== 1) { @@ -124,10 +139,10 @@ private function guessHandledMessages(ClassReflection $reflectionClass): iterabl /** * @param mixed $index * @param mixed $value - * @phpstan-assert-if-true class-string $index - * @phpstan-assert-if-true array $value - * @phpstan-assert-if-false int $index - * @phpstan-assert-if-false class-string $value + * @phpstan-assert-if-true =class-string $index + * @phpstan-assert-if-true =array $value + * @phpstan-assert-if-false =int $index + * @phpstan-assert-if-false =class-string $value */ private static function containOptions($index, $value): bool { diff --git a/src/Type/Symfony/MessengerHandleTraitReturnTypeExtension.php b/src/Type/Symfony/MessengerHandleTraitReturnTypeExtension.php index 2e77dad3..c784a8fd 100644 --- a/src/Type/Symfony/MessengerHandleTraitReturnTypeExtension.php +++ b/src/Type/Symfony/MessengerHandleTraitReturnTypeExtension.php @@ -10,7 +10,6 @@ use PHPStan\Symfony\MessageMapFactory; use PHPStan\Type\ExpressionTypeResolverExtension; use PHPStan\Type\Type; -use ReflectionException; use function count; use function is_null; @@ -35,7 +34,6 @@ public function getType(Expr $expr, Scope $scope): ?Type { if ($this->isSupported($expr, $scope)) { $arg = $expr->getArgs()[0]->value; - /** @var class-string[] $argClassNames */ $argClassNames = $scope->getType($arg)->getObjectClassNames(); if (count($argClassNames) === 1) { @@ -61,7 +59,7 @@ private function getMessageMap(): MessageMap } /** - * @phpstan-assert-if-true MethodCall $expr + * @phpstan-assert-if-true =MethodCall $expr */ private function isSupported(Expr $expr, Scope $scope): bool { @@ -73,14 +71,16 @@ private function isSupported(Expr $expr, Scope $scope): bool return false; } - try { - $methodReflection = $scope->getClassReflection()->getNativeReflection()->getMethod(self::TRAIT_METHOD_NAME); - $declaringClassReflection = $methodReflection->getBetterReflection()->getDeclaringClass(); + $reflectionClass = $scope->getClassReflection()->getNativeReflection(); - return $declaringClassReflection->getName() === self::TRAIT_NAME; - } catch (ReflectionException $e) { + if (!$reflectionClass->hasMethod(self::TRAIT_METHOD_NAME)) { return false; } + + $methodReflection = $reflectionClass->getMethod(self::TRAIT_METHOD_NAME); + $declaringClassReflection = $methodReflection->getBetterReflection()->getDeclaringClass(); + + return $declaringClassReflection->getName() === self::TRAIT_NAME; } } From df90a3bd9b5a3caa4ebc1f6786f100279f6792e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Nowak?= Date: Mon, 30 Dec 2024 07:23:22 +0100 Subject: [PATCH 14/14] code review fixes 2 --- src/Symfony/MessageMapFactory.php | 18 +++++++----------- ...MessengerHandleTraitReturnTypeExtension.php | 7 ++++++- 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/src/Symfony/MessageMapFactory.php b/src/Symfony/MessageMapFactory.php index fc0fcc0e..5c9ef152 100644 --- a/src/Symfony/MessageMapFactory.php +++ b/src/Symfony/MessageMapFactory.php @@ -4,7 +4,6 @@ use PHPStan\Reflection\ClassReflection; use PHPStan\Reflection\ReflectionProvider; -use PHPStan\ShouldNotHappenException; use Symfony\Component\Messenger\Handler\MessageSubscriberInterface; use function class_exists; use function count; @@ -96,14 +95,11 @@ private function guessHandledMessages(ClassReflection $reflectionClass): iterabl $className = $reflectionClass->getName(); foreach ($className::getHandledMessages() as $index => $value) { - try { - if (self::containOptions($index, $value)) { - yield $index => $value; - } else { - yield $value => ['method' => self::DEFAULT_HANDLER_METHOD]; - } - } catch (ShouldNotHappenException $e) { - continue; + $containOptions = self::containOptions($index, $value); + if ($containOptions === true) { + yield $index => $value; + } elseif ($containOptions === false) { + yield $value => ['method' => self::DEFAULT_HANDLER_METHOD]; } } @@ -144,7 +140,7 @@ private function guessHandledMessages(ClassReflection $reflectionClass): iterabl * @phpstan-assert-if-false =int $index * @phpstan-assert-if-false =class-string $value */ - private static function containOptions($index, $value): bool + private static function containOptions($index, $value): ?bool { if (is_string($index) && class_exists($index) && is_array($value)) { return true; @@ -152,7 +148,7 @@ private static function containOptions($index, $value): bool return false; } - throw new ShouldNotHappenException(); + return null; } } diff --git a/src/Type/Symfony/MessengerHandleTraitReturnTypeExtension.php b/src/Type/Symfony/MessengerHandleTraitReturnTypeExtension.php index c784a8fd..a5dce362 100644 --- a/src/Type/Symfony/MessengerHandleTraitReturnTypeExtension.php +++ b/src/Type/Symfony/MessengerHandleTraitReturnTypeExtension.php @@ -33,7 +33,12 @@ public function __construct(MessageMapFactory $symfonyMessageMapFactory) public function getType(Expr $expr, Scope $scope): ?Type { if ($this->isSupported($expr, $scope)) { - $arg = $expr->getArgs()[0]->value; + $args = $expr->getArgs(); + if (count($args) !== 1) { + return null; + } + + $arg = $args[0]->value; $argClassNames = $scope->getType($arg)->getObjectClassNames(); if (count($argClassNames) === 1) {