Code Quality: Account for object being supplied as post, term, or comment#11096
Code Quality: Account for object being supplied as post, term, or comment#11096westonruter wants to merge 18 commits intoWordPress:trunkfrom
object being supplied as post, term, or comment#11096Conversation
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
object being supplied as post, term, and commentobject being supplied as post, term, and comment
object being supplied as post, term, and commentobject being supplied as post, term, or comment
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
| } elseif ( isset( $post->ID ) ) { | ||
| $_post = WP_Post::get_instance( $post->ID ); | ||
| } else { | ||
| $_post = null; |
There was a problem hiding this comment.
I guess some test coverage here would be good.
| $do_object = is_object( $term ); | ||
|
|
||
| $term_id = $do_object ? $term->term_id : ( $term['term_id'] ?? 0 ); | ||
| $term_id = $do_object ? ( $term->term_id ?? 0 ) : ( $term['term_id'] ?? 0 ); |
There was a problem hiding this comment.
Test coverage here too.
| } elseif ( 'raw' === $post->filter ) { | ||
| $_post = new WP_Post( $post ); | ||
| } else { | ||
| } elseif ( isset( $post->ID ) ) { |
There was a problem hiding this comment.
This would mean that $post is no instance of WP_Post(we're in the else of line 1154).
PHPdoc for $post says @param int|WP_Post|null $post Optional. Post ID or post object.
So if you expect this to be a generic object, that is formed like WP_Post, the PHPDoc Block is wrong.
There was a problem hiding this comment.
Ah yes, the PHPDoc needs to be updated to add object as well.
There was a problem hiding this comment.
Or I guess, object should be used instead of WP_Post given our other conversation.
There was a problem hiding this comment.
How/why is it possible that an object that mimics WP_Post is passed here? Doesn’t look like solid architecture
There was a problem hiding this comment.
It's when post data is passed straight from a database query.
Corrected formatting of the parameter description for clarity.
Adds a new test suite for get_post() to ensure complete coverage of: - Global $post retrieval with various empty-like inputs. - Different input types (ID, object, WP_Post instance). - All output formats (OBJECT, ARRAY_A, ARRAY_N). - Filter application and field sanitization. - Handling of unrecognized output types. Co-authored-by: gemini-cli <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Adds a new test suite for sanitize_term() to ensure coverage of: - Sanitizing both objects and arrays. - Handling missing term_id correctly. - Applying different sanitization contexts (raw, edit, display, attribute, js). - Verifying field sanitization results in each context. - Resolves PHPStan type-checking issues with proper assertions. Co-authored-by: gemini-cli <176961590+gemini-code-assist[bot]@users.noreply.github.com>
…into add/isset-checks
…tests Updates the example script tags in get_post() and sanitize_term() tests to use console.log(), which is a more appropriate example for modern testing. Co-authored-by: gemini-cli <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Moves the filter property assertion into the appropriate conditional blocks and removes the redundant final if statement for better code structure. Co-authored-by: gemini-cli <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Simplifies the test logic by removing an unnecessary null check, as subsequent type assertions already provide this verification. Co-authored-by: gemini-cli <176961590+gemini-code-assist[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR improves resilience when core APIs receive plain object instances (e.g., DB row objects) that may be missing expected identifier properties, preventing PHP notices and clarifying constructor expectations in documentation.
Changes:
- Update
sanitize_term()to tolerate term objects missingterm_id. - Update
get_post()to tolerate post-like objects missingIDand returnnullrather than triggering a notice. - Add PHPUnit coverage for these object-input edge cases and update constructor PHPDocs for
WP_PostandWP_Comment.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/phpunit/tests/term/sanitizeTerm.php | Adds tests ensuring sanitize_term() handles objects with/without term_id across contexts. |
| tests/phpunit/tests/post/getPost.php | Adds tests covering get_post() behavior for globals, IDs, WP_Post instances, and post-like objects missing ID. |
| src/wp-includes/taxonomy.php | Avoids undefined property notices by using a default term_id when missing on objects. |
| src/wp-includes/post.php | Prevents undefined property notices by checking for ID before calling WP_Post::get_instance(). |
| src/wp-includes/class-wp-post.php | Updates constructor PHPDoc to reflect that a generic object can be supplied. |
| src/wp-includes/class-wp-comment.php | Updates constructor PHPDoc to reflect that a generic object can be supplied. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ost, site, term, and user. When constructing these objects, the specific instance type is redundantly used in a union with a generic `object`. A generic object can be passed directly from database row result. This also hardens `get_post()` to account for passing an object that lacks an `ID` property. Similarly, `sanitize_term()` is hardened to account for an object lacking a `term_id` property. Comprehensive unit tests are added for `get_post()` and `sanitize_term()`. Developed in #11096 Props westonruter, apermo. See #64238, #64225. git-svn-id: https://develop.svn.wordpress.org/trunk@61789 602fd350-edb4-49c9-b593-d223f7449a82
…ost, site, term, and user. When constructing these objects, the specific instance type is redundantly used in a union with a generic `object`. A generic object can be passed directly from database row result. This also hardens `get_post()` to account for passing an object that lacks an `ID` property. Similarly, `sanitize_term()` is hardened to account for an object lacking a `term_id` property. Comprehensive unit tests are added for `get_post()` and `sanitize_term()`. Developed in WordPress/wordpress-develop#11096 Props westonruter, apermo. See #64238, #64225. Built from https://develop.svn.wordpress.org/trunk@61789 git-svn-id: http://core.svn.wordpress.org/trunk@61095 1a063a9b-81f0-0310-95a4-ce76da25c4cd
Trac ticket: https://core.trac.wordpress.org/ticket/64238
Trac ticket: https://core.trac.wordpress.org/ticket/64225
IDexists on post object inget_post()andterm_idexists on term object insanitize_term().objectmay be supplied to theWP_Commentconstructor.Use of AI Tools
See co-authored commits. Gemini CLI used to help write tests.
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.
Commit Message
Code Quality: Account for
objectbeing supplied as post, term, or comment.IDexists on post object inget_post()andterm_idexists on term object insanitize_term().objectmay be supplied to theWP_Commentconstructor.Developed in #11096
See #64238.