php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #79120 Behaviour difference between extending abstract methods vs non-abstract methods
Submitted: 2020-01-15 15:17 UTC Modified: 2020-01-17 10:01 UTC
From: marijn at suninet dot org Assigned:
Status: Wont fix Package: *General Issues
PHP Version: 7.4.1 OS: Linux
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: marijn at suninet dot org
New email:
PHP Version: OS:

 

 [2020-01-15 15:17 UTC] marijn at suninet dot org
Description:
------------
Hi,

There seems to be a behaviour difference between extending from abstract methods vs non-abstract methods for an incompatible declaration:

https://3v4l.org/gkAm1

In PHP >= 7.0 if you have an incompatible declaration PHP will throw a warning if it's a non-abstract method. However if it's an abstract method and the incompatible declaration is in an extend of an extend (but compatible with the abstract class) then in PHP 7.0 and 7.1 nothing is raised, but since 7.2 a Fatal is raised. This is as far as I can tell also undocumented behaviour, it's not in the backwards incompatible changelist, though I assume it's part of some kind of bug fix.

Either way in my test example I would expect both functions to raise a Warning, or both a Failed, but not one a Warning and one a Fatal, that is inconsistent. The problem might be that even though ExtendsExtendedClass is overriding a non-abstract method (since it is already implemented in ExtendsAbstractClass), the abstract flag is still set (from AbstractClass) which turns what should be a warning into a fatal.

Test script:
---------------
<?php

abstract class AbstractClass {
    public function notAbstract() {}
    public abstract function isAbstract();
}

class ExtendsAbstractClass extends AbstractClass {
    public function notAbstract($arg = null) {}
    public function isAbstract($arg = null) {}
}

class ExtendsExtendedClass extends ExtendsAbstractClass {
    public function notAbstract() {}
    public function isAbstract() {}
}

Expected result:
----------------
Either two warnings or two fatals, not a fatal and a warning.

Actual result:
--------------
PHP 7.0 and 7.1:

Warning: Declaration of ExtendsExtendedClass::notAbstract() should be compatible with ExtendsAbstractClass::notAbstract($arg = NULL) in /in/gkAm1 on line 16

PHP 7.2+:

Warning: Declaration of ExtendsExtendedClass::notAbstract() should be compatible with ExtendsAbstractClass::notAbstract($arg = NULL) in /in/gkAm1 on line 14

Fatal error: Declaration of ExtendsExtendedClass::isAbstract() must be compatible with ExtendsAbstractClass::isAbstract($arg = NULL) in /in/gkAm1 on line 15

Process exited with code 255.

Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2020-01-15 19:14 UTC] requinix@php.net
-Summary: marijn@suninet.org +Summary: Behaviour difference between extending abstract methods vs non-abstract methods
 [2020-01-17 10:01 UTC] nikic@php.net
-Status: Open +Status: Wont fix
 [2020-01-17 10:01 UTC] nikic@php.net
Arguably this is a bug, but taking into account that this is all going to be an unconditional fatal error in PHP 8 anyway, I don't think it is worthwhile to address this issue. To fix this "properly" we'd have to validate the method against multiple signatures (both the direct parent and the closest abstract parent) *just* to decide whether we want to emit a warning or a fatal error. I don't think that's worthwhile. (The pre-7.2 behavior is also incorrect, just in the different direction: It does not indicate that ExtendsExtendedClass::isAbstract() is incompatible with its direct parent.)
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Fri Mar 29 08:01:27 2024 UTC