From 5b27d3f53cebebfea437539edda999f725b2a40b Mon Sep 17 00:00:00 2001 From: Farhad Safarov Date: Fri, 13 Nov 2020 11:58:26 +0300 Subject: [PATCH 1/2] [container] support service subscriber --- src/Handler/ContainerHandler.php | 48 ++++++++++++-- src/Symfony/ContainerMeta.php | 2 +- .../acceptance/ServiceSubscriber.feature | 64 +++++++++++++++++++ 3 files changed, 107 insertions(+), 7 deletions(-) create mode 100644 tests/acceptance/acceptance/ServiceSubscriber.feature diff --git a/src/Handler/ContainerHandler.php b/src/Handler/ContainerHandler.php index b22433cb..d4ca8e1e 100644 --- a/src/Handler/ContainerHandler.php +++ b/src/Handler/ContainerHandler.php @@ -5,7 +5,10 @@ use PhpParser\Node\Expr; use PhpParser\Node\Expr\ClassConstFetch; use PhpParser\Node\Scalar\String_; +use PhpParser\Node\Stmt\Class_; use PhpParser\Node\Stmt\ClassLike; +use PhpParser\Node\Stmt\ClassMethod; +use PhpParser\Node\Stmt\Return_; use Psalm\Codebase; use Psalm\CodeLocation; use Psalm\Context; @@ -19,6 +22,7 @@ use Psalm\SymfonyPsalmPlugin\Issue\PrivateService; use Psalm\SymfonyPsalmPlugin\Issue\ServiceNotFound; use Psalm\SymfonyPsalmPlugin\Symfony\ContainerMeta; +use Psalm\SymfonyPsalmPlugin\Symfony\Service; use Psalm\Type\Atomic\TNamedObject; use Psalm\Type\Union; @@ -58,7 +62,7 @@ public static function afterMethodCallAnalysis( if (!self::isContainerMethod($declaring_method_id, 'get')) { if (self::isContainerMethod($declaring_method_id, 'getparameter')) { $argument = $expr->args[0]->value; - if ($argument instanceof String_ && !self::followsNamingConvention($argument->value)) { + if ($argument instanceof String_ && !self::followsNamingConvention($argument->value) && false === strpos($argument->value, '\\')) { IssueBuffer::accepts( new NamingConventionViolation(new CodeLocation($statements_source, $argument)), $statements_source->getSuppressedIssues() @@ -90,7 +94,7 @@ public static function afterMethodCallAnalysis( $service = self::$containerMeta->get($serviceId); if ($service) { - if (!self::followsNamingConvention($serviceId) && !class_exists($service->getClassName())) { + if (!self::followsNamingConvention($serviceId) && false === strpos($serviceId, '\\')) { IssueBuffer::accepts( new NamingConventionViolation(new CodeLocation($statements_source, $expr->args[0]->value)), $statements_source->getSuppressedIssues() @@ -130,14 +134,46 @@ public static function afterClassLikeVisit( Codebase $codebase, array &$file_replacements = [] ) { + $fileStorage = $codebase->file_storage_provider->get($statements_source->getFilePath()); + if (\in_array($storage->name, ContainerHandler::GET_CLASSLIKES)) { if (self::$containerMeta) { - $file_path = $statements_source->getFilePath(); - $file_storage = $codebase->file_storage_provider->get($file_path); - foreach (self::$containerMeta->getClassNames() as $className) { $codebase->queueClassLikeForScanning($className); - $file_storage->referenced_classlikes[strtolower($className)] = $className; + $fileStorage->referenced_classlikes[strtolower($className)] = $className; + } + } + } + + // see https://symfony.com/doc/current/service_container/service_subscribers_locators.html + if (self::$containerMeta && $stmt instanceof Class_ && isset($storage->class_implements['symfony\contracts\service\servicesubscriberinterface'])) { + foreach ($stmt->stmts as $classStmt) { + if ($classStmt instanceof ClassMethod && 'getSubscribedServices' === $classStmt->name->name && $classStmt->stmts) { + foreach ($classStmt->stmts as $methodStmt) { + if ($methodStmt instanceof Return_ && ($return = $methodStmt->expr) && $return instanceof Expr\Array_) { + foreach ($return->items as $arrayItem) { + if ($arrayItem instanceof Expr\ArrayItem) { + $value = $arrayItem->value; + if (!$value instanceof Expr\ClassConstFetch) { + continue; + } + + /** @var string $className */ + $className = $value->class->getAttribute('resolvedName'); + + $key = $arrayItem->key; + $serviceId = $key instanceof String_ ? $key->value : $className; + + $service = new Service($serviceId, $className); + $service->setIsPublic(true); + self::$containerMeta->add($service); + + $codebase->queueClassLikeForScanning($className); + $fileStorage->referenced_classlikes[strtolower($className)] = $className; + } + } + } + } } } } diff --git a/src/Symfony/ContainerMeta.php b/src/Symfony/ContainerMeta.php index 98362760..65a7ea22 100644 --- a/src/Symfony/ContainerMeta.php +++ b/src/Symfony/ContainerMeta.php @@ -33,7 +33,7 @@ public function get(string $id): ?Service return null; } - private function add(Service $service): void + public function add(Service $service): void { if (($alias = $service->getAlias()) && isset($this->services[$alias])) { $aliasedService = $this->services[$alias]; diff --git a/tests/acceptance/acceptance/ServiceSubscriber.feature b/tests/acceptance/acceptance/ServiceSubscriber.feature new file mode 100644 index 00000000..aade1037 --- /dev/null +++ b/tests/acceptance/acceptance/ServiceSubscriber.feature @@ -0,0 +1,64 @@ +@symfony-4 @symfony-5 +Feature: Service Subscriber + + Background: + Given I have the following config + """ + + + + + + + + + + ../../tests/acceptance/container.xml + + + + """ + + Scenario: Asserting psalm recognizes return type of services defined in getSubscribedServices + Given I have the following code + """ + container = $container; + } + + public function __invoke() + { + /** @psalm-trace $entityManager */ + $entityManager = $this->container->get('em'); + + /** @psalm-trace $validator */ + $validator = $this->container->get(ValidatorInterface::class); + } + + public static function getSubscribedServices() + { + return [ + 'em' => EntityManagerInterface::class, // with key + ValidatorInterface::class, // without key + ]; + } + } + """ + When I run Psalm + Then I see these errors + | Type | Message | + | Trace | $entityManager: Doctrine\ORM\EntityManagerInterface | + | Trace | $validator: Symfony\Component\Validator\Validator\ValidatorInterface | + And I see no other errors From b9ad7019de54d82f009d6dc23f6b2d19982ad0f1 Mon Sep 17 00:00:00 2001 From: Farhad Safarov Date: Sat, 14 Nov 2020 13:13:48 +0300 Subject: [PATCH 2/2] no message --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index d922d11f..a7e6ebea 100644 --- a/README.md +++ b/README.md @@ -12,6 +12,7 @@ vendor/bin/psalm-plugin enable psalm/plugin-symfony ### Features - Detect `ContainerInterface::get()` result type. Works better if you [configure](#configuration) compiled container XML file. +- Support [Service Subscribers](https://github.com/psalm/psalm-plugin-symfony/issues/20). - Detect return type of console arguments (`InputInterface::getArgument()`) and options (`InputInterface::getOption()`). Enforces to use InputArgument and InputOption constants as a part of best practise. - Detects correct Doctrine repository class if entities are configured with annotations.