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

[StaticReflection] Fix unintended behavior in PHP 8.1 and later #3340

Merged
merged 4 commits into from Feb 6, 2023

Conversation

zeriyoshi
Copy link
Contributor

PHP 8.1 changes the behavior of passing null to built-in functions, so that the current Rector does not throw an error when processing non-existent classes and does not properly perform code conversion.

https://www.php.net/manual/en/migration81.deprecated.php#migration81.deprecated.core.null-not-nullable-internal

Applying this fix will at least correct the above problem with Reflection.

But maybe this is an issue that should be addressed on the PHPStan side. I suggest a workaround on the Rector side for now.

$ vendor/bin/phpunit rules-tests/Renaming/Rector/Name/RenameClassRector/RenameMissingParent.php

1) Rector\Tests\Renaming\Rector\Name\RenameClassRector\RenameMissingParent::test with data set #0
TypeError: array_keys(): Argument #1 ($array) must be of type array, null given

phar:///Users/zeriyoshi/Development/rector-src/vendor/phpstan/phpstan/phpstan.phar/vendor/ondrejmirtes/better-reflection/src/Reflection/ReflectionClass.php:1038
phar:///Users/zeriyoshi/Development/rector-src/vendor/phpstan/phpstan/phpstan.phar/vendor/ondrejmirtes/better-reflection/src/Reflection/ReflectionClass.php:1361
phar:///Users/zeriyoshi/Development/rector-src/vendor/phpstan/phpstan/phpstan.phar/vendor/ondrejmirtes/better-reflection/src/Reflection/ReflectionClass.php:1211
phar:///Users/zeriyoshi/Development/rector-src/vendor/phpstan/phpstan/phpstan.phar/vendor/ondrejmirtes/better-reflection/src/Reflection/ReflectionClass.php:1212
phar:///Users/zeriyoshi/Development/rector-src/vendor/phpstan/phpstan/phpstan.phar/vendor/ondrejmirtes/better-reflection/src/Reflection/Adapter/ReflectionClass.php:301
phar:///Users/zeriyoshi/Development/rector-src/vendor/phpstan/phpstan/phpstan.phar/src/Reflection/ClassReflection.php:761
phar:///Users/zeriyoshi/Development/rector-src/vendor/phpstan/phpstan/phpstan.phar/src/Reflection/ClassReflection.php:714
phar:///Users/zeriyoshi/Development/rector-src/vendor/phpstan/phpstan/phpstan.phar/src/PhpDoc/PhpDocBlock.php:227
phar:///Users/zeriyoshi/Development/rector-src/vendor/phpstan/phpstan/phpstan.phar/src/PhpDoc/PhpDocBlock.php:206
phar:///Users/zeriyoshi/Development/rector-src/vendor/phpstan/phpstan/phpstan.phar/src/PhpDoc/PhpDocBlock.php:180
phar:///Users/zeriyoshi/Development/rector-src/vendor/phpstan/phpstan/phpstan.phar/src/PhpDoc/PhpDocBlock.php:172
phar:///Users/zeriyoshi/Development/rector-src/vendor/phpstan/phpstan/phpstan.phar/src/PhpDoc/PhpDocInheritanceResolver.php:41
phar:///Users/zeriyoshi/Development/rector-src/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/NodeScopeResolver.php:3335
phar:///Users/zeriyoshi/Development/rector-src/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/NodeScopeResolver.php:469
phar:///Users/zeriyoshi/Development/rector-src/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/NodeScopeResolver.php:360
phar:///Users/zeriyoshi/Development/rector-src/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/NodeScopeResolver.php:599
phar:///Users/zeriyoshi/Development/rector-src/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/NodeScopeResolver.php:360
phar:///Users/zeriyoshi/Development/rector-src/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/NodeScopeResolver.php:571
phar:///Users/zeriyoshi/Development/rector-src/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/NodeScopeResolver.php:327
/Users/zeriyoshi/Development/rector-src/packages/NodeTypeResolver/PHPStan/Scope/PHPStanNodeScopeResolver.php:348
/Users/zeriyoshi/Development/rector-src/packages/NodeTypeResolver/PHPStan/Scope/PHPStanNodeScopeResolver.php:225
/Users/zeriyoshi/Development/rector-src/packages/NodeTypeResolver/NodeScopeAndMetadataDecorator.php:31
/Users/zeriyoshi/Development/rector-src/src/Application/FileProcessor.php:34
/Users/zeriyoshi/Development/rector-src/src/Application/FileProcessor/PhpFileProcessor.php:109
/Users/zeriyoshi/Development/rector-src/src/Application/FileProcessor/PhpFileProcessor.php:53
/Users/zeriyoshi/Development/rector-src/src/Application/ApplicationFileProcessor.php:126
/Users/zeriyoshi/Development/rector-src/packages/Testing/PHPUnit/AbstractRectorTestCase.php:204
/Users/zeriyoshi/Development/rector-src/packages/Testing/PHPUnit/AbstractRectorTestCase.php:171
/Users/zeriyoshi/Development/rector-src/packages/Testing/PHPUnit/AbstractRectorTestCase.php:119
/Users/zeriyoshi/Development/rector-src/rules-tests/Renaming/Rector/Name/RenameClassRector/RenameMissingParent.php:20

Copy link
Member

@samsonasik samsonasik left a comment

Choose a reason for hiding this comment

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

Looks good

@TomasVotruba
Copy link
Member

Thanks 👍

@TomasVotruba TomasVotruba merged commit 256571c into rectorphp:main Feb 6, 2023
Copy link
Member

@samsonasik samsonasik left a comment

Choose a reason for hiding this comment

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

Could you add fixture for rename implements Interface to ensure no regression? Thank you

@samsonasik
Copy link
Member

Oh, already merged, I will checked on laminas-servicemanager-migration that rename interface instead of class and check if there is no regression

@samsonasik
Copy link
Member

@zeriyoshi I verified in laminas-servicemanager-migration for rename interface and it seems working ok, thank you 👍

➜  laminas-servicemanager-migration git:(1.13.x) ✗ vendor/bin/phpunit 
PHPUnit 9.6.3 by Sebastian Bergmann and contributors.

...................                                               19 / 19 (100%)

Time: 00:03.495, Memory: 166.50 MB

@zeriyoshi zeriyoshi deleted the fix_missing_parent_rename branch February 6, 2023 13:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants