From 2ee879479f7b660a66e528136f07df29154dd139 Mon Sep 17 00:00:00 2001 From: Daniel Scherzer Date: Wed, 25 Sep 2024 16:51:45 -0700 Subject: [PATCH 1/5] GH-16067: test current output So that it is clear what changes --- Zend/tests/abstract_implicit.phpt | 19 +++++++++++++++++++ Zend/tests/anon/gh16067.phpt | 11 +++++++++++ 2 files changed, 30 insertions(+) create mode 100644 Zend/tests/abstract_implicit.phpt create mode 100644 Zend/tests/anon/gh16067.phpt diff --git a/Zend/tests/abstract_implicit.phpt b/Zend/tests/abstract_implicit.phpt new file mode 100644 index 000000000000..1f9f586f9d34 --- /dev/null +++ b/Zend/tests/abstract_implicit.phpt @@ -0,0 +1,19 @@ +--TEST-- +Abstract methods not allowed in classes that are not abstract (GH-16067) +--FILE-- + +--EXPECTF-- +Fatal error: Class NotAbstract contains 1 abstract method and must therefore be declared abstract or implement the remaining method (NotAbstract::bar) in %s on line %d diff --git a/Zend/tests/anon/gh16067.phpt b/Zend/tests/anon/gh16067.phpt new file mode 100644 index 000000000000..9252ceef75bd --- /dev/null +++ b/Zend/tests/anon/gh16067.phpt @@ -0,0 +1,11 @@ +--TEST-- +Compiler prevents explicit `abstract` methods on anonymous classes +--FILE-- + +--EXPECTF-- +Fatal error: Class class@anonymous must implement 1 abstract method (class@anonymous::f) in %s on line 3 From de22d20ace3168884b7ee3d75abd1cd7c9697bbc Mon Sep 17 00:00:00 2001 From: Daniel Scherzer Date: Wed, 25 Sep 2024 17:02:01 -0700 Subject: [PATCH 2/5] GH-16067: prevent invalid `abstract` during compilation of methods For classes that are not declared `abstract`, produce a compiler error for any `abstract` methods. For anonymous classes, since they cannot be made abstract, the error message is slightly different. --- Zend/tests/abstract_implicit.phpt | 2 +- Zend/tests/anon/gh16067.phpt | 2 +- Zend/zend_compile.c | 21 +++++++++++++++++++++ 3 files changed, 23 insertions(+), 2 deletions(-) diff --git a/Zend/tests/abstract_implicit.phpt b/Zend/tests/abstract_implicit.phpt index 1f9f586f9d34..5211074c743e 100644 --- a/Zend/tests/abstract_implicit.phpt +++ b/Zend/tests/abstract_implicit.phpt @@ -16,4 +16,4 @@ class NotAbstract { } ?> --EXPECTF-- -Fatal error: Class NotAbstract contains 1 abstract method and must therefore be declared abstract or implement the remaining method (NotAbstract::bar) in %s on line %d +Fatal error: Class NotAbstract contains abstract method NotAbstract::bar and must therefore be declared abstract in %s on line %d diff --git a/Zend/tests/anon/gh16067.phpt b/Zend/tests/anon/gh16067.phpt index 9252ceef75bd..11bcbcc6f928 100644 --- a/Zend/tests/anon/gh16067.phpt +++ b/Zend/tests/anon/gh16067.phpt @@ -8,4 +8,4 @@ $c = new class { } ?> --EXPECTF-- -Fatal error: Class class@anonymous must implement 1 abstract method (class@anonymous::f) in %s on line 3 +Fatal error: Anonymous class class@anonymous cannot contain abstract method class@anonymous::f in %s on line 4 diff --git a/Zend/zend_compile.c b/Zend/zend_compile.c index 146af9a22df2..2ccfd0c08ff0 100644 --- a/Zend/zend_compile.c +++ b/Zend/zend_compile.c @@ -8067,6 +8067,27 @@ static zend_string *zend_begin_method_decl(zend_op_array *op_array, zend_string zend_error(E_COMPILE_WARNING, "Private methods cannot be final as they are never overridden by other classes"); } + if ((fn_flags & ZEND_ACC_ABSTRACT) && + !(ce->ce_flags & (ZEND_ACC_EXPLICIT_ABSTRACT_CLASS|ZEND_ACC_TRAIT)) && + !in_interface + ) { + // Don't say that the class should be declared abstract if it is + // anonymous and can't be abstract + const char *msg; + if (ce->ce_flags & ZEND_ACC_ANON_CLASS) { + msg = "Anonymous class %s cannot contain abstract method %s::%s"; + } else { + msg = "Class %s contains abstract method %s::%s and must therefore be declared abstract"; + } + zend_error_noreturn( + E_COMPILE_ERROR, + msg, + ZSTR_VAL(ce->name), + ZSTR_VAL(ce->name), + ZSTR_VAL(name) + ); + } + if (in_interface) { if (!(fn_flags & ZEND_ACC_PUBLIC)) { zend_error_noreturn(E_COMPILE_ERROR, "Access type for interface method " From 9a5168db26cb145c0cdb27b18cd92f9c1b9dd297 Mon Sep 17 00:00:00 2001 From: Daniel Scherzer Date: Wed, 25 Sep 2024 17:47:10 -0700 Subject: [PATCH 3/5] Fix tests --- Zend/tests/errmsg/errmsg_018.phpt | 2 +- tests/classes/abstract_derived.phpt | 2 +- tests/classes/abstract_not_declared.phpt | 2 +- tests/classes/abstract_redeclare.phpt | 2 +- tests/classes/abstract_static.phpt | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Zend/tests/errmsg/errmsg_018.phpt b/Zend/tests/errmsg/errmsg_018.phpt index 070247e4d419..752e70abb4d0 100644 --- a/Zend/tests/errmsg/errmsg_018.phpt +++ b/Zend/tests/errmsg/errmsg_018.phpt @@ -10,4 +10,4 @@ class test { echo "Done\n"; ?> --EXPECTF-- -Fatal error: Class test contains 1 abstract method and must therefore be declared abstract or implement the remaining method (test::foo) in %s on line %d +Fatal error: Class test contains abstract method test::foo and must therefore be declared abstract in %s on line %d diff --git a/tests/classes/abstract_derived.phpt b/tests/classes/abstract_derived.phpt index 11c9125a7724..330f9cdcd664 100644 --- a/tests/classes/abstract_derived.phpt +++ b/tests/classes/abstract_derived.phpt @@ -13,4 +13,4 @@ class derived extends base { ?> ===DONE=== --EXPECTF-- -Fatal error: Class derived contains 1 abstract method and must therefore be declared abstract or implement the remaining method (derived::show) in %sabstract_derived.php on line %d +Fatal error: Class derived contains abstract method derived::show and must therefore be declared abstract in %sabstract_derived.php on line %d diff --git a/tests/classes/abstract_not_declared.phpt b/tests/classes/abstract_not_declared.phpt index 8f899f0e718a..592399c6d748 100644 --- a/tests/classes/abstract_not_declared.phpt +++ b/tests/classes/abstract_not_declared.phpt @@ -10,4 +10,4 @@ class fail { echo "Done\n"; // shouldn't be displayed ?> --EXPECTF-- -Fatal error: Class fail contains 1 abstract method and must therefore be declared abstract or implement the remaining method (fail::show) in %s on line %d +Fatal error: Class fail contains abstract method fail::show and must therefore be declared abstract in %s on line %d diff --git a/tests/classes/abstract_redeclare.phpt b/tests/classes/abstract_redeclare.phpt index f8e3d2346524..dafa35d460eb 100644 --- a/tests/classes/abstract_redeclare.phpt +++ b/tests/classes/abstract_redeclare.phpt @@ -16,4 +16,4 @@ class fail extends pass { echo "Done\n"; // Shouldn't be displayed ?> --EXPECTF-- -Fatal error: Class fail contains 1 abstract method and must therefore be declared abstract or implement the remaining method (fail::show) in %sabstract_redeclare.php on line %d +Fatal error: Class fail contains abstract method fail::show and must therefore be declared abstract in %sabstract_redeclare.php on line %d diff --git a/tests/classes/abstract_static.phpt b/tests/classes/abstract_static.phpt index ab13289ab4cd..97c4c910c191 100644 --- a/tests/classes/abstract_static.phpt +++ b/tests/classes/abstract_static.phpt @@ -31,4 +31,4 @@ echo "Done\n"; // shouldn't be displayed --EXPECTF-- Call to function show() -Fatal error: Class fail contains 1 abstract method and must therefore be declared abstract or implement the remaining method (fail::func) in %sabstract_static.php(%d) : eval()'d code on line %d +Fatal error: Class fail contains abstract method fail::func and must therefore be declared abstract in %sabstract_static.php(%d) : eval()'d code on line %d From 11e100ffa99f49814d0dc2c794141e5daeac046a Mon Sep 17 00:00:00 2001 From: Daniel Scherzer Date: Thu, 26 Sep 2024 03:56:49 -0700 Subject: [PATCH 4/5] Handle enums and improve condition --- Zend/tests/enum/no-abstract.phpt | 12 ++++++++++++ Zend/zend_compile.c | 7 ++++--- 2 files changed, 16 insertions(+), 3 deletions(-) create mode 100644 Zend/tests/enum/no-abstract.phpt diff --git a/Zend/tests/enum/no-abstract.phpt b/Zend/tests/enum/no-abstract.phpt new file mode 100644 index 000000000000..a0b8e02c2ee9 --- /dev/null +++ b/Zend/tests/enum/no-abstract.phpt @@ -0,0 +1,12 @@ +--TEST-- +Compiler prevents `abstract` methods on enums classes (GH-16067) +--FILE-- + +--EXPECTF-- +Fatal error: Enum Example cannot contain abstract method Example::foo in %s on line 4 diff --git a/Zend/zend_compile.c b/Zend/zend_compile.c index 2ccfd0c08ff0..60fbe6a8d850 100644 --- a/Zend/zend_compile.c +++ b/Zend/zend_compile.c @@ -8068,14 +8068,15 @@ static zend_string *zend_begin_method_decl(zend_op_array *op_array, zend_string } if ((fn_flags & ZEND_ACC_ABSTRACT) && - !(ce->ce_flags & (ZEND_ACC_EXPLICIT_ABSTRACT_CLASS|ZEND_ACC_TRAIT)) && - !in_interface + !(ce->ce_flags & (ZEND_ACC_EXPLICIT_ABSTRACT_CLASS|ZEND_ACC_TRAIT|ZEND_ACC_INTERFACE)) ) { // Don't say that the class should be declared abstract if it is - // anonymous and can't be abstract + // anonymous or an enum and can't be abstract const char *msg; if (ce->ce_flags & ZEND_ACC_ANON_CLASS) { msg = "Anonymous class %s cannot contain abstract method %s::%s"; + } else if (ce->ce_flags & ZEND_ACC_ENUM) { + msg = "Enum %s cannot contain abstract method %s::%s"; } else { msg = "Class %s contains abstract method %s::%s and must therefore be declared abstract"; } From 8dbb886426cb4216ddde8a49f2919f646b453091 Mon Sep 17 00:00:00 2001 From: Ilija Tovilo Date: Wed, 23 Oct 2024 11:23:15 +0200 Subject: [PATCH 5/5] Improve error messages --- Zend/tests/abstract_implicit.phpt | 2 +- Zend/tests/anon/gh16067.phpt | 2 +- Zend/tests/enum/no-abstract.phpt | 2 +- Zend/tests/errmsg/errmsg_018.phpt | 2 +- Zend/zend_compile.c | 30 +++++++-------------- tests/classes/abstract_derived.phpt | 2 +- tests/classes/abstract_not_declared.phpt | 2 +- tests/classes/abstract_redeclare.phpt | 2 +- tests/classes/abstract_static.phpt | 2 +- tests/classes/interface_method_private.phpt | 2 +- 10 files changed, 19 insertions(+), 29 deletions(-) diff --git a/Zend/tests/abstract_implicit.phpt b/Zend/tests/abstract_implicit.phpt index 5211074c743e..1ccd4d3653fc 100644 --- a/Zend/tests/abstract_implicit.phpt +++ b/Zend/tests/abstract_implicit.phpt @@ -16,4 +16,4 @@ class NotAbstract { } ?> --EXPECTF-- -Fatal error: Class NotAbstract contains abstract method NotAbstract::bar and must therefore be declared abstract in %s on line %d +Fatal error: Class NotAbstract declares abstract method bar() and must therefore be declared abstract in %s on line %d diff --git a/Zend/tests/anon/gh16067.phpt b/Zend/tests/anon/gh16067.phpt index 11bcbcc6f928..dc09d14964a5 100644 --- a/Zend/tests/anon/gh16067.phpt +++ b/Zend/tests/anon/gh16067.phpt @@ -8,4 +8,4 @@ $c = new class { } ?> --EXPECTF-- -Fatal error: Anonymous class class@anonymous cannot contain abstract method class@anonymous::f in %s on line 4 +Fatal error: Anonymous class method f() must not be abstract in %s on line 4 diff --git a/Zend/tests/enum/no-abstract.phpt b/Zend/tests/enum/no-abstract.phpt index a0b8e02c2ee9..2593d67e44de 100644 --- a/Zend/tests/enum/no-abstract.phpt +++ b/Zend/tests/enum/no-abstract.phpt @@ -9,4 +9,4 @@ enum Example { ?> --EXPECTF-- -Fatal error: Enum Example cannot contain abstract method Example::foo in %s on line 4 +Fatal error: Enum method Example::foo() must not be abstract in %s on line 4 diff --git a/Zend/tests/errmsg/errmsg_018.phpt b/Zend/tests/errmsg/errmsg_018.phpt index 752e70abb4d0..e94f99446ce1 100644 --- a/Zend/tests/errmsg/errmsg_018.phpt +++ b/Zend/tests/errmsg/errmsg_018.phpt @@ -10,4 +10,4 @@ class test { echo "Done\n"; ?> --EXPECTF-- -Fatal error: Class test contains abstract method test::foo and must therefore be declared abstract in %s on line %d +Fatal error: Class test declares abstract method foo() and must therefore be declared abstract in %s on line %d diff --git a/Zend/zend_compile.c b/Zend/zend_compile.c index 60fbe6a8d850..a0222b4cfd8b 100644 --- a/Zend/zend_compile.c +++ b/Zend/zend_compile.c @@ -8067,26 +8067,20 @@ static zend_string *zend_begin_method_decl(zend_op_array *op_array, zend_string zend_error(E_COMPILE_WARNING, "Private methods cannot be final as they are never overridden by other classes"); } - if ((fn_flags & ZEND_ACC_ABSTRACT) && - !(ce->ce_flags & (ZEND_ACC_EXPLICIT_ABSTRACT_CLASS|ZEND_ACC_TRAIT|ZEND_ACC_INTERFACE)) - ) { + if ((fn_flags & ZEND_ACC_ABSTRACT) + && !(ce->ce_flags & (ZEND_ACC_EXPLICIT_ABSTRACT_CLASS|ZEND_ACC_TRAIT))) { // Don't say that the class should be declared abstract if it is // anonymous or an enum and can't be abstract - const char *msg; if (ce->ce_flags & ZEND_ACC_ANON_CLASS) { - msg = "Anonymous class %s cannot contain abstract method %s::%s"; - } else if (ce->ce_flags & ZEND_ACC_ENUM) { - msg = "Enum %s cannot contain abstract method %s::%s"; + zend_error_noreturn(E_COMPILE_ERROR, "Anonymous class method %s() must not be abstract", + ZSTR_VAL(name)); + } else if (ce->ce_flags & (ZEND_ACC_ENUM|ZEND_ACC_INTERFACE)) { + zend_error_noreturn(E_COMPILE_ERROR, "%s method %s::%s() must not be abstract", + zend_get_object_type_case(ce, true), ZSTR_VAL(ce->name), ZSTR_VAL(name)); } else { - msg = "Class %s contains abstract method %s::%s and must therefore be declared abstract"; - } - zend_error_noreturn( - E_COMPILE_ERROR, - msg, - ZSTR_VAL(ce->name), - ZSTR_VAL(ce->name), - ZSTR_VAL(name) - ); + zend_error_noreturn(E_COMPILE_ERROR, "Class %s declares abstract method %s() and must therefore be declared abstract", + ZSTR_VAL(ce->name), ZSTR_VAL(name)); + } } if (in_interface) { @@ -8098,10 +8092,6 @@ static zend_string *zend_begin_method_decl(zend_op_array *op_array, zend_string zend_error_noreturn(E_COMPILE_ERROR, "Interface method " "%s::%s() must not be final", ZSTR_VAL(ce->name), ZSTR_VAL(name)); } - if (fn_flags & ZEND_ACC_ABSTRACT) { - zend_error_noreturn(E_COMPILE_ERROR, "Interface method " - "%s::%s() must not be abstract", ZSTR_VAL(ce->name), ZSTR_VAL(name)); - } op_array->fn_flags |= ZEND_ACC_ABSTRACT; } diff --git a/tests/classes/abstract_derived.phpt b/tests/classes/abstract_derived.phpt index 330f9cdcd664..7d0ffff5ca0b 100644 --- a/tests/classes/abstract_derived.phpt +++ b/tests/classes/abstract_derived.phpt @@ -13,4 +13,4 @@ class derived extends base { ?> ===DONE=== --EXPECTF-- -Fatal error: Class derived contains abstract method derived::show and must therefore be declared abstract in %sabstract_derived.php on line %d +Fatal error: Class derived declares abstract method show() and must therefore be declared abstract in %sabstract_derived.php on line %d diff --git a/tests/classes/abstract_not_declared.phpt b/tests/classes/abstract_not_declared.phpt index 592399c6d748..c9ba306a1102 100644 --- a/tests/classes/abstract_not_declared.phpt +++ b/tests/classes/abstract_not_declared.phpt @@ -10,4 +10,4 @@ class fail { echo "Done\n"; // shouldn't be displayed ?> --EXPECTF-- -Fatal error: Class fail contains abstract method fail::show and must therefore be declared abstract in %s on line %d +Fatal error: Class fail declares abstract method show() and must therefore be declared abstract in %s on line %d diff --git a/tests/classes/abstract_redeclare.phpt b/tests/classes/abstract_redeclare.phpt index dafa35d460eb..eb73da571ca4 100644 --- a/tests/classes/abstract_redeclare.phpt +++ b/tests/classes/abstract_redeclare.phpt @@ -16,4 +16,4 @@ class fail extends pass { echo "Done\n"; // Shouldn't be displayed ?> --EXPECTF-- -Fatal error: Class fail contains abstract method fail::show and must therefore be declared abstract in %sabstract_redeclare.php on line %d +Fatal error: Class fail declares abstract method show() and must therefore be declared abstract in %sabstract_redeclare.php on line %d diff --git a/tests/classes/abstract_static.phpt b/tests/classes/abstract_static.phpt index 97c4c910c191..15db2f9a03b9 100644 --- a/tests/classes/abstract_static.phpt +++ b/tests/classes/abstract_static.phpt @@ -31,4 +31,4 @@ echo "Done\n"; // shouldn't be displayed --EXPECTF-- Call to function show() -Fatal error: Class fail contains abstract method fail::func and must therefore be declared abstract in %sabstract_static.php(%d) : eval()'d code on line %d +Fatal error: Class fail declares abstract method func() and must therefore be declared abstract in %sabstract_static.php(%d) : eval()'d code on line %d diff --git a/tests/classes/interface_method_private.phpt b/tests/classes/interface_method_private.phpt index c7a0852ea4a1..d32e3dbdc8e1 100644 --- a/tests/classes/interface_method_private.phpt +++ b/tests/classes/interface_method_private.phpt @@ -4,7 +4,7 @@ ZE2 An interface method cannot be private