diff --git a/src/Application.php b/src/Application.php index 64f2fa9..934d1f6 100644 --- a/src/Application.php +++ b/src/Application.php @@ -119,7 +119,7 @@ public function run(): int $progress = $this->createProgress($options); try { - $report = new RunCoordinator($progress)->run($runPlan); + $report = new RunCoordinator($progress, collectPerformance: $options['perf'])->runWithMetrics($runPlan); } catch (\Throwable $e) { $this->writeError('Runtime error: ' . $e->getMessage() . PHP_EOL); @@ -134,7 +134,7 @@ public function run(): int $this->write($reporter->generate($report)); - return (int) $report->hasViolations(); + return (int) $report->hasFinalViolations(); } /** diff --git a/src/Fix/FixerException.php b/src/Fix/FixerException.php index 7fe9815..998cf34 100644 --- a/src/Fix/FixerException.php +++ b/src/Fix/FixerException.php @@ -28,11 +28,6 @@ public static function cannotFixInvalidContent(Violation $violation): self )); } - public static function cannotReadFixedContent(): self - { - return new self('Cannot read fixed content when no fix application was attempted.'); - } - public static function invalidFixedXml(string $filePath): self { return new self( diff --git a/src/Progress/ConsoleProgress.php b/src/Progress/ConsoleProgress.php index 899235a..1157038 100644 --- a/src/Progress/ConsoleProgress.php +++ b/src/Progress/ConsoleProgress.php @@ -37,7 +37,12 @@ public function start(int $totalFiles): void return; } - $this->write($this->dim(sprintf('Scanning %d file(s)...', $totalFiles)) . PHP_EOL); + $suffix = $totalFiles === 1 ? 'file' : 'files'; + $formattedTotal = number_format($totalFiles); + + $message = sprintf('Scanning %s %s...', $formattedTotal, $suffix); + + $this->write($this->dim($message) . PHP_EOL); $this->drawBar(0, ''); } diff --git a/src/Report/FileReport.php b/src/Report/FileReport.php index 2ec645c..02ec303 100644 --- a/src/Report/FileReport.php +++ b/src/Report/FileReport.php @@ -9,56 +9,243 @@ final class FileReport { + public private(set) float $totalSniffingTime = 0.0; + + public private(set) float $totalFixingTime = 0.0; + + /** @var array */ + public private(set) array $sniffingTimes = []; + + /** @var array */ + public private(set) array $fixingTimes = []; + + public private(set) int $fixingPasses = 0; + + public private(set) bool $changed = false; + /** @var list */ - public private(set) array $violations = []; + public private(set) array $foundViolations; + + /** @var list */ + public private(set) array $finalViolations; public function __construct( public readonly string $filePath, + private readonly bool $collectPerformance = false, ) { } - public function addViolation(Violation $violation): void + public function markChanged(): void { - $this->violations[] = $violation; + $this->changed = true; } - /** @param list $violations */ - public function addViolations(array $violations): void + /** @throws ReportException if found violations were already added */ + public function addFailedViolation(Violation $violation): void + { + if (isset($this->foundViolations)) { + throw ReportException::foundViolationsAlreadyAdded($this->filePath); + } + + $this->foundViolations = [$violation]; + $this->finalViolations = [$violation]; + } + + /** + * @param list $violations + * @throws ReportException if found violations were already added + */ + public function addFoundViolations(array $violations): void + { + if (isset($this->foundViolations)) { + throw ReportException::foundViolationsAlreadyAdded($this->filePath); + } + + $this->foundViolations = $violations; + $this->finalViolations = $violations; + } + + public function getFoundViolationCount(): int + { + return isset($this->foundViolations) ? count($this->foundViolations) : 0; + } + + /** + * @param list $violations + * @throws ReportException if found violations were not added + */ + public function addFinalViolations(array $violations): void { - foreach ($violations as $violation) { - $this->addViolation($violation); + if (!isset($this->foundViolations)) { + throw ReportException::cannotSetFinalViolationsBeforeFoundViolations($this->filePath); } + + $this->finalViolations = $violations; + } + + public function hasFinalViolations(): bool + { + return isset($this->finalViolations) && $this->finalViolations !== []; + } + + public function getFinalViolationCount(): int + { + return isset($this->finalViolations) ? count($this->finalViolations) : 0; + } + + public function getAppliedFixesCount(): int + { + return $this->fixingPasses > 0 + ? max(0, $this->getFoundViolationCount() - $this->getFinalViolationCount()) + : 0; } - /** @return list */ - public function getViolations(): array + public function getSkippedFixesCount(): int { - return $this->violations; + return $this->fixingPasses > 0 + ? $this->getFinalViolationCount() + : 0; } - public function getViolationCount(): int + public function getFixedErrorCount(): int { - return count($this->violations); + return $this->getFixedSeverityCount(Severity::ERROR); } - public function hasViolations(): bool + public function getFixedWarningCount(): int { - return $this->violations !== []; + return $this->getFixedSeverityCount(Severity::WARNING); + } + + public function recordFixingPass(): void + { + $this->fixingPasses++; } public function getErrorCount(): int { - return array_filter( - $this->violations, - static fn(Violation $v): bool => $v->severity === Severity::ERROR, - ) |> count(...); + return $this->countSeverity(Severity::ERROR); } public function getWarningCount(): int + { + return $this->countSeverity(Severity::WARNING); + } + + public function getInfoCount(): int + { + return $this->countSeverity(Severity::INFO); + } + + /** + * @template T + * @param callable(): T $operation + * @return T + */ + public function measureFixing(callable $operation): mixed + { + if (!$this->collectPerformance) { + return $operation(); + } + + $start = microtime(true); + + try { + return $operation(); + } finally { + $this->totalFixingTime += microtime(true) - $start; + } + } + + /** + * @template T + * @param callable(): T $operation + * @return T + */ + public function measureFixer(string $sniffCode, callable $operation): mixed + { + if (!$this->collectPerformance) { + return $operation(); + } + + $start = microtime(true); + + try { + return $operation(); + } finally { + $this->fixingTimes[$sniffCode] ??= 0.0; + $this->fixingTimes[$sniffCode] += microtime(true) - $start; + } + } + + /** + * @template T + * @param callable(): T $operation + * @return T + */ + public function measureSniffing(callable $operation): mixed + { + if (!$this->collectPerformance) { + return $operation(); + } + + $start = microtime(true); + + try { + return $operation(); + } finally { + $this->totalSniffingTime += microtime(true) - $start; + } + } + + /** + * @template T + * @param callable(): T $operation + * @return T + */ + public function measureSniffer(string $sniffCode, callable $operation): mixed + { + if (!$this->collectPerformance) { + return $operation(); + } + + $start = microtime(true); + + try { + return $operation(); + } finally { + $this->sniffingTimes[$sniffCode] ??= 0.0; + $this->sniffingTimes[$sniffCode] += microtime(true) - $start; + } + } + + private function countSeverity(Severity $severity): int + { + if (!isset($this->finalViolations)) { + return 0; + } + + return $this->countViolationSeverity($this->finalViolations, $severity); + } + + private function getFixedSeverityCount(Severity $severity): int + { + if ($this->fixingPasses === 0 || !isset($this->foundViolations, $this->finalViolations)) { + return 0; + } + + $foundViolations = $this->countViolationSeverity($this->foundViolations, $severity); + $finalViolations = $this->countViolationSeverity($this->finalViolations, $severity); + + return max(0, $foundViolations - $finalViolations); + } + + /** @param list $violations */ + private function countViolationSeverity(array $violations, Severity $severity): int { return array_filter( - $this->violations, - static fn(Violation $v): bool => $v->severity === Severity::WARNING, + $violations, + static fn(Violation $violation): bool => $violation->severity === $severity, ) |> count(...); } } diff --git a/src/Report/Report.php b/src/Report/Report.php index 77e7335..f5b03cf 100644 --- a/src/Report/Report.php +++ b/src/Report/Report.php @@ -8,107 +8,212 @@ final class Report { + public private(set) float $totalTime = 0.0; + /** @var array */ - private array $fileReports = []; + public private(set) array $fileReports = []; - private int $filesScanned = 0; + public function __construct(private readonly bool $collectPerformance = false) + { + } - private float $totalTime = 0.0; + /** + * @template T + * @param callable(): T $operation + * @return T + */ + public function measureWallTime(callable $operation): mixed + { + $start = microtime(true); - /** @var array */ - private array $sniffTimes = []; + try { + return $operation(); + } finally { + $this->totalTime = microtime(true) - $start; + } + } + + public function newFileReport(string $filePath): FileReport + { + return $this->fileReports[$filePath] = new FileReport($filePath, $this->collectPerformance); + } public function addFileReport(FileReport $fileReport): void { $this->fileReports[$fileReport->filePath] = $fileReport; } - public function incrementFilesScanned(): void + public function getScannedFilesCount(): int { - $this->filesScanned++; + return count($this->fileReports); } - public function getFilesScanned(): int + public function getViolatingFilesCount(): int { - return $this->filesScanned; + return array_filter( + $this->fileReports, + static fn(FileReport $fileReport): bool => $fileReport->hasFinalViolations(), + ) |> count(...); } - /** @return array */ - public function getFileReports(): array + public function getChangedFilesCount(): int { - return $this->fileReports; + return array_filter( + $this->fileReports, + static fn(FileReport $fileReport): bool => $fileReport->changed, + ) |> count(...); } - public function getTotalViolations(): int + public function getFoundViolationsCount(): int { - $total = 0; - foreach ($this->fileReports as $fr) { - $total += $fr->getViolationCount(); - } + return array_sum(array_map( + static fn(FileReport $fileReport): int => $fileReport->getFoundViolationCount(), + $this->fileReports, + )); + } - return $total; + public function getAppliedFixesCount(): int + { + return array_sum(array_map( + static fn(FileReport $fileReport): int => $fileReport->getAppliedFixesCount(), + $this->fileReports, + )); } - public function getTotalErrors(): int + public function getSkippedFixesCount(): int { - $total = 0; - foreach ($this->fileReports as $fr) { - $total += $fr->getErrorCount(); - } + return array_sum(array_map( + static fn(FileReport $fileReport): int => $fileReport->getSkippedFixesCount(), + $this->fileReports, + )); + } - return $total; + public function getFixedErrorCount(): int + { + return array_sum(array_map( + static fn(FileReport $fileReport): int => $fileReport->getFixedErrorCount(), + $this->fileReports, + )); } - public function getTotalWarnings(): int + public function getFixedWarningCount(): int { - $total = 0; - foreach ($this->fileReports as $fr) { - $total += $fr->getWarningCount(); - } + return array_sum(array_map( + static fn(FileReport $fileReport): int => $fileReport->getFixedWarningCount(), + $this->fileReports, + )); + } - return $total; + public function hasFixingResults(): bool + { + return array_any($this->fileReports, static fn(FileReport $fileReport): bool => $fileReport->fixingPasses > 0); } - public function hasViolations(): bool + public function getFixingPassesCount(): int { - return $this->getTotalViolations() > 0; + return array_sum(array_map( + static fn(FileReport $fileReport): int => $fileReport->fixingPasses, + $this->fileReports, + )); } - /** @return list */ - public function getAllViolations(): array + public function getTotalFixingTime(): float + { + return array_sum(array_map( + static fn(FileReport $fileReport): float => $fileReport->totalFixingTime, + $this->fileReports, + )); + } + + public function getTotalSniffingTime(): float + { + return array_sum(array_map( + static fn(FileReport $fileReport): float => $fileReport->totalSniffingTime, + $this->fileReports, + )); + } + + /** @return array */ + public function getFixingTimes(): array { - $all = []; + $fixingTimes = []; + foreach ($this->fileReports as $fileReport) { - foreach ($fileReport->getViolations() as $violation) { - $all[] = $violation; + foreach ($fileReport->fixingTimes as $sniffCode => $time) { + $fixingTimes[$sniffCode] ??= 0.0; + $fixingTimes[$sniffCode] += $time; } } - return $all; + return $fixingTimes; } - public function setTotalTime(float $time): void + /** @return array */ + public function getSniffingTimes(): array { - $this->totalTime = $time; + $sniffingTimes = []; + + foreach ($this->fileReports as $fileReport) { + foreach ($fileReport->sniffingTimes as $sniffCode => $time) { + $sniffingTimes[$sniffCode] ??= 0.0; + $sniffingTimes[$sniffCode] += $time; + } + } + + return $sniffingTimes; } - public function addSniffTime(string $sniffClass, float $time): void + public function getTotalFinalViolationCount(): int { - if (!isset($this->sniffTimes[$sniffClass])) { - $this->sniffTimes[$sniffClass] = 0.0; - } + return array_sum(array_map( + static fn(FileReport $fileReport): int => $fileReport->getFinalViolationCount(), + $this->fileReports, + )); + } - $this->sniffTimes[$sniffClass] += $time; + public function getTotalErrorLevelViolationCount(): int + { + return array_sum(array_map( + static fn(FileReport $fileReport): int => $fileReport->getErrorCount(), + $this->fileReports, + )); } - public function getTotalTime(): float + public function getTotalWarningLevelViolationCount(): int { - return $this->totalTime; + return array_sum(array_map( + static fn(FileReport $fileReport): int => $fileReport->getWarningCount(), + $this->fileReports, + )); } - /** @return array */ - public function getSniffTimes(): array + /** @api not implemented */ + public function getTotalInfoLevelViolationCount(): int + { + return array_sum(array_map( + static fn(FileReport $fileReport): int => $fileReport->getInfoCount(), + $this->fileReports, + )); + } + + public function hasFinalViolations(): bool + { + return $this->getTotalFinalViolationCount() > 0; + } + + /** @return list */ + public function getAllViolations(): array { - return $this->sniffTimes; + $violations = []; + + foreach ($this->fileReports as $fileReport) { + if (!$fileReport->hasFinalViolations()) { + continue; + } + + array_push($violations, ...$fileReport->finalViolations); + } + + return $violations; } } diff --git a/src/Report/ReportException.php b/src/Report/ReportException.php new file mode 100644 index 0000000..07e0d36 --- /dev/null +++ b/src/Report/ReportException.php @@ -0,0 +1,18 @@ +appendChild($root); $comment = $dom->createComment( - sprintf(' total runtime: %.3fs ', $report->getTotalTime()) + sprintf(' total runtime: %.3fs ', $report->totalTime) ); $root->appendChild($comment); - foreach ($report->getFileReports() as $fileReport) { - if (!$fileReport->hasViolations()) { + foreach ($report->fileReports as $fileReport) { + if (!$fileReport->hasFinalViolations()) { continue; } $fileNode = $dom->createElement('file'); $fileNode->setAttribute('name', RelativePath::fromWorkingDirectory($fileReport->filePath)); - foreach ($fileReport->getViolations() as $violation) { + foreach ($fileReport->finalViolations as $violation) { $errorNode = $dom->createElement('error'); $errorNode->setAttribute('line', (string) $violation->rangeOne()->line); $errorNode->setAttribute('severity', $violation->severity->value); diff --git a/src/Report/Reporter/ConsoleReporter.php b/src/Report/Reporter/ConsoleReporter.php index c7fdb1e..39c2f6b 100644 --- a/src/Report/Reporter/ConsoleReporter.php +++ b/src/Report/Reporter/ConsoleReporter.php @@ -23,8 +23,8 @@ public function generate(Report $report): string { $output = ''; - foreach ($report->getFileReports() as $fileReport) { - if (!$fileReport->hasViolations()) { + foreach ($report->fileReports as $fileReport) { + if (!$fileReport->hasFinalViolations()) { continue; } @@ -34,7 +34,7 @@ public function generate(Report $report): string $output .= $this->bold('FILE: ' . $filePath) . PHP_EOL; $output .= str_repeat('-', min(80, 6 + strlen($filePath))) . PHP_EOL; - foreach ($fileReport->getViolations() as $violation) { + foreach ($fileReport->finalViolations as $violation) { $output .= sprintf( ' %4d | %s | %s | %s', $violation->rangeOne()->line, @@ -60,66 +60,94 @@ public function generate(Report $report): string private function buildSummary(Report $report): string { - $files = $report->getFilesScanned(); - $errors = $report->getTotalErrors(); - $warnings = $report->getTotalWarnings(); - $total = $report->getTotalViolations(); - $time = $report->getTotalTime(); - - $timeLine = sprintf('Total runtime: %.3fs', $time); - - if ($total === 0) { - return $this->green( - sprintf( - 'OK -- %d file(s) scanned, no violations found.', - $files, - ) - ) . PHP_EOL . $this->dim($timeLine); + $lines = $report->hasFixingResults() + ? $this->buildFixingOutcome($report) + : [$this->buildSniffingOutcome($report)]; + + $lines[] = $this->dim(sprintf('Total runtime: %.3fs', $report->totalTime)); + + return implode(PHP_EOL, $lines); + } + + /** @return non-empty-list */ + private function buildFixingOutcome(Report $report): array + { + $lines = [ + $this->green($this->formatFixedSummary($report)), + ]; + + if ($report->hasFinalViolations()) { + $lines[] = $this->red($this->formatFinalSummary($report, 'REMAINING')); + } + + return $lines; + } + + private function buildSniffingOutcome(Report $report): string + { + if ($report->hasFinalViolations()) { + return $this->red($this->formatFinalSummary($report, 'FOUND')); } - return $this->red( - sprintf( - 'FOUND %d violation(s) (%d error(s), %d warning(s)) in %d file(s).', - $total, - $errors, - $warnings, - count($report->getFileReports()), - ) - ) . PHP_EOL . $this->dim($timeLine); + return $this->green(sprintf( + 'OK -- %s scanned, no violations found.', + $this->formatCount('file', $report->getScannedFilesCount()), + )); } private function buildPerformance(Report $report): string { - $totalTime = $report->getTotalTime(); - $sniffTimes = $report->getSniffTimes(); + $totalTime = $report->totalTime; + $rows = $this->collectPerformanceRows($report); - if ($totalTime <= 0.0 || $sniffTimes === []) { + if ($totalTime <= 0.0 || $rows === []) { return $this->dim('No performance data available.'); } - // Sort slowest first - arsort($sniffTimes); - - $output = $this->bold('PERFORMANCE') . PHP_EOL; - $output .= str_repeat('-', 40) . PHP_EOL; + $nameWidth = 40; + foreach (array_keys($rows) as $sniffCode) { + $nameWidth = max($nameWidth, strlen($sniffCode)); + } - $output .= sprintf( - ' Total runtime: %.3fs', - $totalTime - ) . PHP_EOL . PHP_EOL; + $header = sprintf(' %-*s %16s %16s', $nameWidth, '', 'Sniffing', 'Fixing'); + $lines = [$this->bold('PERFORMANCE'), str_repeat('-', strlen($header)), $this->bold($header)]; + + foreach ($rows as $sniffCode => $times) { + $lines[] = sprintf( + ' %-*s %16s %16s', + $nameWidth, + $sniffCode, + $this->formatPerformanceCell($times['sniffing'], $totalTime), + $this->formatPerformanceCell($times['fixing'], $totalTime), + ); + } - foreach ($sniffTimes as $sniff => $time) { - $percent = ($time / $totalTime) * 100; + return implode(PHP_EOL, $lines); + } - $output .= sprintf( - ' %-40s %6.3fs (%5.1f%%)', - $sniff, - $time, - $percent, - ) . PHP_EOL; + /** @return array */ + private function collectPerformanceRows(Report $report): array + { + $sniffingTimes = $report->getSniffingTimes(); + $fixingTimes = $report->getFixingTimes(); + $rows = []; + + foreach ($sniffingTimes + $fixingTimes as $sniffCode => $_) { + $rows[$sniffCode] = [ + 'sniffing' => $sniffingTimes[$sniffCode] ?? null, + 'fixing' => $fixingTimes[$sniffCode] ?? null, + ]; } - return $output; + // Sort slowest first. + uasort( + $rows, + static fn(array $left, array $right): int => + (($right['sniffing'] ?? 0.0) + ($right['fixing'] ?? 0.0)) + <=> (($left['sniffing'] ?? 0.0) + ($left['fixing'] ?? 0.0)), + ); + + return $rows; } private function formatSeverity(Severity $severity): string @@ -131,6 +159,54 @@ private function formatSeverity(Severity $severity): string }; // @codeCoverageIgnore } + private function formatPerformanceCell(?float $time, float $totalTime): string + { + return $time === null + ? '' + : sprintf('%6.3fs (%5.1f%%)', $time, ($time / $totalTime) * 100); + } + + private function formatFixedSummary(Report $report): string + { + $files = $report->getChangedFilesCount(); + $passes = $report->getFixingPassesCount(); + $ending = $report->hasFinalViolations() + ? '.' + : ', no violations remaining.'; + + return sprintf( + 'FIXED %s [%s, %s] in %s%s%s', + $this->formatCount('violation', $report->getAppliedFixesCount()), + $this->formatCount('error', $report->getFixedErrorCount()), + $this->formatCount('warning', $report->getFixedWarningCount()), + $this->formatCount('file', $files), + $files > 0 && $passes > $files + ? sprintf(' (%s passes)', number_format($passes)) + : '', + $ending, + ); + } + + private function formatFinalSummary(Report $report, string $label): string + { + // todo: how about info level? + return sprintf( + '%s %s [%s, %s] in %s.', + $label, + $this->formatCount('violation', $report->getTotalFinalViolationCount()), + $this->formatCount('error', $report->getTotalErrorLevelViolationCount()), + $this->formatCount('warning', $report->getTotalWarningLevelViolationCount()), + $this->formatCount('file', $report->getViolatingFilesCount()), + ); + } + + private function formatCount(string $singular, int $count): string + { + $suffix = $count === 1 ? $singular : $singular . 's'; + + return sprintf('%s %s', number_format($count), $suffix); + } + private function bold(string $text): string { return $this->wrap($text, '1'); diff --git a/src/Report/Reporter/JsonReporter.php b/src/Report/Reporter/JsonReporter.php index eac2dd5..42aeb96 100644 --- a/src/Report/Reporter/JsonReporter.php +++ b/src/Report/Reporter/JsonReporter.php @@ -13,24 +13,30 @@ public function generate(Report $report): string { $data = [ 'totals' => [ - 'files_scanned' => $report->getFilesScanned(), - 'violations' => $report->getTotalViolations(), - 'errors' => $report->getTotalErrors(), - 'warnings' => $report->getTotalWarnings(), + 'files_scanned' => $report->getScannedFilesCount(), + 'violations' => $report->getTotalFinalViolationCount(), + 'errors' => $report->getTotalErrorLevelViolationCount(), + 'warnings' => $report->getTotalWarningLevelViolationCount(), ], 'files' => [], + 'fixing' => [ + 'files_changed' => $report->getChangedFilesCount(), + 'fixes_applied' => $report->getAppliedFixesCount(), + 'fixes_skipped' => $report->getSkippedFixesCount(), + 'fixing_passes' => $report->getFixingPassesCount(), + ], 'performance' => [ - 'total_runtime_seconds' => $report->getTotalTime(), + 'total_runtime_seconds' => $report->totalTime, ], ]; - foreach ($report->getFileReports() as $fileReport) { - if (!$fileReport->hasViolations()) { + foreach ($report->fileReports as $fileReport) { + if (!$fileReport->hasFinalViolations()) { continue; } $violations = []; - foreach ($fileReport->getViolations() as $violation) { + foreach ($fileReport->finalViolations as $violation) { $violations[] = [ 'line' => $violation->rangeOne()->line, 'severity' => $violation->severity, diff --git a/src/Runner/RunCoordinator.php b/src/Runner/RunCoordinator.php index c2de7ff..c17b0de 100644 --- a/src/Runner/RunCoordinator.php +++ b/src/Runner/RunCoordinator.php @@ -8,8 +8,8 @@ use DocbookCS\Fix\FixerException; use DocbookCS\Progress\NullProgress; use DocbookCS\Progress\ProgressInterface; -use DocbookCS\Report\FileReport; use DocbookCS\Report\Report; +use DocbookCS\Report\ReportException; use DocbookCS\Sniff\SniffInterface; use DocbookCS\Source\File; use DocbookCS\Violation\Violation; @@ -18,60 +18,70 @@ final class RunCoordinator { private ProgressInterface $progress; - public function __construct(?ProgressInterface $progress = null) - { + public function __construct( + ?ProgressInterface $progress = null, + private readonly bool $collectPerformance = false, + ) { $this->progress = $progress ?? new NullProgress(); } /** * @throws \InvalidArgumentException if an internal violation is inconsistent - * @throws \RuntimeException if a sniff class cannot be found or does not - * implement SniffInterface. + * @throws ReportException if violations are added in an invalid order + * @throws \RuntimeException if a sniff class cannot be found or does not implement SniffInterface. * @throws FixerException */ - public function run(RunPlan $plan): Report + public function runWithMetrics(RunPlan $plan): Report { - $startTime = microtime(true); + $report = new Report($this->collectPerformance); - $sniffs = $this->instantiateSniffs($plan->sniffs, $plan->mode); + return $report->measureWallTime( + fn(): Report => $this->run($plan, $report), + ); + } - $report = new Report(); - $preprocessor = new EntityPreprocessor($plan->entities); - $processor = new XmlFileProcessor($sniffs, $preprocessor, $report); + /** + * @throws \InvalidArgumentException if an internal violation is inconsistent + * @throws ReportException if violations are added in an invalid order + * @throws \RuntimeException if a sniff class cannot be found or does not implement SniffInterface. + * @throws FixerException + */ + private function run(RunPlan $plan, Report $report): Report + { + $processor = new XmlFileProcessor(new XmlSniffRunner( + $plan->mode, + $this->instantiateSniffs($plan->sniffs), + new EntityPreprocessor($plan->entities), + )); $this->progress->start(count($plan->targets)); foreach ($plan->targets as $filePath => $fileChange) { - $report->incrementFilesScanned(); + $fileReport = $report->newFileReport($filePath); $content = @file_get_contents($filePath); if ($content === false) { - $fileReport = new FileReport($filePath); - $fileReport->addViolation(Violation::fromFileReadFailure($filePath)); - } else { - $file = new File($filePath, $content); - $result = $processor->process($file, $fileChange); - $fileReport = $result->fileReport; - - if ($result->isModified() && @file_put_contents($filePath, $result->fixedContent()) === false) { - throw FixerException::cannotPersist($filePath); - } + $fileReport->addFailedViolation(Violation::fromFileReadFailure($filePath)); + + $this->progress->advance($filePath, $fileReport->getFinalViolationCount()); + continue; } - $violationCount = $fileReport->getViolationCount(); + $file = new File($filePath, $content); + $scope = RunScope::fromFileAndFileChange($file, $fileChange); + + $fixedFile = $processor->process($file, $fileReport, $scope); - if ($fileReport->hasViolations()) { - $report->addFileReport($fileReport); + if ($fixedFile !== null && @file_put_contents($filePath, $fixedFile->content) === false) { + throw FixerException::cannotPersist($filePath); } - $this->progress->advance($filePath, $violationCount); + $this->progress->advance($filePath, $fileReport->getFinalViolationCount()); } $this->progress->finish(); - $report->setTotalTime(microtime(true) - $startTime); - return $report; } @@ -80,7 +90,7 @@ public function run(RunPlan $plan): Report * @return list * @throws \RuntimeException if a sniff class cannot be found or does not implement SniffInterface. */ - private function instantiateSniffs(array $entries, RunMode $mode): array + private function instantiateSniffs(array $entries): array { $sniffs = []; @@ -93,7 +103,7 @@ private function instantiateSniffs(array $entries, RunMode $mode): array ); } - $instance = new $className($mode); + $instance = new $className(); if (!$instance instanceof SniffInterface) { throw new \RuntimeException( diff --git a/src/Runner/XmlFileProcessor.php b/src/Runner/XmlFileProcessor.php index dd5e2ce..477c77c 100644 --- a/src/Runner/XmlFileProcessor.php +++ b/src/Runner/XmlFileProcessor.php @@ -4,15 +4,9 @@ namespace DocbookCS\Runner; -use DocbookCS\Diff\FileChange; -use DocbookCS\Fix\Fix; -use DocbookCS\Fix\FixApplier; -use DocbookCS\Fix\FixPlan; use DocbookCS\Fix\FixerException; use DocbookCS\Report\FileReport; -use DocbookCS\Report\Report; -use DocbookCS\Sniff\Fixable; -use DocbookCS\Sniff\SniffInterface; +use DocbookCS\Report\ReportException; use DocbookCS\Source\File; use DocbookCS\Violation\Violation; @@ -20,143 +14,70 @@ { private const int MAX_FIX_PASSES = 20; - /** @var list */ - private array $sniffs; - - private EntityPreprocessor $preprocessor; - - private Report $report; - - private ViolationScopeFilter $violationScopeFilter; - - /** @param list $sniffs */ public function __construct( - array $sniffs, - ?EntityPreprocessor $preprocessor = null, - ?Report $report = null, + private XmlSniffRunner $xmlSniffRunner, + private XmlFixRunner $xmlFixRunner = new XmlFixRunner(), ) { - $this->sniffs = $sniffs; - $this->preprocessor = $preprocessor ?? new EntityPreprocessor([]); - $this->report = $report ?? new Report(); - $this->violationScopeFilter = new ViolationScopeFilter(); } /** * @throws FixerException * @throws \InvalidArgumentException if an internal violation is inconsistent + * @throws ReportException if violations are added in an invalid order */ - public function process(File $initialFile, ?FileChange $fileChange = null): XmlProcessingResult + public function process(File $file, FileReport $fileReport, RunScope $scope): ?File { - $fileReport = new FileReport($initialFile->path); - $currentFile = $initialFile; - $scope = RunScope::fromFileAndFileChange($initialFile, $fileChange); - $seenContentHashes = [hash('sha256', $currentFile->content) => true]; - $fixPasses = 0; + $seenContentHashes = [hash('sha256', $file->content) => true]; + $initialViolations = null; + $changed = false; while (true) { - $passReport = new FileReport($currentFile->path); + $sniffingResult = $this->xmlSniffRunner->runWithMetrics($file, $fileReport, $scope); - $document = $this->parseXml($currentFile, $passReport); - if ($document === null) { - if ($currentFile->content !== $initialFile->content) { - throw FixerException::invalidFixedXml($currentFile->path); + if ($sniffingResult instanceof \LibXMLError) { + if ($changed) { + throw FixerException::invalidFixedXml($file->path); } - break; + $fileReport->addFailedViolation(Violation::fromXmlParseError($file->path, $sniffingResult)); + return null; } - $fixes = $this->runSniffs($document, $currentFile, $passReport, $scope); + [$passViolations, $fixerBatches] = $sniffingResult; + + $initialViolations ??= $passViolations; - if ($fixes === []) { + if ($fixerBatches === []) { break; } - $fixResult = new FixApplier()->apply($currentFile, $fixes); + $fixResult = $this->xmlFixRunner->runWithMetrics($file, $fileReport, $fixerBatches); if ($fixResult->applied === 0) { break; } - $fixPasses++; $fixedContentHash = hash('sha256', $fixResult->file->content); - if ( - $fixPasses > self::MAX_FIX_PASSES - || $fixResult->file->content === $currentFile->content - || isset($seenContentHashes[$fixedContentHash]) - ) { - throw FixerException::didNotConverge($currentFile->path); + if ($fileReport->fixingPasses > self::MAX_FIX_PASSES || isset($seenContentHashes[$fixedContentHash])) { + throw FixerException::didNotConverge($file->path); } $seenContentHashes[$fixedContentHash] = true; $scope = $scope->after($fixResult->appliedFixes); - $currentFile = $fixResult->file; - } - - $fileReport->addViolations($passReport->getViolations()); - - return new XmlProcessingResult( - fileReport: $fileReport, - initialFile: $initialFile, - currentFile: $currentFile, - ); - } - - /** - * @return list - * @throws FixerException - */ - private function runSniffs(\DOMDocument $document, File $file, FileReport $fileReport, RunScope $scope): array - { - $fixes = []; - - foreach ($this->sniffs as $sniff) { - $start = microtime(true); - - $sniffViolations = $sniff->process($document, $file); - - $this->report->addSniffTime($sniff::getCode(), microtime(true) - $start); - - $relevantViolations = $this->violationScopeFilter->filter($sniffViolations, $document, $file, $scope); - - $fileReport->addViolations($relevantViolations); - - if (!$sniff->mode->isFixMode() || !$sniff instanceof Fixable) { - continue; - } - - $fixer = new ($sniff::getFixerClassName()); - - foreach ($relevantViolations as $violation) { - $fixes[] = $fixer->process($violation); - } + $file = $fixResult->file; + $changed = true; } - return $fixes; - } - - /** @throws \InvalidArgumentException if an internal violation is inconsistent */ - private function parseXml(File $file, FileReport $fileReport): ?\DOMDocument - { - $content = $this->preprocessor->processForParsing($file->content); - - $previousUseErrors = libxml_use_internal_errors(true); - $document = new \DOMDocument(); - $document->preserveWhiteSpace = true; - - // LIBXML_NONET prevents network access. - // No LIBXML_DTDLOAD needed since we stripped the DOCTYPE. - $loaded = $document->loadXML($content, LIBXML_NONET); + $fileReport->addFoundViolations($initialViolations); - $errors = libxml_get_errors(); - libxml_clear_errors(); - libxml_use_internal_errors($previousUseErrors); - - if (!$loaded) { - $fileReport->addViolation(Violation::fromXmlParseError($file->path, $errors[0] ?? null)); + if (!$changed) { return null; } - return $document; + $fileReport->addFinalViolations($passViolations); + $fileReport->markChanged(); + + return $file; } } diff --git a/src/Runner/XmlFixRunner.php b/src/Runner/XmlFixRunner.php new file mode 100644 index 0000000..d8d9aab --- /dev/null +++ b/src/Runner/XmlFixRunner.php @@ -0,0 +1,63 @@ +, + * violations: list + * }> $fixerBatches + */ + public function runWithMetrics(File $file, FileReport $fileReport, array $fixerBatches): FixResult + { + return $fileReport->measureFixing( + fn(): FixResult => $this->run($file, $fileReport, $fixerBatches), + ); + } + + /** + * @param list, + * violations: list + * }> $fixerBatches + */ + private function run(File $file, FileReport $fileReport, array $fixerBatches): FixResult + { + $fixes = []; + $fileReport->recordFixingPass(); + + foreach ($fixerBatches as $batch) { + $batchFixes = $fileReport->measureFixer( + $batch['sniffCode'], + static function () use ($batch): array { + $fixes = []; + $fixerClass = $batch['fixerClass']; + $fixer = new $fixerClass(); + + foreach ($batch['violations'] as $violation) { + $fixes[] = $fixer->process($violation); + } + + return $fixes; + }, + ); + + array_push($fixes, ...$batchFixes); + } + + return new FixApplier()->apply($file, $fixes); + } +} diff --git a/src/Runner/XmlProcessingResult.php b/src/Runner/XmlProcessingResult.php deleted file mode 100644 index a586e70..0000000 --- a/src/Runner/XmlProcessingResult.php +++ /dev/null @@ -1,34 +0,0 @@ -initialFile->content !== $this->currentFile->content; - } - - /** @throws FixerException */ - public function fixedContent(): string - { - if (!$this->isModified()) { - throw FixerException::cannotReadFixedContent(); - } - - return $this->currentFile->content; - } -} diff --git a/src/Runner/XmlSniffRunner.php b/src/Runner/XmlSniffRunner.php new file mode 100644 index 0000000..9f1074d --- /dev/null +++ b/src/Runner/XmlSniffRunner.php @@ -0,0 +1,120 @@ + $sniffs */ + public function __construct( + private RunMode $mode, + private array $sniffs, + private EntityPreprocessor $preprocessor = new EntityPreprocessor(), + private ViolationScopeFilter $violationFilter = new ViolationScopeFilter() + ) { + } + + /** + * @return array{ + * list, + * list, + * violations: list + * }> + * }|\LibXMLError + * @throws \InvalidArgumentException if an internal violation is inconsistent + */ + public function runWithMetrics(File $file, FileReport $fileReport, RunScope $scope): array|\LibXMLError + { + return $fileReport->measureSniffing( + fn(): array|\LibXMLError => $this->run($file, $fileReport, $scope), + ); + } + + /** + * @return array{ + * list, + * list, + * violations: list + * }> + * }|\LibXMLError + * @throws \InvalidArgumentException if an internal violation is inconsistent + */ + private function run(File $file, FileReport $fileReport, RunScope $scope): array|\LibXMLError + { + if (($document = $this->parseXml($file)) instanceof \LibXMLError) { + return $document; + } + + $violations = []; + /** @var list, violations: list}> $fixerBatches */ + $fixerBatches = []; + + foreach ($this->sniffs as $sniffer) { + $sniffViolations = $fileReport->measureSniffer( + $sniffer::getCode(), + // run sniffers and filter violations + fn() => $this->violationFilter->filter( + $sniffer->process($document, $file), + $document, + $file, + $scope, + ), + ); + + array_push($violations, ...$sniffViolations); + + if ($sniffViolations === [] || !$this->mode->isFixMode() || !$sniffer instanceof Fixable) { + continue; + } + + $fixerBatches[] = [ + 'sniffCode' => $sniffer::getCode(), + 'fixerClass' => $sniffer::getFixerClassName(), + 'violations' => $sniffViolations, + ]; + } + + return [$violations, $fixerBatches]; + } + + /** @throws \InvalidArgumentException if the internal violation is inconsistent */ + private function parseXml(File $file): \DOMDocument|\LibXMLError + { + $content = $this->preprocessor->processForParsing($file->content); + + $previousUseErrors = libxml_use_internal_errors(true); + libxml_clear_errors(); + + try { + $document = new \DOMDocument(); + $document->preserveWhiteSpace = true; + + // LIBXML_NONET prevents network access. + // No LIBXML_DTDLOAD needed since we stripped the DOCTYPE. + $loaded = $document->loadXML($content, LIBXML_NONET); + + if ($loaded) { + return $document; + } + + $error = libxml_get_errors()[0]; + } finally { + libxml_clear_errors(); + libxml_use_internal_errors($previousUseErrors); + } + + return $error; + } +} diff --git a/src/Sniff/AbstractSniff.php b/src/Sniff/AbstractSniff.php index aed3eca..f27338e 100644 --- a/src/Sniff/AbstractSniff.php +++ b/src/Sniff/AbstractSniff.php @@ -5,7 +5,6 @@ namespace DocbookCS\Sniff; use DocbookCS\Runner\EntityExpansionMarker; -use DocbookCS\Runner\RunMode; use DocbookCS\Source\File; use DocbookCS\Violation\Severity; use DocbookCS\Violation\SourceRange; @@ -24,11 +23,6 @@ abstract class AbstractSniff implements SniffInterface /** @var array */ protected array $properties = []; - public function __construct( - public RunMode $mode = RunMode::Sniff, - ) { - } - /** @throws \InvalidArgumentException if a configured severity is invalid */ public function setProperty(string $name, string $value): void { diff --git a/src/Sniff/ExceptionNameSniff.php b/src/Sniff/ExceptionNameSniff.php index bcd9f19..b3a5a99 100644 --- a/src/Sniff/ExceptionNameSniff.php +++ b/src/Sniff/ExceptionNameSniff.php @@ -17,8 +17,8 @@ */ final class ExceptionNameSniff extends AbstractSniff implements Fixable { - private const string REPORTING_MESSAGE = '"%s" is wrapped in but should use .'; private const string ELEMENT_NAME = 'classname'; + private const string REPORTING_MESSAGE = '"%s" is wrapped in but should use .'; /** * Default suffixes that indicate the class is an exception or error. diff --git a/src/Sniff/SniffInterface.php b/src/Sniff/SniffInterface.php index 17bc172..bb7743e 100644 --- a/src/Sniff/SniffInterface.php +++ b/src/Sniff/SniffInterface.php @@ -4,7 +4,6 @@ namespace DocbookCS\Sniff; -use DocbookCS\Runner\RunMode; use DocbookCS\Source\File; use DocbookCS\Violation\Violation; @@ -14,10 +13,6 @@ */ interface SniffInterface { - public RunMode $mode { get; } - - public function __construct(RunMode $mode); - /** * Unique, human-readable code for this sniff (e.g. "DocbookCS.MySniff"). */ diff --git a/tests/Unit/ApplicationInputTest.php b/tests/Unit/ApplicationInputTest.php index 3d7a272..1420f2e 100644 --- a/tests/Unit/ApplicationInputTest.php +++ b/tests/Unit/ApplicationInputTest.php @@ -28,6 +28,7 @@ use DocbookCS\Runner\RunPlanner; use DocbookCS\Runner\RunScopeResolver; use DocbookCS\Runner\XmlFileProcessor; +use DocbookCS\Runner\XmlSniffRunner; use DocbookCS\Sniff\AbstractSniff; use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\Attributes\Test; @@ -61,6 +62,7 @@ UsesClass(SniffEntry::class), UsesClass(UpstreamResolver::class), UsesClass(XmlFileProcessor::class), + UsesClass(XmlSniffRunner::class), ] final class ApplicationInputTest extends TestCase { diff --git a/tests/Unit/ApplicationTest.php b/tests/Unit/ApplicationTest.php index bc581fd..197c115 100644 --- a/tests/Unit/ApplicationTest.php +++ b/tests/Unit/ApplicationTest.php @@ -30,15 +30,15 @@ use DocbookCS\Report\Reporter\ConsoleReporter; use DocbookCS\Report\Reporter\JsonReporter; use DocbookCS\Runner\EntityPreprocessor; -use DocbookCS\Runner\RunMode; +use DocbookCS\Runner\XmlFileProcessor; use DocbookCS\Runner\RunCoordinator; +use DocbookCS\Runner\RunMode; use DocbookCS\Runner\RunPlan; use DocbookCS\Runner\RunPlanner; use DocbookCS\Runner\RunScope; use DocbookCS\Runner\RunScopeResolver; use DocbookCS\Runner\ViolationScopeFilter; -use DocbookCS\Runner\XmlFileProcessor; -use DocbookCS\Runner\XmlProcessingResult; +use DocbookCS\Runner\XmlSniffRunner; use DocbookCS\Sniff\ExceptionNameSniff; use DocbookCS\Source\File; use PHPUnit\Framework\Attributes\CoversClass; @@ -68,7 +68,7 @@ CoversClass(RunPlan::class), CoversClass(RunPlanner::class), CoversClass(SniffEntry::class), - CoversClass(XmlFileProcessor::class), + CoversClass(XmlSniffRunner::class), // UsesClass(DiffBaseResolver::class), UsesClass(DiffChangeset::class), @@ -84,7 +84,7 @@ UsesClass(RunScopeResolver::class), UsesClass(UpstreamResolver::class), UsesClass(ViolationScopeFilter::class), - UsesClass(XmlProcessingResult::class), + UsesClass(XmlFileProcessor::class), ] final class ApplicationTest extends TestCase { diff --git a/tests/Unit/Fix/AttributeOrderFixerTest.php b/tests/Unit/Fix/AttributeOrderFixerTest.php index 562967e..d4a38da 100644 --- a/tests/Unit/Fix/AttributeOrderFixerTest.php +++ b/tests/Unit/Fix/AttributeOrderFixerTest.php @@ -9,7 +9,6 @@ use DocbookCS\Fix\FixPlan; use DocbookCS\Fix\Fixer\AttributeOrderFixer; use DocbookCS\Fix\FixResult; -use DocbookCS\Runner\RunMode; use DocbookCS\Sniff\AttributeOrderSniff; use DocbookCS\Source\File; use DocbookCS\Source\Line; @@ -26,7 +25,6 @@ CoversClass(Fix::class), CoversClass(FixApplier::class), CoversClass(FixResult::class), - CoversClass(RunMode::class), CoversClass(Violation::class), // UsesClass(File::class), @@ -43,7 +41,7 @@ public function itMovesXmlIdBeforeXmlns(): void $document = $this->createDocument($content); $source = new File('file.xml', $content); - $violations = new AttributeOrderSniff(RunMode::Fix)->process($document, $source); + $violations = new AttributeOrderSniff()->process($document, $source); $beginOffset = (int)strpos($content, 'createDocument($content); $source = new File('file.xml', $content); - $violations = new AttributeOrderSniff(RunMode::Fix)->process($document, $source); + $violations = new AttributeOrderSniff()->process($document, $source); self::assertCount(1, $violations); diff --git a/tests/Unit/Fix/ExceptionNameFixerTest.php b/tests/Unit/Fix/ExceptionNameFixerTest.php index 7812eb1..d384631 100644 --- a/tests/Unit/Fix/ExceptionNameFixerTest.php +++ b/tests/Unit/Fix/ExceptionNameFixerTest.php @@ -10,7 +10,6 @@ use DocbookCS\Fix\Fixer\ExceptionNameFixer; use DocbookCS\Fix\FixResult; use DocbookCS\Runner\EntityExpansionMarker; -use DocbookCS\Runner\RunMode; use DocbookCS\Sniff\ExceptionNameSniff; use DocbookCS\Source\File; use DocbookCS\Source\Line; @@ -27,7 +26,6 @@ CoversClass(Fix::class), CoversClass(FixApplier::class), CoversClass(FixResult::class), - CoversClass(RunMode::class), CoversClass(Violation::class), // UsesClass(EntityExpansionMarker::class), @@ -45,7 +43,7 @@ public function itReplacesSimpleClassnameTags(): void $document = $this->createDocument($content); $source = new File('file.xml', $content); - $violations = new ExceptionNameSniff(RunMode::Fix)->process($document, $source); + $violations = new ExceptionNameSniff()->process($document, $source); $beginOffset = (int) strpos($content, ''); $untilOffset = (int) strpos($content, ''); @@ -73,7 +71,7 @@ public function itPreservesClassnameAttributes(): void $document = $this->createDocument($content); $source = new File('file.xml', $content); - $violations = new ExceptionNameSniff(RunMode::Fix)->process($document, $source); + $violations = new ExceptionNameSniff()->process($document, $source); self::assertCount(1, $violations); self::assertSame('classname', $violations[0]->rangeOne()->content); @@ -97,7 +95,7 @@ public function itKeepsSourceContentAlignedAfterRegularClassnames(): void $document = $this->createDocument($content); $source = new File('file.xml', $content); - $violations = new ExceptionNameSniff(RunMode::Fix)->process($document, $source); + $violations = new ExceptionNameSniff()->process($document, $source); $sourceContent = 'RuntimeException'; $beginOffset = (int) strpos($content, $sourceContent); @@ -130,7 +128,7 @@ public function itPreservesTagShapedTextInsideComments(): void $document = $this->createDocument($content); $source = new File('file.xml', $content); - $violations = new ExceptionNameSniff(RunMode::Fix)->process($document, $source); + $violations = new ExceptionNameSniff()->process($document, $source); self::assertCount(1, $violations); diff --git a/tests/Unit/Fix/SimparaFixerTest.php b/tests/Unit/Fix/SimparaFixerTest.php index 7ebedb0..9a75cc1 100644 --- a/tests/Unit/Fix/SimparaFixerTest.php +++ b/tests/Unit/Fix/SimparaFixerTest.php @@ -10,7 +10,6 @@ use DocbookCS\Fix\Fixer\SimparaFixer; use DocbookCS\Fix\FixResult; use DocbookCS\Runner\EntityExpansionMarker; -use DocbookCS\Runner\RunMode; use DocbookCS\Sniff\SimparaSniff; use DocbookCS\Source\File; use DocbookCS\Source\Line; @@ -25,7 +24,6 @@ CoversClass(Fix::class), CoversClass(FixApplier::class), CoversClass(FixResult::class), - CoversClass(RunMode::class), CoversClass(SimparaFixer::class), CoversClass(SimparaSniff::class), CoversClass(Violation::class), @@ -45,7 +43,7 @@ public function itReplacesSimpleParaTags(): void $document = $this->createDocument($content); $source = new File('file.xml', $content); - $violations = new SimparaSniff(RunMode::Fix)->process($document, $source); + $violations = new SimparaSniff()->process($document, $source); self::assertCount(1, $violations); self::assertSame('para', $violations[0]->rangeOne()->content); @@ -66,7 +64,7 @@ public function itPreservesParaAttributes(): void $document = $this->createDocument($content); $source = new File('file.xml', $content); - $violations = new SimparaSniff(RunMode::Fix)->process($document, $source); + $violations = new SimparaSniff()->process($document, $source); self::assertCount(1, $violations); self::assertSame('para', $violations[0]->rangeOne()->content); @@ -87,7 +85,7 @@ public function itCanFixNestedInnerParas(): void $document = $this->createDocument($content); $source = new File('file.xml', $content); - $violations = new SimparaSniff(RunMode::Fix)->process($document, $source); + $violations = new SimparaSniff()->process($document, $source); self::assertCount(1, $violations); self::assertSame('para', $violations[0]->rangeOne()->content); @@ -111,7 +109,7 @@ public function itPreservesTagShapedTextInsideComments(): void $document = $this->createDocument($content); $source = new File('file.xml', $content); - $violations = new SimparaSniff(RunMode::Fix)->process($document, $source); + $violations = new SimparaSniff()->process($document, $source); self::assertCount(1, $violations); diff --git a/tests/Unit/Fix/WhitespaceConcernFixersTest.php b/tests/Unit/Fix/WhitespaceConcernFixersTest.php index 1563773..cdfa081 100644 --- a/tests/Unit/Fix/WhitespaceConcernFixersTest.php +++ b/tests/Unit/Fix/WhitespaceConcernFixersTest.php @@ -10,7 +10,6 @@ use DocbookCS\Fix\Fixer\MixedIndentationFixer; use DocbookCS\Fix\Fixer\TrailingWhitespaceFixer; use DocbookCS\Fix\FixResult; -use DocbookCS\Runner\RunMode; use DocbookCS\Sniff\MixedIndentationSniff; use DocbookCS\Sniff\TrailingWhitespaceSniff; use DocbookCS\Source\File; @@ -47,8 +46,8 @@ public function itFixesIndependentWhitespaceConcernsTogether(): void $document->loadXML($content); $source = new File('file.xml', $content); - $trailingViolations = new TrailingWhitespaceSniff(RunMode::Fix)->process($document, $source); - $indentationViolations = new MixedIndentationSniff(RunMode::Fix)->process($document, $source); + $trailingViolations = new TrailingWhitespaceSniff()->process($document, $source); + $indentationViolations = new MixedIndentationSniff()->process($document, $source); self::assertCount(2, $trailingViolations); self::assertCount(1, $indentationViolations); diff --git a/tests/Unit/Progress/ConsoleProgressTest.php b/tests/Unit/Progress/ConsoleProgressTest.php index b2ef99c..7f2ca2b 100644 --- a/tests/Unit/Progress/ConsoleProgressTest.php +++ b/tests/Unit/Progress/ConsoleProgressTest.php @@ -43,9 +43,19 @@ public function itDisplaysTotalFileCountOnStart(): void { $progress = new ConsoleProgress($this->stream, useColors: false); - $progress->start(42); + $progress->start(1_042); - self::assertStringContainsString('42 file(s)', $this->outputConsole()); + self::assertStringContainsString('1,042 files', $this->outputConsole()); + } + + #[Test] + public function itUsesTheSingularFileLabel(): void + { + $progress = new ConsoleProgress($this->stream, useColors: false); + + $progress->start(1); + + self::assertStringContainsString('1 file', $this->outputConsole()); } #[Test] diff --git a/tests/Unit/Report/ReportTest.php b/tests/Unit/Report/ReportTest.php index 4a50dac..520d9d7 100644 --- a/tests/Unit/Report/ReportTest.php +++ b/tests/Unit/Report/ReportTest.php @@ -7,6 +7,7 @@ use DocbookCS\RelativePath; use DocbookCS\Report\FileReport; use DocbookCS\Report\Report; +use DocbookCS\Report\ReportException; use DocbookCS\Violation\Severity; use DocbookCS\Violation\SourceRange; use DocbookCS\Violation\Violation; @@ -19,6 +20,7 @@ CoversClass(FileReport::class), CoversClass(RelativePath::class), CoversClass(Report::class), + CoversClass(ReportException::class), CoversClass(Violation::class), // UsesClass(SourceRange::class), @@ -27,12 +29,15 @@ final class ReportTest extends TestCase { private function createViolation( string $message = 'Some problem', - int $line = 1, - string $sniffCode = 'DocbookCS.Test', Severity $severity = Severity::ERROR, - string $filePath = 'file.xml', ): Violation { - return new Violation($sniffCode, $filePath, $message, [new SourceRange($line, 0, 0)], severity: $severity); + return new Violation( + 'DocbookCS.Test', + 'file.xml', + $message, + [new SourceRange(1, 0, 0)], + severity: $severity, + ); } #[Test] @@ -40,18 +45,18 @@ public function itStartsWithZeroFilesScanned(): void { $report = new Report(); - self::assertSame(0, $report->getFilesScanned()); + self::assertSame(0, $report->getScannedFilesCount()); } #[Test] - public function itIncrementsFilesScanned(): void + public function itCountsFileReportsAsScannedFiles(): void { $report = new Report(); - $report->incrementFilesScanned(); - $report->incrementFilesScanned(); - $report->incrementFilesScanned(); + $report->addFileReport(new FileReport('a.xml')); + $report->addFileReport(new FileReport('b.xml')); + $report->addFileReport(new FileReport('c.xml')); - self::assertSame(3, $report->getFilesScanned()); + self::assertSame(3, $report->getScannedFilesCount()); } #[Test] @@ -59,7 +64,28 @@ public function itStartsWithNoFileReports(): void { $report = new Report(); - self::assertSame([], $report->getFileReports()); + self::assertSame([], $report->fileReports); + } + + #[Test] + public function itRejectsAddingFoundViolationsTwice(): void + { + $fileReport = new FileReport('file.xml'); + $fileReport->addFoundViolations([]); + + $this->expectException(ReportException::class); + + $fileReport->addFoundViolations([]); + } + + #[Test] + public function itRejectsAddingFinalViolationsBeforeFoundViolations(): void + { + $fileReport = new FileReport('file.xml'); + + $this->expectException(ReportException::class); + + $fileReport->addFinalViolations([]); } #[Test] @@ -70,8 +96,8 @@ public function itAddsFileReport(): void $report->addFileReport($fileReport); - self::assertCount(1, $report->getFileReports()); - self::assertSame($fileReport, $report->getFileReports()['src/chapter.xml']); + self::assertCount(1, $report->fileReports); + self::assertSame($fileReport, $report->fileReports['src/chapter.xml']); } #[Test] @@ -91,7 +117,7 @@ public function itKeysFileReportsByFilePath(): void $report->addFileReport(new FileReport('a.xml')); $report->addFileReport(new FileReport('b.xml')); - $keys = array_keys($report->getFileReports()); + $keys = array_keys($report->fileReports); self::assertSame(['a.xml', 'b.xml'], $keys); } @@ -106,25 +132,27 @@ public function itOverwritesFileReportWithSamePath(): void $report->addFileReport($first); $report->addFileReport($second); - self::assertCount(1, $report->getFileReports()); - self::assertSame($second, $report->getFileReports()['file.xml']); + self::assertCount(1, $report->fileReports); + self::assertSame($second, $report->fileReports['file.xml']); } #[Test] public function itReturnsTotalViolationsAcrossAllFiles(): void { $file1 = new FileReport('a.xml'); - $file1->addViolation($this->createViolation(severity: Severity::ERROR)); - $file1->addViolation($this->createViolation(severity: Severity::WARNING)); + $file1->addFoundViolations([ + $this->createViolation(), + $this->createViolation(severity: Severity::WARNING), + ]); $file2 = new FileReport('b.xml'); - $file2->addViolation($this->createViolation(severity: Severity::ERROR)); + $file2->addFoundViolations([$this->createViolation()]); $report = new Report(); $report->addFileReport($file1); $report->addFileReport($file2); - self::assertSame(3, $report->getTotalViolations()); + self::assertSame(3, $report->getTotalFinalViolationCount()); } #[Test] @@ -132,25 +160,29 @@ public function itReturnsZeroTotalViolationsWhenEmpty(): void { $report = new Report(); - self::assertSame(0, $report->getTotalViolations()); + self::assertSame(0, $report->getTotalFinalViolationCount()); } #[Test] public function itReturnsTotalErrorsAcrossAllFiles(): void { $file1 = new FileReport('a.xml'); - $file1->addViolation($this->createViolation(severity: Severity::ERROR)); - $file1->addViolation($this->createViolation(severity: Severity::WARNING)); + $file1->addFoundViolations([ + $this->createViolation(), + $this->createViolation(severity: Severity::WARNING), + ]); $file2 = new FileReport('b.xml'); - $file2->addViolation($this->createViolation(severity: Severity::ERROR)); - $file2->addViolation($this->createViolation(severity: Severity::ERROR)); + $file2->addFoundViolations([ + $this->createViolation(), + $this->createViolation(), + ]); $report = new Report(); $report->addFileReport($file1); $report->addFileReport($file2); - self::assertSame(3, $report->getTotalErrors()); + self::assertSame(3, $report->getTotalErrorLevelViolationCount()); } #[Test] @@ -158,25 +190,29 @@ public function itReturnsZeroTotalErrorsWhenEmpty(): void { $report = new Report(); - self::assertSame(0, $report->getTotalErrors()); + self::assertSame(0, $report->getTotalErrorLevelViolationCount()); } #[Test] public function itReturnsTotalWarningsAcrossAllFiles(): void { $file1 = new FileReport('a.xml'); - $file1->addViolation($this->createViolation(severity: Severity::WARNING)); - $file1->addViolation($this->createViolation(severity: Severity::ERROR)); + $file1->addFoundViolations([ + $this->createViolation(severity: Severity::WARNING), + $this->createViolation(severity: Severity::ERROR), + ]); $file2 = new FileReport('b.xml'); - $file2->addViolation($this->createViolation(severity: Severity::WARNING)); - $file2->addViolation($this->createViolation(severity: Severity::WARNING)); + $file2->addFoundViolations([ + $this->createViolation(severity: Severity::WARNING), + $this->createViolation(severity: Severity::WARNING), + ]); $report = new Report(); $report->addFileReport($file1); $report->addFileReport($file2); - self::assertSame(3, $report->getTotalWarnings()); + self::assertSame(3, $report->getTotalWarningLevelViolationCount()); } #[Test] @@ -184,36 +220,36 @@ public function itReturnsZeroTotalWarningsWhenEmpty(): void { $report = new Report(); - self::assertSame(0, $report->getTotalWarnings()); + self::assertSame(0, $report->getTotalWarningLevelViolationCount()); } #[Test] - public function itHasViolationsWhenViolationsExist(): void + public function itHasFinalViolationsWhenFinalViolationsExist(): void { $fileReport = new FileReport('file.xml'); - $fileReport->addViolation($this->createViolation()); + $fileReport->addFoundViolations([$this->createViolation()]); $report = new Report(); $report->addFileReport($fileReport); - self::assertTrue($report->hasViolations()); + self::assertTrue($report->hasFinalViolations()); } #[Test] - public function itHasNoViolationsWhenEmpty(): void + public function itHasNoFinalViolationsWhenEmpty(): void { $report = new Report(); - self::assertFalse($report->hasViolations()); + self::assertFalse($report->hasFinalViolations()); } #[Test] - public function itHasNoViolationsWhenFilesAreClean(): void + public function itHasNoFinalViolationsWhenFilesAreClean(): void { $report = new Report(); $report->addFileReport(new FileReport('clean.xml')); - self::assertFalse($report->hasViolations()); + self::assertFalse($report->hasFinalViolations()); } #[Test] @@ -224,11 +260,10 @@ public function itReturnsAllViolationsFromAllFiles(): void $v3 = $this->createViolation(message: 'Third'); $file1 = new FileReport('a.xml'); - $file1->addViolation($v1); - $file1->addViolation($v2); + $file1->addFoundViolations([$v1, $v2]); $file2 = new FileReport('b.xml'); - $file2->addViolation($v3); + $file2->addFoundViolations([$v3]); $report = new Report(); $report->addFileReport($file1); @@ -254,41 +289,168 @@ public function itReturnsEmptyListWhenNoViolations(): void public function itDoesNotCountWarningsAsErrors(): void { $fileReport = new FileReport('file.xml'); - $fileReport->addViolation($this->createViolation(severity: Severity::WARNING)); - $fileReport->addViolation($this->createViolation(severity: Severity::WARNING)); + $fileReport->addFoundViolations([ + $this->createViolation(severity: Severity::WARNING), + $this->createViolation(severity: Severity::WARNING), + ]); $report = new Report(); $report->addFileReport($fileReport); - self::assertSame(0, $report->getTotalErrors()); - self::assertSame(2, $report->getTotalWarnings()); - self::assertSame(2, $report->getTotalViolations()); + self::assertSame(0, $report->getTotalErrorLevelViolationCount()); + self::assertSame(2, $report->getTotalWarningLevelViolationCount()); + self::assertSame(2, $report->getTotalFinalViolationCount()); } #[Test] public function itDoesNotCountErrorsAsWarnings(): void { $fileReport = new FileReport('file.xml'); - $fileReport->addViolation($this->createViolation(severity: Severity::ERROR)); - $fileReport->addViolation($this->createViolation(severity: Severity::ERROR)); + $fileReport->addFoundViolations([ + $this->createViolation(), + $this->createViolation(), + ]); $report = new Report(); $report->addFileReport($fileReport); - self::assertSame(2, $report->getTotalErrors()); - self::assertSame(0, $report->getTotalWarnings()); - self::assertSame(2, $report->getTotalViolations()); + self::assertSame(2, $report->getTotalErrorLevelViolationCount()); + self::assertSame(0, $report->getTotalWarningLevelViolationCount()); + self::assertSame(2, $report->getTotalFinalViolationCount()); } #[Test] - public function filesScannedIsIndependentOfFileReports(): void + public function itCountsCleanFileReportsAsScannedFiles(): void { $report = new Report(); - $report->incrementFilesScanned(); - $report->incrementFilesScanned(); - $report->incrementFilesScanned(); + $report->addFileReport(new FileReport('clean1.xml')); + $report->addFileReport(new FileReport('clean2.xml')); + $report->addFileReport(new FileReport('clean3.xml')); + + self::assertSame(3, $report->getScannedFilesCount()); + self::assertCount(3, $report->fileReports); + } + + #[Test] + public function itAggregatesFixingOutcome(): void + { + $first = new FileReport('first.xml'); + $first->markChanged(); + $first->recordFixingPass(); + $first->recordFixingPass(); + $first->addFoundViolations(array_fill(0, 4, $this->createViolation())); + $first->addFinalViolations([$this->createViolation()]); + + $second = new FileReport('second.xml'); + $second->markChanged(); + $second->recordFixingPass(); + $second->addFoundViolations(array_fill(0, 3, $this->createViolation())); + $second->addFinalViolations([$this->createViolation()]); + + $report = new Report(); + $report->addFileReport($first); + $report->addFileReport($second); + + self::assertSame(2, $report->getChangedFilesCount()); + self::assertSame(7, $report->getFoundViolationsCount()); + self::assertSame(5, $report->getAppliedFixesCount()); + self::assertSame(2, $report->getSkippedFixesCount()); + self::assertSame(5, $report->getFixedErrorCount()); + self::assertSame(0, $report->getFixedWarningCount()); + self::assertSame(3, $report->getFixingPassesCount()); + } + + #[Test] + public function itAggregatesSniffTimes(): void + { + $first = new FileReport('first.xml', collectPerformance: true); + $first->measureSniffer('Test.Sniff', static fn() => null); + + $second = new FileReport('second.xml', collectPerformance: true); + $second->measureSniffer('Test.Sniff', static fn() => null); + + $report = new Report(); + $report->addFileReport($first); + $report->addFileReport($second); + + self::assertSame( + $first->sniffingTimes['Test.Sniff'] + $second->sniffingTimes['Test.Sniff'], + $report->getSniffingTimes()['Test.Sniff'], + ); + } + + #[Test] + public function itAggregatesFixerTimes(): void + { + $first = new FileReport('first.xml', collectPerformance: true); + $first->measureFixer('Test.Sniff', static fn() => null); + + $second = new FileReport('second.xml', collectPerformance: true); + $second->measureFixer('Test.Sniff', static fn() => null); + + $report = new Report(); + $report->addFileReport($first); + $report->addFileReport($second); + + self::assertSame( + $first->fixingTimes['Test.Sniff'] + $second->fixingTimes['Test.Sniff'], + $report->getFixingTimes()['Test.Sniff'], + ); + } + + #[Test] + public function itMeasuresSniffingAndReturnsTheOperationResult(): void + { + $fileReport = new FileReport('file.xml', collectPerformance: true); + + $result = $fileReport->measureSniffing(static function (): string { + usleep(1_000); + + return 'result'; + }); + + self::assertSame('result', $result); + self::assertGreaterThan(0.0, $fileReport->totalSniffingTime); + } + + #[Test] + public function itMeasuresFixingAndReturnsTheOperationResult(): void + { + $report = new Report(); + + + $fileReport = new FileReport('file.xml', collectPerformance: true); + $report->addFileReport($fileReport); + + $result = $fileReport->measureFixing(static function (): string { + usleep(1_000); + + return 'result'; + }); + + self::assertSame('result', $result); + self::assertGreaterThan(0.0, $report->getTotalFixingTime()); + } + + #[Test] + public function itRunsOperationsWithoutCollectingPerformanceByDefault(): void + { + $fileReport = new FileReport('file.xml'); + + $result = $fileReport->measureSniffer('Test.Sniff', static fn(): string => 'result'); + + self::assertSame('result', $result); + self::assertSame([], $fileReport->sniffingTimes); + } + + #[Test] + public function itRecordsFixingPasses(): void + { + $fileReport = new FileReport('file.xml'); + + $fileReport->recordFixingPass(); + $fileReport->recordFixingPass(); - self::assertSame(3, $report->getFilesScanned()); - self::assertCount(0, $report->getFileReports()); + self::assertSame(2, $fileReport->fixingPasses); } } diff --git a/tests/Unit/Report/Reporter/CheckstyleReporterTest.php b/tests/Unit/Report/Reporter/CheckstyleReporterTest.php index 69f2958..3291e36 100644 --- a/tests/Unit/Report/Reporter/CheckstyleReporterTest.php +++ b/tests/Unit/Report/Reporter/CheckstyleReporterTest.php @@ -92,6 +92,24 @@ public function itProducesNoFileNodesForEmptyReport(): void self::assertSame(0, $dom->getElementsByTagName('file')->length); } + #[Test] + public function itExcludesFixingOutcome(): void + { + $fileReport = new FileReport('fixed.xml'); + $fileReport->markChanged(); + $fileReport->recordFixingPass(); + + $report = new Report(); + $report->addFileReport($fileReport); + + $output = $this->reporter->generate($report); + $dom = $this->parseOutput($output); + + self::assertSame(0, $dom->getElementsByTagName('file')->length); + self::assertStringContainsString('total runtime:', $output); + self::assertStringNotContainsString('fix', $output); + } + #[Test] public function itSkipsFilesWithNoViolations(): void { @@ -107,7 +125,7 @@ public function itSkipsFilesWithNoViolations(): void public function itIncludesFileNodeWithNameAttribute(): void { $fileReport = new FileReport('src/broken.xml'); - $fileReport->addViolation($this->createViolation()); + $fileReport->addFoundViolations([$this->createViolation()]); $report = new Report(); $report->addFileReport($fileReport); @@ -123,7 +141,7 @@ public function itIncludesFileNodeWithNameAttribute(): void public function itRendersAbsoluteFilePathRelativeToWorkingDirectory(): void { $fileReport = new FileReport((getcwd() ?: '') . '/src/broken.xml'); - $fileReport->addViolation($this->createViolation()); + $fileReport->addFoundViolations([$this->createViolation()]); $report = new Report(); $report->addFileReport($fileReport); @@ -140,7 +158,7 @@ public function itRendersAbsoluteFilePathRelativeToWorkingDirectory(): void public function itSetsLineAttribute(): void { $fileReport = new FileReport('file.xml'); - $fileReport->addViolation($this->createViolation(line: 42)); + $fileReport->addFoundViolations([$this->createViolation(line: 42)]); $report = new Report(); $report->addFileReport($fileReport); @@ -155,7 +173,7 @@ public function itSetsLineAttribute(): void public function itSetsSeverityAttribute(): void { $fileReport = new FileReport('file.xml'); - $fileReport->addViolation($this->createViolation(severity: Severity::WARNING)); + $fileReport->addFoundViolations([$this->createViolation(severity: Severity::WARNING)]); $report = new Report(); $report->addFileReport($fileReport); @@ -170,7 +188,7 @@ public function itSetsSeverityAttribute(): void public function itSetsMessageAttribute(): void { $fileReport = new FileReport('file.xml'); - $fileReport->addViolation($this->createViolation(message: 'Use instead')); + $fileReport->addFoundViolations([$this->createViolation(message: 'Use instead')]); $report = new Report(); $report->addFileReport($fileReport); @@ -185,7 +203,7 @@ public function itSetsMessageAttribute(): void public function itSetsSourceAttribute(): void { $fileReport = new FileReport('file.xml'); - $fileReport->addViolation($this->createViolation(sniffCode: 'DocbookCS.ExceptionName')); + $fileReport->addFoundViolations([$this->createViolation(sniffCode: 'DocbookCS.ExceptionName')]); $report = new Report(); $report->addFileReport($fileReport); @@ -200,9 +218,11 @@ public function itSetsSourceAttribute(): void public function itOutputsMultipleViolationsForOneFile(): void { $fileReport = new FileReport('multi.xml'); - $fileReport->addViolation($this->createViolation(message: 'First', line: 5)); - $fileReport->addViolation($this->createViolation(message: 'Second', line: 10)); - $fileReport->addViolation($this->createViolation(message: 'Third', line: 20)); + $fileReport->addFoundViolations([ + $this->createViolation(message: 'First', line: 5), + $this->createViolation(message: 'Second', line: 10), + $this->createViolation(message: 'Third', line: 20), + ]); $report = new Report(); $report->addFileReport($fileReport); @@ -216,10 +236,10 @@ public function itOutputsMultipleViolationsForOneFile(): void public function itOutputsMultipleFilesWithViolations(): void { $file1 = new FileReport('first.xml'); - $file1->addViolation($this->createViolation(message: 'Issue A')); + $file1->addFoundViolations([$this->createViolation(message: 'Issue A')]); $file2 = new FileReport('second.xml'); - $file2->addViolation($this->createViolation(message: 'Issue B')); + $file2->addFoundViolations([$this->createViolation(message: 'Issue B')]); $report = new Report(); $report->addFileReport($file1); @@ -239,7 +259,7 @@ public function itSkipsCleanFilesAmongDirtyOnes(): void $cleanFile = new FileReport('clean.xml'); $dirtyFile = new FileReport('dirty.xml'); - $dirtyFile->addViolation($this->createViolation()); + $dirtyFile->addFoundViolations([$this->createViolation()]); $report = new Report(); $report->addFileReport($cleanFile); @@ -256,7 +276,7 @@ public function itSkipsCleanFilesAmongDirtyOnes(): void public function itEscapesSpecialCharactersInMessage(): void { $fileReport = new FileReport('escape.xml'); - $fileReport->addViolation($this->createViolation(message: 'Use "quotes" & ')); + $fileReport->addFoundViolations([$this->createViolation(message: 'Use "quotes" & ')]); $report = new Report(); $report->addFileReport($fileReport); diff --git a/tests/Unit/Report/Reporter/ConsoleReporterTest.php b/tests/Unit/Report/Reporter/ConsoleReporterTest.php index ad71756..b6d6ddd 100644 --- a/tests/Unit/Report/Reporter/ConsoleReporterTest.php +++ b/tests/Unit/Report/Reporter/ConsoleReporterTest.php @@ -39,9 +39,8 @@ private function createViolation( int $line = 1, string $sniffCode = 'DocbookCS.Test', Severity $severity = Severity::ERROR, - string $filePath = 'filepath.xml', ): Violation { - return new Violation($sniffCode, $filePath, $message, [new SourceRange($line, 0, 0)], severity: $severity); + return new Violation($sniffCode, 'filepath.xml', $message, [new SourceRange($line, 0, 0)], severity: $severity); } #[Test] @@ -59,34 +58,153 @@ public function itShowsOkSummaryWhenNoViolations(): void { $report = new Report(); $report->addFileReport(new FileReport('clean.xml')); - $report->incrementFilesScanned(); $output = $this->reporter->generate($report); - self::assertStringContainsString('OK -- 1 file(s) scanned, no violations found.', $output); + self::assertStringContainsString('OK -- 1 file scanned, no violations found.', $output); + self::assertStringNotContainsString('FIXING', $output); + } + + #[Test] + public function itShowsNoViolationsRemainingAfterFixing(): void + { + $report = new Report(); + + $fileReport = new FileReport('fixed.xml'); + $violation = $this->createViolation(); + $fileReport->addFoundViolations([$violation]); + $fileReport->addFinalViolations([]); + $fileReport->markChanged(); + $fileReport->recordFixingPass(); + + $report->addFileReport($fileReport); + + $output = $this->reporter->generate($report); + + self::assertStringContainsString( + 'FIXED 1 violation [1 error, 0 warnings] in 1 file, no violations remaining.', + $output, + ); + self::assertStringNotContainsString('REMAINING', $output); + self::assertStringNotContainsString('OK --', $output); + self::assertStringNotContainsString('passes)', $output); + } + + #[Test] + public function itShowsViolationsRemainingAfterFixing(): void + { + $fileReport = new FileReport('dirty.xml'); + $violation = $this->createViolation(); + $fileReport->addFoundViolations([$violation]); + $fileReport->recordFixingPass(); + + $report = new Report(); + $report->addFileReport($fileReport); + + $output = $this->reporter->generate($report); + + self::assertStringContainsString('REMAINING 1 violation [1 error, 0 warnings] in 1 file.', $output); + self::assertStringNotContainsString('passes)', $output); } #[Test] public function itShowsViolationSummaryWhenViolationsExist(): void { $fileReport = new FileReport('dirty.xml'); - $fileReport->addViolation($this->createViolation(severity: Severity::ERROR)); - $fileReport->addViolation($this->createViolation(severity: Severity::WARNING)); + $fileReport->addFoundViolations([ + $this->createViolation(), + $this->createViolation(severity: Severity::WARNING), + ]); + + $report = new Report(); + $report->addFileReport($fileReport); + + $output = $this->reporter->generate($report); + + self::assertStringContainsString('FOUND 2 violations [1 error, 1 warning] in 1 file.', $output); + } + + #[Test] + public function itShowsRemainingViolationsAfterFixing(): void + { + $fileReport = new FileReport('dirty.xml'); + $violation = $this->createViolation(); + $fileReport->addFoundViolations([$violation]); + $fileReport->recordFixingPass(); $report = new Report(); $report->addFileReport($fileReport); - $report->incrementFilesScanned(); $output = $this->reporter->generate($report); - self::assertStringContainsString('FOUND 2 violation(s) (1 error(s), 1 warning(s)) in 1 file(s).', $output); + self::assertStringContainsString( + 'REMAINING 1 violation [1 error, 0 warnings] in 1 file.', + $output, + ); + } + + #[Test] + public function itShowsFixedViolationsAndAdditionalPasses(): void + { + $report = new Report(); + + $first = new FileReport('first.xml'); + $first->markChanged(); + $first->recordFixingPass(); + $first->recordFixingPass(); + $first->addFoundViolations([ + ...array_fill(0, 3, $this->createViolation()), + $this->createViolation(severity: Severity::WARNING), + ]); + $first->addFinalViolations([$this->createViolation(severity: Severity::WARNING)]); + + $second = new FileReport('second.xml'); + $second->markChanged(); + $second->recordFixingPass(); + $second->addFoundViolations([ + $this->createViolation(), + $this->createViolation(severity: Severity::WARNING), + $this->createViolation(severity: Severity::WARNING), + ]); + $second->addFinalViolations([$this->createViolation()]); + + $report->addFileReport($first); + $report->addFileReport($second); + + $output = $this->reporter->generate($report); + + self::assertStringContainsString( + 'FIXED 5 violations [3 errors, 2 warnings] in 2 files (3 passes).', + $output, + ); + self::assertStringNotContainsString('FIXING', $output); + } + + #[Test] + public function itFormatsLargeSummaryCounts(): void + { + $fileReport = new FileReport('fixed.xml'); + $fileReport->markChanged(); + $fileReport->recordFixingPass(); + $fileReport->addFoundViolations(array_fill(0, 1_001, $this->createViolation())); + $fileReport->addFinalViolations([$this->createViolation()]); + + $report = new Report(); + $report->addFileReport($fileReport); + + $output = $this->reporter->generate($report); + + self::assertStringContainsString( + 'FIXED 1,000 violations [1,000 errors, 0 warnings] in 1 file.', + $output, + ); } #[Test] public function itShowsFilePathInHeader(): void { $fileReport = new FileReport('src/broken.xml'); - $fileReport->addViolation($this->createViolation()); + $fileReport->addFoundViolations([$this->createViolation()]); $report = new Report(); $report->addFileReport($fileReport); @@ -100,7 +218,7 @@ public function itShowsFilePathInHeader(): void public function itRendersAbsoluteFilePathRelativeToWorkingDirectory(): void { $fileReport = new FileReport((getcwd() ?: '') . '/src/broken.xml'); - $fileReport->addViolation($this->createViolation()); + $fileReport->addFoundViolations([$this->createViolation()]); $report = new Report(); $report->addFileReport($fileReport); @@ -114,7 +232,7 @@ public function itRendersAbsoluteFilePathRelativeToWorkingDirectory(): void public function itShowsDashSeparatorAfterFileHeader(): void { $fileReport = new FileReport('file.xml'); - $fileReport->addViolation($this->createViolation()); + $fileReport->addFoundViolations([$this->createViolation()]); $report = new Report(); $report->addFileReport($fileReport); @@ -130,7 +248,7 @@ public function itCapsTheDashSeparatorAt80Characters(): void { $longPath = str_repeat('a', 200) . '.xml'; $fileReport = new FileReport($longPath); - $fileReport->addViolation($this->createViolation()); + $fileReport->addFoundViolations([$this->createViolation()]); $report = new Report(); $report->addFileReport($fileReport); @@ -145,7 +263,7 @@ public function itCapsTheDashSeparatorAt80Characters(): void public function itShowsLineNumberInViolation(): void { $fileReport = new FileReport('file.xml'); - $fileReport->addViolation($this->createViolation(line: 42)); + $fileReport->addFoundViolations([$this->createViolation(line: 42)]); $report = new Report(); $report->addFileReport($fileReport); @@ -159,7 +277,7 @@ public function itShowsLineNumberInViolation(): void public function itRightAlignsLineNumberIn4CharWidth(): void { $fileReport = new FileReport('file.xml'); - $fileReport->addViolation($this->createViolation(line: 5)); + $fileReport->addFoundViolations([$this->createViolation(line: 5)]); $report = new Report(); $report->addFileReport($fileReport); @@ -173,7 +291,7 @@ public function itRightAlignsLineNumberIn4CharWidth(): void public function itShowsMessageInViolation(): void { $fileReport = new FileReport('file.xml'); - $fileReport->addViolation($this->createViolation(message: 'Use instead')); + $fileReport->addFoundViolations([$this->createViolation(message: 'Use instead')]); $report = new Report(); $report->addFileReport($fileReport); @@ -187,7 +305,7 @@ public function itShowsMessageInViolation(): void public function itShowsSniffCodeInViolation(): void { $fileReport = new FileReport('file.xml'); - $fileReport->addViolation($this->createViolation(sniffCode: 'DocbookCS.ExceptionName')); + $fileReport->addFoundViolations([$this->createViolation(sniffCode: 'DocbookCS.ExceptionName')]); $report = new Report(); $report->addFileReport($fileReport); @@ -201,7 +319,7 @@ public function itShowsSniffCodeInViolation(): void public function itShowsErrorSeverityLabel(): void { $fileReport = new FileReport('file.xml'); - $fileReport->addViolation($this->createViolation(severity: Severity::ERROR)); + $fileReport->addFoundViolations([$this->createViolation()]); $report = new Report(); $report->addFileReport($fileReport); @@ -215,9 +333,11 @@ public function itShowsErrorSeverityLabel(): void public function itShowsMultipleViolationsForOneFile(): void { $fileReport = new FileReport('multi.xml'); - $fileReport->addViolation($this->createViolation(message: 'First issue', line: 5)); - $fileReport->addViolation($this->createViolation(message: 'Second issue', line: 10)); - $fileReport->addViolation($this->createViolation(message: 'Third issue', line: 20)); + $fileReport->addFoundViolations([ + $this->createViolation(message: 'First issue', line: 5), + $this->createViolation(message: 'Second issue', line: 10), + $this->createViolation(message: 'Third issue', line: 20), + ]); $report = new Report(); $report->addFileReport($fileReport); @@ -233,10 +353,10 @@ public function itShowsMultipleViolationsForOneFile(): void public function itShowsMultipleFileHeaders(): void { $file1 = new FileReport('first.xml'); - $file1->addViolation($this->createViolation(message: 'Issue A')); + $file1->addFoundViolations([$this->createViolation(message: 'Issue A')]); $file2 = new FileReport('second.xml'); - $file2->addViolation($this->createViolation(message: 'Issue B')); + $file2->addFoundViolations([$this->createViolation(message: 'Issue B')]); $report = new Report(); $report->addFileReport($file1); @@ -254,7 +374,7 @@ public function itSkipsCleanFilesAmongDirtyOnes(): void $cleanFile = new FileReport('clean.xml'); $dirtyFile = new FileReport('dirty.xml'); - $dirtyFile->addViolation($this->createViolation()); + $dirtyFile->addFoundViolations([$this->createViolation()]); $report = new Report(); $report->addFileReport($cleanFile); @@ -271,25 +391,22 @@ public function itShowsScannedFileCountInOkSummary(): void { $report = new Report(); $report->addFileReport(new FileReport('a.xml')); - $report->incrementFilesScanned(); $report->addFileReport(new FileReport('b.xml')); - $report->incrementFilesScanned(); $report->addFileReport(new FileReport('c.xml')); - $report->incrementFilesScanned(); $output = $this->reporter->generate($report); - self::assertStringContainsString('3 file(s) scanned', $output); + self::assertStringContainsString('3 files scanned', $output); } #[Test] public function itCountsFilesWithViolationsInFoundSummary(): void { $file1 = new FileReport('a.xml'); - $file1->addViolation($this->createViolation()); + $file1->addFoundViolations([$this->createViolation()]); $file2 = new FileReport('b.xml'); - $file2->addViolation($this->createViolation()); + $file2->addFoundViolations([$this->createViolation()]); $cleanFile = new FileReport('c.xml'); @@ -300,7 +417,7 @@ public function itCountsFilesWithViolationsInFoundSummary(): void $output = $this->reporter->generate($report); - self::assertStringContainsString('in 3 file(s).', $output); + self::assertStringContainsString('in 2 files.', $output); } #[Test] @@ -309,7 +426,7 @@ public function itAppliesAnsiCodesWhenColorsEnabled(): void $reporter = new ConsoleReporter(useColors: true); $report = new Report(); - $report->incrementFilesScanned(); + $report->addFileReport(new FileReport('clean.xml')); $output = $reporter->generate($report); @@ -320,7 +437,7 @@ public function itAppliesAnsiCodesWhenColorsEnabled(): void public function itOmitsAnsiCodesWhenColorsDisabled(): void { $report = new Report(); - $report->incrementFilesScanned(); + $report->addFileReport(new FileReport('clean.xml')); $output = $this->reporter->generate($report); @@ -333,7 +450,7 @@ public function itUsesColorsEnabledByDefault(): void $reporter = new ConsoleReporter(); $report = new Report(); - $report->incrementFilesScanned(); + $report->addFileReport(new FileReport('clean.xml')); $output = $reporter->generate($report); @@ -344,7 +461,7 @@ public function itUsesColorsEnabledByDefault(): void public function itPadsSeverityToSevenCharacters(): void { $fileReport = new FileReport('file.xml'); - $fileReport->addViolation($this->createViolation(severity: Severity::ERROR)); + $fileReport->addFoundViolations([$this->createViolation()]); $report = new Report(); $report->addFileReport($fileReport); @@ -358,12 +475,7 @@ public function itPadsSeverityToSevenCharacters(): void public function itSeparatesFieldsWithPipes(): void { $fileReport = new FileReport('file.xml'); - $fileReport->addViolation($this->createViolation( - message: 'Test message', - line: 1, - sniffCode: 'DocbookCS.Test', - severity: Severity::ERROR, - )); + $fileReport->addFoundViolations([$this->createViolation(message: 'Test message')]); $report = new Report(); $report->addFileReport($fileReport); @@ -389,7 +501,7 @@ public function itShowsNoPerformanceDataWhenEmpty(): void $reporter = new ConsoleReporter(useColors: false, showPerformance: true); $report = new Report(); - $report->incrementFilesScanned(); + $report->addFileReport(new FileReport('clean.xml')); $output = $reporter->generate($report); @@ -402,15 +514,17 @@ public function itShowsPerformanceSectionWithHeader(): void $reporter = new ConsoleReporter(useColors: false, showPerformance: true); $report = new Report(); - $report->incrementFilesScanned(); - - $report->setTotalTime(2.0); - $report->addSniffTime('SniffA', 1.0); + $report->measureWallTime(function () use ($report): void { + $fileReport = new FileReport('clean.xml', collectPerformance: true); + $fileReport->measureSniffer('SniffA', static fn() => usleep(1_000)); + $report->addFileReport($fileReport); + }); $output = $reporter->generate($report); self::assertStringContainsString('PERFORMANCE', $output); - self::assertStringContainsString('Total runtime: 2.000s', $output); + self::assertSame(1, substr_count($output, 'Total runtime:')); + self::assertStringContainsString('Sniffing', $output); } #[Test] @@ -419,11 +533,13 @@ public function itSortsSniffTimesBySlowestFirst(): void $reporter = new ConsoleReporter(useColors: false, showPerformance: true); $report = new Report(); - $report->setTotalTime(3.0); - - $report->addSniffTime('FastSniff', 0.5); - $report->addSniffTime('SlowSniff', 2.0); - $report->addSniffTime('MediumSniff', 1.0); + $report->measureWallTime(function () use ($report): void { + $fileReport = new FileReport('file.xml', collectPerformance: true); + $fileReport->measureSniffer('FastSniff', static fn() => usleep(1_000)); + $fileReport->measureSniffer('SlowSniff', static fn() => usleep(30_000)); + $fileReport->measureSniffer('MediumSniff', static fn() => usleep(10_000)); + $report->addFileReport($fileReport); + }); $output = $reporter->generate($report); @@ -441,13 +557,42 @@ public function itDisplaysTimeAndPercentagePerSniff(): void $reporter = new ConsoleReporter(useColors: false, showPerformance: true); $report = new Report(); - $report->setTotalTime(2.0); + $report->measureWallTime(function () use ($report): void { + $fileReport = new FileReport('file.xml', collectPerformance: true); + $fileReport->measureSniffer('SniffA', static fn() => usleep(1_000)); + $fileReport->measureSniffer('SniffB', static fn() => usleep(1_000)); + $report->addFileReport($fileReport); + }); - $report->addSniffTime('SniffA', 1.0); // 50% + $output = $reporter->generate($report); + + self::assertMatchesRegularExpression( + '/^ SniffA +\d+\.\d{3}s \( *\d+\.\d%\) *$/m', + $output, + ); + } + + #[Test] + public function itDisplaysFixingTimeAndPercentage(): void + { + $reporter = new ConsoleReporter(useColors: false, showPerformance: true); + + $report = new Report(); + $report->measureWallTime(function () use ($report): void { + $fileReport = new FileReport('file.xml', collectPerformance: true); + $fileReport->measureSniffer('SniffA', static fn() => usleep(1_000)); + $fileReport->measureFixing( + fn() => $fileReport->measureFixer('SniffA', static fn() => usleep(1_000)) + ); + $report->addFileReport($fileReport); + }); $output = $reporter->generate($report); - self::assertStringContainsString('1.000s ( 50.0%)', $output); + self::assertMatchesRegularExpression( + '/^ SniffA +\d+\.\d{3}s \( *\d+\.\d%\) +\d+\.\d{3}s \( *\d+\.\d%\) *$/m', + $output, + ); } #[Test] @@ -456,8 +601,11 @@ public function itDoesNotShowPerformanceWhenDisabled(): void $reporter = new ConsoleReporter(useColors: false, showPerformance: false); $report = new Report(); - $report->setTotalTime(2.0); - $report->addSniffTime('SniffA', 1.0); + + $fileReport = new FileReport('file.xml'); + $fileReport->measureSniffer('SniffA', static fn() => null); + + $report->addFileReport($fileReport); $output = $reporter->generate($report); diff --git a/tests/Unit/Report/Reporter/JsonReporterTest.php b/tests/Unit/Report/Reporter/JsonReporterTest.php index 8354a4c..83310e5 100644 --- a/tests/Unit/Report/Reporter/JsonReporterTest.php +++ b/tests/Unit/Report/Reporter/JsonReporterTest.php @@ -91,16 +91,44 @@ public function itCountsScannedFiles(): void { $report = new Report(); $report->addFileReport(new FileReport('a.xml')); - $report->incrementFilesScanned(); $report->addFileReport(new FileReport('b.xml')); - $report->incrementFilesScanned(); $data = $this->parseOutput($this->reporter->generate($report)); self::assertSame(2, $data['totals']['files_scanned'] ?? null); } + #[Test] + public function itIncludesFixingOutcome(): void + { + $first = new FileReport('first.xml'); + $first->markChanged(); + $first->recordFixingPass(); + $first->recordFixingPass(); + $first->addFoundViolations(array_fill(0, 4, $this->createViolation())); + $first->addFinalViolations([$this->createViolation()]); + + $second = new FileReport('second.xml'); + $second->markChanged(); + $second->recordFixingPass(); + $second->addFoundViolations(array_fill(0, 3, $this->createViolation())); + $second->addFinalViolations([$this->createViolation()]); + + $report = new Report(); + $report->addFileReport($first); + $report->addFileReport($second); + + $data = $this->parseOutput($this->reporter->generate($report)); + + self::assertSame([ + 'files_changed' => 2, + 'fixes_applied' => 5, + 'fixes_skipped' => 2, + 'fixing_passes' => 3, + ], $data['fixing']); + } + #[Test] public function itSkipsFilesWithNoViolations(): void { @@ -116,7 +144,7 @@ public function itSkipsFilesWithNoViolations(): void public function itIncludesFileWithViolations(): void { $fileReport = new FileReport('dirty.xml'); - $fileReport->addViolation($this->createViolation()); + $fileReport->addFoundViolations([$this->createViolation()]); $report = new Report(); $report->addFileReport($fileReport); @@ -130,8 +158,10 @@ public function itIncludesFileWithViolations(): void public function itSetsViolationCountPerFile(): void { $fileReport = new FileReport('file.xml'); - $fileReport->addViolation($this->createViolation(message: 'First')); - $fileReport->addViolation($this->createViolation(message: 'Second')); + $fileReport->addFoundViolations([ + $this->createViolation(message: 'First'), + $this->createViolation(message: 'Second'), + ]); $report = new Report(); $report->addFileReport($fileReport); @@ -145,7 +175,7 @@ public function itSetsViolationCountPerFile(): void public function itSetsLineInMessage(): void { $fileReport = new FileReport('file.xml'); - $fileReport->addViolation($this->createViolation(line: 42)); + $fileReport->addFoundViolations([$this->createViolation(line: 42)]); $report = new Report(); $report->addFileReport($fileReport); @@ -159,7 +189,7 @@ public function itSetsLineInMessage(): void public function itSetsSeverityInMessage(): void { $fileReport = new FileReport('file.xml'); - $fileReport->addViolation($this->createViolation(severity: Severity::WARNING)); + $fileReport->addFoundViolations([$this->createViolation(severity: Severity::WARNING)]); $report = new Report(); $report->addFileReport($fileReport); @@ -173,7 +203,7 @@ public function itSetsSeverityInMessage(): void public function itSetsMessageInMessage(): void { $fileReport = new FileReport('file.xml'); - $fileReport->addViolation($this->createViolation(message: 'Use instead')); + $fileReport->addFoundViolations([$this->createViolation(message: 'Use instead')]); $report = new Report(); $report->addFileReport($fileReport); @@ -187,7 +217,7 @@ public function itSetsMessageInMessage(): void public function itSetsSourceInMessage(): void { $fileReport = new FileReport('file.xml'); - $fileReport->addViolation($this->createViolation(sniffCode: 'DocbookCS.ExceptionName')); + $fileReport->addFoundViolations([$this->createViolation(sniffCode: 'DocbookCS.ExceptionName')]); $report = new Report(); $report->addFileReport($fileReport); @@ -201,9 +231,11 @@ public function itSetsSourceInMessage(): void public function itOutputsMultipleViolationsForOneFile(): void { $fileReport = new FileReport('multi.xml'); - $fileReport->addViolation($this->createViolation(message: 'First', line: 5)); - $fileReport->addViolation($this->createViolation(message: 'Second', line: 10)); - $fileReport->addViolation($this->createViolation(message: 'Third', line: 20)); + $fileReport->addFoundViolations([ + $this->createViolation(message: 'First', line: 5), + $this->createViolation(message: 'Second', line: 10), + $this->createViolation(message: 'Third', line: 20), + ]); $report = new Report(); $report->addFileReport($fileReport); @@ -217,10 +249,10 @@ public function itOutputsMultipleViolationsForOneFile(): void public function itOutputsMultipleFilesWithViolations(): void { $file1 = new FileReport('first.xml'); - $file1->addViolation($this->createViolation(message: 'Issue A')); + $file1->addFoundViolations([$this->createViolation(message: 'Issue A')]); $file2 = new FileReport('second.xml'); - $file2->addViolation($this->createViolation(message: 'Issue B')); + $file2->addFoundViolations([$this->createViolation(message: 'Issue B')]); $report = new Report(); $report->addFileReport($file1); @@ -239,7 +271,7 @@ public function itSkipsCleanFilesAmongDirtyOnes(): void $cleanFile = new FileReport('clean.xml'); $dirtyFile = new FileReport('dirty.xml'); - $dirtyFile->addViolation($this->createViolation()); + $dirtyFile->addFoundViolations([$this->createViolation()]); $report = new Report(); $report->addFileReport($cleanFile); @@ -256,11 +288,13 @@ public function itSkipsCleanFilesAmongDirtyOnes(): void public function itCountsTotalViolations(): void { $file1 = new FileReport('a.xml'); - $file1->addViolation($this->createViolation()); - $file1->addViolation($this->createViolation()); + $file1->addFoundViolations([ + $this->createViolation(), + $this->createViolation(), + ]); $file2 = new FileReport('b.xml'); - $file2->addViolation($this->createViolation()); + $file2->addFoundViolations([$this->createViolation()]); $report = new Report(); $report->addFileReport($file1); @@ -275,9 +309,11 @@ public function itCountsTotalViolations(): void public function itCountsTotalErrors(): void { $fileReport = new FileReport('file.xml'); - $fileReport->addViolation($this->createViolation(severity: Severity::ERROR)); - $fileReport->addViolation($this->createViolation(severity: Severity::WARNING)); - $fileReport->addViolation($this->createViolation(severity: Severity::ERROR)); + $fileReport->addFoundViolations([ + $this->createViolation(), + $this->createViolation(severity: Severity::WARNING), + $this->createViolation(), + ]); $report = new Report(); $report->addFileReport($fileReport); @@ -291,9 +327,11 @@ public function itCountsTotalErrors(): void public function itCountsTotalWarnings(): void { $fileReport = new FileReport('file.xml'); - $fileReport->addViolation($this->createViolation(severity: Severity::WARNING)); - $fileReport->addViolation($this->createViolation(severity: Severity::ERROR)); - $fileReport->addViolation($this->createViolation(severity: Severity::WARNING)); + $fileReport->addFoundViolations([ + $this->createViolation(severity: Severity::WARNING), + $this->createViolation(), + $this->createViolation(severity: Severity::WARNING), + ]); $report = new Report(); $report->addFileReport($fileReport); @@ -307,7 +345,7 @@ public function itCountsTotalWarnings(): void public function itDoesNotEscapeSlashesInOutput(): void { $fileReport = new FileReport('path/to/file.xml'); - $fileReport->addViolation($this->createViolation()); + $fileReport->addFoundViolations([$this->createViolation()]); $report = new Report(); $report->addFileReport($fileReport); @@ -322,7 +360,7 @@ public function itDoesNotEscapeSlashesInOutput(): void public function itRendersAbsoluteFilePathRelativeToWorkingDirectory(): void { $fileReport = new FileReport((getcwd() ?: '') . '/path/to/file.xml'); - $fileReport->addViolation($this->createViolation()); + $fileReport->addFoundViolations([$this->createViolation()]); $report = new Report(); $report->addFileReport($fileReport); @@ -358,7 +396,13 @@ public function itUsesPrettyPrintedJson(): void * message: string, * source: string * }> - * }> + * }>, + * fixing: array{ + * files_changed: int, + * fixes_applied: int, + * fixes_skipped: int, + * fixing_passes: int + * } * } */ private function parseOutput(string $json): array diff --git a/tests/Unit/Runner/FixConvergenceTest.php b/tests/Unit/Runner/FixConvergenceTest.php index 00cdf6d..71f5989 100644 --- a/tests/Unit/Runner/FixConvergenceTest.php +++ b/tests/Unit/Runner/FixConvergenceTest.php @@ -16,11 +16,12 @@ use DocbookCS\Report\Report; use DocbookCS\Runner\EntityExpansionMarker; use DocbookCS\Runner\EntityPreprocessor; +use DocbookCS\Runner\XmlFileProcessor; +use DocbookCS\Runner\XmlFixRunner; use DocbookCS\Runner\RunMode; use DocbookCS\Runner\RunScope; use DocbookCS\Runner\ViolationScopeFilter; -use DocbookCS\Runner\XmlFileProcessor; -use DocbookCS\Runner\XmlProcessingResult; +use DocbookCS\Runner\XmlSniffRunner; use DocbookCS\Sniff\AbstractSniff; use DocbookCS\Sniff\ExceptionNameSniff; use DocbookCS\Sniff\Fixable; @@ -39,6 +40,8 @@ #[ CoversClass(XmlFileProcessor::class), + CoversClass(XmlFixRunner::class), + CoversClass(XmlSniffRunner::class), // UsesClass(AbstractSniff::class), UsesClass(EntityExpansionMarker::class), @@ -56,13 +59,12 @@ UsesClass(Line::class), UsesClass(Report::class), UsesClass(RunMode::class), + UsesClass(RunScope::class), UsesClass(SimparaFixer::class), UsesClass(SimparaSniff::class), UsesClass(SourceRange::class), - UsesClass(RunScope::class), UsesClass(Violation::class), UsesClass(ViolationScopeFilter::class), - UsesClass(XmlProcessingResult::class), ] final class FixConvergenceTest extends TestCase { @@ -73,10 +75,10 @@ public function itAppliesIndependentSameLineFixesAndReportsTheFinalSource(): voi $filePath = $this->temporaryFile($source); try { - $processor = new XmlFileProcessor([ - new SimparaSniff(RunMode::Fix), - new ExceptionNameSniff(RunMode::Fix), - ]); + $processor = new XmlFileProcessor(new XmlSniffRunner(RunMode::Fix, [ + new SimparaSniff(), + new ExceptionNameSniff(), + ])); $report = $this->processFile($processor, $filePath); @@ -84,7 +86,8 @@ public function itAppliesIndependentSameLineFixesAndReportsTheFinalSource(): voi 'ARuntimeException', file_get_contents($filePath), ); - self::assertFalse($report->hasViolations()); + self::assertFalse($report->hasFinalViolations()); + self::assertSame(1, $report->fixingPasses); } finally { @unlink($filePath); } @@ -97,7 +100,7 @@ public function itReportsRemainingViolationsAtTheirFinalLines(): void $filePath = $this->temporaryFile($source); try { - $lineBreakSniff = new class (RunMode::Fix) extends AbstractSniff implements Fixable { + $lineBreakSniff = new class extends AbstractSniff implements Fixable { private const string ELEMENT = ''; public static function getCode(): string @@ -129,7 +132,7 @@ public function process(\DOMDocument $document, File $file): array )]; } }; - $badElementSniff = new class (RunMode::Fix) extends AbstractSniff { + $badElementSniff = new class extends AbstractSniff { public static function getCode(): string { return 'Test.BadElement'; @@ -154,16 +157,16 @@ public function process(\DOMDocument $document, File $file): array )]; } }; - $processor = new XmlFileProcessor([ + $processor = new XmlFileProcessor(new XmlSniffRunner(RunMode::Fix, [ $lineBreakSniff, $badElementSniff, - ]); + ])); $report = $this->processFile($processor, $filePath); self::assertSame("\n", file_get_contents($filePath)); - self::assertSame(1, $report->getViolationCount()); - self::assertSame(2, $report->violations[0]->rangeOne()->line); + self::assertSame(1, $report->getFinalViolationCount()); + self::assertSame(2, $report->finalViolations[0]->rangeOne()->line); } finally { @unlink($filePath); } @@ -176,7 +179,7 @@ public function itKeepsChangedLineScopeAlignedAfterFixes(): void $filePath = $this->temporaryFile($source); try { - $lineBreakSniff = new class (RunMode::Fix) extends AbstractSniff implements Fixable { + $lineBreakSniff = new class extends AbstractSniff implements Fixable { public static function getCode(): string { return 'Test.ScopedLineBreak'; @@ -202,7 +205,7 @@ public function process(\DOMDocument $document, File $file): array )]; } }; - $badElementSniff = new class (RunMode::Fix) extends AbstractSniff { + $badElementSniff = new class extends AbstractSniff { public static function getCode(): string { return 'Test.ScopedBadElement'; @@ -224,7 +227,9 @@ public function process(\DOMDocument $document, File $file): array )]; } }; - $processor = new XmlFileProcessor([$lineBreakSniff, $badElementSniff]); + $processor = new XmlFileProcessor( + new XmlSniffRunner(RunMode::Fix, [$lineBreakSniff, $badElementSniff]) + ); $report = $this->processFile( $processor, @@ -233,8 +238,8 @@ public function process(\DOMDocument $document, File $file): array ); self::assertSame("\n\n\n", file_get_contents($filePath)); - self::assertSame(1, $report->getViolationCount()); - self::assertSame(3, $report->violations[0]->rangeOne()->line); + self::assertSame(1, $report->getFinalViolationCount()); + self::assertSame(3, $report->finalViolations[0]->rangeOne()->line); } finally { @unlink($filePath); } @@ -247,7 +252,7 @@ public function itDoesNotPersistFixesThatCycle(): void $filePath = $this->temporaryFile($source); try { - $toggleElementSniff = new class (RunMode::Fix) extends AbstractSniff implements Fixable { + $toggleElementSniff = new class extends AbstractSniff implements Fixable { public static function getCode(): string { return 'Test.ToggleElement'; @@ -273,9 +278,9 @@ public function process(\DOMDocument $document, File $file): array )]; } }; - $processor = new XmlFileProcessor([ + $processor = new XmlFileProcessor(new XmlSniffRunner(RunMode::Fix, [ $toggleElementSniff, - ]); + ])); try { $this->processFile($processor, $filePath); @@ -297,7 +302,7 @@ public function itDoesNotPersistFixesThatProduceInvalidXml(): void $filePath = $this->temporaryFile($source); try { - $invalidXmlSniff = new class (RunMode::Fix) extends AbstractSniff implements Fixable { + $invalidXmlSniff = new class extends AbstractSniff implements Fixable { public static function getCode(): string { return 'Test.InvalidXml'; @@ -322,7 +327,7 @@ public function process(\DOMDocument $document, File $file): array )]; } }; - $processor = new XmlFileProcessor([$invalidXmlSniff]); + $processor = new XmlFileProcessor(new XmlSniffRunner(RunMode::Fix, [$invalidXmlSniff])); try { $this->processFile($processor, $filePath); @@ -342,12 +347,17 @@ private function processFile(XmlFileProcessor $processor, string $path, ?FileCha $content = file_get_contents($path); self::assertIsString($content); - $result = $processor->process(new File($path, $content), $fileChange); - if ($result->isModified()) { - file_put_contents($path, $result->fixedContent()); + $fixedFile = $processor->process( + $file = new File($path, $content), + $fileReport = new FileReport($path), + RunScope::fromFileAndFileChange($file, $fileChange) + ); + + if ($fixedFile !== null) { + file_put_contents($path, $fixedFile->content); } - return $result->fileReport; + return $fileReport; } private function temporaryFile(string $content): string diff --git a/tests/Unit/Runner/RunCoordinatorFileFailureTest.php b/tests/Unit/Runner/RunCoordinatorFileFailureTest.php index 477509b..83c236d 100644 --- a/tests/Unit/Runner/RunCoordinatorFileFailureTest.php +++ b/tests/Unit/Runner/RunCoordinatorFileFailureTest.php @@ -9,7 +9,6 @@ use DocbookCS\Diff\FileChange; use DocbookCS\Progress\ProgressInterface; use DocbookCS\Runner\RunCoordinator; -use DocbookCS\Runner\RunPlan; use DocbookCS\Runner\RunPlanner; use PHPUnit\Framework\Attributes\CoversNothing; use PHPUnit\Framework\Attributes\Test; @@ -55,9 +54,11 @@ public function finish(): void basePath: dirname($xmlFilePath), ); - $report = new RunCoordinator($progress)->run($this->planPaths($config)); + $report = new RunCoordinator($progress)->runWithMetrics( + new RunPlanner($config)->planPaths($config->getIncludePaths()), + ); - self::assertTrue($report->hasViolations()); + self::assertTrue($report->hasFinalViolations()); self::assertSame('DocbookCS.Internal', $report->getAllViolations()[0]->sniffCode); self::assertStringContainsString('Could not read file', $report->getAllViolations()[0]->message); } @@ -89,19 +90,11 @@ static function () use ($xmlFilePath): void { ); $diff = new DiffChangeset([new FileChange($xmlFilePath, [42])]); - $report = new RunCoordinator($progress)->run($this->planDiff($config, $diff)); + $report = new RunCoordinator($progress)->runWithMetrics( + new RunPlanner($config)->planDiff($diff), + ); - self::assertTrue($report->hasViolations()); + self::assertTrue($report->hasFinalViolations()); self::assertSame('DocbookCS.Internal', $report->getAllViolations()[0]->sniffCode); } - - private function planPaths(ConfigData $config): RunPlan - { - return new RunPlanner($config)->planPaths($config->getIncludePaths()); - } - - private function planDiff(ConfigData $config, DiffChangeset $diff): RunPlan - { - return new RunPlanner($config)->planDiff($diff); - } } diff --git a/tests/Unit/Runner/RunReportingTest.php b/tests/Unit/Runner/RunReportingTest.php new file mode 100644 index 0000000..2510b8b --- /dev/null +++ b/tests/Unit/Runner/RunReportingTest.php @@ -0,0 +1,113 @@ +Text'); + + try { + $plan = new RunPlan( + mode: RunMode::Fix, + sniffs: [new SniffEntry(SimparaSniff::class)], + targets: [$filePath => null], + entities: [], + ); + + $report = new RunCoordinator(collectPerformance: true)->runWithMetrics($plan); + + self::assertSame('Text', file_get_contents($filePath)); + self::assertSame(1, $report->getChangedFilesCount()); + self::assertSame(1, $report->getFoundViolationsCount()); + self::assertSame(1, $report->getAppliedFixesCount()); + self::assertSame(0, $report->getSkippedFixesCount()); + self::assertSame(1, $report->getFixingPassesCount()); + self::assertFalse($report->hasFinalViolations()); + self::assertArrayHasKey(SimparaSniff::getCode(), $report->getSniffingTimes()); + self::assertArrayHasKey(SimparaSniff::getCode(), $report->getFixingTimes()); + self::assertGreaterThanOrEqual( + array_sum($report->getSniffingTimes()), + $report->getTotalSniffingTime(), + ); + self::assertGreaterThan(0.0, $report->getTotalFixingTime()); + } finally { + @unlink($filePath); + } + } + + #[Test] + public function itReportsInitialAndFinalViolationsAfterFixingShiftsTheSource(): void + { + $filePath = tempnam(sys_get_temp_dir(), 'docbook-cs-reporting-'); + self::assertIsString($filePath); + file_put_contents($filePath, 'Text'); + + $badElementSniff = new class () extends AbstractSniff { + public static function getCode(): string + { + return 'Test.BadElement'; + } + + public function process(\DOMDocument $document, File $file): array + { + $offset = strpos($file->content, ''); + if ($offset === false) { + return []; + } + + return [$this->createViolation( + $file->path, + 'Bad element.', + [new SourceRange(1, $offset, $offset + strlen(''), '')], + )]; + } + }; + + try { + $report = new RunCoordinator()->runWithMetrics(new RunPlan( + mode: RunMode::Fix, + sniffs: [ + new SniffEntry(SimparaSniff::class), + new SniffEntry($badElementSniff::class), + ], + targets: [$filePath => null], + entities: [], + )); + + $fileReport = $report->fileReports[$filePath]; + + self::assertSame(2, $fileReport->getFoundViolationCount()); + self::assertSame(SimparaSniff::getCode(), $fileReport->foundViolations[0]->sniffCode); + self::assertSame(1, $fileReport->getFinalViolationCount()); + self::assertSame('Test.BadElement', $fileReport->finalViolations[0]->sniffCode); + self::assertGreaterThan( + $fileReport->foundViolations[1]->rangeOne()->beginOffset, + $fileReport->finalViolations[0]->rangeOne()->beginOffset, + ); + self::assertSame(1, $report->getAppliedFixesCount()); + self::assertSame(1, $report->getSkippedFixesCount()); + } finally { + @unlink($filePath); + } + } +} diff --git a/tests/Unit/Runner/RunScopeTest.php b/tests/Unit/Runner/RunScopeTest.php index ad5e580..c439bef 100644 --- a/tests/Unit/Runner/RunScopeTest.php +++ b/tests/Unit/Runner/RunScopeTest.php @@ -26,6 +26,8 @@ use DocbookCS\Report\Report; use DocbookCS\Runner\EntityExpansionMarker; use DocbookCS\Runner\EntityPreprocessor; +use DocbookCS\Runner\XmlFileProcessor; +use DocbookCS\Runner\XmlFixRunner; use DocbookCS\Runner\RunCoordinator; use DocbookCS\Runner\RunMode; use DocbookCS\Runner\RunPlan; @@ -33,8 +35,7 @@ use DocbookCS\Runner\RunScope; use DocbookCS\Runner\RunScopeResolver; use DocbookCS\Runner\ViolationScopeFilter; -use DocbookCS\Runner\XmlFileProcessor; -use DocbookCS\Runner\XmlProcessingResult; +use DocbookCS\Runner\XmlSniffRunner; use DocbookCS\Sniff\AbstractSniff; use DocbookCS\Sniff\SimparaSniff; use DocbookCS\Source\File; @@ -84,7 +85,8 @@ UsesClass(Violation::class), UsesClass(ViolationScopeFilter::class), UsesClass(XmlFileProcessor::class), - UsesClass(XmlProcessingResult::class), + UsesClass(XmlFixRunner::class), + UsesClass(XmlSniffRunner::class), ] final class RunScopeTest extends TestCase { @@ -120,14 +122,14 @@ public function itExpandsReferencedTargetsOnlyWhenWideScopeIsRequested(): void { $config = $this->config(); - self::assertSame(1, $this->executePaths($config, [$this->sourceFile])->getFilesScanned()); + self::assertSame(1, $this->executePaths($config, [$this->sourceFile])->getScannedFilesCount()); self::assertSame( 2, $this->executePaths( $config, [$this->sourceFile], wide: true, - )->getFilesScanned(), + )->getScannedFilesCount(), ); } @@ -148,7 +150,7 @@ public function itFixesExpandedXmlInItsTargetFileOnly(): void self::assertSame('', file_get_contents($this->sourceFile)); self::assertSame('Text', file_get_contents($this->targetFile)); - self::assertFalse($report->hasViolations()); + self::assertFalse($report->hasFinalViolations()); } #[Test] @@ -163,14 +165,13 @@ public function aDiffProvidesItsOwnFilesWithoutConfiguredIncludePaths(): void basePath: $this->directory, ); - $report = $this->executeDiff( - $config, - new DiffChangeset([ + $report = new RunCoordinator()->runWithMetrics( + new RunPlanner($config)->planDiff(new DiffChangeset([ new FileChange($this->sourceFile, [1]), - ]), + ])), ); - self::assertSame(1, $report->getFilesScanned()); + self::assertSame(1, $report->getScannedFilesCount()); } #[Test] @@ -185,14 +186,13 @@ public function aDiffPathUsingAProjectDirectoryKeepsItsSourceRanges(): void basePath: $this->directory, ); - $report = $this->executeDiff( - $config, - new DiffChangeset([ + $report = new RunCoordinator()->runWithMetrics( + new RunPlanner($config)->planDiff(new DiffChangeset([ new FileChange('docs/source.xml', [1]), - ]), + ])), ); - self::assertSame(1, $report->getFilesScanned()); + self::assertSame(1, $report->getScannedFilesCount()); } /** @param list $sniffs */ @@ -215,19 +215,8 @@ private function executePaths( RunMode $mode = RunMode::Sniff, bool $wide = false, ): Report { - return new RunCoordinator()->run( + return new RunCoordinator()->runWithMetrics( new RunPlanner($config, $mode, $wide)->planPaths($paths), ); } - - private function executeDiff( - ConfigData $config, - DiffChangeset $diff, - RunMode $mode = RunMode::Sniff, - bool $wide = false, - ): Report { - return new RunCoordinator()->run( - new RunPlanner($config, $mode, $wide)->planDiff($diff), - ); - } } diff --git a/tests/Unit/Runner/SniffRunnerTest.php b/tests/Unit/Runner/SniffRunnerTest.php index 44657f3..eca2c75 100644 --- a/tests/Unit/Runner/SniffRunnerTest.php +++ b/tests/Unit/Runner/SniffRunnerTest.php @@ -22,6 +22,7 @@ use DocbookCS\Report\Report; use DocbookCS\Runner\EntityExpansionMarker; use DocbookCS\Runner\EntityPreprocessor; +use DocbookCS\Runner\XmlFileProcessor; use DocbookCS\Runner\RunCoordinator; use DocbookCS\Runner\RunMode; use DocbookCS\Runner\RunPlan; @@ -29,8 +30,7 @@ use DocbookCS\Runner\RunScope; use DocbookCS\Runner\RunScopeResolver; use DocbookCS\Runner\ViolationScopeFilter; -use DocbookCS\Runner\XmlFileProcessor; -use DocbookCS\Runner\XmlProcessingResult; +use DocbookCS\Runner\XmlSniffRunner; use DocbookCS\Sniff\SniffInterface; use DocbookCS\Source\File; use DocbookCS\Source\Line; @@ -57,7 +57,7 @@ CoversClass(RunPlanner::class), CoversClass(SniffEntry::class), CoversClass(Violation::class), - CoversClass(XmlFileProcessor::class), + CoversClass(XmlSniffRunner::class), // UsesClass(DiffBaseResolver::class), UsesClass(DiffChangeset::class), @@ -73,7 +73,7 @@ UsesClass(SourceRange::class), UsesClass(UpstreamResolver::class), UsesClass(ViolationScopeFilter::class), - UsesClass(XmlProcessingResult::class), + UsesClass(XmlFileProcessor::class), ] final class SniffRunnerTest extends TestCase { @@ -98,11 +98,11 @@ public function itProcessesFilesWithoutViolations(): void $config = $this->createConfig(); $runner = new RunCoordinator(); - $report = $runner->run($this->planPaths($config)); + $report = $runner->runWithMetrics(new RunPlanner($config)->planPaths($config->getIncludePaths())); - self::assertSame(2, $report->getFilesScanned()); - self::assertFalse($report->hasViolations()); - self::assertCount(0, $report->getFileReports()); + self::assertSame(2, $report->getScannedFilesCount()); + self::assertFalse($report->hasFinalViolations()); + self::assertCount(2, $report->fileReports); } #[Test] // TODO: should be integration @@ -111,12 +111,11 @@ public function itUsesOverridePathsWhenProvided(): void $config = $this->createConfig(); $runner = new RunCoordinator(); - $report = $runner->run($this->planPaths( - $config, + $report = $runner->runWithMetrics(new RunPlanner($config)->planPaths( [self::FIXTURE_DIR . '/../override'], )); - self::assertSame(1, $report->getFilesScanned()); + self::assertSame(1, $report->getScannedFilesCount()); } #[Test] // TODO: should be integration @@ -137,17 +136,13 @@ public function itCallsProgressMethods(): void $config = $this->createConfig(); $runner = new RunCoordinator($progress); - $runner->run($this->planPaths($config)); + $runner->runWithMetrics(new RunPlanner($config)->planPaths($config->getIncludePaths())); } #[Test] // TODO: should be integration public function itAddsFileReportsForFilesWithViolations(): void { - $sniff = new class (RunMode::Sniff) implements SniffInterface { - public function __construct(public RunMode $mode) - { - } - + $sniff = new class implements SniffInterface { public static function getCode(): string { return 'Test.ViolatingSniff'; @@ -174,21 +169,17 @@ public function setProperty(string $name, string $value): void $config = $this->createConfig(sniffs: [new SniffEntry($sniff::class)]); $runner = new RunCoordinator(); - $report = $runner->run($this->planPaths($config)); + $report = $runner->runWithMetrics(new RunPlanner($config)->planPaths($config->getIncludePaths())); - self::assertSame(2, $report->getFilesScanned()); - self::assertCount(2, $report->getFileReports()); - self::assertTrue($report->hasViolations()); + self::assertSame(2, $report->getScannedFilesCount()); + self::assertCount(2, $report->fileReports); + self::assertTrue($report->hasFinalViolations()); } #[Test] // TODO: should be integration public function itStoresAbsolutePathsInFileReports(): void { - $sniff = new class (RunMode::Sniff) implements SniffInterface { - public function __construct(public RunMode $mode) - { - } - + $sniff = new class implements SniffInterface { public static function getCode(): string { return 'Test.ViolatingSniff'; @@ -215,9 +206,9 @@ public function setProperty(string $name, string $value): void $config = $this->createConfig(sniffs: [new SniffEntry($sniff::class)]); $runner = new RunCoordinator(); - $report = $runner->run($this->planPaths($config)); + $report = $runner->runWithMetrics(new RunPlanner($config)->planPaths($config->getIncludePaths())); - foreach ($report->getFileReports() as $fileReport) { + foreach ($report->fileReports as $fileReport) { self::assertTrue( str_starts_with($fileReport->filePath, '/'), 'Expected absolute path, got: ' . $fileReport->filePath, @@ -228,14 +219,8 @@ public function setProperty(string $name, string $value): void #[Test] // TODO: should be integration public function itPassesPropertiesToSniffs(): void { - $sniffClass = new class (RunMode::Sniff) implements SniffInterface { + $sniffClass = new class implements SniffInterface { public static string $captured = ''; - public static RunMode $capturedMode = RunMode::Sniff; - - public function __construct(public RunMode $mode) - { - self::$capturedMode = $mode; - } public function setProperty(string $name, string $value): void { @@ -256,10 +241,9 @@ public function process(\DOMDocument $document, File $file): array $config = $this->createConfig(sniffs: [new SniffEntry($sniffClass::class, ['someProp' => 'someValue'])]); $runner = new RunCoordinator(); - $runner->run($this->planPaths($config, mode: RunMode::Fix)); + $runner->runWithMetrics(new RunPlanner($config)->planPaths($config->getIncludePaths())); self::assertSame('someValue', $sniffClass::$captured); - self::assertSame(RunMode::Fix, $sniffClass::$capturedMode); } #[Test] // TODO: should be integration @@ -272,7 +256,7 @@ public function itThrowsWhenSniffClassDoesNotExist(): void $this->expectException(\RuntimeException::class); $this->expectExceptionMessageIsOrContains('does not exist'); - $runner->run($this->planPaths($config)); + $runner->runWithMetrics(new RunPlanner($config)->planPaths($config->getIncludePaths())); } #[Test] // TODO: should be integration @@ -285,7 +269,7 @@ public function itThrowsWhenClassDoesNotImplementSniffInterface(): void $this->expectException(\RuntimeException::class); $this->expectExceptionMessageIsOrContains('does not implement'); - $runner->run($this->planPaths($config)); + $runner->runWithMetrics(new RunPlanner($config)->planPaths($config->getIncludePaths())); } #[Test] // TODO: should be integration @@ -295,9 +279,9 @@ public function itFiltersFilesToOnlyThoseInTheDiff(): void $runner = new RunCoordinator(); $diff = new DiffChangeset([new FileChange(self::FIXTURE_DIR . '/file_a.xml', [1])]); - $report = $runner->run($this->planDiff($config, $diff)); + $report = $runner->runWithMetrics(new RunPlanner($config)->planDiff($diff)); - self::assertSame(1, $report->getFilesScanned()); + self::assertSame(1, $report->getScannedFilesCount()); } #[Test] // TODO: should be integration @@ -307,9 +291,9 @@ public function itScansNoFilesWhenDiffContainsNoMatchingPaths(): void $runner = new RunCoordinator(); $diff = new DiffChangeset([new FileChange('completely/different/file.xml', [1, 2, 3])]); - $report = $runner->run($this->planDiff($config, $diff)); + $report = $runner->runWithMetrics(new RunPlanner($config)->planDiff($diff)); - self::assertSame(0, $report->getFilesScanned()); + self::assertSame(0, $report->getScannedFilesCount()); } #[Test] // TODO: should be integration @@ -321,9 +305,9 @@ public function itMatchesWhenDiffPathEqualsDiscoveredPath(): void $discoveredPath = self::FIXTURE_DIR . '/file_a.xml'; $diff = new DiffChangeset([new FileChange($discoveredPath, [1])]); - $report = $runner->run($this->planDiff($config, $diff)); + $report = $runner->runWithMetrics(new RunPlanner($config)->planDiff($diff)); - self::assertSame(1, $report->getFilesScanned()); + self::assertSame(1, $report->getScannedFilesCount()); } #[Test] // TODO: should be integration @@ -332,9 +316,9 @@ public function itScansAllFilesWhenNoDiffIsGiven(): void $config = $this->createConfig(); $runner = new RunCoordinator(); - $report = $runner->run($this->planPaths($config)); + $report = $runner->runWithMetrics(new RunPlanner($config)->planPaths($config->getIncludePaths())); - self::assertSame(2, $report->getFilesScanned()); + self::assertSame(2, $report->getScannedFilesCount()); } #[Test] // TODO: should be integration @@ -371,9 +355,9 @@ public function itScansLexicallyEquivalentWideTargetsOnlyOnce(): void ], ); - $report = new RunCoordinator()->run($plan); + $report = new RunCoordinator()->runWithMetrics($plan); - self::assertSame(2, $report->getFilesScanned()); + self::assertSame(2, $report->getScannedFilesCount()); } finally { @unlink($sourceFile); @unlink($targetFile); @@ -385,11 +369,7 @@ public function itScansLexicallyEquivalentWideTargetsOnlyOnce(): void #[Test] // TODO: should be integration public function itReportsNoViolationsForFilesInDiffWithoutAddedLines(): void { - $sniff = new class (RunMode::Sniff) implements SniffInterface { - public function __construct(public RunMode $mode) - { - } - + $sniff = new class implements SniffInterface { public static function getCode(): string { return 'Test.ViolatingSniff'; @@ -417,20 +397,9 @@ public function setProperty(string $name, string $value): void $runner = new RunCoordinator(); $diff = new DiffChangeset([new FileChange(self::FIXTURE_DIR . '/file_a.xml', [])]); - $report = $runner->run($this->planDiff($config, $diff)); - - self::assertSame(1, $report->getFilesScanned()); - self::assertFalse($report->hasViolations()); - } + $report = $runner->runWithMetrics(new RunPlanner($config)->planDiff($diff)); - /** @param list|null $paths */ - private function planPaths(ConfigData $config, ?array $paths = null, RunMode $mode = RunMode::Sniff): RunPlan - { - return new RunPlanner($config, $mode)->planPaths($paths ?? $config->getIncludePaths()); - } - - private function planDiff(ConfigData $config, DiffChangeset $diff, RunMode $mode = RunMode::Sniff): RunPlan - { - return new RunPlanner($config, $mode)->planDiff($diff); + self::assertSame(1, $report->getScannedFilesCount()); + self::assertFalse($report->hasFinalViolations()); } } diff --git a/tests/Unit/Runner/SourceRangeScopeTest.php b/tests/Unit/Runner/SourceRangeScopeTest.php index 89ab020..b506134 100644 --- a/tests/Unit/Runner/SourceRangeScopeTest.php +++ b/tests/Unit/Runner/SourceRangeScopeTest.php @@ -7,18 +7,19 @@ use DocbookCS\Diff\FileChange; use DocbookCS\Fix\Fix; use DocbookCS\Fix\FixApplier; +use DocbookCS\Fix\Fixer\SimparaFixer; use DocbookCS\Fix\FixPlan; use DocbookCS\Fix\FixResult; -use DocbookCS\Fix\Fixer\SimparaFixer; use DocbookCS\Report\FileReport; use DocbookCS\Report\Report; use DocbookCS\Runner\EntityExpansionMarker; use DocbookCS\Runner\EntityPreprocessor; +use DocbookCS\Runner\XmlFileProcessor; +use DocbookCS\Runner\XmlFixRunner; use DocbookCS\Runner\RunMode; use DocbookCS\Runner\RunScope; use DocbookCS\Runner\ViolationScopeFilter; -use DocbookCS\Runner\XmlFileProcessor; -use DocbookCS\Runner\XmlProcessingResult; +use DocbookCS\Runner\XmlSniffRunner; use DocbookCS\Sniff\SimparaSniff; use DocbookCS\Source\File; use DocbookCS\Source\Line; @@ -30,9 +31,10 @@ use PHPUnit\Framework\TestCase; #[ - CoversClass(SimparaSniff::class), CoversClass(RunScope::class), + CoversClass(SimparaSniff::class), CoversClass(XmlFileProcessor::class), + CoversClass(XmlSniffRunner::class), // UsesClass(EntityExpansionMarker::class), UsesClass(EntityPreprocessor::class), @@ -50,7 +52,7 @@ UsesClass(SourceRange::class), UsesClass(Violation::class), UsesClass(ViolationScopeFilter::class), - UsesClass(XmlProcessingResult::class), + UsesClass(XmlFixRunner::class), ] final class SourceRangeScopeTest extends TestCase { @@ -76,19 +78,20 @@ public function itAppliesEveryRangeOfAViolationIntersectingAChangedLine(): void file_put_contents($filePath, $source); try { - $processor = new XmlFileProcessor([ - new SimparaSniff(RunMode::Fix), - ]); + $processor = new XmlFileProcessor(new XmlSniffRunner(RunMode::Fix, [ + new SimparaSniff(), + ])); - $result = $processor->process( - new File($filePath, $source), - new FileChange($filePath, [3]), + $fixedFile = $processor->process( + $file = new File($filePath, $source), + $fileReport = new FileReport($filePath), + RunScope::fromFileAndFileChange($file, new FileChange($filePath, [3])), ); - file_put_contents($filePath, $result->fixedContent()); - $report = $result->fileReport; + self::assertNotNull($fixedFile); + file_put_contents($filePath, $fixedFile->content); self::assertSame($expected, file_get_contents($filePath)); - self::assertFalse($report->hasViolations()); + self::assertFalse($fileReport->hasFinalViolations()); } finally { @unlink($filePath); } @@ -111,16 +114,18 @@ public function itFixesAViolationCausedByADeletedLine(): void XML; - $processor = new XmlFileProcessor([ - new SimparaSniff(RunMode::Fix), - ]); + $processor = new XmlFileProcessor(new XmlSniffRunner(RunMode::Fix, [ + new SimparaSniff(), + ])); - $result = $processor->process( - new File('file.xml', $source), - new FileChange('file.xml', [], deletionAnchors: [3]), + $fixedFile = $processor->process( + $file = new File('file.xml', $source), + $fileReport = new FileReport('file.xml'), + RunScope::fromFileAndFileChange($file, new FileChange('file.xml', [], deletionAnchors: [3])), ); - self::assertSame($expected, $result->fixedContent()); - self::assertFalse($result->fileReport->hasViolations()); + self::assertNotNull($fixedFile); + self::assertSame($expected, $fixedFile->content); + self::assertFalse($fileReport->hasFinalViolations()); } } diff --git a/tests/Unit/Runner/XmlFileProcessorPipelineTest.php b/tests/Unit/Runner/XmlFileProcessorPipelineTest.php index 4a117d5..a58be5a 100644 --- a/tests/Unit/Runner/XmlFileProcessorPipelineTest.php +++ b/tests/Unit/Runner/XmlFileProcessorPipelineTest.php @@ -4,12 +4,13 @@ namespace DocbookCS\Tests\Unit\Runner; +use DocbookCS\Report\FileReport; use DocbookCS\Runner\EntityPreprocessor; -use DocbookCS\Runner\RunMode; use DocbookCS\Runner\XmlFileProcessor; -use DocbookCS\Report\FileReport; +use DocbookCS\Runner\RunMode; +use DocbookCS\Runner\RunScope; +use DocbookCS\Runner\XmlSniffRunner; use DocbookCS\Sniff\AttributeOrderSniff; -use DocbookCS\Sniff\SniffInterface; use DocbookCS\Source\File; use PHPUnit\Framework\Attributes\CoversNothing; use PHPUnit\Framework\Attributes\Test; @@ -21,55 +22,44 @@ final class XmlFileProcessorPipelineTest extends TestCase #[Test] public function itKeepsTheActualSourcePathInViolations(): void { - $workingDirectory = getcwd(); - self::assertIsString($workingDirectory); - - $filePath = tempnam($workingDirectory, 'docbook-cs-'); - self::assertIsString($filePath); - - try { - file_put_contents($filePath, ''); + $file = new File( + '/project/reference/file.xml', + '', + ); + $fileReport = new FileReport($file->path); - $report = $this->process( - $this->processor([new AttributeOrderSniff()]), - '', - $filePath, - ); + new XmlFileProcessor( + new XmlSniffRunner(RunMode::Sniff, [new AttributeOrderSniff()]), + )->process($file, $fileReport, RunScope::fromFileAndFileChange($file, null)); - self::assertCount(1, $report->getViolations()); - self::assertSame($filePath, $report->getViolations()[0]->filePath); - self::assertSame($filePath, $report->filePath); - } finally { - @unlink($filePath); - } + self::assertSame(1, $fileReport->getFinalViolationCount()); + self::assertSame($file->path, $fileReport->finalViolations[0]->filePath); + self::assertSame($file->path, $fileReport->filePath); } #[Test] public function itAppliesFixesToTheOriginalSourceWhenEntitiesExpandBeforeTheViolation(): void { - $filePath = tempnam(sys_get_temp_dir(), 'docbook-cs-'); - self::assertIsString($filePath); - $source = '&prefix;'; - - try { - file_put_contents($filePath, $source); - - $processor = $this->processor( - [new AttributeOrderSniff(RunMode::Fix)], + $file = new File( + 'input.xml', + '&prefix;', + ); + $fileReport = new FileReport($file->path); + $fixedFile = new XmlFileProcessor( + new XmlSniffRunner( + RunMode::Fix, + [new AttributeOrderSniff()], new EntityPreprocessor([ 'prefix' => 'expanded-content-before-tag', ]), - ); + ), + )->process($file, $fileReport, RunScope::fromFileAndFileChange($file, null)); - $this->processFile($processor, $filePath); - - self::assertSame( - '&prefix;', - file_get_contents($filePath), - ); - } finally { - @unlink($filePath); - } + self::assertNotNull($fixedFile); + self::assertSame( + '&prefix;', + $fixedFile->content, + ); } #[Test] // TODO: should be integration @@ -82,58 +72,41 @@ public function itHandlesEntitiesWithoutParseErrors(): void ' ); - $processor = $this->processor([], new EntityPreprocessor([ - 'link.superglobals' => '', - 'php.ini' => '', - ])); - - $report = $this->process($processor, $xml); - - self::assertCount( - 0, - array_filter( - $report->getViolations(), - fn($v) => $v->sniffCode === 'DocbookCS.Internal' - ) + $file = new File('input.xml', $xml); + $fileReport = new FileReport($file->path); + new XmlFileProcessor( + new XmlSniffRunner(RunMode::Sniff, [], new EntityPreprocessor([ + 'link.superglobals' => '', + 'php.ini' => '', + ])), + )->process( + $file, + $fileReport, + RunScope::fromFileAndFileChange($file, null), ); + + self::assertFalse($fileReport->hasFinalViolations()); } #[Test] // TODO: should be integration public function itUsesCustomPreprocessor(): void { - $processor = $this->processor([], new EntityPreprocessor([ - 'custom.entity' => '[X]', - ])); - $xml = $this->xml('&custom.entity;'); - - $report = $this->process($processor, $xml); - - self::assertCount( - 0, - array_filter( - $report->getViolations(), - fn($v) => $v->sniffCode === 'DocbookCS.Internal' - ) + $file = new File('input.xml', $xml); + $fileReport = new FileReport($file->path); + new XmlFileProcessor( + new XmlSniffRunner( + RunMode::Sniff, + [], + new EntityPreprocessor(['custom.entity' => '[X]']), + ), + )->process( + $file, + $fileReport, + RunScope::fromFileAndFileChange($file, null), ); - } - - private function process(XmlFileProcessor $processor, string $content, string $path = 'input.xml'): FileReport - { - return $processor->process(new File($path, $content))->fileReport; - } - private function processFile(XmlFileProcessor $processor, string $path): FileReport - { - $content = file_get_contents($path); - self::assertIsString($content); - - $result = $processor->process(new File($path, $content)); - if ($result->isModified()) { - file_put_contents($path, $result->fixedContent()); - } - - return $result->fileReport; + self::assertFalse($fileReport->hasFinalViolations()); } private function xml(string $body): string @@ -143,13 +116,4 @@ private function xml(string $body): string $body XML; } - - /** @param list $sniffs */ - private function processor(array $sniffs = [], ?EntityPreprocessor $pre = null): XmlFileProcessor - { - return new XmlFileProcessor( - $sniffs, - $pre ?? new EntityPreprocessor([]) // always pass array - ); - } } diff --git a/tests/Unit/Runner/XmlProcessingResultTest.php b/tests/Unit/Runner/XmlProcessingResultTest.php deleted file mode 100644 index e06f8e0..0000000 --- a/tests/Unit/Runner/XmlProcessingResultTest.php +++ /dev/null @@ -1,73 +0,0 @@ -'), - new File('input.xml', ''), - ); - - self::assertFalse($result->isModified()); - } - - #[Test] - public function itHasPendingFixesForModifiedContent(): void - { - $result = new XmlProcessingResult( - new FileReport('input.xml'), - new File('input.xml', ''), - new File('input.xml', ''), - ); - - self::assertTrue($result->isModified()); - } - - #[Test] - public function itThrowsWhenReadingFixedContentWithoutFixApplication(): void - { - $result = new XmlProcessingResult( - new FileReport('input.xml'), - new File('input.xml', ''), - new File('input.xml', ''), - ); - - $this->expectException(FixerException::class); - $this->expectExceptionMessageIsOrContains('Cannot read fixed content when no fix application was attempted.'); - - $result->fixedContent(); - } - - #[Test] - public function itReturnsFixedContentWhenFixApplicationExists(): void - { - $result = new XmlProcessingResult( - new FileReport('input.xml'), - new File('input.xml', ''), - new File('input.xml', ''), - ); - - self::assertSame('', $result->fixedContent()); - } -} diff --git a/tests/Unit/Runner/XmlFileProcessorTest.php b/tests/Unit/Runner/XmlSniffRunnerTest.php similarity index 75% rename from tests/Unit/Runner/XmlFileProcessorTest.php rename to tests/Unit/Runner/XmlSniffRunnerTest.php index 28f4b55..247cce5 100644 --- a/tests/Unit/Runner/XmlFileProcessorTest.php +++ b/tests/Unit/Runner/XmlSniffRunnerTest.php @@ -8,13 +8,13 @@ use DocbookCS\Fix\Fixer\AttributeOrderFixer; use DocbookCS\Fix\FixerException; use DocbookCS\Report\FileReport; -use DocbookCS\Report\Report; use DocbookCS\Runner\EntityPreprocessor; +use DocbookCS\Runner\XmlFileProcessor; +use DocbookCS\Runner\XmlFixRunner; use DocbookCS\Runner\RunMode; use DocbookCS\Runner\RunScope; use DocbookCS\Runner\ViolationScopeFilter; -use DocbookCS\Runner\XmlFileProcessor; -use DocbookCS\Runner\XmlProcessingResult; +use DocbookCS\Runner\XmlSniffRunner; use DocbookCS\Sniff\Fixable; use DocbookCS\Sniff\SniffInterface; use DocbookCS\Source\File; @@ -30,10 +30,9 @@ #[ CoversClass(EntityPreprocessor::class), CoversClass(FileReport::class), - CoversClass(Report::class), CoversClass(Violation::class), CoversClass(ViolationScopeFilter::class), - CoversClass(XmlFileProcessor::class), + CoversClass(XmlSniffRunner::class), // UsesClass(AttributeOrderFixer::class), UsesClass(File::class), @@ -43,14 +42,15 @@ UsesClass(RunMode::class), UsesClass(RunScope::class), UsesClass(SourceRange::class), - UsesClass(XmlProcessingResult::class), + UsesClass(XmlFileProcessor::class), + UsesClass(XmlFixRunner::class), ] -final class XmlFileProcessorTest extends TestCase +final class XmlSniffRunnerTest extends TestCase { #[Test] public function itReportsParseErrors(): void { - $report = $this->process($this->processor(), '', 'bad.xml'); + $report = $this->process($this->runner(), '', 'bad.xml'); $this->assertInternalError($report, 'XML parse error'); } @@ -59,7 +59,7 @@ public function itReportsParseErrors(): void public function itReportsParseErrorsOutsideChangedSourceRanges(): void { $report = $this->process( - $this->processor(), + $this->runner(), '', 'bad.xml', new FileChange('bad.xml', [99]), @@ -72,7 +72,7 @@ public function itReportsParseErrorsOutsideChangedSourceRanges(): void public function itStoresTheProvidedFilePathInFileReports(): void { $filePath = (getcwd() ?: '') . '/nonexistent/path/file.xml'; - $report = $this->process($this->processor(), '', $filePath); + $report = $this->process($this->runner(), '', $filePath); self::assertSame($filePath, $report->filePath); } @@ -82,9 +82,9 @@ public function itAcceptsValidXmlWithoutViolations(): void { $xml = $this->xml('ok'); - $report = $this->process($this->processor(), $xml); + $report = $this->process($this->runner(), $xml); - self::assertFalse($report->hasViolations()); + self::assertFalse($report->hasFinalViolations()); } #[Test] @@ -92,9 +92,9 @@ public function itReturnsZeroViolationsWithoutSniffs(): void { $xml = $this->xml('Hello'); - $report = $this->process($this->processor(), $xml); + $report = $this->process($this->runner(), $xml); - self::assertSame(0, $report->getViolationCount()); + self::assertSame(0, $report->getFinalViolationCount()); } #[Test] @@ -110,9 +110,9 @@ public function itReturnsAllViolationsWithoutDiffFiltering(): void ' ); - $report = $this->process($this->processor([$sniff]), $xml); + $report = $this->process($this->runner([$sniff]), $xml); - self::assertSame(2, $report->getViolationCount()); + self::assertSame(2, $report->getFinalViolationCount()); } #[Test] @@ -129,14 +129,14 @@ public function itFiltersViolationsByChangedLines(): void ); $report = $this->process( - $this->processor([$sniff]), + $this->runner([$sniff], mode: RunMode::Fix), $xml, 'f.xml', new FileChange('f.xml', [3]), ); - self::assertSame(1, $report->getViolationCount()); - self::assertSame(3, $report->violations[0]->rangeOne()->line); + self::assertSame(1, $report->getFinalViolationCount()); + self::assertSame(3, $report->finalViolations[0]->rangeOne()->line); } #[Test] @@ -155,13 +155,13 @@ public function itExpandsElementSpanForNestedChanges(): void ); $report = $this->process( - $this->processor([$sniff]), + $this->runner([$sniff]), $xml, 'x.xml', new FileChange('x.xml', [6]), ); - self::assertSame(1, $report->getViolationCount()); + self::assertSame(1, $report->getFinalViolationCount()); } #[Test] @@ -176,13 +176,13 @@ public function itDropsViolationsWhoseLineHasNoElement(): void ); $report = $this->process( - $this->processor([$sniff]), + $this->runner([$sniff]), $xml, 'x.xml', new FileChange('x.xml', [3]), ); - self::assertSame(0, $report->getViolationCount()); + self::assertSame(0, $report->getFinalViolationCount()); } #[Test] @@ -199,13 +199,13 @@ public function itMatchesChangesInElementOwnTextContent(): void ); $report = $this->process( - $this->processor([$sniff]), + $this->runner([$sniff]), $xml, 'x.xml', new FileChange('x.xml', [4]), ); - self::assertSame(1, $report->getViolationCount()); + self::assertSame(1, $report->getFinalViolationCount()); } #[Test] @@ -223,13 +223,13 @@ public function itBoundsChildSpanByNextSibling(): void ); $report = $this->process( - $this->processor([$sniff]), + $this->runner([$sniff]), $xml, 'x.xml', new FileChange('x.xml', [4]), ); - self::assertSame(1, $report->getViolationCount()); + self::assertSame(1, $report->getFinalViolationCount()); } #[Test] @@ -250,13 +250,13 @@ public function itIgnoresChangesInNonDirectDescendants(): void ); $report = $this->process( - $this->processor([$sniff]), + $this->runner([$sniff]), $xml, 'x.xml', new FileChange('x.xml', [6]), ); - self::assertSame(0, $report->getViolationCount()); + self::assertSame(0, $report->getFinalViolationCount()); } #[Test] @@ -272,13 +272,13 @@ public function itIgnoresChangesOutsideElementSpan(): void ); $report = $this->process( - $this->processor([$sniff]), + $this->runner([$sniff]), $xml, 'x.xml', new FileChange('x.xml', [7]), ); - self::assertSame(0, $report->getViolationCount()); + self::assertSame(0, $report->getFinalViolationCount()); } #[Test] @@ -295,23 +295,19 @@ public function itReportsNoViolationsInDiffModeWhenNoLinesWereAdded(): void ); $report = $this->process( - $this->processor([$sniff]), + $this->runner([$sniff]), $xml, 'f.xml', new FileChange('f.xml', []), ); - self::assertSame(0, $report->getViolationCount()); + self::assertSame(0, $report->getFinalViolationCount()); } #[Test] public function itDoesNotFixViolationsFromNonFixableSniffs(): void { - $sniff = new class (RunMode::Sniff) implements SniffInterface { - public function __construct(public RunMode $mode) - { - } - + $sniff = new class implements SniffInterface { public static function getCode(): string { return 'Test.NonFixable'; @@ -335,19 +331,18 @@ public function setProperty(string $name, string $value): void } }; - $report = $this->process($this->processor([$sniff]), $this->xml('')); + $report = $this->process( + $this->runner([$sniff]), + $this->xml(''), + ); - self::assertSame(1, $report->getViolationCount()); + self::assertSame(1, $report->getFinalViolationCount()); } #[Test] public function itThrowsWhenFixableSniffReportsViolationWithoutContentInFixMode(): void { - $sniff = new class (RunMode::Fix) implements Fixable { - public function __construct(public RunMode $mode) - { - } - + $sniff = new class implements Fixable { public static function getCode(): string { return 'Test.BrokenFixable'; @@ -379,20 +374,23 @@ public function setProperty(string $name, string $value): void $this->expectException(FixerException::class); $this->expectExceptionMessageIsOrContains('Fixers require affected source ranges with source content.'); - $this->process($this->processor([$sniff]), $this->xml('')); + $content = $this->xml(''); + $file = new File('input.xml', $content); + $fileReport = new FileReport($file->path); + new XmlFileProcessor($this->runner([$sniff], mode: RunMode::Fix))->process( + $file, + $fileReport, + RunScope::fromFileAndFileChange($file, null), + ); } /** @param list $lines */ private function sniff(array $lines): SniffInterface { - $sniff = new class (RunMode::Sniff) implements SniffInterface { + $sniff = new class implements SniffInterface { /** @var list */ public array $lines = []; - public function __construct(public RunMode $mode) - { - } - public static function getCode(): string { return 'Test.Stub'; @@ -423,21 +421,29 @@ public function setProperty(string $name, string $value): void } private function process( - XmlFileProcessor $processor, + XmlSniffRunner $runner, string $content, string $path = 'input.xml', ?FileChange $fileChange = null, ): FileReport { - return $processor->process(new File($path, $content), $fileChange)->fileReport; + $file = new File($path, $content); + $fileReport = new FileReport($path); + + new XmlFileProcessor($runner)->process( + $file, + $fileReport, + RunScope::fromFileAndFileChange($file, $fileChange), + ); + + return $fileReport; } /** @param list $sniffs */ - private function processor(array $sniffs = [], ?EntityPreprocessor $pre = null): XmlFileProcessor - { - return new XmlFileProcessor( - $sniffs, - $pre ?? new EntityPreprocessor([]) // always pass array - ); + private function runner( + array $sniffs = [], + RunMode $mode = RunMode::Sniff, + ): XmlSniffRunner { + return new XmlSniffRunner($mode, $sniffs); } private function xml(string $body): string @@ -450,8 +456,8 @@ private function xml(string $body): string private function assertInternalError(FileReport $report, string $messagePart): void { - self::assertTrue($report->hasViolations()); - self::assertSame('DocbookCS.Internal', $report->getViolations()[0]->sniffCode); - self::assertStringContainsString($messagePart, $report->getViolations()[0]->message); + self::assertTrue($report->hasFinalViolations()); + self::assertSame('DocbookCS.Internal', $report->finalViolations[0]->sniffCode); + self::assertStringContainsString($messagePart, $report->finalViolations[0]->message); } }