Skip to content

[Php81][Php82][Privatization] Handle extends non-readonly class on ReadOnlyPropertyRector+ReadOnlyClassRector+FinalizeClassesWithoutChildrenRector#4524

Merged
samsonasik merged 10 commits intomainfrom
readonly-extends-non-readonly
Jul 16, 2023
Merged

[Php81][Php82][Privatization] Handle extends non-readonly class on ReadOnlyPropertyRector+ReadOnlyClassRector+FinalizeClassesWithoutChildrenRector#4524
samsonasik merged 10 commits intomainfrom
readonly-extends-non-readonly

Conversation

@samsonasik
Copy link
Copy Markdown
Member

…adOnlyPropertyRector+ReadOnlyClassRector+FinalizeClassesWithoutChildrenRector
@samsonasik samsonasik requested a review from TomasVotruba as a code owner July 15, 2023 22:21
@samsonasik samsonasik marked this pull request as draft July 15, 2023 22:22
@samsonasik
Copy link
Copy Markdown
Member Author

This is strange that it is not reproducible in CI unit test, while error on local

➜  rector-src git:(readonly-extends-non-readonly) vendor/bin/phpunit tests/Issues/ExtendsNonReadonlyClass/ExtendsNonReadonlyClassTest.php
PHPUnit 10.2.5 by Sebastian Bergmann and contributors.

Runtime:       PHP 8.2.5
Configuration: /Users/samsonasik/www/rector-src/phpunit.xml

F                                                                   1 / 1 (100%)

Time: 00:01.264, Memory: 90.50 MB

There was 1 failure:

1) Rector\Core\Tests\Issues\ExtendsNonReadonlyClass\ExtendsNonReadonlyClassTest::test with data set #0
Failed on fixture file "fixture.php.inc"
Failed asserting that string matches format description.
--- Expected
+++ Actual
@@ @@
 
 use Rector\Core\Tests\Issues\ExtendsNonReadonlyClass\Source\NonReadonlyClass;
 
-final class Fixture extends NonReadonlyClass
+final readonly class Fixture extends NonReadonlyClass
 {
     public function __construct(
-        private readonly string $foo
+        private string $foo
     ) {}

@samsonasik samsonasik force-pushed the readonly-extends-non-readonly branch from 6b8a2ad to d65ee3f Compare July 15, 2023 23:10
@samsonasik
Copy link
Copy Markdown
Member Author

Ok, the error is now reproduced with rename fixture dir

https://github.com/rectorphp/rector-src/actions/runs/5564649404/jobs/10164374851?pr=4524

@samsonasik samsonasik marked this pull request as ready for review July 16, 2023 05:44
@samsonasik
Copy link
Copy Markdown
Member Author

Fixed 🎉 /cc @fvozar @dkarlovi

@samsonasik
Copy link
Copy Markdown
Member Author

All checks have passed 🎉 @TomasVotruba I am merging it ;)

Current solution is by verify ClassReflection from Scope on ReadonlyClassRector 9190a86

this is not ideal solution yet, as the real solution should be on PHPStanNodeScopeResolver itself.

$context = $this->privatesAccessor->getPrivateProperty($mutatingScope, 'context');
$this->privatesAccessor->setPrivateProperty($context, 'classReflection', null);

but at least it fixed on this use case ;)

@samsonasik samsonasik merged commit 3b5db0a into main Jul 16, 2023
@samsonasik samsonasik deleted the readonly-extends-non-readonly branch July 16, 2023 05:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Incorrect behavior of ReadOnlyClassRector

2 participants