Merge pull request #90 from boesing/feature/allow-int-for-string-plac…

…eholder Feature: introduce option to disallow integers for string placeholders
boesing · May 19, 2022 · 051c004 · 051c004
2 parents 521206d + f5a118f
commit 051c004
Show file tree

Hide file tree

Showing 12 changed files with 237 additions and 119 deletions.
diff --git a/README.md b/README.md
@@ -49,7 +49,7 @@ Experimental features can be enabled by extending the plugin configuration as fo
 ```xml
 <pluginClass class="Boesing\PsalmPluginStringf\Plugin">
     <experimental>
-        <ReportPossiblyInvalidArgumentForSpecifier/>
+        <ReportPossiblyInvalidArgumentForSpecifier allowIntegerForString="yes" />
     </experimental>
 </pluginClass>
 ```

diff --git a/composer.lock b/composer.lock
diff --git a/src/EventHandler/PossiblyInvalidArgumentForSpecifierValidator.php b/src/EventHandler/PossiblyInvalidArgumentForSpecifierValidator.php
@@ -26,6 +26,8 @@
 
 final class PossiblyInvalidArgumentForSpecifierValidator implements AfterEveryFunctionCallAnalysisInterface
 {
+    public static bool $allowIntegerForStringPlaceholder = true;
+
     private const FUNCTIONS = [
         'sprintf',
         'printf',
@@ -95,7 +97,8 @@ public function assert(
                 $this->functionName,
                 $template,
                 $context,
-                $phpVersion
+                $phpVersion,
+                self::$allowIntegerForStringPlaceholder
             );
         } catch (InvalidArgumentException $exception) {
             return;
@@ -196,4 +199,16 @@ private function invalidTypeWouldBeCoveredByPsalmItself(Atomic $type): bool
 
         return true;
     }
+
+    /**
+     * @param array<non-empty-string,string> $options
+     */
+    public static function applyOptions(array $options): void
+    {
+        if (! isset($options['allowIntegerForString']) || $options['allowIntegerForString'] === 'yes') {
+            return;
+        }
+
+        self::$allowIntegerForStringPlaceholder = false;
+    }
 }
diff --git a/src/EventHandler/SprintfFunctionReturnProvider.php b/src/EventHandler/SprintfFunctionReturnProvider.php
@@ -44,7 +44,8 @@ public static function getFunctionReturnType(FunctionReturnTypeProviderEvent $ev
                 $functionName,
                 $templateArgument,
                 $context,
-                PhpVersion::fromStatementSource($event->getStatementsSource())
+                PhpVersion::fromStatementSource($event->getStatementsSource()),
+                false
             );
         } catch (InvalidArgumentException $exception) {
             return null;

diff --git a/src/EventHandler/StringfFunctionArgumentValidator.php b/src/EventHandler/StringfFunctionArgumentValidator.php
@@ -135,7 +135,8 @@ private function validate(
                 $this->functionName,
                 $template,
                 $context,
-                $phpVersion
+                $phpVersion,
+                false
             );
         } catch (InvalidArgumentException $exception) {
             return;

diff --git a/src/EventHandler/UnnecessaryFunctionCallValidator.php b/src/EventHandler/UnnecessaryFunctionCallValidator.php
@@ -89,7 +89,8 @@ private function assert(
                 $this->functionName,
                 $template,
                 $context,
-                $phpVersion
+                $phpVersion,
+                false
             );
         } catch (InvalidArgumentException $exception) {
             return;

diff --git a/src/Parser/TemplatedStringParser/Placeholder.php b/src/Parser/TemplatedStringParser/Placeholder.php
@@ -27,25 +27,28 @@ final class Placeholder
 
     private ?Union $type;
 
+    private bool $allowIntegerForStringPlaceholder;
+
     /**
      * @psalm-param non-empty-string $value
      * @psalm-param positive-int $position
      */
-    private function __construct(string $value, int $position)
+    private function __construct(string $value, int $position, bool $allowIntegerForStringPlaceholder)
     {
-        $this->value             = $value;
-        $this->position          = $position;
-        $this->argumentValueType = null;
-        $this->type              = null;
+        $this->value                            = $value;
+        $this->position                         = $position;
+        $this->argumentValueType                = null;
+        $this->type                             = null;
+        $this->allowIntegerForStringPlaceholder = $allowIntegerForStringPlaceholder;
     }
 
     /**
      * @psalm-param non-empty-string $value
      * @psalm-param positive-int $position
      */
-    public static function create(string $value, int $position): self
+    public static function create(string $value, int $position, bool $allowIntegerForStringPlaceholder): self
     {
-        return new self($value, $position);
+        return new self($value, $position, $allowIntegerForStringPlaceholder);
     }
 
     /**
@@ -139,13 +142,13 @@ public function getSuggestedType(): ?Union
         }
 
         try {
-            $type = SpecifierTypeGenerator::create($this->value)->getSuggestedType();
+            $type = SpecifierTypeGenerator::create($this->value, $this->allowIntegerForStringPlaceholder)->getSuggestedType();
         } catch (InvalidArgumentException $exception) {
             return null;
         }
 
         if ($this->repeated === []) {
-            return $type;
+            return $this->type = $type;
         }
 
         $unions = [$type];

diff --git a/src/Parser/TemplatedStringParser/SpecifierTypeGenerator.php b/src/Parser/TemplatedStringParser/SpecifierTypeGenerator.php
@@ -20,27 +20,30 @@ final class SpecifierTypeGenerator
     /** @psalm-var non-empty-string */
     private string $specifier;
 
+    private bool $allowIntegerForStringPlaceholder;
+
     /**
      * @psalm-param non-empty-string $specifier
      */
-    private function __construct(string $specifier)
+    private function __construct(string $specifier, bool $allowIntegerForStringPlaceholder)
     {
-        $this->specifier = $this->parse($specifier);
+        $this->specifier                        = $this->parse($specifier);
+        $this->allowIntegerForStringPlaceholder = $allowIntegerForStringPlaceholder;
     }
 
     /**
      * @psalm-param non-empty-string $specifier
      */
-    public static function create(string $specifier): self
+    public static function create(string $specifier, bool $allowIntegerForStringPlaceholder): self
     {
-        return new self($specifier);
+        return new self($specifier, $allowIntegerForStringPlaceholder);
     }
 
     public function getSuggestedType(): Type\Union
     {
         switch ($this->specifier) {
             case 's':
-                return Type::getString();
+                return $this->stringable();
 
             case 'd':
             case 'f':
@@ -80,4 +83,17 @@ private function numeric(): Type\Union
             new Type\Atomic\TNumericString(),
         ]);
     }
