Changeset 63358

Timestamp:

08/27/2026 09:28:56 AM (7 weeks ago)

Author:

swissspidy

Message:

Code Quality: Narrow post and term query return types by fields.

get_posts(), WP_Query::query() and WP_Term_Query::query() take the query arguments directly, so static analysis can resolve their result from the fields value. Only 'ids' was mapped; 'id=>parent' and every WP_Term_Query field are now covered too.

Two divergences in the cached WP_Query path had to be fixed for the 'id=>parent' type to hold: a query served from the post-queries cache returned parent IDs keyed by their post_parent:{$post_id} cache keys rather than by post ID, and left WP_Query::$posts null when the cached query matched nothing.

Developed in: ​https://github.com/WordPress/wordpress-develop/pull/13216

Props westonruter, swissspidy.
See #65817.

Location:

trunk

Files:

• src/wp-includes/class-wp-query.php (modified) (3 diffs)
• src/wp-includes/class-wp-term-query.php (modified) (5 diffs)
• src/wp-includes/post.php (modified) (1 diff)
• tests/phpunit/tests/post/getPosts.php (modified) (1 diff)
• tests/phpunit/tests/query/fieldsClause.php (modified) (1 diff)
• tests/phpunit/tests/term/query.php (modified) (1 diff)

trunk/src/wp-includes/class-wp-query.php

@@ -3296 +3296 @@
                                                 }
 
