Rework

VincentLanglet · Nov 23, 2023 · 0ea70fd · 0ea70fd
1 parent 7e77c2b
commit 0ea70fd
Show file tree

Hide file tree

Showing 6 changed files with 125 additions and 25 deletions.
diff --git a/composer-require-checker.json b/composer-require-checker.json
@@ -8,7 +8,6 @@
         "Symfony\\Bridge\\Twig\\TokenParser\\TransTokenParser",
         "Symfony\\UX\\TwigComponent\\Twig\\ComponentTokenParser",
         "Symfony\\UX\\TwigComponent\\Twig\\PropsTokenParser",
-        "Twig\\Extra\\Cache\\TokenParser\\CacheTokenParser",
-        "\\SimpleXMLElement"
+        "Twig\\Extra\\Cache\\TokenParser\\CacheTokenParser"
     ]
 }
diff --git a/src/Report/Reporter/CheckstyleReporter.php b/src/Report/Reporter/CheckstyleReporter.php
@@ -8,35 +8,52 @@
 use TwigCsFixer\Report\Report;
 use TwigCsFixer\Report\SniffViolation;
 
-/**
- * Human-readable output with context.
- */
 final class CheckstyleReporter implements ReporterInterface
 {
     public const NAME = 'checkstyle';
 
     public function display(OutputInterface $output, Report $report, ?string $level = null): void
     {
-        $checkstyle = new \SimpleXMLElement('<checkstyle version="1.0.0"/>');
+        $text = '<?xml version="1.0" encoding="UTF-8"?>'."\n";
+
+        $text .= '<checkstyle>'."\n";
 
         foreach ($report->getFiles() as $file) {
             $fileMessages = $report->getMessages($file, $level);
-            if (\count($fileMessages) > 0) {
-                /** @var \SimpleXMLElement $fileNode */
-                $fileNode = $checkstyle->addChild('file');
-                $fileNode->addAttribute('name', $file);
-
-                foreach ($fileMessages as $message) {
-                    /** @var \SimpleXMLElement $violation */
-                    $violation = $fileNode->addChild('violation');
-                    $violation->addAttribute('column', (string) $message->getLinePosition());
-                    $violation->addAttribute('severity', strtolower(SniffViolation::getLevelAsString($message->getLevel())));
-                    $violation->addAttribute('message', $message->getMessage());
-                    $violation->addAttribute('source', $message->getSniffName() ?? '');
+            if (0 === \count($fileMessages)) {
+                continue;
+            }
+
+            $text .= '  <file name="'.$this->xmlEncode($file).'">'."\n";
+            foreach ($fileMessages as $message) {
+                $line = (string) $message->getLine();
+                $linePosition = (string) $message->getLinePosition();
+                $sniffName = $message->getSniffName();
+
+                $text .= '    <error';
+                if ('' !== $line) {
+                    $text .= ' line="'.$line.'"';
+                }
+                if ('' !== $linePosition) {
+                    $text .= ' column="'.$linePosition.'"';
                 }
+                $text .= ' severity="'.strtolower(SniffViolation::getLevelAsString($message->getLevel())).'"';
+                $text .= ' message="'.$this->xmlEncode($message->getMessage()).'"';
+                if (null !== $sniffName) {
+                    $text .= ' source="'.$sniffName.'"';
+                }
+                $text .= '/>'."\n";
             }
+            $text .= '  </file>'."\n";
         }
 
-        $output->writeln((string) $checkstyle->asXML());
+        $text .= '</checkstyle>'."\n";
+
+        $output->writeln($text);
+    }
+
+    private function xmlEncode(string $data): string
+    {
+        return htmlspecialchars($data, \ENT_XML1 | \ENT_QUOTES);
     }
 }
diff --git a/src/Report/ReporterFactory.php b/src/Report/ReporterFactory.php
@@ -5,20 +5,20 @@
 namespace TwigCsFixer\Report;
 
 use InvalidArgumentException;
+use TwigCsFixer\Report\Reporter\CheckstyleReporter;
 use TwigCsFixer\Report\Reporter\NullReporter;
 use TwigCsFixer\Report\Reporter\ReporterInterface;
 use TwigCsFixer\Report\Reporter\TextReporter;
-use TwigCsFixer\Report\Reporter\CheckstyleReporter;
 
 final class ReporterFactory
 {
     public function getReporter(string $format = TextReporter::NAME): ReporterInterface
     {
         return match ($format) {
-            NullReporter::NAME => new NullReporter(),
-            TextReporter::NAME => new TextReporter(),
+            NullReporter::NAME       => new NullReporter(),
+            TextReporter::NAME       => new TextReporter(),
             CheckstyleReporter::NAME => new CheckstyleReporter(),
-            default            => throw new InvalidArgumentException(
+            default                  => throw new InvalidArgumentException(
                 sprintf('No reporter supports the format "%s".', $format)
             ),
         };

diff --git a/tests/Command/TwigCsFixerCommandTest.php b/tests/Command/TwigCsFixerCommandTest.php
@@ -116,13 +116,13 @@ public function testExecuteWithReportOption(): void
 
         $commandTester = new CommandTester($command);
         $commandTester->execute([
-            'paths'      => [$this->getTmpPath(__DIR__.'/Fixtures/file.twig')],
+            'paths'      => [$this->getTmpPath(__DIR__.'/Fixtures')],
             '--no-cache' => true, // To avoid cache output
             '--report'   => 'null',
         ]);
 
         static::assertSame('', $commandTester->getDisplay());
-        static::assertSame(Command::SUCCESS, $commandTester->getStatusCode());
+        static::assertSame(Command::FAILURE, $commandTester->getStatusCode());
     }
 
     public function testExecuteWithConfig(): void

diff --git a/tests/Report/Formatter/CheckstyleReporterTest.php b/tests/Report/Formatter/CheckstyleReporterTest.php
@@ -0,0 +1,82 @@
+<?php
+
+declare(strict_types=1);
+
+namespace TwigCsFixer\Tests\Report\Formatter;
+
+use PHPUnit\Framework\TestCase;
+use SplFileInfo;
+use Symfony\Component\Console\Output\BufferedOutput;
+use Symfony\Component\Console\Output\OutputInterface;
+use TwigCsFixer\Report\Report;
+use TwigCsFixer\Report\Reporter\CheckstyleReporter;
+use TwigCsFixer\Report\SniffViolation;
+
+final class CheckstyleReporterTest extends TestCase
+{
+    /**
+     * @dataProvider displayDataProvider
+     */
+    public function testDisplayErrors(string $expected, ?string $level): void
+    {
+        $textFormatter = new CheckstyleReporter();
+
+        $file = __DIR__.'/Fixtures/file.twig';
+        $report = new Report([new SplFileInfo($file)]);
+
+        $violation0 = new SniffViolation(SniffViolation::LEVEL_NOTICE, 'Notice', $file, 1, 11, 'NoticeSniff');
+        $report->addMessage($violation0);
+        $violation1 = new SniffViolation(SniffViolation::LEVEL_WARNING, 'Warning', $file, 2, 22, 'WarningSniff');
+        $report->addMessage($violation1);
+        $violation2 = new SniffViolation(SniffViolation::LEVEL_ERROR, 'Error', $file, 3, 33, 'ErrorSniff');
+        $report->addMessage($violation2);
+        $violation3 = new SniffViolation(SniffViolation::LEVEL_FATAL, 'Fatal', $file);
+        $report->addMessage($violation3);
+
+        $output = new BufferedOutput(OutputInterface::VERBOSITY_NORMAL, true);
+        $textFormatter->display($output, $report, $level);
+
+        $text = $output->fetch();
+        static::assertStringContainsString($expected, $text);
+    }
+
+    /**
+     * @return iterable<array-key, array{string, string|null}>
+     */
+    public static function displayDataProvider(): iterable
+    {
+        yield [
+            sprintf(
+                <<<EOD
+                    <?xml version="1.0" encoding="UTF-8"?>
+                    <checkstyle>
+                      <file name="%s/Fixtures/file.twig">
+                        <error line="1" column="11" severity="notice" message="Notice" source="NoticeSniff"/>
+                        <error line="2" column="22" severity="warning" message="Warning" source="WarningSniff"/>
+                        <error line="3" column="33" severity="error" message="Error" source="ErrorSniff"/>
+                        <error severity="fatal" message="Fatal"/>
+                      </file>
+                    </checkstyle>
+                    EOD,
+                __DIR__
+            ),
+            null,
+        ];
+
+        yield [
+            sprintf(
+                <<<EOD
+                    <?xml version="1.0" encoding="UTF-8"?>
+                    <checkstyle>
+                      <file name="%s/Fixtures/file.twig">
+                        <error line="3" column="33" severity="error" message="Error" source="ErrorSniff"/>
+                        <error severity="fatal" message="Fatal"/>
+                      </file>
+                    </checkstyle>
+                    EOD,
+                __DIR__
+            ),
+            Report::MESSAGE_TYPE_ERROR,
+        ];
+    }
+}
diff --git a/tests/Report/ReporterFactoryTest.php b/tests/Report/ReporterFactoryTest.php
@@ -5,6 +5,7 @@
 namespace TwigCsFixer\Tests\Report;
 
 use PHPUnit\Framework\TestCase;
+use TwigCsFixer\Report\Reporter\CheckstyleReporter;
 use TwigCsFixer\Report\Reporter\NullReporter;
 use TwigCsFixer\Report\Reporter\TextReporter;
 use TwigCsFixer\Report\ReporterFactory;
@@ -18,6 +19,7 @@ public function testGetReporter(): void
         static::assertInstanceOf(TextReporter::class, $reporterFactory->getReporter());
         static::assertInstanceOf(TextReporter::class, $reporterFactory->getReporter(TextReporter::NAME));
         static::assertInstanceOf(NullReporter::class, $reporterFactory->getReporter(NullReporter::NAME));
+        static::assertInstanceOf(CheckstyleReporter::class, $reporterFactory->getReporter(CheckstyleReporter::NAME));
     }
 
     public function testGetMessageForAnotherFile(): void