Skip to content

Commit 52c90e5

Browse files
ondrejmirtesondrejmirtes
committed
Refactor --fix logic from AnalyseCommand to Patcher class
1 parent bd75a4f commit 52c90e5

File tree

5 files changed

+148
-98
lines changed

5 files changed

+148
-98
lines changed

‎phpstan-baseline.neon

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -114,24 +114,6 @@ parameters:
114114
count: 1
115115
path: src/Collectors/Registry.php
116116

117-
-
118-
message: '#^Call to method getContent\(\) of internal class PhpMerge\\internal\\Line from outside its root namespace PhpMerge\.$#'
119-
identifier: method.internalClass
120-
count: 2
121-
path: src/Command/AnalyseCommand.php
122-
123-
-
124-
message: '#^Call to static method createArray\(\) of internal class PhpMerge\\internal\\Hunk from outside its root namespace PhpMerge\.$#'
125-
identifier: staticMethod.internalClass
126-
count: 2
127-
path: src/Command/AnalyseCommand.php
128-
129-
-
130-
message: '#^Call to static method createArray\(\) of internal class PhpMerge\\internal\\Line from outside its root namespace PhpMerge\.$#'
131-
identifier: staticMethod.internalClass
132-
count: 5
133-
path: src/Command/AnalyseCommand.php
134-
135117
-
136118
message: '#^Anonymous function has an unused use \$container\.$#'
137119
identifier: closure.unusedUse
@@ -234,6 +216,24 @@ parameters:
234216
count: 1
235217
path: src/Diagnose/PHPStanDiagnoseExtension.php
236218

219+
-
220+
message: '#^Call to method getContent\(\) of internal class PhpMerge\\internal\\Line from outside its root namespace PhpMerge\.$#'
221+
identifier: method.internalClass
222+
count: 2
223+
path: src/Fixable/Patcher.php
224+
225+
-
226+
message: '#^Call to static method createArray\(\) of internal class PhpMerge\\internal\\Hunk from outside its root namespace PhpMerge\.$#'
227+
identifier: staticMethod.internalClass
228+
count: 2
229+
path: src/Fixable/Patcher.php
230+
231+
-
232+
message: '#^Call to static method createArray\(\) of internal class PhpMerge\\internal\\Line from outside its root namespace PhpMerge\.$#'
233+
identifier: staticMethod.internalClass
234+
count: 5
235+
path: src/Fixable/Patcher.php
236+
237237
-
238238
message: '#^Call to method getTokenCode\(\) of internal class PhpParser\\Internal\\TokenStream from outside its root namespace PhpParser\.$#'
239239
identifier: method.internalClass

‎src/Command/AnalyseCommand.php

Lines changed: 15 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,7 @@
22

33
namespace PHPStan\Command;
44

5-
use Nette\Utils\Strings;
65
use OndraM\CiDetector\CiDetector;
7-
use PhpMerge\internal\Hunk;
8-
use PhpMerge\internal\Line;
9-
use PhpMerge\MergeConflict;
10-
use PhpMerge\PhpMerge;
116
use PHPStan\Analyser\InternalError;
127
use PHPStan\Command\ErrorFormatter\BaselineNeonErrorFormatter;
138
use PHPStan\Command\ErrorFormatter\BaselinePhpErrorFormatter;
@@ -24,12 +19,13 @@
2419
use PHPStan\File\ParentDirectoryRelativePathHelper;
2520
use PHPStan\File\PathNotFoundException;
2621
use PHPStan\File\RelativePathHelper;
22+
use PHPStan\Fixable\FileChangedException;
23+
use PHPStan\Fixable\MergeConflictException;
24+
use PHPStan\Fixable\Patcher;
2725
use PHPStan\Internal\BytesHelper;
2826
use PHPStan\Internal\DirectoryCreator;
2927
use PHPStan\Internal\DirectoryCreatorException;
3028
use PHPStan\ShouldNotHappenException;
31-
use ReflectionClass;
32-
use SebastianBergmann\Diff\Differ;
3329
use Symfony\Component\Console\Command\Command;
3430
use Symfony\Component\Console\Input\InputArgument;
3531
use Symfony\Component\Console\Input\InputInterface;
@@ -58,16 +54,13 @@
5854
use function is_string;
5955
use function pathinfo;
6056
use function rewind;
61-
use function sha1;
6257
use function sprintf;
6358
use function str_contains;
6459
use function stream_get_contents;
6560
use function strlen;
6661
use function substr;
6762
use const PATHINFO_BASENAME;
6863
use const PATHINFO_EXTENSION;
69-
use const PREG_SPLIT_DELIM_CAPTURE;
70-
use const PREG_SPLIT_NO_EMPTY;
7164

