Repository navigation
Docs: Add PHPStan type assertions - #13896
swissspidy wants to merge 4 commits into
Conversation
Adds `@phpstan-assert-if-true` to functions that check their argument, such as `taxonomy_exists()`, `shortcode_exists()` and `wp_is_uuid()`, and `@phpstan-pure` and `@phpstan-impure` to functions whose result depends only on their arguments, or does not. `addslashes_gpc()` is not marked as pure, since it calls `_deprecated_function()`. 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 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
🟢 Approval recommended
The annotations accurately reflect the reviewed function behavior and introduce no runtime changes.
Review effort: Balanced
Findings: None
What changed in this PR
Adds PHPStan annotations to improve type narrowing and correctly model deterministic and state-dependent functions, without runtime changes.
Changes:
- Adds conditional type assertions to validation helpers.
- Marks deterministic helpers as pure.
- Marks random, time-dependent, and stateful functions as impure.
| File | Description |
|---|---|
src/wp-includes/user.php |
Marks password reset key generation impure. |
src/wp-includes/taxonomy.php |
Adds taxonomy-name type assertion. |
src/wp-includes/shortcodes.php |
Adds shortcode assertions and purity metadata. |
src/wp-includes/rest-api.php |
Marks boolean sanitization pure. |
src/wp-includes/pluggable.php |
Marks mail, nonce, password, and random functions impure. |
src/wp-includes/option.php |
Marks transient retrieval impure. |
src/wp-includes/functions.php |
Adds assertions and purity metadata to general helpers. |
src/wp-includes/formatting.php |
Marks deterministic formatting helpers pure. |
src/wp-admin/includes/menu.php |
Marks CSS-class composition pure. |
src/wp-admin/includes/bookmark.php |
Marks request-dependent link defaults impure. |
💡 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 I couldn't find a WordPress.org profile for @PeterSmith8nss. That GitHub login is listed as a placeholder until the username is confirmed. Props for this PR (swissspidy for the split, westonruter for the review that shaped it): Generated by Claude Code |
Anyone who does not want PHPStan to remember potentially impure return values can set It would be a good idea to agree on an approach before adding |
…ns-purity # Conflicts: # src/wp-includes/formatting.php # src/wp-includes/functions.php # src/wp-includes/shortcodes.php
Drop `@phpstan-impure` until there is agreement on which functions core should mark as impure, given how much of WordPress depends on globals and the database. Drop `@phpstan-pure` too, since with the default `rememberPossiblyImpureFunctionValues: true` it does not change whether PHPStan remembers a function's return value. See #65817. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T3XYNUvB9sQNDXA14uJQeH
swissspidy
changed the title
|
I do not understand the decision to remove the |
|
I wanna add those in a separate PR Edit: see below. |
Split out of #13530, as proposed in this comment (subset 9).
@phpstan-assert-if-trueis added towp_is_stream(),wp_is_uuid(),shortcode_exists(),has_shortcode()andtaxonomy_exists().Changes from #13530
These follow the review analysis:
wp_is_uuid()asserts=lowercase-string&non-falsy-stringwithout theTUuid of stringtemplate, which rejected themixedits callers pass.Changes after review
@phpstan-impureannotations are removed for now. Much of WordPress depends on globals and the database, so nearly every function could be marked impure. Core should first agree on which functions to mark, for example only deliberately random ones such aswp_rand()and ones especially likely to change between calls such asget_transient(), as php-stubs does.@phpstan-pureannotations are removed as well. With the defaultrememberPossiblyImpureFunctionValues: true, PHPStan already remembers return values, so@phpstan-purechanges nothing (see Remembering and forgetting returned values).path_is_absolute()'s assertion is dropped from this PR, since trunk now has@phpstan-assert-if-true non-falsy-string $path.Effect on the analysis
composer run phpstanpasses with no baseline changes.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 the result with
composer run phpstanand 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_01T3XYNUvB9sQNDXA14uJQeH