From 2dcb9cf661639e9d8d7b7a3a87a820cdc0989dff Mon Sep 17 00:00:00 2001 From: Farhad Safarov Date: Wed, 3 Jul 2024 14:48:37 +0300 Subject: [PATCH] [doctrine] improve repository handler --- README.md | 3 +- src/Handler/DoctrineRepositoryHandler.php | 92 ++++++++++--------- src/Plugin.php | 7 +- .../RepositoryClassWithAttributes.feature | 53 +++++++++++ .../fixture/Doctrine/EntityWithAttributes.php | 12 +++ .../EntityWithAttributesRepository.php | 14 +++ 6 files changed, 133 insertions(+), 48 deletions(-) create mode 100644 tests/acceptance/acceptance/doctrine/RepositoryClassWithAttributes.feature create mode 100644 tests/fixture/Doctrine/EntityWithAttributes.php create mode 100644 tests/fixture/Doctrine/EntityWithAttributesRepository.php diff --git a/README.md b/README.md index 10165eb..1766b2f 100644 --- a/README.md +++ b/README.md @@ -23,9 +23,10 @@ vendor/bin/psalm-plugin enable psalm/plugin-symfony ### Features - Detects the `ContainerInterface::get()` result type. Works better if you [configure](#configuration) a compiled container XML file. +- Detects parameter return types from `ContainerInterface::getParameter()`. - Supports [Service Subscribers](https://github.com/psalm/psalm-plugin-symfony/issues/20). Works only if you [configure](#configuration) a compiled container XML file. - Detects return types from console arguments (`InputInterface::getArgument()`) and options (`InputInterface::getOption()`). -Enforces to use "InputArgument" and "InputOption" constants as a best practise. +Enforces to use "InputArgument" and "InputOption" constants as a best practice. - Detects Doctrine repository classes associated to entities when configured via annotations. - Fixes `PossiblyInvalidArgument` for `Symfony\Component\HttpFoundation\Request::getContent()`. The plugin determines the real return type by checking the given argument and marks it as either "string" or "resource". diff --git a/src/Handler/DoctrineRepositoryHandler.php b/src/Handler/DoctrineRepositoryHandler.php index 91d8c88..9b13402 100644 --- a/src/Handler/DoctrineRepositoryHandler.php +++ b/src/Handler/DoctrineRepositoryHandler.php @@ -22,69 +22,73 @@ class DoctrineRepositoryHandler implements AfterMethodCallAnalysisInterface, Aft { public static function afterMethodCallAnalysis(AfterMethodCallAnalysisEvent $event): void { - $expr = $event->getExpr(); $declaring_method_id = $event->getDeclaringMethodId(); - $statements_source = $event->getStatementsSource(); + if (!in_array($declaring_method_id, ['Doctrine\ORM\EntityManagerInterface::getrepository', 'Doctrine\Persistence\ObjectManager::getrepository'])) { + return; + } - if (in_array($declaring_method_id, ['Doctrine\ORM\EntityManagerInterface::getrepository', 'Doctrine\Persistence\ObjectManager::getrepository'])) { - if (!isset($expr->args[0]->value)) { - return; - } + $expr = $event->getExpr(); + if (!isset($expr->args[0]->value)) { + return; + } - $entityName = $expr->args[0]->value; + $entityName = $expr->args[0]->value; + if (!$entityName instanceof Expr\ClassConstFetch) { if ($entityName instanceof String_) { + $statements_source = $event->getStatementsSource(); IssueBuffer::accepts( new RepositoryStringShortcut(new CodeLocation($statements_source, $entityName)), $statements_source->getSuppressedIssues() ); - } elseif ($entityName instanceof Expr\ClassConstFetch) { - /** @psalm-var class-string|null $className */ - $className = $entityName->class->getAttribute('resolvedName'); + } - if (null === $className) { - return; - } + return; + } - try { - $reflectionClass = new \ReflectionClass($className); - - if (\PHP_VERSION_ID >= 80000 && method_exists(\ReflectionClass::class, 'getAttributes')) { - $entityAttributes = $reflectionClass->getAttributes(EntityAnnotation::class); - - foreach ($entityAttributes as $entityAttribute) { - $arguments = $entityAttribute->getArguments(); - - if (isset($arguments['repositoryClass']) && is_string($arguments['repositoryClass'])) { - $event->setReturnTypeCandidate(new Union([new TNamedObject($arguments['repositoryClass'])])); - } - } - } - - if (class_exists(AnnotationReader::class)) { - $reader = new AnnotationReader(); - $entityAnnotation = $reader->getClassAnnotation( - $reflectionClass, - EntityAnnotation::class - ); - - if ($entityAnnotation instanceof EntityAnnotation && $entityAnnotation->repositoryClass) { - $event->setReturnTypeCandidate(new Union([new TNamedObject($entityAnnotation->repositoryClass)])); - } - } - } catch (\ReflectionException $e) { - } + /** @psalm-var class-string|null $className */ + $className = $entityName->class->getAttribute('resolvedName'); + if (null === $className) { + return; + } + + try { + $reflectionClass = new \ReflectionClass($className); + } catch (\ReflectionException) { + return; + } + + $entityAttributes = $reflectionClass->getAttributes(EntityAnnotation::class); + foreach ($entityAttributes as $entityAttribute) { + $arguments = $entityAttribute->getArguments(); + + if (isset($arguments['repositoryClass']) && is_string($arguments['repositoryClass'])) { + $event->setReturnTypeCandidate(new Union([new TNamedObject($arguments['repositoryClass'])])); + + return; + } + } + + if (class_exists(AnnotationReader::class)) { + $reader = new AnnotationReader(); + $entityAnnotation = $reader->getClassAnnotation( + $reflectionClass, + EntityAnnotation::class + ); + + if ($entityAnnotation instanceof EntityAnnotation && $entityAnnotation->repositoryClass) { + $event->setReturnTypeCandidate(new Union([new TNamedObject($entityAnnotation->repositoryClass)])); } } } - public static function afterClassLikeVisit(AfterClassLikeVisitEvent $event) + public static function afterClassLikeVisit(AfterClassLikeVisitEvent $event): void { $stmt = $event->getStmt(); $statements_source = $event->getStatementsSource(); $codebase = $event->getCodebase(); $docblock = $stmt->getDocComment(); - if ($docblock && false !== strpos((string) $docblock, 'repositoryClass')) { + if ($docblock && str_contains((string) $docblock, 'repositoryClass')) { try { $parsedComment = DocComment::parsePreservingLength($docblock); if (isset($parsedComment->tags['Entity'])) { @@ -96,7 +100,7 @@ public static function afterClassLikeVisit(AfterClassLikeVisitEvent $event) $codebase->queueClassLikeForScanning($repositoryClassName); $file_storage->referenced_classlikes[strtolower($repositoryClassName)] = $repositoryClassName; } - } catch (DocblockParseException $e) { + } catch (DocblockParseException) { } } } diff --git a/src/Plugin.php b/src/Plugin.php index 4cd10b3..26f5afa 100644 --- a/src/Plugin.php +++ b/src/Plugin.php @@ -36,7 +36,6 @@ public function __invoke(RegistrationInterface $registration, ?\SimpleXMLElement require_once __DIR__.'/Handler/ConsoleHandler.php'; require_once __DIR__.'/Handler/ContainerDependencyHandler.php'; require_once __DIR__.'/Handler/RequiredSetterHandler.php'; - require_once __DIR__.'/Handler/DoctrineQueryBuilderHandler.php'; require_once __DIR__.'/Provider/FormGetErrorsReturnTypeProvider.php'; $registration->registerHooksFromClass(HeaderBagHandler::class); @@ -45,16 +44,18 @@ public function __invoke(RegistrationInterface $registration, ?\SimpleXMLElement $registration->registerHooksFromClass(RequiredSetterHandler::class); if (class_exists(\Doctrine\ORM\QueryBuilder::class)) { + require_once __DIR__.'/Handler/DoctrineQueryBuilderHandler.php'; $registration->registerHooksFromClass(DoctrineQueryBuilderHandler::class); + + require_once __DIR__.'/Handler/DoctrineRepositoryHandler.php'; + $registration->registerHooksFromClass(DoctrineRepositoryHandler::class); } if (class_exists(AnnotationRegistry::class)) { - require_once __DIR__.'/Handler/DoctrineRepositoryHandler.php'; if (method_exists(AnnotationRegistry::class, 'registerLoader')) { /** @psalm-suppress DeprecatedMethod */ AnnotationRegistry::registerLoader('class_exists'); } - $registration->registerHooksFromClass(DoctrineRepositoryHandler::class); require_once __DIR__.'/Handler/AnnotationHandler.php'; $registration->registerHooksFromClass(AnnotationHandler::class); diff --git a/tests/acceptance/acceptance/doctrine/RepositoryClassWithAttributes.feature b/tests/acceptance/acceptance/doctrine/RepositoryClassWithAttributes.feature new file mode 100644 index 0000000..2c8c5c9 --- /dev/null +++ b/tests/acceptance/acceptance/doctrine/RepositoryClassWithAttributes.feature @@ -0,0 +1,53 @@ +@symfony-common +Feature: RepositoryClass using attributes + + Background: + Given I have issue handlers "UndefinedClass,UnusedVariable" suppressed + And I have Symfony plugin enabled + And I have the following code preamble + """ + getRepository(EntityWithAttributes::class); + } + } + """ + When I run Psalm + Then I see these errors + | Type | Message | + | Trace | $repository: Psalm\SymfonyPsalmPlugin\Tests\Fixture\Doctrine\EntityWithAttributesRepository | + And I see no other errors + + Scenario: Passing variable class does not crash the plugin + Given I have the following code + """ + class SomeService + { + public function __construct(EntityManagerInterface $entityManager) + { + $entity = 'Psalm\SymfonyPsalmPlugin\Tests\Fixture\Doctrine\EntityWithAttributes'; + /** @psalm-trace $repository */ + $repository = $entityManager->getRepository($entity::class); + } + } + """ + When I run Psalm + Then I see these errors + | Type | Message | + | Trace | $repository: Doctrine\ORM\EntityRepository | + | MixedArgument | Argument 1 of Doctrine\ORM\EntityManagerInterface::getRepository cannot be mixed, expecting class-string | + And I see no other errors diff --git a/tests/fixture/Doctrine/EntityWithAttributes.php b/tests/fixture/Doctrine/EntityWithAttributes.php new file mode 100644 index 0000000..ec88ebc --- /dev/null +++ b/tests/fixture/Doctrine/EntityWithAttributes.php @@ -0,0 +1,12 @@ + + */ +class EntityWithAttributesRepository extends EntityRepository +{ +}