From b21b9b6c555c4787d2a99149b9114e76c5bca3e8 Mon Sep 17 00:00:00 2001 From: Brett McBride Date: Thu, 17 Apr 2025 14:58:06 +1000 Subject: [PATCH 1/4] fix ClassEntry typehints - for Arguments, name and class_type were using literal values rather than the expected value - for ReturType, class_type was using the literal value rather than the expected value Fix and add tests --- phper-sys/php_wrapper.c | 22 ++++++++++++++++++---- tests/integration/src/typehints.rs | 8 ++++++-- tests/integration/tests/php/typehints.php | 17 ++++++++++------- 3 files changed, 34 insertions(+), 13 deletions(-) diff --git a/phper-sys/php_wrapper.c b/phper-sys/php_wrapper.c index 555aaeab..1ba1e318 100644 --- a/phper-sys/php_wrapper.c +++ b/phper-sys/php_wrapper.c @@ -516,12 +516,20 @@ phper_zend_begin_arg_with_return_obj_info_ex(bool return_reference, #define static #define const #if PHP_VERSION_ID >= 80000 - ZEND_BEGIN_ARG_WITH_RETURN_OBJ_INFO_EX(info, return_reference, required_num_args, class_name, allow_null) + zend_string *zstr = zend_string_init(class_name, strlen(class_name), /*persistent*/ 1); + //this macro uses class_name as a literal, so we overwrite it immediately + ZEND_BEGIN_ARG_WITH_RETURN_OBJ_INFO_EX(infos, return_reference, required_num_args, class_name, allow_null) + ZEND_END_ARG_INFO() + zend_internal_arg_info info = infos[0]; + info.type.ptr = zstr; + info.type.type_mask = _ZEND_TYPE_NAME_BIT | (allow_null ? MAY_BE_NULL : 0); + return info; #else ZEND_BEGIN_ARG_INFO_EX(info, 0, return_reference, required_num_args) -#endif ZEND_END_ARG_INFO() return info[0]; +#endif + #undef static #undef const } @@ -560,10 +568,16 @@ zend_internal_arg_info phper_zend_arg_obj_info(bool pass_by_ref, (void)class_name; (void)allow_null; #if PHP_VERSION_ID >= 80000 - zend_internal_arg_info info[] = { + zend_string *zstr = zend_string_init(class_name, strlen(class_name), /*persistent*/ 1); + //this macro uses name and class_name as literals, so we overwrite them immediately + zend_internal_arg_info infos[] = { ZEND_ARG_OBJ_INFO_WITH_DEFAULT_VALUE(pass_by_ref, name, class_name, allow_null, NULL) }; - return info[0]; + zend_internal_arg_info info = infos[0]; + info.name = name; + info.type.ptr = zstr; + info.type.type_mask = _ZEND_TYPE_NAME_BIT | (allow_null ? MAY_BE_NULL : 0); + return info; #elif PHP_VERSION_ID >= 70200 zend_internal_arg_info info = { .name = name, diff --git a/tests/integration/src/typehints.rs b/tests/integration/src/typehints.rs index d8334a00..37268fb3 100644 --- a/tests/integration/src/typehints.rs +++ b/tests/integration/src/typehints.rs @@ -58,6 +58,10 @@ pub fn integrate(module: &mut Module) { .with_type_hint(ArgumentTypeHint::Mixed) .with_default_value("1.23"), ) + .argument( + Argument::new("ce") + .with_type_hint(ArgumentTypeHint::ClassEntry(String::from("Stringable"))) + ) .return_type(ReturnType::new(ReturnTypeHint::Void)); } @@ -417,7 +421,7 @@ fn make_arg_typehint_class() -> ClassEntity<()> { phper::ok(()) }) .argument( - Argument::new("classentry") + Argument::new("some_class_entry") .with_type_hint(ArgumentTypeHint::ClassEntry(String::from(I_FOO))), ); @@ -426,7 +430,7 @@ fn make_arg_typehint_class() -> ClassEntity<()> { phper::ok(()) }) .argument( - Argument::new("classentry") + Argument::new("some_class_entry") .with_type_hint(ArgumentTypeHint::ClassEntry(String::from(I_FOO))) .allow_null(), ); diff --git a/tests/integration/tests/php/typehints.php b/tests/integration/tests/php/typehints.php index 92cb63db..13f33fb7 100644 --- a/tests/integration/tests/php/typehints.php +++ b/tests/integration/tests/php/typehints.php @@ -58,9 +58,9 @@ ['testNull', 'null', true, true, '8.2'], - ['testClassEntry', 'class_name', false, true, '8.0'], - ['testClassEntryOptional', 'class_name', false, false, '8.0'], - ['testClassEntryNullable', 'class_name', true, true, '8.0'], + ['testClassEntry', 'IntegrationTest\\TypeHints\\IFoo', false, true, '8.0'], + ['testClassEntryOptional', 'IntegrationTest\\TypeHints\\IFoo', false, false, '8.0'], + ['testClassEntryNullable', 'IntegrationTest\\TypeHints\\IFoo', true, true, '8.0'], ]; // typehints @@ -113,8 +113,8 @@ ['returnMixed', 'mixed', true, '8.0'], ['returnNever', 'never', false, '8.1'], ['returnVoid', 'void', false], - ['returnClassEntry', 'class_name', false, '8.0'], - ['returnClassEntryNullable', 'class_name', true, '8.0'], + ['returnClassEntry', 'IntegrationTest\\TypeHints\\IFoo', false, '8.0'], + ['returnClassEntryNullable', 'IntegrationTest\\TypeHints\\IFoo', true, '8.0'], ]; echo PHP_EOL . 'Testing return typehints' . PHP_EOL; $cls = new \IntegrationTest\TypeHints\ReturnTypeHintTest(); @@ -186,13 +186,14 @@ public function setValue($value): void { } $expectedArgs = [ + // , , ['s', 'string', 'foobarbaz'], ['i', 'int', 42], ['f', 'float', 7.89], ['b', 'bool', true], ['a', 'array', ['a'=>'b']], ['m', 'mixed', 1.23], - + ['ce', 'Stringable'], //default value not supported for ClassEntry ]; if (PHP_VERSION_ID >= 80000) { echo PHP_EOL . 'Testing function typehints' . PHP_EOL; @@ -202,7 +203,9 @@ public function setValue($value): void { echo(sprintf("argument %d..", $i)); assert_eq($input[0], $params[$i]->getName(), sprintf('argument %d has correct name', $i)); assert_eq($input[1], $params[$i]->getType()->getName(), sprintf('argument %d has correct type', $i)); - assert_eq($input[2], $params[$i]->getDefaultValue(), sprintf('argument %d has correct default value', $i)); + if ($input[2]) { + assert_eq($input[2], $params[$i]->getDefaultValue(), sprintf('argument %d has correct default value', $i)); + } echo "PASS" . PHP_EOL; } assert_eq('void', $reflection->getReturnType()->getName(), 'integration_function_typehints return type is void'); From 2664ba87f77bd26e6d11b59d015c6e0cd62683b4 Mon Sep 17 00:00:00 2001 From: Brett McBride Date: Thu, 17 Apr 2025 15:10:57 +1000 Subject: [PATCH 2/4] fmt --- tests/integration/src/typehints.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/src/typehints.rs b/tests/integration/src/typehints.rs index 37268fb3..99f0f24c 100644 --- a/tests/integration/src/typehints.rs +++ b/tests/integration/src/typehints.rs @@ -60,7 +60,7 @@ pub fn integrate(module: &mut Module) { ) .argument( Argument::new("ce") - .with_type_hint(ArgumentTypeHint::ClassEntry(String::from("Stringable"))) + .with_type_hint(ArgumentTypeHint::ClassEntry(String::from("Stringable"))), ) .return_type(ReturnType::new(ReturnTypeHint::Void)); } From 1f06ebec91ca0c8ac76aa93eb1e5a0086f5d20b0 Mon Sep 17 00:00:00 2001 From: Brett McBride Date: Thu, 17 Apr 2025 15:18:54 +1000 Subject: [PATCH 3/4] use non-default-value macro --- phper-sys/php_wrapper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phper-sys/php_wrapper.c b/phper-sys/php_wrapper.c index 1ba1e318..9ff58b02 100644 --- a/phper-sys/php_wrapper.c +++ b/phper-sys/php_wrapper.c @@ -571,7 +571,7 @@ zend_internal_arg_info phper_zend_arg_obj_info(bool pass_by_ref, zend_string *zstr = zend_string_init(class_name, strlen(class_name), /*persistent*/ 1); //this macro uses name and class_name as literals, so we overwrite them immediately zend_internal_arg_info infos[] = { - ZEND_ARG_OBJ_INFO_WITH_DEFAULT_VALUE(pass_by_ref, name, class_name, allow_null, NULL) + ZEND_ARG_OBJ_INFO(pass_by_ref, name, class_name, allow_null) }; zend_internal_arg_info info = infos[0]; info.name = name; From c8b9636513bf2b4b12df2c9a401d34f17c2185bc Mon Sep 17 00:00:00 2001 From: Brett McBride Date: Thu, 17 Apr 2025 16:42:30 +1000 Subject: [PATCH 4/4] fix 8.0-8.2 --- phper-sys/php_wrapper.c | 12 ++++++++++-- tests/integration/tests/php/typehints.php | 2 +- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/phper-sys/php_wrapper.c b/phper-sys/php_wrapper.c index 9ff58b02..a12012d0 100644 --- a/phper-sys/php_wrapper.c +++ b/phper-sys/php_wrapper.c @@ -521,7 +521,11 @@ phper_zend_begin_arg_with_return_obj_info_ex(bool return_reference, ZEND_BEGIN_ARG_WITH_RETURN_OBJ_INFO_EX(infos, return_reference, required_num_args, class_name, allow_null) ZEND_END_ARG_INFO() zend_internal_arg_info info = infos[0]; - info.type.ptr = zstr; + #if PHP_VERSION_ID >= 80300 + info.type.ptr = zstr; + #else + info.type.ptr = ZSTR_VAL(zstr); + #endif info.type.type_mask = _ZEND_TYPE_NAME_BIT | (allow_null ? MAY_BE_NULL : 0); return info; #else @@ -575,7 +579,11 @@ zend_internal_arg_info phper_zend_arg_obj_info(bool pass_by_ref, }; zend_internal_arg_info info = infos[0]; info.name = name; - info.type.ptr = zstr; + #if PHP_VERSION_ID >= 80300 + info.type.ptr = zstr; + #else + info.type.ptr = ZSTR_VAL(zstr); + #endif info.type.type_mask = _ZEND_TYPE_NAME_BIT | (allow_null ? MAY_BE_NULL : 0); return info; #elif PHP_VERSION_ID >= 70200 diff --git a/tests/integration/tests/php/typehints.php b/tests/integration/tests/php/typehints.php index 13f33fb7..1aa07c6e 100644 --- a/tests/integration/tests/php/typehints.php +++ b/tests/integration/tests/php/typehints.php @@ -203,7 +203,7 @@ public function setValue($value): void { echo(sprintf("argument %d..", $i)); assert_eq($input[0], $params[$i]->getName(), sprintf('argument %d has correct name', $i)); assert_eq($input[1], $params[$i]->getType()->getName(), sprintf('argument %d has correct type', $i)); - if ($input[2]) { + if (array_key_exists(2, $input)) { assert_eq($input[2], $params[$i]->getDefaultValue(), sprintf('argument %d has correct default value', $i)); } echo "PASS" . PHP_EOL;