Zend: Add missing ZPP specifier tests by timsurrealedu · Pull Request #23343 · php/php-src · GitHub

Adds missing test coverage for Zend Parameter Parsing (ZPP) specifiers as outlined in GH-23280.

Summary of Changes

Added helper functions to ext/zend_test (test.stub.php and test.c) to exercise missing ZPP specifiers (Z_PARAM_ENUM, Z_PARAM_ARRAY*, Z_PARAM_ARRAY_HT*, Z_PARAM_ARRAY_OR_OBJECT*, Z_PARAM_FUNC*, Z_PARAM_PATH*, Z_PARAM_STRING*, Z_PARAM_STR*, Z_PARAM_ZVAL*, Z_PARAM_VARIADIC*, and union specifiers).
Added .phpt test cases in Zend/tests/zpp/ covering both strict and weak type modes.

Suggested change

      
            		echo $e::class, ': ', $e->getMessage(), PHP_EOL;
          
            		echo $e::class, ': ', $e->getMessage(), "\n";

Would be nice to change all these assertions to use "\n". Context: #23316 and #22799

Your local paths are leaking in here.

Thanks for the review, @NickSdot! Both points are addressed in the latest push (8da2451a):

Local path leak — the --EXPECTF-- sections that hardcoded the anonymous class name (string(106) "class@anonymous%0/home/timsurreal/.../types.inc:9$1") now use the canonical string(%d) "class@anonymous%s" pattern, matching the neighboring ZPP tests. This was the cause of all 10 CI failures (the hardcoded path and byte length differed on CI).
PHP_EOL → "\n" — all assertion echo lines in the newly added tests now use "\n" instead of PHP_EOL per Zend: applied fixers to improve test robustness (8/8) #23316 / Tests: Test exception type in error tests #22799.

All 59 tests in Zend/tests/zpp/ pass locally with zend_test enabled. The 8 failing platform checks should clear on re-run.

Was this generated by an LLM?

@Girgias I know in #23192 you also tested multiple functions in one file, but how would you feel about making it one function one file? The "Writing Tests" docs ask to keep files small, and I believe it makes sense.

Here specifically because:

all 'types.inc' assertions for one function grouped together
function targeted runs possible
comprehensible expect sections
tests are easier to find when they are named after their function

Would you be fine to require that here? I'd offer to send a follow up for yours.

@Girgias I know in #23192 you also tested multiple functions in one file, but how would you feel about making it one function one file? The "Writing Tests" docs ask to keep files small, and I believe it makes sense.

Here specifically because:
* all 'types.inc'  assertions for one function grouped together

* function targeted runs possible

* comprehensible expect sections

* tests are easier to find when they are named after their function
Would you be fine to require that here? I'd offer to send a follow up for yours.

Those are guidelines, and the point is to test ZPP not the functions that expose ZPP. So having multiple files here just makes everything harder for no reason.

Those are guidelines, and the point is to test ZPP not the functions that expose ZPP. So having multiple files here just makes everything harder for no reason.

Fair enough, no splitting. How about making them at least better readable then?

--TEST--
Test array_ht_or_long ZPP specifier (strict_mode)
--EXTENSIONS--
zend_test
--FILE--
<?php

declare(strict_types=1);

$types = require 'types.inc';

$functions = [
    'zend_array_ht_or_long',
    'zend_array_ht_or_long_or_null',
];

foreach ($functions as $i => $function) {

    echo "Using $function:\n\n";

    foreach ($types as $name => $type) {
        printf("  %-16s", "$name:");

        try {
            $result = $function($type);

            $result === [] 
                ? printf("array(0) {}\n")
                : var_dump($result);

        } catch (Throwable $e) {
            echo $e::class, ': ', $e->getMessage(), "\n";
        }
    }

    if ($i !== array_key_last($functions)) {
        echo "\n";
    }
}

