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

Refactor is_a #5907

Merged
merged 3 commits into from
Mar 21, 2021
Merged

Refactor is_a #5907

merged 3 commits into from
Mar 21, 2021

Conversation

samsonasik
Copy link
Member

Fixes #5906

@samsonasik
Copy link
Member Author

@ondrejmirtes this is_a change 5ac198c seems cause error in test:

1) Rector\Tests\NetteToSymfony\Rector\Interface_\DeleteFactoryInterfaceRector\DeleteFactoryInterfaceFileSystemRectorTest::test with data set #0 (Symplify\SmartFileSystem\SmartFileInfo Object (...))
61
Failed asserting that false is true.

@TomasVotruba PHPStan notice seems unrelated as i tried in local and got same error with or without this change:

vendor/bin/phpstan analyse --ansi --error-format symplify packages/BetterPhpDocParser/ValueObject/PhpDocNode/JMS/SerializerTypeTagValueNode.php
Note: Using configuration file /Users/samsonasik/www/rector/phpstan.neon.
PHP Fatal error:  Class Rector\BetterPhpDocParser\ValueObject\PhpDocNode\JMS\SerializerTypeTagValueNode contains 1 abstract method and must therefore be declared abstract or implement the remaining methods (PHPStan\PhpDocParser\Ast\Node::hasAttribute) in /Users/samsonasik/www/rector/packages/BetterPhpDocParser/ValueObject/PhpDocNode/JMS/SerializerTypeTagValueNode.php on line 13
Fatal error: Class Rector\BetterPhpDocParser\ValueObject\PhpDocNode\JMS\SerializerTypeTagValueNode contains 1 abstract method and must therefore be declared abstract or implement the remaining methods (PHPStan\PhpDocParser\Ast\Node::hasAttribute) in /Users/samsonasik/www/rector/packages/BetterPhpDocParser/ValueObject/PhpDocNode/JMS/SerializerTypeTagValueNode.php on line 13

@TomasVotruba
Copy link
Member

@samsonasik I need to merge this #5841 first

@samsonasik
Copy link
Member Author

@TomasVotruba thank you 👍

@samsonasik
Copy link
Member Author

samsonasik commented Mar 19, 2021

@ondrejmirtes I got the patch about failing unit test, the UnionType need to check against ShortenedObjectType to make $returnType re-assigned with new ObjectType($returnType->getFullyQualifiedName()) --> f4ee4c4

@samsonasik
Copy link
Member Author

test error start from change 79f8698

-        return is_a($currentType->getClassName(), $newType->getClassName(), true);
+        return $newType->isSuperTypeOf($currentType)->yes();

@samsonasik
Copy link
Member Author

try check one by one, it seems caused by 2bc0a98

@samsonasik
Copy link
Member Author

somehow, it fixed with return early on yes() check instead of no() and continue:

-                if (! is_a($type->getClassName(), $countableObjectType->getClassName(), true)) {
-                     continue;
+                 if ($countableObjectType->isSuperTypeOf($type)->yes()) {
+                     return true;
+                 }

@samsonasik samsonasik force-pushed the refactor-is-a branch 2 times, most recently from 57452d7 to ed79bce Compare March 19, 2021 17:48
]);

if ($returnType instanceof ShortenedObjectType) {
$returnType = new ObjectType($returnType->getFullyQualifiedName());
Copy link
Member

Choose a reason for hiding this comment

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

The check work without this wraparound, as in other places

Copy link
Member Author

Choose a reason for hiding this comment

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

without this check, it got error in test like in my previous comment #5907 (comment)

@samsonasik
Copy link
Member Author

@ondrejmirtes I tried update other places, but got errors like in my previous comments, I rollback to only apply what you suggested in #5906 .

@samsonasik samsonasik changed the title [WIP] Refactor is_a Refactor is_a Mar 20, 2021
@TomasVotruba
Copy link
Member

I'll look into this

@TomasVotruba TomasVotruba merged commit 747104d into main Mar 21, 2021
@TomasVotruba TomasVotruba deleted the refactor-is-a branch March 21, 2021 12:12
@TomasVotruba
Copy link
Member

Thank you 👍

TomasVotruba added a commit that referenced this pull request May 22, 2024
rectorphp/rector-src@b2c9e52 [PostRector] Move instanceof check early before SimpleParameterProvider static call check (#5907)
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.

is_a() usage in the codebase
2 participants