Improve \Psalm\Internal\Scanner\DocblockParser::parse() (#3736)

This change avoids calling `str_replace()` on the original docblock and
instead only operates on the parsed (and modified) lines. This now makes
it so that if there are substrings of the docblock that match a tag
match, it won't get prematurely removed, therefore avoiding mangling of
the parsed docblock's description.

Fixes: #3735

src/Psalm/Internal/Scanner/DocblockParser.php

@@ -2,18 +2,21 @@
 
 namespace Psalm\Internal\Scanner;
 
-use function trim;
-use function preg_replace;
-use function explode;
-use function preg_match;
-use function strlen;
-use function str_replace;
 use const PREG_OFFSET_CAPTURE;
-use function strpos;
-use function rtrim;
 use function array_filter;
+use function explode;
+use function implode;
 use function min;
+use function preg_match;
+use function preg_replace;
+use function rtrim;
 use function str_repeat;
+use function str_replace;
+use function strlen;
+use function strpos;
+use function strspn;
+use function substr;
+use function trim;
 
 class DocblockParser
 {
@@ -46,16 +49,15 @@ class DocblockParser
 
         $line_offset = 0;
 
-        foreach ($lines as $line) {
+        foreach ($lines as $k => $line) {
             $original_line_length = strlen($line);
 
             $line = str_replace("\r", '', $line);
 
             if (preg_match('/^[ \t]*\*?\s*@([\w\-:]+)[\t ]*(.*)$/sm', $line, $matches, PREG_OFFSET_CAPTURE)) {
                 /** @var array<int, array{string, int}> $matches */
-                list($full_match_info, $type_info, $data_info) = $matches;
+                list($_, $type_info, $data_info) = $matches;
 
-                list($full_match) = $full_match_info;
                 list($type) = $type_info;
                 list($data, $data_offset) = $data_info;
 
@@ -63,8 +65,6 @@ class DocblockParser
                     $data = rtrim(preg_replace('/^[ \t]*\*\s*$/m', '', $data));
                 }
 
-                $docblock = str_replace($full_match, '', $docblock);
-
                 if (empty($special[$type])) {
                     $special[$type] = [];
                 }
@@ -72,28 +72,37 @@ class DocblockParser
                 $data_offset += $line_offset;
 
                 $special[$type][$data_offset + 3] = $data;
+
+                unset($lines[$k]);
+            } else {
+                // Strip the leading *, if present.
+                $lines[$k] = str_replace("\t", ' ', $line);
+                $lines[$k] = preg_replace('/^ *\*/', '', $line);
             }
 
             $line_offset += $original_line_length + 1;
         }
 
-        $docblock = str_replace("\t", '  ', $docblock);
-
         // Smush the whole docblock to the left edge.
         $min_indent = 80;
-        $indent = 0;
-        foreach (array_filter(explode("\n", $docblock)) as $line) {
-            for ($ii = 0; $ii < strlen($line); ++$ii) {
-                if ($line[$ii] != ' ') {
-                    break;
-                }
-                ++$indent;
+        foreach ($lines as $k => $line) {
+            $indent = strspn($line, ' ');
+            if ($indent == strlen($line)) {
+                // This line consists of only spaces. Trim it completely.
+                $lines[$k] = '';
+                continue;
             }
-
             $min_indent = min($indent, $min_indent);
         }
-
-        $docblock = preg_replace('/^' . str_repeat(' ', $min_indent) . '/m', '', $docblock);
+        if ($min_indent > 0) {
+            foreach ($lines as $k => $line) {
+                if (strlen($line) < $min_indent) {
+                    continue;
+                }
+                $lines[$k] = substr($line, $min_indent);
+            }
+        }
+        $docblock = implode("\n", $lines);
         $docblock = rtrim($docblock);
 
         // Trim any empty lines off the front, but leave the indent level if there

src/Psalm/Internal/Scanner/ParsedDocblock.php

@@ -38,11 +38,15 @@ class ParsedDocblock
             $description_lines = explode("\n", $this->description);
 
             foreach ($description_lines as $line) {
-                $doc_comment_text .= $left_padding . (trim($line) ? ' ' . $line : '') . "\n";
+                $doc_comment_text .= $left_padding . ' *' . (trim($line) ? ' ' . $line : '') . "\n";
             }
         }
 
         if ($this->tags) {
+            if (!empty($trimmed_description)) {
+                $doc_comment_text .= $left_padding . ' *' . "\n";
+            }
+
             $last_type = null;
 
             foreach ($this->tags as $type => $lines) {

tests/DocCommentTest.php

@@ -10,7 +10,7 @@ class DocCommentTest extends BaseTestCase
     public function testNewLineIsAddedBetweenAnnotationsByDefault(): void
     {
         $docComment = new ParsedDocblock(
-            '* some desc' . "\n*",
+            'some desc',
             [
                 'param' =>
                     [
@@ -48,7 +48,7 @@ class DocCommentTest extends BaseTestCase
         ParsedDocblock::addNewLineBetweenAnnotations(false);
 
         $docComment = new ParsedDocblock(
-            '* some desc' . "\n*",
+            'some desc',
             [
                 'param' =>
                     [
@@ -84,7 +84,7 @@ class DocCommentTest extends BaseTestCase
         ParsedDocblock::addNewLineBetweenAnnotations(true);
 
         $docComment = new ParsedDocblock(
-            '* some desc' . "\n*",
+            'some desc',
             [
                 'param' =>
                     [
@@ -116,4 +116,72 @@ class DocCommentTest extends BaseTestCase
 
         $this->assertSame($expectedDoc, $docComment->render(''));
     }
+
+    public function testParsingRoundtrip(): void
+    {
+        ParsedDocblock::addNewLineBetweenAnnotations(true);
+
+        $expectedDoc = '/**
+ * some desc
+ *
+ * @param string $bli
+ * @param int $bla
+ *
+ * @throws \Exception
+ *
+ * @return bool
+ */
+';
+        $docComment = DocComment::parsePreservingLength(
+            new \PhpParser\Comment\Doc($expectedDoc)
+        );
+
+        $this->assertSame($expectedDoc, $docComment->render(''));
+    }
+
+    public function testParsingWithIndentation(): void
+    {
+        ParsedDocblock::addNewLineBetweenAnnotations(true);
+
+        $expectedDoc = '/**
+     * some desc
+     *
+     * @param string $bli
+     * @param int $bla
+     *
+     * @throws \Exception
+     *
+     * @return bool
+     */
+    ';
+        $docComment = DocComment::parsePreservingLength(
+            new \PhpParser\Comment\Doc($expectedDoc)
+        );
+
+        $this->assertSame($expectedDoc, $docComment->render('    '));
+    }
+
+    public function testParsingWithCommonPrefixes(): void
+    {
+        ParsedDocblock::addNewLineBetweenAnnotations(true);
+
+        $expectedDoc = '/**
+ * some self-referential desc with " * @return bool
+ * " as part of it.
+ *
+ * @param string $bli
+ * @param string $bli_this_suffix_is_kept
+ * @param int $bla
+ *
+ * @throws \Exception
+ *
+ * @return bool
+ */
+';
+        $docComment = DocComment::parsePreservingLength(
+            new \PhpParser\Comment\Doc($expectedDoc)
+        );
+
+        $this->assertSame($expectedDoc, $docComment->render(''));
+    }
 }