Skip to content

Commit 6029e8d

Browse files
TomasVotrubaTomasVotruba
authored
[Core] Drop file positions on reprinted nodes copied from another file (#8928)
1 parent 2191758 commit 6029e8d

8 files changed

Lines changed: 210 additions & 4 deletions

File tree

‎src/PhpParser/Node/NodeFactory.php‎

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -351,18 +351,36 @@ public function createReprintedNode(Node $node): Node
351351
{
352352
// reset original node, to allow the printer to re-use the node
353353
$node->setAttribute(AttributeKey::ORIGINAL_NODE, null);
354+
$this->removeFilePositions($node);
354355

355356
$this->simpleCallableNodeTraverser->traverseNodesWithCallable(
356357
$node,
357-
static function (Node $subNode): Node {
358+
function (Node $subNode): Node {
358359
$subNode->setAttribute(AttributeKey::ORIGINAL_NODE, null);
360+
$this->removeFilePositions($subNode);
359361
return $subNode;
360362
}
361363
);
362364

363365
return $node;
364366
}
365367

368+
/**
369+
* Positions may come from another file, e.g. a parent class constructor
370+
*/
371+
private function removeFilePositions(Node $node): void
372+
{
373+
$attributes = $node->getAttributes();
374+
unset(
375+
$attributes['startFilePos'],
376+
$attributes['endFilePos'],
377+
$attributes['startTokenPos'],
378+
$attributes['endTokenPos']
379+
);
380+
381+
$node->setAttributes($attributes);
382+
}
383+
366384
private function createArrayItem(mixed $item, string|int|null $key = null): ArrayItem
367385
{
368386
$arrayItem = null;

‎tests/Caching/Detector/ChangedFilesDetectorPortableCacheTest.php‎

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,19 @@
66

77
use FilesystemIterator;
88
use Nette\Utils\FileSystem;
9+
use Rector\Caching\Cache;
10+
use Rector\Caching\Config\FileHashComputer;
911
use Rector\Caching\Detector\ChangedFilesDetector;
12+
use Rector\Caching\ValueObject\Storage\FileCacheStorage;
1013
use Rector\Configuration\Option;
1114
use Rector\Configuration\Parameter\SimpleParameterProvider;
15+
use Rector\FileSystem\FilePathHelper;
1216
use Rector\Testing\PHPUnit\AbstractLazyTestCase;
17+
use Rector\Util\FileHasher;
1318
use RecursiveDirectoryIterator;
1419
use RecursiveIteratorIterator;
1520
use SplFileInfo;
21+
use Symfony\Component\Filesystem\Filesystem as SymfonyFilesystem;
1622
use Webmozart\Assert\Assert;
1723

1824
final class ChangedFilesDetectorPortableCacheTest extends AbstractLazyTestCase
@@ -51,8 +57,6 @@ protected function setUp(): void
5157
{
5258
parent::setUp();
5359

54-
$this->changedFilesDetector = $this->make(ChangedFilesDetector::class);
55-
5660
$workingDirectory = getcwd();
5761
Assert::string($workingDirectory);
5862
$this->originalWorkingDirectory = $workingDirectory;
@@ -65,6 +69,14 @@ protected function setUp(): void
6569
$this->firstCheckoutDirectory = $this->rootDirectory . '/first/project';
6670
$this->secondCheckoutDirectory = $this->rootDirectory . '/second/project';
6771

72+
// own cache directory, as the shared Cache service keeps whichever directory it was first built with
73+
$this->changedFilesDetector = new ChangedFilesDetector(
74+
$this->make(FileHashComputer::class),
75+
new Cache(new FileCacheStorage($this->getCacheDirectory(), $this->make(SymfonyFilesystem::class))),
76+
$this->make(FileHasher::class),
77+
$this->make(FilePathHelper::class)
78+
);
79+
6880
$this->createCheckout($this->firstCheckoutDirectory);
6981
$this->createCheckout($this->secondCheckoutDirectory);
7082

@@ -281,9 +293,14 @@ private function cacheProjectFilesIn(string $directory): void
281293
}
282294
}
283295

296+
private function getCacheDirectory(): string
297+
{
298+
return $this->rootDirectory . '/cache';
299+
}
300+
284301
private function countCacheEntries(): int
285302
{
286-
$cacheDirectory = SimpleParameterProvider::provideStringParameter(Option::CACHE_DIR);
303+
$cacheDirectory = $this->getCacheDirectory();
287304
if (! is_dir($cacheDirectory)) {
288305
return 0;
289306
}

‎tests/Issues/InheritedConstructorFromOtherFile/Fixture/inherited_constructor_from_other_file.php.inc‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
<?php
2+
3+
namespace Rector\Tests\Issues\InheritedConstructorFromOtherFile\Fixture;
4+
5+
use Rector\Tests\Issues\InheritedConstructorFromOtherFile\Source\SomeParentWithDeepConstructor;
6+
7+
final class ChildWithoutConstructor extends SomeParentWithDeepConstructor
8+
{
9+
}
10+
11+
?>
12+
-----
13+
<?php
14+
15+
namespace Rector\Tests\Issues\InheritedConstructorFromOtherFile\Fixture;
16+
17+
use Rector\Tests\Issues\InheritedConstructorFromOtherFile\Source\SomeParentWithDeepConstructor;
18+
19+
final class ChildWithoutConstructor extends SomeParentWithDeepConstructor
20+
{
21+
public function __construct(protected string $name, private readonly \Rector\Tests\Issues\InheritedConstructorFromOtherFile\Source\SomeService $someService)
22+
{
23+
parent::__construct($name);
24+
}
25+
}
26+
27+
?>

‎tests/Issues/InheritedConstructorFromOtherFile/InheritedConstructorFromOtherFileTest.php‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Rector\Tests\Issues\InheritedConstructorFromOtherFile;
6+
7+
use Iterator;
8+
use PHPUnit\Framework\Attributes\DataProvider;
9+
use Rector\Testing\PHPUnit\AbstractRectorTestCase;
10+
11+
final class InheritedConstructorFromOtherFileTest extends AbstractRectorTestCase
12+
{
13+
#[DataProvider('provideData')]
14+
public function test(string $filePath): void
15+
{
16+
$this->doTestFile($filePath);
17+
}
18+
19+
public static function provideData(): Iterator
20+
{
21+
return self::yieldFilesFromDirectory(__DIR__ . '/Fixture');
22+
}
23+
24+
public function provideConfigFilePath(): string
25+
{
26+
return __DIR__ . '/config/configured_rule.php';
27+
}
28+
}

‎tests/Issues/InheritedConstructorFromOtherFile/Source/AddConstructorDependencyRector.php‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Rector\Tests\Issues\InheritedConstructorFromOtherFile\Source;
6+
7+
use PhpParser\Node;
8+
use PhpParser\Node\Stmt\Class_;
9+
use PHPStan\Type\ObjectType;
10+
use Rector\NodeManipulator\ClassDependencyManipulator;
11+
use Rector\PostRector\ValueObject\PropertyMetadata;
12+
use Rector\Rector\AbstractRector;
13+
use Rector\RuleDoc\RuleDefinition;
14+
use Rector\ValueObject\MethodName;
15+
16+
final class AddConstructorDependencyRector extends AbstractRector
17+
{
18+
public function __construct(
19+
private readonly ClassDependencyManipulator $classDependencyManipulator
20+
) {
21+
}
22+
23+
public function getRuleDefinition(): RuleDefinition
24+
{
25+
return new RuleDefinition('', []);
26+
}
27+
28+
/**
29+
* @return array<class-string<Node>>
30+
*/
31+
public function getNodeTypes(): array
32+
{
33+
return [Class_::class];
34+
}
35+
36+
/**
37+
* @param Class_ $node
38+
*/
39+
public function refactor(Node $node): ?Class_
40+
{
41+
if ($node->getMethod(MethodName::CONSTRUCT) !== null) {
42+
return null;
43+
}
44+
45+
$this->classDependencyManipulator->addConstructorDependency(
46+
$node,
47+
new PropertyMetadata('someService', new ObjectType(SomeService::class))
48+
);
49+
50+
return $node;
51+
}
52+
}

‎tests/Issues/InheritedConstructorFromOtherFile/Source/SomeParentWithDeepConstructor.php‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Rector\Tests\Issues\InheritedConstructorFromOtherFile\Source;
6+
7+
abstract class SomeParentWithDeepConstructor
8+
{
9+
/**
10+
* Pushes the constructor further into this file than the child fixture file is long
11+
*/
12+
public function padding(): array
13+
{
14+
return [
15+
'padding padding padding padding padding padding padding padding padding',
16+
'padding padding padding padding padding padding padding padding padding',
17+
'padding padding padding padding padding padding padding padding padding',
18+
'padding padding padding padding padding padding padding padding padding',
19+
'padding padding padding padding padding padding padding padding padding',
20+
'padding padding padding padding padding padding padding padding padding',
21+
'padding padding padding padding padding padding padding padding padding',
22+
'padding padding padding padding padding padding padding padding padding',
23+
'padding padding padding padding padding padding padding padding padding',
24+
'padding padding padding padding padding padding padding padding padding',
25+
'padding padding padding padding padding padding padding padding padding',
26+
'padding padding padding padding padding padding padding padding padding',
27+
'padding padding padding padding padding padding padding padding padding',
28+
'padding padding padding padding padding padding padding padding padding',
29+
'padding padding padding padding padding padding padding padding padding',
30+
'padding padding padding padding padding padding padding padding padding',
31+
'padding padding padding padding padding padding padding padding padding',
32+
'padding padding padding padding padding padding padding padding padding',
33+
'padding padding padding padding padding padding padding padding padding',
34+
'padding padding padding padding padding padding padding padding padding',
35+
'padding padding padding padding padding padding padding padding padding',
36+
'padding padding padding padding padding padding padding padding padding',
37+
'padding padding padding padding padding padding padding padding padding',
38+
'padding padding padding padding padding padding padding padding padding',
39+
'padding padding padding padding padding padding padding padding padding',
40+
];
41+
}
42+
43+
public function __construct(protected string $name)
44+
{
45+
}
46+
}

‎tests/Issues/InheritedConstructorFromOtherFile/Source/SomeService.php‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Rector\Tests\Issues\InheritedConstructorFromOtherFile\Source;
6+
7+
final class SomeService
8+
{
9+
}

‎tests/Issues/InheritedConstructorFromOtherFile/config/configured_rule.php‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
use Rector\Config\RectorConfig;
6+
use Rector\Tests\Issues\InheritedConstructorFromOtherFile\Source\AddConstructorDependencyRector;
7+
8+
return RectorConfig::configure()
9+
->withRules([AddConstructorDependencyRector::class]);

0 commit comments

Comments
 (0)