Opened 5 weeks ago
Last modified 2 days ago
#66052 new defect (bug)
wp_list_pages: incorrect description of $args['include']
| Reported by: | marian1 | Owned by: | |
|---|---|---|---|
| Priority: | normal | Milestone: | 7.2 |
| Component: | General | Version: | |
| Severity: | normal | Keywords: | has-patch has-unit-tests changes-requested |
| Cc: | Focuses: | docs |
Description
The 'include' key of the $args parameter is documented as
@type array $include Comma-separated list of page IDs to include. Default empty
See the function reference.
While the documented type is array, the description says "Comma-separated list of page IDs" which would imply string.
$args['include'] is neither expected to be a comma separated list nor does it have an empty default value. It is expected to be an array of page IDs as correctly indicated by the documented type array. wp_list_pages() does not handle $args['include'] at all. It is passed through to get_pages() which expects $args['include'] to be int[] defaulting to an empty array and describes it as "Array of page IDs to include.".
In contrast, $args['exclude'] is expected to be a comma-separated list of page IDs, defaults to an empty string, and is documented accordingly.
Change History (8)
#2
follow-up:
↓ 3
@
5 weeks ago
@westonruter Maybe related: In php-stubs/wordpress-stubs, functions that take an $args argument referencing another function inherit the shape from the referenced function. In PR #491, we identified a couple of functions where the value type of a shape key differs between the referencing function and the referenced function. Maybe worth a look?!
#3
in reply to: ↑ 2
@
5 weeks ago
@marian1 Sounds great, yes. In looking at the first example of wp_dropdown_users(), I see that it documents $orderby as just a string, but the referenced WP_User_Query::prepare_query() widens this to string|array for the same arg. I see that string|array is under-specific as well. According to the docs, it can be:
May be a single value, an array of values, or a multi-dimensional array with fields as keys and orders ('ASC' or 'DESC') as values.
(I don't see how it is actually multi-dimensional, however.) The correct shape should appear to be at least string|string[]|array<string, string>. Naturally it could be made much narrower with enumerated string literals.
This ticket was mentioned in PR #13431 on WordPress/wordpress-develop by @irozum.
5 weeks ago
#4
- Keywords has-patch has-unit-tests added; needs-patch removed
The include entry in wp_list_pages()'s $args docblock was documented as @type array $include Comma-separated list of page IDs to include. Default empty, which mismatches both its documented type (array) and its actual behavior — $args['include'] is passed straight through to get_pages(), which expects an int[] of page IDs defaulting to an empty array, not a comma-separated string. This corrects the docblock to @type int[]|string $include Array or comma-separated list of page IDs to include. Default empty array., matching how get_pages() documents the same argument, and adds a unit test asserting that wp_list_pages() correctly accepts an array of page IDs for include.
## Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Sonnet 5
Used for: Implementation, tests, and PR description. Reviewed by Igor Rozum.
This ticket was mentioned in PR #13922 on WordPress/wordpress-develop by @sabbir345.
10 days ago
#6
wp_list_pages() cleaned its exclude argument with preg_replace() and then split it with explode(). Both functions expect a string, so passing an array of page IDs caused a fatal TypeError on PHP 8:
wp_list_pages( array( 'exclude' => array( 2, 5 ) ) ); // Fatal error on PHP 8.
Passing an array is a reasonable thing to do: get_pages(), which receives these arguments, accepts arrays. The include argument of wp_list_pages() is also already documented as an array.
Changes
excludeis now parsed withwp_parse_id_list(), so it accepts an array of IDs (integers or numeric strings) as well as the existing comma-separated string.- Backward compatible: the
wp_list_pages_excludesfilter still receives an array of numeric strings, as before. A@phpstan-param numeric-string[]tag now documents this. - The
excludedefault changes from''toarray(). - Docs:
excludeandincludeare now documented asint[]to matchget_pages().includewas typed asarraybut described as a comma-separated list. Added a@since 7.2.0note.
Testing
New PHPUnit tests in tests/phpunit/tests/post/wpListPages.php:
excludeaccepts an array of IDs, as integers or as numeric strings.includeaccepts an array of IDs, as integers or as numeric strings.- The
wp_list_pages_excludesfilter still receives numeric strings when given a comma-separated string with extra spaces.
npm run test:php -- --filter Tests_Post_wpListPages
Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Opus 5.5
Used for: Help with the implementation, tests, and commit/PR wording. I reviewed, tested, and take responsibility for all changes.
---
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.
🤖 Generated with Claude Code
@therssoftware commented on PR #13922:
10 days ago
#7
Test Report
Ticket: https://core.trac.wordpress.org/ticket/66052
PR: https://github.com/WordPress/wordpress-develop/pull/13922
Environment
- WordPress: 7.2-alpha-63166-src (trunk)
- PHP: 8.3.33
- Server: macOS Darwin 24.6.0 (arm64)
- Database: MariaDB 10.11 / MySQL
- PHPUnit: 9.6.37
Steps to Test
- On
trunk(before patch):- Execute
wp_list_pages()passing an array of page IDs toexclude:wp_list_pages( array( 'echo' => false, 'exclude' => array( 1, 2 ) ) );
- Execute
- Observe a fatal
TypeErroron PHP 8+:Fatal error: Uncaught TypeError: explode(): Argument #2 ($string) must be of type string, array given in src/wp-includes/post-template.php:1349
- Apply PR #13922:
- Execute the same call with an array of integer IDs:
wp_list_pages( array( 'echo' => false, 'exclude' => array( 1, 2 ) ) );
- Execute the same call with an array of integer IDs:
- Execute with an array of numeric strings:
wp_list_pages( array( 'echo' => false, 'exclude' => array( '1', '2' ) ) );
- Execute with legacy comma-separated string (with spaces):
wp_list_pages( array( 'echo' => false, 'exclude' => ' 1, 2, 3 ' ) );
- Verify that the
wp_list_pages_excludesfilter continues to receive an array of numeric strings.
- Run automated test suite:
vendor/bin/phpunit tests/phpunit/tests/post/wpListPages.php vendor/bin/phpcs src/wp-includes/post-template.php tests/phpunit/tests/post/wpListPages.php
Test Results
- Before Patch (
trunk):- Reproducible fatal error:
Fatal error: Uncaught TypeError: explode(): Argument #2 ($string) must be of type string, array given in src/wp-includes/post-template.php:1349 Stack trace: #0 src/wp-includes/post-template.php(1349): explode(',', Array) #1 wp_list_pages(Array)
- Reproducible fatal error:
- After Patch (PR #13922):
- Passing an array of IDs (both integers and numeric strings) to
excludeworks as expected without any error or notice. - Legacy comma-separated strings (
'1, 2, 3') continue to exclude pages properly (verified viatest_wp_list_pages_exclude). - The
wp_list_pages_excludesfilter receivesarray( '1', '2', '3' )as expected for backward compatibility. - All 23 tests and 27 assertions in
tests/phpunit/tests/post/wpListPages.phppass:test_wp_list_pages_exclude_should_accept_an_array_of_idswithintegerspassed.test_wp_list_pages_exclude_should_accept_an_array_of_idswithnumeric stringspassed.test_wp_list_pages_include_should_accept_an_array_of_idswithintegerspassed.test_wp_list_pages_include_should_accept_an_array_of_idswithnumeric stringspassed.test_wp_list_pages_excludes_filter_should_receive_numeric_stringspassed.
- PHPCS analysis with
WordPress-Coreruleset passes with 0 errors and 0 warnings on modified code.
- Passing an array of IDs (both integers and numeric strings) to
The PR is complete, maintains backward compatibility, and is ready for commit.
Props therssoftware.
---
AI Disclosure: In accordance with the WordPress AI policy, I disclose that generative AI (Google Antigravity) was used in testing and drafting this test report. All test steps and assertions were run and verified in the local development environment.
![(please configure the [header_logo] section in trac.ini)](/chrome/site/your_project_logo.png)
Related:
wp_list_pages()should be updated to usewp_parse_id_list()instead of doing this:It can be:
In this way, the
excludeparam can actually be widened to bestring|array<int|numeric-string>, which preserves existing documented type, and then also includingstringin what it previously actually required.Note that the args then get passed into
get_pages()which is documented to take:So actually what
wp_list_pages()is passingget_pages()is documented as being wrong. Ultimately, it doesn't matter here either becauseget_pages()useswp_parse_id_list()also. Therefore,get_pages()could have its type widened as well to bestring|array<int|numeric-string>. However, I'd rather not widen something that is already documented to be more specific.This being said, I think
wp_list_pages()could be updated to matchget_pages()in what it is documented to take for theincludeandexcludeargs. I think it's fine that it also allows a comma-separated list of IDs in a string or an array of numeric-strings, but it's better to document the ideal type to encourage authors toward better static analysis. In the end, when an author passes a string, PHPStan can't see into its shape and it can't flag any problematic usage. It can, however, withint[].🧠 (AI was not used in the authoring of this comment.)