Bug #69612 [Opn]: incorrect inheritance validation for type hints in method

Edit report at https://bugs.php.net/bug.php?id=69612&edit=1 ID: 69612 User updated by: rhymoid at gmail dot com Reported by: rhymoid at gmail dot com Summary: incorrect inheritance validation for type hints in method Status: Open Type: Bug -Package: Compile Failure +Package: PHP Language Specification -PHP Version: 5.6.4 +PHP Version: 5.6 Block user comment: N Private report: N New Comment: It seems that there is some disagreement among the developers of PHP as to what "compatibility" means. I'm surprised to find that, in the language specification, it is asserted that functions are invariant in the parameters they accept ("For typed argument, only argument with the same type is compatible."). There's also tests/classes/type_hinting_005b.phpt, which explains the warnings for OverrideWildcard. But that's not how basically any other language treats subtyping of functions. For instance, in #67544, requinix@php.net mentions "Liskov substitution principle" and "contravariance" as "relevant terminology", which implies that PHP should have contravariant parameters for function subtyping. As demonstrated with the provided example, this would also in line with the run-time behaviour of PHP. Please correct the notion of 'compatibility' in the PHP Language Specification to match what everyone expects it to be. Previous Comments: ------------------------------------------------------------------------ [2015-05-09 12:58:02] rhymoid at gmail dot com If the methods in Base are abstract, the declarations cause fatal errors. ------------------------------------------------------------------------ [2015-05-09 12:32:57] rhymoid at gmail dot com I'm going to guess the issue is this code: if (fe_arg_info->type_hint != proto_arg_info->type_hint) { /* Incompatible type hint */ return 0; } In PHP 5.6.4, it lives in ./Zend/zend_compile.c, lines 3340-3343 function zend_do_perform_implementation_check. In HEAD, it lives in ./Zend/zend_inheritance.c (http://git.php.net/?p=php-src.git;a=blob;f=Zend/zend_inheritance.c;h=f32c55aaef6fb12280dc3b9ddf5b8b857dbb3887;hb=HEAD#l245). It seems that with 1bc92476 (titled "- Added scalar typehinting.", from 2010-05-20) the meaning of this piece of code was lost (before it, there was apparently only array type hinting), and nobody ever looked at it again. ------------------------------------------------------------------------ [2015-05-09 11:45:09] rhymoid at gmail dot com Description: ------------ An overriding method in a subclass is technically still compatible if * it gained a default value (or = null in case of class type hints), or * a class type hint is changed into an ancestor class, or * a type hint is removed. These changes do not cause fatal errors in PHP 5.5, but do cause "Strict standards" warnings for the latter two. I believe this is incorrect: no warnings should be issued. This was hinted at in a comment from 2012-02-11 on bug #46851. Test script: --------------- <?php class Fruit {} class Banana extends Fruit {} class Lemon extends Fruit {} class Base { public function methodOne(Banana $x) {} public function methodTwo(array $x) {} } class OverrideNullable extends Base { public function methodOne(Banana $x = null) {} public function methodTwo(array $x = null) {} } class OverrideParent extends Base { public function methodOne(Fruit $x) {} } class OverrideWildcard extends Base { public function methodOne($x) {} public function methodTwo($x) {} } (new Base())->methodOne(new Banana()); (new Base())->methodTwo([]); (new OverrideNullable())->methodOne(); (new OverrideNullable())->methodTwo(); (new OverrideParent())->methodOne(new Lemon()); (new OverrideWildcard())->methodOne(''); (new OverrideWildcard())->methodTwo(''); echo "Output intentionally left blank." , PHP_EOL; Expected result: ---------------- Output intentionally left blank. Actual result: -------------- PHP Strict Standards: Declaration of OverrideParent::methodOne() should be compatible with Base::methodOne(Banana $x) in /private/tmp/test.php on line 22 Strict Standards: Declaration of OverrideParent::methodOne() should be compatible with Base::methodOne(Banana $x) in /private/tmp/test.php on line 22 PHP Strict Standards: Declaration of OverrideWildcard::methodOne() should be compatible with Base::methodOne(Banana $x) in /private/tmp/test.php on line 28 Strict Standards: Declaration of OverrideWildcard::methodOne() should be compatible with Base::methodOne(Banana $x) in /private/tmp/test.php on line 28 PHP Strict Standards: Declaration of OverrideWildcard::methodTwo() should be compatible with Base::methodTwo(array $x) in /private/tmp/test.php on line 28 Strict Standards: Declaration of OverrideWildcard::methodTwo() should be compatible with Base::methodTwo(array $x) in /private/tmp/test.php on line 28 Output intentionally left blank. ------------------------------------------------------------------------ -- Edit this bug report at https://bugs.php.net/bug.php?id=69612&edit=1