Merge pull request #138 from boesing/qa/normalize-code-issues

This normalizes the code issue generation

composer.json

@@ -5,7 +5,7 @@
     "license": "BSD-3-Clause",
     "require": {
         "php": "^7.4 || ~8.0.0 || ~8.1.0",
-        "vimeo/psalm": "^4.23 || ^5.0",
+        "vimeo/psalm": "^4.30 || ^5.0",
         "webmozart/assert": "^1.10"
     },
     "require-dev": {

psalm.xml.dist

@@ -6,19 +6,15 @@
     xmlns="https://getpsalm.org/schema/config"
     xsi:schemaLocation="https://getpsalm.org/schema/config vendor/vimeo/psalm/config.xsd"
     findUnusedPsalmSuppress="true"
+    ensureArrayStringOffsetsExist="true"
+    ensureArrayIntOffsetsExist="true"
+    ignoreInternalFunctionFalseReturn="false"
+    ignoreInternalFunctionNullReturn="false"
 >
     <projectFiles>
         <directory name="src" />
         <ignoreFiles>
             <directory name="vendor" />
         </ignoreFiles>
     </projectFiles>
-    <issueHandlers>
-        <RedundantCondition>
-            <!-- Lets keep these issues as info-level until we remove support for psalm v4 -->
-            <errorLevel type="info">
-                <directory name="src"/>
-            </errorLevel>
-        </RedundantCondition>
-    </issueHandlers>
 </psalm>

src/EventHandler/FunctionArgumentValidator.php

@@ -7,15 +7,15 @@
 use Boesing\PsalmPluginStringf\ArgumentValidator\ArgumentValidator;
 use Boesing\PsalmPluginStringf\Parser\Psalm\PhpVersion;
 use Boesing\PsalmPluginStringf\Parser\TemplatedStringParser\TemplatedStringParser;
+use Boesing\PsalmPluginStringf\Psalm\Issue\TooFewArguments;
+use Boesing\PsalmPluginStringf\Psalm\Issue\TooManyArguments;
 use InvalidArgumentException;
 use PhpParser\Node\Arg;
 use PhpParser\Node\Expr\FuncCall;
 use PhpParser\Node\VariadicPlaceholder;
 use Psalm\CodeLocation;
 use Psalm\Context;
-use Psalm\Issue\ArgumentIssue;
-use Psalm\Issue\TooFewArguments;
-use Psalm\Issue\TooManyArguments;
+use Psalm\Issue\PluginIssue;
 use Psalm\IssueBuffer;
 use Psalm\Plugin\EventHandler\AfterEveryFunctionCallAnalysisInterface;
 use Psalm\Plugin\EventHandler\Event\AfterEveryFunctionCallAnalysisEvent;
@@ -62,7 +62,7 @@ private function createCodeIssue(
         string $functionName,
         int $argumentCount,
         int $requiredArgumentCount
-    ): ArgumentIssue {
+    ): PluginIssue {
         $message = $this->createIssueMessage(
             $functionName,
             $requiredArgumentCount,
@@ -171,7 +171,7 @@ private function validate(
             return;
         }
 
-        IssueBuffer::add($this->createCodeIssue(
+        IssueBuffer::maybeAdd($this->createCodeIssue(
             $this->codeLocation,
             $functionName,
             $validationResult->actualArgumentCount,

src/EventHandler/PossiblyInvalidArgumentForSpecifierValidator.php

@@ -6,11 +6,11 @@
 
 use Boesing\PsalmPluginStringf\Parser\Psalm\PhpVersion;
 use Boesing\PsalmPluginStringf\Parser\TemplatedStringParser\TemplatedStringParser;
+use Boesing\PsalmPluginStringf\Psalm\Issue\PossiblyInvalidArgument;
 use InvalidArgumentException;
 use PhpParser\Node\Arg;
 use Psalm\CodeLocation;
 use Psalm\Context;
-use Psalm\Issue\PossiblyInvalidArgument;
 use Psalm\IssueBuffer;
 use Psalm\Plugin\EventHandler\AfterEveryFunctionCallAnalysisInterface;
 use Psalm\Plugin\EventHandler\Event\AfterEveryFunctionCallAnalysisEvent;
@@ -141,7 +141,7 @@ private function assertArgumentsMatchingPlaceholderTypes(
                 continue;
             }
 
-            IssueBuffer::add(
+            IssueBuffer::maybeAdd(
                 new PossiblyInvalidArgument(
                     sprintf(
                         'Argument %d inferred as "%s" does not match (any of) the suggested type(s) "%s"',
@@ -206,6 +206,8 @@ private function invalidTypeWouldBeCoveredByPsalmItself(Atomic $type): bool
     }
 
     /**
+     * @see \Boesing\PsalmPluginStringf\Plugin::registerFeatureHook()
+     *
      * @param array<non-empty-string,string> $options
      */
     public static function applyOptions(array $options): void

src/EventHandler/UnnecessaryFunctionCallValidator.php

@@ -116,6 +116,6 @@ private function assertFunctionCallMakesSense(
         }
 
         // TODO: find out how to provide psalter functionality
-        IssueBuffer::add(new UnnecessaryFunctionCall($codeLocation, $this->functionName), false);
+        IssueBuffer::maybeAdd(new UnnecessaryFunctionCall($codeLocation, $this->functionName));
     }
 }

src/Parser/TemplatedStringParser/TemplatedStringParser.php

@@ -118,6 +118,7 @@ static function (
         $templateWithoutPlaceholders = $template;
 
         foreach ($placeholders as $placeholder) {
+            assert(isset($placeholder[0]));
             [$placeholderValue, $placeholderIndex] = $placeholder[0];
             $placeholderLength                     = strlen($placeholderValue);
             $templateWithoutPlaceholders           = substr_replace(

src/Plugin.php

@@ -49,7 +49,6 @@ private function registerExperimentalHooks(RegistrationInterface $registration,
         }
 
         foreach ($config->experimental->children() as $element) {
-            assert($element instanceof SimpleXMLElement);
             $name = $element->getName();
             if (! isset(self::EXPERIMENTAL_FEATURES[$name])) {
                 continue;
@@ -93,7 +92,6 @@ private function extractOptionsFromElement(SimpleXMLElement $element): array
         $options = [];
 
         foreach ($element->attributes() ?? [] as $attribute) {
-            assert($attribute instanceof SimpleXMLElement);
             $name = $attribute->getName();
             assert($name !== '');
 

src/Psalm/Issue/PossiblyInvalidArgument.php

@@ -0,0 +1,24 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Boesing\PsalmPluginStringf\Psalm\Issue;
+
+use Psalm\CodeLocation;
+use Psalm\Issue\PluginIssue;
+
+use function strtolower;
+
+final class PossiblyInvalidArgument extends PluginIssue
+{
+    public ?string $function_id;
+
+    public function __construct(
+        string $message,
+        CodeLocation $code_location,
+        ?string $function_id = null
+    ) {
+        parent::__construct($message, $code_location);
+        $this->function_id = $function_id ? strtolower($function_id) : null;
+    }
+}

src/Psalm/Issue/TooFewArguments.php

@@ -0,0 +1,24 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Boesing\PsalmPluginStringf\Psalm\Issue;
+
+use Psalm\CodeLocation;
+use Psalm\Issue\PluginIssue;
+
+use function strtolower;
+
+final class TooFewArguments extends PluginIssue
+{
+    public ?string $function_id;
+
+    public function __construct(
+        string $message,
+        CodeLocation $code_location,
+        ?string $function_id = null
+    ) {
+        parent::__construct($message, $code_location);
+        $this->function_id = $function_id ? strtolower($function_id) : null;
+    }
+}

src/Psalm/Issue/TooManyArguments.php

@@ -0,0 +1,24 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Boesing\PsalmPluginStringf\Psalm\Issue;
+
+use Psalm\CodeLocation;
+use Psalm\Issue\PluginIssue;
+
+use function strtolower;
+
+final class TooManyArguments extends PluginIssue
+{
+    public ?string $function_id;
+
+    public function __construct(
+        string $message,
+        CodeLocation $code_location,
+        ?string $function_id = null
+    ) {
+        parent::__construct($message, $code_location);
+        $this->function_id = $function_id ? strtolower($function_id) : null;
+    }
+}

src/Psalm/Issue/UnnecessaryFunctionCall.php

@@ -5,16 +5,19 @@
 namespace Boesing\PsalmPluginStringf\Psalm\Issue;
 
 use Psalm\CodeLocation;
-use Psalm\Issue\FunctionIssue;
+use Psalm\Issue\PluginIssue;
 
-final class UnnecessaryFunctionCall extends FunctionIssue
+final class UnnecessaryFunctionCall extends PluginIssue
 {
+    public string $function_id;
+
     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,
         );
+
+        $this->function_id = $function_id;
     }
 }

tests/acceptance/SprintfNonEmptyString.feature

@@ -173,28 +173,13 @@ Feature: non empty template passed to sprintf results in non-empty-string
     When I run Psalm
     Then I see no errors
 
-  Scenario: template is empty and value which is passed to the string is boolean (false) on psalm v4-
-    Given I have the following code
-    """
-      /** @psalm-suppress InvalidArgument Ignore the fact that we are passing `false` to sprintf for testing purposes */
-      $string = sprintf('%s', false);
-      nonEmptyString($string);
-    """
-    And I have Psalm older than "5.0" because of "older psalm versions do not have `but` in the error message."
-    When I run Psalm
-    Then I see these errors
-      | Type  | Message |
-      | ArgumentTypeCoercion | Argument 1 of nonEmptyString expects non-empty-string, parent type string provided |
-    And I see no other errors
-
   Scenario: template is empty and value which is passed to the string is boolean (false) on psalm v5+
     Given I have the following code
     """
       /** @psalm-suppress InvalidArgument Ignore the fact that we are passing `false` to sprintf for testing purposes */
       $string = sprintf('%s', false);
       nonEmptyString($string);
     """
-    And I have Psalm newer than "4.99" because of "newer psalm versions do have `but` in the error message."
     When I run Psalm
     Then I see these errors
       | Type  | Message |
@@ -212,21 +197,6 @@ Feature: non empty template passed to sprintf results in non-empty-string
     When I run psalm
     Then I see no errors
 
-  Scenario: template gets passed float argument without knowing its value on psalm v4+
-    Given I have the following code
-    """
-      /** @psalm-var float $float */
-      $float = 0.00;
-      $string = sprintf('%0.2f', $float);
-      nonEmptyString($string);
-    """
-    And I have Psalm older than "5.0" because of "older psalm versions do not have `but` in the error message."
-    When I run psalm
-    Then I see these errors
-      | Type  | Message |
-      | ArgumentTypeCoercion | Argument 1 of nonEmptyString expects non-empty-string, parent type string provided |
-    And I see no other errors
-
   Scenario: template gets passed float argument without knowing its value on psalm v5+
     Given I have the following code
     """
@@ -235,7 +205,6 @@ Feature: non empty template passed to sprintf results in non-empty-string
       $string = sprintf('%0.2f', $float);
       nonEmptyString($string);
     """
-    And I have Psalm newer than "4.99" because of "newer psalm versions do have `but` in the error message."
     When I run psalm
     Then I see these errors
       | Type  | Message |