+
+    private function stringable(): Type\Union
+    {
+        $types = [
+            new Type\Atomic\TString(),
+        ];
+
+        if ($this->allowIntegerForStringPlaceholder) {
+            $types[] = new Type\Atomic\TInt();
+        }
+
+        return new Type\Union($types);
+    }
 }
diff --git a/src/Parser/TemplatedStringParser/TemplatedStringParser.php b/src/Parser/TemplatedStringParser/TemplatedStringParser.php
@@ -42,24 +42,29 @@ final class TemplatedStringParser
 
     private string $template;
 
+    private bool $allowIntegerForStringPlaceholder;
+
     /**
      * @psalm-param positive-int $phpVersion
      */
     private function __construct(
         string $functionName,
         string $template,
-        int $phpVersion
+        int $phpVersion,
+        bool $allowIntegerForStringPlaceholder
     ) {
         $this->template                   = $template;
         $this->templateWithoutPlaceholder = $template;
         $this->placeholders               = [];
-        $this->parse($functionName, $template, $phpVersion);
+        $this->parse($functionName, $template, $phpVersion, $allowIntegerForStringPlaceholder);
+        $this->allowIntegerForStringPlaceholder = $allowIntegerForStringPlaceholder;
     }
 
     private function parse(
         string $functionName,
         string $template,
-        int $phpVersion
+        int $phpVersion,
+        bool $allowIntegerForStringPlaceholder
     ): void {
         $additionalSpecifierDependingOnPhpVersion = '';
         if ($phpVersion >= 80000) {
@@ -133,7 +138,8 @@ static function (
             $initialPlaceholderInstance = $placeholderInstances[$placeholderPosition] ?? null;
             $placeholderInstance        = Placeholder::create(
                 $placeholderValue,
-                $placeholderPosition
+                $placeholderPosition,
+                $allowIntegerForStringPlaceholder
             );
 
             if ($initialPlaceholderInstance !== null) {
@@ -153,12 +159,14 @@ public static function fromArgument(
         string $functionName,
         Arg $templateArgument,
         Context $context,
-        int $phpVersion
+        int $phpVersion,
+        bool $allowIntegerForStringPlaceholder
     ): self {
         return new self(
             $functionName,
             ArgumentValueParser::create($templateArgument->value, $context)->toString(),
-            $phpVersion
+            $phpVersion,
+            $allowIntegerForStringPlaceholder
         );
     }
 

diff --git a/src/Plugin.php b/src/Plugin.php
@@ -46,18 +46,25 @@ private function registerExperimentalHooks(RegistrationInterface $registration,
         }
 
         foreach ($config->experimental->children() as $element) {
-            assert($element !== null);
+            assert($element instanceof SimpleXMLElement);
             $name = $element->getName();
             if (! isset(self::EXPERIMENTAL_FEATURES[$name])) {
                 continue;
             }
 
-            $this->registerFeatureHook($registration, $name);
+            $options = $this->extractOptionsFromElement($element);
+            $this->registerFeatureHook($registration, $name, $options);
         }
     }
 
-    private function registerFeatureHook(RegistrationInterface $registration, string $featureName): void
-    {
+    /**
+     * @param array<non-empty-string,string> $options
+     */
+    private function registerFeatureHook(
+        RegistrationInterface $registration,
+        string $featureName,
+        array $options
+    ): void {
         $eventHandlerClassName = self::EXPERIMENTAL_FEATURES[$featureName];
 
         $fileName =  __DIR__ . sprintf(
@@ -68,5 +75,28 @@ private function registerFeatureHook(RegistrationInterface $registration, string
         require_once $fileName;
 
         $registration->registerHooksFromClass($eventHandlerClassName);
+        if ($eventHandlerClassName !== PossiblyInvalidArgumentForSpecifierValidator::class) {
+            return;
+        }
+
+        $eventHandlerClassName::applyOptions($options);
+    }
+
+    /**
+     * @return array<non-empty-string,string>
+     */
+    private function extractOptionsFromElement(SimpleXMLElement $element): array
+    {
+        $options = [];
+
+        foreach ($element->attributes() ?? [] as $attribute) {
+            assert($attribute instanceof SimpleXMLElement);
+            $name = $attribute->getName();
+            assert($name !== '');
+
+            $options[$name] = (string) $attribute;
+        }
+
+        return $options;
     }
 }
diff --git a/tests/acceptance/PrintfArgumentType.feature b/tests/acceptance/PrintfArgumentType.feature
@@ -39,8 +39,7 @@ Feature: printf argument type verification
     When I run Psalm
     Then I see these errors
       | Type  | Message |
-      | PossiblyInvalidArgument | Argument 1 inferred as "1" does not match (any of) the suggested type(s) "string" |
-      | PossiblyInvalidArgument | Argument 1 inferred as "float(1.035)" does not match (any of) the suggested type(s) "string" |
+      | PossiblyInvalidArgument | Argument 1 inferred as "float(1.035)" does not match (any of) the suggested type(s) "int\|string" |
     And I see no other errors
 
   Scenario: template requires double but invalid values are passed

diff --git a/...ptance/PrintfInvalidArgumentForSpecifierWithDisallowedIntegerForStringPlaceholder.feature b/...ptance/PrintfInvalidArgumentForSpecifierWithDisallowedIntegerForStringPlaceholder.feature
@@ -0,0 +1,44 @@
+Feature: Integer is not allowed to be passed to printf string placeholder when option is set
+
+  Background:
+    Given I have the following config
+    """
+    <?xml version="1.0"?>
+    <psalm errorLevel="1" findUnusedPsalmSuppress="true">
+      <projectFiles>
+        <directory name="."/>
+      </projectFiles>
+      <issueHandlers>
+        <UnusedFunctionCall>
+          <errorLevel type="suppress">
+            <directory name="."/>
+          </errorLevel>
+        </UnusedFunctionCall>
+      </issueHandlers>
+      <plugins>
+        <pluginClass class="Boesing\PsalmPluginStringf\Plugin">
+          <experimental>
+           <ReportPossiblyInvalidArgumentForSpecifier allowIntegerForString="no"/>
+          </experimental>
+        </pluginClass>
+      </plugins>
+    </psalm>
+    """
+    And I have the following code preamble
+    """
+    <?php declare(strict_types=1);
+
+    """
+
+  Scenario: template requires string but integer is passed
+    Given I have the following code
+    """
+      printf('%s', 1);
+      sprintf('%s', 1);
+    """
+    When I run Psalm
+    Then I see these errors
+      | Type  | Message |
+      | PossiblyInvalidArgument | Argument 1 inferred as "1" does not match (any of) the suggested type(s) "string" |
+      | PossiblyInvalidArgument | Argument 1 inferred as "1" does not match (any of) the suggested type(s) "string" |
+    And I see no other errors