Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Invocation parameter types are not compatible with declared

Tags:

php

Phpstorm has inspection: Invocation parameter types are not compatible with declared.

I was suprised that php allow use base type as subtype.

interface Base
{
    public function getId();
}

interface Child extends Base
{

}

interface SecondChildType extends Base
{

}

class ChildImpl implements Child
{
    public function getId()
    {
        return 1;
    }
}

class SecondChildTypeImpl implements SecondChildType
{
    public function getId()
    {
        return 2;
    }
}

class BaseService
{
    public function process(Base $base)
    {
        $childService = new ChildService($base);

        return $childService->process($base); //Invocation parameter types are not compatible with declared
    }
}

class ChildService
{
    public function process(Child $child)
    {
        return $child->getId();
    }
}

class InheritanceTest extends \PHPUnit_Framework_TestCase
{
    public function testInterfacesCanUsesAsSubstitute()
    {
        $baseService = new BaseService();
        $this->assertEquals(1, $baseService->process(new ChildImpl()));
    }

    /**
     * @expectedException \TypeError
     */
    public function testInterfacesCanUsesAsSubstitute_Exception()
    {
        $baseService = new BaseService();
        $baseService->process(new SecondChildTypeImpl());
    }
}

Why first test pass? Why php allowed it?

like image 559
Onedev_Link Avatar asked Jun 17 '16 18:06

Onedev_Link


2 Answers

PhpStorm is warning you that your code potentially allows for an instance of Base to BaseService::process that is NOT a valid Child instance, and therefore can't be passed to ChildService::process.

In your first unit test you provided an instance of Child, which extends Base, so it worked.

In your second unit test you prove in fact that it's possible to cause a PHP error. PhpStorm is simply warning you in advance that your typehints allow for this problem.

If BaseService::process will always call ChildService::process like you have right now, then BaseService::process should typehint its argument to be compatible with ChildService::process as well.


I've modified your code a bit, rewritten some of the class names to be simpler, and removed the getId method. I just want to show this as simple as possible, to help you understand what's going on.

interface Base {}
interface Child extends Base {}
interface Base2 extends Base {}

// This class implements Child, which extends Base. So this will meet either requirement.
class Class1 implements Child {}

// This class implements Base2, which extends Base. 
// So this will meet any Base requirement, but NOT a Child requirement
class Class2 implements Base2 {}


class BaseService
{
    /**
     * Problem! We are requiring Base here, but then we pass the same argument to
     * ChildService->process, which requires Child. 
     * 
     * 1) Class1 WILL work, since it implements Child which extends Base.
     * 
     * 2) Class2 WILL NOT work. Or at least, we can't pass it to ChildService->process
     *    since it only implements Base2 which extends Base. It doesn't implement Child,
     *    therefore ChildService->process won't accept it.
     */
    public function process(Base $base)
    {
        $childService = new ChildService($base);

        return $childService->process($base);
    }
}

class ChildService
{
    /**
     * I will ONLY receive an instance that implements Child. 
     * Class1 will work, but not Class2.
     */
    public function process(Child $child)
    {
        return $child->getId();
    }
}

$service = new BaseService();

// I can do this! I'm passing in Child1, which implements Child, which extends Base.
// So it fulfills the initial Base requirement, and the secondary Child requirement.
$service->process(new Child1());

// I can't do this. While BaseService will initially accept it, ChildService will refuse
// it because this doesn't implement the Child interface as required.
$service->process(new Child2());
like image 140
jszobody Avatar answered Nov 02 '22 22:11

jszobody


I think you're expecting a Liskov Substitution Principle violation, but that's not the case here: ChildService does not derive from BaseService.

As you know, derived classes satisfy base class type hints, but derived class methods cannot strengthen the API of base class methods. So you can pass a Child into a method accepting a Base, but you can't strengthen the signature of a method shared between Child and Base.

The following classic code demonstrates the attempt to violate LSP, and throws a fatal "Declaration must be compatible":

abstract class AbstractService { }
abstract class AbstractFactory { abstract function make(AbstractService $s); }

class ConcreteService extends AbstractService { }
class ConcreteFactory extends AbstractFactory { function make(ConcreteService $s) {} }

You do something very similar in your code, with the crucial difference that ChildService does not inherit from an abstract BaseService. Thus ChildService is free to accept whatever arguments it wants, and there is no fatal error from PHP. If you change base service to abstract and derive child service from it, you'll get an LSP violation. (See also this question.)

Now, BaseService::process() accepts a ChildImpl because ChildImpl is-a Base. It also accepts anything implementing Child, and anything implementing Base. That means the following code is valid:

class BaseImpl implements Base {
    public function getId() { return 0; }
}
(new BaseService)->process(new BaseImpl);

But this will blow up, because BaseService::process hands off to ChildService::process, which takes a Child -- and BaseImpl is not a Child. PHPStorm has performed this static analysis, and it is warning you of the possible run-time consequence of the design.

like image 39
bishop Avatar answered Nov 02 '22 23:11

bishop