Ignore inheritance rules on private methods

Closes GH-5401

diff --git a/UPGRADING b/UPGRADING
index cb4021b8a502fff9945efa666886a5380fd23eef..8e322bd1c133e51b29e6e14b423e4c93f5a9c7d9 100644 (file)
--- a/UPGRADING
+++ b/UPGRADING
@@ -194,6 +194,9 @@ PHP 8.0 UPGRADE NOTES
     RFC: https://wiki.php.net/rfc/locale_independent_float_to_string
   . Removed support for deprecated curly braces for offset access
     RFC: https://wiki.php.net/rfc/deprecate_curly_braces_array_access
+  . Applying the final modifier on a private method will now produce a warning
+    unless that method is the constructor.
+    RFC: https://wiki.php.net/rfc/inheritance_private_methods
 
 - COM:
   . Removed the ability to import case-insensitive constants from type
@@ -584,6 +587,10 @@ PHP 8.0 UPGRADE NOTES
     RFC: https://wiki.php.net/rfc/constructor_promotion
   . Added support for `match` expression.
     RFC: https://wiki.php.net/rfc/match_expression_v2
+  . Private methods declared on a parent class no longer enforce any
+    inheritance rules on the methods of a child class. (with the exception of
+    final private constructors)
+    RFC: https://wiki.php.net/rfc/inheritance_private_methods
 
 - Date:
   . Added DateTime::createFromInterface() and

