Skip to content

Commit cee75f1

Browse files
NickSdotNickSdot
authored
Introduce auto-fixer capabilities (#35)
* feat: add automatic fixers * fix: made violations not reported in non elements * refactor: a scope has ranges, but belongs to a run rather than a source (WIP) * refactor: simplyfied RunScope * refactor: improved Violation and SourceRange * refactor: moved reporting messages to constants * refactor: centralised source range construction * refactor: cleaned up FixApplier * refactor: added `Fix::fromViolationAndRange()` static constructor
1 parent a471125 commit cee75f1

85 files changed

Lines changed: 4858 additions & 769 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎README.md‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -40,15 +40,16 @@ Implement `DocbookCS\Sniff\SniffInterface` (or extend `AbstractSniff`):
4040
namespace Acme\DocbookSniffs;
4141

4242
use DocbookCS\Sniff\AbstractSniff;
43+
use DocbookCS\Source\File;
4344

4445
final class MySniff extends AbstractSniff
4546
{
46-
public function getCode(): string
47+
public static function getCode(): string
4748
{
4849
return 'Acme.MySniff';
4950
}
5051

51-
public function process(\DOMDocument $document, string $content, string $filePath): array
52+
public function process(\DOMDocument $document, File $file): array
5253
{
5354
$violations = [];
5455
// ... inspect $document, add violations via $this->createViolation(...)
@@ -70,9 +71,10 @@ through the working tree. Alternatively, a unified diff can be piped or file and
7071
directory paths passed. The inspection scope is limited to the given diff or the
7172
full contents of the given file paths.
7273

73-
XML references are expanded by default, but reported violations remain limited
74-
to the given scope. With `--wide`, every file inferred from paths or a diff is
75-
checked as a whole, and referenced `SYSTEM` XML files are recursively included.
74+
XML references are expanded by default, but violations and fixes remain limited
75+
to the given scope. With `--wide`, every file, inferred from paths or diff, as
76+
a whole will be checked and referenced `SYSTEM` XML files recursively included
77+
in violation reports and fixing.
7678

7779
| Input | `--wide` | Full File(s) | References |
7880
|------------|---------:|-------------:|-----------:|

‎bin/docbook-cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ use DocbookCS\Application;
1414

1515
foreach ($candidates as $file) {
1616
if (!is_file($file)) {
17-
continue;
17+
continue;
1818
}
1919

2020
require $file;

‎phpcs.xml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@
22
<ruleset name="PHP_CodeSniffer" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="https://raw.githubusercontent.com/squizlabs/PHP_CodeSniffer/refs/heads/master/phpcs.xsd">
33
<description>DocbookCS Coding Standard</description>
44
<rule ref="PSR12"/>
5+
<rule ref="PSR2.Methods.FunctionCallSignature.SpaceBeforeCloseBracket">
6+
<exclude-pattern type="relative">tests/*</exclude-pattern>
7+
</rule>
58

69
<arg name="cache" value="var/.phpcs-cache.json"/>
710
<arg name="basepath" value="./"/>

‎src/Application.php‎

Lines changed: 27 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,9 @@
1414
use DocbookCS\Report\Reporter\ConsoleReporter;
1515
use DocbookCS\Report\Reporter\JsonReporter;
1616
use DocbookCS\Report\Reporter\ReporterInterface;
17+
use DocbookCS\Runner\RunCoordinator;
18+
use DocbookCS\Runner\RunMode;
1719
use DocbookCS\Runner\RunPlanner;
18-
use DocbookCS\Runner\SniffRunner;
1920

2021
final class Application
2122
{
@@ -104,7 +105,11 @@ public function run(): int
104105
}
105106

106107
try {
107-
$runPlan = new RunPlanner($config, $options['wide'])->plan($options['paths'], $this->unifiedDiff);
108+
$runPlan = new RunPlanner(
109+
config: $config,
110+
mode: RunMode::fromFixFlag($options['fix']),
111+
wide: $options['wide'],
112+
)->plan($options['paths'], $this->unifiedDiff);
108113
} catch (\Throwable $e) {
109114
$this->writeError('Error resolving input: ' . $e->getMessage() . PHP_EOL);
110115

@@ -114,8 +119,7 @@ public function run(): int
114119
$progress = $this->createProgress($options);
115120

116121
try {
117-
$runner = new SniffRunner($progress);
118-
$report = $runner->run($runPlan);
122+
$report = new RunCoordinator($progress)->run($runPlan);
119123
} catch (\Throwable $e) {
120124
$this->writeError('Runtime error: ' . $e->getMessage() . PHP_EOL);
121125

@@ -140,12 +144,13 @@ public function run(): int
140144
* config: string,
141145
* report: string,
142146
* colors: bool,
147+
* fix: bool,
148+
* wide: bool,
143149
* quiet: bool,
144150
* paths: list<string>,
145-
* wide: bool,
146151
* perf: bool,
147152
* }
148-
* @throws \InvalidArgumentException for unsupported options.
153+
* @throws \InvalidArgumentException for unsupported or removed options.
149154
*/
150155
private function parseArgv(): array
151156
{
@@ -155,9 +160,10 @@ private function parseArgv(): array
155160
'config' => self::DEFAULT_CONFIG,
156161
'report' => 'console',
157162
'colors' => $this->detectColorSupport(),
163+
'fix' => false,
164+
'wide' => false,
158165
'quiet' => false,
159166
'paths' => [],
160-
'wide' => false,
161167
'perf' => false,
162168
];
163169

@@ -228,6 +234,12 @@ private function parseArgv(): array
228234
continue;
229235
}
230236

237+
if ($arg === '--fix') {
238+
$result['fix'] = true;
239+
$i++;
240+
continue;
241+
}
242+
231243
if ($arg === '--wide') {
232244
$result['wide'] = true;
233245
$i++;
@@ -351,6 +363,8 @@ private function printHelp(): void
351363
--report=<format> Output format: console (default), checkstyle, json.
352364
--colors Force ANSI color output.
353365
--no-colors Disable ANSI color output.
366+
--fix Automatically fix violations when fixers exist
367+
(experimental).
354368
--wide Check whole selected files and recursively include
355369
referenced XML files.
356370
@@ -362,9 +376,14 @@ private function printHelp(): void
362376
docbook-cs
363377
docbook-cs --config=myconfig.xml reference/
364378
docbook-cs --report=checkstyle --no-colors > report.xml
379+
docbook-cs . --fix
380+
docbook-cs reference/
365381
docbook-cs reference/strings/functions/strlen.xml
382+
docbook-cs reference/strings/functions/strlen.xml --wide
383+
docbook-cs reference/strings/functions/strlen.xml --wide --fix
366384
git diff HEAD | docbook-cs
367-
git diff HEAD | docbook-cs --wide --report=checkstyle
385+
git diff HEAD | docbook-cs --wide
386+
git diff HEAD | docbook-cs --wide --fix --report=checkstyle
368387

369388
HELP;
370389

‎src/Diff/DiffParser.php‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,11 @@ public function parse(string $diff): Diff
101101
$fileChanges = [];
102102

103103
foreach ($changedLinesByFile as $filePath => $lineNumbers) {
104-
$fileChanges[] = new FileChange($filePath, $lineNumbers, $deletionAnchorsByFile[$filePath]);
104+
$fileChanges[] = new FileChange(
105+
$filePath,
106+
$lineNumbers,
107+
$deletionAnchorsByFile[$filePath],
108+
);
105109
}
106110

107111
return new Diff($fileChanges);

‎src/Fix/Fix.php‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace DocbookCS\Fix;
6+
7+
use DocbookCS\Violation\SourceRange;
8+
use DocbookCS\Violation\Violation;
9+
10+
final readonly class Fix
11+
{
12+
public static function fromViolationAndRange(Violation $violation, SourceRange $range, string $replacement): self
13+
{
14+
return new self(
15+
filePath: $violation->filePath,
16+
beginOffset: $range->beginOffset,
17+
untilOffset: $range->untilOffset,
18+
replacement: $replacement,
19+
sniffCode: $violation->sniffCode,
20+
expectedContent: $range->content,
21+
);
22+
}
23+
24+
public function __construct(
25+
public string $filePath,
26+
public int $beginOffset,
27+
public int $untilOffset,
28+
public string $replacement,
29+
public string $sniffCode,
30+
public ?string $expectedContent = null,
31+
) {
32+
}
33+
}

‎src/Fix/FixApplier.php‎

Lines changed: 200 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,200 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace DocbookCS\Fix;
6+
7+
use DocbookCS\Source\File;
8+
9+
final class FixApplier
10+
{
11+
/**
12+
* @param list<Fix|FixPlan> $fixes
13+
*/
14+
public function apply(File $file, array $fixes): FixResult
15+
{
16+
if ($fixes === []) {
17+
return new FixResult($file);
18+
}
19+
20+
$plans = array_map(
21+
static fn ($fix): FixPlan => $fix instanceof FixPlan ? $fix : new FixPlan($fix),
22+
$fixes,
23+
);
24+
25+
usort($plans, static fn (FixPlan $a, FixPlan $b): int => $a->firstOffset() <=> $b->firstOffset());
26+
27+
/** @var list<Fix> $acceptedFixes */
28+
$acceptedFixes = [];
29+
$acceptedPlans = 0;
30+
$skipped = 0;
31+
32+
$content = $file->content;
33+
$length = strlen($content);
34+
35+
foreach ($plans as $plan) {
36+
if (!$this->canApply($file, $length, $plan, $acceptedFixes)) {
37+
$skipped++;
38+
continue;
39+
}
40+
41+
if (!$this->changesContent($content, $plan)) {
42+
$skipped++;
43+
continue;
44+
}
45+
46+
foreach ($plan->fixes as $fix) {
47+
$this->insertFix($acceptedFixes, $fix);
48+
}
49+
50+
$acceptedPlans++;
51+
}
52+
53+
$fixedContent = '';
54+
$sourceOffset = 0;
55+
56+
foreach ($acceptedFixes as $fix) {
57+
$fixedContent .= substr($content, $sourceOffset, $fix->beginOffset - $sourceOffset);
58+
$fixedContent .= $fix->replacement;
59+
$sourceOffset = $fix->untilOffset;
60+
}
61+
62+
$fixedContent .= substr($content, $sourceOffset);
63+
64+
return new FixResult(
65+
file: $file->withContent($fixedContent),
66+
applied: $acceptedPlans,
67+
skipped: $skipped,
68+
appliedFixes: $acceptedFixes,
69+
);
70+
}
71+
72+
/**
73+
* @param list<Fix> $acceptedFixes
74+
*/
75+
private function canApply(File $file, int $contentLength, FixPlan $plan, array $acceptedFixes): bool
76+
{
77+
$content = $file->content;
78+
$first = $plan->fixes[0];
79+
$previous = null;
80+
81+
foreach ($plan->fixes as $fix) {
82+
if (!$this->canAcceptFix($file, $contentLength, $first, $previous, $fix, $acceptedFixes)) {
83+
return false;
84+
}
85+
86+
$currentContent = substr($content, $fix->beginOffset, $fix->untilOffset - $fix->beginOffset);
87+
88+
if ($fix->expectedContent !== null && $currentContent !== $fix->expectedContent) {
89+
return false;
90+
}
91+
92+
$previous = $fix;
93+
}
94+
95+
return true;
96+
}
97+
98+
private function changesContent(string $content, FixPlan $plan): bool
99+
{
100+
foreach ($plan->fixes as $fix) {
101+
$currentContent = substr($content, $fix->beginOffset, $fix->untilOffset - $fix->beginOffset);
102+
103+
if ($currentContent !== $fix->replacement) {
104+
return true;
105+
}
106+
}
107+
108+
return false;
109+
}
110+
111+
/**
112+
* @param list<Fix> $acceptedFixes
113+
*/
114+
private function conflictsWithAcceptedFix(Fix $fix, array $acceptedFixes): bool
115+
{
116+
$index = $this->insertionIndex($acceptedFixes, $fix);
117+
118+
return ($index > 0 && self::overlaps($acceptedFixes[$index - 1], $fix))
119+
|| ($index < count($acceptedFixes) && self::overlaps($acceptedFixes[$index], $fix));
120+
}
121+
122+
/** @param list<Fix> $fixes */
123+
private function insertFix(array &$fixes, Fix $fix): void
124+
{
125+
$lastIndex = count($fixes) - 1;
126+
if ($lastIndex < 0 || self::compare($fixes[$lastIndex], $fix) < 0) {
127+
$fixes[] = $fix;
128+
return;
129+
}
130+
131+
array_splice($fixes, $this->insertionIndex($fixes, $fix), 0, [$fix]);
132+
}
133+
134+
/** @param list<Fix> $fixes */
135+
private function insertionIndex(array $fixes, Fix $fix): int
136+
{
137+
$low = 0;
138+
$high = count($fixes);
139+
140+
while ($low < $high) {
141+
$middle = intdiv($low + $high, 2);
142+
if (self::compare($fixes[$middle], $fix) < 0) {
143+
$low = $middle + 1;
144+
} else {
145+
$high = $middle;
146+
}
147+
}
148+
149+
return $low;
150+
}
151+
152+
private static function compare(Fix $a, Fix $b): int
153+
{
154+
return [
155+
$a->beginOffset,
156+
$a->untilOffset,
157+
] <=> [
158+
$b->beginOffset,
159+
$b->untilOffset,
160+
];
161+
}
162+
163+
private static function overlaps(Fix $a, Fix $b): bool
164+
{
165+
$aIsInsertion = $a->beginOffset === $a->untilOffset;
166+
$bIsInsertion = $b->beginOffset === $b->untilOffset;
167+
168+
if ($aIsInsertion && $bIsInsertion) {
169+
return $a->beginOffset === $b->beginOffset;
170+
}
171+
172+
if ($aIsInsertion) {
173+
return $a->beginOffset > $b->beginOffset && $a->beginOffset < $b->untilOffset;
174+
}
175+
176+
if ($bIsInsertion) {
177+
return $b->beginOffset > $a->beginOffset && $b->beginOffset < $a->untilOffset;
178+
}
179+
180+
return $a->beginOffset < $b->untilOffset && $b->beginOffset < $a->untilOffset;
181+
}
182+
183+
/** @param list<Fix> $acceptedFixes */
184+
private function canAcceptFix(
185+
File $file,
186+
int $contentLength,
187+
Fix $first,
188+
?Fix $previous,
189+
Fix $fix,
190+
array $acceptedFixes
191+
): bool {
192+
return $fix->filePath === $file->path
193+
&& $fix->sniffCode === $first->sniffCode
194+
&& $fix->beginOffset >= 0
195+
&& $fix->untilOffset >= $fix->beginOffset
196+
&& $fix->untilOffset <= $contentLength
197+
&& ($previous === null || !self::overlaps($previous, $fix))
198+
&& !$this->conflictsWithAcceptedFix($fix, $acceptedFixes);
199+
}
200+
}

0 commit comments

Comments
 (0)