Inspection warning for multiple types

Is there a way to have PhpStorm warn me for the following incorrect code?

/** @var boolean|MyClass $a */
$a = false;
$a->myMethod();

Obviously $a is normaly returned by a function call that has boolean|MyClass @return hint, but the same applies.

A method that returns an object or false in the case of failure is common in our huge and not easliy refactored project. I agree that it's not good practice to have a mixed return type, but also I'm sure that a lot of projects use this.

I'd like PhpStorm to allow code-completion on $a, to let me find ->myMethod() in the MyClass class, but I sure want to be warned when ->myMethod is possibly not available.

Ideally the code below should not raise a warning.

/** @var false|MyClass $a */
$a = false;
if ($a != false) {
$a->myMethod();
}

 

/** @var boolean|MyClass $a */
$a = false;

if ($a instanceof MyClass) {
$a->getMaxIdleTime();
}

 

In my quest to find an answer I even found this 'solution' is not adequate, as it now incorrectly warns for the ->myMethod() call:

/** @var boolean $a */
$a = false;

if ($a instanceof MyClass) {
$a->myMethod();
}

 

Only this would work as I expect it to:

/** @var boolean $a */
$a = false;

if ($a instanceof MyClass) {
/** @var MyClass $a */
$a->myMethod();
}
2
6 comments

The initial code piece (+dummy class & method) doesn't return any errors by PHP. 

Could you please provide a full code sample w can use to for tests? 

0
Avatar
Permanently deleted user

Dmitry, obviously the following code will product a fatal 

/** @var boolean|MyClass $a */
$a
= false;
$a->myMethod();

PHP Fatal error:  Call to a member function myMethod() on boolean

Note that normally $a is the result of a function call and most of the times will an object of type MyClass, but sometimes return false on errors.

The typehint for $a is boolean|MyClass which means that PhpStorm knows that sometimes the ->myMethod() call would produce a fatal. I can find no way to enable code inspection warning for this type of situation. And I wish to know if I am wrong, or maybe there already is a feature request for inspection for this type of issue. I don't know what it's called, so there is no way I can find this in google or in your issue tracker. I tried.

0

@Casper

The key point here is that while IDE detects that $a is of boolean type on $a = false; line ... it still treats the next line fine because of the @var PHPDoc above .. which has priority over automatic detection.

And when you get your $a as a returned value from some function/method -- this does function properly .. because IDE cannot say what type $a is exactly (as it will be know during runtime only).

Lots of exiting code uses such approach (even PHP core functions/classes). So current behaviour is correct.

 

>Ideally the code below should not raise a warning.

It does not for me:

P.S. Such code is a correct way of working with such variables.

0
Avatar
Permanently deleted user

I'm not saying the behaviour is incorrect. I'm asking if there is an option to enable an inspection for possible bugs in my code. I will try to rephrase:

I have methods that return objects and sometimes false. This is a fact of life. This is not what I want to warn for.

People that write code (me and my colleagues) tend to forget to check for the false case, optimists as we are, and assume that the returned value is always an object. This obviously wrong. We should always check the false case and act accordingly, but instead we write code like:

$Invoice = $Repository->getObject();
$Invoice->getId();

This causes a lot of fatal error's in edge-case situations in production code.

I'd like to know if there is a way PhpStorm could help me spot this incorrect use of ->getId(), as it is not always an Invoice, but really is false or an Invoice object. The warning should be on ->getId() because calling that method is not always possible!

 

My guestimate is that about 30% of the fatal errors occurring in our production code can be attributed to this type of situation. I dug a little deeper and found that currently our monitoring system says:

Network and other external factors            33.00
Memory and CPU use 31.42
Call to a member function on bool or null 32.43
Type mismatch 2.36
Undefined method 0.79

As 'Network and other external factors' and 'Memory and CPU use' is not ever found using code inspection, leaving them out of the equation:

Call to a member function on bool or null     91.14
Type mismatch 6.65
Undefined method 2.22

91% of the fatal errors from our production system can be derived to the problem I am trying to describe in this post. As Andriy wrote: "Lots of exiting code uses such approach (even PHP core functions/classes)." I'm sure we are not the only one with this problem and I'm confident that my numbers are a pretty good reflection of what happens in production code in general.

This is why we are actively moving away from mixed return types, and use exceptions for these cases where in the past false was returned. But still, we have million lines of code that is written with mixed return types. Whenever a developer uses such a method that has a mixed return type I want to make sure PhpStorm warns the developer if the false (or null) situation is not handled properly.

2

+ for this task

very annoying lack of validation

0

We have a similar request for nullable symbols: https://youtrack.jetbrains.com/issue/WI-15078
It does not cover your concern fully because you are asking for a warning on any non-object type, but still.

0

Please sign in to leave a comment.