From c6854cf5676ce3505e4fc0875c2aee1259f83044 Mon Sep 17 00:00:00 2001
From: asrar <aszenz@gmail.com>
Date: Sat, 21 May 2022 17:39:51 +0200
Subject: [PATCH 1/4] Adds support for fixing missing throws doc block

---
 .../Analyzer/FunctionLikeAnalyzer.php         | 17 +++++++++
 .../Internal/Analyzer/ProjectAnalyzer.php     |  1 +
 .../FunctionDocblockManipulator.php           | 21 +++++++++++
 .../FileManipulationTestCase.php              |  1 +
 .../ThrowsBlockAdditionTest.php               | 37 +++++++++++++++++++
 5 files changed, 77 insertions(+)
 create mode 100644 tests/FileManipulation/ThrowsBlockAdditionTest.php

diff --git a/src/Psalm/Internal/Analyzer/FunctionLikeAnalyzer.php b/src/Psalm/Internal/Analyzer/FunctionLikeAnalyzer.php
index 6e3d216c61e..2fea7dd9ce8 100644
--- a/src/Psalm/Internal/Analyzer/FunctionLikeAnalyzer.php
+++ b/src/Psalm/Internal/Analyzer/FunctionLikeAnalyzer.php
@@ -706,6 +706,10 @@ public function analyze(
             }
         }
 
