From a687a7de686c30171d812a635292c3d964f5c458 Mon Sep 17 00:00:00 2001 From: Daniel Scherzer Date: Mon, 9 Jun 2025 13:24:57 -0700 Subject: [PATCH 01/23] Add `#[\DelayedTargetValidation]` attribute --- .../has_runtime_errors.phpt | 225 ++++++++++++++++++ .../no_compile_errors.phpt | 46 ++++ .../repetition_errors.phpt | 13 + Zend/zend_attributes.c | 41 ++-- Zend/zend_attributes.h | 5 + Zend/zend_attributes.stub.php | 6 + Zend/zend_attributes_arginfo.h | 16 +- Zend/zend_compile.c | 23 +- ext/reflection/php_reflection.c | 46 ++-- 9 files changed, 380 insertions(+), 41 deletions(-) create mode 100644 Zend/tests/attributes/delayed_target_validation/has_runtime_errors.phpt create mode 100644 Zend/tests/attributes/delayed_target_validation/no_compile_errors.phpt create mode 100644 Zend/tests/attributes/delayed_target_validation/repetition_errors.phpt diff --git a/Zend/tests/attributes/delayed_target_validation/has_runtime_errors.phpt b/Zend/tests/attributes/delayed_target_validation/has_runtime_errors.phpt new file mode 100644 index 0000000000000..0d280bd446159 --- /dev/null +++ b/Zend/tests/attributes/delayed_target_validation/has_runtime_errors.phpt @@ -0,0 +1,225 @@ +--TEST-- +#[\DelayedTargetValidation] prevents target errors at compile time +--FILE-- +v = $v2; + echo __METHOD__ . "\n"; + } +} + +#[DelayedTargetValidation] +#[Attribute] +function demoFn() { + echo __FUNCTION__ . "\n"; +} + +#[DelayedTargetValidation] +#[Attribute] +const EXAMPLE = true; + +$cases = [ + new ReflectionClass('Demo'), + new ReflectionClassConstant('Demo', 'FOO'), + new ReflectionProperty('Demo', 'v'), + new ReflectionMethod('Demo', '__construct'), + new ReflectionParameter([ 'Demo', '__construct' ], 'v2'), + new ReflectionProperty('Demo', 'v2'), + new ReflectionFunction('demoFn'), + new ReflectionConstant('EXAMPLE'), +]; +foreach ($cases as $r) { + echo str_repeat("*", 20) . "\n"; + echo $r . "\n"; + $attributes = $r->getAttributes(); + var_dump($attributes); + try { + $attributes[1]->newInstance(); + } catch (Error $e) { + echo get_class($e) . ": " . $e->getMessage() . "\n"; + } +} + +?> +--EXPECTF-- +******************** +Class [ class Demo ] { + @@ %s %d-%d + + - Constants [1] { + Constant [ public string FOO ] { BAR } + } + + - Static properties [0] { + } + + - Static methods [0] { + } + + - Properties [2] { + Property [ public string $v ] + Property [ public string $v2 ] + } + + - Methods [1] { + Method [ public method __construct ] { + @@ %s %d - %d + + - Parameters [1] { + Parameter #0 [ string $v2 ] + } + } + } +} + +array(2) { + [0]=> + object(ReflectionAttribute)#%d (1) { + ["name"]=> + string(23) "DelayedTargetValidation" + } + [1]=> + object(ReflectionAttribute)#%d (1) { + ["name"]=> + string(9) "NoDiscard" + } +} +Error: Attribute "NoDiscard" cannot target class (allowed targets: function, method) +******************** +Constant [ public string FOO ] { BAR } + +array(2) { + [0]=> + object(ReflectionAttribute)#%d (1) { + ["name"]=> + string(23) "DelayedTargetValidation" + } + [1]=> + object(ReflectionAttribute)#%d (1) { + ["name"]=> + string(9) "Attribute" + } +} +Error: Attribute "Attribute" cannot target class constant (allowed targets: class) +******************** +Property [ public string $v ] + +array(2) { + [0]=> + object(ReflectionAttribute)#%d (1) { + ["name"]=> + string(23) "DelayedTargetValidation" + } + [1]=> + object(ReflectionAttribute)#%d (1) { + ["name"]=> + string(9) "Attribute" + } +} +Error: Attribute "Attribute" cannot target property (allowed targets: class) +******************** +Method [ public method __construct ] { + @@ %s %d - %d + + - Parameters [1] { + Parameter #0 [ string $v2 ] + } +} + +array(2) { + [0]=> + object(ReflectionAttribute)#%d (1) { + ["name"]=> + string(23) "DelayedTargetValidation" + } + [1]=> + object(ReflectionAttribute)#%d (1) { + ["name"]=> + string(9) "Attribute" + } +} +Error: Attribute "Attribute" cannot target method (allowed targets: class) +******************** +Parameter #0 [ string $v2 ] +array(2) { + [0]=> + object(ReflectionAttribute)#%d (1) { + ["name"]=> + string(23) "DelayedTargetValidation" + } + [1]=> + object(ReflectionAttribute)#%d (1) { + ["name"]=> + string(9) "Attribute" + } +} +Error: Attribute "Attribute" cannot target parameter (allowed targets: class) +******************** +Property [ public string $v2 ] + +array(2) { + [0]=> + object(ReflectionAttribute)#%d (1) { + ["name"]=> + string(23) "DelayedTargetValidation" + } + [1]=> + object(ReflectionAttribute)#%d (1) { + ["name"]=> + string(9) "Attribute" + } +} +Error: Attribute "Attribute" cannot target property (allowed targets: class) +******************** +Function [ function demoFn ] { + @@ %s %d - %d +} + +array(2) { + [0]=> + object(ReflectionAttribute)#%d (1) { + ["name"]=> + string(23) "DelayedTargetValidation" + } + [1]=> + object(ReflectionAttribute)#%d (1) { + ["name"]=> + string(9) "Attribute" + } +} +Error: Attribute "Attribute" cannot target function (allowed targets: class) +******************** +Constant [ bool EXAMPLE ] { 1 } + +array(2) { + [0]=> + object(ReflectionAttribute)#%d (1) { + ["name"]=> + string(23) "DelayedTargetValidation" + } + [1]=> + object(ReflectionAttribute)#%d (1) { + ["name"]=> + string(9) "Attribute" + } +} +Error: Attribute "Attribute" cannot target constant (allowed targets: class) diff --git a/Zend/tests/attributes/delayed_target_validation/no_compile_errors.phpt b/Zend/tests/attributes/delayed_target_validation/no_compile_errors.phpt new file mode 100644 index 0000000000000..ea96a069d748c --- /dev/null +++ b/Zend/tests/attributes/delayed_target_validation/no_compile_errors.phpt @@ -0,0 +1,46 @@ +--TEST-- +#[\DelayedTargetValidation] prevents target errors at compile time +--FILE-- +v = $v2; + echo __METHOD__ . "\n"; + } +} + +#[DelayedTargetValidation] +#[Attribute] +function demoFn() { + echo __FUNCTION__ . "\n"; +} + +$o = new Demo( "foo" ); +demoFn(); + +#[DelayedTargetValidation] +#[Attribute] +const EXAMPLE = true; + +?> +--EXPECT-- +Demo::__construct +demoFn diff --git a/Zend/tests/attributes/delayed_target_validation/repetition_errors.phpt b/Zend/tests/attributes/delayed_target_validation/repetition_errors.phpt new file mode 100644 index 0000000000000..5c8f9bfc9dde2 --- /dev/null +++ b/Zend/tests/attributes/delayed_target_validation/repetition_errors.phpt @@ -0,0 +1,13 @@ +--TEST-- +#[\DelayedTargetValidation] does not prevent repetition errors +--FILE-- + +--EXPECTF-- +Fatal error: Attribute "NoDiscard" must not be repeated in %s on line %d diff --git a/Zend/zend_attributes.c b/Zend/zend_attributes.c index 3256e220d8f3a..da8c796900345 100644 --- a/Zend/zend_attributes.c +++ b/Zend/zend_attributes.c @@ -32,6 +32,7 @@ ZEND_API zend_class_entry *zend_ce_sensitive_parameter_value; ZEND_API zend_class_entry *zend_ce_override; ZEND_API zend_class_entry *zend_ce_deprecated; ZEND_API zend_class_entry *zend_ce_nodiscard; +ZEND_API zend_class_entry *zend_ce_delayed_target_validation; static zend_object_handlers attributes_object_handlers_sensitive_parameter_value; @@ -72,25 +73,21 @@ uint32_t zend_attribute_attribute_get_flags(zend_attribute *attr, zend_class_ent static void validate_allow_dynamic_properties( zend_attribute *attr, uint32_t target, zend_class_entry *scope) { + const char *msg = NULL; if (scope->ce_flags & ZEND_ACC_TRAIT) { - zend_error_noreturn(E_ERROR, "Cannot apply #[\\AllowDynamicProperties] to trait %s", - ZSTR_VAL(scope->name) - ); - } - if (scope->ce_flags & ZEND_ACC_INTERFACE) { - zend_error_noreturn(E_ERROR, "Cannot apply #[\\AllowDynamicProperties] to interface %s", - ZSTR_VAL(scope->name) - ); - } - if (scope->ce_flags & ZEND_ACC_READONLY_CLASS) { - zend_error_noreturn(E_ERROR, "Cannot apply #[\\AllowDynamicProperties] to readonly class %s", - ZSTR_VAL(scope->name) - ); + msg = "Cannot apply #[\\AllowDynamicProperties] to trait %s"; + } else if (scope->ce_flags & ZEND_ACC_INTERFACE) { + msg = "Cannot apply #[\\AllowDynamicProperties] to interface %s"; + } else if (scope->ce_flags & ZEND_ACC_READONLY_CLASS) { + msg = "Cannot apply #[\\AllowDynamicProperties] to readonly class %s"; + } else if (scope->ce_flags & ZEND_ACC_ENUM) { + msg = "Cannot apply #[\\AllowDynamicProperties] to enum %s"; } - if (scope->ce_flags & ZEND_ACC_ENUM) { - zend_error_noreturn(E_ERROR, "Cannot apply #[\\AllowDynamicProperties] to enum %s", - ZSTR_VAL(scope->name) - ); + if (msg != NULL) { + if (target & ZEND_ATTRIBUTE_NO_TARGET_VALIDATION) { + return; + } + zend_error_noreturn(E_ERROR, msg, ZSTR_VAL(scope->name) ); } scope->ce_flags |= ZEND_ACC_ALLOW_DYNAMIC_PROPERTIES; } @@ -505,7 +502,12 @@ ZEND_API zend_internal_attribute *zend_mark_internal_attribute(zend_class_entry if (zend_string_equals(attr->name, zend_ce_attribute->name)) { internal_attr = pemalloc(sizeof(zend_internal_attribute), 1); internal_attr->ce = ce; - internal_attr->flags = Z_LVAL(attr->args[0].value); + if (Z_TYPE(attr->args[0].value) == IS_NULL) { + // Apply default of Attribute::TARGET_ALL + internal_attr->flags = ZEND_ATTRIBUTE_TARGET_ALL; + } else { + internal_attr->flags = Z_LVAL(attr->args[0].value); + } internal_attr->validator = NULL; zend_string *lcname = zend_string_tolower_ex(ce->name, 1); @@ -567,6 +569,9 @@ void zend_register_attribute_ce(void) zend_ce_nodiscard = register_class_NoDiscard(); attr = zend_mark_internal_attribute(zend_ce_nodiscard); + + zend_ce_delayed_target_validation = register_class_DelayedTargetValidation(); + attr = zend_mark_internal_attribute(zend_ce_delayed_target_validation); } void zend_attributes_shutdown(void) diff --git a/Zend/zend_attributes.h b/Zend/zend_attributes.h index a4d6b28c0094a..792b0d23e7a23 100644 --- a/Zend/zend_attributes.h +++ b/Zend/zend_attributes.h @@ -34,6 +34,10 @@ #define ZEND_ATTRIBUTE_IS_REPEATABLE (1<<7) #define ZEND_ATTRIBUTE_FLAGS ((1<<8) - 1) +/* Not a real flag, just passed to validators when target validation is * + * suppressed; must not conflict with any of the real flags above. */ +#define ZEND_ATTRIBUTE_NO_TARGET_VALIDATION (1<<8) + /* Flags for zend_attribute.flags */ #define ZEND_ATTRIBUTE_PERSISTENT (1<<0) #define ZEND_ATTRIBUTE_STRICT_TYPES (1<<1) @@ -50,6 +54,7 @@ extern ZEND_API zend_class_entry *zend_ce_sensitive_parameter_value; extern ZEND_API zend_class_entry *zend_ce_override; extern ZEND_API zend_class_entry *zend_ce_deprecated; extern ZEND_API zend_class_entry *zend_ce_nodiscard; +extern ZEND_API zend_class_entry *zend_ce_delayed_target_validation; typedef struct { zend_string *name; diff --git a/Zend/zend_attributes.stub.php b/Zend/zend_attributes.stub.php index 242b751160864..78f39a85bd5e7 100644 --- a/Zend/zend_attributes.stub.php +++ b/Zend/zend_attributes.stub.php @@ -97,3 +97,9 @@ final class NoDiscard public function __construct(?string $message = null) {} } + +/** + * @strict-properties + */ +#[Attribute] +final class DelayedTargetValidation {} diff --git a/Zend/zend_attributes_arginfo.h b/Zend/zend_attributes_arginfo.h index 5e7f581dd2226..b735b5eb52682 100644 --- a/Zend/zend_attributes_arginfo.h +++ b/Zend/zend_attributes_arginfo.h @@ -1,5 +1,5 @@ /* This is a generated file, edit the .stub.php file instead. - * Stub hash: 715016d1ff1b0a6abb325a71083eff397a080c44 */ + * Stub hash: 2d538e455657bc49ae0aa98b5534838cd9ffe487 */ ZEND_BEGIN_ARG_INFO_EX(arginfo_class_Attribute___construct, 0, 0, 0) ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, flags, IS_LONG, 0, "Attribute::TARGET_ALL") @@ -276,3 +276,17 @@ static zend_class_entry *register_class_NoDiscard(void) return class_entry; } + +static zend_class_entry *register_class_DelayedTargetValidation(void) +{ + zend_class_entry ce, *class_entry; + + INIT_CLASS_ENTRY(ce, "DelayedTargetValidation", NULL); + class_entry = zend_register_internal_class_with_flags(&ce, NULL, ZEND_ACC_FINAL|ZEND_ACC_NO_DYNAMIC_PROPERTIES); + + zend_string *attribute_name_Attribute_class_DelayedTargetValidation_0 = zend_string_init_interned("Attribute", sizeof("Attribute") - 1, 1); + zend_add_class_attribute(class_entry, attribute_name_Attribute_class_DelayedTargetValidation_0, 0); + zend_string_release(attribute_name_Attribute_class_DelayedTargetValidation_0); + + return class_entry; +} diff --git a/Zend/zend_compile.c b/Zend/zend_compile.c index 020b63e362be8..4ff4ed6dbd3cc 100644 --- a/Zend/zend_compile.c +++ b/Zend/zend_compile.c @@ -7557,19 +7557,28 @@ static void zend_compile_attributes( } if (*attributes != NULL) { + /* Allow delaying target validation for forward compatibility. */ + zend_attribute *delayed_target_validation = zend_get_attribute_str( + *attributes, + "delayedtargetvalidation", + strlen("delayedtargetvalidation") + ); + uint32_t extra_flags = delayed_target_validation ? ZEND_ATTRIBUTE_NO_TARGET_VALIDATION : 0; /* Validate attributes in a secondary loop (needed to detect repeated attributes). */ ZEND_HASH_PACKED_FOREACH_PTR(*attributes, attr) { if (attr->offset != offset || NULL == (config = zend_internal_attribute_get(attr->lcname))) { continue; } - if (!(target & (config->flags & ZEND_ATTRIBUTE_TARGET_ALL))) { - zend_string *location = zend_get_attribute_target_names(target); - zend_string *allowed = zend_get_attribute_target_names(config->flags); + if (delayed_target_validation == NULL) { + if (!(target & (config->flags & ZEND_ATTRIBUTE_TARGET_ALL))) { + zend_string *location = zend_get_attribute_target_names(target); + zend_string *allowed = zend_get_attribute_target_names(config->flags); - zend_error_noreturn(E_ERROR, "Attribute \"%s\" cannot target %s (allowed targets: %s)", - ZSTR_VAL(attr->name), ZSTR_VAL(location), ZSTR_VAL(allowed) - ); + zend_error_noreturn(E_ERROR, "Attribute \"%s\" cannot target %s (allowed targets: %s)", + ZSTR_VAL(attr->name), ZSTR_VAL(location), ZSTR_VAL(allowed) + ); + } } if (!(config->flags & ZEND_ATTRIBUTE_IS_REPEATABLE)) { @@ -7579,7 +7588,7 @@ static void zend_compile_attributes( } if (config->validator != NULL) { - config->validator(attr, target, CG(active_class_entry)); + config->validator(attr, target | extra_flags, CG(active_class_entry)); } } ZEND_HASH_FOREACH_END(); } diff --git a/ext/reflection/php_reflection.c b/ext/reflection/php_reflection.c index b5ca21d500a82..3098d0ee0740d 100644 --- a/ext/reflection/php_reflection.c +++ b/ext/reflection/php_reflection.c @@ -7321,26 +7321,42 @@ ZEND_METHOD(ReflectionAttribute, newInstance) RETURN_THROWS(); } - if (ce->type == ZEND_USER_CLASS) { - uint32_t flags = zend_attribute_attribute_get_flags(marker, ce); - if (EG(exception)) { - RETURN_THROWS(); - } + /* This code can be reached under one of three possible conditions: + * - the attribute is an internal attribute, and it had the target and + * and repetition validated already + * - the attribute is an internal attribute and repetition was validated + * already, but the target was not validated due to the presence of + * #[DelayedTargetValidation] + * - the attribute is a user attribute, and neither target nor repetition + * have been validated. + * + * It is not worth checking for the presence of #[DelayedTargetValidation] + * to determine if we should run target validation for internal attributes; + * it is faster just to do the validation, which will always pass if the + * attribute is absent. + */ + uint32_t flags = zend_attribute_attribute_get_flags(marker, ce); + if (EG(exception)) { + RETURN_THROWS(); + } - if (!(attr->target & flags)) { - zend_string *location = zend_get_attribute_target_names(attr->target); - zend_string *allowed = zend_get_attribute_target_names(flags); + if (!(attr->target & flags)) { + zend_string *location = zend_get_attribute_target_names(attr->target); + zend_string *allowed = zend_get_attribute_target_names(flags); - zend_throw_error(NULL, "Attribute \"%s\" cannot target %s (allowed targets: %s)", - ZSTR_VAL(attr->data->name), ZSTR_VAL(location), ZSTR_VAL(allowed) - ); + zend_throw_error(NULL, "Attribute \"%s\" cannot target %s (allowed targets: %s)", + ZSTR_VAL(attr->data->name), ZSTR_VAL(location), ZSTR_VAL(allowed) + ); - zend_string_release(location); - zend_string_release(allowed); + zend_string_release(location); + zend_string_release(allowed); - RETURN_THROWS(); - } + RETURN_THROWS(); + } + /* Repetition validation is done even if #[DelayedTargetValidation] is used + * and so can be skipped for internal attributes. */ + if (ce->type == ZEND_USER_CLASS) { if (!(flags & ZEND_ATTRIBUTE_IS_REPEATABLE)) { if (zend_is_attribute_repeated(attr->attributes, attr->data)) { zend_throw_error(NULL, "Attribute \"%s\" must not be repeated", ZSTR_VAL(attr->data->name)); From 5bc6d09fbf30ae201dcf577b8d75c2c135c99eea Mon Sep 17 00:00:00 2001 From: Daniel Scherzer Date: Mon, 9 Jun 2025 13:44:00 -0700 Subject: [PATCH 02/23] Avoid uninitialized memory --- Zend/zend_attributes.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Zend/zend_attributes.c b/Zend/zend_attributes.c index da8c796900345..f9c993581e443 100644 --- a/Zend/zend_attributes.c +++ b/Zend/zend_attributes.c @@ -502,7 +502,7 @@ ZEND_API zend_internal_attribute *zend_mark_internal_attribute(zend_class_entry if (zend_string_equals(attr->name, zend_ce_attribute->name)) { internal_attr = pemalloc(sizeof(zend_internal_attribute), 1); internal_attr->ce = ce; - if (Z_TYPE(attr->args[0].value) == IS_NULL) { + if (attr->argc == 0) { // Apply default of Attribute::TARGET_ALL internal_attr->flags = ZEND_ATTRIBUTE_TARGET_ALL; } else { From 7093f3f51d50a0aaa6c6399e1eb17198786a786f Mon Sep 17 00:00:00 2001 From: Daniel Scherzer Date: Sun, 6 Jul 2025 10:30:02 -0700 Subject: [PATCH 03/23] Run validators for delayed validators --- .../errors_from_validator.phpt | 180 ++++++++++++++++++ .../has_runtime_errors.phpt | 2 +- .../validator_success.phpt | 62 ++++++ Zend/zend_attributes.c | 12 ++ Zend/zend_attributes.h | 6 +- ext/reflection/php_reflection.c | 15 ++ 6 files changed, 275 insertions(+), 2 deletions(-) create mode 100644 Zend/tests/attributes/delayed_target_validation/errors_from_validator.phpt create mode 100644 Zend/tests/attributes/delayed_target_validation/validator_success.phpt diff --git a/Zend/tests/attributes/delayed_target_validation/errors_from_validator.phpt b/Zend/tests/attributes/delayed_target_validation/errors_from_validator.phpt new file mode 100644 index 0000000000000..d0948e5fd7ad0 --- /dev/null +++ b/Zend/tests/attributes/delayed_target_validation/errors_from_validator.phpt @@ -0,0 +1,180 @@ +--TEST-- +#[\DelayedTargetValidation] affects errors from validators +--FILE-- +getAttributes(); + var_dump($attributes); + try { + $attributes[1]->newInstance(); + } catch (Error $e) { + echo get_class($e) . ": " . $e->getMessage() . "\n"; + } +} + +?> +--EXPECTF-- +******************** +Trait [ trait DemoTrait ] { + @@ %s %d-%d + + - Constants [0] { + } + + - Static properties [0] { + } + + - Static methods [0] { + } + + - Properties [0] { + } + + - Methods [0] { + } +} + +array(2) { + [0]=> + object(ReflectionAttribute)#%d (1) { + ["name"]=> + string(23) "DelayedTargetValidation" + } + [1]=> + object(ReflectionAttribute)#%d (1) { + ["name"]=> + string(22) "AllowDynamicProperties" + } +} +Error: Cannot apply #[AllowDynamicProperties] to trait DemoTrait +******************** +Interface [ interface DemoInterface ] { + @@ %s %d-%d + + - Constants [0] { + } + + - Static properties [0] { + } + + - Static methods [0] { + } + + - Properties [0] { + } + + - Methods [0] { + } +} + +array(2) { + [0]=> + object(ReflectionAttribute)#%d (1) { + ["name"]=> + string(23) "DelayedTargetValidation" + } + [1]=> + object(ReflectionAttribute)#%d (1) { + ["name"]=> + string(22) "AllowDynamicProperties" + } +} +Error: Cannot apply #[AllowDynamicProperties] to interface DemoInterface +******************** +Class [ readonly class DemoReadonly ] { + @@ %s %d-%d + + - Constants [0] { + } + + - Static properties [0] { + } + + - Static methods [0] { + } + + - Properties [0] { + } + + - Methods [0] { + } +} + +array(2) { + [0]=> + object(ReflectionAttribute)#%d (1) { + ["name"]=> + string(23) "DelayedTargetValidation" + } + [1]=> + object(ReflectionAttribute)#%d (1) { + ["name"]=> + string(22) "AllowDynamicProperties" + } +} +Error: Cannot apply #[AllowDynamicProperties] to readonly class DemoReadonly +******************** +Enum [ enum DemoEnum implements UnitEnum ] { + @@ %s %d-%d + + - Constants [0] { + } + + - Static properties [0] { + } + + - Static methods [1] { + Method [ static public method cases ] { + + - Parameters [0] { + } + - Return [ array ] + } + } + + - Properties [1] { + Property [ public protected(set) readonly string $name ] + } + + - Methods [0] { + } +} + +array(2) { + [0]=> + object(ReflectionAttribute)#%d (1) { + ["name"]=> + string(23) "DelayedTargetValidation" + } + [1]=> + object(ReflectionAttribute)#%d (1) { + ["name"]=> + string(22) "AllowDynamicProperties" + } +} +Error: Cannot apply #[AllowDynamicProperties] to enum DemoEnum diff --git a/Zend/tests/attributes/delayed_target_validation/has_runtime_errors.phpt b/Zend/tests/attributes/delayed_target_validation/has_runtime_errors.phpt index 0d280bd446159..bb65b126172fb 100644 --- a/Zend/tests/attributes/delayed_target_validation/has_runtime_errors.phpt +++ b/Zend/tests/attributes/delayed_target_validation/has_runtime_errors.phpt @@ -1,5 +1,5 @@ --TEST-- -#[\DelayedTargetValidation] prevents target errors at compile time +#[\DelayedTargetValidation] has errors at runtime --FILE-- dynamic = true; +var_dump($obj); + +$ref = new ReflectionClass('DemoClass'); +echo $ref . "\n"; +$attributes = $ref->getAttributes(); +var_dump($attributes); +var_dump($attributes[1]->newInstance()); + +?> +--EXPECTF-- +object(DemoClass)#%d (0) { +} +object(DemoClass)#%d (1) { + ["dynamic"]=> + bool(true) +} +Class [ class DemoClass ] { + @@ %s %d-%d + + - Constants [0] { + } + + - Static properties [0] { + } + + - Static methods [0] { + } + + - Properties [0] { + } + + - Methods [0] { + } +} + +array(2) { + [0]=> + object(ReflectionAttribute)#%d (1) { + ["name"]=> + string(23) "DelayedTargetValidation" + } + [1]=> + object(ReflectionAttribute)#%d (1) { + ["name"]=> + string(22) "AllowDynamicProperties" + } +} +object(AllowDynamicProperties)#%d (0) { +} diff --git a/Zend/zend_attributes.c b/Zend/zend_attributes.c index f9c993581e443..c9ee641f84f95 100644 --- a/Zend/zend_attributes.c +++ b/Zend/zend_attributes.c @@ -87,8 +87,20 @@ static void validate_allow_dynamic_properties( if (target & ZEND_ATTRIBUTE_NO_TARGET_VALIDATION) { return; } + if (target & ZEND_ATTRIBUTE_DELAYED_TARGET_VALIDATION) { + // Should not have passed the first time + ZEND_ASSERT((scope->ce_flags & ZEND_ACC_ALLOW_DYNAMIC_PROPERTIES) == 0); + // Throw a catchable error at runtime + zend_throw_error(NULL, msg, ZSTR_VAL(scope->name)); + return; + } zend_error_noreturn(E_ERROR, msg, ZSTR_VAL(scope->name) ); } + if (target & ZEND_ATTRIBUTE_DELAYED_TARGET_VALIDATION) { + // Should have passed the first time + ZEND_ASSERT((scope->ce_flags & ZEND_ACC_ALLOW_DYNAMIC_PROPERTIES) != 0); + return; + } scope->ce_flags |= ZEND_ACC_ALLOW_DYNAMIC_PROPERTIES; } diff --git a/Zend/zend_attributes.h b/Zend/zend_attributes.h index 792b0d23e7a23..08f199c7cd52a 100644 --- a/Zend/zend_attributes.h +++ b/Zend/zend_attributes.h @@ -34,10 +34,14 @@ #define ZEND_ATTRIBUTE_IS_REPEATABLE (1<<7) #define ZEND_ATTRIBUTE_FLAGS ((1<<8) - 1) -/* Not a real flag, just passed to validators when target validation is * +/* Not a real flag, just passed to validators when target validation is * suppressed; must not conflict with any of the real flags above. */ #define ZEND_ATTRIBUTE_NO_TARGET_VALIDATION (1<<8) +/* Not a real flag, just passed to validators when target validation is + * being run at runtime; must not conflict with any of the real flags above. */ +#define ZEND_ATTRIBUTE_DELAYED_TARGET_VALIDATION (1<<9) + /* Flags for zend_attribute.flags */ #define ZEND_ATTRIBUTE_PERSISTENT (1<<0) #define ZEND_ATTRIBUTE_STRICT_TYPES (1<<1) diff --git a/ext/reflection/php_reflection.c b/ext/reflection/php_reflection.c index 3098d0ee0740d..32ae0ca16e983 100644 --- a/ext/reflection/php_reflection.c +++ b/ext/reflection/php_reflection.c @@ -7354,6 +7354,21 @@ ZEND_METHOD(ReflectionAttribute, newInstance) RETURN_THROWS(); } + /* Run the delayed validator function for internal attributes */ + if (ce->type == ZEND_INTERNAL_CLASS) { + zend_internal_attribute *config = zend_internal_attribute_get(attr->data->lcname); + if (config != NULL && config->validator != NULL) { + config->validator( + attr->data, + attr->target | ZEND_ATTRIBUTE_DELAYED_TARGET_VALIDATION, + attr->scope + ); + if (EG(exception)) { + RETURN_THROWS(); + } + } + } + /* Repetition validation is done even if #[DelayedTargetValidation] is used * and so can be skipped for internal attributes. */ if (ce->type == ZEND_USER_CLASS) { From fa9574646f5f8411311882df4510227b3cf25257 Mon Sep 17 00:00:00 2001 From: Daniel Scherzer Date: Sun, 6 Jul 2025 11:06:45 -0700 Subject: [PATCH 04/23] Fix for reflection --- ext/reflection/php_reflection.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/ext/reflection/php_reflection.c b/ext/reflection/php_reflection.c index 32ae0ca16e983..55374a676d157 100644 --- a/ext/reflection/php_reflection.c +++ b/ext/reflection/php_reflection.c @@ -7321,6 +7321,12 @@ ZEND_METHOD(ReflectionAttribute, newInstance) RETURN_THROWS(); } + zend_attribute *delayed_target_validation = zend_get_attribute_str( + attr->attributes, + "delayedtargetvalidation", + strlen("delayedtargetvalidation") + ); + /* This code can be reached under one of three possible conditions: * - the attribute is an internal attribute, and it had the target and * and repetition validated already @@ -7329,17 +7335,15 @@ ZEND_METHOD(ReflectionAttribute, newInstance) * #[DelayedTargetValidation] * - the attribute is a user attribute, and neither target nor repetition * have been validated. - * - * It is not worth checking for the presence of #[DelayedTargetValidation] - * to determine if we should run target validation for internal attributes; - * it is faster just to do the validation, which will always pass if the - * attribute is absent. */ uint32_t flags = zend_attribute_attribute_get_flags(marker, ce); if (EG(exception)) { RETURN_THROWS(); } + /* No harm in always running target validation, for internal attributes + * with #[DelayedTargetValidation] it isn't necessary but will always + * succeed. */ if (!(attr->target & flags)) { zend_string *location = zend_get_attribute_target_names(attr->target); zend_string *allowed = zend_get_attribute_target_names(flags); @@ -7355,12 +7359,12 @@ ZEND_METHOD(ReflectionAttribute, newInstance) } /* Run the delayed validator function for internal attributes */ - if (ce->type == ZEND_INTERNAL_CLASS) { + if (delayed_target_validation && ce->type == ZEND_INTERNAL_CLASS) { zend_internal_attribute *config = zend_internal_attribute_get(attr->data->lcname); if (config != NULL && config->validator != NULL) { config->validator( attr->data, - attr->target | ZEND_ATTRIBUTE_DELAYED_TARGET_VALIDATION, + flags | ZEND_ATTRIBUTE_DELAYED_TARGET_VALIDATION, attr->scope ); if (EG(exception)) { From 04f280b640ba4b86714e2e0070c4e4b5f95e1b7b Mon Sep 17 00:00:00 2001 From: Daniel Scherzer Date: Wed, 9 Jul 2025 16:07:59 -0700 Subject: [PATCH 05/23] Test application of all internal attributes, fix 2 bugs --- .../with_AllowDynamicProperties.phpt | 67 +++++++++++++++ .../with_Attribute.phpt | 81 +++++++++++++++++++ .../with_Deprecated.phpt | 66 +++++++++++++++ .../with_NoDiscard.phpt | 66 +++++++++++++++ .../with_Override_error.phpt | 18 +++++ .../with_Override_okay.phpt | 64 +++++++++++++++ .../with_ReturnTypeWillChange.phpt | 70 ++++++++++++++++ .../with_SensitiveParameter.phpt | 70 ++++++++++++++++ Zend/zend_attributes.c | 7 ++ Zend/zend_compile.c | 22 +++-- 10 files changed, 526 insertions(+), 5 deletions(-) create mode 100644 Zend/tests/attributes/delayed_target_validation/with_AllowDynamicProperties.phpt create mode 100644 Zend/tests/attributes/delayed_target_validation/with_Attribute.phpt create mode 100644 Zend/tests/attributes/delayed_target_validation/with_Deprecated.phpt create mode 100644 Zend/tests/attributes/delayed_target_validation/with_NoDiscard.phpt create mode 100644 Zend/tests/attributes/delayed_target_validation/with_Override_error.phpt create mode 100644 Zend/tests/attributes/delayed_target_validation/with_Override_okay.phpt create mode 100644 Zend/tests/attributes/delayed_target_validation/with_ReturnTypeWillChange.phpt create mode 100644 Zend/tests/attributes/delayed_target_validation/with_SensitiveParameter.phpt diff --git a/Zend/tests/attributes/delayed_target_validation/with_AllowDynamicProperties.phpt b/Zend/tests/attributes/delayed_target_validation/with_AllowDynamicProperties.phpt new file mode 100644 index 0000000000000..b9154f37f73d0 --- /dev/null +++ b/Zend/tests/attributes/delayed_target_validation/with_AllowDynamicProperties.phpt @@ -0,0 +1,67 @@ +--TEST-- +#[\DelayedTargetValidation] with #[\AllowDynamicProperties]: invalid targets don't error +--FILE-- +val = $str; + } + + #[DelayedTargetValidation] + #[AllowDynamicProperties] // Does nothing here + public function printVal() { + echo 'Value is: ' . $this->val . "\n"; + } + +} + +#[DelayedTargetValidation] +#[AllowDynamicProperties] // Does nothing here +function demoFn() { + echo __FUNCTION__ . "\n"; + return 456; +} + +#[DelayedTargetValidation] +#[AllowDynamicProperties] // Does nothing here +const GLOBAL_CONST = 'BAR'; + +$d = new DemoClass('example'); +$d->printVal(); +var_dump($d->val); +var_dump(DemoClass::CLASS_CONST); +demoFn(); +var_dump(GLOBAL_CONST); + +$d->missingProp = 'foo'; +var_dump($d); +?> +--EXPECTF-- +Got: example +Value is: example +string(7) "example" +string(3) "FOO" +demoFn +string(3) "BAR" +object(DemoClass)#%d (2) { + ["val"]=> + string(7) "example" + ["missingProp"]=> + string(3) "foo" +} diff --git a/Zend/tests/attributes/delayed_target_validation/with_Attribute.phpt b/Zend/tests/attributes/delayed_target_validation/with_Attribute.phpt new file mode 100644 index 0000000000000..13b33dca53ee8 --- /dev/null +++ b/Zend/tests/attributes/delayed_target_validation/with_Attribute.phpt @@ -0,0 +1,81 @@ +--TEST-- +#[\DelayedTargetValidation] with #[\Attribute]: invalid targets don't error +--FILE-- +val = $str; + } + + #[DelayedTargetValidation] + #[Attribute] // Does nothing here + public function printVal() { + echo 'Value is: ' . $this->val . "\n"; + } + +} + +#[DelayedTargetValidation] +#[Attribute] // Does nothing here +function demoFn() { + echo __FUNCTION__ . "\n"; + return 456; +} + +#[DelayedTargetValidation] +#[Attribute] // Does nothing here +const GLOBAL_CONST = 'BAR'; + +$d = new DemoClass('example'); +$d->printVal(); +var_dump($d->val); +var_dump(DemoClass::CLASS_CONST); +demoFn(); +var_dump(GLOBAL_CONST); + +#[DemoClass('BAZ')] +#[NonAttribute] +class WithDemoAttribs {} + +$ref = new ReflectionClass(WithDemoAttribs::class); +$attribs = $ref->getAttributes(); +var_dump($attribs[0]->newInstance()); +var_dump($attribs[1]->newInstance()); + +?> +--EXPECTF-- +Got: example +Value is: example +string(7) "example" +string(3) "FOO" +demoFn +string(3) "BAR" +Got: BAZ +object(DemoClass)#5 (1) { + ["val"]=> + string(3) "BAZ" +} + +Fatal error: Uncaught Error: Attempting to use non-attribute class "NonAttribute" as attribute in %s:%d +Stack trace: +#0 %s(%d): ReflectionAttribute->newInstance() +#1 {main} + thrown in %s on line %d diff --git a/Zend/tests/attributes/delayed_target_validation/with_Deprecated.phpt b/Zend/tests/attributes/delayed_target_validation/with_Deprecated.phpt new file mode 100644 index 0000000000000..e0d3d4c4ae592 --- /dev/null +++ b/Zend/tests/attributes/delayed_target_validation/with_Deprecated.phpt @@ -0,0 +1,66 @@ +--TEST-- +#[\DelayedTargetValidation] with #[\Deprecated]: valid targets are deprecated +--FILE-- +val = $str; + } + + #[DelayedTargetValidation] + #[Deprecated] // Does something here + public function printVal() { + echo 'Value is: ' . $this->val . "\n"; + return 123; + } +} + +#[DelayedTargetValidation] +#[Deprecated] // Does something here +function demoFn() { + echo __FUNCTION__ . "\n"; + return 456; +} + +#[DelayedTargetValidation] +#[Deprecated] // Does something here +const GLOBAL_CONST = 'BAR'; + +$d = new DemoClass('example'); +$d->printVal(); +var_dump($d->val); +var_dump(DemoClass::CLASS_CONST); +demoFn(); +var_dump(GLOBAL_CONST); +?> +--EXPECTF-- +Got: example + +Deprecated: Method DemoClass::printVal() is deprecated in %s on line %d +Value is: example +string(7) "example" + +Deprecated: Constant DemoClass::CLASS_CONST is deprecated in %s on line %d +string(3) "FOO" + +Deprecated: Function demoFn() is deprecated in %s on line %d +demoFn + +Deprecated: Constant GLOBAL_CONST is deprecated in %s on line %d +string(3) "BAR" diff --git a/Zend/tests/attributes/delayed_target_validation/with_NoDiscard.phpt b/Zend/tests/attributes/delayed_target_validation/with_NoDiscard.phpt new file mode 100644 index 0000000000000..affc3a691deaf --- /dev/null +++ b/Zend/tests/attributes/delayed_target_validation/with_NoDiscard.phpt @@ -0,0 +1,66 @@ +--TEST-- +#[\DelayedTargetValidation] with #[\NoDiscard]: valid targets complain about discarding +--FILE-- +val = $str; + } + + #[DelayedTargetValidation] + #[NoDiscard] // Does something here + public function printVal() { + echo 'Value is: ' . $this->val . "\n"; + return 123; + } +} + +#[DelayedTargetValidation] +#[NoDiscard] // Does something here +function demoFn() { + echo __FUNCTION__ . "\n"; + return 456; +} + +#[DelayedTargetValidation] +#[NoDiscard] // Does nothing here +const GLOBAL_CONST = 'BAR'; + +$d = new DemoClass('example'); +$d->printVal(); +$v = $d->printVal(); +var_dump($d->val); +var_dump(DemoClass::CLASS_CONST); +demoFn(); +$v = demoFn(); +var_dump(GLOBAL_CONST); +?> +--EXPECTF-- +Got: example + +Warning: The return value of method DemoClass::printVal() should either be used or intentionally ignored by casting it as (void) in %s on line %d +Value is: example +Value is: example +string(7) "example" +string(3) "FOO" + +Warning: The return value of function demoFn() should either be used or intentionally ignored by casting it as (void) in %s on line %d +demoFn +demoFn +string(3) "BAR" diff --git a/Zend/tests/attributes/delayed_target_validation/with_Override_error.phpt b/Zend/tests/attributes/delayed_target_validation/with_Override_error.phpt new file mode 100644 index 0000000000000..46b05713917b0 --- /dev/null +++ b/Zend/tests/attributes/delayed_target_validation/with_Override_error.phpt @@ -0,0 +1,18 @@ +--TEST-- +#[\DelayedTargetValidation] with #[\Override]: non-overrides still error +--FILE-- +val . "\n"; + return 123; + } +} + +?> +--EXPECTF-- +Fatal error: DemoClass::printVal() has #[\Override] attribute, but no matching parent method exists in %s on line %d diff --git a/Zend/tests/attributes/delayed_target_validation/with_Override_okay.phpt b/Zend/tests/attributes/delayed_target_validation/with_Override_okay.phpt new file mode 100644 index 0000000000000..33e171e1be156 --- /dev/null +++ b/Zend/tests/attributes/delayed_target_validation/with_Override_okay.phpt @@ -0,0 +1,64 @@ +--TEST-- +#[\DelayedTargetValidation] with #[\Override]: invalid targets or actual overrides don't do anything +--FILE-- +val = $str; + } + + #[DelayedTargetValidation] + #[Override] // Does something here + public function printVal() { + echo 'Value is: ' . $this->val . "\n"; + return 123; + } +} + +#[DelayedTargetValidation] +#[Override] // Does nothing here +function demoFn() { + echo __FUNCTION__ . "\n"; + return 456; +} + +#[DelayedTargetValidation] +#[Override] // Does nothing here +const GLOBAL_CONST = 'BAR'; + +$d = new DemoClass('example'); +$d->printVal(); +var_dump($d->val); +var_dump(DemoClass::CLASS_CONST); +demoFn(); +var_dump(GLOBAL_CONST); +?> +--EXPECT-- +Got: example +Value is: example +string(7) "example" +string(3) "FOO" +demoFn +string(3) "BAR" diff --git a/Zend/tests/attributes/delayed_target_validation/with_ReturnTypeWillChange.phpt b/Zend/tests/attributes/delayed_target_validation/with_ReturnTypeWillChange.phpt new file mode 100644 index 0000000000000..0cc3bb4a4b9a4 --- /dev/null +++ b/Zend/tests/attributes/delayed_target_validation/with_ReturnTypeWillChange.phpt @@ -0,0 +1,70 @@ +--TEST-- +#[\DelayedTargetValidation] with #[\ReturnTypeWillChange]: valid targets suppress return type warnings +--FILE-- +val = $str; + } + + #[DelayedTargetValidation] + #[ReturnTypeWillChange] // Does something here + public function printVal() { + echo 'Value is: ' . $this->val . "\n"; + } + + #[DelayedTargetValidation] + #[ReturnTypeWillChange] // Does something here + public function count() { + return 5; + } +} + +#[DelayedTargetValidation] +#[ReturnTypeWillChange] // Does nothing here +function demoFn() { + echo __FUNCTION__ . "\n"; + return 456; +} + +#[DelayedTargetValidation] +#[ReturnTypeWillChange] // Does nothing here +const GLOBAL_CONST = 'BAR'; + +$d = new DemoClass('example'); +$d->printVal(); +var_dump($d->val); +var_dump(DemoClass::CLASS_CONST); +demoFn(); +var_dump(GLOBAL_CONST); +?> +--EXPECTF-- +Deprecated: Return type of WithoutAttrib::count() should either be compatible with Countable::count(): int, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in %s on line %d +Got: example +Value is: example +string(7) "example" +string(3) "FOO" +demoFn +string(3) "BAR" diff --git a/Zend/tests/attributes/delayed_target_validation/with_SensitiveParameter.phpt b/Zend/tests/attributes/delayed_target_validation/with_SensitiveParameter.phpt new file mode 100644 index 0000000000000..440d47d42b38e --- /dev/null +++ b/Zend/tests/attributes/delayed_target_validation/with_SensitiveParameter.phpt @@ -0,0 +1,70 @@ +--TEST-- +#[\DelayedTargetValidation] with #[\SensitiveParameter]: parameter still redacted +--FILE-- +val = $str; + } + + #[DelayedTargetValidation] + #[SensitiveParameter] // Does nothing here + public function printVal( + #[DelayedTargetValidation] + #[SensitiveParameter] + $sensitive + ) { + throw new Exception('Testing backtrace'); + } + +} + +#[DelayedTargetValidation] +#[SensitiveParameter] // Does nothing here +function demoFn() { + echo __FUNCTION__ . "\n"; + return 456; +} + +#[DelayedTargetValidation] +#[SensitiveParameter] // Does nothing here +const GLOBAL_CONST = 'BAR'; + +$d = new DemoClass('example'); +var_dump($d->val); +var_dump(DemoClass::CLASS_CONST); +demoFn(); +var_dump(GLOBAL_CONST); + +$d->printVal('BAZ'); + + +?> +--EXPECTF-- +Got: example +string(7) "example" +string(3) "FOO" +demoFn +string(3) "BAR" + +Fatal error: Uncaught Exception: Testing backtrace in %s:%d +Stack trace: +#0 %s(%d): DemoClass->printVal(Object(SensitiveParameterValue)) +#1 {main} + thrown in %s on line %d diff --git a/Zend/zend_attributes.c b/Zend/zend_attributes.c index c9ee641f84f95..bc2e00deb5a30 100644 --- a/Zend/zend_attributes.c +++ b/Zend/zend_attributes.c @@ -73,6 +73,13 @@ uint32_t zend_attribute_attribute_get_flags(zend_attribute *attr, zend_class_ent static void validate_allow_dynamic_properties( zend_attribute *attr, uint32_t target, zend_class_entry *scope) { + if (scope == NULL) { + // Only reachable when validator is run but the attribute isn't applied + // to a class; in the case of delayed target validation reflection will + // complain about the target before running the validator; + ZEND_ASSERT(target & ZEND_ATTRIBUTE_NO_TARGET_VALIDATION); + return; + } const char *msg = NULL; if (scope->ce_flags & ZEND_ACC_TRAIT) { msg = "Cannot apply #[\\AllowDynamicProperties] to trait %s"; diff --git a/Zend/zend_compile.c b/Zend/zend_compile.c index 4ff4ed6dbd3cc..d095e0cf56243 100644 --- a/Zend/zend_compile.c +++ b/Zend/zend_compile.c @@ -7558,11 +7558,23 @@ static void zend_compile_attributes( if (*attributes != NULL) { /* Allow delaying target validation for forward compatibility. */ - zend_attribute *delayed_target_validation = zend_get_attribute_str( - *attributes, - "delayedtargetvalidation", - strlen("delayedtargetvalidation") - ); + zend_attribute *delayed_target_validation = NULL; + if (target == ZEND_ATTRIBUTE_TARGET_PARAMETER) { + ZEND_ASSERT(offset >= 1); + // zend_get_parameter_attribute_str will add 1 too + delayed_target_validation = zend_get_parameter_attribute_str( + *attributes, + "delayedtargetvalidation", + strlen("delayedtargetvalidation"), + offset - 1 + ); + } else { + delayed_target_validation = zend_get_attribute_str( + *attributes, + "delayedtargetvalidation", + strlen("delayedtargetvalidation") + ); + } uint32_t extra_flags = delayed_target_validation ? ZEND_ATTRIBUTE_NO_TARGET_VALIDATION : 0; /* Validate attributes in a secondary loop (needed to detect repeated attributes). */ ZEND_HASH_PACKED_FOREACH_PTR(*attributes, attr) { From 285af5467315dad4a5a4432758dbd797130f803e Mon Sep 17 00:00:00 2001 From: Daniel Scherzer Date: Wed, 9 Jul 2025 16:12:56 -0700 Subject: [PATCH 06/23] Missing comment --- .../delayed_target_validation/with_SensitiveParameter.phpt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Zend/tests/attributes/delayed_target_validation/with_SensitiveParameter.phpt b/Zend/tests/attributes/delayed_target_validation/with_SensitiveParameter.phpt index 440d47d42b38e..fb0ffb737a562 100644 --- a/Zend/tests/attributes/delayed_target_validation/with_SensitiveParameter.phpt +++ b/Zend/tests/attributes/delayed_target_validation/with_SensitiveParameter.phpt @@ -28,7 +28,7 @@ class DemoClass { public function printVal( #[DelayedTargetValidation] #[SensitiveParameter] - $sensitive + $sensitive // Does something here ) { throw new Exception('Testing backtrace'); } From e0e7b71e0ba38de0df9720237bd6e23b16c8d83a Mon Sep 17 00:00:00 2001 From: Daniel Scherzer Date: Fri, 25 Jul 2025 10:47:21 -0700 Subject: [PATCH 07/23] Tests for property hooks --- .../errors_from_validator.phpt | 17 ++++ .../has_runtime_errors.phpt | 85 ++++++++++++++++--- .../no_compile_errors.phpt | 15 +++- .../with_AllowDynamicProperties.phpt | 58 ++++++++----- .../with_Attribute.phpt | 56 +++++++----- .../with_Deprecated.phpt | 60 ++++++++----- .../with_NoDiscard.phpt | 58 ++++++++----- .../with_Override_error_get.phpt | 18 ++++ ...r.phpt => with_Override_error_method.phpt} | 14 +-- .../with_Override_error_set.phpt | 18 ++++ .../with_Override_okay.phpt | 68 +++++++++------ .../with_ReturnTypeWillChange.phpt | 70 ++++++++------- .../with_SensitiveParameter.phpt | 62 ++++++++------ 13 files changed, 409 insertions(+), 190 deletions(-) create mode 100644 Zend/tests/attributes/delayed_target_validation/with_Override_error_get.phpt rename Zend/tests/attributes/delayed_target_validation/{with_Override_error.phpt => with_Override_error_method.phpt} (58%) create mode 100644 Zend/tests/attributes/delayed_target_validation/with_Override_error_set.phpt diff --git a/Zend/tests/attributes/delayed_target_validation/errors_from_validator.phpt b/Zend/tests/attributes/delayed_target_validation/errors_from_validator.phpt index d0948e5fd7ad0..f14bda38158d3 100644 --- a/Zend/tests/attributes/delayed_target_validation/errors_from_validator.phpt +++ b/Zend/tests/attributes/delayed_target_validation/errors_from_validator.phpt @@ -19,11 +19,28 @@ readonly class DemoReadonly {} #[AllowDynamicProperties] enum DemoEnum {} +class DemoClass { + #[DelayedTargetValidation] + #[NoDiscard] // Does nothing here + public $val; + + public string $hooked { + #[DelayedTargetValidation] + // #[NoDiscard] // Does nothing here + get => $this->hooked; + #[DelayedTargetValidation] + // #[NoDiscard] // Does nothing here + set => $value; + } +} + $cases = [ new ReflectionClass('DemoTrait'), new ReflectionClass('DemoInterface'), new ReflectionClass('DemoReadonly'), new ReflectionClass('DemoEnum'), + // new ReflectionProperty('DemoClass', 'hooked')->getHook(PropertyHookType::Get), + // new ReflectionProperty('DemoClass', 'hooked')->getHook(PropertyHookType::Set), ]; foreach ($cases as $r) { echo str_repeat("*", 20) . "\n"; diff --git a/Zend/tests/attributes/delayed_target_validation/has_runtime_errors.phpt b/Zend/tests/attributes/delayed_target_validation/has_runtime_errors.phpt index bb65b126172fb..7a2f9d32c0b2b 100644 --- a/Zend/tests/attributes/delayed_target_validation/has_runtime_errors.phpt +++ b/Zend/tests/attributes/delayed_target_validation/has_runtime_errors.phpt @@ -13,16 +13,25 @@ class Demo { #[DelayedTargetValidation] #[Attribute] - public string $v; + public string $v1; + + public string $v2 { + #[DelayedTargetValidation] + #[Attribute] + get => $this->v2; + #[DelayedTargetValidation] + #[Attribute] + set => $value; + } #[DelayedTargetValidation] #[Attribute] public function __construct( #[DelayedTargetValidation] #[Attribute] - public string $v2 + public string $v3 ) { - $this->v = $v2; + $this->v1 = $v3; echo __METHOD__ . "\n"; } } @@ -40,10 +49,12 @@ const EXAMPLE = true; $cases = [ new ReflectionClass('Demo'), new ReflectionClassConstant('Demo', 'FOO'), - new ReflectionProperty('Demo', 'v'), + new ReflectionProperty('Demo', 'v1'), + new ReflectionProperty('Demo', 'v2')->getHook(PropertyHookType::Get), + new ReflectionProperty('Demo', 'v2')->getHook(PropertyHookType::Set), new ReflectionMethod('Demo', '__construct'), - new ReflectionParameter([ 'Demo', '__construct' ], 'v2'), - new ReflectionProperty('Demo', 'v2'), + new ReflectionParameter([ 'Demo', '__construct' ], 'v3'), + new ReflectionProperty('Demo', 'v3'), new ReflectionFunction('demoFn'), new ReflectionConstant('EXAMPLE'), ]; @@ -62,7 +73,7 @@ foreach ($cases as $r) { ?> --EXPECTF-- ******************** -Class [ class Demo ] { +Class [ class Demo ] { @@ %s %d-%d - Constants [1] { @@ -75,9 +86,10 @@ Class [ class Demo ] { - Static methods [0] { } - - Properties [2] { - Property [ public string $v ] + - Properties [3] { + Property [ public string $v1 ] Property [ public string $v2 ] + Property [ public string $v3 ] } - Methods [1] { @@ -85,7 +97,7 @@ Class [ class Demo ] { @@ %s %d - %d - Parameters [1] { - Parameter #0 [ string $v2 ] + Parameter #0 [ string $v3 ] } } } @@ -121,7 +133,7 @@ array(2) { } Error: Attribute "Attribute" cannot target class constant (allowed targets: class) ******************** -Property [ public string $v ] +Property [ public string $v1 ] array(2) { [0]=> @@ -137,11 +149,56 @@ array(2) { } Error: Attribute "Attribute" cannot target property (allowed targets: class) ******************** +Method [ public method $v2::get ] { + @@ %s %d - %d + + - Parameters [0] { + } + - Return [ string ] +} + +array(2) { + [0]=> + object(ReflectionAttribute)#%d (1) { + ["name"]=> + string(23) "DelayedTargetValidation" + } + [1]=> + object(ReflectionAttribute)#%d (1) { + ["name"]=> + string(9) "Attribute" + } +} +Error: Attribute "Attribute" cannot target method (allowed targets: class) +******************** +Method [ public method $v2::set ] { + @@ %s %d - %d + + - Parameters [1] { + Parameter #0 [ string $value ] + } + - Return [ void ] +} + +array(2) { + [0]=> + object(ReflectionAttribute)#%d (1) { + ["name"]=> + string(23) "DelayedTargetValidation" + } + [1]=> + object(ReflectionAttribute)#%d (1) { + ["name"]=> + string(9) "Attribute" + } +} +Error: Attribute "Attribute" cannot target method (allowed targets: class) +******************** Method [ public method __construct ] { @@ %s %d - %d - Parameters [1] { - Parameter #0 [ string $v2 ] + Parameter #0 [ string $v3 ] } } @@ -159,7 +216,7 @@ array(2) { } Error: Attribute "Attribute" cannot target method (allowed targets: class) ******************** -Parameter #0 [ string $v2 ] +Parameter #0 [ string $v3 ] array(2) { [0]=> object(ReflectionAttribute)#%d (1) { @@ -174,7 +231,7 @@ array(2) { } Error: Attribute "Attribute" cannot target parameter (allowed targets: class) ******************** -Property [ public string $v2 ] +Property [ public string $v3 ] array(2) { [0]=> diff --git a/Zend/tests/attributes/delayed_target_validation/no_compile_errors.phpt b/Zend/tests/attributes/delayed_target_validation/no_compile_errors.phpt index ea96a069d748c..b2e14a235f8a5 100644 --- a/Zend/tests/attributes/delayed_target_validation/no_compile_errors.phpt +++ b/Zend/tests/attributes/delayed_target_validation/no_compile_errors.phpt @@ -13,16 +13,25 @@ class Demo { #[DelayedTargetValidation] #[Attribute] - public string $v; + public string $v1; + + public string $v2 { + #[DelayedTargetValidation] + #[Attribute] + get => $this->v2; + #[DelayedTargetValidation] + #[Attribute] + set => $value; + } #[DelayedTargetValidation] #[Attribute] public function __construct( #[DelayedTargetValidation] #[Attribute] - public string $v2 + public string $v3 ) { - $this->v = $v2; + $this->v1 = $v3; echo __METHOD__ . "\n"; } } diff --git a/Zend/tests/attributes/delayed_target_validation/with_AllowDynamicProperties.phpt b/Zend/tests/attributes/delayed_target_validation/with_AllowDynamicProperties.phpt index b9154f37f73d0..83f738491c57e 100644 --- a/Zend/tests/attributes/delayed_target_validation/with_AllowDynamicProperties.phpt +++ b/Zend/tests/attributes/delayed_target_validation/with_AllowDynamicProperties.phpt @@ -6,36 +6,45 @@ #[DelayedTargetValidation] #[AllowDynamicProperties] // Does something here class DemoClass { - #[DelayedTargetValidation] - #[AllowDynamicProperties] // Does nothing here - public $val; + #[DelayedTargetValidation] + #[AllowDynamicProperties] // Does nothing here + public $val; - #[DelayedTargetValidation] - #[AllowDynamicProperties] // Does nothing here - public const CLASS_CONST = 'FOO'; + public string $hooked { + #[DelayedTargetValidation] + #[AllowDynamicProperties] // Does nothing here + get => $this->hooked; + #[DelayedTargetValidation] + #[AllowDynamicProperties] // Does nothing here + set => $value; + } - public function __construct( - #[DelayedTargetValidation] - #[AllowDynamicProperties] // Does nothing here - $str - ) { - echo "Got: $str\n"; - $this->val = $str; - } + #[DelayedTargetValidation] + #[AllowDynamicProperties] // Does nothing here + public const CLASS_CONST = 'FOO'; - #[DelayedTargetValidation] - #[AllowDynamicProperties] // Does nothing here - public function printVal() { - echo 'Value is: ' . $this->val . "\n"; - } + public function __construct( + #[DelayedTargetValidation] + #[AllowDynamicProperties] // Does nothing here + $str + ) { + echo "Got: $str\n"; + $this->val = $str; + } + + #[DelayedTargetValidation] + #[AllowDynamicProperties] // Does nothing here + public function printVal() { + echo 'Value is: ' . $this->val . "\n"; + } } #[DelayedTargetValidation] #[AllowDynamicProperties] // Does nothing here function demoFn() { - echo __FUNCTION__ . "\n"; - return 456; + echo __FUNCTION__ . "\n"; + return 456; } #[DelayedTargetValidation] @@ -45,6 +54,8 @@ const GLOBAL_CONST = 'BAR'; $d = new DemoClass('example'); $d->printVal(); var_dump($d->val); +$d->hooked = "foo"; +var_dump($d->hooked); var_dump(DemoClass::CLASS_CONST); demoFn(); var_dump(GLOBAL_CONST); @@ -56,12 +67,15 @@ var_dump($d); Got: example Value is: example string(7) "example" +string(3) "foo" string(3) "FOO" demoFn string(3) "BAR" -object(DemoClass)#%d (2) { +object(DemoClass)#%d (3) { ["val"]=> string(7) "example" + ["hooked"]=> + string(3) "foo" ["missingProp"]=> string(3) "foo" } diff --git a/Zend/tests/attributes/delayed_target_validation/with_Attribute.phpt b/Zend/tests/attributes/delayed_target_validation/with_Attribute.phpt index 13b33dca53ee8..edc7d2a7905fb 100644 --- a/Zend/tests/attributes/delayed_target_validation/with_Attribute.phpt +++ b/Zend/tests/attributes/delayed_target_validation/with_Attribute.phpt @@ -8,36 +8,45 @@ class NonAttribute {} #[DelayedTargetValidation] #[Attribute] // Does something here class DemoClass { - #[DelayedTargetValidation] - #[Attribute] // Does nothing here - public $val; + #[DelayedTargetValidation] + #[Attribute] // Does nothing here + public $val; + + public string $hooked { + #[DelayedTargetValidation] + #[Attribute] // Does nothing here + get => $this->hooked; + #[DelayedTargetValidation] + #[Attribute] // Does nothing here + set => $value; + } - #[DelayedTargetValidation] - #[Attribute] // Does nothing here - public const CLASS_CONST = 'FOO'; + #[DelayedTargetValidation] + #[Attribute] // Does nothing here + public const CLASS_CONST = 'FOO'; - public function __construct( - #[DelayedTargetValidation] - #[Attribute] // Does nothing here - $str - ) { - echo "Got: $str\n"; - $this->val = $str; - } + public function __construct( + #[DelayedTargetValidation] + #[Attribute] // Does nothing here + $str + ) { + echo "Got: $str\n"; + $this->val = $str; + } - #[DelayedTargetValidation] - #[Attribute] // Does nothing here - public function printVal() { - echo 'Value is: ' . $this->val . "\n"; - } + #[DelayedTargetValidation] + #[Attribute] // Does nothing here + public function printVal() { + echo 'Value is: ' . $this->val . "\n"; + } } #[DelayedTargetValidation] #[Attribute] // Does nothing here function demoFn() { - echo __FUNCTION__ . "\n"; - return 456; + echo __FUNCTION__ . "\n"; + return 456; } #[DelayedTargetValidation] @@ -47,6 +56,8 @@ const GLOBAL_CONST = 'BAR'; $d = new DemoClass('example'); $d->printVal(); var_dump($d->val); +$d->hooked = "foo"; +var_dump($d->hooked); var_dump(DemoClass::CLASS_CONST); demoFn(); var_dump(GLOBAL_CONST); @@ -65,6 +76,7 @@ var_dump($attribs[1]->newInstance()); Got: example Value is: example string(7) "example" +string(3) "foo" string(3) "FOO" demoFn string(3) "BAR" @@ -72,6 +84,8 @@ Got: BAZ object(DemoClass)#5 (1) { ["val"]=> string(3) "BAZ" + ["hooked"]=> + uninitialized(string) } Fatal error: Uncaught Error: Attempting to use non-attribute class "NonAttribute" as attribute in %s:%d diff --git a/Zend/tests/attributes/delayed_target_validation/with_Deprecated.phpt b/Zend/tests/attributes/delayed_target_validation/with_Deprecated.phpt index e0d3d4c4ae592..093b0abb08e0c 100644 --- a/Zend/tests/attributes/delayed_target_validation/with_Deprecated.phpt +++ b/Zend/tests/attributes/delayed_target_validation/with_Deprecated.phpt @@ -6,36 +6,45 @@ #[DelayedTargetValidation] #[Deprecated] // Does nothing here class DemoClass { - #[DelayedTargetValidation] - #[Deprecated] // Does nothing here - public $val; + #[DelayedTargetValidation] + #[Deprecated] // Does nothing here + public $val; - #[DelayedTargetValidation] - #[Deprecated] // Does something here - public const CLASS_CONST = 'FOO'; + public string $hooked { + #[DelayedTargetValidation] + #[Deprecated] // Does something here + get => $this->hooked; + #[DelayedTargetValidation] + #[Deprecated] // Does something here + set => $value; + } - public function __construct( - #[DelayedTargetValidation] - #[Deprecated] // Does nothing here - $str - ) { - echo "Got: $str\n"; - $this->val = $str; - } + #[DelayedTargetValidation] + #[Deprecated] // Does something here + public const CLASS_CONST = 'FOO'; - #[DelayedTargetValidation] - #[Deprecated] // Does something here - public function printVal() { - echo 'Value is: ' . $this->val . "\n"; - return 123; - } + public function __construct( + #[DelayedTargetValidation] + #[Deprecated] // Does nothing here + $str + ) { + echo "Got: $str\n"; + $this->val = $str; + } + + #[DelayedTargetValidation] + #[Deprecated] // Does something here + public function printVal() { + echo 'Value is: ' . $this->val . "\n"; + return 123; + } } #[DelayedTargetValidation] #[Deprecated] // Does something here function demoFn() { - echo __FUNCTION__ . "\n"; - return 456; + echo __FUNCTION__ . "\n"; + return 456; } #[DelayedTargetValidation] @@ -45,6 +54,8 @@ const GLOBAL_CONST = 'BAR'; $d = new DemoClass('example'); $d->printVal(); var_dump($d->val); +$d->hooked = "foo"; +var_dump($d->hooked); var_dump(DemoClass::CLASS_CONST); demoFn(); var_dump(GLOBAL_CONST); @@ -56,6 +67,11 @@ Deprecated: Method DemoClass::printVal() is deprecated in %s on line %d Value is: example string(7) "example" +Deprecated: Method DemoClass::$hooked::set() is deprecated in %s on line %d + +Deprecated: Method DemoClass::$hooked::get() is deprecated in %s on line %d +string(3) "foo" + Deprecated: Constant DemoClass::CLASS_CONST is deprecated in %s on line %d string(3) "FOO" diff --git a/Zend/tests/attributes/delayed_target_validation/with_NoDiscard.phpt b/Zend/tests/attributes/delayed_target_validation/with_NoDiscard.phpt index affc3a691deaf..0ffe5cf78763e 100644 --- a/Zend/tests/attributes/delayed_target_validation/with_NoDiscard.phpt +++ b/Zend/tests/attributes/delayed_target_validation/with_NoDiscard.phpt @@ -6,36 +6,45 @@ #[DelayedTargetValidation] #[NoDiscard] // Does nothing here class DemoClass { - #[DelayedTargetValidation] - #[NoDiscard] // Does nothing here - public $val; + #[DelayedTargetValidation] + #[NoDiscard] // Does nothing here + public $val; - #[DelayedTargetValidation] - #[NoDiscard] // Does nothing here - public const CLASS_CONST = 'FOO'; + public string $hooked { + #[DelayedTargetValidation] + // #[NoDiscard] // Does nothing here + get => $this->hooked; + #[DelayedTargetValidation] + // #[NoDiscard] // Does nothing here + set => $value; + } - public function __construct( - #[DelayedTargetValidation] - #[NoDiscard] // Does nothing here - $str - ) { - echo "Got: $str\n"; - $this->val = $str; - } + #[DelayedTargetValidation] + #[NoDiscard] // Does nothing here + public const CLASS_CONST = 'FOO'; - #[DelayedTargetValidation] - #[NoDiscard] // Does something here - public function printVal() { - echo 'Value is: ' . $this->val . "\n"; - return 123; - } + public function __construct( + #[DelayedTargetValidation] + #[NoDiscard] // Does nothing here + $str + ) { + echo "Got: $str\n"; + $this->val = $str; + } + + #[DelayedTargetValidation] + #[NoDiscard] // Does something here + public function printVal() { + echo 'Value is: ' . $this->val . "\n"; + return 123; + } } #[DelayedTargetValidation] #[NoDiscard] // Does something here function demoFn() { - echo __FUNCTION__ . "\n"; - return 456; + echo __FUNCTION__ . "\n"; + return 456; } #[DelayedTargetValidation] @@ -46,6 +55,10 @@ $d = new DemoClass('example'); $d->printVal(); $v = $d->printVal(); var_dump($d->val); +$d->hooked = "foo"; +var_dump($d->hooked); +// NODiscard does not support property hooks, this should not complain +$d->hooked; var_dump(DemoClass::CLASS_CONST); demoFn(); $v = demoFn(); @@ -58,6 +71,7 @@ Warning: The return value of method DemoClass::printVal() should either be used Value is: example Value is: example string(7) "example" +string(3) "foo" string(3) "FOO" Warning: The return value of function demoFn() should either be used or intentionally ignored by casting it as (void) in %s on line %d diff --git a/Zend/tests/attributes/delayed_target_validation/with_Override_error_get.phpt b/Zend/tests/attributes/delayed_target_validation/with_Override_error_get.phpt new file mode 100644 index 0000000000000..a33e83d517a30 --- /dev/null +++ b/Zend/tests/attributes/delayed_target_validation/with_Override_error_get.phpt @@ -0,0 +1,18 @@ +--TEST-- +#[\DelayedTargetValidation] with #[\Override]: non-overrides still error (get hook) +--FILE-- + $this->hooked; + set => $value; + } +} + +?> +--EXPECTF-- +Fatal error: DemoClass::$hooked::get() has #[\Override] attribute, but no matching parent method exists in %s on line %d diff --git a/Zend/tests/attributes/delayed_target_validation/with_Override_error.phpt b/Zend/tests/attributes/delayed_target_validation/with_Override_error_method.phpt similarity index 58% rename from Zend/tests/attributes/delayed_target_validation/with_Override_error.phpt rename to Zend/tests/attributes/delayed_target_validation/with_Override_error_method.phpt index 46b05713917b0..ecca1daff0fd3 100644 --- a/Zend/tests/attributes/delayed_target_validation/with_Override_error.phpt +++ b/Zend/tests/attributes/delayed_target_validation/with_Override_error_method.phpt @@ -1,16 +1,16 @@ --TEST-- -#[\DelayedTargetValidation] with #[\Override]: non-overrides still error +#[\DelayedTargetValidation] with #[\Override]: non-overrides still error (method) --FILE-- val . "\n"; - return 123; - } + #[DelayedTargetValidation] + #[Override] // Does something here + public function printVal() { + echo 'Value is: ' . $this->val . "\n"; + return 123; + } } ?> diff --git a/Zend/tests/attributes/delayed_target_validation/with_Override_error_set.phpt b/Zend/tests/attributes/delayed_target_validation/with_Override_error_set.phpt new file mode 100644 index 0000000000000..d99580646c548 --- /dev/null +++ b/Zend/tests/attributes/delayed_target_validation/with_Override_error_set.phpt @@ -0,0 +1,18 @@ +--TEST-- +#[\DelayedTargetValidation] with #[\Override]: non-overrides still error (set hook) +--FILE-- + $this->hooked; + #[DelayedTargetValidation] + #[Override] // Does something here + set => $value; + } +} + +?> +--EXPECTF-- +Fatal error: DemoClass::$hooked::set() has #[\Override] attribute, but no matching parent method exists in %s on line %d diff --git a/Zend/tests/attributes/delayed_target_validation/with_Override_okay.phpt b/Zend/tests/attributes/delayed_target_validation/with_Override_okay.phpt index 33e171e1be156..505a4ddb9e003 100644 --- a/Zend/tests/attributes/delayed_target_validation/with_Override_okay.phpt +++ b/Zend/tests/attributes/delayed_target_validation/with_Override_okay.phpt @@ -4,44 +4,59 @@ $this->hooked; + set => $value; + } + + public function printVal() { + echo __METHOD__ . "\n"; + } } #[DelayedTargetValidation] #[Override] // Does nothing here class DemoClass extends Base { - #[DelayedTargetValidation] - #[Override] // Does nothing here - public $val; + #[DelayedTargetValidation] + #[Override] // Does nothing here + public $val; + + public string $hooked { + #[DelayedTargetValidation] + #[Override] // Does something here + get => $this->hooked; + #[DelayedTargetValidation] + #[Override] // Does something here + set => $value; + } - #[DelayedTargetValidation] - #[Override] // Does nothing here - public const CLASS_CONST = 'FOO'; + #[DelayedTargetValidation] + #[Override] // Does nothing here + public const CLASS_CONST = 'FOO'; - public function __construct( - #[DelayedTargetValidation] - #[Override] // Does nothing here - $str - ) { - echo "Got: $str\n"; - $this->val = $str; - } + public function __construct( + #[DelayedTargetValidation] + #[Override] // Does nothing here + $str + ) { + echo "Got: $str\n"; + $this->val = $str; + } - #[DelayedTargetValidation] - #[Override] // Does something here - public function printVal() { - echo 'Value is: ' . $this->val . "\n"; - return 123; - } + #[DelayedTargetValidation] + #[Override] // Does something here + public function printVal() { + echo 'Value is: ' . $this->val . "\n"; + return 123; + } } #[DelayedTargetValidation] #[Override] // Does nothing here function demoFn() { - echo __FUNCTION__ . "\n"; - return 456; + echo __FUNCTION__ . "\n"; + return 456; } #[DelayedTargetValidation] @@ -51,6 +66,8 @@ const GLOBAL_CONST = 'BAR'; $d = new DemoClass('example'); $d->printVal(); var_dump($d->val); +$d->hooked = "foo"; +var_dump($d->hooked); var_dump(DemoClass::CLASS_CONST); demoFn(); var_dump(GLOBAL_CONST); @@ -59,6 +76,7 @@ var_dump(GLOBAL_CONST); Got: example Value is: example string(7) "example" +string(3) "foo" string(3) "FOO" demoFn string(3) "BAR" diff --git a/Zend/tests/attributes/delayed_target_validation/with_ReturnTypeWillChange.phpt b/Zend/tests/attributes/delayed_target_validation/with_ReturnTypeWillChange.phpt index 0cc3bb4a4b9a4..c562075cd6c05 100644 --- a/Zend/tests/attributes/delayed_target_validation/with_ReturnTypeWillChange.phpt +++ b/Zend/tests/attributes/delayed_target_validation/with_ReturnTypeWillChange.phpt @@ -4,49 +4,58 @@ $this->hooked; + #[DelayedTargetValidation] + #[ReturnTypeWillChange] // Does nothing here + set => $value; + } - public function __construct( - #[DelayedTargetValidation] - #[ReturnTypeWillChange] // Does nothing here - $str - ) { - echo "Got: $str\n"; - $this->val = $str; - } + #[DelayedTargetValidation] + #[ReturnTypeWillChange] // Does nothing here + public const CLASS_CONST = 'FOO'; - #[DelayedTargetValidation] - #[ReturnTypeWillChange] // Does something here - public function printVal() { - echo 'Value is: ' . $this->val . "\n"; - } + public function __construct( + #[DelayedTargetValidation] + #[ReturnTypeWillChange] // Does nothing here + $str + ) { + echo "Got: $str\n"; + $this->val = $str; + } - #[DelayedTargetValidation] - #[ReturnTypeWillChange] // Does something here - public function count() { - return 5; - } + #[DelayedTargetValidation] + #[ReturnTypeWillChange] // Does something here + public function printVal() { + echo 'Value is: ' . $this->val . "\n"; + } + + #[DelayedTargetValidation] + #[ReturnTypeWillChange] // Does something here + public function count() { + return 5; + } } #[DelayedTargetValidation] #[ReturnTypeWillChange] // Does nothing here function demoFn() { - echo __FUNCTION__ . "\n"; - return 456; + echo __FUNCTION__ . "\n"; + return 456; } #[DelayedTargetValidation] @@ -56,6 +65,8 @@ const GLOBAL_CONST = 'BAR'; $d = new DemoClass('example'); $d->printVal(); var_dump($d->val); +$d->hooked = "foo"; +var_dump($d->hooked); var_dump(DemoClass::CLASS_CONST); demoFn(); var_dump(GLOBAL_CONST); @@ -65,6 +76,7 @@ Deprecated: Return type of WithoutAttrib::count() should either be compatible wi Got: example Value is: example string(7) "example" +string(3) "foo" string(3) "FOO" demoFn string(3) "BAR" diff --git a/Zend/tests/attributes/delayed_target_validation/with_SensitiveParameter.phpt b/Zend/tests/attributes/delayed_target_validation/with_SensitiveParameter.phpt index fb0ffb737a562..270c031f6bbf0 100644 --- a/Zend/tests/attributes/delayed_target_validation/with_SensitiveParameter.phpt +++ b/Zend/tests/attributes/delayed_target_validation/with_SensitiveParameter.phpt @@ -6,40 +6,49 @@ #[DelayedTargetValidation] #[SensitiveParameter] // Does nothing here class DemoClass { - #[DelayedTargetValidation] - #[SensitiveParameter] // Does nothing here - public $val; + #[DelayedTargetValidation] + #[SensitiveParameter] // Does nothing here + public $val; - #[DelayedTargetValidation] - #[SensitiveParameter] // Does nothing here - public const CLASS_CONST = 'FOO'; + public string $hooked { + #[DelayedTargetValidation] + #[SensitiveParameter] // Does nothing here + get => $this->hooked; + #[DelayedTargetValidation] + #[SensitiveParameter] // Does nothing here + set => $value; + } - public function __construct( - #[DelayedTargetValidation] - #[SensitiveParameter] // Does something here - $str - ) { - echo "Got: $str\n"; - $this->val = $str; - } + #[DelayedTargetValidation] + #[SensitiveParameter] // Does nothing here + public const CLASS_CONST = 'FOO'; - #[DelayedTargetValidation] - #[SensitiveParameter] // Does nothing here - public function printVal( - #[DelayedTargetValidation] - #[SensitiveParameter] - $sensitive // Does something here - ) { - throw new Exception('Testing backtrace'); - } + public function __construct( + #[DelayedTargetValidation] + #[SensitiveParameter] // Does something here + $str + ) { + echo "Got: $str\n"; + $this->val = $str; + } + + #[DelayedTargetValidation] + #[SensitiveParameter] // Does nothing here + public function printVal( + #[DelayedTargetValidation] + #[SensitiveParameter] + $sensitive // Does something here + ) { + throw new Exception('Testing backtrace'); + } } #[DelayedTargetValidation] #[SensitiveParameter] // Does nothing here function demoFn() { - echo __FUNCTION__ . "\n"; - return 456; + echo __FUNCTION__ . "\n"; + return 456; } #[DelayedTargetValidation] @@ -48,6 +57,8 @@ const GLOBAL_CONST = 'BAR'; $d = new DemoClass('example'); var_dump($d->val); +$d->hooked = "foo"; +var_dump($d->hooked); var_dump(DemoClass::CLASS_CONST); demoFn(); var_dump(GLOBAL_CONST); @@ -59,6 +70,7 @@ $d->printVal('BAZ'); --EXPECTF-- Got: example string(7) "example" +string(3) "foo" string(3) "FOO" demoFn string(3) "BAR" From 2935829c107cbd4aea0c8e23cc695baaad7eb3ed Mon Sep 17 00:00:00 2001 From: Daniel Scherzer Date: Fri, 25 Jul 2025 11:13:05 -0700 Subject: [PATCH 08/23] Delay validation of #[\NoDiscard] on property hooks --- .../errors_from_validator.phpt | 53 +++++++++++++++++-- .../with_NoDiscard.phpt | 4 +- Zend/zend_attributes.c | 26 +++++++++ Zend/zend_compile.c | 35 ++++++------ ext/reflection/php_reflection.c | 47 +++++++++++----- 5 files changed, 127 insertions(+), 38 deletions(-) diff --git a/Zend/tests/attributes/delayed_target_validation/errors_from_validator.phpt b/Zend/tests/attributes/delayed_target_validation/errors_from_validator.phpt index f14bda38158d3..9759f29e3336e 100644 --- a/Zend/tests/attributes/delayed_target_validation/errors_from_validator.phpt +++ b/Zend/tests/attributes/delayed_target_validation/errors_from_validator.phpt @@ -26,10 +26,10 @@ class DemoClass { public string $hooked { #[DelayedTargetValidation] - // #[NoDiscard] // Does nothing here + #[NoDiscard] // Does nothing here get => $this->hooked; #[DelayedTargetValidation] - // #[NoDiscard] // Does nothing here + #[NoDiscard] // Does nothing here set => $value; } } @@ -39,8 +39,8 @@ $cases = [ new ReflectionClass('DemoInterface'), new ReflectionClass('DemoReadonly'), new ReflectionClass('DemoEnum'), - // new ReflectionProperty('DemoClass', 'hooked')->getHook(PropertyHookType::Get), - // new ReflectionProperty('DemoClass', 'hooked')->getHook(PropertyHookType::Set), + new ReflectionProperty('DemoClass', 'hooked')->getHook(PropertyHookType::Get), + new ReflectionProperty('DemoClass', 'hooked')->getHook(PropertyHookType::Set), ]; foreach ($cases as $r) { echo str_repeat("*", 20) . "\n"; @@ -195,3 +195,48 @@ array(2) { } } Error: Cannot apply #[AllowDynamicProperties] to enum DemoEnum +******************** +Method [ public method $hooked::get ] { + @@ %s %d - %d + + - Parameters [0] { + } + - Return [ string ] +} + +array(2) { + [0]=> + object(ReflectionAttribute)#%d (1) { + ["name"]=> + string(23) "DelayedTargetValidation" + } + [1]=> + object(ReflectionAttribute)#%d (1) { + ["name"]=> + string(9) "NoDiscard" + } +} +Error: #[\NoDiscard] is not supported for property hooks +******************** +Method [ public method $hooked::set ] { + @@ %s %d - %d + + - Parameters [1] { + Parameter #0 [ string $value ] + } + - Return [ void ] +} + +array(2) { + [0]=> + object(ReflectionAttribute)#%d (1) { + ["name"]=> + string(23) "DelayedTargetValidation" + } + [1]=> + object(ReflectionAttribute)#%d (1) { + ["name"]=> + string(9) "NoDiscard" + } +} +Error: #[\NoDiscard] is not supported for property hooks diff --git a/Zend/tests/attributes/delayed_target_validation/with_NoDiscard.phpt b/Zend/tests/attributes/delayed_target_validation/with_NoDiscard.phpt index 0ffe5cf78763e..17baca1800512 100644 --- a/Zend/tests/attributes/delayed_target_validation/with_NoDiscard.phpt +++ b/Zend/tests/attributes/delayed_target_validation/with_NoDiscard.phpt @@ -12,10 +12,10 @@ class DemoClass { public string $hooked { #[DelayedTargetValidation] - // #[NoDiscard] // Does nothing here + #[NoDiscard] // Does nothing here get => $this->hooked; #[DelayedTargetValidation] - // #[NoDiscard] // Does nothing here + #[NoDiscard] // Does nothing here set => $value; } diff --git a/Zend/zend_attributes.c b/Zend/zend_attributes.c index bc2e00deb5a30..3dc9aa01dbab1 100644 --- a/Zend/zend_attributes.c +++ b/Zend/zend_attributes.c @@ -228,6 +228,31 @@ ZEND_METHOD(Deprecated, __construct) } } +static void validate_nodiscard( + zend_attribute *attr, uint32_t target, zend_class_entry *scope) +{ + /* There isn't an easy way to identify the *method* that the attribute is + * applied to in a manner that works during both compilation (normal + * validation) and runtime (delayed validation). So, handle them separately. + */ + if (CG(in_compilation)) { + ZEND_ASSERT((target & ZEND_ATTRIBUTE_DELAYED_TARGET_VALIDATION) == 0); + zend_op_array *op_array = CG(active_op_array); + const zend_string *prop_info_name = CG(context).active_property_info_name; + if (prop_info_name == NULL) { + op_array->fn_flags |= ZEND_ACC_NODISCARD; + return; + } + // Applied to a hook, either throw or ignore + if (target & ZEND_ATTRIBUTE_NO_TARGET_VALIDATION) { + return; + } + zend_error_noreturn(E_COMPILE_ERROR, "#[\\NoDiscard] is not supported for property hooks"); + } + /* At runtime, no way to identify the target method; Reflection will handle + * throwing the error if needed. */ +} + ZEND_METHOD(NoDiscard, __construct) { zend_string *message = NULL; @@ -588,6 +613,7 @@ void zend_register_attribute_ce(void) zend_ce_nodiscard = register_class_NoDiscard(); attr = zend_mark_internal_attribute(zend_ce_nodiscard); + attr->validator = validate_nodiscard; zend_ce_delayed_target_validation = register_class_DelayedTargetValidation(); attr = zend_mark_internal_attribute(zend_ce_delayed_target_validation); diff --git a/Zend/zend_compile.c b/Zend/zend_compile.c index d095e0cf56243..34784ee9502bd 100644 --- a/Zend/zend_compile.c +++ b/Zend/zend_compile.c @@ -7582,8 +7582,9 @@ static void zend_compile_attributes( continue; } - if (delayed_target_validation == NULL) { - if (!(target & (config->flags & ZEND_ATTRIBUTE_TARGET_ALL))) { + bool run_validator = true; + if (!(target & (config->flags & ZEND_ATTRIBUTE_TARGET_ALL))) { + if (delayed_target_validation == NULL) { zend_string *location = zend_get_attribute_target_names(target); zend_string *allowed = zend_get_attribute_target_names(config->flags); @@ -7591,6 +7592,7 @@ static void zend_compile_attributes( ZSTR_VAL(attr->name), ZSTR_VAL(location), ZSTR_VAL(allowed) ); } + run_validator = false; } if (!(config->flags & ZEND_ATTRIBUTE_IS_REPEATABLE)) { @@ -7599,7 +7601,8 @@ static void zend_compile_attributes( } } - if (config->validator != NULL) { + // Validators are not run if the target is already invalid + if (run_validator && config->validator != NULL) { config->validator(attr, target | extra_flags, CG(active_class_entry)); } } ZEND_HASH_FOREACH_END(); @@ -8446,6 +8449,10 @@ static zend_op_array *zend_compile_func_decl_ex( CG(active_op_array) = op_array; + zend_oparray_context_begin(&orig_oparray_context, op_array); + CG(context).active_property_info_name = property_info_name; + CG(context).active_property_hook_kind = hook_kind; + if (decl->child[4]) { int target = ZEND_ATTRIBUTE_TARGET_FUNCTION; @@ -8475,15 +8482,7 @@ static zend_op_array *zend_compile_func_decl_ex( op_array->fn_flags |= ZEND_ACC_DEPRECATED; } - zend_attribute *nodiscard_attribute = zend_get_attribute_str( - op_array->attributes, - "nodiscard", - sizeof("nodiscard")-1 - ); - - if (nodiscard_attribute) { - op_array->fn_flags |= ZEND_ACC_NODISCARD; - } + // ZEND_ACC_NODISCARD is added via an attribute validator } /* Do not leak the class scope into free standing functions, even if they are dynamically @@ -8497,10 +8496,6 @@ static zend_op_array *zend_compile_func_decl_ex( op_array->fn_flags |= ZEND_ACC_TOP_LEVEL; } - zend_oparray_context_begin(&orig_oparray_context, op_array); - CG(context).active_property_info_name = property_info_name; - CG(context).active_property_hook_kind = hook_kind; - { /* Push a separator to the loop variable stack */ zend_loop_var dummy_var; @@ -8535,9 +8530,11 @@ static zend_op_array *zend_compile_func_decl_ex( } if (op_array->fn_flags & ZEND_ACC_NODISCARD) { - if (is_hook) { - zend_error_noreturn(E_COMPILE_ERROR, "#[\\NoDiscard] is not supported for property hooks"); - } + /* ZEND_ACC_NODISCARD gets added by the attribute validator, but only + * if the method is not a hook; if it is a hook, then the validator + * should have either thrown an error or done nothing due to delayed + * target validation. */ + ZEND_ASSERT(!is_hook); if (op_array->fn_flags & ZEND_ACC_HAS_RETURN_TYPE) { zend_arg_info *return_info = CG(active_op_array)->arg_info - 1; diff --git a/ext/reflection/php_reflection.c b/ext/reflection/php_reflection.c index 55374a676d157..a96e958b98bf7 100644 --- a/ext/reflection/php_reflection.c +++ b/ext/reflection/php_reflection.c @@ -157,6 +157,7 @@ typedef struct _attribute_reference { zend_class_entry *scope; zend_string *filename; uint32_t target; + bool on_property_hook; } attribute_reference; typedef enum { @@ -1254,7 +1255,7 @@ static void _extension_string(smart_str *str, const zend_module_entry *module, c /* {{{ reflection_attribute_factory */ static void reflection_attribute_factory(zval *object, HashTable *attributes, zend_attribute *data, - zend_class_entry *scope, uint32_t target, zend_string *filename) + zend_class_entry *scope, uint32_t target, zend_string *filename, bool on_property_hook) { reflection_object *intern; attribute_reference *reference; @@ -1267,6 +1268,7 @@ static void reflection_attribute_factory(zval *object, HashTable *attributes, ze reference->scope = scope; reference->filename = filename ? zend_string_copy(filename) : NULL; reference->target = target; + reference->on_property_hook = on_property_hook; intern->ptr = reference; intern->ref_type = REF_TYPE_ATTRIBUTE; ZVAL_STR_COPY(reflection_prop_name(object), data->name); @@ -1274,7 +1276,8 @@ static void reflection_attribute_factory(zval *object, HashTable *attributes, ze /* }}} */ static int read_attributes(zval *ret, HashTable *attributes, zend_class_entry *scope, - uint32_t offset, uint32_t target, zend_string *name, zend_class_entry *base, zend_string *filename) /* {{{ */ + uint32_t offset, uint32_t target, zend_string *name, zend_class_entry *base, zend_string *filename, + bool on_property_hook) /* {{{ */ { ZEND_ASSERT(attributes != NULL); @@ -1287,7 +1290,7 @@ static int read_attributes(zval *ret, HashTable *attributes, zend_class_entry *s ZEND_HASH_PACKED_FOREACH_PTR(attributes, attr) { if (attr->offset == offset && zend_string_equals(attr->lcname, filter)) { - reflection_attribute_factory(&tmp, attributes, attr, scope, target, filename); + reflection_attribute_factory(&tmp, attributes, attr, scope, target, filename, on_property_hook); add_next_index_zval(ret, &tmp); } } ZEND_HASH_FOREACH_END(); @@ -1319,7 +1322,7 @@ static int read_attributes(zval *ret, HashTable *attributes, zend_class_entry *s } } - reflection_attribute_factory(&tmp, attributes, attr, scope, target, filename); + reflection_attribute_factory(&tmp, attributes, attr, scope, target, filename, on_property_hook); add_next_index_zval(ret, &tmp); } ZEND_HASH_FOREACH_END(); @@ -1328,7 +1331,7 @@ static int read_attributes(zval *ret, HashTable *attributes, zend_class_entry *s /* }}} */ static void reflect_attributes(INTERNAL_FUNCTION_PARAMETERS, HashTable *attributes, - uint32_t offset, zend_class_entry *scope, uint32_t target, zend_string *filename) /* {{{ */ + uint32_t offset, zend_class_entry *scope, uint32_t target, zend_string *filename, bool on_property_hook) /* {{{ */ { zend_string *name = NULL; zend_long flags = 0; @@ -1361,7 +1364,7 @@ static void reflect_attributes(INTERNAL_FUNCTION_PARAMETERS, HashTable *attribut array_init(return_value); - if (FAILURE == read_attributes(return_value, attributes, scope, offset, target, name, base, filename)) { + if (FAILURE == read_attributes(return_value, attributes, scope, offset, target, name, base, filename, on_property_hook)) { RETURN_THROWS(); } } @@ -2066,15 +2069,21 @@ ZEND_METHOD(ReflectionFunctionAbstract, getAttributes) GET_REFLECTION_OBJECT_PTR(fptr); + bool on_property_hook = false; + if (fptr->common.scope && (fptr->common.fn_flags & (ZEND_ACC_CLOSURE|ZEND_ACC_FAKE_CLOSURE)) != ZEND_ACC_CLOSURE) { target = ZEND_ATTRIBUTE_TARGET_METHOD; + if (fptr->common.prop_info != NULL) { + on_property_hook = true; + } } else { target = ZEND_ATTRIBUTE_TARGET_FUNCTION; } reflect_attributes(INTERNAL_FUNCTION_PARAM_PASSTHRU, fptr->common.attributes, 0, fptr->common.scope, target, - fptr->type == ZEND_USER_FUNCTION ? fptr->op_array.filename : NULL); + fptr->type == ZEND_USER_FUNCTION ? fptr->op_array.filename : NULL, + on_property_hook); } /* }}} */ @@ -2916,7 +2925,8 @@ ZEND_METHOD(ReflectionParameter, getAttributes) reflect_attributes(INTERNAL_FUNCTION_PARAM_PASSTHRU, attributes, param->offset + 1, scope, ZEND_ATTRIBUTE_TARGET_PARAMETER, - param->fptr->type == ZEND_USER_FUNCTION ? param->fptr->op_array.filename : NULL); + param->fptr->type == ZEND_USER_FUNCTION ? param->fptr->op_array.filename : NULL, + false); } /* {{{ Returns the index of the parameter, starting from 0 */ @@ -4038,7 +4048,8 @@ ZEND_METHOD(ReflectionClassConstant, getAttributes) reflect_attributes(INTERNAL_FUNCTION_PARAM_PASSTHRU, ref->attributes, 0, ref->ce, ZEND_ATTRIBUTE_TARGET_CLASS_CONST, - ref->ce->type == ZEND_USER_CLASS ? ref->ce->info.user.filename : NULL); + ref->ce->type == ZEND_USER_CLASS ? ref->ce->info.user.filename : NULL, + false); } /* }}} */ @@ -4443,7 +4454,8 @@ ZEND_METHOD(ReflectionClass, getAttributes) reflect_attributes(INTERNAL_FUNCTION_PARAM_PASSTHRU, ce->attributes, 0, ce, ZEND_ATTRIBUTE_TARGET_CLASS, - ce->type == ZEND_USER_CLASS ? ce->info.user.filename : NULL); + ce->type == ZEND_USER_CLASS ? ce->info.user.filename : NULL, + false); } /* }}} */ @@ -6390,7 +6402,8 @@ ZEND_METHOD(ReflectionProperty, getAttributes) reflect_attributes(INTERNAL_FUNCTION_PARAM_PASSTHRU, ref->prop->attributes, 0, ref->prop->ce, ZEND_ATTRIBUTE_TARGET_PROPERTY, - ref->prop->ce->type == ZEND_USER_CLASS ? ref->prop->ce->info.user.filename : NULL); + ref->prop->ce->type == ZEND_USER_CLASS ? ref->prop->ce->info.user.filename : NULL, + false); } /* }}} */ @@ -7364,13 +7377,21 @@ ZEND_METHOD(ReflectionAttribute, newInstance) if (config != NULL && config->validator != NULL) { config->validator( attr->data, - flags | ZEND_ATTRIBUTE_DELAYED_TARGET_VALIDATION, + attr->target | ZEND_ATTRIBUTE_DELAYED_TARGET_VALIDATION, attr->scope ); if (EG(exception)) { RETURN_THROWS(); } } + /* For #[NoDiscard], the attribute does not work on property hooks, but + * at runtime the validator has no way to access the method that an + * attribute is applied to, attr->scope is just the overall class entry + * that the method is a part of. */ + if (ce == zend_ce_nodiscard && attr->on_property_hook) { + zend_throw_error(NULL, "#[\\NoDiscard] is not supported for property hooks");; + RETURN_THROWS(); + } } /* Repetition validation is done even if #[DelayedTargetValidation] is used @@ -7902,7 +7923,7 @@ ZEND_METHOD(ReflectionConstant, getAttributes) reflect_attributes(INTERNAL_FUNCTION_PARAM_PASSTHRU, const_->attributes, 0, NULL, ZEND_ATTRIBUTE_TARGET_CONST, - const_->filename); + const_->filename, false); } ZEND_METHOD(ReflectionConstant, __toString) From 4e867ee9b43e17071e1c983ca72a0dfc89be0a5f Mon Sep 17 00:00:00 2001 From: Daniel Scherzer Date: Fri, 1 Aug 2025 07:37:54 -0700 Subject: [PATCH 09/23] Update tests; tweak comments --- .../errors_from_validator.phpt | 12 ++++-------- .../has_runtime_errors.phpt | 2 +- .../delayed_target_validation/with_NoDiscard.phpt | 2 +- Zend/zend_attributes.c | 9 ++++----- 4 files changed, 10 insertions(+), 15 deletions(-) diff --git a/Zend/tests/attributes/delayed_target_validation/errors_from_validator.phpt b/Zend/tests/attributes/delayed_target_validation/errors_from_validator.phpt index 9759f29e3336e..1da43ea89c8c3 100644 --- a/Zend/tests/attributes/delayed_target_validation/errors_from_validator.phpt +++ b/Zend/tests/attributes/delayed_target_validation/errors_from_validator.phpt @@ -20,10 +20,6 @@ readonly class DemoReadonly {} enum DemoEnum {} class DemoClass { - #[DelayedTargetValidation] - #[NoDiscard] // Does nothing here - public $val; - public string $hooked { #[DelayedTargetValidation] #[NoDiscard] // Does nothing here @@ -88,7 +84,7 @@ array(2) { string(22) "AllowDynamicProperties" } } -Error: Cannot apply #[AllowDynamicProperties] to trait DemoTrait +Error: Cannot apply #[\AllowDynamicProperties] to trait DemoTrait ******************** Interface [ interface DemoInterface ] { @@ %s %d-%d @@ -121,7 +117,7 @@ array(2) { string(22) "AllowDynamicProperties" } } -Error: Cannot apply #[AllowDynamicProperties] to interface DemoInterface +Error: Cannot apply #[\AllowDynamicProperties] to interface DemoInterface ******************** Class [ readonly class DemoReadonly ] { @@ %s %d-%d @@ -154,7 +150,7 @@ array(2) { string(22) "AllowDynamicProperties" } } -Error: Cannot apply #[AllowDynamicProperties] to readonly class DemoReadonly +Error: Cannot apply #[\AllowDynamicProperties] to readonly class DemoReadonly ******************** Enum [ enum DemoEnum implements UnitEnum ] { @@ %s %d-%d @@ -194,7 +190,7 @@ array(2) { string(22) "AllowDynamicProperties" } } -Error: Cannot apply #[AllowDynamicProperties] to enum DemoEnum +Error: Cannot apply #[\AllowDynamicProperties] to enum DemoEnum ******************** Method [ public method $hooked::get ] { @@ %s %d - %d diff --git a/Zend/tests/attributes/delayed_target_validation/has_runtime_errors.phpt b/Zend/tests/attributes/delayed_target_validation/has_runtime_errors.phpt index 7a2f9d32c0b2b..b1efb538b328b 100644 --- a/Zend/tests/attributes/delayed_target_validation/has_runtime_errors.phpt +++ b/Zend/tests/attributes/delayed_target_validation/has_runtime_errors.phpt @@ -88,7 +88,7 @@ Class [ class Demo ] { - Properties [3] { Property [ public string $v1 ] - Property [ public string $v2 ] + Property [ public string $v2 { get; set; } ] Property [ public string $v3 ] } diff --git a/Zend/tests/attributes/delayed_target_validation/with_NoDiscard.phpt b/Zend/tests/attributes/delayed_target_validation/with_NoDiscard.phpt index 17baca1800512..f2571ec07c2f1 100644 --- a/Zend/tests/attributes/delayed_target_validation/with_NoDiscard.phpt +++ b/Zend/tests/attributes/delayed_target_validation/with_NoDiscard.phpt @@ -57,7 +57,7 @@ $v = $d->printVal(); var_dump($d->val); $d->hooked = "foo"; var_dump($d->hooked); -// NODiscard does not support property hooks, this should not complain +// NoDiscard does not support property hooks, this should not complain $d->hooked; var_dump(DemoClass::CLASS_CONST); demoFn(); diff --git a/Zend/zend_attributes.c b/Zend/zend_attributes.c index 3dc9aa01dbab1..4a0c78320a085 100644 --- a/Zend/zend_attributes.c +++ b/Zend/zend_attributes.c @@ -74,9 +74,9 @@ static void validate_allow_dynamic_properties( zend_attribute *attr, uint32_t target, zend_class_entry *scope) { if (scope == NULL) { - // Only reachable when validator is run but the attribute isn't applied - // to a class; in the case of delayed target validation reflection will - // complain about the target before running the validator; + /* Only reachable when validator is run but the attribute isn't applied + * to a class; in the case of delayed target validation reflection will + * complain about the target before running the validator. */ ZEND_ASSERT(target & ZEND_ATTRIBUTE_NO_TARGET_VALIDATION); return; } @@ -233,8 +233,7 @@ static void validate_nodiscard( { /* There isn't an easy way to identify the *method* that the attribute is * applied to in a manner that works during both compilation (normal - * validation) and runtime (delayed validation). So, handle them separately. - */ + * validation) and runtime (delayed validation). So, handle them separately. */ if (CG(in_compilation)) { ZEND_ASSERT((target & ZEND_ATTRIBUTE_DELAYED_TARGET_VALIDATION) == 0); zend_op_array *op_array = CG(active_op_array); From 4bd054c2a13551941bb572d39395863a8f39ec7c Mon Sep 17 00:00:00 2001 From: Daniel Scherzer Date: Mon, 11 Aug 2025 09:23:04 -0700 Subject: [PATCH 10/23] #[\Attribute] --- ... => validator_AllowDynamicProperties.phpt} | 60 +----- .../validator_Attribute.phpt | 180 ++++++++++++++++++ .../validator_NoDiscard.phpt | 79 ++++++++ Zend/zend_attributes.c | 7 + 4 files changed, 267 insertions(+), 59 deletions(-) rename Zend/tests/attributes/delayed_target_validation/{errors_from_validator.phpt => validator_AllowDynamicProperties.phpt} (70%) create mode 100644 Zend/tests/attributes/delayed_target_validation/validator_Attribute.phpt create mode 100644 Zend/tests/attributes/delayed_target_validation/validator_NoDiscard.phpt diff --git a/Zend/tests/attributes/delayed_target_validation/errors_from_validator.phpt b/Zend/tests/attributes/delayed_target_validation/validator_AllowDynamicProperties.phpt similarity index 70% rename from Zend/tests/attributes/delayed_target_validation/errors_from_validator.phpt rename to Zend/tests/attributes/delayed_target_validation/validator_AllowDynamicProperties.phpt index 1da43ea89c8c3..63add9e445a91 100644 --- a/Zend/tests/attributes/delayed_target_validation/errors_from_validator.phpt +++ b/Zend/tests/attributes/delayed_target_validation/validator_AllowDynamicProperties.phpt @@ -1,5 +1,5 @@ --TEST-- -#[\DelayedTargetValidation] affects errors from validators +#[\DelayedTargetValidation] with #[\AllowDynamicProperties]: validator errors delayed --FILE-- $this->hooked; - #[DelayedTargetValidation] - #[NoDiscard] // Does nothing here - set => $value; - } -} - $cases = [ new ReflectionClass('DemoTrait'), new ReflectionClass('DemoInterface'), new ReflectionClass('DemoReadonly'), new ReflectionClass('DemoEnum'), - new ReflectionProperty('DemoClass', 'hooked')->getHook(PropertyHookType::Get), - new ReflectionProperty('DemoClass', 'hooked')->getHook(PropertyHookType::Set), ]; foreach ($cases as $r) { echo str_repeat("*", 20) . "\n"; @@ -191,48 +178,3 @@ array(2) { } } Error: Cannot apply #[\AllowDynamicProperties] to enum DemoEnum -******************** -Method [ public method $hooked::get ] { - @@ %s %d - %d - - - Parameters [0] { - } - - Return [ string ] -} - -array(2) { - [0]=> - object(ReflectionAttribute)#%d (1) { - ["name"]=> - string(23) "DelayedTargetValidation" - } - [1]=> - object(ReflectionAttribute)#%d (1) { - ["name"]=> - string(9) "NoDiscard" - } -} -Error: #[\NoDiscard] is not supported for property hooks -******************** -Method [ public method $hooked::set ] { - @@ %s %d - %d - - - Parameters [1] { - Parameter #0 [ string $value ] - } - - Return [ void ] -} - -array(2) { - [0]=> - object(ReflectionAttribute)#%d (1) { - ["name"]=> - string(23) "DelayedTargetValidation" - } - [1]=> - object(ReflectionAttribute)#%d (1) { - ["name"]=> - string(9) "NoDiscard" - } -} -Error: #[\NoDiscard] is not supported for property hooks diff --git a/Zend/tests/attributes/delayed_target_validation/validator_Attribute.phpt b/Zend/tests/attributes/delayed_target_validation/validator_Attribute.phpt new file mode 100644 index 0000000000000..0571024f19ced --- /dev/null +++ b/Zend/tests/attributes/delayed_target_validation/validator_Attribute.phpt @@ -0,0 +1,180 @@ +--TEST-- +#[\DelayedTargetValidation] with #[\Attribute]: validator errors delayed +--FILE-- +getAttributes(); + var_dump($attributes); + try { + $attributes[1]->newInstance(); + } catch (Error $e) { + echo get_class($e) . ": " . $e->getMessage() . "\n"; + } +} + +?> +--EXPECTF-- +******************** +Trait [ trait DemoTrait ] { + @@ %s %d-%d + + - Constants [0] { + } + + - Static properties [0] { + } + + - Static methods [0] { + } + + - Properties [0] { + } + + - Methods [0] { + } +} + +array(2) { + [0]=> + object(ReflectionAttribute)#%d (1) { + ["name"]=> + string(23) "DelayedTargetValidation" + } + [1]=> + object(ReflectionAttribute)#%d (1) { + ["name"]=> + string(9) "Attribute" + } +} +Error: Cannot apply #[\Attribute] to trait DemoTrait +******************** +Interface [ interface DemoInterface ] { + @@ %s %d-%d + + - Constants [0] { + } + + - Static properties [0] { + } + + - Static methods [0] { + } + + - Properties [0] { + } + + - Methods [0] { + } +} + +array(2) { + [0]=> + object(ReflectionAttribute)#%d (1) { + ["name"]=> + string(23) "DelayedTargetValidation" + } + [1]=> + object(ReflectionAttribute)#%d (1) { + ["name"]=> + string(9) "Attribute" + } +} +Error: Cannot apply #[\Attribute] to interface DemoInterface +******************** +Class [ abstract class DemoAbstract ] { + @@ %s %d-%d + + - Constants [0] { + } + + - Static properties [0] { + } + + - Static methods [0] { + } + + - Properties [0] { + } + + - Methods [0] { + } +} + +array(2) { + [0]=> + object(ReflectionAttribute)#%d (1) { + ["name"]=> + string(23) "DelayedTargetValidation" + } + [1]=> + object(ReflectionAttribute)#%d (1) { + ["name"]=> + string(9) "Attribute" + } +} +Error: Cannot apply #[\Attribute] to abstract class DemoAbstract +******************** +Enum [ enum DemoEnum implements UnitEnum ] { + @@ %s %d-%d + + - Constants [0] { + } + + - Static properties [0] { + } + + - Static methods [1] { + Method [ static public method cases ] { + + - Parameters [0] { + } + - Return [ array ] + } + } + + - Properties [1] { + Property [ public protected(set) readonly string $name ] + } + + - Methods [0] { + } +} + +array(2) { + [0]=> + object(ReflectionAttribute)#%d (1) { + ["name"]=> + string(23) "DelayedTargetValidation" + } + [1]=> + object(ReflectionAttribute)#%d (1) { + ["name"]=> + string(9) "Attribute" + } +} +Error: Cannot apply #[\Attribute] to enum DemoEnum diff --git a/Zend/tests/attributes/delayed_target_validation/validator_NoDiscard.phpt b/Zend/tests/attributes/delayed_target_validation/validator_NoDiscard.phpt new file mode 100644 index 0000000000000..e3cfe9d1c1095 --- /dev/null +++ b/Zend/tests/attributes/delayed_target_validation/validator_NoDiscard.phpt @@ -0,0 +1,79 @@ +--TEST-- +#[\DelayedTargetValidation] with #[\NoDiscard]: validator errors delayed +--FILE-- + $this->hooked; + #[DelayedTargetValidation] + #[NoDiscard] // Does nothing here + set => $value; + } +} + +$cases = [ + new ReflectionProperty('DemoClass', 'hooked')->getHook(PropertyHookType::Get), + new ReflectionProperty('DemoClass', 'hooked')->getHook(PropertyHookType::Set), +]; +foreach ($cases as $r) { + echo str_repeat("*", 20) . "\n"; + echo $r . "\n"; + $attributes = $r->getAttributes(); + var_dump($attributes); + try { + $attributes[1]->newInstance(); + } catch (Error $e) { + echo get_class($e) . ": " . $e->getMessage() . "\n"; + } +} + +?> +--EXPECTF-- +******************** +Method [ public method $hooked::get ] { + @@ %s %d - %d + + - Parameters [0] { + } + - Return [ string ] +} + +array(2) { + [0]=> + object(ReflectionAttribute)#%d (1) { + ["name"]=> + string(23) "DelayedTargetValidation" + } + [1]=> + object(ReflectionAttribute)#%d (1) { + ["name"]=> + string(9) "NoDiscard" + } +} +Error: #[\NoDiscard] is not supported for property hooks +******************** +Method [ public method $hooked::set ] { + @@ %s %d - %d + + - Parameters [1] { + Parameter #0 [ string $value ] + } + - Return [ void ] +} + +array(2) { + [0]=> + object(ReflectionAttribute)#%d (1) { + ["name"]=> + string(23) "DelayedTargetValidation" + } + [1]=> + object(ReflectionAttribute)#%d (1) { + ["name"]=> + string(9) "NoDiscard" + } +} +Error: #[\NoDiscard] is not supported for property hooks diff --git a/Zend/zend_attributes.c b/Zend/zend_attributes.c index 4a0c78320a085..fea708491e20d 100644 --- a/Zend/zend_attributes.c +++ b/Zend/zend_attributes.c @@ -125,6 +125,13 @@ static void validate_attribute( msg = "Cannot apply #[\\Attribute] to abstract class %s"; } if (msg != NULL) { + if (target & ZEND_ATTRIBUTE_NO_TARGET_VALIDATION) { + return; + } + if (target & ZEND_ATTRIBUTE_DELAYED_TARGET_VALIDATION) { + zend_throw_error(NULL, msg, ZSTR_VAL(scope->name)); + return; + } zend_error_noreturn(E_ERROR, msg, ZSTR_VAL(scope->name)); } } From 175fed1f6038ebfe8b94b35da186797a3e2f9a34 Mon Sep 17 00:00:00 2001 From: Daniel Scherzer Date: Mon, 11 Aug 2025 09:38:51 -0700 Subject: [PATCH 11/23] UPGRADING --- UPGRADING | 16 ++++++++++++++++ UPGRADING.INTERNALS | 12 ++++++++++++ 2 files changed, 28 insertions(+) diff --git a/UPGRADING b/UPGRADING index cf146f6fa16d6..b201e76075394 100644 --- a/UPGRADING +++ b/UPGRADING @@ -53,6 +53,8 @@ PHP 8.5 UPGRADE NOTES . Applying #[\Attribute] to an abstract class, enum, interface, or trait triggers an error during compilation. Previously, the attribute could be added, but when ReflectionAttribute::newInstance() was called an error would be thrown. + The error can be delayed from compilation to runtime using the new + #[\DelayedTargetValidation] attribute. - DOM: . Cloning a DOMNamedNodeMap, DOMNodeList, Dom\NamedNodeMap, Dom\NodeList, @@ -184,6 +186,11 @@ PHP 8.5 UPGRADE NOTES RFC: https://wiki.php.net/rfc/final_promotion . #[\Override] can now be applied to properties. RFC: https://wiki.php.net/rfc/override_properties + . The #[\DelayedTargetValidation] attribute can be used to suppress + compile-time errors from core (or extension) attributes that are used on + invalid targets. These errors are instead reported at runtime if and when + ReflectionAttribute::newInstance() is called. + RFC: https://wiki.php.net/rfc/delayedtargetvalidation_attribute - Curl: . Added support for share handles that are persisted across multiple PHP @@ -528,6 +535,11 @@ PHP 8.5 UPGRADE NOTES hooks are final, and whether the property is virtual. This also affects the output of ReflectionClass::__toString() when a class contains hooked properties. + . ReflectionAttribute::newInstance() can now throw errors for internal + attributes if the attribute was applied on an invalid target and the + error was delayed from compile-time to runtime via the + #[\DelayedTargetValidation] attribute. + RFC: https://wiki.php.net/rfc/delayedtargetvalidation_attribute - Session: . session_start is stricter in regard to the option argument. @@ -648,6 +660,10 @@ PHP 8.5 UPGRADE NOTES - Core: . NoDiscard attribute was added. RFC: https://wiki.php.net/rfc/marking_return_value_as_important + . DelayedTargetValidation is an attribute that, when added, delays any errors + from *other* internal attributes about being applied to invalid targets from + compile-time to runtime. + RFC: https://wiki.php.net/rfc/delayedtargetvalidation_attribute - Curl: . CurlSharePersistentHandle representing a share handle that is persisted diff --git a/UPGRADING.INTERNALS b/UPGRADING.INTERNALS index b7396941e5023..2881160c5ee6c 100644 --- a/UPGRADING.INTERNALS +++ b/UPGRADING.INTERNALS @@ -28,6 +28,18 @@ PHP 8.5 INTERNALS UPGRADE NOTES extra layer of indirection can be removed. In other cases a zval can be heap-allocated and stored in the pointer as a minimal change to keep compatibility. + . The `target` parameter for validator callbacks for internal attributes + can now include in the bitmask the new flags + ZEND_ATTRIBUTE_NO_TARGET_VALIDATION and + ZEND_ATTRIBUTE_DELAYED_TARGET_VALIDATION. The first indicates that, if the + attribute is being applied to an invalid target, no error should be thrown + at compile time. The second indicates that validation is being performed + at runtime and that if an attribute is being applied to an invalid target, + a catchable error should be thrown. Target validation is different from + the actual functionality of the attribute; e.g. the #[\Override] attribute + will trigger an error if the function does not override a method from the + parent class or from an interface; that error does not get delayed. + RFC: https://wiki.php.net/rfc/delayedtargetvalidation_attribute - Hash . Hash functions now use proper hash_spec_result enum for return values From d983c477c391dffc6cd4a0bb1d9b324392b27452 Mon Sep 17 00:00:00 2001 From: Daniel Scherzer Date: Mon, 11 Aug 2025 09:39:41 -0700 Subject: [PATCH 12/23] without --- ext/reflection/php_reflection.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/reflection/php_reflection.c b/ext/reflection/php_reflection.c index a96e958b98bf7..87ba787e6062d 100644 --- a/ext/reflection/php_reflection.c +++ b/ext/reflection/php_reflection.c @@ -7355,7 +7355,7 @@ ZEND_METHOD(ReflectionAttribute, newInstance) } /* No harm in always running target validation, for internal attributes - * with #[DelayedTargetValidation] it isn't necessary but will always + * without #[DelayedTargetValidation] it isn't necessary but will always * succeed. */ if (!(attr->target & flags)) { zend_string *location = zend_get_attribute_target_names(attr->target); From f31f883173ec9377ff8af364f31490e33d554897 Mon Sep 17 00:00:00 2001 From: Daniel Scherzer Date: Tue, 12 Aug 2025 06:35:10 -0700 Subject: [PATCH 13/23] Update for #[\Override] on properties See #19061 --- .../with_Override_error_prop.phpt | 15 +++++++++++++++ .../with_Override_okay.phpt | 4 +++- 2 files changed, 18 insertions(+), 1 deletion(-) create mode 100644 Zend/tests/attributes/delayed_target_validation/with_Override_error_prop.phpt diff --git a/Zend/tests/attributes/delayed_target_validation/with_Override_error_prop.phpt b/Zend/tests/attributes/delayed_target_validation/with_Override_error_prop.phpt new file mode 100644 index 0000000000000..3a44fa9b22cda --- /dev/null +++ b/Zend/tests/attributes/delayed_target_validation/with_Override_error_prop.phpt @@ -0,0 +1,15 @@ +--TEST-- +#[\DelayedTargetValidation] with #[\Override]: non-overrides still error (normal property) +--FILE-- + +--EXPECTF-- +Fatal error: DemoClass::$prop has #[\Override] attribute, but no matching parent property exists in %s on line %d diff --git a/Zend/tests/attributes/delayed_target_validation/with_Override_okay.phpt b/Zend/tests/attributes/delayed_target_validation/with_Override_okay.phpt index 505a4ddb9e003..dd077f4b9cbd3 100644 --- a/Zend/tests/attributes/delayed_target_validation/with_Override_okay.phpt +++ b/Zend/tests/attributes/delayed_target_validation/with_Override_okay.phpt @@ -5,6 +5,8 @@ class Base { + public $val; + public string $hooked { get => $this->hooked; set => $value; @@ -19,7 +21,7 @@ class Base { #[Override] // Does nothing here class DemoClass extends Base { #[DelayedTargetValidation] - #[Override] // Does nothing here + #[Override] // Does something here public $val; public string $hooked { From fc0eb674c88b4d213d78cd6957376fe18eb227d9 Mon Sep 17 00:00:00 2001 From: Daniel Scherzer Date: Thu, 14 Aug 2025 11:23:52 -0700 Subject: [PATCH 14/23] Validators return error messages --- UPGRADING.INTERNALS | 19 ++++++------ Zend/zend_attributes.c | 51 +++++++++++---------------------- Zend/zend_attributes.h | 8 ++---- Zend/zend_compile.c | 9 ++++-- ext/reflection/php_reflection.c | 8 ++++-- ext/zend_test/test.c | 5 ++-- 6 files changed, 42 insertions(+), 58 deletions(-) diff --git a/UPGRADING.INTERNALS b/UPGRADING.INTERNALS index 2881160c5ee6c..f52fb2bf5f338 100644 --- a/UPGRADING.INTERNALS +++ b/UPGRADING.INTERNALS @@ -28,17 +28,16 @@ PHP 8.5 INTERNALS UPGRADE NOTES extra layer of indirection can be removed. In other cases a zval can be heap-allocated and stored in the pointer as a minimal change to keep compatibility. + * The validator callbacks for internal attribute now return `zend_string *` + rather than `void`; instead of emitting an error when an attribute is + applied incorrectly, the error message should be returned as a zend_string + pointer. . The `target` parameter for validator callbacks for internal attributes - can now include in the bitmask the new flags - ZEND_ATTRIBUTE_NO_TARGET_VALIDATION and - ZEND_ATTRIBUTE_DELAYED_TARGET_VALIDATION. The first indicates that, if the - attribute is being applied to an invalid target, no error should be thrown - at compile time. The second indicates that validation is being performed - at runtime and that if an attribute is being applied to an invalid target, - a catchable error should be thrown. Target validation is different from - the actual functionality of the attribute; e.g. the #[\Override] attribute - will trigger an error if the function does not override a method from the - parent class or from an interface; that error does not get delayed. + can now include in the bitmask the new flag + ZEND_ATTRIBUTE_DELAYED_TARGET_VALIDATION. The flag indicates that + validation is being performed at runtime and that other than returning an + error message, the validator should not do anything, most notably including + modification of any class properties. RFC: https://wiki.php.net/rfc/delayedtargetvalidation_attribute - Hash diff --git a/Zend/zend_attributes.c b/Zend/zend_attributes.c index fea708491e20d..109324add4dab 100644 --- a/Zend/zend_attributes.c +++ b/Zend/zend_attributes.c @@ -70,16 +70,10 @@ uint32_t zend_attribute_attribute_get_flags(zend_attribute *attr, zend_class_ent return ZEND_ATTRIBUTE_TARGET_ALL; } -static void validate_allow_dynamic_properties( +static zend_string *validate_allow_dynamic_properties( zend_attribute *attr, uint32_t target, zend_class_entry *scope) { - if (scope == NULL) { - /* Only reachable when validator is run but the attribute isn't applied - * to a class; in the case of delayed target validation reflection will - * complain about the target before running the validator. */ - ZEND_ASSERT(target & ZEND_ATTRIBUTE_NO_TARGET_VALIDATION); - return; - } + ZEND_ASSERT(scope != NULL); const char *msg = NULL; if (scope->ce_flags & ZEND_ACC_TRAIT) { msg = "Cannot apply #[\\AllowDynamicProperties] to trait %s"; @@ -91,27 +85,20 @@ static void validate_allow_dynamic_properties( msg = "Cannot apply #[\\AllowDynamicProperties] to enum %s"; } if (msg != NULL) { - if (target & ZEND_ATTRIBUTE_NO_TARGET_VALIDATION) { - return; - } - if (target & ZEND_ATTRIBUTE_DELAYED_TARGET_VALIDATION) { - // Should not have passed the first time - ZEND_ASSERT((scope->ce_flags & ZEND_ACC_ALLOW_DYNAMIC_PROPERTIES) == 0); - // Throw a catchable error at runtime - zend_throw_error(NULL, msg, ZSTR_VAL(scope->name)); - return; - } - zend_error_noreturn(E_ERROR, msg, ZSTR_VAL(scope->name) ); + smart_str str = {0}; + smart_str_append_printf(&str, msg, ZSTR_VAL(scope->name)); + return smart_str_extract(&str); } if (target & ZEND_ATTRIBUTE_DELAYED_TARGET_VALIDATION) { // Should have passed the first time ZEND_ASSERT((scope->ce_flags & ZEND_ACC_ALLOW_DYNAMIC_PROPERTIES) != 0); - return; + return NULL; } scope->ce_flags |= ZEND_ACC_ALLOW_DYNAMIC_PROPERTIES; + return NULL; } -static void validate_attribute( +static zend_string *validate_attribute( zend_attribute *attr, uint32_t target, zend_class_entry *scope) { const char *msg = NULL; @@ -125,15 +112,11 @@ static void validate_attribute( msg = "Cannot apply #[\\Attribute] to abstract class %s"; } if (msg != NULL) { - if (target & ZEND_ATTRIBUTE_NO_TARGET_VALIDATION) { - return; - } - if (target & ZEND_ATTRIBUTE_DELAYED_TARGET_VALIDATION) { - zend_throw_error(NULL, msg, ZSTR_VAL(scope->name)); - return; - } - zend_error_noreturn(E_ERROR, msg, ZSTR_VAL(scope->name)); + smart_str str = {0}; + smart_str_append_printf(&str, msg, ZSTR_VAL(scope->name)); + return smart_str_extract(&str); } + return NULL; } ZEND_METHOD(Attribute, __construct) @@ -235,7 +218,7 @@ ZEND_METHOD(Deprecated, __construct) } } -static void validate_nodiscard( +static zend_string *validate_nodiscard( zend_attribute *attr, uint32_t target, zend_class_entry *scope) { /* There isn't an easy way to identify the *method* that the attribute is @@ -247,16 +230,14 @@ static void validate_nodiscard( const zend_string *prop_info_name = CG(context).active_property_info_name; if (prop_info_name == NULL) { op_array->fn_flags |= ZEND_ACC_NODISCARD; - return; + return NULL; } // Applied to a hook, either throw or ignore - if (target & ZEND_ATTRIBUTE_NO_TARGET_VALIDATION) { - return; - } - zend_error_noreturn(E_COMPILE_ERROR, "#[\\NoDiscard] is not supported for property hooks"); + return ZSTR_INIT_LITERAL("#[\\NoDiscard] is not supported for property hooks", 0); } /* At runtime, no way to identify the target method; Reflection will handle * throwing the error if needed. */ + return NULL; } ZEND_METHOD(NoDiscard, __construct) diff --git a/Zend/zend_attributes.h b/Zend/zend_attributes.h index 08f199c7cd52a..ec0414d5b1e63 100644 --- a/Zend/zend_attributes.h +++ b/Zend/zend_attributes.h @@ -34,13 +34,9 @@ #define ZEND_ATTRIBUTE_IS_REPEATABLE (1<<7) #define ZEND_ATTRIBUTE_FLAGS ((1<<8) - 1) -/* Not a real flag, just passed to validators when target validation is - * suppressed; must not conflict with any of the real flags above. */ -#define ZEND_ATTRIBUTE_NO_TARGET_VALIDATION (1<<8) - /* Not a real flag, just passed to validators when target validation is * being run at runtime; must not conflict with any of the real flags above. */ -#define ZEND_ATTRIBUTE_DELAYED_TARGET_VALIDATION (1<<9) +#define ZEND_ATTRIBUTE_DELAYED_TARGET_VALIDATION (1<<8) /* Flags for zend_attribute.flags */ #define ZEND_ATTRIBUTE_PERSISTENT (1<<0) @@ -79,7 +75,7 @@ typedef struct _zend_attribute { typedef struct _zend_internal_attribute { zend_class_entry *ce; uint32_t flags; - void (*validator)(zend_attribute *attr, uint32_t target, zend_class_entry *scope); + zend_string* (*validator)(zend_attribute *attr, uint32_t target, zend_class_entry *scope); } zend_internal_attribute; ZEND_API zend_attribute *zend_get_attribute(HashTable *attributes, zend_string *lcname); diff --git a/Zend/zend_compile.c b/Zend/zend_compile.c index 34784ee9502bd..82e91d9264fe1 100644 --- a/Zend/zend_compile.c +++ b/Zend/zend_compile.c @@ -7575,7 +7575,6 @@ static void zend_compile_attributes( strlen("delayedtargetvalidation") ); } - uint32_t extra_flags = delayed_target_validation ? ZEND_ATTRIBUTE_NO_TARGET_VALIDATION : 0; /* Validate attributes in a secondary loop (needed to detect repeated attributes). */ ZEND_HASH_PACKED_FOREACH_PTR(*attributes, attr) { if (attr->offset != offset || NULL == (config = zend_internal_attribute_get(attr->lcname))) { @@ -7603,7 +7602,13 @@ static void zend_compile_attributes( // Validators are not run if the target is already invalid if (run_validator && config->validator != NULL) { - config->validator(attr, target | extra_flags, CG(active_class_entry)); + zend_string *error = config->validator(attr, target, CG(active_class_entry)); + if (error != NULL) { + if (delayed_target_validation == false) { + zend_error_noreturn(E_COMPILE_ERROR, ZSTR_VAL(error)); + } + zend_string_efree(error); + } } } ZEND_HASH_FOREACH_END(); } diff --git a/ext/reflection/php_reflection.c b/ext/reflection/php_reflection.c index 87ba787e6062d..f35199b3afb0b 100644 --- a/ext/reflection/php_reflection.c +++ b/ext/reflection/php_reflection.c @@ -7375,12 +7375,14 @@ ZEND_METHOD(ReflectionAttribute, newInstance) if (delayed_target_validation && ce->type == ZEND_INTERNAL_CLASS) { zend_internal_attribute *config = zend_internal_attribute_get(attr->data->lcname); if (config != NULL && config->validator != NULL) { - config->validator( + zend_string *error = config->validator( attr->data, attr->target | ZEND_ATTRIBUTE_DELAYED_TARGET_VALIDATION, attr->scope ); - if (EG(exception)) { + if (error != NULL) { + zend_throw_exception(zend_ce_error, ZSTR_VAL(error), 0); + zend_string_efree(error); RETURN_THROWS(); } } @@ -7389,7 +7391,7 @@ ZEND_METHOD(ReflectionAttribute, newInstance) * attribute is applied to, attr->scope is just the overall class entry * that the method is a part of. */ if (ce == zend_ce_nodiscard && attr->on_property_hook) { - zend_throw_error(NULL, "#[\\NoDiscard] is not supported for property hooks");; + zend_throw_error(NULL, "#[\\NoDiscard] is not supported for property hooks"); RETURN_THROWS(); } } diff --git a/ext/zend_test/test.c b/ext/zend_test/test.c index c2e5ae6130e7d..af60dffb4bf14 100644 --- a/ext/zend_test/test.c +++ b/ext/zend_test/test.c @@ -958,11 +958,12 @@ static zend_function *zend_test_class_static_method_get(zend_class_entry *ce, ze return zend_std_get_static_method(ce, name, NULL); } -void zend_attribute_validate_zendtestattribute(zend_attribute *attr, uint32_t target, zend_class_entry *scope) +zend_string *zend_attribute_validate_zendtestattribute(zend_attribute *attr, uint32_t target, zend_class_entry *scope) { if (target != ZEND_ATTRIBUTE_TARGET_CLASS) { - zend_error(E_COMPILE_ERROR, "Only classes can be marked with #[ZendTestAttribute]"); + return ZSTR_INIT_LITERAL("Only classes can be marked with #[ZendTestAttribute]", 0); } + return NULL; } static ZEND_METHOD(_ZendTestClass, __toString) From af89164cb04911ea327116512e8ca745c603ab3f Mon Sep 17 00:00:00 2001 From: Daniel Scherzer Date: Thu, 14 Aug 2025 11:30:48 -0700 Subject: [PATCH 15/23] %s --- Zend/zend_compile.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Zend/zend_compile.c b/Zend/zend_compile.c index 82e91d9264fe1..6fe8b2dc310c8 100644 --- a/Zend/zend_compile.c +++ b/Zend/zend_compile.c @@ -7605,7 +7605,7 @@ static void zend_compile_attributes( zend_string *error = config->validator(attr, target, CG(active_class_entry)); if (error != NULL) { if (delayed_target_validation == false) { - zend_error_noreturn(E_COMPILE_ERROR, ZSTR_VAL(error)); + zend_error_noreturn(E_COMPILE_ERROR, "%s", ZSTR_VAL(error)); } zend_string_efree(error); } From 5513a544563b48b43ccf8bcfe3dd81c1984b5bf6 Mon Sep 17 00:00:00 2001 From: Daniel Scherzer Date: Fri, 15 Aug 2025 11:33:34 -0700 Subject: [PATCH 16/23] Only run validators once, remember the error --- UPGRADING.INTERNALS | 11 +--- .../opcache_validator_errors.inc | 5 ++ .../opcache_validator_errors.phpt | 58 +++++++++++++++++++ Zend/zend_attributes.c | 33 ++++------- Zend/zend_attributes.h | 7 +-- Zend/zend_compile.c | 4 +- ext/opcache/zend_file_cache.c | 2 + ext/opcache/zend_persist_calc.c | 3 + ext/reflection/php_reflection.c | 24 ++------ 9 files changed, 94 insertions(+), 53 deletions(-) create mode 100644 Zend/tests/attributes/delayed_target_validation/opcache_validator_errors.inc create mode 100644 Zend/tests/attributes/delayed_target_validation/opcache_validator_errors.phpt diff --git a/UPGRADING.INTERNALS b/UPGRADING.INTERNALS index f52fb2bf5f338..a74c15930dec3 100644 --- a/UPGRADING.INTERNALS +++ b/UPGRADING.INTERNALS @@ -28,16 +28,11 @@ PHP 8.5 INTERNALS UPGRADE NOTES extra layer of indirection can be removed. In other cases a zval can be heap-allocated and stored in the pointer as a minimal change to keep compatibility. - * The validator callbacks for internal attribute now return `zend_string *` + . The validator callbacks for internal attribute now return `zend_string *` rather than `void`; instead of emitting an error when an attribute is applied incorrectly, the error message should be returned as a zend_string - pointer. - . The `target` parameter for validator callbacks for internal attributes - can now include in the bitmask the new flag - ZEND_ATTRIBUTE_DELAYED_TARGET_VALIDATION. The flag indicates that - validation is being performed at runtime and that other than returning an - error message, the validator should not do anything, most notably including - modification of any class properties. + pointer. If the error will be delayed until runtime, it is stored in the + new `validation_error` field of the `zend_attribute` struct. RFC: https://wiki.php.net/rfc/delayedtargetvalidation_attribute - Hash diff --git a/Zend/tests/attributes/delayed_target_validation/opcache_validator_errors.inc b/Zend/tests/attributes/delayed_target_validation/opcache_validator_errors.inc new file mode 100644 index 0000000000000..275df500c7e93 --- /dev/null +++ b/Zend/tests/attributes/delayed_target_validation/opcache_validator_errors.inc @@ -0,0 +1,5 @@ + +--FILE-- +getAttributes(); +var_dump($attributes); +try { + $attributes[1]->newInstance(); +} catch (Error $e) { + echo get_class($e) . ": " . $e->getMessage() . "\n"; +} + +?> +--EXPECTF-- +Trait [ trait DemoTrait ] { + @@ %s %d-%d + + - Constants [0] { + } + + - Static properties [0] { + } + + - Static methods [0] { + } + + - Properties [0] { + } + + - Methods [0] { + } +} + +array(2) { + [0]=> + object(ReflectionAttribute)#%d (1) { + ["name"]=> + string(23) "DelayedTargetValidation" + } + [1]=> + object(ReflectionAttribute)#%d (1) { + ["name"]=> + string(22) "AllowDynamicProperties" + } +} +Error: Cannot apply #[\AllowDynamicProperties] to trait DemoTrait diff --git a/Zend/zend_attributes.c b/Zend/zend_attributes.c index 109324add4dab..7a436b9810f9c 100644 --- a/Zend/zend_attributes.c +++ b/Zend/zend_attributes.c @@ -89,11 +89,6 @@ static zend_string *validate_allow_dynamic_properties( smart_str_append_printf(&str, msg, ZSTR_VAL(scope->name)); return smart_str_extract(&str); } - if (target & ZEND_ATTRIBUTE_DELAYED_TARGET_VALIDATION) { - // Should have passed the first time - ZEND_ASSERT((scope->ce_flags & ZEND_ACC_ALLOW_DYNAMIC_PROPERTIES) != 0); - return NULL; - } scope->ce_flags |= ZEND_ACC_ALLOW_DYNAMIC_PROPERTIES; return NULL; } @@ -221,23 +216,15 @@ ZEND_METHOD(Deprecated, __construct) static zend_string *validate_nodiscard( zend_attribute *attr, uint32_t target, zend_class_entry *scope) { - /* There isn't an easy way to identify the *method* that the attribute is - * applied to in a manner that works during both compilation (normal - * validation) and runtime (delayed validation). So, handle them separately. */ - if (CG(in_compilation)) { - ZEND_ASSERT((target & ZEND_ATTRIBUTE_DELAYED_TARGET_VALIDATION) == 0); - zend_op_array *op_array = CG(active_op_array); - const zend_string *prop_info_name = CG(context).active_property_info_name; - if (prop_info_name == NULL) { - op_array->fn_flags |= ZEND_ACC_NODISCARD; - return NULL; - } - // Applied to a hook, either throw or ignore - return ZSTR_INIT_LITERAL("#[\\NoDiscard] is not supported for property hooks", 0); + ZEND_ASSERT(CG(in_compilation)); + zend_op_array *op_array = CG(active_op_array); + const zend_string *prop_info_name = CG(context).active_property_info_name; + if (prop_info_name == NULL) { + op_array->fn_flags |= ZEND_ACC_NODISCARD; + return NULL; } - /* At runtime, no way to identify the target method; Reflection will handle - * throwing the error if needed. */ - return NULL; + // Applied to a hook + return ZSTR_INIT_LITERAL("#[\\NoDiscard] is not supported for property hooks", 0); } ZEND_METHOD(NoDiscard, __construct) @@ -467,6 +454,9 @@ static void attr_free(zval *v) zend_string_release(attr->name); zend_string_release(attr->lcname); + if (attr->validation_error != NULL) { + zend_string_release(attr->validation_error); + } for (uint32_t i = 0; i < attr->argc; i++) { if (attr->args[i].name) { @@ -499,6 +489,7 @@ ZEND_API zend_attribute *zend_add_attribute(HashTable **attributes, zend_string } attr->lcname = zend_string_tolower_ex(attr->name, persistent); + attr->validation_error = NULL; attr->flags = flags; attr->lineno = lineno; attr->offset = offset; diff --git a/Zend/zend_attributes.h b/Zend/zend_attributes.h index ec0414d5b1e63..10227c2d1e8ef 100644 --- a/Zend/zend_attributes.h +++ b/Zend/zend_attributes.h @@ -34,10 +34,6 @@ #define ZEND_ATTRIBUTE_IS_REPEATABLE (1<<7) #define ZEND_ATTRIBUTE_FLAGS ((1<<8) - 1) -/* Not a real flag, just passed to validators when target validation is - * being run at runtime; must not conflict with any of the real flags above. */ -#define ZEND_ATTRIBUTE_DELAYED_TARGET_VALIDATION (1<<8) - /* Flags for zend_attribute.flags */ #define ZEND_ATTRIBUTE_PERSISTENT (1<<0) #define ZEND_ATTRIBUTE_STRICT_TYPES (1<<1) @@ -64,6 +60,9 @@ typedef struct { typedef struct _zend_attribute { zend_string *name; zend_string *lcname; + /* Only non-null for internal attributes with validation errors that are + * delayed until runtime via #[\DelayedTargetValidation] */ + zend_string *validation_error; uint32_t flags; uint32_t lineno; /* Parameter offsets start at 1, everything else uses 0. */ diff --git a/Zend/zend_compile.c b/Zend/zend_compile.c index 6fe8b2dc310c8..f1e17c49ea8f9 100644 --- a/Zend/zend_compile.c +++ b/Zend/zend_compile.c @@ -7606,8 +7606,10 @@ static void zend_compile_attributes( if (error != NULL) { if (delayed_target_validation == false) { zend_error_noreturn(E_COMPILE_ERROR, "%s", ZSTR_VAL(error)); + zend_string_efree(error); + } else { + attr->validation_error = error; } - zend_string_efree(error); } } } ZEND_HASH_FOREACH_END(); diff --git a/ext/opcache/zend_file_cache.c b/ext/opcache/zend_file_cache.c index 4b47bb54e7acd..4ec285be84348 100644 --- a/ext/opcache/zend_file_cache.c +++ b/ext/opcache/zend_file_cache.c @@ -461,6 +461,7 @@ static void zend_file_cache_serialize_attribute(zval *zv, SERIALIZE_STR(attr->name); SERIALIZE_STR(attr->lcname); + SERIALIZE_STR(attr->validation_error); for (i = 0; i < attr->argc; i++) { SERIALIZE_STR(attr->args[i].name); @@ -1352,6 +1353,7 @@ static void zend_file_cache_unserialize_attribute(zval *zv, zend_persistent_scri UNSERIALIZE_STR(attr->name); UNSERIALIZE_STR(attr->lcname); + UNSERIALIZE_STR(attr->validation_error); for (i = 0; i < attr->argc; i++) { UNSERIALIZE_STR(attr->args[i].name); diff --git a/ext/opcache/zend_persist_calc.c b/ext/opcache/zend_persist_calc.c index 639d7d5446705..106a69f5dd383 100644 --- a/ext/opcache/zend_persist_calc.c +++ b/ext/opcache/zend_persist_calc.c @@ -181,6 +181,9 @@ static void zend_persist_attributes_calc(HashTable *attributes) ADD_SIZE(ZEND_ATTRIBUTE_SIZE(attr->argc)); ADD_INTERNED_STRING(attr->name); ADD_INTERNED_STRING(attr->lcname); + if (attr->validation_error != NULL) { + ADD_INTERNED_STRING(attr->validation_error); + } for (i = 0; i < attr->argc; i++) { if (attr->args[i].name) { diff --git a/ext/reflection/php_reflection.c b/ext/reflection/php_reflection.c index f35199b3afb0b..702053115d6b6 100644 --- a/ext/reflection/php_reflection.c +++ b/ext/reflection/php_reflection.c @@ -7373,27 +7373,13 @@ ZEND_METHOD(ReflectionAttribute, newInstance) /* Run the delayed validator function for internal attributes */ if (delayed_target_validation && ce->type == ZEND_INTERNAL_CLASS) { - zend_internal_attribute *config = zend_internal_attribute_get(attr->data->lcname); - if (config != NULL && config->validator != NULL) { - zend_string *error = config->validator( - attr->data, - attr->target | ZEND_ATTRIBUTE_DELAYED_TARGET_VALIDATION, - attr->scope - ); - if (error != NULL) { - zend_throw_exception(zend_ce_error, ZSTR_VAL(error), 0); - zend_string_efree(error); - RETURN_THROWS(); - } - } - /* For #[NoDiscard], the attribute does not work on property hooks, but - * at runtime the validator has no way to access the method that an - * attribute is applied to, attr->scope is just the overall class entry - * that the method is a part of. */ - if (ce == zend_ce_nodiscard && attr->on_property_hook) { - zend_throw_error(NULL, "#[\\NoDiscard] is not supported for property hooks"); + zend_string *error = attr->data->validation_error; + if (error != NULL) { + zend_throw_exception(zend_ce_error, ZSTR_VAL(error), 0); RETURN_THROWS(); } + } else { + ZEND_ASSERT(attr->data->validation_error == NULL); } /* Repetition validation is done even if #[DelayedTargetValidation] is used From 9044549cca084aca6b1477a9a4362bfe4373aac9 Mon Sep 17 00:00:00 2001 From: Daniel Scherzer Date: Fri, 15 Aug 2025 11:50:23 -0700 Subject: [PATCH 17/23] -on_property_hook --- ext/reflection/php_reflection.c | 37 +++++++++++---------------------- 1 file changed, 12 insertions(+), 25 deletions(-) diff --git a/ext/reflection/php_reflection.c b/ext/reflection/php_reflection.c index 702053115d6b6..677a4b391c348 100644 --- a/ext/reflection/php_reflection.c +++ b/ext/reflection/php_reflection.c @@ -157,7 +157,6 @@ typedef struct _attribute_reference { zend_class_entry *scope; zend_string *filename; uint32_t target; - bool on_property_hook; } attribute_reference; typedef enum { @@ -1255,7 +1254,7 @@ static void _extension_string(smart_str *str, const zend_module_entry *module, c /* {{{ reflection_attribute_factory */ static void reflection_attribute_factory(zval *object, HashTable *attributes, zend_attribute *data, - zend_class_entry *scope, uint32_t target, zend_string *filename, bool on_property_hook) + zend_class_entry *scope, uint32_t target, zend_string *filename) { reflection_object *intern; attribute_reference *reference; @@ -1268,7 +1267,6 @@ static void reflection_attribute_factory(zval *object, HashTable *attributes, ze reference->scope = scope; reference->filename = filename ? zend_string_copy(filename) : NULL; reference->target = target; - reference->on_property_hook = on_property_hook; intern->ptr = reference; intern->ref_type = REF_TYPE_ATTRIBUTE; ZVAL_STR_COPY(reflection_prop_name(object), data->name); @@ -1276,8 +1274,7 @@ static void reflection_attribute_factory(zval *object, HashTable *attributes, ze /* }}} */ static int read_attributes(zval *ret, HashTable *attributes, zend_class_entry *scope, - uint32_t offset, uint32_t target, zend_string *name, zend_class_entry *base, zend_string *filename, - bool on_property_hook) /* {{{ */ + uint32_t offset, uint32_t target, zend_string *name, zend_class_entry *base, zend_string *filename) /* {{{ */ { ZEND_ASSERT(attributes != NULL); @@ -1290,7 +1287,7 @@ static int read_attributes(zval *ret, HashTable *attributes, zend_class_entry *s ZEND_HASH_PACKED_FOREACH_PTR(attributes, attr) { if (attr->offset == offset && zend_string_equals(attr->lcname, filter)) { - reflection_attribute_factory(&tmp, attributes, attr, scope, target, filename, on_property_hook); + reflection_attribute_factory(&tmp, attributes, attr, scope, target, filename); add_next_index_zval(ret, &tmp); } } ZEND_HASH_FOREACH_END(); @@ -1322,7 +1319,7 @@ static int read_attributes(zval *ret, HashTable *attributes, zend_class_entry *s } } - reflection_attribute_factory(&tmp, attributes, attr, scope, target, filename, on_property_hook); + reflection_attribute_factory(&tmp, attributes, attr, scope, target, filename); add_next_index_zval(ret, &tmp); } ZEND_HASH_FOREACH_END(); @@ -1331,7 +1328,7 @@ static int read_attributes(zval *ret, HashTable *attributes, zend_class_entry *s /* }}} */ static void reflect_attributes(INTERNAL_FUNCTION_PARAMETERS, HashTable *attributes, - uint32_t offset, zend_class_entry *scope, uint32_t target, zend_string *filename, bool on_property_hook) /* {{{ */ + uint32_t offset, zend_class_entry *scope, uint32_t target, zend_string *filename) /* {{{ */ { zend_string *name = NULL; zend_long flags = 0; @@ -1364,7 +1361,7 @@ static void reflect_attributes(INTERNAL_FUNCTION_PARAMETERS, HashTable *attribut array_init(return_value); - if (FAILURE == read_attributes(return_value, attributes, scope, offset, target, name, base, filename, on_property_hook)) { + if (FAILURE == read_attributes(return_value, attributes, scope, offset, target, name, base, filename)) { RETURN_THROWS(); } } @@ -2069,21 +2066,15 @@ ZEND_METHOD(ReflectionFunctionAbstract, getAttributes) GET_REFLECTION_OBJECT_PTR(fptr); - bool on_property_hook = false; - if (fptr->common.scope && (fptr->common.fn_flags & (ZEND_ACC_CLOSURE|ZEND_ACC_FAKE_CLOSURE)) != ZEND_ACC_CLOSURE) { target = ZEND_ATTRIBUTE_TARGET_METHOD; - if (fptr->common.prop_info != NULL) { - on_property_hook = true; - } } else { target = ZEND_ATTRIBUTE_TARGET_FUNCTION; } reflect_attributes(INTERNAL_FUNCTION_PARAM_PASSTHRU, fptr->common.attributes, 0, fptr->common.scope, target, - fptr->type == ZEND_USER_FUNCTION ? fptr->op_array.filename : NULL, - on_property_hook); + fptr->type == ZEND_USER_FUNCTION ? fptr->op_array.filename : NULL); } /* }}} */ @@ -2925,8 +2916,7 @@ ZEND_METHOD(ReflectionParameter, getAttributes) reflect_attributes(INTERNAL_FUNCTION_PARAM_PASSTHRU, attributes, param->offset + 1, scope, ZEND_ATTRIBUTE_TARGET_PARAMETER, - param->fptr->type == ZEND_USER_FUNCTION ? param->fptr->op_array.filename : NULL, - false); + param->fptr->type == ZEND_USER_FUNCTION ? param->fptr->op_array.filename : NULL); } /* {{{ Returns the index of the parameter, starting from 0 */ @@ -4048,8 +4038,7 @@ ZEND_METHOD(ReflectionClassConstant, getAttributes) reflect_attributes(INTERNAL_FUNCTION_PARAM_PASSTHRU, ref->attributes, 0, ref->ce, ZEND_ATTRIBUTE_TARGET_CLASS_CONST, - ref->ce->type == ZEND_USER_CLASS ? ref->ce->info.user.filename : NULL, - false); + ref->ce->type == ZEND_USER_CLASS ? ref->ce->info.user.filename : NULL); } /* }}} */ @@ -4454,8 +4443,7 @@ ZEND_METHOD(ReflectionClass, getAttributes) reflect_attributes(INTERNAL_FUNCTION_PARAM_PASSTHRU, ce->attributes, 0, ce, ZEND_ATTRIBUTE_TARGET_CLASS, - ce->type == ZEND_USER_CLASS ? ce->info.user.filename : NULL, - false); + ce->type == ZEND_USER_CLASS ? ce->info.user.filename : NULL); } /* }}} */ @@ -6402,8 +6390,7 @@ ZEND_METHOD(ReflectionProperty, getAttributes) reflect_attributes(INTERNAL_FUNCTION_PARAM_PASSTHRU, ref->prop->attributes, 0, ref->prop->ce, ZEND_ATTRIBUTE_TARGET_PROPERTY, - ref->prop->ce->type == ZEND_USER_CLASS ? ref->prop->ce->info.user.filename : NULL, - false); + ref->prop->ce->type == ZEND_USER_CLASS ? ref->prop->ce->info.user.filename : NULL); } /* }}} */ @@ -7911,7 +7898,7 @@ ZEND_METHOD(ReflectionConstant, getAttributes) reflect_attributes(INTERNAL_FUNCTION_PARAM_PASSTHRU, const_->attributes, 0, NULL, ZEND_ATTRIBUTE_TARGET_CONST, - const_->filename, false); + const_->filename); } ZEND_METHOD(ReflectionConstant, __toString) From 317f2b8706001268563d5222e66b60285c199b74 Mon Sep 17 00:00:00 2001 From: Daniel Scherzer Date: Fri, 15 Aug 2025 13:33:03 -0700 Subject: [PATCH 18/23] Cleanup and tweaks --- UPGRADING | 4 +--- Zend/zend_attributes.c | 9 ++------- Zend/zend_attributes.stub.php | 2 +- Zend/zend_attributes_arginfo.h | 5 +++-- Zend/zend_compile.c | 5 +++-- ext/reflection/php_reflection.c | 8 +++++--- 6 files changed, 15 insertions(+), 18 deletions(-) diff --git a/UPGRADING b/UPGRADING index b201e76075394..6d59134c237cf 100644 --- a/UPGRADING +++ b/UPGRADING @@ -660,9 +660,7 @@ PHP 8.5 UPGRADE NOTES - Core: . NoDiscard attribute was added. RFC: https://wiki.php.net/rfc/marking_return_value_as_important - . DelayedTargetValidation is an attribute that, when added, delays any errors - from *other* internal attributes about being applied to invalid targets from - compile-time to runtime. + . DelayedTargetValidation attribute was added. RFC: https://wiki.php.net/rfc/delayedtargetvalidation_attribute - Curl: diff --git a/Zend/zend_attributes.c b/Zend/zend_attributes.c index 7a436b9810f9c..0fa1f53fb9c8d 100644 --- a/Zend/zend_attributes.c +++ b/Zend/zend_attributes.c @@ -217,9 +217,9 @@ static zend_string *validate_nodiscard( zend_attribute *attr, uint32_t target, zend_class_entry *scope) { ZEND_ASSERT(CG(in_compilation)); - zend_op_array *op_array = CG(active_op_array); const zend_string *prop_info_name = CG(context).active_property_info_name; if (prop_info_name == NULL) { + zend_op_array *op_array = CG(active_op_array); op_array->fn_flags |= ZEND_ACC_NODISCARD; return NULL; } @@ -524,12 +524,7 @@ ZEND_API zend_internal_attribute *zend_mark_internal_attribute(zend_class_entry if (zend_string_equals(attr->name, zend_ce_attribute->name)) { internal_attr = pemalloc(sizeof(zend_internal_attribute), 1); internal_attr->ce = ce; - if (attr->argc == 0) { - // Apply default of Attribute::TARGET_ALL - internal_attr->flags = ZEND_ATTRIBUTE_TARGET_ALL; - } else { - internal_attr->flags = Z_LVAL(attr->args[0].value); - } + internal_attr->flags = Z_LVAL(attr->args[0].value); internal_attr->validator = NULL; zend_string *lcname = zend_string_tolower_ex(ce->name, 1); diff --git a/Zend/zend_attributes.stub.php b/Zend/zend_attributes.stub.php index 78f39a85bd5e7..6db68d4d418e2 100644 --- a/Zend/zend_attributes.stub.php +++ b/Zend/zend_attributes.stub.php @@ -101,5 +101,5 @@ public function __construct(?string $message = null) {} /** * @strict-properties */ -#[Attribute] +#[Attribute(Attribute::TARGET_ALL)] final class DelayedTargetValidation {} diff --git a/Zend/zend_attributes_arginfo.h b/Zend/zend_attributes_arginfo.h index b735b5eb52682..a271df8e91de9 100644 --- a/Zend/zend_attributes_arginfo.h +++ b/Zend/zend_attributes_arginfo.h @@ -1,5 +1,5 @@ /* This is a generated file, edit the .stub.php file instead. - * Stub hash: 2d538e455657bc49ae0aa98b5534838cd9ffe487 */ + * Stub hash: fa08288df8338c1a16fbf83c179c4084a56007e1 */ ZEND_BEGIN_ARG_INFO_EX(arginfo_class_Attribute___construct, 0, 0, 0) ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, flags, IS_LONG, 0, "Attribute::TARGET_ALL") @@ -285,8 +285,9 @@ static zend_class_entry *register_class_DelayedTargetValidation(void) class_entry = zend_register_internal_class_with_flags(&ce, NULL, ZEND_ACC_FINAL|ZEND_ACC_NO_DYNAMIC_PROPERTIES); zend_string *attribute_name_Attribute_class_DelayedTargetValidation_0 = zend_string_init_interned("Attribute", sizeof("Attribute") - 1, 1); - zend_add_class_attribute(class_entry, attribute_name_Attribute_class_DelayedTargetValidation_0, 0); + zend_attribute *attribute_Attribute_class_DelayedTargetValidation_0 = zend_add_class_attribute(class_entry, attribute_name_Attribute_class_DelayedTargetValidation_0, 1); zend_string_release(attribute_name_Attribute_class_DelayedTargetValidation_0); + ZVAL_LONG(&attribute_Attribute_class_DelayedTargetValidation_0->args[0].value, ZEND_ATTRIBUTE_TARGET_ALL); return class_entry; } diff --git a/Zend/zend_compile.c b/Zend/zend_compile.c index f1e17c49ea8f9..e675f8ae6755c 100644 --- a/Zend/zend_compile.c +++ b/Zend/zend_compile.c @@ -8539,8 +8539,9 @@ static zend_op_array *zend_compile_func_decl_ex( if (op_array->fn_flags & ZEND_ACC_NODISCARD) { /* ZEND_ACC_NODISCARD gets added by the attribute validator, but only * if the method is not a hook; if it is a hook, then the validator - * should have either thrown an error or done nothing due to delayed - * target validation. */ + * will have returned an error message, even if the error message was + * delayed with #[\DelayedTargetValidation] that ZEND_ACC_NODISCARD + * flag should not have been added. */ ZEND_ASSERT(!is_hook); if (op_array->fn_flags & ZEND_ACC_HAS_RETURN_TYPE) { diff --git a/ext/reflection/php_reflection.c b/ext/reflection/php_reflection.c index 677a4b391c348..68b3593a63c9b 100644 --- a/ext/reflection/php_reflection.c +++ b/ext/reflection/php_reflection.c @@ -7331,8 +7331,10 @@ ZEND_METHOD(ReflectionAttribute, newInstance) * - the attribute is an internal attribute, and it had the target and * and repetition validated already * - the attribute is an internal attribute and repetition was validated - * already, but the target was not validated due to the presence of - * #[DelayedTargetValidation] + * already, the internal validator might have been run if the target was + * correct, but any error would have been stored in + * `zend_attribute.validation_error` instead of being thrown due to the + * presence of #[DelayedTargetValidation] * - the attribute is a user attribute, and neither target nor repetition * have been validated. */ @@ -7358,7 +7360,7 @@ ZEND_METHOD(ReflectionAttribute, newInstance) RETURN_THROWS(); } - /* Run the delayed validator function for internal attributes */ + /* Report the delayed validator error for internal attributes */ if (delayed_target_validation && ce->type == ZEND_INTERNAL_CLASS) { zend_string *error = attr->data->validation_error; if (error != NULL) { From 877fcfe4b97b51b44427289a575589c027cb9da0 Mon Sep 17 00:00:00 2001 From: Daniel Scherzer Date: Sat, 16 Aug 2025 08:18:13 -0700 Subject: [PATCH 19/23] Happy path at the bottom --- Zend/zend_attributes.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/Zend/zend_attributes.c b/Zend/zend_attributes.c index 0fa1f53fb9c8d..8ca46bc0de269 100644 --- a/Zend/zend_attributes.c +++ b/Zend/zend_attributes.c @@ -218,13 +218,13 @@ static zend_string *validate_nodiscard( { ZEND_ASSERT(CG(in_compilation)); const zend_string *prop_info_name = CG(context).active_property_info_name; - if (prop_info_name == NULL) { - zend_op_array *op_array = CG(active_op_array); - op_array->fn_flags |= ZEND_ACC_NODISCARD; - return NULL; + if (prop_info_name != NULL) { + // Applied to a hook + return ZSTR_INIT_LITERAL("#[\\NoDiscard] is not supported for property hooks", 0); } - // Applied to a hook - return ZSTR_INIT_LITERAL("#[\\NoDiscard] is not supported for property hooks", 0); + zend_op_array *op_array = CG(active_op_array); + op_array->fn_flags |= ZEND_ACC_NODISCARD; + return NULL; } ZEND_METHOD(NoDiscard, __construct) From b2b24fd955c48a092ba9d7dcc334f242f43f4b13 Mon Sep 17 00:00:00 2001 From: Daniel Scherzer Date: Sat, 16 Aug 2025 19:41:36 -0700 Subject: [PATCH 20/23] Simplify reflection for non-debug builds --- ext/reflection/php_reflection.c | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/ext/reflection/php_reflection.c b/ext/reflection/php_reflection.c index 68b3593a63c9b..17b6ffd7cdc78 100644 --- a/ext/reflection/php_reflection.c +++ b/ext/reflection/php_reflection.c @@ -7321,12 +7321,6 @@ ZEND_METHOD(ReflectionAttribute, newInstance) RETURN_THROWS(); } - zend_attribute *delayed_target_validation = zend_get_attribute_str( - attr->attributes, - "delayedtargetvalidation", - strlen("delayedtargetvalidation") - ); - /* This code can be reached under one of three possible conditions: * - the attribute is an internal attribute, and it had the target and * and repetition validated already @@ -7360,15 +7354,23 @@ ZEND_METHOD(ReflectionAttribute, newInstance) RETURN_THROWS(); } - /* Report the delayed validator error for internal attributes */ - if (delayed_target_validation && ce->type == ZEND_INTERNAL_CLASS) { - zend_string *error = attr->data->validation_error; - if (error != NULL) { - zend_throw_exception(zend_ce_error, ZSTR_VAL(error), 0); - RETURN_THROWS(); - } - } else { - ZEND_ASSERT(attr->data->validation_error == NULL); + if (attr->data->validation_error != NULL) { + /* Delayed validation errors should only be set for internal attributes. */ + ZEND_ASSERT(ce->type == ZEND_INTERNAL_CLASS); + /* Delayed validation errors should only be set when + * #[\DelayedTargetValidation] is used. Searching for the attribute is + * more expensive than just an assertion and so we don't worry about it + * for non-debug builds. See discussion on GH-18817. */ +#if ZEND_DEBUG + zend_attribute *delayed_target_validation = zend_get_attribute_str( + attr->attributes, + "delayedtargetvalidation", + strlen("delayedtargetvalidation") + ); + ZEND_ASSERT(delayed_target_validation != NULL); +#endif + zend_throw_exception(zend_ce_error, ZSTR_VAL(attr->data->validation_error), 0); + RETURN_THROWS(); } /* Repetition validation is done even if #[DelayedTargetValidation] is used From 72dc9df87858c803797e36a8decb5f84905ce0a9 Mon Sep 17 00:00:00 2001 From: Daniel Scherzer Date: Sat, 16 Aug 2025 19:49:41 -0700 Subject: [PATCH 21/23] zend_persist_attributes --- ext/opcache/zend_persist.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/ext/opcache/zend_persist.c b/ext/opcache/zend_persist.c index eb339ee5a4f4b..38e58d5a16632 100644 --- a/ext/opcache/zend_persist.c +++ b/ext/opcache/zend_persist.c @@ -311,6 +311,9 @@ static HashTable *zend_persist_attributes(HashTable *attributes) zend_accel_store_interned_string(copy->name); zend_accel_store_interned_string(copy->lcname); + if (copy->validation_error) { + zend_accel_store_interned_string(copy->validation_error); + } for (i = 0; i < copy->argc; i++) { if (copy->args[i].name) { From 74b10f922daf41f33108ab2b7517b9031af97b81 Mon Sep 17 00:00:00 2001 From: Daniel Scherzer Date: Sun, 17 Aug 2025 07:52:34 -0700 Subject: [PATCH 22/23] Suggestions from TimWolla --- Zend/zend_attributes.c | 8 ++------ Zend/zend_compile.c | 2 +- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/Zend/zend_attributes.c b/Zend/zend_attributes.c index 8ca46bc0de269..4777e5ca08ad1 100644 --- a/Zend/zend_attributes.c +++ b/Zend/zend_attributes.c @@ -85,9 +85,7 @@ static zend_string *validate_allow_dynamic_properties( msg = "Cannot apply #[\\AllowDynamicProperties] to enum %s"; } if (msg != NULL) { - smart_str str = {0}; - smart_str_append_printf(&str, msg, ZSTR_VAL(scope->name)); - return smart_str_extract(&str); + return zend_strpprintf(0, msg, ZSTR_VAL(scope->name)); } scope->ce_flags |= ZEND_ACC_ALLOW_DYNAMIC_PROPERTIES; return NULL; @@ -107,9 +105,7 @@ static zend_string *validate_attribute( msg = "Cannot apply #[\\Attribute] to abstract class %s"; } if (msg != NULL) { - smart_str str = {0}; - smart_str_append_printf(&str, msg, ZSTR_VAL(scope->name)); - return smart_str_extract(&str); + return zend_strpprintf(0, msg, ZSTR_VAL(scope->name)); } return NULL; } diff --git a/Zend/zend_compile.c b/Zend/zend_compile.c index e675f8ae6755c..126e8f1644b68 100644 --- a/Zend/zend_compile.c +++ b/Zend/zend_compile.c @@ -7604,7 +7604,7 @@ static void zend_compile_attributes( if (run_validator && config->validator != NULL) { zend_string *error = config->validator(attr, target, CG(active_class_entry)); if (error != NULL) { - if (delayed_target_validation == false) { + if (delayed_target_validation == NULL) { zend_error_noreturn(E_COMPILE_ERROR, "%s", ZSTR_VAL(error)); zend_string_efree(error); } else { From 3dac774325c6bd0c20bf6a4c0665c65806dd7ca7 Mon Sep 17 00:00:00 2001 From: Daniel Scherzer Date: Sun, 17 Aug 2025 14:34:54 -0700 Subject: [PATCH 23/23] comment cleanup --- Zend/zend_compile.c | 4 ++-- ext/reflection/php_reflection.c | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Zend/zend_compile.c b/Zend/zend_compile.c index 126e8f1644b68..205023fa69b64 100644 --- a/Zend/zend_compile.c +++ b/Zend/zend_compile.c @@ -7561,7 +7561,7 @@ static void zend_compile_attributes( zend_attribute *delayed_target_validation = NULL; if (target == ZEND_ATTRIBUTE_TARGET_PARAMETER) { ZEND_ASSERT(offset >= 1); - // zend_get_parameter_attribute_str will add 1 too + /* zend_get_parameter_attribute_str will add 1 too */ delayed_target_validation = zend_get_parameter_attribute_str( *attributes, "delayedtargetvalidation", @@ -7600,7 +7600,7 @@ static void zend_compile_attributes( } } - // Validators are not run if the target is already invalid + /* Validators are not run if the target is already invalid */ if (run_validator && config->validator != NULL) { zend_string *error = config->validator(attr, target, CG(active_class_entry)); if (error != NULL) { diff --git a/ext/reflection/php_reflection.c b/ext/reflection/php_reflection.c index 17b6ffd7cdc78..ab89c47bbb21a 100644 --- a/ext/reflection/php_reflection.c +++ b/ext/reflection/php_reflection.c @@ -7358,9 +7358,9 @@ ZEND_METHOD(ReflectionAttribute, newInstance) /* Delayed validation errors should only be set for internal attributes. */ ZEND_ASSERT(ce->type == ZEND_INTERNAL_CLASS); /* Delayed validation errors should only be set when - * #[\DelayedTargetValidation] is used. Searching for the attribute is - * more expensive than just an assertion and so we don't worry about it - * for non-debug builds. See discussion on GH-18817. */ + * #[\DelayedTargetValidation] is used. Searching for the attribute is + * more expensive than just an assertion and so we don't worry about it + * for non-debug builds. See discussion on GH-18817. */ #if ZEND_DEBUG zend_attribute *delayed_target_validation = zend_get_attribute_str( attr->attributes,