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

GetClassToInstanceOfRector invalid fix to instanceof for strict type check #8639

Open
jeremyVignelles opened this issue May 15, 2024 · 4 comments
Labels

Comments

@jeremyVignelles
Copy link

jeremyVignelles commented May 15, 2024

Bug Report

Subject Details
Rector version v1.0.4

I'm using getclass($object) === A::class to test if my variable is exactly of type A (and not a subclass).
Rector tries to edit this into an instanceof check, due to the GetClassToInstanceOfRector rule

Minimal PHP Code Causing Issue

<?php

class A {}
class B extends A {}

$c = new B();

if (get_class($c) === A::class) {
    echo 'KO - it is exactly of type A';
} else {
    echo 'OK - it is not exactly of type A';
}

Code gets "fixed" to $c instanceof \A

Expected Behaviour

I should be able to express that I am willing to keep that get_class call to check that exact type.

@jeremyVignelles
Copy link
Author

Seems like $c::class === A::class is OK for rector, is that the recommended thing to do ?

@samsonasik
Copy link
Member

@TomasVotruba @jeremyVignelles I am thinking that this rule should only cover !== instead of ===, as it seems valid? it requires some verification tho...

<?php

class A {}
class B extends A {}

$c = new B();

-if (get_class($c) !== A::class) {
+if (! $c instanceof A) {
   
}

wdyt?

@samsonasik
Copy link
Member

Oh, the result seems different https://3v4l.org/7cbU6

@samsonasik
Copy link
Member

If the type is exactly of what it defined (sub type), it can be checked by something like:

        $varType = $this->nodeTypeResolver->getNativeType($varNode);
        $objectType = new ObjectType($className);

        if ($varType instanceof ObjectType
            && ! $this->typeComparator->areTypesEqual($varType, $objectType)
            && $this->typeComparator->isSubtype($varType, $objectType)) {
            return null;
        }

some verification on dynamic variable may be needed. I guess for your use case, you can skip the rule:

https://getrector.com/documentation/ignoring-rules-or-paths

as it may be usefull to improve type check, but on some very specific use case, it may cause a bug.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants