php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Request #69612 incorrect inheritance validation for type hints in method
Submitted: 2015-05-09 11:45 UTC Modified: 2015-05-11 11:25 UTC
Votes:4
Avg. Score:3.5 ± 1.1
Reproduced:4 of 4 (100.0%)
Same Version:1 (25.0%)
Same OS:1 (25.0%)
From: rhymoid at gmail dot com Assigned:
Status: Open Package: Class/Object related
PHP Version: 5.6 OS:
Private report: No CVE-ID: None
View Add Comment Developer Edit
Welcome! If you don't have a Git account, you can't do anything here.
You can add a comment by following this link or if you reported this bug, you can edit this bug over here.
(description)
Block user comment
Status: Assign to:
Package:
Bug Type:
Summary:
From: rhymoid at gmail dot com
New email:
PHP Version: OS:

 

 [2015-05-09 11:45 UTC] 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.


Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2015-05-09 12:32 UTC] rhymoid at gmail dot com
-PHP Version: 5.5.24 +PHP Version: 5.6.4
 [2015-05-09 12:32 UTC] 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 12:58 UTC] rhymoid at gmail dot com
-Summary: incorrect strict warnings for type hints in method +Summary: incorrect inheritance validation for type hints in method -Package: Compile Warning +Package: Compile Failure
 [2015-05-09 12:58 UTC] rhymoid at gmail dot com
If the methods in Base are abstract, the declarations cause fatal errors.
 [2015-05-10 13:42 UTC] rhymoid at gmail dot com
-Package: Compile Failure +Package: PHP Language Specification -PHP Version: 5.6.4 +PHP Version: 5.6
 [2015-05-10 13:42 UTC] rhymoid at gmail dot com
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.
 [2015-05-11 11:25 UTC] cmb@php.net
-Package: PHP Language Specification +Package: Class/Object related
 [2015-05-11 11:25 UTC] cmb@php.net
Not allowing contravariant parameter type hints is not a bug; it
is rather a restriction that's not uncommon for programming
languages (C++ and Java, for instance, also accept only invariant
parameter types; covariance is treated as function overloading).
The restriction is there for technical reasons (classes used for
parameter type hinting do not yet have to be declared when the
code is compiled; see <http://3v4l.org/YVj6W>). This may change in
the future.

Therefore I'm changing this issue to feature request; it might
also be classified as documentation problem, because the php.net
manual doesn't seem to mention the restriction to invariant
parameter type hints.
 [2015-05-11 11:25 UTC] cmb@php.net
-Type: Bug +Type: Feature/Change Request
 
PHP Copyright © 2001-2019 The PHP Group
All rights reserved.
Last updated: Mon Mar 25 05:01:26 2019 UTC