?>
--EXPECT--
Using zend_array_ht_or_long:

  null:            TypeError: zend_array_ht_or_long(): Argument #1 ($param) must be of type array|int, null given
  false:           TypeError: zend_array_ht_or_long(): Argument #1 ($param) must be of type array|int, false given
  true:            TypeError: zend_array_ht_or_long(): Argument #1 ($param) must be of type array|int, true given
  42:              int(42)
  73.5:            TypeError: zend_array_ht_or_long(): Argument #1 ($param) must be of type array|int, float given
  'string':        TypeError: zend_array_ht_or_long(): Argument #1 ($param) must be of type array|int, string given
  '15':            TypeError: zend_array_ht_or_long(): Argument #1 ($param) must be of type array|int, string given
  '56.7':          TypeError: zend_array_ht_or_long(): Argument #1 ($param) must be of type array|int, string given
  'stdClass':      TypeError: zend_array_ht_or_long(): Argument #1 ($param) must be of type array|int, string given
  anon class name: TypeError: zend_array_ht_or_long(): Argument #1 ($param) must be of type array|int, string given
  []:              array(0) {}
  new stdClass():  TypeError: zend_array_ht_or_long(): Argument #1 ($param) must be of type array|int, stdClass given
  new S():         TypeError: zend_array_ht_or_long(): Argument #1 ($param) must be of type array|int, S given
  STDOUT:          TypeError: zend_array_ht_or_long(): Argument #1 ($param) must be of type array|int, resource given

Using zend_array_ht_or_long_or_null:

  null:            NULL
  false:           TypeError: zend_array_ht_or_long_or_null(): Argument #1 ($param) must be of type array|int|null, false given
  true:            TypeError: zend_array_ht_or_long_or_null(): Argument #1 ($param) must be of type array|int|null, true given
  42:              int(42)
  73.5:            TypeError: zend_array_ht_or_long_or_null(): Argument #1 ($param) must be of type array|int|null, float given
  'string':        TypeError: zend_array_ht_or_long_or_null(): Argument #1 ($param) must be of type array|int|null, string given
  '15':            TypeError: zend_array_ht_or_long_or_null(): Argument #1 ($param) must be of type array|int|null, string given
  '56.7':          TypeError: zend_array_ht_or_long_or_null(): Argument #1 ($param) must be of type array|int|null, string given
  'stdClass':      TypeError: zend_array_ht_or_long_or_null(): Argument #1 ($param) must be of type array|int|null, string given
  anon class name: TypeError: zend_array_ht_or_long_or_null(): Argument #1 ($param) must be of type array|int|null, string given
  []:              array(0) {}
  new stdClass():  TypeError: zend_array_ht_or_long_or_null(): Argument #1 ($param) must be of type array|int|null, stdClass given
  new S():         TypeError: zend_array_ht_or_long_or_null(): Argument #1 ($param) must be of type array|int|null, S given
  STDOUT:          TypeError: zend_array_ht_or_long_or_null(): Argument #1 ($param) must be of type array|int|null, resource given

The foreachcould potentially even be a function in types.inc then, so that the --FILE-- section, and hence each test, would look like (+ $ref stuff):

--TEST--
Test array_ht_or_long ZPP specifier (strict_mode)
--EXTENSIONS--
zend_test
--FILE--
<?php

declare(strict_types=1);

$types = require 'types.inc';

run_zpp_types([
    'zend_array_ht_or_long',
    'zend_array_ht_or_long_or_null',
]);

?>
--EXPECT--
...

This logically only affects the types.inc consumer tests. Acceptable middle ground?

Add tests for remaining Zend parameter parsing (ZPP) specifiers including arrays, strings, paths, callables, objects, enums, variadics, and zvals in both strict and weak type modes. Also add corresponding test helper functions in ext/zend_test. Closes phpGH-23280.

Hi @Girgias, @NickSdot,

Was this generated by an LLM?

Yes, I used an LLM as an assistant to help scaffold the boilerplate and repetitive test matrix combinations for the missing specifiers. The C helpers in ext/zend_test, the GC refcounting fixes, and all test expectations have been checked and verified against php-src.

I have updated all added tests to adopt @NickSdot's formatting proposal (grouping by function headers with aligned output columns) and squashed the branch into a single clean commit. All 59 ZPP tests pass cleanly.

