Phpinspectionsea: NPEs: respect variables type hinting

[erickskrauch]

Code example:

<?php

class User {

    public function action() {}

}

class Test {

    public function performAction() {
        /** @var User $user */
        $user = $this->getUser();
        $user->action();
    }

    /**
     * It also may be annotation like this:
     * @return User|null
     */
    public function getUser(): ?User {
        return new User();
    }

}

I expect, that if I overwrite type to be not nullable object, then inspection will trust me and do not highlight every usage of this variable.

[funivan]

@erickskrauch thanks for reports. In this special case I think inspection is correct. Why you use type hint here ?

[kalessil]

Thank you for reporting @erickskrauch =)

First, it's can be a bug in type resolving (by IDE itself).
Secon implicit return types have more trust than PhpDoc (PhpDoc does nothing at runtime), but I'd like to understand what causes ?User when newly created object returned. Why so?

[erickskrauch]

@kalessil I used ?User as return type just for example. In my current project I have some methods, that incapsulate some db queries and return some AR records (Yii2). In some cases method called few times. First time in validator, that know how to deal with null result, and another time in some action method, that running after validation and can be called only after validation passed. In this case, method unable to return null.

Also, it's a bit weird to see inspection detecting every nullable variable usage, but not first time access (because after first call it will be NPE or valid object of expected type).

[kalessil]

The inspection is not aware of any additional context (like first time use) - otherwise IDE would burn CPU =)

I'm not promising to end up with solution, but will check if we can utilize type-hint as in the example.

[erickskrauch]

@kalessil thanks :)

Just another example:

http://www.yiiframework.com/doc-2.0/yii-di-servicelocator.html#get()-detail

/** @var ServiceClass $service */
$service = Yii::$app->get('serviceName');
$service->method1();
$service->method2();

And each method call of this service leads to inspection highlight, although the IDEA understands the specified type for this variable in the annotation.

[kalessil]

@rentalhost @funivan : decision for NPE inspection to respect /** @var User $user */ before assignments is yours, what do we do?

[rentalhost]

Hmm... It is a big dilemma, once that getUser() could be User on past then ?User now, and will be hard to identify a phpdoc override. By otherside, I don't see another solution to that, so I think that we should treat it as an unfortunate exception.

In brief: I think that phpdoc should override method return type.

[funivan]

Agree with @rentalhost "phpdoc should override method return type."
Inside the project, var can be omitted (in most cases). But we have frameworks and other libraries which does not provide the proper type. So we can write @var or !==null every type. @var seems to be less annoying.

[kalessil]

Thanks, guys, we'll support the type specification.

[erickskrauch]

@kalessil Ultimate label/milestone means, that this feature will be available only in the paid version of the plugin?

[kalessil]

@erickskrauch : correct.

Here is described https://kalessil.github.io/page-blog-post-about-ea-ultimate.html how ultimate and extended roadmaps are planned. (shortly: Extended is migration tool and Ultimate is more about finding bugs/security issues).

[erickskrauch]

Oh... It's a bit sad because pay 50-100$ per ability to suppress inspection of the Community edition is too expensive. I think it should be part of the Null reference inspection and be available in the upcoming update. But it is only my vision.

[kalessil]

I also forget to mention that the Ultimate has a free 2-week trial, so people can decide to migrate on it or not.

[kalessil]

Sold, I'll back-port changes to extended.

[kalessil]

Follow up ticket is #783