Skip to content

Ignore inheritance rules on private methods #5401

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Zend/tests/method_argument_binding.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ class C extends B {
(new C)->test();

class D {
private final function method(&$x) {
private function method(&$x) {
++$x;
}
}
Expand Down
4 changes: 4 additions & 0 deletions Zend/zend_compile.c
Original file line number Diff line number Diff line change
Expand Up @@ -6091,6 +6091,10 @@ void zend_begin_method_decl(zend_op_array *op_array, zend_string *name, zend_boo

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 "
Expand Down
12 changes: 8 additions & 4 deletions Zend/zend_inheritance.c
Original file line number Diff line number Diff line change
Expand Up @@ -810,6 +810,14 @@ static zend_always_inline inheritance_status do_inheritance_check_on_method_ex(z
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;
Expand Down Expand Up @@ -851,10 +859,6 @@ static zend_always_inline inheritance_status do_inheritance_check_on_method_ex(z
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;

Expand Down
2 changes: 1 addition & 1 deletion tests/classes/clone_005.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
21 changes: 21 additions & 0 deletions tests/classes/final_private_ctor.phpt
Original file line number Diff line number Diff line change
@@ -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
51 changes: 51 additions & 0 deletions tests/classes/inheritance_007.phpt
Original file line number Diff line number Diff line change
@@ -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();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

?>

?>
--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
16 changes: 16 additions & 0 deletions tests/classes/inheritance_008.phpt
Original file line number Diff line number Diff line change
@@ -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';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

?>

?>
--EXPECT--
OK
14 changes: 14 additions & 0 deletions tests/classes/inheritance_009.phpt
Original file line number Diff line number Diff line change
@@ -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';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

?>

?>
--EXPECT--
OK