Find and fix code issues where functionality is available

src/Psalm/Checker/FileChecker.php

@@ -4,6 +4,7 @@ namespace Psalm\Checker;
 use PhpParser;
 use Psalm\Config;
 use Psalm\Context;
+use Psalm\FileManipulation\FileManipulationBuffer;
 use Psalm\IssueBuffer;
 use Psalm\StatementsSource;
 use Psalm\Type;
@@ -426,6 +427,7 @@ class FileChecker extends SourceChecker implements StatementsSource
         FunctionChecker::clearCache();
         StatementsChecker::clearCache();
         IssueBuffer::clearCache();
+        FileManipulationBuffer::clearCache();
         FunctionLikeChecker::clearCache();
     }
 

src/Psalm/Checker/FunctionLikeChecker.php

@@ -309,7 +309,8 @@ abstract class FunctionLikeChecker extends SourceChecker implements StatementsSo
                         new MismatchingDocblockParamType(
                             'Parameter $' . $function_param->name . ' has wrong type \'' . $param_type .
                                 '\', should be \'' . $signature_type . '\'',
-                            $function_param->type_location
+                            $function_param->type_location,
+                            (string)$signature_type
                         ),
                         $storage->suppressed_issues
                     )) {
@@ -416,7 +417,8 @@ abstract class FunctionLikeChecker extends SourceChecker implements StatementsSo
                     new MismatchingDocblockReturnType(
                         'Docblock has incorrect return type \'' . $storage->return_type .
                             '\', should be \'' . $storage->signature_return_type . '\'',
-                        $storage->return_type_location
+                        $storage->return_type_location,
+                        (string) $storage->signature_return_type
                     ),
                     $storage->suppressed_issues
                 )) {
@@ -1162,7 +1164,7 @@ abstract class FunctionLikeChecker extends SourceChecker implements StatementsSo
             )
         );
 
