From 9a6468e098e1889853134965dc34395c82fd7156 Mon Sep 17 00:00:00 2001 From: NickSdot Date: Sun, 5 Jul 2026 14:00:38 +0700 Subject: [PATCH 1/2] Improved readonly errors Ref: https://github.com/php/php-src/pull/22588#discussion_r3523422058 --- .../abstract_get_set_readonly_public.phpt | 13 ++++++++++ .../interface_get_set_readonly.phpt | 15 ----------- ....phpt => interface_readonly_get_only.phpt} | 2 +- ...terface_readonly_get_set_implicit_set.phpt | 13 ++++++++++ ...interface_readonly_get_set_public_set.phpt | 13 ++++++++++ Zend/zend_inheritance.c | 25 +++++++++++++++++++ 6 files changed, 65 insertions(+), 16 deletions(-) create mode 100644 Zend/tests/property_hooks/abstract_get_set_readonly_public.phpt delete mode 100644 Zend/tests/property_hooks/interface_get_set_readonly.phpt rename Zend/tests/property_hooks/{interface_get_only_readonly.phpt => interface_readonly_get_only.phpt} (79%) create mode 100644 Zend/tests/property_hooks/interface_readonly_get_set_implicit_set.phpt create mode 100644 Zend/tests/property_hooks/interface_readonly_get_set_public_set.phpt diff --git a/Zend/tests/property_hooks/abstract_get_set_readonly_public.phpt b/Zend/tests/property_hooks/abstract_get_set_readonly_public.phpt new file mode 100644 index 000000000000..37d20a3989df --- /dev/null +++ b/Zend/tests/property_hooks/abstract_get_set_readonly_public.phpt @@ -0,0 +1,13 @@ +--TEST-- +Public readonly property has restricted set visibility for public get/set abstract property +--FILE-- + +--EXPECTF-- +Fatal error: Set access level of readonly property C::$prop is protected(set), but must be public(set) (as in class P) in %s on line %d diff --git a/Zend/tests/property_hooks/interface_get_set_readonly.phpt b/Zend/tests/property_hooks/interface_get_set_readonly.phpt deleted file mode 100644 index 692a0bc7e787..000000000000 --- a/Zend/tests/property_hooks/interface_get_set_readonly.phpt +++ /dev/null @@ -1,15 +0,0 @@ ---TEST-- -readonly property does not satisfy get/set interface property ---DESCRIPTION-- -The error message should be improved, the set access level comes from readonly. ---FILE-- - ---EXPECTF-- -Fatal error: Set access level of C::$prop must be omitted (as in class I) in %s on line %d diff --git a/Zend/tests/property_hooks/interface_get_only_readonly.phpt b/Zend/tests/property_hooks/interface_readonly_get_only.phpt similarity index 79% rename from Zend/tests/property_hooks/interface_get_only_readonly.phpt rename to Zend/tests/property_hooks/interface_readonly_get_only.phpt index 38ac375a4957..8004048bd80d 100644 --- a/Zend/tests/property_hooks/interface_get_only_readonly.phpt +++ b/Zend/tests/property_hooks/interface_readonly_get_only.phpt @@ -1,5 +1,5 @@ --TEST-- -readonly property satisfies get only interface property +Readonly property satisfies get-only interface property --FILE-- +--EXPECTF-- +Fatal error: Set access level of readonly property C::$prop is protected(set), but must be public(set) (as in interface I) in %s on line %d diff --git a/Zend/tests/property_hooks/interface_readonly_get_set_public_set.phpt b/Zend/tests/property_hooks/interface_readonly_get_set_public_set.phpt new file mode 100644 index 000000000000..6ac6ea75c671 --- /dev/null +++ b/Zend/tests/property_hooks/interface_readonly_get_set_public_set.phpt @@ -0,0 +1,13 @@ +--TEST-- +Readonly public(set) property does not implement set hook of get/set interface property +--FILE-- + +--EXPECTF-- +Fatal error: Class C contains 1 abstract method and must therefore be declared abstract or implement the remaining method (I::$prop::set) in %s on line %d diff --git a/Zend/zend_inheritance.c b/Zend/zend_inheritance.c index d70d08f5f1c4..11e4bb26894e 100644 --- a/Zend/zend_inheritance.c +++ b/Zend/zend_inheritance.c @@ -224,6 +224,20 @@ static const char *zend_asymmetric_visibility_string(uint32_t fn_flags) /* {{{ * } } +static const char *zend_set_visibility_string(uint32_t set_visibility) /* {{{ */ +{ + switch (set_visibility) { + case ZEND_ACC_PUBLIC_SET: + return "public(set)"; + case ZEND_ACC_PROTECTED_SET: + return "protected(set)"; + case ZEND_ACC_PRIVATE_SET: + return "private(set)"; + default: + ZEND_UNREACHABLE(); + } +} + static zend_string *resolve_class_name(const zend_class_entry *scope, zend_string *name) { ZEND_ASSERT(scope); if (zend_string_equals_ci(name, ZSTR_KNOWN(ZEND_STR_PARENT)) && scope->parent) { @@ -1486,6 +1500,17 @@ static void do_inherit_property(zend_property_info *parent_info, zend_string *ke } uint32_t child_set_visibility = child_info->flags & ZEND_ACC_PPP_SET_MASK; if (child_set_visibility > parent_set_visibility) { + if (child_info->flags & ZEND_ACC_READONLY) { + zend_error_noreturn( + E_COMPILE_ERROR, + "Set access level of readonly property %s::$%s is %s, but must be %s (as in %s %s)%s", + ZSTR_VAL(ce->name), ZSTR_VAL(key), + zend_set_visibility_string(child_set_visibility), + zend_set_visibility_string(parent_set_visibility), + zend_get_object_type_case(parent_info->ce, false), + ZSTR_VAL(parent_info->ce->name), + !(parent_info->flags & ZEND_ACC_PPP_SET_MASK) ? "" : " or weaker"); + } zend_error_noreturn( E_COMPILE_ERROR, "Set access level of %s::$%s must be %s (as in class %s)%s", From 5eddf0d405e15633bca10ed806720c296e66acf3 Mon Sep 17 00:00:00 2001 From: NickSdot Date: Sun, 5 Jul 2026 18:37:03 +0700 Subject: [PATCH 2/2] Alternative readonly error improvements --- .../abstract_get_only_readonly.phpt | 18 +++++++++ .../abstract_get_set_readonly.phpt | 13 ++++--- .../abstract_get_set_readonly_promoted.phpt | 13 +++++++ .../abstract_get_set_readonly_public.phpt | 13 ++++--- ...ract_get_set_readonly_public_promoted.phpt | 13 +++++++ .../abstract_get_set_readonly_public_set.phpt | 16 ++++++++ ..._get_set_readonly_public_set_promoted.phpt | 13 +++++++ .../interface_readonly_get_set.phpt | 16 ++++++++ ...terface_readonly_get_set_implicit_set.phpt | 13 ------- .../interface_readonly_get_set_promoted.phpt | 13 +++++++ ...interface_readonly_get_set_public_set.phpt | 9 +++-- ..._readonly_get_set_public_set_promoted.phpt | 13 +++++++ Zend/zend_inheritance.c | 39 +++++++------------ 13 files changed, 151 insertions(+), 51 deletions(-) create mode 100644 Zend/tests/property_hooks/abstract_get_only_readonly.phpt create mode 100644 Zend/tests/property_hooks/abstract_get_set_readonly_promoted.phpt create mode 100644 Zend/tests/property_hooks/abstract_get_set_readonly_public_promoted.phpt create mode 100644 Zend/tests/property_hooks/abstract_get_set_readonly_public_set.phpt create mode 100644 Zend/tests/property_hooks/abstract_get_set_readonly_public_set_promoted.phpt create mode 100644 Zend/tests/property_hooks/interface_readonly_get_set.phpt delete mode 100644 Zend/tests/property_hooks/interface_readonly_get_set_implicit_set.phpt create mode 100644 Zend/tests/property_hooks/interface_readonly_get_set_promoted.phpt create mode 100644 Zend/tests/property_hooks/interface_readonly_get_set_public_set_promoted.phpt diff --git a/Zend/tests/property_hooks/abstract_get_only_readonly.phpt b/Zend/tests/property_hooks/abstract_get_only_readonly.phpt new file mode 100644 index 000000000000..b911e8f7d861 --- /dev/null +++ b/Zend/tests/property_hooks/abstract_get_only_readonly.phpt @@ -0,0 +1,18 @@ +--TEST-- +Readonly property satisfies get-only abstract property +--FILE-- +prop = $prop; + } +} +$c = new C(42); +var_dump($c->prop); +?> +--EXPECT-- +int(42) diff --git a/Zend/tests/property_hooks/abstract_get_set_readonly.phpt b/Zend/tests/property_hooks/abstract_get_set_readonly.phpt index e8cb95019cc0..d82579379e16 100644 --- a/Zend/tests/property_hooks/abstract_get_set_readonly.phpt +++ b/Zend/tests/property_hooks/abstract_get_set_readonly.phpt @@ -1,13 +1,16 @@ --TEST-- -readonly property does not satisfy get/set abstract property +Readonly property cannot implement set hook of get/set abstract property --FILE-- prop = $prop; + } } ?> --EXPECTF-- -Fatal error: Class C contains 1 abstract method and must therefore be declared abstract or implement the remaining method (P::$prop::set) in %s on line %d +Fatal error: Readonly property C::$prop cannot implement set hook required by class A::$prop in %s on line %d diff --git a/Zend/tests/property_hooks/abstract_get_set_readonly_promoted.phpt b/Zend/tests/property_hooks/abstract_get_set_readonly_promoted.phpt new file mode 100644 index 000000000000..ff4070e9413d --- /dev/null +++ b/Zend/tests/property_hooks/abstract_get_set_readonly_promoted.phpt @@ -0,0 +1,13 @@ +--TEST-- +Readonly promoted property cannot implement set hook of get/set abstract property +--FILE-- + +--EXPECTF-- +Fatal error: Readonly property C::$prop cannot implement set hook required by class A::$prop in %s on line %d diff --git a/Zend/tests/property_hooks/abstract_get_set_readonly_public.phpt b/Zend/tests/property_hooks/abstract_get_set_readonly_public.phpt index 37d20a3989df..e9b976ff787a 100644 --- a/Zend/tests/property_hooks/abstract_get_set_readonly_public.phpt +++ b/Zend/tests/property_hooks/abstract_get_set_readonly_public.phpt @@ -1,13 +1,16 @@ --TEST-- -Public readonly property has restricted set visibility for public get/set abstract property +Readonly property cannot implement set hook of public get/set abstract property --FILE-- prop = $prop; + } } ?> --EXPECTF-- -Fatal error: Set access level of readonly property C::$prop is protected(set), but must be public(set) (as in class P) in %s on line %d +Fatal error: Readonly property C::$prop cannot implement set hook required by class A::$prop in %s on line %d diff --git a/Zend/tests/property_hooks/abstract_get_set_readonly_public_promoted.phpt b/Zend/tests/property_hooks/abstract_get_set_readonly_public_promoted.phpt new file mode 100644 index 000000000000..3754f4b4afa7 --- /dev/null +++ b/Zend/tests/property_hooks/abstract_get_set_readonly_public_promoted.phpt @@ -0,0 +1,13 @@ +--TEST-- +Readonly promoted property cannot implement set hook of public get/set abstract property +--FILE-- + +--EXPECTF-- +Fatal error: Readonly property C::$prop cannot implement set hook required by class A::$prop in %s on line %d diff --git a/Zend/tests/property_hooks/abstract_get_set_readonly_public_set.phpt b/Zend/tests/property_hooks/abstract_get_set_readonly_public_set.phpt new file mode 100644 index 000000000000..9b200c4869b4 --- /dev/null +++ b/Zend/tests/property_hooks/abstract_get_set_readonly_public_set.phpt @@ -0,0 +1,16 @@ +--TEST-- +Readonly public(set) property cannot implement set hook of public get/set abstract property +--FILE-- +prop = $prop; + } +} +?> +--EXPECTF-- +Fatal error: Readonly property C::$prop cannot implement set hook required by class A::$prop in %s on line %d diff --git a/Zend/tests/property_hooks/abstract_get_set_readonly_public_set_promoted.phpt b/Zend/tests/property_hooks/abstract_get_set_readonly_public_set_promoted.phpt new file mode 100644 index 000000000000..26d9f6891634 --- /dev/null +++ b/Zend/tests/property_hooks/abstract_get_set_readonly_public_set_promoted.phpt @@ -0,0 +1,13 @@ +--TEST-- +Readonly promoted public(set) property cannot implement set hook of public get/set abstract property +--FILE-- + +--EXPECTF-- +Fatal error: Readonly property C::$prop cannot implement set hook required by class A::$prop in %s on line %d diff --git a/Zend/tests/property_hooks/interface_readonly_get_set.phpt b/Zend/tests/property_hooks/interface_readonly_get_set.phpt new file mode 100644 index 000000000000..f90d70f5da53 --- /dev/null +++ b/Zend/tests/property_hooks/interface_readonly_get_set.phpt @@ -0,0 +1,16 @@ +--TEST-- +Readonly property cannot implement set hook of get/set interface property +--FILE-- +prop = $prop; + } +} +?> +--EXPECTF-- +Fatal error: Readonly property C::$prop cannot implement set hook required by interface I::$prop in %s on line %d diff --git a/Zend/tests/property_hooks/interface_readonly_get_set_implicit_set.phpt b/Zend/tests/property_hooks/interface_readonly_get_set_implicit_set.phpt deleted file mode 100644 index 8e8db454ab7a..000000000000 --- a/Zend/tests/property_hooks/interface_readonly_get_set_implicit_set.phpt +++ /dev/null @@ -1,13 +0,0 @@ ---TEST-- -Readonly property has restricted set visibility for get/set interface property ---FILE-- - ---EXPECTF-- -Fatal error: Set access level of readonly property C::$prop is protected(set), but must be public(set) (as in interface I) in %s on line %d diff --git a/Zend/tests/property_hooks/interface_readonly_get_set_promoted.phpt b/Zend/tests/property_hooks/interface_readonly_get_set_promoted.phpt new file mode 100644 index 000000000000..023036fefda9 --- /dev/null +++ b/Zend/tests/property_hooks/interface_readonly_get_set_promoted.phpt @@ -0,0 +1,13 @@ +--TEST-- +Readonly promoted property cannot implement set hook of get/set interface property +--FILE-- + +--EXPECTF-- +Fatal error: Readonly property C::$prop cannot implement set hook required by interface I::$prop in %s on line %d diff --git a/Zend/tests/property_hooks/interface_readonly_get_set_public_set.phpt b/Zend/tests/property_hooks/interface_readonly_get_set_public_set.phpt index 6ac6ea75c671..3005e4976b76 100644 --- a/Zend/tests/property_hooks/interface_readonly_get_set_public_set.phpt +++ b/Zend/tests/property_hooks/interface_readonly_get_set_public_set.phpt @@ -1,13 +1,16 @@ --TEST-- -Readonly public(set) property does not implement set hook of get/set interface property +Readonly public(set) property cannot implement set hook of get/set interface property --FILE-- prop = $prop; + } } ?> --EXPECTF-- -Fatal error: Class C contains 1 abstract method and must therefore be declared abstract or implement the remaining method (I::$prop::set) in %s on line %d +Fatal error: Readonly property C::$prop cannot implement set hook required by interface I::$prop in %s on line %d diff --git a/Zend/tests/property_hooks/interface_readonly_get_set_public_set_promoted.phpt b/Zend/tests/property_hooks/interface_readonly_get_set_public_set_promoted.phpt new file mode 100644 index 000000000000..5f047290ac80 --- /dev/null +++ b/Zend/tests/property_hooks/interface_readonly_get_set_public_set_promoted.phpt @@ -0,0 +1,13 @@ +--TEST-- +Readonly promoted public(set) property cannot implement set hook of get/set interface property +--FILE-- + +--EXPECTF-- +Fatal error: Readonly property C::$prop cannot implement set hook required by interface I::$prop in %s on line %d diff --git a/Zend/zend_inheritance.c b/Zend/zend_inheritance.c index 11e4bb26894e..5fe9a731091b 100644 --- a/Zend/zend_inheritance.c +++ b/Zend/zend_inheritance.c @@ -224,20 +224,6 @@ static const char *zend_asymmetric_visibility_string(uint32_t fn_flags) /* {{{ * } } -static const char *zend_set_visibility_string(uint32_t set_visibility) /* {{{ */ -{ - switch (set_visibility) { - case ZEND_ACC_PUBLIC_SET: - return "public(set)"; - case ZEND_ACC_PROTECTED_SET: - return "protected(set)"; - case ZEND_ACC_PRIVATE_SET: - return "private(set)"; - default: - ZEND_UNREACHABLE(); - } -} - static zend_string *resolve_class_name(const zend_class_entry *scope, zend_string *name) { ZEND_ASSERT(scope); if (zend_string_equals_ci(name, ZSTR_KNOWN(ZEND_STR_PARENT)) && scope->parent) { @@ -1489,6 +1475,20 @@ static void do_inherit_property(zend_property_info *parent_info, zend_string *ke ZSTR_VAL(ce->name), ZSTR_VAL(key)); } } + if (parent_info->hooks + && parent_info->hooks[ZEND_PROPERTY_HOOK_SET] + && parent_info->hooks[ZEND_PROPERTY_HOOK_SET]->common.fn_flags & ZEND_ACC_ABSTRACT + && child_info->flags & ZEND_ACC_READONLY + && !property_has_operation(child_info, ZEND_PROPERTY_HOOK_SET)) { + zend_error_noreturn( + E_COMPILE_ERROR, + "Readonly property %s::$%s cannot implement set hook required by %s %s::$%s", + ZSTR_VAL(ce->name), + ZSTR_VAL(key), + zend_get_object_type_case(parent_info->ce, false), + ZSTR_VAL(parent_info->ce->name), + ZSTR_VAL(key)); + } if (UNEXPECTED((child_info->flags & ZEND_ACC_PPP_SET_MASK)) /* Get-only virtual properties have no set visibility, so any child visibility is fine. */ && !(parent_info->hooks && (parent_info->flags & ZEND_ACC_VIRTUAL) && !parent_info->hooks[ZEND_PROPERTY_HOOK_SET])) { @@ -1500,17 +1500,6 @@ static void do_inherit_property(zend_property_info *parent_info, zend_string *ke } uint32_t child_set_visibility = child_info->flags & ZEND_ACC_PPP_SET_MASK; if (child_set_visibility > parent_set_visibility) { - if (child_info->flags & ZEND_ACC_READONLY) { - zend_error_noreturn( - E_COMPILE_ERROR, - "Set access level of readonly property %s::$%s is %s, but must be %s (as in %s %s)%s", - ZSTR_VAL(ce->name), ZSTR_VAL(key), - zend_set_visibility_string(child_set_visibility), - zend_set_visibility_string(parent_set_visibility), - zend_get_object_type_case(parent_info->ce, false), - ZSTR_VAL(parent_info->ce->name), - !(parent_info->flags & ZEND_ACC_PPP_SET_MASK) ? "" : " or weaker"); - } zend_error_noreturn( E_COMPILE_ERROR, "Set access level of %s::$%s must be %s (as in class %s)%s",