fix: Fix support for comments (#49)

Closes #47.
theofidry · Jul 24, 2024 · 706e960 · 706e960
1 parent be5a048
commit 706e960
Show file tree

Hide file tree

Showing 4 changed files with 154 additions and 9 deletions.
diff --git a/infection.json5.dist b/infection.json5.dist
@@ -30,6 +30,11 @@
                 ".+\\$charactersToTrim = \\$multiline \\?.+",
             ]
         },
+        "FalseValue": {
+            "ignore": [
+                "Fidry\\Makefile\\Parser::parsePrerequisites"
+            ]
+        },
         "FunctionCallRemoval": {
             "ignore": [
                 "Fidry\\Makefile\\Test\\BaseMakefileTestCase::executeCommand",
@@ -47,6 +52,11 @@
             ]
         },
         "PublicVisibility": false,
+        "SpreadOneItem": {
+            "ignore": [
+                "Fidry\\Makefile\\Parser::parsePrerequisites"
+            ]
+        },
         "UnwrapArrayValues": {
             "ignore": [
                 "Fidry\\Makefile\\Parser::parseLine"

diff --git a/src/Parser.php b/src/Parser.php
@@ -49,11 +49,13 @@
 use function str_contains;
 use function str_ends_with;
 use function str_starts_with;
-use function trim;
 use const PHP_EOL;
 
 final class Parser
 {
+    private const COMMENT_LINE = '/(?<nonCommentPart>.*?)(?<commentPart>#.*)/';
+    private const MULTILINE_DELIMITER = '\\';
+
     /**
      * @return list<Rule>
      */
@@ -95,7 +97,7 @@ private static function parseLine(
         }
 
         $previousMultiline = $multiline;
-        $multiline = str_ends_with($line, '\\');
+        $multiline = self::isMultiline($line);
 
         if (false === $previousMultiline) {
             $targetParts = explode(':', $line);
@@ -143,12 +145,33 @@ private static function parsePrerequisites(
         bool $multiline,
         bool &$ignoreNextLinesOfMultiline
     ): array {
-        if ($ignoreNextLinesOfMultiline && $multiline) {
+        if (($ignoreNextLinesOfMultiline && $multiline)
+            || '' === $dependencies
+        ) {
             return [];
         }
 
-        if (str_contains($dependencies, '##')) {
-            return [trim($dependencies)];
+        if ($multiline) {
+            $dependenciesParts = explode(self::MULTILINE_DELIMITER, $dependencies);
+
+            return [
+                ...self::parsePrerequisites($dependenciesParts[0], false, $ignoreNextLinesOfMultiline),
+                // There cannot be more parts by design
+                ...self::parsePrerequisites($dependenciesParts[1] ?? '', false, $ignoreNextLinesOfMultiline),
+            ];
+        }
+
+        if (str_starts_with($dependencies, '#')) {
+            return [$dependencies];
+        }
+
+        if (str_contains($dependencies, '#')) {
+            [$nonCommentPart, $commentPart] = self::splitComment($dependencies);
+
+            return [
+                ...self::parsePrerequisites($nonCommentPart, false, $ignoreNextLinesOfMultiline),
+                ...self::parsePrerequisites($commentPart, false, $ignoreNextLinesOfMultiline),
+            ];
         }
 
         $semicolonPosition = mb_strpos($dependencies, ';');
@@ -158,15 +181,40 @@ private static function parsePrerequisites(
             $ignoreNextLinesOfMultiline = true;
         }
 
-        $charactersToTrim = $multiline ? '\\'."\t" : "\t";
-
         return array_values(
             array_filter(
                 array_map(
-                    static fn (string $dependency) => ltrim($dependency, $charactersToTrim),
+                    static fn (string $dependency) => ltrim($dependency, "\t"),
                     explode(' ', $dependencies),
                 ),
             ),
         );
     }
+
+    private static function isMultiline(string $line): bool
+    {
+        $lineWithoutComment = rtrim(self::trimComment($line));
+
+        return str_ends_with($lineWithoutComment, self::MULTILINE_DELIMITER);
+    }
+
+    private static function trimComment(string $line): string
+    {
+        return self::splitComment($line)[0];
+    }
+
+    /**
+     * @psalm-suppress LessSpecificReturnStatement,MoreSpecificReturnType,PossiblyNullArrayAccess,PossiblyUndefinedStringArrayOffset
+     *
+     * @return array{string, string}
+     */
+    private static function splitComment(string $line): array
+    {
+        return preg_match(self::COMMENT_LINE, $line, $matches)
+            ? [
+                $matches['nonCommentPart'],
+                $matches['commentPart'],
+            ]
+            : [$line, ''];
+    }
 }
diff --git a/tests/ParserTest.php b/tests/ParserTest.php
@@ -295,6 +295,20 @@ public static function makefileContentProvider(): iterable
             [],
         ];
 
+        yield 'regular comment' => [
+            <<<'MAKEFILE'
+                # comment
+                MAKEFILE,
+            [],
+        ];
+
+        yield '"command" comment' => [
+            <<<'MAKEFILE'
+                ## comment
+                MAKEFILE,
+            [],
+        ];
+
         yield 'variable & comment with command' => [
             <<<'MAKEFILE'
                 FLOCK := flock
@@ -316,6 +330,79 @@ public static function makefileContentProvider(): iterable
             ],
         ];
 
+        yield 'target with regular comment' => [
+            <<<'MAKEFILE'
+                check: # comment
+                MAKEFILE,
+            [
+                new Rule('check', ['# comment']),
+            ],
+        ];
+
+        yield 'target with regular comment with trailing space' => [
+            'check: # comment ',
+            [
+                new Rule('check', ['# comment']),
+            ],
+        ];
+
+        yield 'target with "command" comment' => [
+            <<<'MAKEFILE'
+                check: ## comment
+                MAKEFILE,
+            [
+                new Rule('check', ['## comment']),
+            ],
+        ];
+
+        yield 'multiline target with regular comments' => [
+            <<<'MAKEFILE'
+                check: \ # Comment A
+                    pre_req0 \ # Comment B 
+                    pre_req1 \ # Comment C # Command C bis
+                    pre_req2 # Comment D with multiline \ in comment
+                # Comment E 
+                MAKEFILE,
+            [
+                new Rule(
+                    'check',
+                    [
+                        '# Comment A',
+                        'pre_req0',
+                        '# Comment B',
+                        'pre_req1',
+                        '# Comment C # Command C bis',
+                        'pre_req2',
+                        '# Comment D with multiline \ in comment',
+                    ],
+                ),
+            ],
+        ];
+
+        yield 'multiline target with command comments' => [
+            <<<'MAKEFILE'
+                check: \ ## Comment A
+                    pre_req0 \ ## Comment B 
+                    pre_req1 \ ## Comment C ## Command C bis
+                    pre_req2 ## Comment D with multiline \ in comment
+                ## Comment E 
+                MAKEFILE,
+            [
+                new Rule(
+                    'check',
+                    [
+                        '## Comment A',
+                        'pre_req0',
+                        '## Comment B',
+                        'pre_req1',
+                        '## Comment C ## Command C bis',
+                        'pre_req2',
+                        '## Comment D with multiline \ in comment',
+                    ],
+                ),
+            ],
+        ];
+
         yield from self::officialDocumentationExamples();
     }
 

diff --git a/tests/ThrowableToStringMapper.php b/tests/ThrowableToStringMapper.php
@@ -54,7 +54,7 @@ public static function map(Throwable $throwable): string
             }
 
             /** @psalm-suppress UndefinedClass */
-            if ($throwable instanceof PhptAssertionFailedError) {
+            if ($throwable instanceof PHPTAssertionFailedError) {
                 /** @psalm-suppress UndefinedMethod */
                 $buffer .= $throwable->diff();
             }