Skip to content

Commit

Permalink
Check abstract method signatures coming from traits
Browse files Browse the repository at this point in the history
  • Loading branch information
nikic committed Mar 26, 2020
1 parent beaacbc commit f74e30c
Show file tree
Hide file tree
Showing 12 changed files with 218 additions and 76 deletions.
19 changes: 18 additions & 1 deletion UPGRADING
Expand Up @@ -163,6 +163,21 @@ PHP 8.0 UPGRADE NOTES
accepted, and func as assumed to refer to T1::func. Now it will generate a
fatal error instead, and either T1::func or T2::func needs to be written
explicitly.
. The signature of abstract methods defined in traits is now checked against
the implementing class method:

trait MyTrait {
abstract private function neededByTrait(): string;
}

class MyClass {
use MyTrait;

// Error, because of return type mismatch.
private function neededByTrait(): int { return 42; }
}

RFC: https://wiki.php.net/rfc/abstract_trait_method_validation

- COM:
. Removed the ability to import case-insensitive constants from type
Expand Down Expand Up @@ -428,8 +443,10 @@ PHP 8.0 UPGRADE NOTES
. Some consistency fixes to variable syntax have been applied, for example
writing `Foo::BAR::$baz` is now allowed.
RFC: https://wiki.php.net/rfc/variable_syntax_tweaks
. Added Stringable.
. Added Stringable interface, which is automatically implemented if a class
defines a __toString() method.
RFC: https://wiki.php.net/rfc/stringable
. Traits can now define abstract private methods.

- Date:
. Added DateTime::createFromInterface() and
Expand Down
18 changes: 18 additions & 0 deletions Zend/tests/traits/abstract_method_1.phpt
@@ -0,0 +1,18 @@
--TEST--
Abstract method from trait enforced in class
--FILE--
<?php

trait T {
abstract public function neededByTheTrait(int $a, string $b);
}

class C {
use T;

public function neededByTheTrait(array $a, object $b) {}
}

?>
--EXPECTF--
Fatal error: Declaration of C::neededByTheTrait(array $a, object $b) must be compatible with T::neededByTheTrait(int $a, string $b) in %s on line %d
22 changes: 22 additions & 0 deletions Zend/tests/traits/abstract_method_2.phpt
@@ -0,0 +1,22 @@
--TEST--
Mutually incompatible methods from traits are fine as long as the final method is compatible
--FILE--
<?php

trait T1 {
abstract public function test();
}
trait T2 {
abstract public function test(): int;
}

class C {
use T1, T2;

public function test(): int {}
}

?>
===DONE===
--EXPECT--
===DONE===
18 changes: 18 additions & 0 deletions Zend/tests/traits/abstract_method_3.phpt
@@ -0,0 +1,18 @@
--TEST--
Private abstract method from trait enforced in class
--FILE--
<?php

trait T {
abstract private function neededByTheTrait(int $a, string $b);
}

class C {
use T;

private function neededByTheTrait(array $a, object $b) {}
}

?>
--EXPECTF--
Fatal error: Declaration of C::neededByTheTrait(array $a, object $b) must be compatible with T::neededByTheTrait(int $a, string $b) in %s on line %d
20 changes: 20 additions & 0 deletions Zend/tests/traits/abstract_method_4.phpt
@@ -0,0 +1,20 @@
--TEST--
Visibility enforcement on abstract trait methods
--FILE--
<?php

trait T {
abstract public function method(int $a, string $b);
}

class C {
use T;

/* For backwards-compatibility reasons, visibility is not enforced here. */
private function method(int $a, string $b) {}
}

?>
===DONE===
--EXPECT--
===DONE===
18 changes: 18 additions & 0 deletions Zend/tests/traits/abstract_method_5.phpt
@@ -0,0 +1,18 @@
--TEST--
Staticness enforcement on abstract trait methods
--FILE--
<?php

trait T {
abstract static public function method(int $a, string $b);
}

class C {
use T;

public function method(int $a, string $b) {}
}

?>
--EXPECTF--
Fatal error: Cannot make static method T::method() non static in class C in %s on line %d
20 changes: 20 additions & 0 deletions Zend/tests/traits/abstract_method_6.phpt
@@ -0,0 +1,20 @@
--TEST--
Abstract private trait method not implemented
--FILE--
<?php

trait T {
abstract private function method(int $a, string $b);
}

abstract class C {
use T;
}

class D extends C {
private function method(int $a, string $b) {}
}

?>
--EXPECTF--
Fatal error: Class C must implement 1 abstract private method (C::method) in %s on line %d
23 changes: 23 additions & 0 deletions Zend/tests/traits/abstract_method_7.phpt
@@ -0,0 +1,23 @@
--TEST--
Abstract private trait method forwarded as abstract protected method
--FILE--
<?php

trait T {
abstract private function method(int $a, string $b);
}

abstract class C {
use T;

abstract protected function method(int $a, string $b);
}

class D extends C {
protected function method(int $a, string $b) {}
}

?>
===DONE===
--EXPECT--
===DONE===
2 changes: 1 addition & 1 deletion Zend/tests/traits/bug60217b.phpt
Expand Up @@ -22,4 +22,4 @@ class CBroken {
$o = new CBroken;
$o->foo(1);
--EXPECTF--
Fatal error: Declaration of TBroken1::foo($a) must be compatible with TBroken2::foo($a, $b = 0) in %s
Fatal error: Declaration of CBroken::foo($a) must be compatible with TBroken2::foo($a, $b = 0) in %s on line %d
2 changes: 1 addition & 1 deletion Zend/tests/traits/bug60217c.phpt
Expand Up @@ -22,4 +22,4 @@ class CBroken {
$o = new CBroken;
$o->foo(1);
--EXPECTF--
Fatal error: Declaration of TBroken2::foo($a) must be compatible with TBroken1::foo($a, $b = 0) in %s on line %d
Fatal error: Declaration of CBroken::foo($a) must be compatible with TBroken1::foo($a, $b = 0) in %s on line %d
2 changes: 1 addition & 1 deletion Zend/zend_compile.c
Expand Up @@ -6111,7 +6111,7 @@ void zend_begin_method_decl(zend_op_array *op_array, zend_string *name, zend_boo
}

if (op_array->fn_flags & ZEND_ACC_ABSTRACT) {
if (op_array->fn_flags & ZEND_ACC_PRIVATE) {
if ((op_array->fn_flags & ZEND_ACC_PRIVATE) && !(ce->ce_flags & ZEND_ACC_TRAIT)) {
zend_error_noreturn(E_COMPILE_ERROR, "%s function %s::%s() cannot be declared private",
in_interface ? "Interface" : "Abstract", ZSTR_VAL(ce->name), ZSTR_VAL(name));
}
Expand Down

0 comments on commit f74e30c

Please sign in to comment.