PhpStorm not recognizing return type as extension of another

1.1k views Asked by At

Scenario

I'm using PHP7 type hinting to say that my function has return type City.

public function getCityById(int $city_id) : City { ... }

In this function, I return the outcome of running a finder.

return $this->city_finder->findById($city_id);

But PhpStorm complains here, because the findById() function returns AbstractModel.

But I have class City extends AbstractModel, so this isn't a problem. PhpStorm doesn't seem to recognize this, however, and highlights the return statement with a warning.

I don't want to mute this type of warning (disable inspection), because it is an important warning to have.

Question

Can I make PhpStorm recognize that this return statement will satisfy the return type?

Additional info

One workaround is to extract a variable, and annotate it, like so:

/** @var City $city */
$city = $this->city_finder->findById($city_id);
return $city;

At this point, it stops warning me about it, but it seems like the extra line should be avoidable since it only exists to mute a warning in the IDE.

The findById() function is protected against returning the wrong type, because a Finder class is generated on a per-model basis.

$this->city_finder = $this->orm->getFinder(City::class);
//...
$city = $city_finder->findById(...);
2

There are 2 answers

3
Flying On

PHPStorm is correct.

Your findById() returns AbstractModel and it is more broad that City that you're trying to return. In a case if you'll receive from findById() some other class that is also inherited from AbstractModel, but is not a City or its descendant - you will receive fatal error from PHP and it is what PHPStorm warns you about.

You can workaround this by either adding annotation, as you do already or by explicitly checking if ($city instanceof City) {return $city; }. It may look slightly bloated by will be safe in runtime.

0
IMSoP On

PHPStorm is technically right here: according to the contracts of your functions, the statement might return the wrong value.

$result = $this->city_finder->findById($city_id);

The only thing we can know about $result here is what findById promises us, and that's that it's an AbstractModel. It might be a City, but it might equally be a User, or a Widget, or an anonymous object defined as class extends AbstractModel {}.

The underlying problem is that $this->city_finder is an instance of some abstract repository; while you know that it will always handle City objects, that's not actually built into your type system. If it was a more specific class, it could declare that it returned City objects from its findById method, and the code would be type safe.


Based on your comment that your ORM takes in the class name as a parameter ($finder = $orm->getFinder(City::class);), if PHP supported "generics" or "template meta-programming", you might write something like this:

class ORM {
    public function getFinder<T>(): Finder<T> { ... }
}
class Finder<T> {
     public function findById(int $id): T { ...}
}
$finder = $orm->getFinder<City>();
$city = $finder->findById($city_id);

The analyser would know that $finder was an instance of Finder<City>, and therefore that findById would return a City.

Without generics, though, there is no way to tell the analyser that if given City::class in its constructor, the Finder class will always return instances of that class.