Issue with more specific return type

Jean-FrancoisHIVERT

Hi,

I just found this issue:

<?php

declare(strict_types=1);

namespace Devsense\Common\ReturnType;

abstract class ParentClass {}

abstract class CustomClass extends ParentClass {}

abstract class SimpleClass extends CustomClass {}

final class A extends SimpleClass {}

trait MyTrait
{
    protected function _test(): ParentClass
    {
        return new A();
    }
}

final class MyClass
{
    use MyTrait;

    public function test(): SimpleClass
    {
        return $this->_test();
    }
}

Thank you for the fix.

JakubMisek

Jean-FrancoisHIVERT, the error looks alright.

ParentClass can't be cast to SimpleClass (the type hierarchy is the other way around).

Jean-FrancoisHIVERT

@JakubMisek It should be possible:
https://www.php.net/manual/en/language.oop5.variance.php

As of PHP 7.4.0, full covariance and contravariance support was added.

Example with php -a (console mode)

php > abstract class A { }
php > class B extends A { }
php > class C extends A { }
php > 
php > abstract class D { public function f1(): A { return new A(); } }
php > class E extends D { public function f1(): B { return new B(); } }
php > class F extends D { public function f1(): C { return new C(); } }
php > 
php > $e = new E();
php > $f = new F();
php > 
php > var_dump($e->f1());
object(B)#3 (0) {
}
php > 
php > var_dump($f->f1());
object(C)#3 (0) {
}

JakubMisek

Jean-FrancoisHIVERT but in your case, it is:

public function test(): SimpleClass {
  return $this->_test(); // <-- ParentClass
}

which is equivalent to:

// ...
class F extends D { public function f1(): C { return new B(); } }
// ...

resulting in

PHP Fatal error:  Uncaught TypeError: F::f1(): Return value must be of type C, B returned

The error is really correct - try returning new ParentClass from _test(),

    protected function _test(): ParentClass
    {
        return new ParentClass(); // make it non-abstract
    }

Fatal error: Uncaught TypeError: Devsense\Common\ReturnType\MyClass::test(): Return value must be of type Devsense\Common\ReturnType\SimpleClass, Devsense\Common\ReturnType\ParentClass returned

You can "avoid" the runtime error by returning A instead of ParentClass, but then the type hint is misleading.

Jean-FrancoisHIVERT

@JakubMisek This is the execution of the test with a var_dump at the end:

JakubMisek

Jean-FrancoisHIVERT, make it return ParentClass, as type hinted:

    protected function _test(): ParentClass
    {
        return new ParentClass(); // make it non-abstract
    }

Jean-FrancoisHIVERT

@JakubMisek Another test, it works with interfaces but not with inheritance:

<?php

declare(strict_types=1);

namespace Devsense\Common\ReturnType;

interface ParentInterface {}

interface SimpleInterface extends ParentInterface {}

abstract class ParentClass implements ParentInterface {}

abstract class CustomClass extends ParentClass
{
    public function getP(): ParentClass
    {
        return new S();
    }
}

abstract class SimpleClass extends CustomClass implements SimpleInterface
{
    public function getS(): SimpleClass
    {
        return $this->getP();
    }
}

final class A extends SimpleClass {}

final class S extends SimpleClass {}

final class C extends CustomClass {}

trait MyTrait
{
    protected function _test1(): ParentClass
    {
        return new A();
    }

    protected function _test2(): ParentInterface
    {
        return new A();
    }
}

final class MyClass
{
    use MyTrait;

    /**
     * https://community.devsense.com/d/2586-issue-with-more-specific-return-type
     */
    public function test1(): SimpleClass
    {
        return $this->_test1();
    }

    public function test2(): SimpleInterface
    {
        return $this->_test2();
    }
}

$a = new MyClass();
$r1 = $a->test1();
$r2 = $a->test2();

var_dump($r1, $r2);

JakubMisek

Jean-FrancoisHIVERT, we intentionally ignore this error with interfaces because there were too many of them in popular legacy frameworks.

The following warning:

Return value of getS() is expected to be of type Devsense\Common\ReturnType\SimpleClass, Devsense\Common\ReturnType\ParentClass returnedPHP(PHP0408)

is correct. ParentClass can't be converted to SimpleClass.

We should also report the same problem with ParentInterface and SimpleInterface.