Phpinspectionsea: [FR] extending "'instanceof' can be used" inspection

[orklah]

| Subject | Details |
| :------------- | :---------------------------------------------------------------------------- |
| Issue type | Feature request |
| Plugin | Php Inspections (EA Extended) |
| Language level | PHP 5.3+ |

There is a lot of PHP function who can check if a variable or a string is somewhat linked with an other one. With a quick research, I can came up with a few:
is_a()
get_class()
is_subclass_of()
get_parent_class()
class_parents()
class_implements()
instanceof

Each has its own use, some of them check Classes, some check Interfaces. Some of them can use string, some can't. etc...

This is one case I hate dealing with in PHP (especially in older versions where not all params are available) and I never figure out which I should use in what case.

There is already an inspection in EA who suggest replacing "get_class($var) === "Class"" with $var instanceof "Class". I think there is a lot more case which could be checked with all of the above functions.

For example I can think of:

in_array('MyClass', class_implements($MyParentClass)); //-> $MyParentClass instanceof 'MyClass'
in_array($MyClassObject, class_implements($MyParentClass)); //-> $MyParentClass instanceof $MyClassObject
in_array('MyClass', class_implements('MyParentClass')); -> // Can't replace with instanceof
in_array($MyClassObject, class_implements('MyParentClass')); -> // Can't replace with instanceof

I could try to make a comprehensive list of what can be done and what can't but this is a lot of work and I just want to make sure this is something that could be integrated in this plugin before beggining seriously.

Any feedback?

[kalessil]

@orklah: my feedback is positive, please go ahead with populating such list.
I'll take care of implementing and maintaining the inspection later.

[orklah]

Hi,

I tried to do something clear, but I'm not sure if it's going to be easy to apprehend.

Here's the code:
Repo

There are 7 experiments files, each one is a standalone file which can be executed. If everything is ok, there shouldn't be an "alert" message in the output.

For each experiment, I played with 3 things: a Class, a children Class and an interface. I made every combination and I tried to put them in 4 categories:

• success: the target of the inspection
• failures: either the illogical cases or cases where both methods fail
• false positives: cases where the old method was working but the new one don't
• not applicable: cases where the new method work but the old one wasn't

With those results, I tried to came up with conditions and setbacks for each experiment that I detailed in a comment near the beggining of the file.

Some experiments have serious setbacks, so every replacement may not be needed in the final inspection, I'll let you be the judge.

Finally, there is a test file you can use to test wether the inspection suggest a replacement and wether it avoids the false positives.

Feel free to use and modify everything to your needs. Please tell me if I can make something better or more clear. It's my first time doing something like this.

[kalessil]

Awesome, thank you @orklah! I'll transfer the results into inspection over holidays.

[kalessil]

Implemented!

[orklah]

Awesome! I can't wait for the next release :)

[kalessil]

Should happen ~ mid of January =)

[orklah]

Hi!

Thanks for the early release!

I went back to my test file to check if the improved inspection caught all the cases. It seems to have missed some of them:

1) Every cases involving an interface. For example:

in_array('MyInterface', class_implements($MyClassObject));
is_a($MyClassObject, 'MyInterface');
is_subclass_of($MyClassObject, 'MyInterface');
is_subclass_of($MyClassObject, 'MyInterface', false);

2) is_a with $allow_string = true, but with an object in first param anyway
When we type:

is_a($MyClassObject, 'MyClass', true);

It doesn't work. But the type of $MyClassObject is an object so it shouldn't be a problem to suggest the change

Is this on purpose?

Thanks as always!

[kalessil]

Will check test fixtures, probably it was intended (I don't remember exactly).

[kalessil]

Re-opening in order to review comments