Skip to content
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions src/Rector/AbstractRector.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
use PHPStan\Type\ObjectType;
use PHPStan\Type\Type;
use Rector\Application\ChangedNodeScopeRefresher;
use Rector\Application\NodeAttributeReIndexer;
use Rector\Application\Provider\CurrentFileProvider;
use Rector\BetterPhpDocParser\Comment\CommentsMerger;
use Rector\ChangesReporting\ValueObject\RectorWithLineChange;
Expand Down Expand Up @@ -136,6 +137,14 @@ final public function enterNode(Node $node): int|Node|null|array
// ensure origNode pulled before refactor to avoid changed during refactor, ref https://3v4l.org/YMEGN
$originalNode = $node->getAttribute(AttributeKey::ORIGINAL_NODE) ?? $node;

// this reindex is needed as when multiple rules apply
// the existing node position can already be removed/moved by different rule from "parent" node
//
// that modify/remove deep node, for example:
// - first rule: - Class_ → ClassMethod → remove index 0
// - second rule: - ClassMethod → here fetch the index 0 no longer exists
NodeAttributeReIndexer::reIndexNodeAttributes($node);
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@TomasVotruba this is the fix, the reindex early before refactor() is needed, as the node position can already be removed by other rule that use parent node as node type to check.

I added comment above for future reference.


$refactoredNodeOrState = $this->refactor($node);

// nothing to change → continue
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
<?php

namespace Rector\Tests\Issues\IssuePropertyPromoRemoveDelegatingParent\Fixture;

use Rector\Tests\Issues\IssuePropertyPromoRemoveDelegatingParent\Source\SomeParentWithEmptyConstruct;

class Fixture extends SomeParentWithEmptyConstruct
{
private string $prop;

public function __construct(string $prop)
{
$this->prop = $prop;
parent::__construct();
}
}

?>
-----
<?php

namespace Rector\Tests\Issues\IssuePropertyPromoRemoveDelegatingParent\Fixture;

use Rector\Tests\Issues\IssuePropertyPromoRemoveDelegatingParent\Source\SomeParentWithEmptyConstruct;

class Fixture extends SomeParentWithEmptyConstruct
{
public function __construct(private string $prop)
{
parent::__construct();
}
}

?>
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
<?php

declare(strict_types=1);

namespace Rector\Tests\Issues\IssuePropertyPromoRemoveDelegatingParent;

use Iterator;
use PHPUnit\Framework\Attributes\DataProvider;
use Rector\Testing\PHPUnit\AbstractRectorTestCase;

final class IssuePropertyPromoRemoveDelegatingParentTest extends AbstractRectorTestCase
{
#[DataProvider('provideData')]
public function test(string $filePath): void
{
$this->doTestFile($filePath);
}

public static function provideData(): Iterator
{
return self::yieldFilesFromDirectory(__DIR__ . '/Fixture');
}

public function provideConfigFilePath(): string
{
return __DIR__ . '/config/configured_rule.php';
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
<?php

declare(strict_types=1);

namespace Rector\Tests\Issues\IssuePropertyPromoRemoveDelegatingParent\Source;

class SomeParentWithEmptyConstruct
{
public function __construct()
{
$this->init();
}

private function init(): void
{
echo 'A init';
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<?php

use Rector\Config\RectorConfig;
use Rector\DeadCode\Rector\ClassMethod\RemoveParentDelegatingConstructorRector;
use Rector\Php80\Rector\Class_\ClassPropertyAssignToConstructorPromotionRector;

return RectorConfig::configure()
->withRules([
ClassPropertyAssignToConstructorPromotionRector::class,
RemoveParentDelegatingConstructorRector::class,
]);
Loading