From 8615d40f4c8acb58a000f5bc7e0d0a0cfd353b7f Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Sun, 5 Nov 2023 20:55:04 +0700 Subject: [PATCH] [NodeTypeResolver] Handle nullable extended class on ->isObjectType() on DowngradeReflectionGetAttributesRector (#5224) * [NodeTypeResolver] Handle nullable extended class on ->isObjectType() on DowngradeReflectionGetAttributesRector * [ci-review] Rector Rectify * [ci-review] Rector Rectify * Fixed :tada: * fix --------- Co-authored-by: GitHub Action --- .../NodeTypeResolver/NodeTypeResolver.php | 2 +- .../PHPUnit/AbstractRectorTestCase.php | 16 ++++----- .../config/configured_rule.php | 2 +- ...rideAttributeToOverriddenMethodsRector.php | 18 ++++++---- ...owngradeNullableReflectionPropertyTest.php | 28 +++++++++++++++ .../use_nullable_reflection_property.php.inc | 35 +++++++++++++++++++ .../config/configured_rule.php | 10 ++++++ 7 files changed, 93 insertions(+), 18 deletions(-) create mode 100644 tests/Issues/DowngradeNullableReflectionProperty/DowngradeNullableReflectionPropertyTest.php create mode 100644 tests/Issues/DowngradeNullableReflectionProperty/Fixture/use_nullable_reflection_property.php.inc create mode 100644 tests/Issues/DowngradeNullableReflectionProperty/config/configured_rule.php diff --git a/packages/NodeTypeResolver/NodeTypeResolver.php b/packages/NodeTypeResolver/NodeTypeResolver.php index 653eff814a7..fcc39df3095 100644 --- a/packages/NodeTypeResolver/NodeTypeResolver.php +++ b/packages/NodeTypeResolver/NodeTypeResolver.php @@ -438,7 +438,7 @@ private function isMatchingUnionType(Type $resolvedType, ObjectType $requiredObj return $this->isMatchObjectWithoutClassType($type, $requiredObjectType); } - return $type->isSuperTypeOf($requiredObjectType) + return $requiredObjectType->isSuperTypeOf($type) ->yes(); } diff --git a/packages/Testing/PHPUnit/AbstractRectorTestCase.php b/packages/Testing/PHPUnit/AbstractRectorTestCase.php index de8addde084..e523597ca2b 100644 --- a/packages/Testing/PHPUnit/AbstractRectorTestCase.php +++ b/packages/Testing/PHPUnit/AbstractRectorTestCase.php @@ -94,12 +94,10 @@ protected function setUp(): void $this->bootFromConfigFiles([$configFile]); $rectorsGenerator = $rectorConfig->tagged(RectorInterface::class); - if ($rectorsGenerator instanceof RewindableGenerator) { - $rectors = iterator_to_array($rectorsGenerator->getIterator()); - } else { + $rectors = $rectorsGenerator instanceof RewindableGenerator + ? iterator_to_array($rectorsGenerator->getIterator()) // no rules at all, e.g. in case of only post rector run - $rectors = []; - } + : []; /** @var RectorNodeTraverser $rectorNodeTraverser */ $rectorNodeTraverser = $rectorConfig->make(RectorNodeTraverser::class); @@ -231,7 +229,7 @@ private function doTestFileMatchesExpectedContent( // the file is now changed (if any rule matches) $rectorTestResult = $this->processFilePath($originalFilePath); - $changedContent = $rectorTestResult->getChangedContents(); + $changedContents = $rectorTestResult->getChangedContents(); $fixtureFilename = basename($fixtureFilePath); $failureMessage = sprintf('Failed on fixture file "%s"', $fixtureFilename); @@ -247,12 +245,12 @@ private function doTestFileMatchesExpectedContent( } try { - $this->assertSame($expectedFileContents, $changedContent, $failureMessage); + $this->assertSame($expectedFileContents, $changedContents, $failureMessage); } catch (ExpectationFailedException) { - FixtureFileUpdater::updateFixtureContent($originalFileContent, $changedContent, $fixtureFilePath); + FixtureFileUpdater::updateFixtureContent($originalFileContent, $changedContents, $fixtureFilePath); // if not exact match, check the regex version (useful for generated hashes/uuids in the code) - $this->assertStringMatchesFormat($expectedFileContents, $changedContent, $failureMessage); + $this->assertStringMatchesFormat($expectedFileContents, $changedContents, $failureMessage); } } diff --git a/rules-tests/Renaming/Rector/MethodCall/RenameMethodRector/config/configured_rule.php b/rules-tests/Renaming/Rector/MethodCall/RenameMethodRector/config/configured_rule.php index 7f674ae8af6..732f57d75ac 100644 --- a/rules-tests/Renaming/Rector/MethodCall/RenameMethodRector/config/configured_rule.php +++ b/rules-tests/Renaming/Rector/MethodCall/RenameMethodRector/config/configured_rule.php @@ -29,7 +29,7 @@ new MethodCallRename(DifferentInterface::class, 'renameMe', 'toNewVersion'), // reflection method name - new MethodCallRename(ReflectionMethod::class, 'getTentativeReturnType', 'getReturnType'), + new MethodCallRename(ReflectionFunctionAbstract::class, 'getTentativeReturnType', 'getReturnType'), // with array key new MethodCallRenameWithArrayKey('Nette\Utils\Html', 'addToArray', 'addToHtmlArray', 'hey'), diff --git a/rules/Php83/Rector/ClassMethod/AddOverrideAttributeToOverriddenMethodsRector.php b/rules/Php83/Rector/ClassMethod/AddOverrideAttributeToOverriddenMethodsRector.php index b8194e21fa1..f5ab2fd7a35 100644 --- a/rules/Php83/Rector/ClassMethod/AddOverrideAttributeToOverriddenMethodsRector.php +++ b/rules/Php83/Rector/ClassMethod/AddOverrideAttributeToOverriddenMethodsRector.php @@ -22,7 +22,7 @@ * @see https://wiki.php.net/rfc/marking_overriden_methods * @see \Rector\Tests\Php83\Rector\ClassMethod\AddOverrideAttributeToOverriddenMethodsRector\AddOverrideAttributeToOverriddenMethodsRectorTest */ -class AddOverrideAttributeToOverriddenMethodsRector extends AbstractRector implements MinPhpVersionInterface +final class AddOverrideAttributeToOverriddenMethodsRector extends AbstractRector implements MinPhpVersionInterface { public function __construct( private readonly ReflectionProvider $reflectionProvider, @@ -97,22 +97,25 @@ public function refactor(Node $node): ?Node $hasChanged = false; - foreach ($node->getMethods() as $method) { - if ($method->name->toString() === '__construct') { + foreach ($node->getMethods() as $classMethod) { + if ($classMethod->name->toString() === '__construct') { continue; } + // Private methods should be ignored - if ($parentClassReflection->hasNativeMethod($method->name->toString())) { + if ($parentClassReflection->hasNativeMethod($classMethod->name->toString())) { // ignore if it is a private method on the parent - $parentMethod = $parentClassReflection->getNativeMethod($method->name->toString()); + $parentMethod = $parentClassReflection->getNativeMethod($classMethod->name->toString()); if ($parentMethod->isPrivate()) { continue; } + // ignore if it already uses the attribute - if ($this->phpAttributeAnalyzer->hasPhpAttribute($method, 'Override')) { + if ($this->phpAttributeAnalyzer->hasPhpAttribute($classMethod, 'Override')) { continue; } - $method->attrGroups[] = new AttributeGroup([new Attribute(new FullyQualified('Override'))]); + + $classMethod->attrGroups[] = new AttributeGroup([new Attribute(new FullyQualified('Override'))]); $hasChanged = true; } } @@ -134,6 +137,7 @@ private function shouldSkipClass(Class_ $class): bool if ($this->classAnalyzer->isAnonymousClass($class)) { return true; } + if (! $class->extends instanceof FullyQualified) { return true; } diff --git a/tests/Issues/DowngradeNullableReflectionProperty/DowngradeNullableReflectionPropertyTest.php b/tests/Issues/DowngradeNullableReflectionProperty/DowngradeNullableReflectionPropertyTest.php new file mode 100644 index 00000000000..d117063552b --- /dev/null +++ b/tests/Issues/DowngradeNullableReflectionProperty/DowngradeNullableReflectionPropertyTest.php @@ -0,0 +1,28 @@ +doTestFile($filePath); + } + + public static function provideData(): Iterator + { + return self::yieldFilesFromDirectory(__DIR__ . '/Fixture'); + } + + public function provideConfigFilePath(): string + { + return __DIR__ . '/config/configured_rule.php'; + } +} diff --git a/tests/Issues/DowngradeNullableReflectionProperty/Fixture/use_nullable_reflection_property.php.inc b/tests/Issues/DowngradeNullableReflectionProperty/Fixture/use_nullable_reflection_property.php.inc new file mode 100644 index 00000000000..eded0cfff2f --- /dev/null +++ b/tests/Issues/DowngradeNullableReflectionProperty/Fixture/use_nullable_reflection_property.php.inc @@ -0,0 +1,35 @@ +getAttributes('SomeAttribute')[0] ?? null) { + return true; + } + + return false; + } +} + +?> +----- +getAttributes('SomeAttribute') : [])[0] ?? null) { + return true; + } + + return false; + } +} + +?> diff --git a/tests/Issues/DowngradeNullableReflectionProperty/config/configured_rule.php b/tests/Issues/DowngradeNullableReflectionProperty/config/configured_rule.php new file mode 100644 index 00000000000..9ebd576ba73 --- /dev/null +++ b/tests/Issues/DowngradeNullableReflectionProperty/config/configured_rule.php @@ -0,0 +1,10 @@ +rule(DowngradeReflectionGetAttributesRector::class); +};