Code Quality: Improve comment API type coverage.

Add a `Data_Array` array-shape type describing the keys returned by `WP_Comment::to_array()`, and narrow the properties it covers: `comment_approved` and the two datetime fields become `non-empty-string`, and the values core uses for it and for `comment_type` are documented. `comment_type` itself stays a plain `string`, because comments created before 5.5.0 may store an empty string rather than 'comment', which is why `get_comment_type()` normalizes that case on read.

Declare the 21 post fields that `WP_Comment::__get()` proxies to the comment's post as `@property-read`, typed to match the corresponding `WP_Post` property. These were previously invisible to static analysis, IDE completion, and the generated documentation. Also correct `WP_Comment::$children`, previously a bare `array`, which is `null` until `get_children()` populates it, and type the comment arrays keyed by comment ID as `array<int, WP_Comment>`, since PHP coerces the numeric string `comment_ID` to an integer key on assignment.

Add conditional return types to `WP_Comment::get_children()`, `get_comment()`, `get_comments()`, and `get_approved_comments()`, and document every argument `::get_children()` actually accepts, several of which were already in use but undocumented. Comment ID arrays are narrowed to `non-negative-int[]`, and `WP_Comment_Query::$comments` is typed as `null` until a query is run.

Several latent issues surfaced by the analysis are fixed:

* `WP_Comment::get_children()` now returns a `count` or `fields` query directly rather than storing it in the children cache. That cache holds `WP_Comment` objects and is read back by `add_child()`, `get_child()`, and the 'flat' format, so writing an integer or a list of IDs into it left the object returning the wrong thing on a subsequent call.
* `get_comment()` now hands only numeric values to `WP_Comment::get_instance()`. Previously anything that was not a `WP_Comment` or some other object fell through to be cast to an integer ID, even if it wasn't numeric. Now `null` is returned in such cases.
* `WP_Comment::get_instance()` ignores a non-object read from the comment cache rather than passing it to the `WP_Comment` constructor, where `get_object_vars()` would raise a `TypeError`.
* `WP_Comment::__isset()` returns `false`, and `WP_Comment::__get()` returns `null`, when the comment's post no longer exists, instead of raising a `TypeError` and a warning respectively. `__get()` also returns `null` when the comment is not attached to a post at all; previously `get_post( 0 )` fell back to the global `$post`, so the getter returned an unrelated post's field even though `__isset()` reported that same property as unset.

Developed in WordPress#12606.
Follow-up to r34583, r62648, r62694, r62717.

Props westonruter, adamsilverstein.
See #64898.


git-svn-id: https://develop.svn.wordpress.org/trunk@62822 602fd350-edb4-49c9-b593-d223f7449a82

Author: westonruter

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

