Deduct return type from environment variable processors (#346)

Fixes #345
psalm · Jun 6, 2024 · 58e1092 · 58e1092
1 parent 0fe09bf
commit 58e1092
Show file tree

Hide file tree

Showing 4 changed files with 77 additions and 20 deletions.
diff --git a/src/Handler/ParameterBagHandler.php b/src/Handler/ParameterBagHandler.php
@@ -42,30 +42,18 @@ public static function afterMethodCallAnalysis(AfterMethodCallAnalysisEvent $eve
         }
 
         $argument = $expr->args[0]->value->value;
+
         try {
-            $parameter = self::$containerMeta->getParameter($argument);
-        } catch (ParameterNotFoundException $e) {
+            $parameterTypes = self::$containerMeta->guessParameterType($argument);
+        } catch (ParameterNotFoundException) {
             // maybe emit ParameterNotFound issue
             return;
         }
 
-        // @todo find a better way to calculate return type
-        switch (gettype($parameter)) {
-            case 'string':
-                $event->setReturnTypeCandidate(new Union([Atomic::create('string')]));
-                break;
-            case 'boolean':
-                $event->setReturnTypeCandidate(new Union([Atomic::create('bool')]));
-                break;
-            case 'integer':
-                $event->setReturnTypeCandidate(new Union([Atomic::create('int')]));
-                break;
-            case 'double':
-                $event->setReturnTypeCandidate(new Union([Atomic::create('float')]));
-                break;
-            case 'array':
-                $event->setReturnTypeCandidate(new Union([Atomic::create('array')]));
-                break;
+        if (null === $parameterTypes || [] === $parameterTypes) {
+            return;
         }
+
+        $event->setReturnTypeCandidate(new Union(array_map(fn (string $parameterType): Atomic => Atomic::create($parameterType), $parameterTypes)));
     }
 }
diff --git a/src/Symfony/ContainerMeta.php b/src/Symfony/ContainerMeta.php
@@ -9,7 +9,9 @@
 use Symfony\Component\DependencyInjection\Argument\ServiceClosureArgument;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
 use Symfony\Component\DependencyInjection\Definition;
+use Symfony\Component\DependencyInjection\EnvVarProcessor;
 use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException;
+use Symfony\Component\DependencyInjection\Exception\ParameterNotFoundException;
 use Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException;
 use Symfony\Component\DependencyInjection\Loader\XmlFileLoader;
 use Symfony\Component\DependencyInjection\Reference;
@@ -69,6 +71,29 @@ public function getParameter(string $key): mixed
         return $this->container->getParameter($key);
     }
 
+    /**
+     * @throw ParameterNotFoundException
+     *
+     * @return ?array<string>
+     */
+    public function guessParameterType(string $key): ?array
+    {
+        $parameter = $this->getParameter($key);
+
+        if (is_string($parameter) && str_starts_with($parameter, '%env(')) {
+            return $this->envParameterType($parameter);
+        }
+
+        return match (gettype($parameter)) {
+            'string' => ['string'],
+            'boolean' => ['bool'],
+            'integer' => ['int'],
+            'double' => ['float'],
+            'array' => ['array'],
+            default => null,
+        };
+    }
+
     /**
      * @return array<string>
      */
@@ -168,4 +193,17 @@ private function getDefinition(string $id): Definition
 
         return $definition;
     }
+
+    private function envParameterType(string $envParameter): ?array
+    {
+        // extract bool from %env(bool:ENV_PARAM)%, string from %env(string:ENV_PARAM)%
+        $type = preg_match('/^%env\((\w+):/', $envParameter, $matches) ? $matches[1] : null;
+
+        $envVarTypes = EnvVarProcessor::getProvidedTypes();
+        if (!isset($envVarTypes[$type])) {
+            return null;
+        }
+
+        return explode('|', $envVarTypes[$type]);
+    }
 }
diff --git a/tests/acceptance/container.xml b/tests/acceptance/container.xml
@@ -21,6 +21,12 @@
         </parameter>
       </parameter>
     </parameter>
+    <parameter key="env_param_bool">%env(bool:ENV_PARAM)%</parameter>
+    <parameter key="env_param_string">%env(string:ENV_PARAM)%</parameter>
+    <parameter key="env_param_custom_type">%env(custom_type:ENV_PARAM)%</parameter>
+    <parameter key="env_param_json">%env(json:ENV_PARAM)%</parameter>
+    <parameter key="env_param_enum">%env(enum:ENV_PARAM)%</parameter>
+    <parameter key="env_param_url">%env(url:ENV_PARAM)%</parameter>
   </parameters>
   <services>
     <service id="doctrine.orm.entity_manager" alias="doctrine.orm.default_entity_manager" public="true"/>

diff --git a/tests/unit/Symfony/ContainerMetaTest.php b/tests/unit/Symfony/ContainerMetaTest.php
@@ -117,7 +117,32 @@ public function testGetParameter(): void
         ], $this->containerMeta->getParameter('nested_collection'));
     }
 
-    public function testGetParameterP(): void
+    /**
+     * @dataProvider guessParameterTypeProvider
+     */
+    public function testGuessParameterType(?array $expectedTypes, string $parameterName): void
+    {
+        $this->assertSame($expectedTypes, $this->containerMeta->guessParameterType($parameterName));
+    }
+
+    public function guessParameterTypeProvider(): iterable
+    {
+        yield [['string'], 'kernel.environment'];
+        yield [['bool'], 'debug_enabled'];
+        yield [['string'], 'version'];
+        yield [['int'], 'integer_one'];
+        yield [['float'], 'pi'];
+        yield [['array'], 'collection1'];
+        yield [['array'], 'nested_collection'];
+        yield [['bool'], 'env_param_bool'];
+        yield [['string'], 'env_param_string'];
+        yield [null, 'env_param_custom_type'];
+        yield [['array'], 'env_param_json'];
+        yield [['BackedEnum'], 'env_param_enum'];
+        yield [['array'], 'env_param_url'];
+    }
+
+    public function testGetParameterNonExistent(): void
     {
         $this->expectException(ParameterNotFoundException::class);
         $this->containerMeta->getParameter('non_existent');