+        /**
+         * @var list<class-string>
+         */
+        $missingThrowsDocblockErrors = [];
         foreach ($statements_analyzer->getUncaughtThrows($context) as $possibly_thrown_exception => $codelocations) {
             $is_expected = false;
 
@@ -719,6 +723,7 @@ public function analyze(
             }
 
             if (!$is_expected) {
+                $missingThrowsDocblockErrors[] = $possibly_thrown_exception;
                 foreach ($codelocations as $codelocation) {
                     // issues are suppressed in ThrowAnalyzer, CallAnalyzer, etc.
                     IssueBuffer::maybeAdd(
@@ -732,6 +737,18 @@ public function analyze(
             }
         }
 
+        if (
+            $codebase->alter_code
+            && isset($project_analyzer->getIssuesToFix()['MissingThrowsDocblock'])
+        ) {
+            $manipulator = FunctionDocblockManipulator::getForFunction(
+                $project_analyzer,
+                $this->source->getFilePath(),
+                $this->function
+            );
+            $manipulator->addThrowsDocblock($missingThrowsDocblockErrors);
+        }
+
         if ($codebase->taint_flow_graph
             && $this->function instanceof ClassMethod
             && $cased_method_id
diff --git a/src/Psalm/Internal/Analyzer/ProjectAnalyzer.php b/src/Psalm/Internal/Analyzer/ProjectAnalyzer.php
index 4e99005e19e..7275069e174 100644
--- a/src/Psalm/Internal/Analyzer/ProjectAnalyzer.php
+++ b/src/Psalm/Internal/Analyzer/ProjectAnalyzer.php
@@ -1322,6 +1322,7 @@ public function setIssuesToFix(array $issues): void
 
         $supported_issues_to_fix[] = 'MissingImmutableAnnotation';
         $supported_issues_to_fix[] = 'MissingPureAnnotation';
+        $supported_issues_to_fix[] = 'MissingThrowsDocblock';
 
         $unsupportedIssues = array_diff(array_keys($issues), $supported_issues_to_fix);
 
diff --git a/src/Psalm/Internal/FileManipulation/FunctionDocblockManipulator.php b/src/Psalm/Internal/FileManipulation/FunctionDocblockManipulator.php
index 147ba4e7ccf..d2d9da9a060 100644
--- a/src/Psalm/Internal/FileManipulation/FunctionDocblockManipulator.php
+++ b/src/Psalm/Internal/FileManipulation/FunctionDocblockManipulator.php
@@ -96,6 +96,9 @@ class FunctionDocblockManipulator
     /** @var bool */
     private $is_pure = false;
 
+    /** @var list<class-string> */
+    private $throwsExceptions = [];
+
     /**
      * @param  Closure|Function_|ClassMethod|ArrowFunction $stmt
      */
@@ -395,6 +398,16 @@ private function getDocblock(): string
             $modified_docblock = true;
             $parsed_docblock->tags['psalm-pure'] = [''];
         }
+        if (\count($this->throwsExceptions) > 0) {
+            $modified_docblock = true;
+            $parsed_docblock->tags['throws'] = [
+                \array_reduce(
+                    $this->throwsExceptions,
+                    fn(string $throwsClause, string $exception) => $throwsClause === '' ? $exception : $throwsClause.'|'.$exception,
+                    ''
+                )
+            ];
+        }
 
 
         if ($this->new_phpdoc_return_type && $this->new_phpdoc_return_type !== $old_phpdoc_return_type) {
@@ -528,6 +541,14 @@ public function makePure(): void
         $this->is_pure = true;
     }
 
+    /**
+     * @param list<class-string> $exceptions
+     */
+    public function addThrowsDocblock(array $exceptions): void
+    {
+        $this->throwsExceptions = $exceptions;
+    }
+
     public static function clearCache(): void
     {
         self::$manipulators = [];
diff --git a/tests/FileManipulation/FileManipulationTestCase.php b/tests/FileManipulation/FileManipulationTestCase.php
index 61b9d7f724d..9ff5f542e83 100644
--- a/tests/FileManipulation/FileManipulationTestCase.php
+++ b/tests/FileManipulation/FileManipulationTestCase.php
@@ -86,6 +86,7 @@ public function testValidCode(
             $safe_types
         );
         $this->project_analyzer->getCodebase()->allow_backwards_incompatible_changes = $allow_backwards_incompatible_changes;
+        $this->project_analyzer->getConfig()->check_for_throws_docblock = true;
 
         if (strpos(static::class, 'Unused') || strpos(static::class, 'Unnecessary')) {
             $this->project_analyzer->getCodebase()->reportUnusedCode();
diff --git a/tests/FileManipulation/ThrowsBlockAdditionTest.php b/tests/FileManipulation/ThrowsBlockAdditionTest.php
new file mode 100644
index 00000000000..6214c7f0908
--- /dev/null
+++ b/tests/FileManipulation/ThrowsBlockAdditionTest.php
@@ -0,0 +1,37 @@
+<?php
+
+namespace Psalm\Tests\FileManipulation;
+
+class ThrowsBlockAdditionTest extends FileManipulationTestCase
+{
+    /**
+     * @return array<string,array{string,string,string,string[],bool}>
+     */
+    public function providerValidCodeParse(): array
+    {
+        return [
+            'addThrowsAnnotationToFunction' => [
+                '<?php
+                    function foo(string $s): string {
+                        if("" === $s) {
+                            throw new \InvalidArgumentException();
+                        }
+                        return $s;
+                    }',
+                '<?php
+                    /**
+                     * @throws InvalidArgumentException
+                     */
+                    function foo(string $s): string {
+                        if("" === $s) {
+                            throw new \InvalidArgumentException();
+                        }
+                        return $s;
+                    }',
+                '7.4',
+                ['MissingThrowsDocblock'],
+                true,
+            ],
+        ];
+    }
+}

From e28831dff89ca762edd73ac68040b4f51f7e6bb5 Mon Sep 17 00:00:00 2001
From: asrar <aszenz@gmail.com>
Date: Sun, 22 May 2022 18:27:38 +0200
Subject: [PATCH 2/4] feat: fix ci + preserve existing throws

---
 .../Analyzer/FunctionLikeAnalyzer.php         |  6 ++-
 .../FunctionDocblockManipulator.php           | 23 +++++---
 .../ThrowsBlockAdditionTest.php               | 54 +++++++++++++++++++
 3 files changed, 73 insertions(+), 10 deletions(-)

diff --git a/src/Psalm/Internal/Analyzer/FunctionLikeAnalyzer.php b/src/Psalm/Internal/Analyzer/FunctionLikeAnalyzer.php
index 2fea7dd9ce8..cc5814a25e7 100644
--- a/src/Psalm/Internal/Analyzer/FunctionLikeAnalyzer.php
+++ b/src/Psalm/Internal/Analyzer/FunctionLikeAnalyzer.php
@@ -65,6 +65,8 @@
 use function array_merge;
 use function array_search;
 use function array_values;
+use function assert;
+use function class_exists;
 use function count;
 use function end;
 use function in_array;
@@ -723,6 +725,7 @@ public function analyze(
             }
 
             if (!$is_expected) {
+                assert(class_exists($possibly_thrown_exception));
                 $missingThrowsDocblockErrors[] = $possibly_thrown_exception;
                 foreach ($codelocations as $codelocation) {
                     // issues are suppressed in ThrowAnalyzer, CallAnalyzer, etc.
@@ -737,8 +740,7 @@ public function analyze(
             }
         }
 
-        if (
-            $codebase->alter_code
+        if ($codebase->alter_code
             && isset($project_analyzer->getIssuesToFix()['MissingThrowsDocblock'])
         ) {
             $manipulator = FunctionDocblockManipulator::getForFunction(
diff --git a/src/Psalm/Internal/FileManipulation/FunctionDocblockManipulator.php b/src/Psalm/Internal/FileManipulation/FunctionDocblockManipulator.php
index d2d9da9a060..bd2ab9ed6e7 100644
--- a/src/Psalm/Internal/FileManipulation/FunctionDocblockManipulator.php
+++ b/src/Psalm/Internal/FileManipulation/FunctionDocblockManipulator.php
@@ -14,7 +14,9 @@
 use Psalm\Internal\Analyzer\ProjectAnalyzer;
 use Psalm\Internal\Scanner\ParsedDocblock;
 
+use function array_key_exists;
 use function array_merge;
+use function array_reduce;
 use function count;
 use function is_string;
 use function ltrim;
@@ -398,15 +400,20 @@ private function getDocblock(): string
             $modified_docblock = true;
             $parsed_docblock->tags['psalm-pure'] = [''];
         }
-        if (\count($this->throwsExceptions) > 0) {
+        if (count($this->throwsExceptions) > 0) {
             $modified_docblock = true;
-            $parsed_docblock->tags['throws'] = [
-                \array_reduce(
-                    $this->throwsExceptions,
-                    fn(string $throwsClause, string $exception) => $throwsClause === '' ? $exception : $throwsClause.'|'.$exception,
-                    ''
-                )
-            ];
+            $inferredThrowsClause = array_reduce(
+                $this->throwsExceptions,
+                function (string $throwsClause, string $exception) {
+                    return $throwsClause === '' ? $exception : $throwsClause.'|'.$exception;
+                },
+                ''
+            );
+            if (array_key_exists('throws', $parsed_docblock->tags)) {
+                $parsed_docblock->tags['throws'][] = $inferredThrowsClause;
+            } else {
+                $parsed_docblock->tags['throws'] = [$inferredThrowsClause];
+            }
         }
 
 
diff --git a/tests/FileManipulation/ThrowsBlockAdditionTest.php b/tests/FileManipulation/ThrowsBlockAdditionTest.php
index 6214c7f0908..93a15f2ee61 100644
--- a/tests/FileManipulation/ThrowsBlockAdditionTest.php
+++ b/tests/FileManipulation/ThrowsBlockAdditionTest.php
@@ -32,6 +32,60 @@ function foo(string $s): string {
                 ['MissingThrowsDocblock'],
                 true,
             ],
+            'addMultipleThrowsAnnotationToFunction' => [
+                '<?php
+                    function foo(string $s): string {
+                        if("" === $s) {
+                            throw new \InvalidArgumentException();
+                        }
+                        if("" === \trim($s)) {
+                            throw new \DomainException();
+                        }
+                        return $s;
+                    }',
+                '<?php
+                    /**
+                     * @throws InvalidArgumentException|DomainException
+                     */
+                    function foo(string $s): string {
+                        if("" === $s) {
+                            throw new \InvalidArgumentException();
+                        }
+                        if("" === \trim($s)) {
+                            throw new \DomainException();
+                        }
+                        return $s;
+                    }',
+                '7.4',
+                ['MissingThrowsDocblock'],
+                true,
+            ],
+            'preservesExistingThrowsAnnotationToFunction' => [
+                '<?php
+                    /**
+                     * @throws InvalidArgumentException|DomainException
+                     */
+                    function foo(string $s): string {
+                        if("" === $s) {
+                            throw new \Exception();
+                        }
+                        return $s;
+                    }',
+                '<?php
+                    /**
+                     * @throws InvalidArgumentException|DomainException
+                     * @throws Exception
+                     */
+                    function foo(string $s): string {
+                        if("" === $s) {
+                            throw new \Exception();
+                        }
+                        return $s;
+                    }',
+                '7.4',
+                ['MissingThrowsDocblock'],
+                true,
+            ],
         ];
     }
 }

From f94b7abb18e955ea8530fb3aa87b6bcd3cf89a88 Mon Sep 17 00:00:00 2001
From: asrar <aszenz@gmail.com>
Date: Sun, 22 May 2022 18:38:18 +0200
Subject: [PATCH 3/4] chore: add another test

---
 .../ThrowsBlockAdditionTest.php               | 32 +++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/tests/FileManipulation/ThrowsBlockAdditionTest.php b/tests/FileManipulation/ThrowsBlockAdditionTest.php
index 93a15f2ee61..ae9eabf415d 100644
--- a/tests/FileManipulation/ThrowsBlockAdditionTest.php
+++ b/tests/FileManipulation/ThrowsBlockAdditionTest.php
@@ -86,6 +86,38 @@ function foo(string $s): string {
                 ['MissingThrowsDocblock'],
                 true,
             ],
+            'doesNotAddDuplicateThrows' => [
+                '<?php
+                    /**
+                     * @throws InvalidArgumentException
+                     */
+                    function foo(string $s): string {
+                        if("" === $s) {
+                            throw new \InvalidArgumentException();
+                        }
+                        if("" === \trim($s)) {
+                            throw new \DomainException();
+                        }
+                        return $s;
+                    }',
+                '<?php
+                    /**
+                     * @throws InvalidArgumentException
+                     * @throws DomainException
+                     */
+                    function foo(string $s): string {
+                        if("" === $s) {
+                            throw new \InvalidArgumentException();
+                        }
+                        if("" === \trim($s)) {
+                            throw new \DomainException();
+                        }
+                        return $s;
+                    }',
+                '7.4',
+                ['MissingThrowsDocblock'],
+                true,
+            ],
         ];
     }
 }

From e3f46d9a3cb3eb3932e8a48a4c9bd6156a4494ba Mon Sep 17 00:00:00 2001
From: asrar <aszenz@gmail.com>
Date: Mon, 23 May 2022 19:45:33 +0200
Subject: [PATCH 4/4] refactor: use list<string>

---
 src/Psalm/Internal/Analyzer/FunctionLikeAnalyzer.php          | 4 ----
 .../Internal/FileManipulation/FunctionDocblockManipulator.php | 4 ++--
 2 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/src/Psalm/Internal/Analyzer/FunctionLikeAnalyzer.php b/src/Psalm/Internal/Analyzer/FunctionLikeAnalyzer.php
index cc5814a25e7..faf22a70627 100644
--- a/src/Psalm/Internal/Analyzer/FunctionLikeAnalyzer.php
+++ b/src/Psalm/Internal/Analyzer/FunctionLikeAnalyzer.php
@@ -708,9 +708,6 @@ public function analyze(
             }
         }
 
-        /**
-         * @var list<class-string>
-         */
         $missingThrowsDocblockErrors = [];
         foreach ($statements_analyzer->getUncaughtThrows($context) as $possibly_thrown_exception => $codelocations) {
             $is_expected = false;
@@ -725,7 +722,6 @@ public function analyze(
             }
 
             if (!$is_expected) {
-                assert(class_exists($possibly_thrown_exception));
                 $missingThrowsDocblockErrors[] = $possibly_thrown_exception;
                 foreach ($codelocations as $codelocation) {
                     // issues are suppressed in ThrowAnalyzer, CallAnalyzer, etc.
diff --git a/src/Psalm/Internal/FileManipulation/FunctionDocblockManipulator.php b/src/Psalm/Internal/FileManipulation/FunctionDocblockManipulator.php
index bd2ab9ed6e7..ca652333ea8 100644
--- a/src/Psalm/Internal/FileManipulation/FunctionDocblockManipulator.php
+++ b/src/Psalm/Internal/FileManipulation/FunctionDocblockManipulator.php
@@ -98,7 +98,7 @@ class FunctionDocblockManipulator
     /** @var bool */
     private $is_pure = false;
 
-    /** @var list<class-string> */
+    /** @var list<string> */
     private $throwsExceptions = [];
 
     /**
@@ -549,7 +549,7 @@ public function makePure(): void
     }
 
     /**
-     * @param list<class-string> $exceptions
+     * @param list<string> $exceptions
      */
     public function addThrowsDocblock(array $exceptions): void
     {