-                                                /** @var int[] */
-                                                $post_parents = wp_cache_get_multiple( $post_parent_cache_keys, 'posts' );
-
-                                                foreach ( $post_parents as $cache_key => $post_parent ) {
+                                                /** @var array<non-falsy-string, int|false> $cached_post_parents */
+                                                $cached_post_parents = wp_cache_get_multiple( $post_parent_cache_keys, 'posts' );
+
+                                                $post_parents = array();
+                                                $this->posts  = array();
+
+                                                foreach ( $cached_post_parents as $cache_key => $post_parent ) {
                                                         $obj              = new stdClass();
                                                         $obj->ID          = (int) str_replace( 'post_parent:', '', $cache_key );
@@ -3305 +3308 @@
 
                                                         $this->posts[] = $obj;
+
+                                                        $post_parents[ $obj->ID ] = $obj->post_parent;
                                                 }
 
@@ -3969 +3974 @@
          *
          * @phpstan-return (
-         *     $query is array{ fields: 'ids', ... } ? int[] : WP_Post[]
+         *     $query is array{ fields: 'ids', ... }
+         *         ? int[]
+         *         : ( $query is array{ fields: 'id=>parent', ... }
+         *             ? array<int, int>
+         *             : WP_Post[] )
          * )
          */

trunk/src/wp-includes/class-wp-term-query.php

@@ -301 +301 @@
          *
          * @param string|array $query Array or URL query string of parameters.
-         * @return WP_Term[]|int[]|string[]|string Array of terms, or number of terms as numeric string
-         *                                         when 'count' is passed to `$args['fields']`.
+         * @return WP_Term[]|int[]|string[]|int|string Array of terms, or number of terms as numeric string
+         *                                             when 'count' is passed to `$args['fields']`, or the
+         *                                             integer 0 when the queried parent term is not in the
+         *                                             taxonomy hierarchy.
+         *
+         * @phpstan-return (
+         *     $query is array{ fields: 'count', ... }
+         *         ? 0|numeric-string
+         *         : ( $query is array{ fields: 'ids'|'tt_ids', ... }
+         *             ? int[]
+         *             : ( $query is array{ fields: 'id=>parent', ... }
+         *                 ? array<int, int>
+         *                 : ( $query is array{ fields: 'names'|'slugs', ... }
+         *                     ? string[]
+         *                     : ( $query is array{ fields: 'id=>name'|'id=>slug', ... }
+         *                         ? array<int, string>
+         *                         : WP_Term[] ) ) ) )
+         * )
          */
         public function query( $query ) {
@@ -319 +335 @@
          *   - 'all_with_object_id'
          *
-         * The following will result in a numeric string being returned:
+         * The following will result in a numeric string being returned, or the integer 0
+         * when the queried parent term is not in the taxonomy hierarchy:
          *
          *   - 'count'
@@ -330 +347 @@
          *   - 'slugs'
          *
-         * The following will result in an array of numeric strings being returned:
+         * The following will result in an array of integers being returned:
          *
          *   - 'id=>parent'
-         *
-         * The following will result in an array of integers being returned:
-         *
          *   - 'ids'
          *   - 'tt_ids'
@@ -343 +357 @@
          * @global wpdb $wpdb WordPress database abstraction object.
          *
-         * @return WP_Term[]|int[]|string[]|string Array of terms, or number of terms as numeric string
-         *                                         when 'count' is passed to `$args['fields']`.
+         * @return WP_Term[]|int[]|string[]|int|string Array of terms, or number of terms as numeric string
+         *                                             when 'count' is passed to `$args['fields']`, or the
+         *                                             integer 0 when the queried parent term is not in the
+         *                                             taxonomy hierarchy.
+         *
+         * @phpstan-return 0|numeric-string|int[]|array<int, int>|string[]|array<int, string>|WP_Term[]
          */
         public function get_terms() {
@@ -803 +821 @@
 
                 if ( 'count' === $_fields ) {
+                        // The request selects a single COUNT column, so it only returns null on a database error.
+                        /** @var numeric-string $count */
                         $count = $wpdb->get_var( $this->request ); // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared
                         if ( $args['cache_results'] ) {

trunk/src/wp-includes/post.php

@@ -2620 +2620 @@
  *
  * @phpstan-return (
- *     $args is array{ fields: 'ids', ... } ? int[] : WP_Post[]
+ *     $args is array{ fields: 'ids', ... }
+ *         ? int[]
+ *         : ( $args is array{ fields: 'id=>parent', ... }
+ *             ? array<int, int>
+ *             : WP_Post[] )
  * )
  */

trunk/tests/phpunit/tests/post/getPosts.php

@@ -177 +177 @@
                 $this->assertSame( array( $second_post_id ), $found_post_ids );
         }
+
+        /**
+         * Verifies what get_posts() returns for the `id=>parent` fields value.
+         *
+         * The second call is served from the `post-queries` cache, which previously returned
+         * the parent IDs keyed by the `post_parent:{$post_id}` cache keys they are stored
+         * under rather than by post ID.
+         *
+         * The cache hit is not specific to an external object cache being in use: the second
+         * call is served from the cache with the default in-memory implementation as well.
+         *
+         * @ticket 65817
+         */
+        public function test_should_return_parent_ids_keyed_by_post_id_when_cached(): void {
+                $parent_id = self::factory()->post->create( array( 'post_type' => 'page' ) );
+                $this->assertIsInt( $parent_id, 'The parent page was not created.' );
+
+                $child_id = self::factory()->post->create(
+                        array(
+                                'post_type'   => 'page',
+                                'post_parent' => $parent_id,
+                        )
+                );
+                $this->assertIsInt( $child_id, 'The child page was not created.' );
+
+                $args = array(
+                        'post_type'   => 'page',
+                        'fields'      => 'id=>parent',
+                        'numberposts' => -1,
+                        'orderby'     => 'ID',
+                        'order'       => 'ASC',
+                );
+
+                $expected = array(
+                        $parent_id => 0,
+                        $child_id  => $parent_id,
+                );
+
+                $this->assertSame( $expected, get_posts( $args ), 'The uncached call is not of the expected form.' );
+                $this->assertSame( $expected, get_posts( $args ), 'The cached call is not of the expected form.' );
+        }
 }

trunk/tests/phpunit/tests/query/fieldsClause.php

@@ -213 +213 @@
 
         /**
+         * Tests the value returned when the fields are limited to the ID and parent sub-set.
+         *
+         * The second query is served from the `post-queries` cache, which previously returned
+         * the parent IDs keyed by the `post_parent:{$post_id}` cache keys they are stored
+         * under rather than by post ID.
+         *
+         * The cache hit is not specific to an external object cache being in use: the second
+         * query is served from the cache with the default in-memory implementation as well.
+         *
+         * @ticket 65817
+         *
+         * @covers WP_Query::query
+         */
+        public function test_id_and_parent_subset_should_return_parent_ids_keyed_by_post_id_when_cached() {
+                $parent_id = self::factory()->post->create( array( 'post_type' => 'page' ) );
+                $this->assertIsInt( $parent_id, 'The parent page was not created.' );
+
+                $child_id = self::factory()->post->create(
+                        array(
+                                'post_type'   => 'page',
+                                'post_parent' => $parent_id,
+                        )
+                );
+                $this->assertIsInt( $child_id, 'The child page was not created.' );
+
+                $query_args = array(
+                        'post_type'      => 'page',
+                        'fields'         => 'id=>parent',
+                        'posts_per_page' => -1,
+                        'orderby'        => 'ID',
+                        'order'          => 'ASC',
+                );
+
+                $expected = array(
+                        $parent_id => 0,
+                        $child_id  => $parent_id,
+                );
+
+                $q1 = new WP_Query();
+                $this->assertSame( $expected, $q1->query( $query_args ), 'The uncached query is not of the expected form.' );
+
+                $q2 = new WP_Query();
+                $this->assertSame( $expected, $q2->query( $query_args ), 'The cached query is not of the expected form.' );
+        }
+
+        /**
+         * Tests the value returned when the fields are limited to the ID and parent sub-set
+         * and the query results are not cached.
+         *
+         * With caching disabled every query runs against the database, so the `post-queries`
+         * cache is never consulted.
+         *
+         * @ticket 65817
+         *
+         * @covers WP_Query::query
+         */
+        public function test_id_and_parent_subset_should_return_parent_ids_keyed_by_post_id_when_not_caching_results() {
+                $parent_id = self::factory()->post->create( array( 'post_type' => 'page' ) );
+                $this->assertIsInt( $parent_id, 'The parent page was not created.' );
+
+                $child_id = self::factory()->post->create(
+                        array(
+                                'post_type'   => 'page',
+                                'post_parent' => $parent_id,
+                        )
+                );
+                $this->assertIsInt( $child_id, 'The child page was not created.' );
+
+                $query_args = array(
+                        'post_type'      => 'page',
+                        'fields'         => 'id=>parent',
+                        'posts_per_page' => -1,
+                        'orderby'        => 'ID',
+                        'order'          => 'ASC',
+                        'cache_results'  => false,
+                );
+
+                $expected = array(
+                        $parent_id => 0,
+                        $child_id  => $parent_id,
+                );
+
+                $q1 = new WP_Query();
+                $this->assertSame( $expected, $q1->query( $query_args ), 'First query is not of the expected form.' );
+
+                $q2 = new WP_Query();
+                $this->assertSame( $expected, $q2->query( $query_args ), 'Second query is not of the expected form.' );
+        }
+
+        /**
+         * Tests the posts property when the fields are limited to the ID and parent sub-set
+         * and the query matches nothing.
+         *
+         * An empty result set is cached like any other, and the cached branch only ever
+         * appended to the posts property, so a second identical query left it as null where
+         * the first left it an empty array.
+         *
+         * @ticket 65817
+         */
+        public function test_id_and_parent_subset_should_populate_posts_property_when_cached_query_has_no_results() {
+                $query_args = array(
+                        'post_type' => 'wptests_pt',
+                        'fields'    => 'id=>parent',
+                        'name'      => 'this-slug-does-not-exist',
+                );
+
+                $q1 = new WP_Query();
+                $this->assertSame( array(), $q1->query( $query_args ), 'The uncached query did not return an empty array.' );
+                $this->assertSame( array(), $q1->posts, 'The posts property is not an empty array after the uncached query.' );
+
+                $q2 = new WP_Query();
+                $this->assertSame( array(), $q2->query( $query_args ), 'The cached query did not return an empty array.' );
+                $this->assertSame( array(), $q2->posts, 'The posts property is not an empty array after the cached query.' );
+        }
+
+        /**
          * Filters the posts fields.
          *

trunk/tests/phpunit/tests/term/query.php

@@ -1208 +1208 @@
                 $this->assertSame( ltrim( $q->request ), $q->request, 'The query has leading whitespace' );
         }
+
+        /**
+         * Verifies what WP_Term_Query::query() returns for the `id=>parent` fields value.
+         *
+         * Unlike WP_Query, both the uncached and the cached path run the results through
+         * WP_Term_Query::format_terms(), so the parent IDs are keyed by term ID either way.
+         *
+         * The assertion is repeated for a second query so that the cached path is covered
+         * both when an external object cache is in use and when it is not.
+         *
+         * @ticket 65817
+         *
+         * @covers WP_Term_Query::query
+         */
+        public function test_id_and_parent_fields_should_return_parent_ids_keyed_by_term_id() {
+                register_taxonomy( 'wptests_tax_hierarchical', 'post', array( 'hierarchical' => true ) );
+
+                $parent_id = self::factory()->term->create( array( 'taxonomy' => 'wptests_tax_hierarchical' ) );
+                $this->assertIsInt( $parent_id, 'The parent term was not created.' );
+
+                $child_id = self::factory()->term->create(
+                        array(
+                                'taxonomy' => 'wptests_tax_hierarchical',
+                                'parent'   => $parent_id,
+                        )
+                );
+                $this->assertIsInt( $child_id, 'The child term was not created.' );
+
+                $query_args = array(
+                        'taxonomy'   => 'wptests_tax_hierarchical',
+                        'fields'     => 'id=>parent',
+                        'hide_empty' => false,
+                        'orderby'    => 'term_id',
+                        'order'      => 'ASC',
+                );
+
+                $expected = array(
+                        $parent_id => 0,
+                        $child_id  => $parent_id,
+                );
+
+                $q1 = new WP_Term_Query();
+                $this->assertSame( $expected, $q1->query( $query_args ), 'First query is not of the expected form.' );
+
+                $q2 = new WP_Term_Query();
+                $this->assertSame( $expected, $q2->query( $query_args ), 'Second query is not of the expected form.' );
+        }
 }