Skip to content

[Renaming] Handle crash in trait use on RenameClassRector#4196

Merged
samsonasik merged 5 commits intomainfrom
crash-trait-use
Jun 12, 2023
Merged

[Renaming] Handle crash in trait use on RenameClassRector#4196
samsonasik merged 5 commits intomainfrom
crash-trait-use

Conversation

@samsonasik
Copy link
Copy Markdown
Member

Given the following code:

trait LockDelete {
    public function extractObjectsId() {}
}

abstract class SkipNotChangedInTraitUse
{
    use LockDelete {
        LockDelete::extractObjectsId insteadof LockRemove;
    }
}

cause crash:

There was 1 error:

1) Rector\Tests\Renaming\Rector\Name\RenameClassRector\RenameClassRectorTest::test with data set #9
Rector\Core\Exception\ShouldNotHappenException: Scope not available on "PhpParser\Node\Name\FullyQualified" node with parent node of "PhpParser\Node\Stmt\TraitUseAdaptation\Precedence", but is required by a refactorWithScope() method of "Rector\Renaming\Rector\Name\RenameClassRector" rule. Fix scope refresh on changed nodes first

Ref https://getrector.com/demo/b533c7c7-2946-4ca9-8311-7e438f35ff2a
Fixes rectorphp/rector#7984

This PR try to fix it.

@samsonasik samsonasik requested a review from TomasVotruba as a code owner June 12, 2023 14:37
@samsonasik
Copy link
Copy Markdown
Member Author

Fixed /cc @GeoDaz with get Scope from ScopeFactory->createFromFile() when target node, eg: FullyQualified don't have Scope.

For note: FullyQualified when it is part of class, eg: class Foo, the Foo has Scope initiated, but on trait use, it somehow not have it, and can't be refreshed as need to be as Stmt which only allowed for Expr to Expression

see

if ($node instanceof Expr) {
return [new Expression($node)];
}

@samsonasik
Copy link
Copy Markdown
Member Author

All checks have passed 🎉 @TomasVotruba I think it is ready.

@samsonasik
Copy link
Copy Markdown
Member Author

@TomasVotruba I am merging it ;)

@samsonasik samsonasik merged commit 8c25c54 into main Jun 12, 2023
@samsonasik samsonasik deleted the crash-trait-use branch June 12, 2023 16:32
@GeoDaz
Copy link
Copy Markdown

GeoDaz commented Jun 13, 2023

Thank you very much @samsonasik

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

Could not process => Fix scope refresh on changed nodes first

2 participants