Skip to content

Commit

Permalink
Improve error message class type
Browse files Browse the repository at this point in the history
Refer to interfaces/enums instead of classes in more places.

Closes GH-7792
Closes GH-8187
  • Loading branch information
iluuu1994 committed Mar 20, 2022
1 parent b9b134d commit 733023b
Show file tree
Hide file tree
Showing 16 changed files with 108 additions and 19 deletions.
1 change: 1 addition & 0 deletions NEWS
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ PHP NEWS
- Core:
. Fixed bug #81380 (Observer may not be initialized properly). (krakjoe)
. Fixed bug GH-7771 (Fix filename/lineno of constant expressions). (ilutov)
. Fixed bug GH-7792 (Improve class type in error messages). (ilutov)

- Intl:
. Update all grandfathered language tags with preferred values
Expand Down
2 changes: 1 addition & 1 deletion Zend/tests/class_alias_009.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 1 addition & 1 deletion Zend/tests/constants/final_constants/final_const12.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 1 addition & 1 deletion Zend/tests/enum/no-enum-implements-backed-enum.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 1 addition & 1 deletion Zend/tests/enum/no-enum-implements-unit-enum.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -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
14 changes: 14 additions & 0 deletions Zend/tests/gh7792_1.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
--TEST--
GH-7792 (Refer to enum as enum instead of class)
--FILE--
<?php

interface A {
public function a(): void;
}

enum B implements A {}

?>
--EXPECTF--
Fatal error: Enum B must implement 1 abstract private method (A::a) in %s on line %d
10 changes: 10 additions & 0 deletions Zend/tests/gh7792_2.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
--TEST--
GH-7792 (Refer to enum as enum instead of class)
--FILE--
<?php

enum Foo implements Throwable {}

?>
--EXPECTF--
Fatal error: Enum Foo cannot implement interface Throwable in %s on line %d
18 changes: 18 additions & 0 deletions Zend/tests/gh7792_3.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
--TEST--
GH-7792 (Refer to enum as enum instead of class)
--FILE--
<?php

interface A {
const FOO = 'foo';
}

interface B {
const FOO = 'foo';
}

enum Foo implements A, B {}

?>
--EXPECTF--
Fatal error: Enum Foo inherits both A::FOO and B::FOO, which is ambiguous in %s on line %d
12 changes: 12 additions & 0 deletions Zend/tests/gh7792_4.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
--TEST--
GH-7792 (Refer to enum as enum instead of class)
--FILE--
<?php

interface A {}

enum Foo implements A, A {}

?>
--EXPECTF--
Fatal error: Enum Foo cannot implement previously implemented interface A in %s on line %d
10 changes: 10 additions & 0 deletions Zend/tests/gh7792_5.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
--TEST--
GH-7792 (Refer to enum as enum instead of class)
--FILE--
<?php

enum Foo implements Traversable {}

?>
--EXPECT--
Fatal error: Enum Foo must implement interface Traversable as part of either Iterator or IteratorAggregate in Unknown on line 0
2 changes: 1 addition & 1 deletion Zend/tests/objects_014.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -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
12 changes: 7 additions & 5 deletions Zend/zend_API.c
Original file line number Diff line number Diff line change
Expand Up @@ -4783,14 +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 upper_case ? "Enum" : "enum";
} else {
return "class";
return upper_case ? "Class" : "class";
}
}
/* }}} */
Expand Down
12 changes: 11 additions & 1 deletion Zend/zend_API.h
Original file line number Diff line number Diff line change
Expand Up @@ -721,7 +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_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);

Expand Down
7 changes: 6 additions & 1 deletion Zend/zend_exceptions.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
18 changes: 12 additions & 6 deletions Zend/zend_inheritance.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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 */
Expand Down Expand Up @@ -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) {
Expand All @@ -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),
Expand Down
3 changes: 2 additions & 1 deletion Zend/zend_interfaces.c
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down

0 comments on commit 733023b

Please sign in to comment.