diff --git a/Zend/tests/method_argument_binding.phpt b/Zend/tests/method_argument_binding.phpt
index f01690ea4e7db13ce396628bc0851210ac250e00..62720030349f0925959b6fd0e3eed274b100d46d 100644 (file)
--- a/Zend/tests/method_argument_binding.phpt
+++ b/Zend/tests/method_argument_binding.phpt
@@ -24,7 +24,7 @@ class C extends B {
 (new C)->test();
 
 class D {
-    private final function method(&$x) {
+    private function method(&$x) {
         ++$x;
     }
 }

diff --git a/Zend/zend_compile.c b/Zend/zend_compile.c
index 118c9681d22d8d1e13c8abf1056f686e8b607c92..6725e7ee826f531e2b3396140b31a44a6282025f 100644 (file)
--- a/Zend/zend_compile.c
+++ b/Zend/zend_compile.c
@@ -6486,6 +6486,10 @@ zend_string *zend_begin_method_decl(zend_op_array *op_array, zend_string *name,
 
        zend_string *lcname;
 
+       if ((fn_flags & ZEND_ACC_PRIVATE) && (fn_flags & ZEND_ACC_FINAL) && !zend_is_constructor(name)) {
+               zend_error(E_COMPILE_WARNING, "Private methods cannot be final as they are never overridden by other classes");
+       }
+
        if (in_interface) {
                if (!(fn_flags & ZEND_ACC_PUBLIC) || (fn_flags & (ZEND_ACC_FINAL|ZEND_ACC_ABSTRACT))) {
                        zend_error_noreturn(E_COMPILE_ERROR, "Access type for interface method "

diff --git a/Zend/zend_inheritance.c b/Zend/zend_inheritance.c
index 8a3c9ba1aac45e220cdf14ef2d22d2723b4a63f6..b70ce01b971abb603e73b8dbf6dbeb5fb093f293 100644 (file)
--- a/Zend/zend_inheritance.c
+++ b/Zend/zend_inheritance.c
@@ -828,6 +828,14 @@ static zend_always_inline inheritance_status do_inheritance_check_on_method_ex(
        uint32_t parent_flags = parent->common.fn_flags;
        zend_function *proto;
 
+       if (UNEXPECTED((parent_flags & ZEND_ACC_PRIVATE) && !(parent_flags & ZEND_ACC_ABSTRACT) && !(parent_flags & ZEND_ACC_CTOR))) {
+               if (!check_only) {
+                       child->common.fn_flags |= ZEND_ACC_CHANGED;
+               }
+               /* The parent method is private and not an abstract so we don't need to check any inheritance rules */
+               return INHERITANCE_SUCCESS;
+       }
+
        if (!checked && UNEXPECTED(parent_flags & ZEND_ACC_FINAL)) {
                if (check_only) {
                        return INHERITANCE_ERROR;
@@ -869,10 +877,6 @@ static zend_always_inline inheritance_status do_inheritance_check_on_method_ex(
                child->common.fn_flags |= ZEND_ACC_CHANGED;
        }
 
-       if ((parent_flags & ZEND_ACC_PRIVATE) && !(parent_flags & ZEND_ACC_ABSTRACT)) {
-               return INHERITANCE_SUCCESS;
-       }
-
        proto = parent->common.prototype ?
                parent->common.prototype : parent;
 

diff --git a/tests/classes/clone_005.phpt b/tests/classes/clone_005.phpt
index f7592214804764c0079c7c3e609b9043b2f22aff..b8e3870a8dcdffd2a3a5520dab3652cf51ea0d05 100644 (file)
--- a/tests/classes/clone_005.phpt
+++ b/tests/classes/clone_005.phpt
@@ -6,7 +6,7 @@ abstract class base {
   public $a = 'base';
 
   // disallow cloning once forever
-  final private function __clone() {}
+  final protected function __clone() {}
 }
 
 class test extends base {

diff --git a/tests/classes/final_private_ctor.phpt b/tests/classes/final_private_ctor.phpt
new file mode 100644 (file)
index 0000000..37dbe56
--- /dev/null
+++ b/tests/classes/final_private_ctor.phpt
@@ -0,0 +1,21 @@
+--TEST--
+Final private constructors cannot be overridden
+--FILE--
+<?php
+
+class Base
+{
+    private final function __construct()
+    {
+    }
+}
+class Extended extends Base
+{
+    public function __construct()
+    {
+    }
+}
+
+?>
+--EXPECTF--
+Fatal error: Cannot override final method Base::__construct() in %s on line %d

diff --git a/tests/classes/inheritance_007.phpt b/tests/classes/inheritance_007.phpt
new file mode 100644 (file)
index 0000000..9115dc0
--- /dev/null
+++ b/tests/classes/inheritance_007.phpt
@@ -0,0 +1,51 @@
+--TEST--
+Ensure private methods with the same name are not checked for inheritance rules - final
+--FILE--
+<?php
+class A {
+    function callYourPrivates() {
+        $this->normalPrivate();
+        $this->finalPrivate();
+    }
+    function notOverridden_callYourPrivates() {
+        $this->normalPrivate();
+        $this->finalPrivate();
+    }
+    private function normalPrivate() {
+        echo __METHOD__ . PHP_EOL;
+    }
+    final private function finalPrivate() {
+        echo __METHOD__ . PHP_EOL;
+    }
+}
+class B extends A {
+    function callYourPrivates() {
+        $this->normalPrivate();
+        $this->finalPrivate();
+    }
+    private function normalPrivate() {
+        echo __METHOD__ . PHP_EOL;
+    }
+    final private function finalPrivate() {
+        echo __METHOD__ . PHP_EOL;
+    }
+}
+$a = new A();
+$a->callYourPrivates();
+$a->notOverridden_callYourPrivates();
+$b = new B();
+$b->callYourPrivates();
+$b->notOverridden_callYourPrivates();
+?>
+--EXPECTF--
+Warning: Private methods cannot be final as they are never overridden by other classes %s
+
+Warning: Private methods cannot be final as they are never overridden by other classes %s
+A::normalPrivate
+A::finalPrivate
+A::normalPrivate
+A::finalPrivate
+B::normalPrivate
+B::finalPrivate
+A::normalPrivate
+A::finalPrivate

diff --git a/tests/classes/inheritance_008.phpt b/tests/classes/inheritance_008.phpt
new file mode 100644 (file)
index 0000000..2bc2783
--- /dev/null
+++ b/tests/classes/inheritance_008.phpt
@@ -0,0 +1,16 @@
+--TEST--
+Ensure private methods with the same name are not checked for inheritance rules - static
+--FILE--
+<?php
+class A {
+    static private function foo() { }
+    private function bar() {}
+}
+class B extends A {
+    private function foo() {}
+    static private function bar() {}
+}
+echo 'OK';
+?>
+--EXPECT--
+OK

diff --git a/tests/classes/inheritance_009.phpt b/tests/classes/inheritance_009.phpt
new file mode 100644 (file)
index 0000000..042eec7
--- /dev/null
+++ b/tests/classes/inheritance_009.phpt
@@ -0,0 +1,14 @@
+--TEST--
+Ensure private methods with the same name are not checked for inheritance rules - abstract
+--FILE--
+<?php
+class A {
+    private function test() {}
+}
+abstract class B extends A {
+    abstract function test();
+}
+echo 'OK';
+?>
+--EXPECT--
+OK