From: Pedro Magalhães Date: Thu, 16 Apr 2020 17:53:13 +0000 (+0100) Subject: Ignore inheritance rules on private methods X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=272b887b7b8d48bc1615938dde825fe4b3af0eb5;p=php Ignore inheritance rules on private methods Closes GH-5401 --- diff --git a/UPGRADING b/UPGRADING index cb4021b8a5..8e322bd1c1 100644 --- 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 f01690ea4e..6272003034 100644 --- 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 118c9681d2..6725e7ee82 100644 --- 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 8a3c9ba1aa..b70ce01b97 100644 --- 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 f759221480..b8e3870a8d 100644 --- 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 index 0000000000..37dbe56c31 --- /dev/null +++ b/tests/classes/final_private_ctor.phpt @@ -0,0 +1,21 @@ +--TEST-- +Final private constructors cannot be overridden +--FILE-- + +--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 index 0000000000..9115dc00e8 --- /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-- +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 index 0000000000..2bc2783e2c --- /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-- + +--EXPECT-- +OK diff --git a/tests/classes/inheritance_009.phpt b/tests/classes/inheritance_009.phpt new file mode 100644 index 0000000000..042eec7435 --- /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-- + +--EXPECT-- +OK