The point of this issue was to help people to teach php-src. Not to have an LLM do the work for them. So I'm going to be very inclinded to close this PR as it defeats the whole purpose of giving people an avenue to actually learn how to contribute.

I have updated all added tests to adopt @NickSdot's formatting proposal (grouping by function headers with aligned output columns) and squashed the branch into a single clean commit. All 59 ZPP tests pass cleanly.

It seems you no longer test the $ref =& $type; cases. What I proposed was a draft to visualise what I mean (see the $ref note). The array/foreach still needs a way to define/run the relevant reference tested functions. I think having a function in types.inc would be neat and make the tests again more lean.

Though, @Girgias would anyway first need to approve what I propose. Personally, I find it much more clear now.

timsurrealedu requested a review from kocsismate as a code owner August 18, 2026 08:24

github-actions Bot added the Extension: zend_test label Aug 18, 2026

NickSdot reviewed Aug 18, 2026

View reviewed changes

Girgias requested changes Aug 18, 2026

View reviewed changes

timsurrealedu force-pushed the zpp-tests-missing branch from 8da2451 to a5922ee Compare August 19, 2026 12:50

+              string(8) "stdClass"
+              string(8) "stdClass"
+              Using anon class name:
+              string(106) "class@anonymous%0/home/timsurreal/Documents/mycode/githubContributions/php-src/Zend/tests/zpp/types.inc:9$1"

Zend: Add missing ZPP specifier tests by timsurrealedu · Pull Request #23343 · php/php-src · GitHub

Uh oh!

Zend: Add missing ZPP specifier tests - #23343

Zend: Add missing ZPP specifier tests#23343
timsurrealedu wants to merge 1 commit into
php:masterfrom
timsurrealedu:zpp-tests-missing

timsurrealedu commented Aug 18, 2026 •
edited
Loading

Uh oh!

Uh oh!

NickSdot Aug 18, 2026

Uh oh!

NickSdot Aug 18, 2026

Uh oh!

timsurrealedu commented Aug 18, 2026

Uh oh!

Girgias left a comment

Uh oh!

NickSdot commented Aug 18, 2026

Uh oh!

Girgias commented Aug 18, 2026

Uh oh!

NickSdot commented Aug 18, 2026

Uh oh!

timsurrealedu commented Aug 19, 2026

Uh oh!

Girgias commented Aug 19, 2026

Uh oh!

NickSdot commented Aug 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	echo $e::class, ': ', $e->getMessage(), PHP_EOL;
	echo $e::class, ': ', $e->getMessage(), "\n";

Zend: Add missing ZPP specifier tests by timsurrealedu · Pull Request #23343 · php/php-src · GitHub

Uh oh!

Zend: Add missing ZPP specifier tests - #23343

Zend: Add missing ZPP specifier tests#23343timsurrealedu wants to merge 1 commit intophp:masterphp/php-src:masterfrom timsurrealedu:zpp-tests-missingtimsurrealedu/php-src:zpp-tests-missingCopy head branch name to clipboard

Conversation

timsurrealedu commented Aug 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary of Changes

Uh oh!

NickSdot Aug 18, 2026

Choose a reason for hiding this comment

Uh oh!

NickSdot Aug 18, 2026

Choose a reason for hiding this comment

Uh oh!

timsurrealedu commented Aug 18, 2026

Uh oh!

Girgias left a comment

Choose a reason for hiding this comment

Uh oh!

NickSdot commented Aug 18, 2026

Uh oh!

Girgias commented Aug 18, 2026

Uh oh!

NickSdot commented Aug 18, 2026

Uh oh!

timsurrealedu commented Aug 19, 2026

Uh oh!

Girgias commented Aug 19, 2026

Uh oh!

NickSdot commented Aug 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Zend: Add missing ZPP specifier tests#23343
timsurrealedu wants to merge 1 commit into
php:masterfrom
timsurrealedu:zpp-tests-missing

timsurrealedu commented Aug 18, 2026 •
edited
Loading