Merge pull request #21 from boesing/feature/unnecessary-function-call

Feature: unnecessary function call

.gitignore

@@ -1,6 +1,8 @@
 /vendor/
 /psalm.xml
 /phpcs.xml
-/tests/**/*
+/tests/_output
+/tests/_run
+/tests/_support/_generated
 !/tests/_support
 !/tests/acceptance

README.md

@@ -23,6 +23,7 @@ vendor/bin/psalm-plugin enable boesing/psalm-plugin-stringf
 - Parses `sprintf` and `printf` arguments to verify if the number of passed arguments matches the amount of specifiers
 - Verifies if the return value of `sprintf` might be a `non-empty-string`
 - Verifies possibly invalid argument of `sprintf` and `printf` ([experimental](#report-possibly-invalid-argument-for-specifier))
+- Verifies unnecessary function calls of `sprintf` and `printf` ([experimental](#report-unnecessary-function-calls))
 
 ## Experimental
 
@@ -73,6 +74,27 @@ printf('%d', 'foo');
 PossiblyInvalidArgument: Argument 1 inferred as "string" does not match (any of) the suggested type(s) "float\|int\|numeric-string"
 ```
 
+### Report Unnecessary Function Calls
+
+```xml
+<pluginClass class="Boesing\PsalmPluginStringf\Plugin">
+    <experimental>
+        <ReportUnnecessaryFunctionCalls/>
+    </experimental>
+</pluginClass>
+```
+
+The `ReportUnnecessaryFunctionCalls` experimental feature will report `UnnecessaryFunctionCall` errors for
+function calls to `sprintf` or `printf` which can be omitted. Here are some examples:
+
+```php
+printf('Some text without any placeholder');
+sprintf('Some text without any placeholder');
+```
+
+```
+UnnecessaryFunctionCall: Function call is unnecessary as there is no placeholder within the template.
+```
 
 
 ## Release Versioning Disclaimer

phpcs.xml.dist

@@ -17,6 +17,7 @@
         <exclude name="Squiz.NamingConventions.ValidVariableName.MemberNotCamelCaps"/>
         <exclude name="SlevomatCodingStandard.ControlStructures.UselessIfConditionWithReturn.UselessIfCondition"/>
         <exclude name="Generic.Files.LineLength.TooLong"/>
+        <exclude name="Squiz.NamingConventions.ValidVariableName.NotCamelCaps"/>
     </rule>
 
     <file>src</file>

src/EventHandler/PossiblyInvalidArgumentForSpecifierValidator.php

@@ -8,7 +8,6 @@
 use Boesing\PsalmPluginStringf\Parser\TemplatedStringParser\TemplatedStringParser;
 use InvalidArgumentException;
 use PhpParser\Node\Arg;
-use PhpParser\Node\Expr\FuncCall;
 use Psalm\CodeLocation;
 use Psalm\Context;
 use Psalm\Issue\PossiblyInvalidArgument;
@@ -38,18 +37,14 @@ final class PossiblyInvalidArgumentForSpecifierValidator implements AfterEveryFu
     /** @psalm-var non-empty-list<Arg> */
     private array $arguments;
 
-    private FuncCall $functionCall;
-
     /**
      * @param non-empty-string    $functionName
      * @param non-empty-list<Arg> $arguments
      */
     public function __construct(
-        FuncCall $functionCall,
         string $functionName,
         array $arguments
     ) {
-        $this->functionCall = $functionCall;
         $this->functionName = $functionName;
         $this->arguments    = $arguments;
     }
@@ -74,7 +69,6 @@ public static function afterEveryFunctionCallAnalysis(AfterEveryFunctionCallAnal
         $context = $event->getContext();
 
         (new self(
-            $expression,
             $functionId,
             $arguments
         ))->assert(

src/EventHandler/UnnecessaryFunctionCallValidator.php

@@ -0,0 +1,113 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Boesing\PsalmPluginStringf\EventHandler;
+
+use Boesing\PsalmPluginStringf\Parser\Psalm\PhpVersion;
+use Boesing\PsalmPluginStringf\Parser\TemplatedStringParser\TemplatedStringParser;
+use Boesing\PsalmPluginStringf\Psalm\Issue\UnnecessaryFunctionCall;
+use InvalidArgumentException;
+use PhpParser\Node\Arg;
+use Psalm\CodeLocation;
+use Psalm\Context;
+use Psalm\IssueBuffer;
+use Psalm\Plugin\EventHandler\AfterEveryFunctionCallAnalysisInterface;
+use Psalm\Plugin\EventHandler\Event\AfterEveryFunctionCallAnalysisEvent;
+use Webmozart\Assert\Assert;
+
+use function in_array;
+
+final class UnnecessaryFunctionCallValidator implements AfterEveryFunctionCallAnalysisInterface
+{
+    private const FUNCTIONS = [
+        'sprintf',
+        'printf',
+    ];
+
+    /** @psalm-var non-empty-string */
+    private string $functionName;
+
+    /** @psalm-var non-empty-list<Arg> */
+    private array $arguments;
+
+    /**
+     * @param non-empty-string    $functionName
+     * @param non-empty-list<Arg> $arguments
+     */
+    public function __construct(
+        string $functionName,
+        array $arguments
+    ) {
+        $this->functionName = $functionName;
+        $this->arguments    = $arguments;
+    }
+
+    public static function afterEveryFunctionCallAnalysis(AfterEveryFunctionCallAnalysisEvent $event): void
+    {
+        $functionId = $event->getFunctionId();
+        if (! in_array($functionId, self::FUNCTIONS, true)) {
+            return;
+        }
+
+        $expression = $event->getExpr();
+        $arguments  = $expression->args;
+
+        $template = $arguments[0] ?? null;
+        if ($template === null) {
+            return;
+        }
+
+        Assert::isNonEmptyList($arguments);
+
+        $context = $event->getContext();
+
+        (new self(
+            $functionId,
+            $arguments
+        ))->assert(
+            new CodeLocation($event->getStatementsSource(), $expression),
+            $context,
+            PhpVersion::fromCodebase($event->getCodebase())
+        );
+    }
+
+    /**
+     * @psalm-param positive-int $phpVersion
+     */
+    private function assert(
+        CodeLocation $codeLocation,
+        Context $context,
+        int $phpVersion
+    ): void {
+        $template = $this->arguments[0];
+
+        try {
+            $parsed = TemplatedStringParser::fromArgument(
+                $this->functionName,
+                $template,
+                $context,
+                $phpVersion
+            );
+        } catch (InvalidArgumentException $exception) {
+            return;
+        }
+
+        $this->assertFunctionCallMakesSense(
+            $codeLocation,
+            $parsed
+        );
+    }
+
+    private function assertFunctionCallMakesSense(
+        CodeLocation $codeLocation,
+        TemplatedStringParser $parsed
+    ): void {
+        if ($parsed->getTemplate() !== $parsed->getTemplateWithoutPlaceholder()) {
+            return;
+        }
+
+        // TODO: find out how to provide psalter functionality
+        IssueBuffer::add(new UnnecessaryFunctionCall($codeLocation, $this->functionName), false);
+    }
+}

src/Plugin.php

@@ -7,6 +7,7 @@
 use Boesing\PsalmPluginStringf\EventHandler\PossiblyInvalidArgumentForSpecifierValidator;
 use Boesing\PsalmPluginStringf\EventHandler\SprintfFunctionReturnProvider;
 use Boesing\PsalmPluginStringf\EventHandler\StringfFunctionArgumentValidator;
+use Boesing\PsalmPluginStringf\EventHandler\UnnecessaryFunctionCallValidator;
 use Psalm\Plugin\PluginEntryPointInterface;
 use Psalm\Plugin\RegistrationInterface;
 use SimpleXMLElement;
@@ -21,6 +22,7 @@ final class Plugin implements PluginEntryPointInterface
 {
     private const EXPERIMENTAL_FEATURES = [
         'ReportPossiblyInvalidArgumentForSpecifier' => PossiblyInvalidArgumentForSpecifierValidator::class,
+        'ReportUnnecessaryFunctionCalls' => UnnecessaryFunctionCallValidator::class,
     ];
 
     public function __invoke(RegistrationInterface $registration, ?SimpleXMLElement $config = null): void

src/Psalm/Issue/UnnecessaryFunctionCall.php

@@ -0,0 +1,20 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Boesing\PsalmPluginStringf\Psalm\Issue;
+
+use Psalm\CodeLocation;
+use Psalm\Issue\FunctionIssue;
+
+final class UnnecessaryFunctionCall extends FunctionIssue
+{
+    public function __construct(CodeLocation $code_location, string $function_id)
+    {
+        parent::__construct(
+            'Function call is unnecessary as there is no placeholder within the template.',
+            $code_location,
+            $function_id
+        );
+    }
+}

tests/acceptance/PrintfUnnecessaryFunctionCall.feature

@@ -0,0 +1,44 @@
+Feature: unnecessary printf function call
+
+  Background:
+    Given I have the following config
+    """
+    <?xml version="1.0"?>
+    <psalm errorLevel="1" findUnusedPsalmSuppress="true">
+      <projectFiles>
+        <directory name="."/>
+      </projectFiles>
+      <plugins>
+        <pluginClass class="Boesing\PsalmPluginStringf\Plugin">
+          <experimental>
+           <ReportUnnecessaryFunctionCalls/>
+          </experimental>
+        </pluginClass>
+      </plugins>
+    </psalm>
+    """
+    And I have the following code preamble
+    """
+    <?php declare(strict_types=1);
+
+    """
+
+  Scenario: template is empty
+    Given I have the following code
+    """
+      printf('');
+    """
+    When I run Psalm
+    Then I see these errors
+      | Type  | Message |
+      | UnnecessaryFunctionCall | Function call is unnecessary as there is no placeholder within the template |
+
+  Scenario: template contains no identifier
+    Given I have the following code
+    """
+      printf('Foo bar baz');
+    """
+    When I run Psalm
+    Then I see these errors
+      | Type  | Message |
+      | UnnecessaryFunctionCall | Function call is unnecessary as there is no placeholder within the template |

tests/acceptance/SprintfUnnecessaryFunctionCall.feature

@@ -0,0 +1,44 @@
+Feature: unnecessary sprintf function call
+
+  Background:
+    Given I have the following config
+    """
+    <?xml version="1.0"?>
+    <psalm errorLevel="1" findUnusedPsalmSuppress="true">
+      <projectFiles>
+        <directory name="."/>
+      </projectFiles>
+      <plugins>
+        <pluginClass class="Boesing\PsalmPluginStringf\Plugin">
+          <experimental>
+           <ReportUnnecessaryFunctionCalls/>
+          </experimental>
+        </pluginClass>
+      </plugins>
+    </psalm>
+    """
+    And I have the following code preamble
+    """
+    <?php declare(strict_types=1);
+
+    """
+
+  Scenario: template is empty
+    Given I have the following code
+    """
+      sprintf('');
+    """
+    When I run Psalm
+    Then I see these errors
+      | Type  | Message |
+      | UnnecessaryFunctionCall | Function call is unnecessary as there is no placeholder within the template |
+
+  Scenario: template contains no identifier
+    Given I have the following code
+    """
+      sprintf('Foo bar baz');
+    """
+    When I run Psalm
+    Then I see these errors
+      | Type  | Message |
+      | UnnecessaryFunctionCall | Function call is unnecessary as there is no placeholder within the template |