@@ -93,8 +93,11 @@ class WP_Comment_Query {
 	/**
 	 * List of comments located by the query.
 	 *
+	 * Null until a query has been run.
+	 *
 	 * @since 4.0.0
-	 * @var int[]|WP_Comment[]
+	 * @var int[]|WP_Comment[]|null
+	 * @phpstan-var non-negative-int[]|array<int, WP_Comment>|null
 	 */
 	public $comments;
 
@@ -103,6 +106,7 @@ class WP_Comment_Query {
 	 *
 	 * @since 4.4.0
 	 * @var int
+	 * @phpstan-var non-negative-int
 	 */
 	public $found_comments = 0;
 
@@ -111,6 +115,7 @@ class WP_Comment_Query {
 	 *
 	 * @since 4.4.0
 	 * @var int
+	 * @phpstan-var non-negative-int
 	 */
 	public $max_num_pages = 0;
 
@@ -359,7 +364,8 @@ public function parse_query( $query = '' ) {
 	 * @since 4.2.0 Moved parsing to WP_Comment_Query::parse_query().
 	 *
 	 * @param string|array $query Array or URL query string of parameters.
-	 * @return array|int List of comments, or number of comments when 'count' is passed as a query var.
+	 * @return WP_Comment[]|int[]|int List of comments, or number of comments when 'count' is passed as a query var.
+	 * @phpstan-return array<int, WP_Comment>|non-negative-int[]|non-negative-int
 	 */
 	public function query( $query ) {
 		$this->query_vars = wp_parse_args( $query );
@@ -374,6 +380,7 @@ public function query( $query ) {
 	 * @global wpdb $wpdb WordPress database abstraction object.
 	 *
 	 * @return int|int[]|WP_Comment[] List of comments or number of found comments if `$count` argument is true.
+	 * @phpstan-return array<int, WP_Comment>|non-negative-int[]|non-negative-int
 	 */
 	public function get_comments() {
 		global $wpdb;
@@ -541,6 +548,7 @@ public function get_comments() {
 	 * @global wpdb $wpdb WordPress database abstraction object.
 	 *
 	 * @return int|array A single count of comment IDs if a count query. An array of comment IDs if a full query.
+	 * @phpstan-return non-negative-int|list<non-negative-int>
 	 */
 	protected function get_comment_ids() {
 		global $wpdb;

src/wp-includes/class-wp-comment.php

@@ -10,7 +10,52 @@
 /**
  * Core class used to organize comments as instantiated objects with defined members.
  *
+ * The `@property-read` fields below are not stored on the comment. They are proxied to the
+ * comment's post by {@see WP_Comment::__get()}, and are null when the comment is not attached
+ * to a post or when that post no longer exists.
+ *
  * @since 4.4.0
+ *
+ * @property-read numeric-string|''|null $post_author
+ * @property-read string|null            $post_date
+ * @property-read string|null            $post_date_gmt
+ * @property-read string|null            $post_content
+ * @property-read string|null            $post_title
+ * @property-read string|null            $post_excerpt
+ * @property-read non-empty-string|null  $post_status
+ * @property-read non-empty-string|null  $comment_status
+ * @property-read non-empty-string|null  $ping_status
+ * @property-read string|null            $post_name
+ * @property-read string|null            $to_ping
+ * @property-read string|null            $pinged
+ * @property-read string|null            $post_modified
+ * @property-read string|null            $post_modified_gmt
+ * @property-read string|null            $post_content_filtered
+ * @property-read non-negative-int|null  $post_parent
+ * @property-read string|null            $guid
+ * @property-read int|null               $menu_order
+ * @property-read non-empty-string|null  $post_type
+ * @property-read string|null            $post_mime_type
+ * @property-read numeric-string|null    $comment_count
+ *
+ * @phpstan-type Data_Array array{
+ *     comment_ID: numeric-string,
+ *     comment_post_ID: numeric-string,
+ *     comment_author: string,
+ *     comment_author_email: string,
+ *     comment_author_url: string,
+ *     comment_author_IP: string,
+ *     comment_date: non-empty-string,
+ *     comment_date_gmt: non-empty-string,
+ *     comment_content: string,
+ *     comment_karma: numeric-string,
+ *     comment_approved: non-empty-string,
+ *     comment_agent: string,
+ *     comment_type: string,
+ *     comment_parent: numeric-string,
+ *     user_id: numeric-string,
+ *     ...
+ * }
  */
 #[AllowDynamicProperties]
 final class WP_Comment {
@@ -74,6 +119,7 @@ final class WP_Comment {
 	 *
 	 * @since 4.4.0
 	 * @var string
+	 * @phpstan-var non-empty-string
 	 */
 	public $comment_date = '0000-00-00 00:00:00';
 
@@ -82,6 +128,7 @@ final class WP_Comment {
 	 *
 	 * @since 4.4.0
 	 * @var string
+	 * @phpstan-var non-empty-string
 	 */
 	public $comment_date_gmt = '0000-00-00 00:00:00';
 
@@ -107,8 +154,12 @@ final class WP_Comment {
 	/**
 	 * Comment approval status.
 	 *
+	 * The values used in core are '0' (unapproved), '1' (approved), 'spam', 'trash',
+	 * and 'post-trashed' (set for every comment on a post that is moved to the trash).
+	 *
 	 * @since 4.4.0
 	 * @var string
+	 * @phpstan-var non-empty-string
 	 */
 	public $comment_approved = '1';
 
@@ -123,6 +174,13 @@ final class WP_Comment {
 	/**
 	 * Comment type.
 	 *
+	 * The values used in core are 'comment', 'pingback', 'trackback', and 'note'. Custom
+	 * comment types are possible.
+	 *
+	 * Comments created before 5.5.0 may store an empty string rather than 'comment', so this
+	 * cannot be relied upon to be non-empty. {@see get_comment_type()} normalizes that case
+	 * when reading.
+	 *
 	 * @since 4.4.0
 	 * @since 5.5.0 Default value changed to `comment`.
 	 * @var string
@@ -154,8 +212,16 @@ final class WP_Comment {
 	/**
 	 * Comment children.
 	 *
+	 * Mapping of comment ID to WP_Comment object, as populated by the default
+	 * `hierarchical => 'threaded'` argument of get_children(). Note that if a
+	 * caller passes a `hierarchical` value of 'flat' or `false` to
+	 * get_children(), a sequentially-keyed array of WP_Comment objects (also
+	 * including all descendants, in the 'flat' case) is stored here instead.
+	 *
+	 * Null until populated by {@see WP_Comment::get_children()}.
+	 *
 	 * @since 4.4.0
-	 * @var array
+	 * @var array<int, WP_Comment>|null
 	 */
 	protected $children;
 
@@ -171,7 +237,8 @@ final class WP_Comment {
 	 * Post fields.
 	 *
 	 * @since 4.4.0
-	 * @var array
+	 * @var string[]
+	 * @phpstan-var list<non-empty-string>
 	 */
 	protected $post_fields = array( 'post_author', 'post_date', 'post_date_gmt', 'post_content', 'post_title', 'post_excerpt', 'post_status', 'comment_status', 'ping_status', 'post_name', 'to_ping', 'pinged', 'post_modified', 'post_modified_gmt', 'post_content_filtered', 'post_parent', 'guid', 'menu_order', 'post_type', 'post_mime_type', 'comment_count' );
 
@@ -183,6 +250,7 @@ final class WP_Comment {
 	 * @global wpdb $wpdb WordPress database abstraction object.
 	 *
 	 * @param int $id Comment ID.
+	 * @phpstan-param int|numeric-string $id
 	 * @return WP_Comment|false Comment object, otherwise false.
 	 */
 	public static function get_instance( $id ) {
@@ -195,7 +263,8 @@ public static function get_instance( $id ) {
 
 		$_comment = wp_cache_get( $comment_id, 'comment' );
 
-		if ( ! $_comment ) {
+		if ( ! is_object( $_comment ) ) {
+			/** @var object{ comment_ID: string, comment_post_ID: string, comment_author: string, comment_author_email: string, comment_author_url: string, comment_author_IP: string, comment_date: string, comment_date_gmt: string, comment_content: string, comment_karma: string, comment_approved: string, comment_agent: string, comment_type: string, comment_parent: string, user_id: string }|null $_comment */
 			$_comment = $wpdb->get_row( $wpdb->prepare( "SELECT * FROM $wpdb->comments WHERE comment_ID = %d LIMIT 1", $comment_id ) );
 
 			if ( ! $_comment ) {
@@ -228,19 +297,28 @@ public function __construct( $comment ) {
 	 *
 	 * @since 4.4.0
 	 *
-	 * @return array Object as array.
+	 * @return array<string, mixed> Object as array.
+	 * @phpstan-return Data_Array
 	 */
-	public function to_array() {
-		return get_object_vars( $this );
+	public function to_array(): array {
+		/** @var Data_Array $comment */
+		$comment = get_object_vars( $this );
+		return $comment;
 	}
 
 	/**
 	 * Gets the children of a comment.
 	 *
 	 * @since 4.4.0
+	 * @since 7.1.0 A `count` or `fields` query now returns its result directly rather than
+	 *              erroneously storing it in the comment's children cache.
 	 *
 	 * @param array $args {
-	 *     Array of arguments used to pass to get_comments() and determine format.
+	 *     Array of arguments used to pass to {@see get_comments()} and determine format.
+	 *     Any other argument accepted by {@see WP_Comment_Query::__construct()} may also be passed, and is
+	 *     forwarded to `get_comments()`. Note that `parent` is always overridden with this comment's ID.
+	 *     A `$count` or `$fields` query returns the direct children only, and does not populate the
+	 *     comment's cached children, since that cache holds `WP_Comment` objects.
 	 *
 	 *     @type string $format        Return value format. 'tree' for a hierarchical tree, 'flat' for a flattened array.
 	 *                                 Default 'tree'.
@@ -267,8 +345,42 @@ public function to_array() {
 	 *                                 the value of $meta_key, and the array keys of
 	 *                                 `$meta_query`. Also accepts false, an empty array, or
 	 *                                 'none' to disable `ORDER BY` clause.
+	 *     @type string $fields        Which fields to return. Accepts 'ids' for comment IDs, or an
+	 *                                 empty string for full `WP_Comment` objects. Default empty.
+	 *     @type bool   $count         Whether to return a comment count rather than comments.
+	 *                                 Default false.
+	 *     @type string $type          Limit results to comments of a given type, such as 'comment',
+	 *                                 'pingback', 'trackback', or 'note'. Accepts 'all' for every
+	 *                                 type. Default empty.
+	 *     @type int    $number        Maximum number of comments to retrieve. Default empty (no limit).
+	 *     @type int    $post_id       Limit results to comments on a given post. Default 0.
+	 *     @type string $order         How to order retrieved comments. Accepts 'ASC' or 'DESC'.
+	 *                                 Default 'DESC'.
 	 * }
-	 * @return WP_Comment[] Array of `WP_Comment` objects.
+	 * @return WP_Comment[]|int[]|int Array of `WP_Comment` objects, an array of comment IDs when
+	 *                                `$fields` is 'ids', or the number of children when `$count`
+	 *                                is true.
+	 *
+	 * @phpstan-param array{
+	 *                    format?: 'tree'|'flat',
+	 *                    status?: 'hold'|'approve'|'all'|string,
+	 *                    hierarchical?: 'threaded'|'flat'|false,
+	 *                    orderby?: string|string[]|false,
+	 *                    fields?: 'ids'|'',
+	 *                    count?: bool,
+	 *                    type?: string,
+	 *                    number?: int,
+	 *                    post_id?: int,
+	 *                    order?: 'ASC'|'DESC',
+	 *                    ...
+	 *                } $args
+	 * @phpstan-return (
+	 *     $args is array{ count: true, ... } ? non-negative-int : (
+	 *         $args is array{ fields: 'ids', ... } ? non-negative-int[] : (
+	 *             $args is array{ format: 'flat', ... } ? list<WP_Comment> : array<int, WP_Comment>
+	 *         )
+	 *     )
+	 * )
 	 */
 	public function get_children( $args = array() ) {
 		$defaults = array(
@@ -278,9 +390,29 @@ public function get_children( $args = array() ) {
 			'orderby'      => '',
 		);
 
+		/** @var array{ format: 'tree'|'flat', status: string, hierarchical: 'threaded'|'flat'|false, orderby: string|string[]|false, fields?: 'ids'|'', count?: bool, type?: string, number?: int, post_id?: int, order?: 'ASC'|'DESC', ... } $_args */
 		$_args           = wp_parse_args( $args, $defaults );
 		$_args['parent'] = $this->comment_ID;
 
+		/*
+		 * A 'count' or 'ids' query returns an integer or a list of comment IDs rather than
+		 * WP_Comment objects. Neither may be written to the children cache, which holds
+		 * WP_Comment objects and is read back by add_child(), get_child(), and the 'flat'
+		 * format below. Return the result directly and leave the cache untouched. The two
+		 * branches must stay separate: each is narrowed independently, and `count` is only
+		 * safe to overwrite in the 'ids' branch.
+		 */
+		if ( ! empty( $_args['count'] ) ) {
+			return get_comments( $_args );
+		} elseif ( isset( $_args['fields'] ) && 'ids' === $_args['fields'] ) {
+			$_args['count'] = false; // For static analysis of the conditional return type.
+			return get_comments( $_args );
+		}
+
+		// Only WP_Comment objects are returned past this point. Stated positively for static analysis.
+		$_args['count']  = false;
+		$_args['fields'] = '';
+
 		if ( is_null( $this->children ) ) {
 			if ( $this->populated_children ) {
 				$this->children = array();
@@ -315,8 +447,8 @@ public function get_children( $args = array() ) {
 	 *
 	 * @param WP_Comment $child Child comment.
 	 */
-	public function add_child( WP_Comment $child ) {
-		$this->children[ $child->comment_ID ] = $child;
+	public function add_child( WP_Comment $child ): void {
+		$this->children[ (int) $child->comment_ID ] = $child;
 	}
 
 	/**
@@ -341,7 +473,7 @@ public function get_child( $child_id ) {
 	 *
 	 * @param bool $set Whether the comment's children have already been populated.
 	 */
-	public function populated_children( $set ) {
+	public function populated_children( $set ): void {
 		$this->populated_children = (bool) $set;
 	}
 
@@ -351,14 +483,15 @@ public function populated_children( $set ) {
 	 * If `$name` matches a post field, the comment post will be loaded and the post's value checked.
 	 *
 	 * @since 4.4.0
+	 * @since 7.1.0 Returns false instead of causing a fatal error when the comment's post cannot be found.
 	 *
 	 * @param string $name Property to check if set.
 	 * @return bool Whether the property is set.
 	 */
 	public function __isset( $name ) {
 		if ( in_array( $name, $this->post_fields, true ) && 0 !== (int) $this->comment_post_ID ) {
-			$post = get_post( $this->comment_post_ID );
-			return property_exists( $post, $name );
+			$post = get_post( (int) $this->comment_post_ID );
+			return $post && property_exists( $post, $name );
 		}
 
 		return false;
@@ -370,14 +503,20 @@ public function __isset( $name ) {
 	 * If `$name` matches a post field, the comment post will be loaded and the post's value returned.
 	 *
 	 * @since 4.4.0
+	 * @since 7.1.0 Returns null instead of the global post's field when the comment is not attached to
+	 *              a post, and no longer raises a warning when the comment's post cannot be found.
 	 *
 	 * @param string $name Property name.
 	 * @return mixed
 	 */
 	public function __get( $name ) {
-		if ( in_array( $name, $this->post_fields, true ) ) {
-			$post = get_post( $this->comment_post_ID );
+		if ( in_array( $name, $this->post_fields, true ) && 0 !== (int) $this->comment_post_ID ) {
+			$post = get_post( (int) $this->comment_post_ID );
+			if ( ! $post ) {
+				return null;
+			}
 			return $post->$name;
 		}
+		return null;
 	}
 }