7265
/**
7366
* @phpstan-import-type Trace from InternalError as InternalErrorTrace
@@ -524,79 +517,29 @@ protected function execute(InputInterface $input, OutputInterface $output): int
524517
$exitCode = 1;
525518
} else {
526519
$skippedCount = 0;
527-
$fixableErrorsByFile = [];
520+
$diffsByFile = [];
528521
foreach ($fixableErrors as $fixableError) {
529522
$fixFile = $fixableError->getFilePath();
530523
if ($fixableError->getTraitFilePath() !== null) {
531524
$fixFile = $fixableError->getTraitFilePath();
532525
}
533526

534-
$fixableErrorsByFile[$fixFile][] = $fixableError;
535-
}
536-
537-
$differ = $container->getByType(Differ::class);
538-
539-
foreach ($fixableErrorsByFile as $file => $fileFixableErrors) {
540-
$fileContents = FileReader::read($file);
541-
$fileHash = sha1($fileContents);
542-
$diffHunks = [];
543-
foreach ($fileFixableErrors as $fileFixableError) {
544-
$diff = $fileFixableError->getFixedErrorDiff();
545-
if ($diff === null) {
546-
throw new ShouldNotHappenException();
547-
}
548-
if ($diff->originalHash !== $fileHash) {
549-
$skippedCount++;
550-
continue;
551-
}
552-
553-
$diffHunks[] = Hunk::createArray(Line::createArray($diff->diff));
554-
}
555-
556-
if (count($diffHunks) === 0) {
557-
continue;
527+
if ($fixableError->getFixedErrorDiff() === null) {
528+
throw new ShouldNotHappenException();
558529
}
559530

560-
$baseLines = Line::createArray(array_map(
561-
static fn ($l) => [$l, Differ::OLD],
562-
self::splitStringByLines($fileContents),
563-
));
564-
565-
$refMerge = new ReflectionClass(PhpMerge::class);
566-
$refMergeMethod = $refMerge->getMethod('mergeHunks');
567-
$refMergeMethod->setAccessible(true);
568-
569-
$result = Line::createArray(array_map(
570-
static fn ($l) => [$l, Differ::OLD],
571-
$refMergeMethod->invokeArgs(null, [
572-
$baseLines,
573-
$diffHunks[0],
574-
[],
575-
]),
576-
));
577-
578-
for ($i = 0; $i < count($diffHunks); $i++) {
579-
/** @var MergeConflict[] $conflicts */
580-
$conflicts = [];
581-
$merged = $refMergeMethod->invokeArgs(null, [
582-
$baseLines,
583-
Hunk::createArray(Line::createArray($differ->diffToArray($fileContents, implode('', array_map(static fn ($l) => $l->getContent(), $result))))),
584-
$diffHunks[$i],
585-
&$conflicts,
586-
]);
587-
if (count($conflicts) > 0) {
588-
$skippedCount += count($diffHunks);
589-
continue 2;
590-
}
591-
592-
$result = Line::createArray(array_map(
593-
static fn ($l) => [$l, Differ::OLD],
594-
$merged,
595-
));
531+
$diffsByFile[$fixFile][] = $fixableError->getFixedErrorDiff();
532+
}
596533

534+
$patcher = $container->getByType(Patcher::class);
535+
foreach ($diffsByFile as $file => $diffs) {
536+
try {
537+
$finalFileContents = $patcher->applyDiffs($file, $diffs);
538+
} catch (FileChangedException | MergeConflictException) {
539+
$skippedCount += count($diffs);
540+
continue;
597541
}
598542

599-
$finalFileContents = implode('', array_map(static fn ($l) => $l->getContent(), $result));
600543
FileWriter::write($file, $finalFileContents);
601544
}
602545

