Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Privatization] Do not remove comment on ChangeReadOnlyPropertyWithDefaultValueToConstantRector #3204

Merged
merged 6 commits into from
Dec 16, 2022

Conversation

samsonasik
Copy link
Member

The comment on ChangeReadOnlyPropertyWithDefaultValueToConstantRector should be mirrored during transformation.

@samsonasik
Copy link
Member Author

Fixed 🎉

@samsonasik
Copy link
Member Author

samsonasik commented Dec 15, 2022

It seems cause error in DowngradePropertyPromotionRector test:


There was 1 failure:

1) Rector\Tests\DowngradePhp80\Rector\Class_\DowngradePropertyPromotionRector\DowngradePropertyPromotionRectorTest::test with data set #0 ('/home/runner/work/rector-src/...hp.inc')
Failed asserting that string matches format description.
--- Expected
+++ Actual
@@ @@
 class ArrayShapes
 {
     /**
-     * @var array<string, array{\stdClass, \Exception}>
+     * @var array<string, array{\stdClass, \Exception, ...}>

https://github.com/rectorphp/rector-src/actions/runs/3706328433/jobs/6281403238

@samsonasik samsonasik marked this pull request as draft December 15, 2022 17:16
@samsonasik
Copy link
Member Author

@samsonasik
Copy link
Member Author

Let's pin to use pin to phpstan/phpdoc-parser 1.15.0

@samsonasik samsonasik marked this pull request as ready for review December 15, 2022 17:23
@samsonasik
Copy link
Member Author

All checks have passed 🎉 @TomasVotruba it is ready for review.

@TomasVotruba
Copy link
Member

TomasVotruba commented Dec 15, 2022

Thanks 👍

Is this solved with phpdoc-parser 1.15.1?

@samsonasik
Copy link
Member Author

samsonasik commented Dec 15, 2022

That's different issue. The ChangeReadOnlyPropertyWithDefaultValueToConstantRector not copied comment is because COMMENT attribute is not mirrored .

The issue on DowngradePropertyPromotionRectorTest is appear after phpstan/phpdoc-parser 1.15.1 just released that cause array shape issue so I pin to phpdoc-parser 1.15.0 for now.

To reproduce the issue on rector-downgrade-php repo package:

cd rector-downgrade-php
composer update

vendor/bin/phpunit rules-tests/DowngradePhp80/Rector/Class_/DowngradePropertyPromotionRector/DowngradePropertyPromotionRectorTest.php
PHPUnit 9.5.27 by Sebastian Bergmann and contributors.

.....F..                                                            8 / 8 (100%)

Time: 00:00.854, Memory: 72.50 MB

There was 1 failure:

1) Rector\Tests\DowngradePhp80\Rector\Class_\DowngradePropertyPromotionRector\DowngradePropertyPromotionRectorTest::test with data set #5 ('/Users/samsonasik/www/rector-...hp.inc')
Failed asserting that string matches format description.
--- Expected
+++ Actual
@@ @@
 class ArrayShapes
 {
     /**
-     * @var array<string, array{\stdClass, \Exception}>
+     * @var array<string, array{\stdClass, \Exception, ...}>
      */
     private array $value;
     /** @param array<string, array{\stdClass, \Exception}> $value */

/Users/samsonasik/www/rector-downgrade-php/vendor/rector/rector-src/packages/Testing/PHPUnit/AbstractRectorTestCase.php:186
/Users/samsonasik/www/rector-downgrade-php/vendor/rector/rector-src/packages/Testing/PHPUnit/AbstractRectorTestCase.php:119
/Users/samsonasik/www/rector-downgrade-php/rules-tests/DowngradePhp80/Rector/Class_/DowngradePropertyPromotionRector/DowngradePropertyPromotionRectorTest.php:17

By pin to phpdoc-parser 1.15.0, it is resolved:

composer require phpstan/phpdoc-parser:1.15.0
./composer.json has been updated
Running composer update phpstan/phpdoc-parser
Loading composer repositories with package information
Updating dependencies
Lock file operations: 0 installs, 1 update, 0 removals
  - Downgrading phpstan/phpdoc-parser (1.15.1 => 1.15.0)
Writing lock file
Installing dependencies from lock file (including require-dev)
Package operations: 0 installs, 1 update, 0 removals
  - Downloading phpstan/phpdoc-parser (1.15.0)
No patches supplied.
Gathering patches for dependencies. This might take a minute.
  - Downgrading phpstan/phpdoc-parser (1.15.1 => 1.15.0): Extracting archive
Generating autoload files
61 packages you are using are looking for funding.
Use the `composer fund` command to find out more!
phpstan/extension-installer: Extensions installed
No security vulnerability advisories found
➜  rector-downgrade-php git:(main) ✗ vendor/bin/phpunit rules-tests/DowngradePhp80/Rector/Class_/DowngradePropertyPromotionRector/DowngradePropertyPromotionRectorTest.php
PHPUnit 9.5.27 by Sebastian Bergmann and contributors.

........                                                            8 / 8 (100%)

Time: 00:00.925, Memory: 82.50 MB

OK (8 tests, 16 assertions)

@samsonasik
Copy link
Member Author

The array shape bug is due to new sealed config on ArrayShapeNode on phpdoc-parser 1.15.1

phpstan/phpdoc-parser@950bddf

@samsonasik
Copy link
Member Author

The issue on phpdoc-parser 1.15.1 ArrayShape is, the sealed default is true

https://github.com/phpstan/phpdoc-parser/blob/950bddf2c4203fa08ad27b8349bba1f32a8a7ebe/src/Parser/TypeParser.php#L508

@samsonasik
Copy link
Member Author

I am merging it ;)

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.

3 participants