-        if (!$return_type && !$project_checker->update_docblocks && !$is_to_string) {
+        if (!$return_type && !$project_checker->add_docblocks && !$is_to_string) {
             if ($this->function instanceof Closure) {
                 if (IssueBuffer::accepts(
                     new MissingClosureReturnType(
@@ -1204,13 +1206,12 @@ abstract class FunctionLikeChecker extends SourceChecker implements StatementsSo
                 }
             }
 
-            if (!$project_checker->update_docblocks) {
             return null;
         }
-        }
 
         if (!$return_type) {
             if (!$inferred_return_type->isMixed()) {
+                // $project_checker->add_docblocks is always true here
                 $this->addDocblockReturnType($project_checker, $inferred_return_type);
             }
 
@@ -1284,19 +1285,12 @@ abstract class FunctionLikeChecker extends SourceChecker implements StatementsSo
                 && !$declared_return_type->isNullable()
                 && !$declared_return_type->isVoid()
             ) {
-                if ($project_checker->update_docblocks) {
-                    if (!in_array('InvalidReturnType', $this->suppressed_issues, true)) {
-                        $this->addDocblockReturnType($project_checker, $inferred_return_type);
-                    }
-
-                    return null;
-                }
-
                 if (IssueBuffer::accepts(
                     new NullableInferredReturnType(
                         'The declared return type \'' . $declared_return_type . '\' for ' . $cased_method_id .
                             ' is not nullable, but \'' . $inferred_return_type . '\' contains null',
-                        $return_type_location
+                        $return_type_location,
+                        (string) $inferred_return_type
                     ),
                     $this->suppressed_issues
                 )) {
@@ -1308,19 +1302,12 @@ abstract class FunctionLikeChecker extends SourceChecker implements StatementsSo
                 && !$declared_return_type->isFalsable()
                 && !$declared_return_type->hasBool()
             ) {
-                if ($project_checker->update_docblocks) {
-                    if (!in_array('InvalidReturnType', $this->suppressed_issues, true)) {
-                        $this->addDocblockReturnType($project_checker, $inferred_return_type);
-                    }
-
-                    return null;
-                }
-
                 if (IssueBuffer::accepts(
                     new FalsableInferredReturnType(
                         'The declared return type \'' . $declared_return_type . '\' for ' . $cased_method_id .
                             ' does not allow false, but \'' . $inferred_return_type . '\' contains false',
-                        $return_type_location
+                        $return_type_location,
+                        (string) $inferred_return_type
                     ),
                     $this->suppressed_issues
                 )) {
@@ -1338,14 +1325,6 @@ abstract class FunctionLikeChecker extends SourceChecker implements StatementsSo
                 $type_coerced,
                 $type_coerced_from_mixed
             )) {
-                if ($project_checker->update_docblocks) {
-                    if (!in_array('InvalidReturnType', $this->suppressed_issues, true)) {
-                        $this->addDocblockReturnType($project_checker, $inferred_return_type);
-                    }
-
-                    return null;
-                }
-
                 // is the declared return type more specific than the inferred one?
                 if ($type_coerced) {
                     if (IssueBuffer::accepts(
@@ -1363,7 +1342,8 @@ abstract class FunctionLikeChecker extends SourceChecker implements StatementsSo
                         new InvalidReturnType(
                             'The declared return type \'' . $declared_return_type . '\' for ' . $cased_method_id .
                                 ' is incorrect, got \'' . $inferred_return_type . '\'',
-                            $return_type_location
+                            $return_type_location,
+                            (string) $inferred_return_type
                         ),
                         $this->suppressed_issues
                     )) {
@@ -1371,14 +1351,6 @@ abstract class FunctionLikeChecker extends SourceChecker implements StatementsSo
                     }
                 }
             } elseif (!$inferred_return_type->isNullable() && $declared_return_type->isNullable()) {
-                if ($project_checker->update_docblocks) {
-                    if (!in_array('InvalidReturnType', $this->suppressed_issues, true)) {
-                        $this->addDocblockReturnType($project_checker, $inferred_return_type);
-                    }
-
-                    return null;
-                }
-
                 if (IssueBuffer::accepts(
                     new LessSpecificReturnType(
                         'The inferred return type \'' . $inferred_return_type . '\' for ' . $cased_method_id .

src/Psalm/Checker/ProjectChecker.php

@@ -87,7 +87,7 @@ class ProjectChecker
     /**
      * @var bool
      */
-    public $update_docblocks = false;
+    public $add_docblocks = false;
 
     /**
      * @var bool
@@ -255,9 +255,9 @@ class ProjectChecker
     private $replace_code = false;
 
     /**
-     * @var bool
+     * @var array<string, bool>
      */
-    private $fix_code = false;
+    private $issues_to_fix = [];
 
     const TYPE_CONSOLE = 'console';
     const TYPE_JSON = 'json';
@@ -272,7 +272,6 @@ class ProjectChecker
      * @param string        $output_format
      * @param int           $threads
      * @param bool          $debug_output
-     * @param bool          $update_docblocks
      * @param bool          $collect_references
      * @param string        $find_references_to
      * @param string        $reports
@@ -285,7 +284,6 @@ class ProjectChecker
         $output_format = self::TYPE_CONSOLE,
         $threads = 1,
         $debug_output = false,
-        $update_docblocks = false,
         $collect_references = false,
         $find_references_to = null,
         $reports = null
@@ -296,7 +294,6 @@ class ProjectChecker
         $this->show_info = $show_info;
         $this->debug_output = $debug_output;
         $this->threads = $threads;
-        $this->update_docblocks = $update_docblocks;
         $this->collect_references = $collect_references;
         $this->find_references_to = $find_references_to;
 
@@ -335,7 +332,7 @@ class ProjectChecker
     }
 
     /**
-     * @return self
+     * @return ProjectChecker
      */
     public static function getInstance()
     {
@@ -951,7 +948,7 @@ class ProjectChecker
                     echo 'Analyzing ' . $file_checker->getFilePath() . PHP_EOL;
                 }
 
-                $file_checker->analyze(null, $this->update_docblocks);
+                $file_checker->analyze(null);
             };
 
         $pool_size = $this->threads;
@@ -1007,15 +1004,7 @@ class ProjectChecker
             }
         }
 
-        if ($this->replace_code) {
+        if ($this->replace_code || $this->add_docblocks || $this->issues_to_fix) {
-            foreach ($this->files_to_report as $file_path) {
-                $this->updateFile($file_path, true);
-            }
-        } elseif ($this->update_docblocks) {
-            foreach ($this->files_to_report as $file_path) {
-                $this->updateFile($file_path, true);
-            }
-        } elseif ($this->fix_code) {
             foreach ($this->files_to_report as $file_path) {
                 $this->updateFile($file_path, true);
             }
@@ -1030,7 +1019,11 @@ class ProjectChecker
      */
     public function updateFile($file_path, $output_changes = false)
     {
+        if ($this->add_docblocks) {
             $new_return_type_manipulations = FunctionDocblockManipulator::getManipulationsForFile($file_path);
+        } else {
+            $new_return_type_manipulations = [];
+        }
 
         $other_manipulations = FileManipulationBuffer::getForFile($file_path);
 
@@ -2088,6 +2081,14 @@ class ProjectChecker
         }
     }
 
+    /**
+     * @return void
+     */
+    public function addDocblocksAfterCompletion()
+    {
+        $this->add_docblocks = true;
+    }
+
     /**
      * @return void
      */
@@ -2097,10 +2098,20 @@ class ProjectChecker
     }
 
     /**
+     * @param array<string, bool> $issues
+     *
      * @return void
      */
-    public function fixCodeAfterCompletion()
+    public function fixIssuesAfterCompletion(array $issues)
     {
-        $this->fix_code = true;
+        $this->issues_to_fix = $issues;
+    }
+
+    /**
+     * @return array<string, bool>
+     */
+    public function getIssuesToFix()
+    {
+        return $this->issues_to_fix;
     }
 }

src/Psalm/Checker/StatementsChecker.php

@@ -1023,7 +1023,7 @@ class StatementsChecker extends SourceChecker implements StatementsSource
 
                 if ($storage->return_type
                     && !$storage->return_type->isMixed()
-                    && !$project_checker->update_docblocks
+                    && !$project_checker->add_docblocks
                 ) {
                     $inferred_type = ExpressionChecker::fleshOutType(
                         $project_checker,

src/Psalm/FileManipulation/FileManipulationBuffer.php

@@ -38,4 +38,12 @@ class FileManipulationBuffer
 
         return $file_manipulations;
     }
+
+    /**
+     * @return void
+     */
+    public static function clearCache()
+    {
+        self::$file_manipulations = [];
+    }
 }

src/Psalm/Issue/CodeIssue.php

@@ -22,8 +22,10 @@ abstract class CodeIssue
      * @param string        $message
      * @param CodeLocation  $code_location
      */
-    public function __construct($message, CodeLocation $code_location)
+    public function __construct(
-    {
+        $message,
+        CodeLocation $code_location
+    ) {
         $this->code_location = $code_location;
         $this->message = $message;
     }

src/Psalm/Issue/FalsableInferredReturnType.php

@@ -1,6 +1,6 @@
 <?php
 namespace Psalm\Issue;
 
-class FalsableInferredReturnType extends CodeError
+class FalsableInferredReturnType extends FixableCodeIssue
 {
 }

src/Psalm/Issue/FixableCodeIssue.php

@@ -0,0 +1,35 @@
+<?php
+namespace Psalm\Issue;
+
+use Psalm\CodeLocation;
+use Psalm\FileManipulation\FileManipulation;
+
+abstract class FixableCodeIssue extends CodeIssue
+{
+    /**
+     * @var ?string
+     */
+    protected $replacement_text;
+
+    /**
+     * @param string        $message
+     * @param CodeLocation  $code_location
+     * @param string|null   $replacement_text
+     */
+    public function __construct(
+        $message,
+        CodeLocation $code_location,
+        $replacement_text = null
+    ) {
+        parent::__construct($message, $code_location);
+        $this->replacement_text = $replacement_text;
+    }
+
+    /**
+     * @return ?string
+     */
+    public function getReplacementText()
+    {
+        return $this->replacement_text;
+    }
+}

src/Psalm/Issue/InvalidReturnType.php

@@ -1,6 +1,6 @@
 <?php
 namespace Psalm\Issue;
 
-class InvalidReturnType extends CodeError
+class InvalidReturnType extends FixableCodeIssue
 {
 }

src/Psalm/Issue/MismatchingDocblockParamType.php

@@ -1,6 +1,6 @@
 <?php
 namespace Psalm\Issue;
 
-class MismatchingDocblockParamType extends CodeError
+class MismatchingDocblockParamType extends FixableCodeIssue
 {
 }

src/Psalm/Issue/MismatchingDocblockReturnType.php

@@ -1,6 +1,6 @@
 <?php
 namespace Psalm\Issue;
 
-class MismatchingDocblockReturnType extends CodeError
+class MismatchingDocblockReturnType extends FixableCodeIssue
 {
 }

src/Psalm/Issue/NullableInferredReturnType.php

@@ -1,6 +1,6 @@
 <?php
 namespace Psalm\Issue;
 
-class NullableInferredReturnType extends CodeError
+class NullableInferredReturnType extends FixableCodeIssue
 {
 }

src/Psalm/IssueBuffer.php

@@ -3,6 +3,8 @@ namespace Psalm;
 
 use LSS\Array2XML;
 use Psalm\Checker\ProjectChecker;
+use Psalm\FileManipulation\FileManipulation;
+use Psalm\FileManipulation\FileManipulationBuffer;
 use Psalm\Issue\CodeIssue;
 
 class IssueBuffer
@@ -49,6 +51,7 @@ class IssueBuffer
     {
         $config = Config::getInstance();
 
+
         $fqcn_parts = explode('\\', get_class($e));
         $issue_type = array_pop($fqcn_parts);
 
@@ -84,6 +87,22 @@ class IssueBuffer
         $fqcn_parts = explode('\\', get_class($e));
         $issue_type = array_pop($fqcn_parts);
 
+        $issues_to_fix = \Psalm\Checker\ProjectChecker::getInstance()->getIssuesToFix();
+
+        if (isset($issues_to_fix[$issue_type])
+            && $e instanceof \Psalm\Issue\FixableCodeIssue
+            && $replacement_text = $e->getReplacementText()
+        ) {
+            $code_location = $e->getLocation();
+            $bounds = $code_location->getSelectionBounds();
+            FileManipulationBuffer::add(
+                $e->getFilePath(),
+                [new FileManipulation($bounds[0], $bounds[1], $replacement_text)]
+            );
+
+            return false;
+        }
+
         $error_message = $issue_type . ' - ' . $e->getShortLocation() . ' - ' . $e->getMessage();
 
         $reporting_level = $config->getReportingLevelForFile($issue_type, $e->getFilePath());

src/psalm.php

@@ -367,7 +367,7 @@ $find_references_to = isset($options['find-references-to']) && is_string($option
     ? $options['find-references-to']
     : null;
 
-$update_docblocks = isset($options['update-docblocks']);
+$add_docblocks = isset($options['add-docblocks']);
 
 $threads = isset($options['threads']) ? (int)$options['threads'] : 1;
 
@@ -383,7 +383,6 @@ $project_checker = new ProjectChecker(
     $output_format,
     $threads,
     $debug,
-    $update_docblocks,
     $find_dead_code || $find_references_to !== null,
     $find_references_to,
     isset($options['report']) && is_string($options['report']) ? $options['report'] : null
@@ -429,7 +428,12 @@ if (isset($options['fix-issues'])) {
 
     $issues = explode(',', $options['fix-issues']);
 
-    $project_checker->fixIssuesAfterCompletion($issues);
+    $keyed_issues = [];
+    foreach ($issues as $issue) {
+        $keyed_issues[$issue] = true;
+    }
+
+    $project_checker->fixIssuesAfterCompletion($keyed_issues);
 }
 
 /** @psalm-suppress MixedArgument */

tests/ConfigTest.php

@@ -61,7 +61,10 @@ class ConfigTest extends TestCase
              * @return bool
              */
             function ($issue_name) {
-                return !empty($issue_name) && $issue_name !== 'CodeError' && $issue_name !== 'CodeIssue';
+                return !empty($issue_name)
+                    && $issue_name !== 'CodeError'
+                    && $issue_name !== 'CodeIssue'
+                    && $issue_name !== 'FixableCodeIssue';
             }
         );
     }

tests/FileManipulationTest.php

@@ -31,7 +31,7 @@ class FileManipulationTest extends TestCase
 
         $this->project_checker->setConfig(self::$config);
 
-        $this->project_checker->update_docblocks = true;
+        $this->project_checker->add_docblocks = true;
     }
 
     /**
@@ -65,6 +65,8 @@ class FileManipulationTest extends TestCase
         );
 
         $file_checker = new FileChecker($file_path, $this->project_checker);
+        $this->project_checker->addDocblocksAfterCompletion();
+        $this->project_checker->fixIssuesAfterCompletion(['InvalidReturnType' => true]);
         $file_checker->visitAndAnalyzeMethods($context);
         $this->project_checker->updateFile($file_path);
         $this->assertSame($output_code, $this->project_checker->getFileContents($file_path));

tests/ReportOutputTest.php

@@ -47,7 +47,6 @@ class ReportOutputTest extends TestCase
                 1,
                 false,
                 false,
-                false,
                 null,
                 '/tmp/report' . $extension
             );
@@ -70,7 +69,6 @@ class ReportOutputTest extends TestCase
             1,
             false,
             false,
-            false,
             null,
             '/tmp/report.log'
         );