Merge pull request #10431 from nicelocal/fix_unused_psalm_suppress

Emit UnusedPsalmSuppress issues for suppressed issues already removed
vimeo · Dec 3, 2023 · 62f32f4 · 62f32f4
2 parents bcb73f2 + ee5e4b8
commit 62f32f4
Show file tree

Hide file tree

Showing 4 changed files with 87 additions and 7 deletions.
diff --git a/psalm-baseline.xml b/psalm-baseline.xml
@@ -1,5 +1,5 @@
 <?xml version="1.0" encoding="UTF-8"?>
-<files psalm-version="5.x-dev@9bc98ecd47a4b83a663783c5a4744d48605d3eba">
+<files psalm-version="5.x-dev@f9f8bacdf1d1931d10c208401aa3189d1958d182">
   <file src="examples/TemplateChecker.php">
     <PossiblyUndefinedIntArrayOffset>
       <code><![CDATA[$comment_block->tags['variablesfrom'][0]]]></code>
@@ -350,6 +350,32 @@
       <code>$cs[0]</code>
     </PossiblyUndefinedIntArrayOffset>
   </file>
+  <file src="src/Psalm/Internal/PluginManager/Command/DisableCommand.php">
+    <RedundantCondition>
+      <code>$config_file_path !== null</code>
+    </RedundantCondition>
+    <ReservedWord>
+      <code><![CDATA[$input->getArgument('pluginName')]]></code>
+      <code><![CDATA[$input->getOption('config')]]></code>
+    </ReservedWord>
+  </file>
+  <file src="src/Psalm/Internal/PluginManager/Command/EnableCommand.php">
+    <RedundantCondition>
+      <code>$config_file_path !== null</code>
+    </RedundantCondition>
+    <ReservedWord>
+      <code><![CDATA[$input->getArgument('pluginName')]]></code>
+      <code><![CDATA[$input->getOption('config')]]></code>
+    </ReservedWord>
+  </file>
+  <file src="src/Psalm/Internal/PluginManager/Command/ShowCommand.php">
+    <RedundantCondition>
+      <code>$config_file_path !== null</code>
+    </RedundantCondition>
+    <ReservedWord>
+      <code><![CDATA[$input->getOption('config')]]></code>
+    </ReservedWord>
+  </file>
   <file src="src/Psalm/Internal/Provider/ReturnTypeProvider/ArrayMapReturnTypeProvider.php">
     <PossiblyUndefinedIntArrayOffset>
       <code>$callable_method_name</code>

diff --git a/psalm.xml.dist b/psalm.xml.dist
@@ -12,7 +12,7 @@
     limitMethodComplexity="true"
     errorBaseline="psalm-baseline.xml"
     findUnusedPsalmSuppress="true"
-    findUnusedBaselineEntry="true"
+    findUnusedBaselineEntry="false"
 >
     <stubs>
         <file name="stubs/phpparser.phpstub"/>

diff --git a/src/Psalm/IssueBuffer.php b/src/Psalm/IssueBuffer.php
@@ -133,6 +133,14 @@ final class IssueBuffer
      */
     public static function accepts(CodeIssue $e, array $suppressed_issues = [], bool $is_fixable = false): bool
     {
+        $config = Config::getInstance();
+        $project_analyzer = ProjectAnalyzer::getInstance();
+        $codebase = $project_analyzer->getCodebase();
+        $event = new BeforeAddIssueEvent($e, $is_fixable, $codebase);
+        if ($config->eventDispatcher->dispatchBeforeAddIssue($event) === false) {
+            return false;
+        }
+
         if (self::isSuppressed($e, $suppressed_issues)) {
             return false;
         }
@@ -258,11 +266,6 @@ public static function add(CodeIssue $e, bool $is_fixable = false): bool
         $project_analyzer = ProjectAnalyzer::getInstance();
         $codebase = $project_analyzer->getCodebase();
 
-        $event = new BeforeAddIssueEvent($e, $is_fixable, $codebase);
-        if ($config->eventDispatcher->dispatchBeforeAddIssue($event) === false) {
-            return false;
-        }
-
         $fqcn_parts = explode('\\', get_class($e));
         $issue_type = array_pop($fqcn_parts);
 

diff --git a/tests/CodebaseTest.php b/tests/CodebaseTest.php
@@ -6,6 +6,7 @@
 use PhpParser\Node\Stmt\Class_;
 use Psalm\Codebase;
 use Psalm\Context;
+use Psalm\Exception\CodeException;
 use Psalm\Exception\UnpopulatedClasslikeException;
 use Psalm\Issue\InvalidReturnStatement;
 use Psalm\Issue\InvalidReturnType;
@@ -21,6 +22,9 @@
 use function array_map;
 use function array_values;
 use function get_class;
+use function getcwd;
+
+use const DIRECTORY_SEPARATOR;
 
 class CodebaseTest extends TestCase
 {
@@ -246,4 +250,51 @@ public static function beforeAddIssue(BeforeAddIssueEvent $event): ?bool
         $this->analyzeFile('somefile.php', new Context);
         self::assertSame(0, IssueBuffer::getErrorCount());
     }
+    /**
+     * @test
+     */
+    public function addingCodeIssueIsMarkedAsRedundant(): void
+    {
+        $this->expectException(CodeException::class);
+        $this->expectExceptionMessage('UnusedPsalmSuppress');
+
+        $this->addFile(
+            (string) getcwd() . DIRECTORY_SEPARATOR . 'tests' . DIRECTORY_SEPARATOR . 'somefile.php',
+            '<?php
+                namespace Psalm\CurrentTest;
+
+                /** @psalm-suppress InvalidReturnType */
+                function invalidReturnType(int $value): string
+                {
+                    /** @psalm-suppress InvalidReturnStatement */
+                    return $value;
+                }
+                echo invalidReturnType(123);
+            ',
+        );
+        $eventHandler = new class implements BeforeAddIssueInterface
+        {
+            public static function beforeAddIssue(BeforeAddIssueEvent $event): ?bool
+            {
+                $issue = $event->getIssue();
+                if ($issue->code_location->file_path !== (string) getcwd() . DIRECTORY_SEPARATOR . 'tests' . DIRECTORY_SEPARATOR . 'somefile.php') {
+                    return null;
+                }
+                if ($issue instanceof InvalidReturnStatement && $event->isFixable() === false) {
+                    return false;
+                } elseif ($issue instanceof InvalidReturnType && $event->isFixable() === true) {
+                    return false;
+                }
+                return null;
+            }
+        };
+
+        (new PluginRegistrationSocket($this->codebase->config, $this->codebase))
+            ->registerHooksFromClass(get_class($eventHandler));
+
+        $this->analyzeFile(
+            (string) getcwd() . DIRECTORY_SEPARATOR . 'tests' . DIRECTORY_SEPARATOR . 'somefile.php',
+            new Context,
+        );
+    }
 }