From ea0a6a8cc7537ec59b485287dc069e8c904166e1 Mon Sep 17 00:00:00 2001 From: Ilija Tovilo Date: Thu, 10 Mar 2022 14:41:07 +0100 Subject: [PATCH 1/2] Improve error message class type Refer to interfaces/enums instead of classes in more places. Closes GH-7792 --- Zend/tests/class_alias_009.phpt | 2 +- .../final_constants/final_const12.phpt | 2 +- .../enum/no-enum-implements-backed-enum.phpt | 2 +- .../enum/no-enum-implements-unit-enum.phpt | 2 +- Zend/tests/gh7792_1.phpt | 14 ++++++++++++++ Zend/tests/gh7792_2.phpt | 10 ++++++++++ Zend/tests/gh7792_3.phpt | 18 ++++++++++++++++++ Zend/tests/gh7792_4.phpt | 12 ++++++++++++ Zend/tests/gh7792_5.phpt | 10 ++++++++++ Zend/tests/objects_014.phpt | 2 +- Zend/zend_API.c | 16 ++++++++++++++++ Zend/zend_API.h | 1 + Zend/zend_exceptions.c | 7 ++++++- Zend/zend_inheritance.c | 18 ++++++++++++------ Zend/zend_interfaces.c | 3 ++- 15 files changed, 106 insertions(+), 13 deletions(-) create mode 100644 Zend/tests/gh7792_1.phpt create mode 100644 Zend/tests/gh7792_2.phpt create mode 100644 Zend/tests/gh7792_3.phpt create mode 100644 Zend/tests/gh7792_4.phpt create mode 100644 Zend/tests/gh7792_5.phpt diff --git a/Zend/tests/class_alias_009.phpt b/Zend/tests/class_alias_009.phpt index f17769e1f4213..bb72312577aff 100644 --- a/Zend/tests/class_alias_009.phpt +++ b/Zend/tests/class_alias_009.phpt @@ -11,4 +11,4 @@ interface c extends a, b { } ?> --EXPECTF-- -Fatal error: Class c cannot implement previously implemented interface a in %s on line %d +Fatal error: Interface c cannot implement previously implemented interface a in %s on line %d diff --git a/Zend/tests/constants/final_constants/final_const12.phpt b/Zend/tests/constants/final_constants/final_const12.phpt index 300239f6a630e..3130af11da8bd 100644 --- a/Zend/tests/constants/final_constants/final_const12.phpt +++ b/Zend/tests/constants/final_constants/final_const12.phpt @@ -19,4 +19,4 @@ interface I3 extends I1, I2 ?> --EXPECTF-- -Fatal error: Class I3 inherits both I1::C and I2::C, which is ambiguous in %s on line %d +Fatal error: Interface I3 inherits both I1::C and I2::C, which is ambiguous in %s on line %d diff --git a/Zend/tests/enum/no-enum-implements-backed-enum.phpt b/Zend/tests/enum/no-enum-implements-backed-enum.phpt index 69922c13a8f51..c93994ad8bf58 100644 --- a/Zend/tests/enum/no-enum-implements-backed-enum.phpt +++ b/Zend/tests/enum/no-enum-implements-backed-enum.phpt @@ -7,4 +7,4 @@ enum Foo: int implements BackedEnum {} ?> --EXPECTF-- -Fatal error: Class Foo cannot implement previously implemented interface BackedEnum in %s on line %d +Fatal error: Enum Foo cannot implement previously implemented interface BackedEnum in %s on line %d diff --git a/Zend/tests/enum/no-enum-implements-unit-enum.phpt b/Zend/tests/enum/no-enum-implements-unit-enum.phpt index 458efbb67cdfa..ae1890c430518 100644 --- a/Zend/tests/enum/no-enum-implements-unit-enum.phpt +++ b/Zend/tests/enum/no-enum-implements-unit-enum.phpt @@ -7,4 +7,4 @@ enum Foo implements UnitEnum {} ?> --EXPECTF-- -Fatal error: Class Foo cannot implement previously implemented interface UnitEnum in %s on line %d +Fatal error: Enum Foo cannot implement previously implemented interface UnitEnum in %s on line %d diff --git a/Zend/tests/gh7792_1.phpt b/Zend/tests/gh7792_1.phpt new file mode 100644 index 0000000000000..a342f169d937c --- /dev/null +++ b/Zend/tests/gh7792_1.phpt @@ -0,0 +1,14 @@ +--TEST-- +GH-7792 (Refer to enum as enum instead of class) +--FILE-- + +--EXPECTF-- +Fatal error: Enum B must implement 1 abstract private method (A::a) in %s on line %d diff --git a/Zend/tests/gh7792_2.phpt b/Zend/tests/gh7792_2.phpt new file mode 100644 index 0000000000000..41f47a6e870ac --- /dev/null +++ b/Zend/tests/gh7792_2.phpt @@ -0,0 +1,10 @@ +--TEST-- +GH-7792 (Refer to enum as enum instead of class) +--FILE-- + +--EXPECTF-- +Fatal error: Enum Foo cannot implement interface Throwable in %s on line %d diff --git a/Zend/tests/gh7792_3.phpt b/Zend/tests/gh7792_3.phpt new file mode 100644 index 0000000000000..67e2c1c99dda1 --- /dev/null +++ b/Zend/tests/gh7792_3.phpt @@ -0,0 +1,18 @@ +--TEST-- +GH-7792 (Refer to enum as enum instead of class) +--FILE-- + +--EXPECTF-- +Fatal error: Enum Foo inherits both A::FOO and B::FOO, which is ambiguous in %s on line %d diff --git a/Zend/tests/gh7792_4.phpt b/Zend/tests/gh7792_4.phpt new file mode 100644 index 0000000000000..0d09835d7ee3a --- /dev/null +++ b/Zend/tests/gh7792_4.phpt @@ -0,0 +1,12 @@ +--TEST-- +GH-7792 (Refer to enum as enum instead of class) +--FILE-- + +--EXPECTF-- +Fatal error: Enum Foo cannot implement previously implemented interface A in %s on line %d diff --git a/Zend/tests/gh7792_5.phpt b/Zend/tests/gh7792_5.phpt new file mode 100644 index 0000000000000..824ed9ed1810d --- /dev/null +++ b/Zend/tests/gh7792_5.phpt @@ -0,0 +1,10 @@ +--TEST-- +GH-7792 (Refer to enum as enum instead of class) +--FILE-- + +--EXPECT-- +Fatal error: Enum Foo must implement interface Traversable as part of either Iterator or IteratorAggregate in Unknown on line 0 diff --git a/Zend/tests/objects_014.phpt b/Zend/tests/objects_014.phpt index 138fd4f294660..858de748b8e2e 100644 --- a/Zend/tests/objects_014.phpt +++ b/Zend/tests/objects_014.phpt @@ -12,4 +12,4 @@ interface bar extends foo, foo { echo "Done\n"; ?> --EXPECTF-- -Fatal error: Class bar cannot implement previously implemented interface foo in %s on line %d +Fatal error: Interface bar cannot implement previously implemented interface foo in %s on line %d diff --git a/Zend/zend_API.c b/Zend/zend_API.c index 3ef291c315596..fcd9db169578a 100644 --- a/Zend/zend_API.c +++ b/Zend/zend_API.c @@ -4789,12 +4789,28 @@ ZEND_API ZEND_COLD const char *zend_get_object_type(const zend_class_entry *ce) return "trait"; } else if (ce->ce_flags & ZEND_ACC_INTERFACE) { return "interface"; + } else if (ce->ce_flags & ZEND_ACC_ENUM) { + return "enum"; } else { return "class"; } } /* }}} */ +ZEND_API ZEND_COLD const char *zend_get_object_type_uc(const zend_class_entry *ce) /* {{{ */ +{ + if(ce->ce_flags & ZEND_ACC_TRAIT) { + return "Trait"; + } else if (ce->ce_flags & ZEND_ACC_INTERFACE) { + return "Interface"; + } else if (ce->ce_flags & ZEND_ACC_ENUM) { + return "Enum"; + } else { + return "Class"; + } +} +/* }}} */ + ZEND_API bool zend_is_iterable(zval *iterable) /* {{{ */ { switch (Z_TYPE_P(iterable)) { diff --git a/Zend/zend_API.h b/Zend/zend_API.h index ecb6e8296befb..37f58d30a3f45 100644 --- a/Zend/zend_API.h +++ b/Zend/zend_API.h @@ -722,6 +722,7 @@ static zend_always_inline zend_result zend_forbid_dynamic_call(void) } ZEND_API ZEND_COLD const char *zend_get_object_type(const zend_class_entry *ce); +ZEND_API ZEND_COLD const char *zend_get_object_type_uc(const zend_class_entry *ce); ZEND_API bool zend_is_iterable(zval *iterable); diff --git a/Zend/zend_exceptions.c b/Zend/zend_exceptions.c index 7138713510120..85e21c64a037b 100644 --- a/Zend/zend_exceptions.c +++ b/Zend/zend_exceptions.c @@ -67,8 +67,13 @@ static int zend_implement_throwable(zend_class_entry *interface, zend_class_entr return SUCCESS; } + bool can_extend = (class_type->ce_flags & ZEND_ACC_ENUM) == 0; + zend_error_noreturn(E_ERROR, - "Class %s cannot implement interface %s, extend Exception or Error instead", + can_extend + ? "%s %s cannot implement interface %s, extend Exception or Error instead" + : "%s %s cannot implement interface %s", + zend_get_object_type_uc(class_type), ZSTR_VAL(class_type->name), ZSTR_VAL(interface->name)); return FAILURE; diff --git a/Zend/zend_inheritance.c b/Zend/zend_inheritance.c index 82af3d8afa75a..2709fb5639186 100644 --- a/Zend/zend_inheritance.c +++ b/Zend/zend_inheritance.c @@ -1285,7 +1285,7 @@ static void do_inherit_property(zend_property_info *parent_info, zend_string *ke static inline void do_implement_interface(zend_class_entry *ce, zend_class_entry *iface) /* {{{ */ { if (!(ce->ce_flags & ZEND_ACC_INTERFACE) && iface->interface_gets_implemented && iface->interface_gets_implemented(iface, ce) == FAILURE) { - zend_error_noreturn(E_CORE_ERROR, "Class %s could not implement interface %s", ZSTR_VAL(ce->name), ZSTR_VAL(iface->name)); + zend_error_noreturn(E_CORE_ERROR, "%s %s could not implement interface %s", zend_get_object_type_uc(ce), ZSTR_VAL(ce->name), ZSTR_VAL(iface->name)); } /* This should be prevented by the class lookup logic. */ ZEND_ASSERT(ce != iface); @@ -1610,7 +1610,8 @@ static bool do_inherit_constant_check( if (old_constant->ce != parent_constant->ce && old_constant->ce != ce) { zend_error_noreturn(E_COMPILE_ERROR, - "Class %s inherits both %s::%s and %s::%s, which is ambiguous", + "%s %s inherits both %s::%s and %s::%s, which is ambiguous", + zend_get_object_type_uc(ce), ZSTR_VAL(ce->name), ZSTR_VAL(old_constant->ce->name), ZSTR_VAL(name), ZSTR_VAL(parent_constant->ce->name), ZSTR_VAL(name)); @@ -1729,7 +1730,10 @@ static void zend_do_implement_interfaces(zend_class_entry *ce, zend_class_entry if (interfaces[j] == iface) { if (j >= num_parent_interfaces) { efree(interfaces); - zend_error_noreturn(E_COMPILE_ERROR, "Class %s cannot implement previously implemented interface %s", ZSTR_VAL(ce->name), ZSTR_VAL(iface->name)); + zend_error_noreturn(E_COMPILE_ERROR, "%s %s cannot implement previously implemented interface %s", + zend_get_object_type_uc(ce), + ZSTR_VAL(ce->name), + ZSTR_VAL(iface->name)); return; } /* skip duplications */ @@ -2311,6 +2315,7 @@ void zend_verify_abstract_class(zend_class_entry *ce) /* {{{ */ zend_function *func; zend_abstract_info ai; bool is_explicit_abstract = (ce->ce_flags & ZEND_ACC_EXPLICIT_ABSTRACT_CLASS) != 0; + bool can_be_abstract = (ce->ce_flags & ZEND_ACC_ENUM) == 0; memset(&ai, 0, sizeof(ai)); ZEND_HASH_MAP_FOREACH_PTR(&ce->function_table, func) { @@ -2324,9 +2329,10 @@ void zend_verify_abstract_class(zend_class_entry *ce) /* {{{ */ } ZEND_HASH_FOREACH_END(); if (ai.cnt) { - zend_error_noreturn(E_ERROR, !is_explicit_abstract - ? "Class %s contains %d abstract method%s and must therefore be declared abstract or implement the remaining methods (" MAX_ABSTRACT_INFO_FMT MAX_ABSTRACT_INFO_FMT MAX_ABSTRACT_INFO_FMT ")" - : "Class %s must implement %d abstract private method%s (" MAX_ABSTRACT_INFO_FMT MAX_ABSTRACT_INFO_FMT MAX_ABSTRACT_INFO_FMT ")", + zend_error_noreturn(E_ERROR, !is_explicit_abstract && can_be_abstract + ? "%s %s contains %d abstract method%s and must therefore be declared abstract or implement the remaining methods (" MAX_ABSTRACT_INFO_FMT MAX_ABSTRACT_INFO_FMT MAX_ABSTRACT_INFO_FMT ")" + : "%s %s must implement %d abstract private method%s (" MAX_ABSTRACT_INFO_FMT MAX_ABSTRACT_INFO_FMT MAX_ABSTRACT_INFO_FMT ")", + zend_get_object_type_uc(ce), ZSTR_VAL(ce->name), ai.cnt, ai.cnt > 1 ? "s" : "", DISPLAY_ABSTRACT_FN(0), diff --git a/Zend/zend_interfaces.c b/Zend/zend_interfaces.c index 3defb97d2ec0e..1fa2493f17af5 100644 --- a/Zend/zend_interfaces.c +++ b/Zend/zend_interfaces.c @@ -266,7 +266,8 @@ static int zend_implement_traversable(zend_class_entry *interface, zend_class_en } } } - zend_error_noreturn(E_CORE_ERROR, "Class %s must implement interface %s as part of either %s or %s", + zend_error_noreturn(E_CORE_ERROR, "%s %s must implement interface %s as part of either %s or %s", + zend_get_object_type_uc(class_type), ZSTR_VAL(class_type->name), ZSTR_VAL(zend_ce_traversable->name), ZSTR_VAL(zend_ce_iterator->name), From a7db5a51b9775a211ee5cb6a556fe8d471ce3f74 Mon Sep 17 00:00:00 2001 From: Ilija Tovilo Date: Fri, 18 Mar 2022 10:25:57 +0100 Subject: [PATCH 2/2] Combine get_object_type uc/lc functions --- Zend/zend_API.c | 26 ++++++-------------------- Zend/zend_API.h | 13 +++++++++++-- 2 files changed, 17 insertions(+), 22 deletions(-) diff --git a/Zend/zend_API.c b/Zend/zend_API.c index fcd9db169578a..6ce9f593e17dd 100644 --- a/Zend/zend_API.c +++ b/Zend/zend_API.c @@ -4783,30 +4783,16 @@ ZEND_API void zend_restore_error_handling(zend_error_handling *saved) /* {{{ */ } /* }}} */ -ZEND_API ZEND_COLD const char *zend_get_object_type(const zend_class_entry *ce) /* {{{ */ +ZEND_API ZEND_COLD const char *zend_get_object_type_case(const zend_class_entry *ce, bool upper_case) /* {{{ */ { - if(ce->ce_flags & ZEND_ACC_TRAIT) { - return "trait"; + if (ce->ce_flags & ZEND_ACC_TRAIT) { + return upper_case ? "Trait" : "trait"; } else if (ce->ce_flags & ZEND_ACC_INTERFACE) { - return "interface"; + return upper_case ? "Interface" : "interface"; } else if (ce->ce_flags & ZEND_ACC_ENUM) { - return "enum"; + return upper_case ? "Enum" : "enum"; } else { - return "class"; - } -} -/* }}} */ - -ZEND_API ZEND_COLD const char *zend_get_object_type_uc(const zend_class_entry *ce) /* {{{ */ -{ - if(ce->ce_flags & ZEND_ACC_TRAIT) { - return "Trait"; - } else if (ce->ce_flags & ZEND_ACC_INTERFACE) { - return "Interface"; - } else if (ce->ce_flags & ZEND_ACC_ENUM) { - return "Enum"; - } else { - return "Class"; + return upper_case ? "Class" : "class"; } } /* }}} */ diff --git a/Zend/zend_API.h b/Zend/zend_API.h index 37f58d30a3f45..705fe25e7665a 100644 --- a/Zend/zend_API.h +++ b/Zend/zend_API.h @@ -721,8 +721,17 @@ static zend_always_inline zend_result zend_forbid_dynamic_call(void) return SUCCESS; } -ZEND_API ZEND_COLD const char *zend_get_object_type(const zend_class_entry *ce); -ZEND_API ZEND_COLD const char *zend_get_object_type_uc(const zend_class_entry *ce); +ZEND_API ZEND_COLD const char *zend_get_object_type_case(const zend_class_entry *ce, bool upper_case); + +static zend_always_inline const char *zend_get_object_type(const zend_class_entry *ce) +{ + return zend_get_object_type_case(ce, false); +} + +static zend_always_inline const char *zend_get_object_type_uc(const zend_class_entry *ce) +{ + return zend_get_object_type_case(ce, true); +} ZEND_API bool zend_is_iterable(zval *iterable);