Edit report at https://bugs.php.net/bug.php?id=73987&edit=1
ID: 73987
Updated by: requinix@php.net
Reported by: requinix@php.net
Summary: Method compatibility check looks to original
definition and not parent
Status: Open
Type: Bug
Package: Class/Object related
PHP Version: 7.1.1
Block user comment: N
Private report: N
New Comment:
> As "B extends A" doesn't imply "B implements I" indeed.
PHP says it does: https://3v4l.org/ZQgJG (that PHP checks
B<->I compatibility proves it)
C# says it does: http://ideone.com/AZRdRy
Java says it does: http://ideone.com/CZICMN
Longer explanation - I know you are familiar with this, but I'm writing it all out for anyone
else reading.
1. method($param) vs. method($param=null)
LSP says parameters must be contravariant (parent->child) meaning one used in a child class must
be the same or more permissive. $param=null is more permissive than $param as it allows the
parameter to be omitted. Therefore
(a) is_a(A,I) and I::example($a, $b) therefore A::example($a, $b=null) is allowed
(b) is_a(B,I) and I::example($a, $b) therefore B::example($a, $b) is allowed
(c) is_a(B,A) and A::example($a, $b=null) therefore B::example($a, $b) is NOT allowed
According to examples #1 and #2, PHP allows (c) to happen because it only checks I->A, I->B
inheritance and not A->B. It is also clear that I->A and I->B does not imply A->B.
If PHP instead checks I->A and A->B (the parent class/interface) then example #1 fails in the
same way that example #2 fails. Yes this is a change, and some code that "works" today
will not "work" tomorrow, however as the accepts_a() demo shows such code does not
actually "work" today - the fact that nobody has noticed means they haven't written
an accepts_a() function yet.
If PHP does check I->A and A->B then it implies I->B as well.
2. method() vs. method():int
LSP says return types are covariant (parent<-child); PHP requires invariance when the parent uses
a return type and covariance when the parent does not. Covariance is being the same or less
permissive. No type is like saying ":mixed", and int is less permissive than mixed.
Therefore
(a) is_a(A,I) and I::example() therefore A::example():int is allowed
(b) is_a(B,I) and I::example() therefore B::example():string is allowed
(c) is_a(B,A) and A::example():int therefore B::example():string is NOT allowed
Again, according to examples #3 and #4, PHP allows (c) because it only checks I<-A, I<-B
inheritance and not A<-B. I<-A and I<-B also does not imply A<-B.
Again, if PHP checked I<-A and A<-B then example #3 fails; existing working code does not
actually work as accepts_a_expects_int() demonstrates.
And again, if PHP checks I<-A and A<-B then it implies I<-B.
The goal of changing validation to be against the parent rather than the interface is not to allow
more code. It is to be more strict and to disallow some code which seems to, but does not truly,
work correctly.
> the suggested way will in fact disallow even what works today
Do you have an example that does not already violate LSP?
> even more advanced examples are possible
Such as?
Previous Comments:
------------------------------------------------------------------------
[2017-01-27 13:51:51] ab@php.net
It's clear, that there's no method overloading of the methods with the same name in PHP,
but the suggested way will in fact disallow even what works today. Shouldn't the interface be
defined explicitly in this case? As semantically the examples look not controversial, in other
programming languages with stronger OOP, even more advanced examples are possible. As "B
extends A" doesn't imply "B implements I" indeed.
Thanks.
------------------------------------------------------------------------
[2017-01-27 09:13:13] mail at pmmaga dot net
I had a go at fixing this issue. Please review the PR. Thanks
------------------------------------------------------------------------
[2017-01-24 14:29:36] requinix@php.net
Related To: Bug #73985
------------------------------------------------------------------------
[2017-01-24 14:24:53] requinix@php.net
Description:
------------
Originally spotted as bug #73985.
When PHP validates method signatures for compatibility, if a method is defined in an interface then
compatibility is measured against the interface and any parent method's signature is ignored.
This leads to inconsistencies...
Example #1: method is defined in an interface (valid) - https://3v4l.org/pblqI
----------
interface I {
public function example($a, $b, $c);
}
class A implements I {
public function example($a, $b = null, $c = null) { } // compatible with I::example
}
class B extends A {
public function example($a, $b, $c = null) { } // compatible with I::example
}
Example #2: method is not defined in an interface (invalid) - https://3v4l.org/bJJ1h
----------
//interface I {
// public function example($a, $b, $c);
//}
class A {
public function example($a, $b = null, $c = null) { }
}
class B extends A {
public function example($a, $b, $c = null) { } // not compatible with A::example
}
The same signature is used in A and B, however only the second has a problem. The first is easy to
explain on its own ("example" was defined in I so methods must be compatible with
I::example) and the second is easy to explain on its own ("example" was defined in A so
methods must be compatible with A::example) however the two together are inconsistent.
The problem appears when calling a function using an I or A parameter type - https://3v4l.org/OneV3
---
function accepts_i(I $i) {
$i->example(1, 2, 3);
}
accepts_i(new B); // no problem
function accepts_a(A $a) {
$a->example(1);
}
accepts_a(new B); // problem
---
PHP <7.1: missing argument 2 for B::example
PHP >=7.1: ArgumentCountError: Too few arguments to function B::example, 1 passed
A::example() only has one required argument, therefore it should be safe for accepts_a to call
->example(1). But it isn't.
Like with method parameters, this problem also exists for return types.
Example #3: interface does not have return type (valid) - https://3v4l.org/Jik7I
----------
interface I {
public function example();
}
class A implements I {
public function example(): int { } // compatible with I::example
}
class B extends A {
public function example(): string { } // compatible with I::example
}
Example #4: class has a return type (invalid) - https://3v4l.org/n1q0G
----------
<?php
//interface I {
// public function example();
//}
class A {
public function example(): int { }
}
class B extends A {
public function example(): string { } // not compatible with A::example
}
Like with method parameters, this can result in unexpected behavior. Unlike with method parameters,
there's no warning about it - https://3v4l.org/TUbfJ
---
function accepts_i_expects_any(I $i) {
var_dump($i->example());
}
accepts_i_expects_any(new B); // receives string, no problem
function accepts_a_expects_int(A $a) {
var_dump($a->example());
}
accepts_a_expects_int(new B); // receives string, problem
---
Proposed solution: methods in a subclass are validated against methods in the nearest ancestor who
(re)defines the method - be that normally, as abstract, or using a trait. The result is that since A
implemented example(), B::example gets validated against that rather than the original definition of
I::example.
BC: Yes, but code reliant on current behavior is susceptible to the "inconsistencies"
noted earlier so it's already flawed.
Test script:
---------------
<?php // https://3v4l.org/gl1Gt
interface I {
public function example($a, $b, $c);
}
class A implements I {
public function example($a, $b = null, $c = null): int { }
}
class B extends A {
public function example($a, $b, $c = null): string { }
}
?>
Expected result:
----------------
Error that B::example is not compatible with A::example, due to the return type (a fatal error by
itself) and the second required argument (a warning by itself).
Actual result:
--------------
No error(s).
------------------------------------------------------------------------
--
Edit this bug report at https://bugs.php.net/bug.php?id=73987&edit=1