From 2b7f69210ba2f752d9dc24f0e27d0e8c416c4c47 Mon Sep 17 00:00:00 2001 From: Kenneth Sills <132029135+Kenneth-Sills@users.noreply.github.com> Date: Mon, 27 May 2024 00:18:38 +0000 Subject: [PATCH 01/10] refactor(formatters): improved discoverability of formatters & their properties Whether or not a formatter supports progress bars is now encapsulated in the formatter class itself, as opposed to being scattered in a couple places and relying on maintainers remembering. Both this progress bar support and the formatter names are also implemented as static methods now, whose inclusion can be promised by their shared interface. The console output retains its `::NAME` constant because it needs to be used in the default parameters of a configuration constructor (being a default setting). Finally, there remains one hard-coded reference to the `json` output format, but only over in the `ListCheckersCommand`. I've left this alone since this is not the same type of formatter. Don't want to step on any toes. :) --- src/Configuration/ConfigurationFactory.php | 13 +++++++++---- src/Console/Command/AbstractCheckCommand.php | 2 +- src/Console/Command/ListCheckersCommand.php | 2 +- .../EasyCodingStandardConsoleApplication.php | 2 +- src/Console/Output/CheckstyleOutputFormatter.php | 14 +++++++------- src/Console/Output/ConsoleOutputFormatter.php | 7 ++++++- src/Console/Output/JsonOutputFormatter.php | 14 +++++++------- .../Console/Output/OutputFormatterInterface.php | 4 +++- 8 files changed, 35 insertions(+), 23 deletions(-) diff --git a/src/Configuration/ConfigurationFactory.php b/src/Configuration/ConfigurationFactory.php index 825a8ef534..fb545a5ede 100644 --- a/src/Configuration/ConfigurationFactory.php +++ b/src/Configuration/ConfigurationFactory.php @@ -5,8 +5,7 @@ namespace Symplify\EasyCodingStandard\Configuration; use Symfony\Component\Console\Input\InputInterface; -use Symplify\EasyCodingStandard\Console\Output\CheckstyleOutputFormatter; -use Symplify\EasyCodingStandard\Console\Output\JsonOutputFormatter; +use Symplify\EasyCodingStandard\Console\Output\OutputFormatterCollector; use Symplify\EasyCodingStandard\DependencyInjection\SimpleParameterProvider; use Symplify\EasyCodingStandard\Exception\Configuration\SourceNotFoundException; use Symplify\EasyCodingStandard\ValueObject\Configuration; @@ -14,6 +13,11 @@ final class ConfigurationFactory { + public function __construct( + private readonly OutputFormatterCollector $outputFormatterCollector + ) { + } + /** * Needs to run in the start of the life cycle, since the rest of workflow uses it. */ @@ -66,8 +70,9 @@ private function canShowProgressBar(InputInterface $input): bool } $outputFormat = $input->getOption(Option::OUTPUT_FORMAT); - $formatsWithoutProgressBar = [CheckstyleOutputFormatter::NAME, JsonOutputFormatter::NAME]; - if (in_array($outputFormat, $formatsWithoutProgressBar, true)) { + $formatter = $this->outputFormatterCollector->getByName($outputFormat); + + if (! $formatter->hasSupportForProgressBars()) { return false; } diff --git a/src/Console/Command/AbstractCheckCommand.php b/src/Console/Command/AbstractCheckCommand.php index 1ae1314f57..2e2c13146d 100644 --- a/src/Console/Command/AbstractCheckCommand.php +++ b/src/Console/Command/AbstractCheckCommand.php @@ -52,7 +52,7 @@ protected function configure(): void null, InputOption::VALUE_REQUIRED, 'Select output format', - ConsoleOutputFormatter::NAME + ConsoleOutputFormatter::getName() ); $this->addOption(Option::MEMORY_LIMIT, null, InputOption::VALUE_REQUIRED, 'Memory limit for check'); diff --git a/src/Console/Command/ListCheckersCommand.php b/src/Console/Command/ListCheckersCommand.php index 4b463ea4dc..0bbd7a1eb8 100644 --- a/src/Console/Command/ListCheckersCommand.php +++ b/src/Console/Command/ListCheckersCommand.php @@ -39,7 +39,7 @@ protected function configure(): void null, InputOption::VALUE_REQUIRED, 'Select output format', - ConsoleOutputFormatter::NAME + ConsoleOutputFormatter::getName() ); $this->addOption(Option::CONFIG, 'c', InputOption::VALUE_REQUIRED, 'Path to config file'); diff --git a/src/Console/EasyCodingStandardConsoleApplication.php b/src/Console/EasyCodingStandardConsoleApplication.php index e1f7ae0b31..30dcc8724b 100644 --- a/src/Console/EasyCodingStandardConsoleApplication.php +++ b/src/Console/EasyCodingStandardConsoleApplication.php @@ -92,7 +92,7 @@ private function shouldPrintMetaInformation(InputInterface $input): bool $outputFormat = $input->getParameterOption('--' . Option::OUTPUT_FORMAT); - return $outputFormat === ConsoleOutputFormatter::NAME; + return $outputFormat === ConsoleOutputFormatter::getName(); } private function addExtraOptions(InputDefinition $inputDefinition): void diff --git a/src/Console/Output/CheckstyleOutputFormatter.php b/src/Console/Output/CheckstyleOutputFormatter.php index 5e95073dd2..4a1572eef2 100644 --- a/src/Console/Output/CheckstyleOutputFormatter.php +++ b/src/Console/Output/CheckstyleOutputFormatter.php @@ -18,11 +18,6 @@ */ final readonly class CheckstyleOutputFormatter implements OutputFormatterInterface { - /** - * @var string - */ - public const NAME = 'checkstyle'; - public function __construct( private EasyCodingStandardStyle $easyCodingStandardStyle, private ExitCodeResolver $exitCodeResolver @@ -40,9 +35,14 @@ public function report(ErrorAndDiffResult $errorAndDiffResult, Configuration $co return $this->exitCodeResolver->resolve($errorAndDiffResult, $configuration); } - public function getName(): string + public static function getName(): string + { + return 'checkstyle'; + } + + public static function hasSupportForProgressBars(): bool { - return self::NAME; + return false; } /** diff --git a/src/Console/Output/ConsoleOutputFormatter.php b/src/Console/Output/ConsoleOutputFormatter.php index df96b21c84..f26288c838 100644 --- a/src/Console/Output/ConsoleOutputFormatter.php +++ b/src/Console/Output/ConsoleOutputFormatter.php @@ -53,11 +53,16 @@ public function report(ErrorAndDiffResult $errorAndDiffResult, Configuration $co return $this->exitCodeResolver->resolve($errorAndDiffResult, $configuration); } - public function getName(): string + public static function getName(): string { return self::NAME; } + public static function hasSupportForProgressBars(): bool + { + return true; + } + /** * @param FileDiff[] $fileDiffs */ diff --git a/src/Console/Output/JsonOutputFormatter.php b/src/Console/Output/JsonOutputFormatter.php index 2ee4fd4b59..c397836d4b 100644 --- a/src/Console/Output/JsonOutputFormatter.php +++ b/src/Console/Output/JsonOutputFormatter.php @@ -16,11 +16,6 @@ */ final readonly class JsonOutputFormatter implements OutputFormatterInterface { - /** - * @var string - */ - public const NAME = 'json'; - /** * @var string */ @@ -43,9 +38,14 @@ public function report(ErrorAndDiffResult $errorAndDiffResult, Configuration $co return $this->exitCodeResolver->resolve($errorAndDiffResult, $configuration); } - public function getName(): string + public static function getName(): string + { + return 'json'; + } + + public static function hasSupportForProgressBars(): bool { - return self::NAME; + return false; } /** diff --git a/src/Contract/Console/Output/OutputFormatterInterface.php b/src/Contract/Console/Output/OutputFormatterInterface.php index e89a00eeb4..8cce554e34 100644 --- a/src/Contract/Console/Output/OutputFormatterInterface.php +++ b/src/Contract/Console/Output/OutputFormatterInterface.php @@ -15,5 +15,7 @@ interface OutputFormatterInterface */ public function report(ErrorAndDiffResult $errorAndDiffResult, Configuration $configuration): int; - public function getName(): string; + public static function getName(): string; + + public static function hasSupportForProgressBars(): bool; } From 723447da61d1a4aeb1e2c9dc3ede4bf68ded34f4 Mon Sep 17 00:00:00 2001 From: Kenneth Sills <132029135+Kenneth-Sills@users.noreply.github.com> Date: Mon, 27 May 2024 00:19:23 +0000 Subject: [PATCH 02/10] feat(formatters): added a Gitlab/Code Climate output formatter --- src/Console/Output/GitlabOutputFormatter.php | 226 ++++++++++++++++++ .../LazyContainerFactory.php | 6 + 2 files changed, 232 insertions(+) create mode 100644 src/Console/Output/GitlabOutputFormatter.php diff --git a/src/Console/Output/GitlabOutputFormatter.php b/src/Console/Output/GitlabOutputFormatter.php new file mode 100644 index 0000000000..25c783b7d2 --- /dev/null +++ b/src/Console/Output/GitlabOutputFormatter.php @@ -0,0 +1,226 @@ +isFixer() && $config->shouldShowDiffs()) + ? merge( + $this->generateIssuesForErrors($results->getErrors()), + $this->generateIssuesForFixes($results->getFileDiffs()), + ) + : $this->generateIssuesForErrors($results->getErrors()); + + $output = $this->encode($reportedQualityIssues); + + $this->easyCodingStandardStyle->writeln($output); + return $this->exitCodeResolver->resolve($results, $config); + } + + /** + * @param CodingStandardError[] $errors + * @return GitlabIssue[] + */ + private function generateIssuesForErrors(array $errors): array + { + return map( + fn (CodingStandardError $error) => [ + 'type' => 'issue', + 'description' => $error->getMessage(), + 'check_name' => $error->getCheckerClass(), + 'fingerprint' => $this->generateFingerprint( + $error->getCheckerClass(), + $error->getMessage(), + $error->getRelativeFilePath(), + ), + 'severity' => 'blocker', + 'categories' => ['Style'], + 'location' => [ + 'path' => $error->getRelativeFilePath(), + 'lines' => [ + 'begin' => $error->getLine(), + 'end' => $error->getLine(), + ], + ], + ], + $errors, + ); + } + + /** + * Reports each chunk of changes as a separate issue. + * + * @param FileDiff[] $diffs + * @return GitlabIssue[] + */ + private function generateIssuesForFixes(array $diffs): array + { + return merge( + ...map( + fn (FileDiff $diff) => map( + fn (Chunk $chunk) => $this->generateIssueForChunk($diff, $chunk), + $this->diffParser->parse($diff->getDiff())[0]->chunks(), + ), + $diffs, + ), + ); + } + + /** + * @return GitlabIssue + */ + private function generateIssueForChunk(FileDiff $diff, Chunk $chunk): array + { + $checkersAsFqcns = implode(',', $diff->getAppliedCheckers()); + $checkersAsClasses = implode(', ', map( + fn (string $checker) => preg_replace('/.*\\\/', '', $checker), + $diff->getAppliedCheckers(), + )); + + $message = "Chunk has fixable errors: {$checkersAsClasses}"; + $lineStart = $chunk->start(); + $lineEnd = $lineStart + $chunk->startRange() - 1; + + return [ + 'type' => 'issue', + 'description' => $message, + 'check_name' => $checkersAsFqcns, + 'fingerprint' => $this->generateFingerprint( + $checkersAsFqcns, + $message, + $diff->getRelativeFilePath(), + implode('\n', map(fn (Line $line) => "{$line->type()}:{$line->content()}", $chunk->lines())), + ), + 'severity' => 'blocker', + 'categories' => ['Style'], + 'remediation_points' => 50_000, + 'location' => [ + 'path' => $diff->getRelativeFilePath(), + 'lines' => [ + 'begin' => $lineStart, + 'end' => $lineEnd, + ], + ], + ]; + } + + /** + * Generate a fingerprint for a given quality issue. This is used to + * track the presence of an issue between runs, so it should be unique + * and consistent for a given issue. + * + * Subsequently, changing the fingerprint or the data it uses is + * _technically_ a breaking change. Users would see existing issues being + * marked as "new" on the commit proceeding updating ECS. + * + * DO NOT include position information as salting, or every time + * lines are added/removed the lines below it will be reported as + * new errors. + */ + private function generateFingerprint( + string $checker, + string $message, + string $relativeFilePath, + string $salt = '', + ): string { + // We implode to add a separator that cannot show up in PHP + // class names or Linux file names and SHOULD never show up in + // messages. This guarantees the same fingerprint won't be generated + // by accident, by the associative property of concatenation. As in: + // + // (ABC + ABC = ABCABC) == (ABCA + BC = ABCABC) + // (ABC + \0 + ABC = ABC\0ABC) != (ABCA + \0 + BC = ABCA\0BC) + return md5(implode("\0", [$checker, $message, $relativeFilePath, $salt])); + } + + /** + * @param GitlabIssue[] $lineItems + */ + private function encode(array $lineItems): string + { + return json_encode( + $lineItems, + JSON_THROW_ON_ERROR | JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE + ); + } +} diff --git a/src/DependencyInjection/LazyContainerFactory.php b/src/DependencyInjection/LazyContainerFactory.php index 5d54ae5edd..c3ad4f1664 100644 --- a/src/DependencyInjection/LazyContainerFactory.php +++ b/src/DependencyInjection/LazyContainerFactory.php @@ -12,6 +12,7 @@ use PhpCsFixer\Differ\UnifiedDiffer; use PhpCsFixer\Fixer\FixerInterface; use PhpCsFixer\WhitespacesFixerConfig; +use SebastianBergmann\Diff\Parser as DiffParser; use Symfony\Component\Console\Style\SymfonyStyle; use Symplify\EasyCodingStandard\Application\SingleFileProcessor; use Symplify\EasyCodingStandard\Caching\Cache; @@ -20,6 +21,7 @@ use Symplify\EasyCodingStandard\Config\ECSConfig; use Symplify\EasyCodingStandard\Console\Output\CheckstyleOutputFormatter; use Symplify\EasyCodingStandard\Console\Output\ConsoleOutputFormatter; +use Symplify\EasyCodingStandard\Console\Output\GitlabOutputFormatter; use Symplify\EasyCodingStandard\Console\Output\JsonOutputFormatter; use Symplify\EasyCodingStandard\Console\Output\OutputFormatterCollector; use Symplify\EasyCodingStandard\Console\Style\EasyCodingStandardStyle; @@ -81,7 +83,11 @@ static function (Container $container): EasyCodingStandardStyle { return $cacheFactory->create(); }); + // diffing + $ecsConfig->singleton(DiffParser::class); + // output + $ecsConfig->singleton(GitlabOutputFormatter::class); $ecsConfig->singleton(CheckstyleOutputFormatter::class); $ecsConfig->singleton(ConsoleOutputFormatter::class); $ecsConfig->singleton(JsonOutputFormatter::class); From 23a28ebe760240b025b6326f66cf08b86b9289d7 Mon Sep 17 00:00:00 2001 From: Kenneth Sills <132029135+Kenneth-Sills@users.noreply.github.com> Date: Mon, 27 May 2024 00:20:43 +0000 Subject: [PATCH 03/10] docs(formatters): added a README section on output formats for clarity I, personally, didn't realize this tool supported output formatting at first, and the existing issues seemed to have been closed without mentioning the addition of them. I'd just like to make it more obvious for new users! --- README.md | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/README.md b/README.md index 9ac11106b9..be62a4dc3d 100644 --- a/README.md +++ b/README.md @@ -167,6 +167,22 @@ composer fix-cs
+### Controlling Output Format + +You may want to use ECS to generate reports for third-party tooling. + +We currently provide formatters for: + +- `console`: Human-oriented printing à la PHP CS Fixer. +- `json`: A custom JSON blob for arbitrary tooling. +- `checkstyle`: Useful for Github Action Reports. +- `gitlab`: For Gitlab code quality reports or Code Climate tooling. + +For information on how each of these behave, refer to their respective +[implementations](src/Console/Output/). + +
+ ## FAQ ### How do I clear cache? From 527179118bfb919e1b37eb4af426c088dd869e43 Mon Sep 17 00:00:00 2001 From: Kenneth Sills <132029135+Kenneth-Sills@users.noreply.github.com> Date: Mon, 27 May 2024 00:29:37 +0000 Subject: [PATCH 04/10] refactor(formatters): ran automated fixers on changed files --- src/Configuration/ConfigurationFactory.php | 4 +- src/Console/Output/GitlabOutputFormatter.php | 59 +++++++++++--------- 2 files changed, 35 insertions(+), 28 deletions(-) diff --git a/src/Configuration/ConfigurationFactory.php b/src/Configuration/ConfigurationFactory.php index fb545a5ede..798d146026 100644 --- a/src/Configuration/ConfigurationFactory.php +++ b/src/Configuration/ConfigurationFactory.php @@ -11,10 +11,10 @@ use Symplify\EasyCodingStandard\ValueObject\Configuration; use Symplify\EasyCodingStandard\ValueObject\Option; -final class ConfigurationFactory +final readonly class ConfigurationFactory { public function __construct( - private readonly OutputFormatterCollector $outputFormatterCollector + private OutputFormatterCollector $outputFormatterCollector ) { } diff --git a/src/Console/Output/GitlabOutputFormatter.php b/src/Console/Output/GitlabOutputFormatter.php index 25c783b7d2..cc81f8517e 100644 --- a/src/Console/Output/GitlabOutputFormatter.php +++ b/src/Console/Output/GitlabOutputFormatter.php @@ -82,19 +82,19 @@ public static function hasSupportForProgressBars(): bool /** * @return ExitCode::* */ - public function report(ErrorAndDiffResult $results, Configuration $config): int + public function report(ErrorAndDiffResult $errorAndDiffResult, Configuration $configuration): int { - $reportedQualityIssues = (! $config->isFixer() && $config->shouldShowDiffs()) + $reportedQualityIssues = (! $configuration->isFixer() && $configuration->shouldShowDiffs()) ? merge( - $this->generateIssuesForErrors($results->getErrors()), - $this->generateIssuesForFixes($results->getFileDiffs()), + $this->generateIssuesForErrors($errorAndDiffResult->getErrors()), + $this->generateIssuesForFixes($errorAndDiffResult->getFileDiffs()), ) - : $this->generateIssuesForErrors($results->getErrors()); + : $this->generateIssuesForErrors($errorAndDiffResult->getErrors()); $output = $this->encode($reportedQualityIssues); $this->easyCodingStandardStyle->writeln($output); - return $this->exitCodeResolver->resolve($results, $config); + return $this->exitCodeResolver->resolve($errorAndDiffResult, $configuration); } /** @@ -104,22 +104,22 @@ public function report(ErrorAndDiffResult $results, Configuration $config): int private function generateIssuesForErrors(array $errors): array { return map( - fn (CodingStandardError $error) => [ + fn (CodingStandardError $codingStandardError): array => [ 'type' => 'issue', - 'description' => $error->getMessage(), - 'check_name' => $error->getCheckerClass(), + 'description' => $codingStandardError->getMessage(), + 'check_name' => $codingStandardError->getCheckerClass(), 'fingerprint' => $this->generateFingerprint( - $error->getCheckerClass(), - $error->getMessage(), - $error->getRelativeFilePath(), + $codingStandardError->getCheckerClass(), + $codingStandardError->getMessage(), + $codingStandardError->getRelativeFilePath(), ), 'severity' => 'blocker', 'categories' => ['Style'], 'location' => [ - 'path' => $error->getRelativeFilePath(), + 'path' => $codingStandardError->getRelativeFilePath(), 'lines' => [ - 'begin' => $error->getLine(), - 'end' => $error->getLine(), + 'begin' => $codingStandardError->getLine(), + 'end' => $codingStandardError->getLine(), ], ], ], @@ -137,9 +137,9 @@ private function generateIssuesForFixes(array $diffs): array { return merge( ...map( - fn (FileDiff $diff) => map( - fn (Chunk $chunk) => $this->generateIssueForChunk($diff, $chunk), - $this->diffParser->parse($diff->getDiff())[0]->chunks(), + fn (FileDiff $fileDiff): array => map( + fn (Chunk $chunk): array => $this->generateIssueForChunk($fileDiff, $chunk), + $this->diffParser->parse($fileDiff->getDiff())[0]->chunks(), ), $diffs, ), @@ -149,15 +149,15 @@ private function generateIssuesForFixes(array $diffs): array /** * @return GitlabIssue */ - private function generateIssueForChunk(FileDiff $diff, Chunk $chunk): array + private function generateIssueForChunk(FileDiff $fileDiff, Chunk $chunk): array { - $checkersAsFqcns = implode(',', $diff->getAppliedCheckers()); + $checkersAsFqcns = implode(',', $fileDiff->getAppliedCheckers()); $checkersAsClasses = implode(', ', map( - fn (string $checker) => preg_replace('/.*\\\/', '', $checker), - $diff->getAppliedCheckers(), + static fn (string $checker): string => preg_replace('/.*\\\/', '', $checker) ?? $checker, + $fileDiff->getAppliedCheckers(), )); - $message = "Chunk has fixable errors: {$checkersAsClasses}"; + $message = 'Chunk has fixable errors: ' . $checkersAsClasses; $lineStart = $chunk->start(); $lineEnd = $lineStart + $chunk->startRange() - 1; @@ -168,14 +168,21 @@ private function generateIssueForChunk(FileDiff $diff, Chunk $chunk): array 'fingerprint' => $this->generateFingerprint( $checkersAsFqcns, $message, - $diff->getRelativeFilePath(), - implode('\n', map(fn (Line $line) => "{$line->type()}:{$line->content()}", $chunk->lines())), + $fileDiff->getRelativeFilePath(), + implode( + '\n', + map(static fn (Line $line): string => sprintf( + '%d:%s', + $line->type(), + $line->content() + ), $chunk->lines()) + ), ), 'severity' => 'blocker', 'categories' => ['Style'], 'remediation_points' => 50_000, 'location' => [ - 'path' => $diff->getRelativeFilePath(), + 'path' => $fileDiff->getRelativeFilePath(), 'lines' => [ 'begin' => $lineStart, 'end' => $lineEnd, From cae3069bd7e18cd48fcb025e5c93a81a8d4085ac Mon Sep 17 00:00:00 2001 From: Kenneth Sills <132029135+Kenneth-Sills@users.noreply.github.com> Date: Mon, 27 May 2024 22:43:06 +0000 Subject: [PATCH 05/10] fix(formatters): set gitlab issue severity to 'minor' My initial line of thinking about setting to 'blocker' was flawed. Not everyone uses this tooling the same way and they may want it to show up in the MR widget/diffs without _actually_ blocking. Since ECS focuses on code style standards instead of correctness issues, this seems an appropriate level. --- src/Console/Output/GitlabOutputFormatter.php | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/Console/Output/GitlabOutputFormatter.php b/src/Console/Output/GitlabOutputFormatter.php index cc81f8517e..0df119df02 100644 --- a/src/Console/Output/GitlabOutputFormatter.php +++ b/src/Console/Output/GitlabOutputFormatter.php @@ -34,7 +34,9 @@ * and with cascading effects, multiple major refactors would be required in * order to accurately report exactly which fixes apply to exactly which lines. * - * Finally, all reported errors will be marked as "blocker" severity. + * Finally, all reported errors will be marked as "minor" severity, since + * DevOps downstream can choose if CI/CD ignores and our Sniffers don't + * currently provide their own levels. * * As a warning to future maintainers, I believe Gitlab's documentation may be * slightly wrong. It's not a subset of the Code Climate format as insinuated, @@ -48,7 +50,7 @@ * description: string, * check_name: string, * fingerprint: string, - * severity: 'blocker', + * severity: 'minor', * categories: array{'Style'}, * remediation_points?: int, * location: array{ @@ -113,7 +115,7 @@ private function generateIssuesForErrors(array $errors): array $codingStandardError->getMessage(), $codingStandardError->getRelativeFilePath(), ), - 'severity' => 'blocker', + 'severity' => 'minor', 'categories' => ['Style'], 'location' => [ 'path' => $codingStandardError->getRelativeFilePath(), @@ -178,7 +180,7 @@ private function generateIssueForChunk(FileDiff $fileDiff, Chunk $chunk): array ), $chunk->lines()) ), ), - 'severity' => 'blocker', + 'severity' => 'minor', 'categories' => ['Style'], 'remediation_points' => 50_000, 'location' => [ From fa8f755e0ae13e5f57abeb5deb109b244d1810df Mon Sep 17 00:00:00 2001 From: Kenneth Sills <132029135+Kenneth-Sills@users.noreply.github.com> Date: Mon, 27 May 2024 22:44:27 +0000 Subject: [PATCH 06/10] refactor(formatters): create separate method to generate report string This just makes testing easier, as seen in the existing test classes. --- src/Console/Output/GitlabOutputFormatter.php | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/Console/Output/GitlabOutputFormatter.php b/src/Console/Output/GitlabOutputFormatter.php index 0df119df02..c3188b65e7 100644 --- a/src/Console/Output/GitlabOutputFormatter.php +++ b/src/Console/Output/GitlabOutputFormatter.php @@ -85,6 +85,13 @@ public static function hasSupportForProgressBars(): bool * @return ExitCode::* */ public function report(ErrorAndDiffResult $errorAndDiffResult, Configuration $configuration): int + { + $output = $this->generateReport($errorAndDiffResult, $configuration); + $this->easyCodingStandardStyle->writeln($output); + return $this->exitCodeResolver->resolve($errorAndDiffResult, $configuration); + } + + public function generateReport(ErrorAndDiffResult $errorAndDiffResult, Configuration $configuration): string { $reportedQualityIssues = (! $configuration->isFixer() && $configuration->shouldShowDiffs()) ? merge( @@ -93,10 +100,7 @@ public function report(ErrorAndDiffResult $errorAndDiffResult, Configuration $co ) : $this->generateIssuesForErrors($errorAndDiffResult->getErrors()); - $output = $this->encode($reportedQualityIssues); - - $this->easyCodingStandardStyle->writeln($output); - return $this->exitCodeResolver->resolve($errorAndDiffResult, $configuration); + return $this->encode($reportedQualityIssues); } /** From e13e1bae56b546de11c8087fa510e46fdff26067 Mon Sep 17 00:00:00 2001 From: Kenneth Sills <132029135+Kenneth-Sills@users.noreply.github.com> Date: Mon, 27 May 2024 22:45:03 +0000 Subject: [PATCH 07/10] chore: stop linting test fixtures, which may intentionally contain errors --- ecs.php | 1 + 1 file changed, 1 insertion(+) diff --git a/ecs.php b/ecs.php index 35e89d61bb..46c1908f9b 100644 --- a/ecs.php +++ b/ecs.php @@ -8,4 +8,5 @@ ->withPaths([__DIR__ . '/config', __DIR__ . '/src', __DIR__ . '/tests']) ->withRules([LineLengthFixer::class]) ->withRootFiles() + ->withSkip(['*/Source/*', '*/Fixture/*']) ->withPreparedSets(psr12: true, common: true); From 4f001b4310bc5b46b191a121256985f71872709c Mon Sep 17 00:00:00 2001 From: Kenneth Sills <132029135+Kenneth-Sills@users.noreply.github.com> Date: Mon, 27 May 2024 22:46:39 +0000 Subject: [PATCH 08/10] test(formatters): initial testing for the Gitlab output formatting This is by no means comprehensive, but it at least confirms it respects configurations as expected, formatting is right for trivial cases, and that fingerprints are deterministic. --- .../Fixture/gitlab/errors_and_fixes.json | 54 ++++++ .../Output/Fixture/gitlab/no_issues.json | 1 + .../Output/Fixture/gitlab/only_errors.json | 36 ++++ .../Output/Fixture/gitlab/only_fixes.json | 20 ++ .../gitlab/only_fixes_with_offset.json | 20 ++ .../Output/GitlabOutputFormatterTest.php | 178 ++++++++++++++++++ .../Output/Source/RandomFileWithEdits.php | 11 ++ .../RandomFileWithEditsAndSimpleOffset.php | 13 ++ .../Source/RandomFileWithSimpleOffset.php | 11 ++ 9 files changed, 344 insertions(+) create mode 100644 tests/Console/Output/Fixture/gitlab/errors_and_fixes.json create mode 100644 tests/Console/Output/Fixture/gitlab/no_issues.json create mode 100644 tests/Console/Output/Fixture/gitlab/only_errors.json create mode 100644 tests/Console/Output/Fixture/gitlab/only_fixes.json create mode 100644 tests/Console/Output/Fixture/gitlab/only_fixes_with_offset.json create mode 100644 tests/Console/Output/GitlabOutputFormatterTest.php create mode 100644 tests/Console/Output/Source/RandomFileWithEdits.php create mode 100644 tests/Console/Output/Source/RandomFileWithEditsAndSimpleOffset.php create mode 100644 tests/Console/Output/Source/RandomFileWithSimpleOffset.php diff --git a/tests/Console/Output/Fixture/gitlab/errors_and_fixes.json b/tests/Console/Output/Fixture/gitlab/errors_and_fixes.json new file mode 100644 index 0000000000..06cf036d1b --- /dev/null +++ b/tests/Console/Output/Fixture/gitlab/errors_and_fixes.json @@ -0,0 +1,54 @@ +[ + { + "type": "issue", + "description": "This is a test", + "check_name": "PHP_CodeSniffer\\Standards\\Generic\\Sniffs\\Files\\LineLengthSniff", + "fingerprint": "059d56cd9cfe7bced6fabd5d211e84ec", + "severity": "minor", + "categories": [ + "Style" + ], + "location": { + "path": "/workspaces/easy-coding-standard-with-gitlab-output/tests/Console/Output/Source/RandomFile.php", + "lines": { + "begin": 3, + "end": 3 + } + } + }, + { + "type": "issue", + "description": "This is another test", + "check_name": "PHP_CodeSniffer\\Standards\\Generic\\Sniffs\\Files\\LineLengthSniff", + "fingerprint": "efdcf6305557fb153186ce6776029ae7", + "severity": "minor", + "categories": [ + "Style" + ], + "location": { + "path": "/workspaces/easy-coding-standard-with-gitlab-output/tests/Console/Output/Source/RandomFile.php", + "lines": { + "begin": 5, + "end": 5 + } + } + }, + { + "type": "issue", + "description": "Chunk has fixable errors: LineLengthFixer", + "check_name": "Symplify\\CodingStandard\\Fixer\\LineLength\\LineLengthFixer", + "fingerprint": "c1b8fa798585a0ec85e62e87cd085e6e", + "severity": "minor", + "categories": [ + "Style" + ], + "remediation_points": 50000, + "location": { + "path": "/workspaces/easy-coding-standard-with-gitlab-output/tests/Console/Output/Source/RandomFile.php", + "lines": { + "begin": 4, + "end": 9 + } + } + } +] diff --git a/tests/Console/Output/Fixture/gitlab/no_issues.json b/tests/Console/Output/Fixture/gitlab/no_issues.json new file mode 100644 index 0000000000..fe51488c70 --- /dev/null +++ b/tests/Console/Output/Fixture/gitlab/no_issues.json @@ -0,0 +1 @@ +[] diff --git a/tests/Console/Output/Fixture/gitlab/only_errors.json b/tests/Console/Output/Fixture/gitlab/only_errors.json new file mode 100644 index 0000000000..3536e1599d --- /dev/null +++ b/tests/Console/Output/Fixture/gitlab/only_errors.json @@ -0,0 +1,36 @@ +[ + { + "type": "issue", + "description": "This is a test", + "check_name": "PHP_CodeSniffer\\Standards\\Generic\\Sniffs\\Files\\LineLengthSniff", + "fingerprint": "059d56cd9cfe7bced6fabd5d211e84ec", + "severity": "minor", + "categories": [ + "Style" + ], + "location": { + "path": "/workspaces/easy-coding-standard-with-gitlab-output/tests/Console/Output/Source/RandomFile.php", + "lines": { + "begin": 3, + "end": 3 + } + } + }, + { + "type": "issue", + "description": "This is another test", + "check_name": "PHP_CodeSniffer\\Standards\\Generic\\Sniffs\\Files\\LineLengthSniff", + "fingerprint": "efdcf6305557fb153186ce6776029ae7", + "severity": "minor", + "categories": [ + "Style" + ], + "location": { + "path": "/workspaces/easy-coding-standard-with-gitlab-output/tests/Console/Output/Source/RandomFile.php", + "lines": { + "begin": 5, + "end": 5 + } + } + } +] diff --git a/tests/Console/Output/Fixture/gitlab/only_fixes.json b/tests/Console/Output/Fixture/gitlab/only_fixes.json new file mode 100644 index 0000000000..be0b8d1bb1 --- /dev/null +++ b/tests/Console/Output/Fixture/gitlab/only_fixes.json @@ -0,0 +1,20 @@ +[ + { + "type": "issue", + "description": "Chunk has fixable errors: LineLengthFixer", + "check_name": "Symplify\\CodingStandard\\Fixer\\LineLength\\LineLengthFixer", + "fingerprint": "c1b8fa798585a0ec85e62e87cd085e6e", + "severity": "minor", + "categories": [ + "Style" + ], + "remediation_points": 50000, + "location": { + "path": "/workspaces/easy-coding-standard-with-gitlab-output/tests/Console/Output/Source/RandomFile.php", + "lines": { + "begin": 4, + "end": 9 + } + } + } +] diff --git a/tests/Console/Output/Fixture/gitlab/only_fixes_with_offset.json b/tests/Console/Output/Fixture/gitlab/only_fixes_with_offset.json new file mode 100644 index 0000000000..5de77ab27d --- /dev/null +++ b/tests/Console/Output/Fixture/gitlab/only_fixes_with_offset.json @@ -0,0 +1,20 @@ +[ + { + "type": "issue", + "description": "Chunk has fixable errors: LineLengthFixer", + "check_name": "Symplify\\CodingStandard\\Fixer\\LineLength\\LineLengthFixer", + "fingerprint": "c1b8fa798585a0ec85e62e87cd085e6e", + "severity": "minor", + "categories": [ + "Style" + ], + "remediation_points": 50000, + "location": { + "path": "/workspaces/easy-coding-standard-with-gitlab-output/tests/Console/Output/Source/RandomFile.php", + "lines": { + "begin": 6, + "end": 11 + } + } + } +] diff --git a/tests/Console/Output/GitlabOutputFormatterTest.php b/tests/Console/Output/GitlabOutputFormatterTest.php new file mode 100644 index 0000000000..1f3d181b90 --- /dev/null +++ b/tests/Console/Output/GitlabOutputFormatterTest.php @@ -0,0 +1,178 @@ +gitlabOutputFormatter = $this->make(gitlabOutputFormatter::class); + $this->colorConsoleDiffFormatter = $this->make(ColorConsoleDiffFormatter::class); + $this->differ = $this->make(DifferInterface::class); + } + + /** + * @return array{CodingStandardError[], FileDiff[]} + */ + public function getMockedIssues(string $filePathForChanges = null): array + { + $filePathForOriginal = __DIR__ . '/Source/RandomFile.php'; + + $simulatedErrors = [ + new CodingStandardError(3, 'This is a test', LineLengthSniff::class, $filePathForOriginal), + new CodingStandardError(5, 'This is another test', LineLengthSniff::class, $filePathForOriginal), + ]; + + if ($filePathForChanges === null) { + return [$simulatedErrors, []]; + } + + $diff = $this->differ->diff( + file_get_contents($filePathForOriginal) ?: 'ERROR 1', + file_get_contents($filePathForChanges) ?: 'ERROR 2', + ); + + $simulatedFixes = [ + new FileDiff( + $filePathForOriginal, + $diff, + $this->colorConsoleDiffFormatter->format($diff), + [LineLengthFixer::class], + ), + ]; + + return [$simulatedErrors, $simulatedFixes]; + } + + public function testGracefullyHandlesNoIssues(): void + { + $configuration = new Configuration(); + $filePathForExpectedOutput = __DIR__ . '/Fixture/gitlab/no_issues.json'; + + $errorAndDiffResult = new ErrorAndDiffResult([], [], []); + + $this->assertJsonStringEqualsJsonFile( + $filePathForExpectedOutput, + $this->gitlabOutputFormatter->generateReport($errorAndDiffResult, $configuration), + ); + } + + public function testReportsErrorsInTheRightFormat(): void + { + $configuration = new Configuration(); + $filePathForExpectedOutput = __DIR__ . '/Fixture/gitlab/only_errors.json'; + + [$simulatedErrors] = $this->getMockedIssues(); + + $errorAndDiffResult = new ErrorAndDiffResult($simulatedErrors, [], []); + $output = $this->gitlabOutputFormatter->generateReport($errorAndDiffResult, $configuration); + + $this->assertJsonStringEqualsJsonFile($filePathForExpectedOutput, $output); + } + + public function testReportsFixesInTheRightFormat(): void + { + $configuration = new Configuration(); + $filePathForChanges = __DIR__ . '/Source/RandomFileWithEdits.php'; + $filePathForExpectedOutput = __DIR__ . '/Fixture/gitlab/only_fixes.json'; + + [$_, $simulatedFixes] = $this->getMockedIssues($filePathForChanges); + + $errorAndDiffResult = new ErrorAndDiffResult([], $simulatedFixes, []); + $output = $this->gitlabOutputFormatter->generateReport($errorAndDiffResult, $configuration); + + $this->assertJsonStringEqualsJsonFile($filePathForExpectedOutput, $output); + } + + public function testReportsErrorsAndFixesByDefault(): void + { + $configuration = new Configuration(); + $filePathForChanges = __DIR__ . '/Source/RandomFileWithEdits.php'; + $filePathForExpectedOutput = __DIR__ . '/Fixture/gitlab/errors_and_fixes.json'; + + [$simulatedErrors, $simulatedFixes] = $this->getMockedIssues($filePathForChanges); + + $errorAndDiffResult = new ErrorAndDiffResult($simulatedErrors, $simulatedFixes, []); + $output = $this->gitlabOutputFormatter->generateReport($errorAndDiffResult, $configuration); + + $this->assertJsonStringEqualsJsonFile($filePathForExpectedOutput, $output); + } + + public function testReportsOnlyErrorsWithNoDiffsFlag(): void + { + $configuration = new Configuration(showDiffs: false); + $filePathForChanges = __DIR__ . '/Source/RandomFileWithEdits.php'; + $filePathForExpectedOutput = __DIR__ . '/Fixture/gitlab/only_errors.json'; + + [$simulatedErrors, $simulatedFixes] = $this->getMockedIssues($filePathForChanges); + + $errorAndDiffResult = new ErrorAndDiffResult($simulatedErrors, $simulatedFixes, []); + $output = $this->gitlabOutputFormatter->generateReport($errorAndDiffResult, $configuration); + + $this->assertJsonStringEqualsJsonFile($filePathForExpectedOutput, $output); + } + + public function testReportsOnlyErrorsWithFixFlag(): void + { + $configuration = new Configuration(isFixer: true); + $filePathForChanges = __DIR__ . '/Source/RandomFileWithEdits.php'; + $filePathForExpectedOutput = __DIR__ . '/Fixture/gitlab/only_errors.json'; + + [$simulatedErrors, $simulatedFixes] = $this->getMockedIssues($filePathForChanges); + + $errorAndDiffResult = new ErrorAndDiffResult($simulatedErrors, $simulatedFixes, []); + $output = $this->gitlabOutputFormatter->generateReport($errorAndDiffResult, $configuration); + + $this->assertJsonStringEqualsJsonFile($filePathForExpectedOutput, $output); + } + + // #[Depends('testReportsFixesInTheRightFormat')] + public function testIssueFingerpintsDoNotChangeFromSimpleLineOffsets(): void + { + $configuration = new Configuration(); + $filePathForOriginal = __DIR__ . '/Source/RandomFileWithSimpleOffset.php'; + $mockedFilePathForOriginal = __DIR__ . '/Source/RandomFile.php'; + $filePathForChanges = __DIR__ . '/Source/RandomFileWithEditsAndSimpleOffset.php'; + $filePathForExpectedOutput = __DIR__ . '/Fixture/gitlab/only_fixes_with_offset.json'; + + $diff = $this->differ->diff( + file_get_contents($filePathForOriginal) ?: 'ERROR 1', + file_get_contents($filePathForChanges) ?: 'ERROR 2', + ); + + $simulatedFixes = [ + new FileDiff( + $mockedFilePathForOriginal, + $diff, + $this->colorConsoleDiffFormatter->format($diff), + [LineLengthFixer::class], + ), + ]; + + $errorAndDiffResult = new ErrorAndDiffResult([], $simulatedFixes, []); + $output = $this->gitlabOutputFormatter->generateReport($errorAndDiffResult, $configuration); + + $this->assertJsonStringEqualsJsonFile($filePathForExpectedOutput, $output); + } +} diff --git a/tests/Console/Output/Source/RandomFileWithEdits.php b/tests/Console/Output/Source/RandomFileWithEdits.php new file mode 100644 index 0000000000..705d9d8af6 --- /dev/null +++ b/tests/Console/Output/Source/RandomFileWithEdits.php @@ -0,0 +1,11 @@ + Date: Mon, 27 May 2024 22:52:10 +0000 Subject: [PATCH 09/10] test(formatters): minor test improvement for Gitlab formatter - Uncommenting an attribute I disabled for debugging. - Added an important clarifying comment. --- tests/Console/Output/GitlabOutputFormatterTest.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/Console/Output/GitlabOutputFormatterTest.php b/tests/Console/Output/GitlabOutputFormatterTest.php index 1f3d181b90..ebe989f801 100644 --- a/tests/Console/Output/GitlabOutputFormatterTest.php +++ b/tests/Console/Output/GitlabOutputFormatterTest.php @@ -6,6 +6,7 @@ use PHP_CodeSniffer\Standards\Generic\Sniffs\Files\LineLengthSniff; use PhpCsFixer\Differ\DifferInterface; +use PHPUnit\Framework\Attributes\Depends; use Symplify\CodingStandard\Fixer\LineLength\LineLengthFixer; use Symplify\EasyCodingStandard\Console\Formatter\ColorConsoleDiffFormatter; use Symplify\EasyCodingStandard\Console\Output\GitlabOutputFormatter; @@ -147,7 +148,7 @@ public function testReportsOnlyErrorsWithFixFlag(): void $this->assertJsonStringEqualsJsonFile($filePathForExpectedOutput, $output); } - // #[Depends('testReportsFixesInTheRightFormat')] + #[Depends('testReportsFixesInTheRightFormat')] public function testIssueFingerpintsDoNotChangeFromSimpleLineOffsets(): void { $configuration = new Configuration(); @@ -161,6 +162,7 @@ public function testIssueFingerpintsDoNotChangeFromSimpleLineOffsets(): void file_get_contents($filePathForChanges) ?: 'ERROR 2', ); + // We need to mock the filepath because it's used as fingerprint material. $simulatedFixes = [ new FileDiff( $mockedFilePathForOriginal, From 1439d1df3960d8f6cc8880d36630be7666b0641f Mon Sep 17 00:00:00 2001 From: Kenneth Sills <132029135+Kenneth-Sills@users.noreply.github.com> Date: Mon, 27 May 2024 23:06:59 +0000 Subject: [PATCH 10/10] test(formatters): fix incorrect paths (and fingerprints) for Gitlab formatter These should have been relative, but they were absolute. There's no correctness issue in the formatter itself, it just repeats the paths it's given. But test cases should generally be correct regardless. --- .../Fixture/gitlab/errors_and_fixes.json | 12 +-- .../Output/Fixture/gitlab/only_errors.json | 8 +- .../Output/Fixture/gitlab/only_fixes.json | 4 +- .../gitlab/only_fixes_with_offset.json | 4 +- .../Output/GitlabOutputFormatterTest.php | 100 ++++++++++-------- 5 files changed, 67 insertions(+), 61 deletions(-) diff --git a/tests/Console/Output/Fixture/gitlab/errors_and_fixes.json b/tests/Console/Output/Fixture/gitlab/errors_and_fixes.json index 06cf036d1b..7589c0f93b 100644 --- a/tests/Console/Output/Fixture/gitlab/errors_and_fixes.json +++ b/tests/Console/Output/Fixture/gitlab/errors_and_fixes.json @@ -3,13 +3,13 @@ "type": "issue", "description": "This is a test", "check_name": "PHP_CodeSniffer\\Standards\\Generic\\Sniffs\\Files\\LineLengthSniff", - "fingerprint": "059d56cd9cfe7bced6fabd5d211e84ec", + "fingerprint": "3a2b7d4273d6f644730e17c73cef9295", "severity": "minor", "categories": [ "Style" ], "location": { - "path": "/workspaces/easy-coding-standard-with-gitlab-output/tests/Console/Output/Source/RandomFile.php", + "path": "tests/Console/Output/Source/RandomFile.php", "lines": { "begin": 3, "end": 3 @@ -20,13 +20,13 @@ "type": "issue", "description": "This is another test", "check_name": "PHP_CodeSniffer\\Standards\\Generic\\Sniffs\\Files\\LineLengthSniff", - "fingerprint": "efdcf6305557fb153186ce6776029ae7", + "fingerprint": "84089637032031fa43547c8d854d4ab5", "severity": "minor", "categories": [ "Style" ], "location": { - "path": "/workspaces/easy-coding-standard-with-gitlab-output/tests/Console/Output/Source/RandomFile.php", + "path": "tests/Console/Output/Source/RandomFile.php", "lines": { "begin": 5, "end": 5 @@ -37,14 +37,14 @@ "type": "issue", "description": "Chunk has fixable errors: LineLengthFixer", "check_name": "Symplify\\CodingStandard\\Fixer\\LineLength\\LineLengthFixer", - "fingerprint": "c1b8fa798585a0ec85e62e87cd085e6e", + "fingerprint": "6c9be79241a22740764324c389c90f98", "severity": "minor", "categories": [ "Style" ], "remediation_points": 50000, "location": { - "path": "/workspaces/easy-coding-standard-with-gitlab-output/tests/Console/Output/Source/RandomFile.php", + "path": "tests/Console/Output/Source/RandomFile.php", "lines": { "begin": 4, "end": 9 diff --git a/tests/Console/Output/Fixture/gitlab/only_errors.json b/tests/Console/Output/Fixture/gitlab/only_errors.json index 3536e1599d..c49e61c26a 100644 --- a/tests/Console/Output/Fixture/gitlab/only_errors.json +++ b/tests/Console/Output/Fixture/gitlab/only_errors.json @@ -3,13 +3,13 @@ "type": "issue", "description": "This is a test", "check_name": "PHP_CodeSniffer\\Standards\\Generic\\Sniffs\\Files\\LineLengthSniff", - "fingerprint": "059d56cd9cfe7bced6fabd5d211e84ec", + "fingerprint": "3a2b7d4273d6f644730e17c73cef9295", "severity": "minor", "categories": [ "Style" ], "location": { - "path": "/workspaces/easy-coding-standard-with-gitlab-output/tests/Console/Output/Source/RandomFile.php", + "path": "tests/Console/Output/Source/RandomFile.php", "lines": { "begin": 3, "end": 3 @@ -20,13 +20,13 @@ "type": "issue", "description": "This is another test", "check_name": "PHP_CodeSniffer\\Standards\\Generic\\Sniffs\\Files\\LineLengthSniff", - "fingerprint": "efdcf6305557fb153186ce6776029ae7", + "fingerprint": "84089637032031fa43547c8d854d4ab5", "severity": "minor", "categories": [ "Style" ], "location": { - "path": "/workspaces/easy-coding-standard-with-gitlab-output/tests/Console/Output/Source/RandomFile.php", + "path": "tests/Console/Output/Source/RandomFile.php", "lines": { "begin": 5, "end": 5 diff --git a/tests/Console/Output/Fixture/gitlab/only_fixes.json b/tests/Console/Output/Fixture/gitlab/only_fixes.json index be0b8d1bb1..d96ef53335 100644 --- a/tests/Console/Output/Fixture/gitlab/only_fixes.json +++ b/tests/Console/Output/Fixture/gitlab/only_fixes.json @@ -3,14 +3,14 @@ "type": "issue", "description": "Chunk has fixable errors: LineLengthFixer", "check_name": "Symplify\\CodingStandard\\Fixer\\LineLength\\LineLengthFixer", - "fingerprint": "c1b8fa798585a0ec85e62e87cd085e6e", + "fingerprint": "6c9be79241a22740764324c389c90f98", "severity": "minor", "categories": [ "Style" ], "remediation_points": 50000, "location": { - "path": "/workspaces/easy-coding-standard-with-gitlab-output/tests/Console/Output/Source/RandomFile.php", + "path": "tests/Console/Output/Source/RandomFile.php", "lines": { "begin": 4, "end": 9 diff --git a/tests/Console/Output/Fixture/gitlab/only_fixes_with_offset.json b/tests/Console/Output/Fixture/gitlab/only_fixes_with_offset.json index 5de77ab27d..2892d75711 100644 --- a/tests/Console/Output/Fixture/gitlab/only_fixes_with_offset.json +++ b/tests/Console/Output/Fixture/gitlab/only_fixes_with_offset.json @@ -3,14 +3,14 @@ "type": "issue", "description": "Chunk has fixable errors: LineLengthFixer", "check_name": "Symplify\\CodingStandard\\Fixer\\LineLength\\LineLengthFixer", - "fingerprint": "c1b8fa798585a0ec85e62e87cd085e6e", + "fingerprint": "6c9be79241a22740764324c389c90f98", "severity": "minor", "categories": [ "Style" ], "remediation_points": 50000, "location": { - "path": "/workspaces/easy-coding-standard-with-gitlab-output/tests/Console/Output/Source/RandomFile.php", + "path": "tests/Console/Output/Source/RandomFile.php", "lines": { "begin": 6, "end": 11 diff --git a/tests/Console/Output/GitlabOutputFormatterTest.php b/tests/Console/Output/GitlabOutputFormatterTest.php index ebe989f801..c9288a6df6 100644 --- a/tests/Console/Output/GitlabOutputFormatterTest.php +++ b/tests/Console/Output/GitlabOutputFormatterTest.php @@ -10,6 +10,7 @@ use Symplify\CodingStandard\Fixer\LineLength\LineLengthFixer; use Symplify\EasyCodingStandard\Console\Formatter\ColorConsoleDiffFormatter; use Symplify\EasyCodingStandard\Console\Output\GitlabOutputFormatter; +use Symplify\EasyCodingStandard\FileSystem\StaticRelativeFilePathHelper; use Symplify\EasyCodingStandard\SniffRunner\ValueObject\Error\CodingStandardError; use Symplify\EasyCodingStandard\Testing\PHPUnit\AbstractTestCase; use Symplify\EasyCodingStandard\ValueObject\Configuration; @@ -33,43 +34,10 @@ protected function setUp(): void $this->differ = $this->make(DifferInterface::class); } - /** - * @return array{CodingStandardError[], FileDiff[]} - */ - public function getMockedIssues(string $filePathForChanges = null): array - { - $filePathForOriginal = __DIR__ . '/Source/RandomFile.php'; - - $simulatedErrors = [ - new CodingStandardError(3, 'This is a test', LineLengthSniff::class, $filePathForOriginal), - new CodingStandardError(5, 'This is another test', LineLengthSniff::class, $filePathForOriginal), - ]; - - if ($filePathForChanges === null) { - return [$simulatedErrors, []]; - } - - $diff = $this->differ->diff( - file_get_contents($filePathForOriginal) ?: 'ERROR 1', - file_get_contents($filePathForChanges) ?: 'ERROR 2', - ); - - $simulatedFixes = [ - new FileDiff( - $filePathForOriginal, - $diff, - $this->colorConsoleDiffFormatter->format($diff), - [LineLengthFixer::class], - ), - ]; - - return [$simulatedErrors, $simulatedFixes]; - } - public function testGracefullyHandlesNoIssues(): void { $configuration = new Configuration(); - $filePathForExpectedOutput = __DIR__ . '/Fixture/gitlab/no_issues.json'; + $filePathForExpectedOutput = $this->path('/Fixture/gitlab/no_issues.json'); $errorAndDiffResult = new ErrorAndDiffResult([], [], []); @@ -82,7 +50,7 @@ public function testGracefullyHandlesNoIssues(): void public function testReportsErrorsInTheRightFormat(): void { $configuration = new Configuration(); - $filePathForExpectedOutput = __DIR__ . '/Fixture/gitlab/only_errors.json'; + $filePathForExpectedOutput = $this->path('/Fixture/gitlab/only_errors.json'); [$simulatedErrors] = $this->getMockedIssues(); @@ -95,8 +63,8 @@ public function testReportsErrorsInTheRightFormat(): void public function testReportsFixesInTheRightFormat(): void { $configuration = new Configuration(); - $filePathForChanges = __DIR__ . '/Source/RandomFileWithEdits.php'; - $filePathForExpectedOutput = __DIR__ . '/Fixture/gitlab/only_fixes.json'; + $filePathForChanges = $this->path('/Source/RandomFileWithEdits.php'); + $filePathForExpectedOutput = $this->path('/Fixture/gitlab/only_fixes.json'); [$_, $simulatedFixes] = $this->getMockedIssues($filePathForChanges); @@ -109,8 +77,8 @@ public function testReportsFixesInTheRightFormat(): void public function testReportsErrorsAndFixesByDefault(): void { $configuration = new Configuration(); - $filePathForChanges = __DIR__ . '/Source/RandomFileWithEdits.php'; - $filePathForExpectedOutput = __DIR__ . '/Fixture/gitlab/errors_and_fixes.json'; + $filePathForChanges = $this->path('/Source/RandomFileWithEdits.php'); + $filePathForExpectedOutput = $this->path('/Fixture/gitlab/errors_and_fixes.json'); [$simulatedErrors, $simulatedFixes] = $this->getMockedIssues($filePathForChanges); @@ -123,8 +91,8 @@ public function testReportsErrorsAndFixesByDefault(): void public function testReportsOnlyErrorsWithNoDiffsFlag(): void { $configuration = new Configuration(showDiffs: false); - $filePathForChanges = __DIR__ . '/Source/RandomFileWithEdits.php'; - $filePathForExpectedOutput = __DIR__ . '/Fixture/gitlab/only_errors.json'; + $filePathForChanges = $this->path('/Source/RandomFileWithEdits.php'); + $filePathForExpectedOutput = $this->path('/Fixture/gitlab/only_errors.json'); [$simulatedErrors, $simulatedFixes] = $this->getMockedIssues($filePathForChanges); @@ -137,8 +105,8 @@ public function testReportsOnlyErrorsWithNoDiffsFlag(): void public function testReportsOnlyErrorsWithFixFlag(): void { $configuration = new Configuration(isFixer: true); - $filePathForChanges = __DIR__ . '/Source/RandomFileWithEdits.php'; - $filePathForExpectedOutput = __DIR__ . '/Fixture/gitlab/only_errors.json'; + $filePathForChanges = $this->path('/Source/RandomFileWithEdits.php'); + $filePathForExpectedOutput = $this->path('/Fixture/gitlab/only_errors.json'); [$simulatedErrors, $simulatedFixes] = $this->getMockedIssues($filePathForChanges); @@ -152,10 +120,10 @@ public function testReportsOnlyErrorsWithFixFlag(): void public function testIssueFingerpintsDoNotChangeFromSimpleLineOffsets(): void { $configuration = new Configuration(); - $filePathForOriginal = __DIR__ . '/Source/RandomFileWithSimpleOffset.php'; - $mockedFilePathForOriginal = __DIR__ . '/Source/RandomFile.php'; - $filePathForChanges = __DIR__ . '/Source/RandomFileWithEditsAndSimpleOffset.php'; - $filePathForExpectedOutput = __DIR__ . '/Fixture/gitlab/only_fixes_with_offset.json'; + $filePathForOriginal = $this->path('/Source/RandomFileWithSimpleOffset.php'); + $mockedFilePathForOriginal = $this->path('/Source/RandomFile.php'); + $filePathForChanges = $this->path('/Source/RandomFileWithEditsAndSimpleOffset.php'); + $filePathForExpectedOutput = $this->path('/Fixture/gitlab/only_fixes_with_offset.json'); $diff = $this->differ->diff( file_get_contents($filePathForOriginal) ?: 'ERROR 1', @@ -177,4 +145,42 @@ public function testIssueFingerpintsDoNotChangeFromSimpleLineOffsets(): void $this->assertJsonStringEqualsJsonFile($filePathForExpectedOutput, $output); } + + private function path(string $path): string + { + return StaticRelativeFilePathHelper::resolveFromCwd(__DIR__ . $path); + } + + /** + * @return array{CodingStandardError[], FileDiff[]} + */ + private function getMockedIssues(string $filePathForChanges = null): array + { + $filePathForOriginal = $this->path('/Source/RandomFile.php'); + + $simulatedErrors = [ + new CodingStandardError(3, 'This is a test', LineLengthSniff::class, $filePathForOriginal), + new CodingStandardError(5, 'This is another test', LineLengthSniff::class, $filePathForOriginal), + ]; + + if ($filePathForChanges === null) { + return [$simulatedErrors, []]; + } + + $diff = $this->differ->diff( + file_get_contents($filePathForOriginal) ?: 'ERROR 1', + file_get_contents($filePathForChanges) ?: 'ERROR 2', + ); + + $simulatedFixes = [ + new FileDiff( + $filePathForOriginal, + $diff, + $this->colorConsoleDiffFormatter->format($diff), + [LineLengthFixer::class], + ), + ]; + + return [$simulatedErrors, $simulatedFixes]; + } }