Repository navigation
Docs: Add PHPStan integer ranges and literal types - #13893
swissspidy wants to merge 20 commits into
Conversation
Adds integer ranges such as `int<0, max>`, and unions of literal values, where a function only returns or accepts those values. Compared to the function map in php-stubs/wordpress-stubs, `sanitize_title_with_dashes()` also accepts the `'query'` context, `WP_Locale::$word_count_type` can be `null` before `init()` runs, and the link category IDs returned by `get_bookmark_field()` and `sanitize_bookmark_field()` can be `0`, since they come from `absint()`. The map's narrowing of the nonce `$action` and `wp_hash()`'s `$scheme` is left out, as core passes wider types to both. See #65817. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VieGtSQtUoVfHiGwTmkvtc
|
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 Unlinked AccountsThe following contributors have not linked their GitHub and WordPress.org accounts: @claude. Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases. 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. |
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. |
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Some new annotations exclude supported inputs or runtime return values, producing inaccurate static-analysis contracts.
Review effort: Balanced
Findings: 2
Open (4)
What changed in this PR
Adds PHPStan integer ranges and literal types to WordPress Core, extracted from #13530 for focused review.
Changes:
- Narrows function parameters and return values.
- Models nullable locale word-count state.
- Removes the resolved PHPStan baseline entry.
| File | Description |
|---|---|
tests/phpstan/baselines/nullCoalesce.property.neon |
Removes obsolete suppression. |
src/wp-includes/user.php |
Narrows user ID return. |
src/wp-includes/script-loader.php |
Defines suffix literals. |
src/wp-includes/rest-api.php |
Defines authorization codes. |
src/wp-includes/post-template.php |
Narrows revision type. |
src/wp-includes/plugin.php |
Narrows hook counts. |
src/wp-includes/pluggable.php |
Defines nonce results. |
src/wp-includes/load.php |
Narrows protocol and site ID. |
src/wp-includes/l10n.php |
Defines word-count literals. |
src/wp-includes/functions.php |
Adds ranges and literal types. |
src/wp-includes/formatting.php |
Narrows formatting parameters. |
src/wp-includes/comment.php |
Defines comment statuses. |
src/wp-includes/class-wpdb.php |
Narrows row offset. |
src/wp-includes/class-wp-locale.php |
Models word-count property and return. |
src/wp-includes/bookmark.php |
Narrows bookmark fields and results. |
src/wp-admin/includes/template.php |
Narrows meta-box count. |
src/wp-admin/includes/taxonomy.php |
Narrows category update result. |
src/wp-admin/includes/class-custom-image-header.php |
Defines selector types. |
src/wp-admin/includes/bookmark.php |
Narrows link operation results. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
🤖 Comment by Claude Opus 5.5 Props for this subset, gathered from the full history of Props for this PR (swissspidy for the split, westonruter for the review that shaped it): Generated by Claude Code |
| * @phpstan-return 401|403 | ||
| */ | ||
| function rest_authorization_required_code() { | ||
| return is_user_logged_in() ? 403 : 401; |
There was a problem hiding this comment.
Slightly off-topic, but could 401 and 403 be replaced with WP_Http::UNAUTHORIZED and WP_Http::FORBIDDEN, respectively, both in the call and in the docblock, as well as anywhere else a status code is used or returned?
There was a problem hiding this comment.
Probably, but we can do that in separate ticket/PR. There are many instances of this throughout core
Keep the return type of sanitize_bookmark_field() and get_bookmark_field() mixed for fields other than link_category, link_id and link_rating, since the raw context and the field filters can return any value. Update the canonical @return and @var tags to match the PHPStan types, and use non-negative-int and positive-int, the named forms core already uses most, instead of int<0, max> and int<1, max>. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015BuoHZQvJMvPEYp4Sfjgvd
Conflicts in the docblocks of `sanitize_title_with_dashes()` and `wp_json_encode()` were resolved by keeping both the branch's `@phpstan-param` tags and trunk's new `@phpstan-return` tags. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The hook counters were documented as `int[]` (or a bare `array` in the test helpers), and since PHPStan reads `@global` tags as inline `@var` annotations, `did_action()` and `did_filter()` could not satisfy their new `non-negative-int` return types. Each count starts at 1 and is only ever incremented, so `array<string, positive-int>` is the accurate type. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
`edit_link()` and `wp_update_link()` are annotated as returning a `non-negative-int`, but `wp_insert_link()` only promised `int`, so PHPStan flagged both wrappers on their unchanged `return` lines. The `int` was accurate: on the update path, `$link_id` was taken as-is from the sanitized (and filterable) input, so passing a negative `link_id` made the update match no rows and the negative ID was returned. The ID is now passed through `absint()`, so an invalid negative ID is treated as 0 (an insert) rather than an update of a link that cannot exist. On the insert path, `$link_id` comes from `wpdb::$insert_id`, which is now annotated as `non-negative-int`, since an AUTO_INCREMENT ID is never negative. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…tive-int` `wp_update_category()` is annotated as returning `non-negative-int|false`, but it returns `wp_insert_category()`, which only promised `int`, so PHPStan flagged its unchanged `return` line. `wp_insert_category()` returns the `term_id` from `wp_insert_term()` or `wp_update_term()`, so their array shapes are narrowed to `term_id: non-negative-int` as well. To make that true rather than merely asserted, the IDs those functions return are now passed through `absint()`: - The value returned by the `term_id_filter` filter, in both functions. Previously a filter could leak a negative number or a numeric string into the returned array. - The ID of an existing duplicate term, which is read from a database row and so was only cast with `(int)`. The remaining path in `wp_insert_term()` already uses `wpdb::$insert_id`, which is now `non-negative-int`. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Now that `wpdb::$insert_id` is a `non-negative-int`, the existing baselined error for passing `$term_id` as the `$fallback_title` of `sanitize_title()` in `wp_insert_term()` reports `int<0, max>` instead of `int`. Split the baseline entry so each message matches its single occurrence, rather than leaving one error unignored and the other entry reporting an unmet count. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
`get_current_user_id()` is annotated as returning a `non-negative-int`, but it returns `(int) $user->ID`, and `WP_User::$ID` was only typed as `int`, so PHPStan flagged its unchanged `return` line. Although `wp_get_current_user()` is pluggable, it is documented to return a `WP_User`, and the `WP_User` class itself is not pluggable, so the narrowed property type holds for whatever object a replacement returns. This matches `WP_Post::$ID`, which is already annotated as `non-negative-int`. To make the type true rather than merely asserted, the two places in `WP_User` that set the ID, `init()` and the deprecated `id` handling in `__set()`, now use `absint()` instead of an `(int)` cast. A negative ID was never valid, and is now treated as 0, the ID of a user who is not logged in. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Now that `WP_User::$ID` is a `non-negative-int`, two baselined errors report `int<0, max>` instead of `int`, so their patterns no longer matched: - The `post_author` user ID in the array passed to `wp_insert_post()` by `wp_xmlrpc_server::blogger_newPost()`. - The IDs passed as the `$fallback` class name to `sanitize_html_class()` in `get_body_class()`: the author's user ID, and the `ID` of the queried object. The latter was already narrowed for `WP_Post`, but `get_queried_object()` may also return a `WP_User`, so it was only `int` until now. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The earlier commits narrowing the IDs from `wp_insert_link()`, `wp_insert_term()`, `wp_update_term()` and `WP_User` to `non-negative-int` used `absint()` to make the types hold, and their messages described this as turning a negative ID into 0. That was wrong: `absint()` is `abs( intval() )`, so it flips the sign, and an invalid ID such as -5 became the valid ID 5, which could refer to an existing user, term or link. Each of these now casts with `(int)` and then clamps with `max( 0, … )`, so a negative ID becomes 0 and never resolves to a real object. PHPStan infers `int<0, max>` from this, so the narrowed return types still hold. The restored `(int)` casts in `WP_User::init()`, `WP_User::__set()` and the duplicate-term check in `wp_insert_term()` report `cast.int` for casting a `mixed` value, as they did on trunk. The cast in `wp_insert_link()` is new and reports the same error, because `wp_unslash()` erases the array shape returned by `sanitize_bookmark()`. `wp_update_link()` already has this error for its own cast of `link_id`. It is left as is, since the link manager is disabled by default. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
`wp_scripts_get_suffix()` is annotated as returning `''|'.min'`, but its `static $suffixes` cache was untyped, so PHPStan saw each return as `mixed` and could not verify the annotation; it only reworded the existing `return.type` errors. An inline `@var` gives the cache its shape, so both returns now check out. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
`wp_verify_nonce()` is annotated as returning `1|2|false`, but `check_admin_referer()` and `check_ajax_referer()`, which return its result directly, still declared `int`, so callers lost the literal type one call away. Their conditional returns now use `1|2`, keeping `false` only for the cases where they return rather than exit on an invalid nonce: `$action` of -1 for `check_admin_referer()`, and `$stop` of false for `check_ajax_referer()`. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
`get_current_blog_id()` is annotated as returning a `non-negative-int`, but its sibling `get_current_network_id()` still returned plain `int`. `get_current_network_id()` returns 1, `absint()` of the current network's ID, or `get_main_network_id()`. The last of these returned the `(int)` cast of the `get_main_network_id` filter, which could be negative, so `get_main_network_id()` now clamps that value with `max( 0, … )` and is narrowed as well. A negative network ID was never valid, and is now treated as 0, which never matches a real network. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Clamping the ID with `max( 0, … )` turned a negative `link_id` into 0, which `wp_insert_link()` treats as a request to insert, so a call meant to update a (nonexistent) link created a new one and fired `add_link`. Before the clamp, a negative ID took the update path, which matched no rows and wrote nothing. A negative ID is now rejected up front by returning 0, the same failure value the function already returns for a missing name or URL. Nothing is written, and `$link_id` is still narrowed to `non-negative-int` for the return type. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
`sanitize_title_with_dashes()` runs as a `sanitize_title` filter callback, so it receives whatever context is passed to `sanitize_title()`. Core itself passes `'old-save'` in `wp_check_for_changed_slugs()`, and plugins may pass any string. The function only distinguishes `'save'` from everything else, so the `'display'|'save'|'query'` union was narrower than the values it actually handles, and the documented `string` is the accurate type. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
`WP_User::init()` clamped a negative ID to 0 for the `ID` property, but stored the data object untouched, so `$user->data->ID` (and so `$user->get( 'ID' )`) still held the negative value while `$user->ID` was 0. The ID is now validated before the data is stored: when it is missing or negative, both the property and the data are set to 0. A valid ID is left as it is in the data, so a database row keeps its numeric-string `ID`. A test covers a negative ID, a missing ID and a database row. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
🤖 Comment by Claude Opus 5.5 A summary follows of the work done on this branch since 29e0353 and e729b55: a trunk merge, two rounds of code review, and the fixes that came out of them. Merge of trunk (9d8d956)The merge conflicted in two docblocks, where trunk had added tags next to the ones on this branch. Both sides were kept:
Review round 1: errors on unchanged lines
Considered and dropped: typing Review round 2A second review of the branch against trunk produced these changes:
Two of the findings turned out to be false positives and were left alone: Plugin impact: the WordPress.org plugin directory was searched (via Veloria) for code that builds Review round 3A third review was done after 63ec1b7. Its two findings were checked against the PHPStan output, and neither needed a change:
Everything else the review checked was found to be correct. That covers the nonce function returns, the network, blog and user ID returns, ResultA full level-10 run at 63ec1b7, from a cold result cache, gives 12 fewer errors than trunk:
All but one of the introduced errors are existing errors reworded with the narrower types (for example |
|
@swissspidy I did some rounds of reviews and I added some changes. I didn't want the introduction of these new annotations to cause new PHPStan errors. LMKWYT |
`get_bookmark()` returns an object, an array or null, never a `WP_Error`, so the `is_wp_error()` check after it could never match. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
| * False if the nonce is invalid. Only possible when `$stop` is false, | ||
| * as the function otherwise exits rather than returning false. | ||
| * @phpstan-return ( $stop is true ? int : int|false ) | ||
| * @phpstan-return ( $stop is true ? 1|2 : 1|2|false ) |
There was a problem hiding this comment.
If $stop is true, then it returns 1|2|never. But I think we settled that we wouldn't include never or void in a union.
Split out of #13530, as proposed in this comment (subset 6).
non-negative-intorpositive-int. Examples:get_current_user_id(),get_current_blog_id(),did_action(),did_filter(),do_meta_boxes(), the link update functions,wp_json_encode()'s$depthandwpdb::get_row()'s$y.wp_get_server_protocol()rest_authorization_required_code()wp_verify_nonce()wp_get_comment_status()wp_scripts_get_suffix()is_new_day()antispambot()'s$hex_encodingwp_trigger_error()'s$error_level__return_*()helpersChanges from #13530
These follow the review analysis:
sanitize_title_with_dashes()also accepts the'query'context, whichsanitize_title_for_query()passes.WP_Locale::$word_count_typeincludesnull, because it is unset untilinit()runs. This removes an entry from thenullCoalesce.propertybaseline.get_bookmark_field()andsanitize_bookmark_field()return link category IDs asnon-negative-int, becauseabsint()can produce0.-1|string $actionon the nonce functions.check_admin_referer()andcheck_ajax_referer()passint|string.wp_hash()'s$scheme. The auth cookie functions passstring, andwp_salt()supports any scheme.int $countonWP_Translations::translate_plural(). It contradicts the method's ownint|floatdocumentation.Changes after review
non-negative-intandpositive-int, which core already uses most, instead ofint<0, max>andint<1, max>.get_bookmark_field()andsanitize_bookmark_field()use a conditional return type. Onlylink_category,link_idandlink_ratingare narrowed. Every other field staysmixed, since therawcontext and the field filters can return any value. The canonical@returnofget_bookmark_field()is nowmixedto match.WP_Locale::$word_count_type's canonical@varis nowstring|null.wp_list_post_revisions()'s$typedeliberately leaves out the array form, which has been deprecated since 3.6.0, like the deprecated parameters in Docs: Narrow deprecated parameters to their default values in PHPStan types #13889.Effect on the analysis
composer run phpstanpasses, andnullCoalesce.propertyloses one entry. At level 10, compared against trunk, 12 errors are fixed and 19 are introduced. Most of the 19 arereturn.typeerrors where PHPStan infersintfrom a cast and cannot see the range, or are existing errors reworded with the narrower type. (These counts were measured before the changes after review.)Trac ticket: https://core.trac.wordpress.org/ticket/65817
Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Opus 5.5
Used for: Splitting #13530 by tag kind, applying the corrections from the review analysis on #13530 and from review on this PR, checking each subset with
composer run phpstan(also at level 10, compared against trunk) andphpcs, and drafting the commit messages and this description. Reviewed by me.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
https://claude.ai/code/session_015BuoHZQvJMvPEYp4Sfjgvd