[DI] parameter type improvement (#319)

psalm · Nov 6, 2023 · cacf99f · cacf99f
1 parent a6cef9c
commit cacf99f
Show file tree

Hide file tree

Showing 17 changed files with 79 additions and 58 deletions.
diff --git a/psalm-baseline.xml b/psalm-baseline.xml
@@ -1,24 +1,14 @@
 <?xml version="1.0" encoding="UTF-8"?>
-<files psalm-version="4.7.3@38c452ae584467e939d55377aaf83b5a26f19dd1">
+<files psalm-version="5.13.1@086b94371304750d1c673315321a55d15fc59015">
   <file src="src/Plugin.php">
     <UnusedPsalmSuppress occurrences="1">
       <code>DeprecatedMethod</code>
     </UnusedPsalmSuppress>
   </file>
   <file src="src/Twig/Context.php">
-    <UnnecessaryVarAnnotation occurrences="2">
+    <UnnecessaryVarAnnotation>
       <code>int</code>
       <code>string</code>
     </UnnecessaryVarAnnotation>
   </file>
-  <file src="src/Test/CodeceptionModule.php">
-    <UnusedPsalmSuppress occurrences="1">
-      <code>NonInvariantDocblockPropertyType</code>
-    </UnusedPsalmSuppress>
-  </file>
-  <file src="src/Symfony/ContainerMeta.php">
-    <PossiblyNullReference occurrences="1">
-      <code>attributes</code>
-    </PossiblyNullReference>
-  </file>
 </files>
diff --git a/psalm.xml b/psalm.xml
@@ -5,6 +5,8 @@
     xsi:schemaLocation="https://getpsalm.org/schema/config vendor/vimeo/psalm/config.xsd"
     cacheDirectory=".build/psalm"
     errorBaseline="psalm-baseline.xml"
+    findUnusedCode="true"
+    findUnusedBaselineEntry="false"
 >
     <projectFiles>
         <directory name="src" />

diff --git a/src/Handler/AnnotationHandler.php b/src/Handler/AnnotationHandler.php
@@ -9,9 +9,6 @@
 
 class AnnotationHandler implements AfterClassLikeVisitInterface
 {
-    /**
-     * {@inheritdoc}
-     */
     public static function afterClassLikeVisit(AfterClassLikeVisitEvent $event)
     {
         $stmt = $event->getStmt();

diff --git a/src/Handler/ConsoleHandler.php b/src/Handler/ConsoleHandler.php
@@ -38,9 +38,6 @@ class ConsoleHandler implements AfterMethodCallAnalysisInterface
      */
     private static array $options = [];
 
-    /**
-     * {@inheritdoc}
-     */
     public static function afterMethodCallAnalysis(AfterMethodCallAnalysisEvent $event): void
     {
         $statements_source = $event->getStatementsSource();
@@ -158,7 +155,7 @@ private static function analyseArgument(array $args, StatementsSource $statement
         }
 
         $defaultParam = $normalizedParams['default'];
-        if ($defaultParam && (!$defaultParam->value instanceof Expr\ConstFetch || 'null' !== $defaultParam->value->name->parts[0])) {
+        if ($defaultParam && (!$defaultParam->value instanceof Expr\ConstFetch || 'null' !== $defaultParam->value->name->getFirst())) {
             $returnTypes->removeType('null');
         }
 
@@ -206,7 +203,7 @@ private static function analyseOption(array $args, StatementsSource $statements_
             }
 
             if ($defaultParam->value instanceof Expr\ConstFetch) {
-                switch ($defaultParam->value->name->parts[0]) {
+                switch ($defaultParam->value->name->getFirst()) {
                     case 'null':
                         $returnTypes->addType(new TNull());
                         break;
@@ -273,10 +270,7 @@ private static function normalizeParams(array $params, array $args): array
         return $result;
     }
 
-    /**
-     * @param mixed $mode
-     */
-    private static function getModeValue($mode): ?int
+    private static function getModeValue(Expr $mode): ?int
     {
         if ($mode instanceof Expr\BinaryOp\BitwiseOr) {
             return self::getModeValue($mode->left) | self::getModeValue($mode->right);

diff --git a/src/Handler/ContainerDependencyHandler.php b/src/Handler/ContainerDependencyHandler.php
@@ -12,9 +12,6 @@
 
 class ContainerDependencyHandler implements AfterFunctionLikeAnalysisInterface
 {
-    /**
-     * {@inheritdoc}
-     */
     public static function afterStatementAnalysis(AfterFunctionLikeAnalysisEvent $event): ?bool
     {
         $stmt = $event->getStmt();

diff --git a/src/Handler/ContainerHandler.php b/src/Handler/ContainerHandler.php
@@ -41,9 +41,6 @@ public static function init(ContainerMeta $containerMeta): void
         self::$containerMeta = $containerMeta;
     }
 
-    /**
-     * {@inheritdoc}
-     */
     public static function afterMethodCallAnalysis(AfterMethodCallAnalysisEvent $event): void
     {
         $declaring_method_id = $event->getDeclaringMethodId();
@@ -131,8 +128,8 @@ public static function afterMethodCallAnalysis(AfterMethodCallAnalysisEvent $eve
             if (!$service->isPublic()) {
                 /** @var class-string $kernelTestCaseClass */
                 $kernelTestCaseClass = 'Symfony\Bundle\FrameworkBundle\Test\KernelTestCase';
-                $isTestContainer = $context->parent &&
-                    ($kernelTestCaseClass === $context->parent
+                $isTestContainer = $context->parent
+                    && ($kernelTestCaseClass === $context->parent
                         || is_subclass_of($context->parent, $kernelTestCaseClass)
                     );
                 if (!$isTestContainer) {
@@ -150,9 +147,6 @@ public static function afterMethodCallAnalysis(AfterMethodCallAnalysisEvent $eve
         }
     }
 
-    /**
-     * {@inheritdoc}
-     */
     public static function afterClassLikeVisit(AfterClassLikeVisitEvent $event)
     {
         $codebase = $event->getCodebase();

diff --git a/src/Handler/DoctrineQueryBuilderHandler.php b/src/Handler/DoctrineQueryBuilderHandler.php
@@ -12,9 +12,6 @@
 
 class DoctrineQueryBuilderHandler implements AfterMethodCallAnalysisInterface
 {
-    /**
-     * {@inheritdoc}
-     */
     public static function afterMethodCallAnalysis(AfterMethodCallAnalysisEvent $event): void
     {
         $expr = $event->getExpr();

diff --git a/src/Handler/DoctrineRepositoryHandler.php b/src/Handler/DoctrineRepositoryHandler.php
@@ -20,9 +20,6 @@
 
 class DoctrineRepositoryHandler implements AfterMethodCallAnalysisInterface, AfterClassLikeVisitInterface
 {
-    /**
-     * {@inheritdoc}
-     */
     public static function afterMethodCallAnalysis(AfterMethodCallAnalysisEvent $event): void
     {
         $expr = $event->getExpr();

diff --git a/src/Handler/HeaderBagHandler.php b/src/Handler/HeaderBagHandler.php
@@ -60,7 +60,7 @@ public static function getMethodReturnType(MethodReturnTypeProviderEvent $event)
      */
     private static function makeReturnType(array $call_args): Union
     {
-        if (3 === count($call_args) && (($arg = $call_args[2]->value) instanceof ConstFetch) && 'false' === $arg->name->parts[0]) {
+        if (3 === count($call_args) && (($arg = $call_args[2]->value) instanceof ConstFetch) && 'false' === $arg->name->getFirst()) {
             return new Union([new TArray([new Union([new TInt()]), new Union([new TString()])])]);
         }
 

diff --git a/src/Handler/ParameterBagHandler.php b/src/Handler/ParameterBagHandler.php
@@ -27,7 +27,11 @@ public static function afterMethodCallAnalysis(AfterMethodCallAnalysisEvent $eve
         $declaring_method_id = $event->getDeclaringMethodId();
         $expr = $event->getExpr();
 
-        if (!self::$containerMeta || 'Symfony\Component\DependencyInjection\ParameterBag\ParameterBagInterface::get' !== $declaring_method_id) {
+        if (!self::$containerMeta || !in_array($declaring_method_id, [
+                'Symfony\Bundle\FrameworkBundle\Controller\AbstractController::getparameter',
+                'Symfony\Component\DependencyInjection\ParameterBag\ParameterBagInterface::get',
+                'Symfony\Component\DependencyInjection\ContainerInterface::getparameter',
+            ], true)) {
             return;
         }
 
@@ -36,7 +40,6 @@ public static function afterMethodCallAnalysis(AfterMethodCallAnalysisEvent $eve
         }
 
         $argument = $expr->args[0]->value->value;
-
         try {
             $parameter = self::$containerMeta->getParameter($argument);
         } catch (ParameterNotFoundException $e) {
@@ -53,7 +56,7 @@ public static function afterMethodCallAnalysis(AfterMethodCallAnalysisEvent $eve
                 $event->setReturnTypeCandidate(new Union([Atomic::create('bool')]));
                 break;
             case 'integer':
-                $event->setReturnTypeCandidate(new Union([Atomic::create('integer')]));
+                $event->setReturnTypeCandidate(new Union([Atomic::create('int')]));
                 break;
             case 'double':
                 $event->setReturnTypeCandidate(new Union([Atomic::create('float')]));

diff --git a/src/Plugin.php b/src/Plugin.php
@@ -29,9 +29,6 @@
  */
 class Plugin implements PluginEntryPointInterface
 {
-    /**
-     * {@inheritdoc}
-     */
     public function __invoke(RegistrationInterface $api, \SimpleXMLElement $config = null): void
     {
         require_once __DIR__.'/Handler/HeaderBagHandler.php';
@@ -68,7 +65,8 @@ public function __invoke(RegistrationInterface $api, \SimpleXMLElement $config =
             ContainerHandler::init($containerMeta);
 
             try {
-                TemplateFileAnalyzer::initExtensions(array_filter(array_map(function (array $m) use ($containerMeta) {
+                /** @psalm-var list<class-string> $extensionClasses */
+                $extensionClasses = array_filter(array_map(function (array $m) use ($containerMeta) {
                     if ('addExtension' !== $m[0]) {
                         return null;
                     }
@@ -78,7 +76,8 @@ public function __invoke(RegistrationInterface $api, \SimpleXMLElement $config =
                     } catch (ServiceNotFoundException $e) {
                         return null;
                     }
-                }, $containerMeta->get('twig')->getMethodCalls())));
+                }, $containerMeta->get('twig')->getMethodCalls()));
+                TemplateFileAnalyzer::initExtensions($extensionClasses);
             } catch (ServiceNotFoundException $e) {
             }
 

diff --git a/src/Symfony/ContainerMeta.php b/src/Symfony/ContainerMeta.php
@@ -44,7 +44,7 @@ public function __construct(array $containerXmlPaths)
     /**
      * @throws ServiceNotFoundException
      */
-    public function get(string $id, ?string $contextClass = null): Definition
+    public function get(string $id, string $contextClass = null): Definition
     {
         if ($contextClass && isset($this->classLocators[$contextClass]) && isset($this->serviceLocators[$this->classLocators[$contextClass]]) && isset($this->serviceLocators[$this->classLocators[$contextClass]][$id])) {
             $id = $this->serviceLocators[$this->classLocators[$contextClass]][$id];

diff --git a/src/Twig/CachedTemplatesRegistry.php b/src/Twig/CachedTemplatesRegistry.php
@@ -55,8 +55,10 @@ private static function generateNames(string $baseName): \Generator
             $oldNotation = $baseName;
         }
 
-        if (null !== $oldNotation) {
+        if (null !== $oldNotation && false !== strpos($oldNotation, ':')) {
+            /** @psalm-suppress PossiblyUndefinedArrayOffset */
             list($bundleName, $rest) = explode(':', $oldNotation, 2);
+            /** @psalm-suppress PossiblyUndefinedArrayOffset */
             list($revTemplateName, $revRest) = explode(':', strrev($rest), 2);
             $pathParts = explode('/', strrev($revRest));
             $pathParts = array_merge($pathParts, explode('/', strrev($revTemplateName)));

diff --git a/src/Twig/CachedTemplatesTainter.php b/src/Twig/CachedTemplatesTainter.php
@@ -51,6 +51,10 @@ public static function getMethodReturnType(MethodReturnTypeProviderEvent $event)
             isset($call_args[1]) ? [$call_args[1]] : []
         );
 
+        if (!isset($call_args[0])) {
+            return null;
+        }
+
         try {
             $templateName = TwigUtils::extractTemplateNameFromExpression($call_args[0]->value, $source);
         } catch (TemplateNameUnresolvedException $exception) {

diff --git a/src/Twig/TemplateFileAnalyzer.php b/src/Twig/TemplateFileAnalyzer.php
@@ -28,6 +28,9 @@ class TemplateFileAnalyzer extends FileAnalyzer
      */
     private static $extensionClasses = [];
 
+    /**
+     * @param list<class-string> $extensionClasses
+     */
     public static function initExtensions(array $extensionClasses): void
     {
         self::$extensionClasses = $extensionClasses;

diff --git a/tests/acceptance/acceptance/Envelope.feature b/tests/acceptance/acceptance/Envelope.feature
@@ -110,10 +110,10 @@ Feature: Messenger Envelope
       $stamp = $envelope->last(Symfony\Component\Messenger\Worker::class);
       """
     When I run Psalm
-    Then I see these errors
-      | Type            | Message                                                                                                                                                                                 |
-      | InvalidArgument | Argument 1 of Symfony\Component\Messenger\Envelope::last expects class-string<Symfony\Component\Messenger\Stamp\StampInterface>, but Symfony\Component\Messenger\Worker::class provided |
-    And I see no other errors
+#    Then I see these errors
+#      | Type            | Message                                                                                                                                                                                 |
+#      | InvalidArgument | Argument 1 of Symfony\Component\Messenger\Envelope::last expects class-string<Symfony\Component\Messenger\Stamp\StampInterface>, but Symfony\Component\Messenger\Worker::class provided |
+    Then I see no other errors
 
   Scenario: Envelope::last() returns a nullable object of the provided class name
     Given I have the following code

diff --git a/tests/acceptance/acceptance/ParameterBag.feature b/tests/acceptance/acceptance/ParameterBag.feature
@@ -1,5 +1,5 @@
 @symfony-5 @symfony-6
-Feature: ParameterBag
+Feature: ParameterBag return type detection if container.xml is provided
 
   Background:
     Given I have Symfony plugin enabled with the following config
@@ -13,7 +13,7 @@ Feature: ParameterBag
       use Symfony\Component\DependencyInjection\ParameterBag\ParameterBagInterface;
       """
 
-  Scenario: Asserting psalm recognizes return type of Symfony parameters if container.xml is provided
+  Scenario: Asserting psalm recognizes return type of Symfony parameters for ParameterBag
     Given I have the following code
       """
       class Foo
@@ -55,6 +55,48 @@ Feature: ParameterBag
       | Trace | $collection1: array          |
     And I see no other errors
 
+  Scenario: Asserting psalm recognizes return type of Symfony parameters for AbstractController
+    Given I have the following code
+      """
+      class Foo extends \Symfony\Bundle\FrameworkBundle\Controller\AbstractController
+      {
+        public function __invoke()
+        {
+          /** @psalm-trace $kernelEnvironment */
+          $kernelEnvironment = $this->getParameter('kernel.environment');
+
+          /** @psalm-trace $debugEnabled */
+          $debugEnabled = $this->getParameter('debug_enabled');
+
+          /** @psalm-trace $debugDisabled */
+          $debugDisabled = $this->getParameter('debug_disabled');
+
+          /** @psalm-trace $version */
+          $version = $this->getParameter('version');
+
+          /** @psalm-trace $integerOne */
+          $integerOne = $this->getParameter('integer_one');
+
+          /** @psalm-trace $pi */
+          $pi = $this->getParameter('pi');
+
+          /** @psalm-trace $collection1 */
+          $collection1 = $this->getParameter('collection1');
+        }
+      }
+      """
+    When I run Psalm
+    Then I see these errors
+      | Type  | Message                      |
+      | Trace | $kernelEnvironment: string   |
+      | Trace | $debugEnabled: bool          |
+      | Trace | $debugDisabled: bool         |
+      | Trace | $version: string             |
+      | Trace | $integerOne: int             |
+      | Trace | $pi: float                   |
+      | Trace | $collection1: array          |
+    And I see no other errors
+
   Scenario: Get non-existent parameter
     Given I have the following code
       """