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

From: Date: Mon, 11 May 2015 11:25:26 +0000
Subject: Bug #69612 [Opn]: incorrect inheritance validation for type hints in method
References: 1  Groups: php.bugs 
Request: Send a blank email to php-bugs+get-192617@lists.php.net to get a copy of this message
Edit report at https://bugs.php.net/bug.php?id=69612&edit=1

 ID:                 69612
 Updated by:         cmb@php.net
 Reported by:        rhymoid at gmail dot com
 Summary:            incorrect inheritance validation for type hints in
                     method
 Status:             Open
 Type:               Bug
-Package:            PHP Language Specification
+Package:            Class/Object related
 PHP Version:        5.6
 Block user comment: N
 Private report:     N

 New Comment:

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.


Previous Comments:
------------------------------------------------------------------------
[2015-05-10 13:42:18] 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-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


Thread (5 messages)

« previous php.bugs (#192617) next »