@@ -679,14 +622,6 @@ protected function execute(InputInterface $input, OutputInterface $output): int
679622
);
680623
}
681624

682-
/**
683-
* @return string[]
684-
*/
685-
private static function splitStringByLines(string $input): array
686-
{
687-
return Strings::split($input, '/(.*\R)/', PREG_SPLIT_DELIM_CAPTURE | PREG_SPLIT_NO_EMPTY);
688-
}
689-
690625
private function createStreamOutput(): StreamOutput
691626
{
692627
$resource = fopen('php://memory', 'w', false);

‎src/Fixable/FileChangedException.php

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Fixable;
4+
5+
use Exception;
6+
7+
class FileChangedException extends Exception
8+
{
9+
10+
}

‎src/Fixable/MergeConflictException.php

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Fixable;
4+
5+
use Exception;
6+
7+
class MergeConflictException extends Exception
8+
{
9+
10+
}

‎src/Fixable/Patcher.php

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Fixable;
4+
5+
use Nette\Utils\Strings;
6+
use PhpMerge\internal\Hunk;
7+
use PhpMerge\internal\Line;
8+
use PhpMerge\MergeConflict;
9+
use PhpMerge\PhpMerge;
10+
use PHPStan\Analyser\FixedErrorDiff;
11+
use PHPStan\DependencyInjection\AutowiredService;
12+
use PHPStan\File\FileReader;
13+
use ReflectionClass;
14+
use SebastianBergmann\Diff\Differ;
15+
16+
#[AutowiredService]
17+
final class Patcher
18+
{
19+
20+
public function __construct(private Differ $differ)
21+
{
22+
}
23+
24+
/**
25+
* @param FixedErrorDiff[] $diffs
26+
* @throws FileChangedException
27+
* @throws MergeConflictException
28+
*/
29+
public function applyDiffs(string $fileName, array $diffs): string
30+
{
31+
$fileContents = FileReader::read($fileName);
32+
$fileHash = sha1($fileContents);
33+
$diffHunks = [];
34+
foreach ($diffs as $diff) {
35+
if ($diff->originalHash !== $fileHash) {
36+
throw new FileChangedException();
37+
}
38+
39+
$diffHunks[] = Hunk::createArray(Line::createArray($diff->diff));
40+
}
41+
42+
if (count($diffHunks) === 0) {
43+
return $fileContents;
44+
}
45+
46+
$baseLines = Line::createArray(array_map(
47+
static fn ($l) => [$l, Differ::OLD],
48+
self::splitStringByLines($fileContents),
49+
));
50+
51+
$refMerge = new ReflectionClass(PhpMerge::class);
52+
$refMergeMethod = $refMerge->getMethod('mergeHunks');
53+
$refMergeMethod->setAccessible(true);
54+
55+
$result = Line::createArray(array_map(
56+
static fn ($l) => [$l, Differ::OLD],
57+
$refMergeMethod->invokeArgs(null, [
58+
$baseLines,
59+
$diffHunks[0],
60+
[],
61+
]),
62+
));
63+
64+
for ($i = 0; $i < count($diffHunks); $i++) {
65+
/** @var MergeConflict[] $conflicts */
66+
$conflicts = [];
67+
$merged = $refMergeMethod->invokeArgs(null, [
68+
$baseLines,
69+
Hunk::createArray(Line::createArray($this->differ->diffToArray($fileContents, implode('', array_map(static fn ($l) => $l->getContent(), $result))))),
70+
$diffHunks[$i],
71+
&$conflicts,
72+
]);
73+
if (count($conflicts) > 0) {
74+
throw new MergeConflictException();
75+
}
76+
77+
$result = Line::createArray(array_map(
78+
static fn ($l) => [$l, Differ::OLD],
79+
$merged,
80+
));
81+
82+
}
83+
84+
return implode('', array_map(static fn ($l) => $l->getContent(), $result));
85+
}
86+
87+
/**
88+
* @return string[]
89+
*/
90+
private static function splitStringByLines(string $input): array
91+
{
92+
return Strings::split($input, '/(.*\R)/', PREG_SPLIT_DELIM_CAPTURE | PREG_SPLIT_NO_EMPTY);
93+
}
94+
95+
}

0 commit comments

Comments
 (0)