Make WordPress Core

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)

#1 @westonruter
5 weeks ago

  • Keywords needs-patch added
  • Milestone Awaiting Review → 7.2

Related:

wp_list_pages() should be updated to use wp_parse_id_list() instead of doing this:

<?php
// Sanitize, mostly to keep spaces out.
$parsed_args['exclude'] = preg_replace( '/[^0-9,]/', '', $parsed_args['exclude'] );

// Allow plugins to filter an array of excluded pages (but don't put a nullstring into the array).
$exclude_array = ( $parsed_args['exclude'] ) ? explode( ',', $parsed_args['exclude'] ) : array();

/**
 * Filters the array of pages to exclude from the pages list.
 *
 * @since 2.1.0
 *
 * @param string[] $exclude_array An array of page IDs to exclude.
 */
$parsed_args['exclude'] = implode( ',', apply_filters( 'wp_list_pages_excludes', $exclude_array ) );

It can be:

<?php
$exclude_array = array_map( 'strval', wp_parse_id_list( $parsed_args['exclude'] ) );

/**
 * Filters the array of pages to exclude from the pages list.
 *
 * @since 2.1.0
 *
 * @param string[] $exclude_array An array of page IDs to exclude.
 * @phpstan-param numeric-string[] $exclude_array
 */
$parsed_args['exclude'] = implode( ',', apply_filters( 'wp_list_pages_excludes', $exclude_array ) );

In this way, the exclude param can actually be widened to be string|array<int|numeric-string>, which preserves existing documented type, and then also including string in what it previously actually required.

Note that the args then get passed into get_pages() which is documented to take:

 *     @type int[]        $exclude      Array of page IDs to exclude. Default empty array.
 *     @type int[]        $include      Array of page IDs to include. Cannot be used with `$child_of`,
 *                                      `$parent`, `$exclude`, `$meta_key`, `$meta_value`, or `$hierarchical`.
 *                                      Default empty array.

So actually what wp_list_pages() is passing get_pages() is documented as being wrong. Ultimately, it doesn't matter here either because get_pages() uses wp_parse_id_list() also. Therefore, get_pages() could have its type widened as well to be string|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 match get_pages() in what it is documented to take for the include and exclude args. 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, with int[].

🧠 (AI was not used in the authoring of this comment.)

#2 follow-up: @marian1
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 @westonruter
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.

#5 @westonruter
5 weeks ago

  • Keywords changes-requested added

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

  • exclude is now parsed with wp_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_excludes filter still receives an array of numeric strings, as before. A @phpstan-param numeric-string[] tag now documents this.
  • The exclude default changes from '' to array().
  • Docs: exclude and include are now documented as int[] to match get_pages(). include was typed as array but described as a comma-separated list. Added a @since 7.2.0 note.

Testing

New PHPUnit tests in tests/phpunit/tests/post/wpListPages.php:

  • exclude accepts an array of IDs, as integers or as numeric strings.
  • include accepts an array of IDs, as integers or as numeric strings.
  • The wp_list_pages_excludes filter 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

  1. On trunk (before patch):
    • Execute wp_list_pages() passing an array of page IDs to exclude:
      wp_list_pages( array( 'echo' => false, 'exclude' => array( 1, 2 ) ) );
      
  • Observe a fatal TypeError on 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
    
  1. Apply PR #13922:
    • Execute the same call with an array of integer IDs:
      wp_list_pages( array( 'echo' => false, 'exclude' => array( 1, 2 ) ) );
      
  • 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_excludes filter continues to receive an array of numeric strings.
  1. 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)
      
  • After Patch (PR #13922):
    • Passing an array of IDs (both integers and numeric strings) to exclude works as expected without any error or notice.
    • Legacy comma-separated strings ('1, 2, 3') continue to exclude pages properly (verified via test_wp_list_pages_exclude).
    • The wp_list_pages_excludes filter receives array( '1', '2', '3' ) as expected for backward compatibility.
    • All 23 tests and 27 assertions in tests/phpunit/tests/post/wpListPages.php pass:
      • test_wp_list_pages_exclude_should_accept_an_array_of_ids with integers passed.
      • test_wp_list_pages_exclude_should_accept_an_array_of_ids with numeric strings passed.
      • test_wp_list_pages_include_should_accept_an_array_of_ids with integers passed.
      • test_wp_list_pages_include_should_accept_an_array_of_ids with numeric strings passed.
      • test_wp_list_pages_excludes_filter_should_receive_numeric_strings passed.
    • PHPCS analysis with WordPress-Core ruleset passes with 0 errors and 0 warnings on modified code.

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.

#8 @sabbir345
2 days ago

@westonruter PR #13922 implements the approach you suggested in comment:1
(wp_parse_id_list() for exclude, int[] docs for include/exclude).
It has tests and a test report. Could you take a look? The changes-requested
keyword was for PR #13431.

Note: See TracTickets for help on using tickets.