Opened 5 weeks ago
Last modified 4 hours ago
#65817 reviewing task (blessed)
PHPStan code quality improvements for 7.2
| Reported by: | desrosj | Owned by: | westonruter |
|---|---|---|---|
| Priority: | normal | Milestone: | 7.2 |
| Component: | General | Version: | |
| Severity: | normal | Keywords: | good-first-bug has-patch has-unit-tests |
| Cc: | Focuses: |
Description (last modified by )
This ticket is for various code quality issues and improvements surfaced via PHPStan.
PHPStan was integrated into the development workflow in #61175, and the rule level was bumped from 0 to 5 in #64680, with existing errors suppressed with baselines.
Anyone looking for code quality issues to fix should look at the errors in the baselines in tests/phpstan/baselines. Priority should be given to errors which are on the lower rule levels.
Baselines added for each rule level:
👉 Please do not open PRs which fix all errors at once, or even all errors in one error-specific baseline, unless there are just a few. Conversely, avoid opening a PR to just fix a single error out of many, unless it is justified. Try to keep PRs organized by component or by file. PRs can be opened referencing this Trac ticket, avoiding the need to open a ticket for these ongoing improvements. Code archeology will often be needed to understand why a given error exists and this discovery should be included in each PR as background.
For information about how to use PHPStan in the wordpress-develop codebase, see the README. In particular, note how baselines are regenerated after errors are fixed.
Previously:
Change History (153)
This ticket was mentioned in PR #12938 on WordPress/wordpress-develop by @callumbw95.
5 weeks ago
#4
- Keywords has-patch has-unit-tests added; needs-patch removed
#5
@
5 weeks ago
I've opened https://github.com/WordPress/wordpress-develop/pull/12938 for the class.nameCase baseline.
There was a single occurrence: @global Mo[] $l10n in WP_Locale_Switcher::load_translations(). The class is MO (wp-includes/pomo/mo.php) and no class named Mo exists; the other ten @global $l10n annotations in l10n.php already use MO[], so this file was the only outlier.
git blame dates it to [38961] (2016-10-26), the changeset that introduced WP_Locale_Switcher so it has been there since the file was written and was never copied elsewhere.
Since it was the only occurrence, the fix empties the baseline, so tests/phpstan/baselines/class.nameCase.neon is deleted along with its includes entry in phpstan.neon.dist. Regenerated via composer phpstan:baselines -- --identifier=class.nameCase, not hand-edited.
PHPStan is clean and the PHPUnit suite is unchanged (30774 tests, 86 warnings, 44 skipped, before and after).
@westonruter commented on PR #12938:
5 weeks ago
#6
This is what I'm talking about! 🎉
This ticket was mentioned in PR #12958 on WordPress/wordpress-develop by @callumbw95.
4 weeks ago
#7
DB_COLLATE and WP_DEVELOPMENT_MODE both vary per installation, but neither is listed in dynamicConstantNames. PHPStan therefore resolves each to the empty string it happens to be given during analysis:
DB_COLLATEcomes fromwp-config-sample.php, pulled in viascanFiles, where it isdefine( 'DB_COLLATE', '' );.WP_DEVELOPMENT_MODEcomes fromtests/phpstan/bootstrap.php, where it isdefine( 'WP_DEVELOPMENT_MODE', '' );.
Any conditional guarding either constant is then analysed as having a constant result, which accounts for the three baselined errors below. The conditionals are correct as written, so this change touches configuration only and leaves every source file alone.
Adding both constants to dynamicConstantNames resolves the three errors and empties two baselines. Those files are deleted along with their includes entries in phpstan.neon.dist.
| Identifier | Location | Expression |
|---|---|---|
ternary.alwaysFalse | class-wp-debug-data.php:1548 | DB_COLLATE ? DB_COLLATE : __( 'Empty value' )
|
ternary.alwaysFalse | class-wp-debug-data.php:1634 | WP_DEVELOPMENT_MODE ? WP_DEVELOPMENT_MODE : __( 'Disabled' )
|
booleanAnd.rightAlwaysFalse | class-wpdb.php:846 | defined( 'DB_COLLATE' ) && DB_COLLATE
|
Taking these errors at face value would suggest deleting the branch PHPStan considers unreachable. That would be a regression. It would drop the reported value on any site that sets either constant, and in wpdb::init_charset() it would force utf8_general_ci on multisite whatever collation the site has configured.
### Why these two belong on the list
WP_DEVELOPMENT_MODE accepts 'core', 'plugin', 'theme', 'all', or an empty string to disable, as documented on wp_get_development_mode() in wp-includes/load.php. It is defined in tests/phpstan/bootstrap.php immediately alongside WP_DEBUG, WP_DEBUG_DISPLAY, WP_DEBUG_LOG, WP_CACHE, SCRIPT_DEBUG, MEDIA_TRASH and SHORTINIT, and all seven of those are already in dynamicConstantNames.
Core's own development environment also disagrees with the value the analysis assumes. tools/local-env/scripts/install.js writes WP_DEVELOPMENT_MODE from LOCAL_WP_DEVELOPMENT_MODE, which .env.example sets to core. A checkout installed with npm run env:install has the constant set to a non-empty value while PHPStan reads it as ''.
DB_COLLATE is set per site in wp-config.php and is empty only in the shipped sample file. DB_CHARSET sits next to it and is read the same way.
### Background
dynamicConstantNames arrived in [61699] (2026-02-20), the changeset that integrated PHPStan into the core development workflow. git log -S against base.neon shows that neither constant has appeared on the list at any point, so these errors have been baselined since the baselines were first generated.
### Testing instructions
- On
trunk,npm run typecheck:phpreports[OK] No errors, because all three occurrences are baselined. - Delete
tests/phpstan/baselines/ternary.alwaysFalse.neonandtests/phpstan/baselines/booleanAnd.rightAlwaysFalse.neonalong with theirincludesentries, then run it again. PHPStan reports:Ternary operator condition is always false.twice insrc/wp-admin/includes/class-wp-debug-data.phpRight side of && is always false.insrc/wp-includes/class-wpdb.php
- With this branch applied,
npm run typecheck:phpreports[OK] No errorswith both baselines gone and no new errors elsewhere. npm run test:phpis unchanged: 30774 tests, 4559286 assertions, 86 warnings, 44 skipped, identical before and after.
The usual risk when adding to dynamicConstantNames runs the other way. Making a value unknown can surface new errors in code that relied on PHPStan narrowing it. The full run above found none.
## Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Opus 5
Used for: tracing the three baselined errors back to the constant values supplied by scanFiles and bootstrap.php, git log -S archaeology against base.neon, and drafting this description. The diagnosis that this is a configuration issue rather than a source defect, the configuration change itself, the baseline regeneration, and verification against full PHPStan and PHPUnit runs were reviewed and confirmed by me in a local development environment.
#8
@
4 weeks ago
Second one, https://github.com/WordPress/wordpress-develop/pull/12958, this time a config-only change that clears three errors across two baselines.
DB_COLLATE and WP_DEVELOPMENT_MODE are both per-install values, but neither is in dynamicConstantNames, so PHPStan resolves each to the empty string it gets during analysis: DB_COLLATE from wp-config-sample.php via scanFiles, WP_DEVELOPMENT_MODE from tests/phpstan/bootstrap.php. Anything guarding either constant then looks like it has a constant result:
ternary.alwaysFalse, twice inclass-wp-debug-data.php(lines 1548 and 1634)booleanAnd.rightAlwaysFalse, inclass-wpdb.phpatdefined( 'DB_COLLATE') && DB_COLLATE
Worth flagging that the obvious reading of these is wrong. Deleting the branch PHPStan calls unreachable would drop the reported value on any site that sets either constant, and in wpdb::init_charset() would force utf8_general_ci on multisite whatever the site has configured. The conditionals are fine; only the analysis was off. So no source file changes here.
Two things that suggest this is an omission rather than deliberate. WP_DEVELOPMENT_MODE is defined in tests/phpstan/bootstrap.php in the same block as WP_DEBUG, WP_DEBUG_DISPLAY, WP_DEBUG_LOG, WP_CACHE, SCRIPT_DEBUG, MEDIA_TRASH and SHORTINIT, all seven of which are already on the list. And tools/local-env/scripts/install.js sets it from
LOCAL_WP_DEVELOPMENT_MODE, which .env.example defaults to core, so a standard npm run env:install checkout has a non-empty value while the analysis assumes ''.
git log -S against base.neon shows neither constant has been on the list at any point since [61699].
Both baselines empty out, so the files and their includes entries in phpstan.neon.dist are removed. Regenerated with composer phpstan:baselines per identifier, not hand-edited. PHPStan is clean and PHPUnit is unchanged (30774 tests, 86 warnings, 44 skipped).
The usual risk with a dynamicConstantNames addition is the reverse of this one, that widening a constant surfaces new errors elsewhere. The full run found none.
@SergeyBiryukov commented on PR #12938:
4 weeks ago
#10
Thanks for the PR! Merged in r63169.
This ticket was mentioned in PR #12970 on WordPress/wordpress-develop by @callumbw95.
4 weeks ago
#12
The @param annotation for $type on WP_Feed_Cache_Transient::__construct() documents the type as Base::TYPE_FEED|Base::TYPE_IMAGE. WP_Feed_Cache_Transient is declared in the global namespace and imports nothing, so PHPStan resolves the bare Base to \Base, which does not exist anywhere in core. That is the single parameter.unresolvableType occurrence in the baseline:
PHPDoc tag @param for parameter $type contains unresolvable type. src/wp-includes/class-wp-feed-cache-transient.php
Qualifying both constants fixes the reference. The annotation is the only thing that changes, so there is no behaviour to affect.
- * @param string $location URL location (scheme is used to determine handler). - * @param string $name Unique identifier for cache object. - * @param Base::TYPE_FEED|Base::TYPE_IMAGE $type Either `TYPE_FEED` ('spc') for SimplePie data, - * or `TYPE_IMAGE` ('spi') for image data. + * @param string $location URL location (scheme is used to determine handler). + * @param string $name Unique identifier for cache object. + * @param SimplePie\Cache\Base::TYPE_FEED|SimplePie\Cache\Base::TYPE_IMAGE $type Either `TYPE_FEED` ('spc') for SimplePie data, + * or `TYPE_IMAGE` ('spi') for image data.
This empties tests/phpstan/baselines/parameter.unresolvableType.neon, so the file is deleted along with its includes entry in phpstan.neon.dist, per the instruction in the baseline's own header.
### Where the annotation came from
SimplePie\Cache\Base carries the same line in the bundled library, at src/wp-includes/SimplePie/src/Cache/Base.php:39:
* @param Base::TYPE_FEED|Base::TYPE_IMAGE $type Either TYPE_FEED for SimplePie data, or TYPE_IMAGE for image data
There it is correct, because that file opens with namespace SimplePie\Cache; and Base resolves to the interface being declared. The line was carried across to the core implementation in [59141] (2024-09-30), the SimplePie 1.8.0 update that namespaced the library and updated WP_Feed_Cache_Transient to match the new interface signature. The destination file has no namespace and no use statement, so the shorthand stopped resolving at that point and has been baselined ever since.
The rest of the file already spells the interface out in full: the class declaration reads implements SimplePie\Cache\Base, the @since 6.7.0 line in this same docblock says SimplePie\Cache\Base, and save() below annotates array|SimplePie\SimplePie. Qualifying the constants makes the @param consistent with its neighbours rather than introducing a new convention.
Adding use SimplePie\Cache\Base; would be shorter, but only two files in the root of wp-includes/ use imports at all, both added recently, so a use statement here would be the novel choice. Fully qualifying keeps this a documentation-only change.
### On the column widths
The type column widens by 32 characters, which pushes the longest line in the block to 137 characters. Keeping the @param name and description columns aligned is what the inline documentation standards ask for, and there is ample precedent: 47 @param lines in src/wp-includes/ are already 137 characters or longer, the longest being 189. composer lint passes on the file either way, so this is alignment rather than a constraint.
### Testing instructions
- On
trunk,npm run typecheck:phpreports[OK] No errors, because the occurrence is baselined. - Delete
tests/phpstan/baselines/parameter.unresolvableType.neonand itsincludesentry, then run it again. PHPStan reportsPHPDoc tag @param for parameter $type contains unresolvable type.insrc/wp-includes/class-wp-feed-cache-transient.php. - With this branch applied,
npm run typecheck:phpreports[OK] No errorswith the baseline gone and nothing new elsewhere. The baseline directory goes from 73 files to 72. - Regenerating confirms the baseline is genuinely empty rather than hand-removed:
composer phpstan:baselines -- --identifier=parameter.unresolvableType
It reports no remaining errors and leaves both deletions in place. composer lintis clean on the changed file.npm run test:phppasses: 30853 tests, 4559514 assertions, 86 warnings, 44 skipped, exit 0, no failures or errors. Run against this branch at996c6d6864.
## Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Opus 5
Used for: tracing the unresolvable type to the missing namespace context, git log -S archaeology identifying [59141] as the origin, checking the bundled SimplePie source for the original annotation, surveying existing use statements and @param line lengths across wp-includes/, and drafting this description. The diagnosis, the choice to fully qualify rather than add an import, the baseline regeneration, and verification against full PHPStan, PHPCS and PHPUnit runs were reviewed and confirmed by me in a local development environment.
This ticket was mentioned in PR #12975 on WordPress/wordpress-develop by @johnbillion.
4 weeks ago
#14
Narrows array to string[] and adds a PHPStan generic.
I opted to use the @phpstan- prefix for the generic. Still need to make a decision on the preferred approach in general for prefixing tags.
Trac ticket: Core-65817
@westonruter commented on PR #12975:
4 weeks ago
#16
🤖 _Comment from Claude Opus 5_
The @phpstan-template T on these two functions reports three errors on wpdb::_escape(). Chasing them ended up somewhere different from where it started, so here is both the result and the route.
1309 Parameter #1 $data of method wpdb::_escape() contains unresolvable type. [argument.unresolvableType]
1309 Return type of call to method wpdb::_escape() contains unresolvable type. [method.unresolvableReturnType]
1318 Method wpdb::_escape() should return T of array<string>|string
but returns array<string>|string. [return.type]
## Why a template cannot work here
@phpstan-return T requires PHPStan to prove the returned value is the caller's *exact* T. _escape() never passes its argument through — it rebuilds it ($data[ $k ] = …) or reassigns it ($data = $this->_real_escape( $data )), and both produce a plain string / array<string>, not T.
This is not about the recursion. Deleting the recursive branch entirely, leaving a flat loop, leaves the error verbatim. Templates carry through pass-through functions, which is exactly why esc_sql() reported nothing while _escape() did. No choice of bound fixes it.
The other two errors come from the recursive branch. tests/phpstan/base.neon sets treatPhpDocTypesAsCertain: false, so PHPStan still analyses if ( is_array( $v ) ) even though $v is a string under the narrowed contract — and it still narrows inside, where \PHPStan\dumpType( $v ) reports *NEVER*. A template parameter cannot be inferred from never.
## What is committed
@phpstan-template TKey of array-key @phpstan-param string|array<TKey, string> $data @phpstan-return ( $data is string ? string : array<TKey, string> )
on both functions, with the body of _escape() unchanged — the diff is docblocks only, so there is no behaviour to review and nested arrays keep working exactly as before. That was verified by reimplementing the original algorithm alongside and diffing both on flat strings, 2-deep, 4-deep, mixed depths, empty arrays, non-scalar leaves, int/bool/null leaves, and preserved string and int keys: identical on all ten.
Callers get what the template was meant to provide, plus key preservation:
| input | result |
|---|---|
string | string
|
string[] | array<string>
|
array<string, string> | array<string, string>
|
array{a: string, b: string} | array<'a'\||'b', string>
|
implode( ',', … ) over the result | clean |
## The ordering is load-bearing
Testing $data is string first is what makes the unchanged body type-check. Inside the dead recursive branch $data is never, and a never argument satisfies whichever case is tested first. With string first the recursive call resolves to string and nothing widens. The reverse order resolves it to an array, widening $data to array<array<string>|string> and contradicting the return type:
( $data is string ? string : string[] )→ clean( $data is array ? string[] : string )→should return array<string>|string but returns array<array<string>|string>|string
There is a comment above the method saying so, because it is not obvious and the failure mode if someone flips it is confusing.
Two dead ends worth recording, so nobody re-walks them. A genuinely recursive type is not expressible: @phpstan-type EscapableData string|array<array-key, EscapableData> is rejected as typeAlias.circular, and any bounded depth is off by one, because the recursive call always produces one level more than declared. And widening to string|mixed[] measures badly — it adds 12 implode expects array<string>, array<mixed> given errors across the core call sites and silences two genuine findings, while giving callers nothing over the current string|array.
## Effect on core call sites
Measured across the ten files that call esc_sql(): 8 errors resolved, all implode expects array<string>, array<mixed> given in WP_Site_Query and WP_Network_Query, which call _escape() directly.
Six argument.templateType errors appear in WP_Comment_Query, WP_User_Query, and WP_Date_Query, where the value passed to esc_sql() is mixed so no key type can be inferred. Those same lines already report the argument type for the same reason, so this is a second symptom of an existing gap rather than new breakage. I have a follow-up branch that clears all six by typing the clause arrays in WP_Meta_Query and WP_Date_Query — it removes 51 errors across those ten files — but it touches three unrelated classes and belongs in its own ticket rather than here.
## One known limitation
PHPStan validates a conditional return type by checking the body against the union of its cases. The array<TKey, string> case is concrete enough to be checked: probes that deliberately corrupt either the value type or the string case are both reported. An earlier draft used a mixed[] fallback instead, which swallowed any array and made that half of the annotation unfalsifiable — dropping it restored the check.
While in here: no core call site passes a multi-dimensional array to esc_sql(), and every one feeds the result straight to implode(), join(), or sprintf(), which break on nested arrays regardless of how well the leaves were escaped. The recursion in _escape() has no reachable beneficiary in core. Worth knowing before anyone treats the narrowed string[] contract as a regression.
This ticket was mentioned in PR #13023 on WordPress/wordpress-develop by @westonruter.
4 weeks ago
#17
Empties and deletes the isset.variable PHPStan baseline. All six entries are fixed, so tests/phpstan/baselines/isset.variable.neon is removed along with its line in the includes of phpstan.neon.dist — the intended end state for each of these files.
Every error was the same shape: a call to isset() on a variable that PHPStan can prove is always defined and not nullable, or is never defined at all. In each case the check was dead, and in each case the surrounding code already told you why.
## The six errors
| File | Error | Fix |
| :--- | :--- | :--- |
class-custom-image-header.php | $_POST always exists | elseif ( isset( $_POST ) ) → else. The superglobal is always set, so the branch was unconditional.
|
class-wp-oembed.php | $loader always exists | Dropped && isset( $loader ). PHP_VERSION_ID is constant within a request, so the identical PHP_VERSION_ID < 80000 guard above already decides it. $loader = null is initialized for the benefit of editors that do not correlate the two constant conditions.
|
media.php | $_POST always exists | isset( $_POST ) && count( $_POST ) → ! empty( $_POST ), which is that expression by definition and also survives a non-countable $_POST instead of throwing.
|
class-wp-block-parser.php | $namespace always exists | Dropped isset( $namespace ) &&. namespace is a non-trailing optional group under PREG_OFFSET_CAPTURE, so PHP always populates it as array( '', -1 ); the -1 !== $namespace[1] test was carrying all the logic.
|
file.php | $stylesheet always exists | isset( $stylesheet ) → $stylesheet, plus a $stylesheet = null in the initializer block above. The sole assignment is guarded by ! empty( $args['theme'] ), so the variable is either null or guaranteed truthy — isset() and truthiness cannot diverge.
|
template.php | $s never defined | See below. |
## load_template() and the $s global
This one was not a redundant check but the opposite: $s is genuinely undefined as far as PHPStan is concerned, because it arrives via extract( $wp_query->query_vars, EXTR_SKIP ). Trunk suppresses the resulting variable.undefined with an inline @phpstan-ignore, and the isset.variable report was baselined.
Two changes let both go. The array is first assigned to a local, since extract() on a property expression gives PHPStan nothing to work with. That local then carries an annotation for the one key the function reads:
/** @var array{ s?: scalar, ... } $query_vars */ $query_vars = $wp_query->query_vars;
An earlier revision of this branch put that shape on WP_Query::$query_vars itself. That was wrong twice over, and 99f51e4 reverts it:
- It was inaccurate.
parse_query()gatessonly withis_scalar(), so ints, floats and bools pass through untouched.Tests_Query_ParseQuery::test_parse_query_s_typeasserts exactly that —3,3.5andtrueall survive a round trip unchanged. Hencescalar, notstring, and hence the cast thatesc_attr()now receives (behavior-preserving, since it already coerces). - It cost more than it saved. An unsealed array shape is *stricter* than a plain
arrayfor offset reads: every key other than the one named becomes "might not exist". Narrowing the shared public property removed 4 errors at rule level 10 and introduced 12, acrossWP_Queryitself,WP_Media_List_Tableand three REST controllers, plus a further 12 in theparseQuerytest file. None of those files changed, so a diff-of-changed-lines check could not have caught it.
Scoped to the local, the same annotation measures at 0 new errors and 3 removed against a full level 10 run. A plain @var is used rather than @phpstan-var so editors read it too. Documenting the full shape of query_vars belongs with #60745, not here.
## Brought forward from #11151
#11151 bumped the rule level to 1 and created these baselines. An earlier revision of that branch also fixed level 1 errors across ten files; those fixes were reverted in cce3ac0 so that the pull request stayed limited to the level bump and its tooling, with the fixes to be proposed separately. This is that follow-up for the isset.variable subset.
Two of the six overlap:
| Change | Status |
| :--- | :--- |
class-custom-image-header.php | Cherry-picked unchanged. 2a3f13d is 95ddce2 from that branch; the two have an identical git patch-id.
|
file.php | Same error, narrower fix. 25de3ac collapsed the chain to if ( $plugin ) … else …, deleting the else { $url = admin_url(); } fallback as unreachable. That reasoning holds — the function returns missing_theme_or_plugin when neither is set — but this PR keeps the fallback and tests elseif ( $stylesheet ) instead, so no reachable-looking branch is removed on the strength of a static-analysis argument. The $stylesheet = null initializer is common to both.
|
The other four are new. The locate_template() change from that branch is not included here; it addressed variable.undefined and remains deferred.
## Follow-up revisions
Each maps to a specific hunk:
| Revision | Why |
| :--- | :--- |
| r28407 | Eliminate use of extract() in get_media_item() — introduced the isset( $_POST ) && count( $_POST ) guard being replaced.
|
| r32298 | Escape the $s global — introduced the isset( $s ) / esc_attr( $s ) pair in load_template().
|
| r41721 | Introduce sandboxed live editing of PHP files — introduced wp_edit_theme_plugin_file() and its isset( $stylesheet ) check.
|
| r48789 | Only call libxml_disable_entity_loader() in PHP < 8 — introduced the PHP_VERSION_ID < 80000 && isset( $loader ) condition.
|
| r60351 | Remove unnecessary isset() check in Custom_Image_Header::step_2() — the direct precedent, in the same method, from the 6.9 round of this work.
|
| r61504 | Restore block parser in Core — the current WP_Block_Parser::next_token() body.
|
| r61699 | Integrate PHPStan into the core development workflow — added the inline @phpstan-ignore variable.undefined in load_template() that this removes.
|
| r63019 | Raise the PHPStan rule level to 1 — created the isset.variable baseline this empties.
|
## Draft SVN commit message
Code Quality: Resolve the `isset.variable` PHPStan errors.
Each of the six baselined errors was an `isset()` on a variable that is always defined and not nullable, or never defined at all. The `$_POST` superglobal is always set, so the checks on it in `Custom_Image_Header::step_2()` and `get_media_item()` were unconditional; the latter becomes `! empty( $_POST )`, which is what `isset()` plus `count()` already meant. `PHP_VERSION_ID` is constant within a request, so the guard above the `$loader` check in `WP_oEmbed::_parse_xml()` already decides it. The `namespace` group in `WP_Block_Parser::next_token()` is a non-trailing optional group under `PREG_OFFSET_CAPTURE`, which PHP always populates. In `wp_edit_theme_plugin_file()` the only assignment to `$stylesheet` is guarded by a `! empty()` on the argument it comes from, so a truthiness test cannot diverge from `isset()`.
The remaining error is the reverse case: `$s` in `load_template()` is undefined to static analysis because it arrives through `extract()`. Assigning the query vars to a local first, and annotating that local as `array{ s?: scalar, ... }`, resolves both that report and the inline `@phpstan-ignore` above it. The annotation is deliberately local rather than on `WP_Query::$query_vars`, where an unsealed shape makes every unnamed key "might not exist" and reports more than it resolves. `scalar` rather than `string` is what `WP_Query::parse_query()` actually guarantees, since it gates the value only with `is_scalar()`.
With the last entry fixed, the baseline file and its entry in the `includes` of `phpstan.neon.dist` are removed.
Developed in PR_URL_PLACEHOLDER.
Follow-up to r28407, r32298, r41721, r48789, r60351, r61504, r61699, r63019.
See #65817.
The Props line is deliberately absent — it should come from props-bot once this has been reviewed and tested.
## Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Opus 5
Used for: Analysis of each PHPStan error and its surrounding history, the fixes, the commit messages, this description and the draft commit message above. Every change was directed, reviewed and revised by me. The equivalence argument for each fix, the measured error counts, and the reverted WP_Query annotation were all verified in the working tree.
@westonruter commented on PR #13023:
4 weeks ago
#18
@irozum Hi. It seems like https://github.com/WordPress/wordpress-develop/pull/13023#pullrequestreview-4921984872 was written by AI. When you use AI to add reviews, please disclose how you have done so. Otherwise, it is misleading given that your comment says “I” and “me” when actually it was “it”. Please refer to the AI Guidelines.
Please also add any necessary AI disclosure to https://github.com/WordPress/wordpress-develop/pull/12975#pullrequestreview-4911674226.
This ticket was mentioned in PR #13041 on WordPress/wordpress-develop by @callumbw95.
4 weeks ago
#19
WP_REST_Template_Autosaves_Controller declares its own private $parent_post_type and assigns it in the constructor on the line after parent::__construct(), which already performs that assignment:
public function __construct( $parent_post_type ) { parent::__construct( $parent_post_type ); $this->parent_post_type = $parent_post_type; // never read
WP_REST_Autosaves_Controller declares $parent_post_type privately as well, so PHP allocates a separate slot for each declaration rather than reusing one. Nothing on the subclass ever reads its copy, which is the single property.onlyWritten occurrence in the baseline:
Property WP_REST_Template_Autosaves_Controller::$parent_post_type is never read, only written. src/wp-includes/rest-api/endpoints/class-wp-rest-template-autosaves-controller.php
Removing the declaration and the assignment leaves the two remaining assignments in that block over-aligned, so they are realigned to match.
### Why this cannot change behaviour
Private properties with the same name in a class and its subclass do not share storage, and a method inherited from the ancestor always reads the ancestor's slot regardless of whether the subclass declares its own. That is worth demonstrating rather than asserting:
class A { private $p; public function __construct( $v ) { $this->p = $v; } public function readIt() { return $this->p; } } class B extends A { private $p; public function __construct( $v ) { parent::__construct( $v ); $this->p = $v; } } class C extends A {} var_dump( array_keys( (array) new B( 'val' ) ) ); // ["\0A\0p", "\0B\0p"] two slots var_dump( array_keys( (array) new C( 'val' ) ) ); // ["\0A\0p"] one slot var_dump( ( new B( 'val' ) )->readIt() ); // 'val' var_dump( ( new C( 'val' ) )->readIt() ); // 'val'
B is the current shape and C is the shape after this change. The inherited readIt() returns the same value either way, because it reads A's slot in both cases. The subclass copy is pure overhead.
The property is private, so it has no public or extender-facing surface and removing it carries no backward compatibility cost, despite the @since 6.4.0 on its docblock.
### How it got there
This class and WP_REST_Template_Revisions_Controller were added together in [56819] (2023-10-10), which introduced both files in one go at 276 and 297 lines. The two constructors are near-copies of one another.
The revisions sibling genuinely needs its shadowed copy. WP_REST_Template_Revisions_Controller::get_parent() reads $this->parent_post_type from the subclass, and the ancestor's copy is private and therefore out of scope there, so without the redeclaration that read would fail:
protected function get_parent( $parent_template_id ) { $template = get_block_template( $parent_template_id, $this->parent_post_type );
No method on the autosaves controller does anything equivalent. The property is load-bearing in one of the pair and vestigial in the other, which is consistent with the two constructors having been written from the same starting point. Neither the declaration nor the assignment has been touched since [56819].
### One thing I left alone
WP_REST_Autosaves_Controller::$parent_post_type, the ancestor's own copy, also looks like it is written and never read. Its own methods do not reference it, and the reads at WP_REST_Revisions_Controller lines 164 and 826 resolve to that class's slot rather than this one.
PHPStan does not report it, including when the baselines are suppressed during regeneration, so I have not touched it. I cannot account for why the two are treated differently and did not want to act on a claim I could not substantiate. Flagging it in case it is of interest.
### Testing instructions
- On
trunk,npm run typecheck:phpreports[OK] No errors, because the occurrence is baselined. - Delete
tests/phpstan/baselines/property.onlyWritten.neonand itsincludesentry, then run it again. PHPStan reportsProperty WP_REST_Template_Autosaves_Controller::$parent_post_type is never read, only written. - With this branch applied,
npm run typecheck:phpreports[OK] No errorswith the baseline gone and nothing new elsewhere. The baseline directory goes from 70 files to 69. - Regenerating confirms the baseline is genuinely empty rather than hand-removed:
composer phpstan:baselines -- --identifier=property.onlyWritten
It reports no remaining errors and leaves both deletions in place. composer lintis clean on the changed file, including the realigned assignments.- Autosave and template coverage passes:
npm run test:php -- --filter 'Autosave|Template'gives 779 tests, 2594 assertions, 5 skipped, exit 0.tests/phpunit/tests/rest-api/wpRestTemplateAutosavesController.phpwas added alongside the class in [56819] and has 23 test methods, all of which construct the controller. - Full suite passes: 30854 tests, 4559519 assertions, 86 warnings, 44 skipped, exit 0, no failures or errors.
## Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Opus 5
Used for: tracing the property through the four-class hierarchy to establish which copies are read, git log archaeology identifying [56819] as the origin, constructing the private-shadowing demonstration above, and drafting this description. The diagnosis, the decision to leave the ancestor's copy alone, the baseline regeneration, and verification against full PHPStan, PHPCS and PHPUnit runs were reviewed and confirmed by me in a local development environment.
@westonruter commented on PR #12975:
4 weeks ago
#20
@irozum As I mentioned in https://github.com/WordPress/wordpress-develop/pull/13023#issuecomment-5274041787, it seems like https://github.com/WordPress/wordpress-develop/pull/12975#pullrequestreview-4911674226 was written by AI. When you use AI to add reviews, please disclose how you have done so. Otherwise, it is misleading given that your comment says “I” and “me” when actually it was “it”. Please refer to the AI Guidelines.
This ticket was mentioned in PR #13044 on WordPress/wordpress-develop by @westonruter.
4 weeks ago
#22
Widens the @param annotation on the esc_*() escaping functions from string to string|int|float, and adds an explicit (string) cast to each.
This clears 54 baseline entries covering 101 errors from tests/phpstan/baselines/argument.type.neon. esc_attr() alone accounted for 94 of them — the largest single cluster in that file.
## Background
esc_attr(), esc_html(), esc_js(), esc_textarea(), and esc_xml() were all annotated @param string $text, but none of them has ever required a string at runtime. The two helpers they delegate to each open by coercing:
_wp_specialchars()—$text = (string) $text;wp_check_invalid_utf8()— the same
Tracing that cast back through [11380] (which deprecated wp_specialchars() in favour of esc_html()) to [10298] puts it in 2009. Since esc_attr() and esc_html() are @since 2.8.0, also 2009, the coercion is as old as the functions themselves. Only the annotation ever claimed otherwise — which is why passing an int, overwhelmingly a post ID, a term ID, or a count, has always worked while still registering as a static analysis error.
Confirmed against the current codebase (PHP 8.5):
| input | esc_attr() | esc_html() | esc_js() | esc_textarea()
|
|---|---|---|---|---|
42 | '42' | '42' | '42' | '42'
|
-7 | '-7' | '-7' | '-7' | '-7'
|
1.5 | '1.5' | '1.5' | '1.5' | '1.5'
|
true | '1' | '1' | '1' | '1'
|
false | '' | '' | '' | ''
|
## Why the cast is not redundant
For esc_attr(), esc_html(), and esc_js() the value is already coerced downstream, so the new cast does not change what gets escaped. What it does change is the second argument handed to the attribute_escape, esc_html, and js_escape filters. Each is documented as @param string $text The text prior to being escaped, but callbacks previously received the raw int or float. The cast makes that documented contract true.
esc_textarea() is the one case where the cast also affects escaping: it passed $text straight to htmlspecialchars(), so esc_textarea( null ) emitted a PHP 8.1+ deprecation. That no longer happens.
## Why bool is excluded
bool is accepted at runtime, and the cast handles it, but it is deliberately left out of the annotation. Casting true yields '1' while false yields '', and an empty string is indistinguishable from a missing value, an empty option, or a failed lookup in any output context. There is no output context where '' is a meaningful rendering of *false*.
Core has exactly two call sites passing a bool, and they split evenly:
wp-admin/options-general.php:193—data-state="<?php echo esc_attr( has_site_icon() ); ?>". This works, becausesite-icon.jscompares against the literal'1'and writes back'1'/''. The convention is real but implicit.wp-admin/customize.php:291—aria-pressed="<?php echo esc_attr( $active ); ?>". This rendersaria-pressed="1"for the desktop button andaria-pressed=""for tablet and mobile. Valid ARIA tristate values are only"true","false","mixed", and"undefined", so all three buttons are exposed as non-toggles untilcontrols.js(which correctly writes"true"/"false") runs.
Admitting bool to the signature would permanently silence the second one. Both stay baselined instead, so they remain visible as outstanding work. The accessibility fix is intentionally not in this PR and will be handled separately.
## Scope
esc_textarea() and esc_xml() had no baselined errors and are included only to keep the family consistent — five functions with identical semantics should not carry three different parameter annotations.
Call sites are not touched. Nothing here changes escaping behaviour for any input that was already a string.
## Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Opus 5
Used for: Investigating the runtime coercion behaviour and its history, the docblock and cast changes, regenerating the baselines, and drafting this description. The decision to exclude bool and to defer the accessibility fix was mine, as was review of the result.
This ticket was mentioned in PR #13046 on WordPress/wordpress-develop by @westonruter.
4 weeks ago
#24
The responsive-preview buttons in the Customizer emit invalid values for aria-pressed, so none of the three is exposed to assistive technology as a toggle button on initial render.
## The defect
wp-admin/customize.php rendered the attribute through esc_attr() with a bool:
$active = ! empty( $settings['default'] ); ... <button type="button" ... aria-pressed="<?php echo esc_attr( $active ); ?>" ...>
PHP casts true to '1' and false to the empty string. Since only the Desktop device carries 'default' => true, the markup came out as:
<button ... aria-pressed="1" data-device="desktop"> <button ... aria-pressed="" data-device="tablet"> <button ... aria-pressed="" data-device="mobile">
aria-pressed is a tristate attribute whose only valid values are true, false, mixed, and undefined. Both 1 and the empty string are invalid, and an invalid or empty value is treated as undefined, which means the element is not exposed as a toggle button at all. So on page load the pressed state of all three buttons was unavailable to screen reader users, and the currently selected preview size was not announced.
The state does become correct after the first interaction, because customize-controls.js sets it via jQuery:
.attr( 'aria-pressed', false ); ... .attr( 'aria-pressed', true );
jQuery stringifies those to "false" and "true", which are valid. So the JavaScript was always right and the server render disagreed with it — the markup only repaired itself once the user clicked something.
## The fix
Emit the literals directly, so the initial render agrees with what the JavaScript later writes:
aria-pressed="<?php echo $active ? 'true' : 'false'; ?>"
No escaping is needed since both values are literals. This matches the existing pattern for aria-expanded in wp-admin/includes/template.php, which assigns 'true'/'false' as strings.
Every aria-pressed, aria-expanded, aria-selected, aria-checked, aria-disabled, aria-current, and aria-invalid value rendered from PHP across src/wp-admin, src/wp-includes, and src/wp-content/themes was checked. This was the only one not already a valid literal.
## Also included
wp-admin/options-general.php used the same bool-to-string cast for the Site Icon button's data-state:
data-state="<?php echo esc_attr( has_site_icon() ); ?>"
That one is not a defect. site-icon.js compares against the literal '1' and writes back '1'/'', so the cast happened to produce exactly the sentinels the script expects. It is changed to has_site_icon() ? '1' : '' purely to state that contract rather than leave it resting on PHP's cast rules. Behaviour is identical.
It is included here because these were the only two places in core passing a bool to an escaping function. Commit r63296 widened the esc_*() annotations to string|int|float and deliberately excluded bool so that both call sites stayed visible rather than being silently permitted by the signature; resolving them empties the last of the esc_*() entries from tests/phpstan/baselines/argument.type.neon.
Trac ticket: Core-65817
## Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Opus 5
Used for: Identifying the invalid attribute values, sweeping core for other occurrences, the fix itself, and drafting this description. Reviewed by me.
This ticket was mentioned in PR #13057 on WordPress/wordpress-develop by @tstokes8040.
4 weeks ago
#25
PHPStan infers WP_HTML_Tag_Processor::parse_next_tag() as pure, so it retains
the STATE_READY narrowing assigned to $parser_state on line 981 across the
call on line 989 — even though parse_next_tag() reassigns that property
throughout its body. Every state comparison after the call is then decided at
analysis time rather than at runtime.
The effects compound. The three !== comparisons on lines 1004–1006 each
resolve to true, so both && nodes in that condition resolve to true, so
the early return true on line 1008 is treated as unconditional. Everything
from line 1012 to the end of base_class_next_token() is consequently analyzed
as unreachable, which in turn makes skip_rawtext() and skip_script_data()
appear uncalled and $skip_newline_at appear never assigned an int.
@phpstan-impure is the annotation PHPStan's own tip recommends here, and there
is precedent for it in this very file: next_tag() already carries one.
Ten baselined errors across six identifiers are resolved:
| Identifier | Removed |
|---|---|
booleanAnd.alwaysTrue | 2 |
notIdentical.alwaysTrue | 3 |
identical.alwaysFalse | 1 |
deadCode.unreachable | 1 |
method.unused | 2 |
property.unusedType | 1 |
booleanAnd.alwaysTrue and property.unusedType reach zero, so as the baseline
headers direct, both files are deleted along with their includes entries in
phpstan.neon.dist. The baselines were regenerated with
composer phpstan:baselines -- --identifier=..., not edited by hand.
### Testing instructions
npm run typecheck:phpontrunkreports[OK] No errors, because the occurrences are baselined.- Delete
tests/phpstan/baselines/booleanAnd.alwaysTrue.neonand itsincludesentry, then re-run: PHPStan reportsResult of && is always true.twice atsrc/wp-includes/html-api/class-wp-html-tag-processor.php:1004. - With this branch applied,
npm run typecheck:phpreports[OK] No errorsacross 1289 files with both baselines gone.composer lintis also clean for the modified file. npm run test:php— 30865 tests, 4558966 assertions, 86 warnings, 44 skipped, and one failure:Tests_Script_Modules_WpScriptModules::test_default_script_module_files_exist, looking forsrc/wp-includes/js/dist/script-modules/a11y/index.js. That path is gitignored build output that is unbuilt in my checkout, and the test fails identically on unmodifiedtrunk, so it is unrelated to this change.npm run test:php -- --group html-apipasses 1714 tests, 6107 assertions, 0 failures.
Documentation-only change; no runtime behaviour is affected.
## Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Opus 5
Used for: locating the occurrence via the PHPStan booleanAnd.alwaysTrue
baseline, diagnosing the purity-inference cause and its knock-on effects,
regenerating the baselines, and drafting this description. The change itself and
the verification against the full PHPStan, PHPCS and PHPUnit runs were reviewed
and confirmed by me in a local development environment.
@irozum commented on PR #12975:
4 weeks ago
#27
You're right — I've added an AI assistance disclosure to the review above. Going forward, reviews from this account will include that by default.
@irozum commented on PR #13023:
4 weeks ago
#28
Fair catch — disclosure has been added to both this review and the one on #12975. Won't happen again.
@irozum commented on PR #12975:
4 weeks ago
#29
You're right — I've added an AI assistance disclosure to the review above. Going forward, reviews from this account will include that by default.
@irozum commented on PR #13023:
4 weeks ago
#30
Fair catch — disclosure has been added to both this review and the one on #12975. Won't happen again.
@afercia commented on PR #13046:
4 weeks ago
#31
So on page load the pressed state of all three buttons was unavailable t
The state does become correct after the first interaction, becausecustomize-controls.jssets it via jQuery:
While I do see aria-pressed="" / aria-pressed="1" in the markup, on page load I see in the DOM they have already been changed to true / false before any interaction. Am I missing something?
Anyways, the fix makes totally sense. Good catch.
@westonruter commented on PR #13046:
4 weeks ago
#32
Am I missing something?
@afercia no, I don't think you're missing anything. This is a minor correctness fix which probably won't have any a11y impact.
Thank you for reviewing.
This ticket was mentioned in PR #13064 on WordPress/wordpress-develop by @westonruter.
3 weeks ago
#33
Fixes the static analysis regressions introduced during the 7.1 cycle: errors that trunk reports and a baseline generated from 7.0.0's src does not.
Each was verified before being touched. An error is only treated as a regression if the symbol whose type provoked it was itself changed after the 7.0 tag — following the *symbol* rather than the reporting line, because a docblock edit in one file surfaces errors in files that have not been edited in years. r62178 changing WP_Widget::form() produced 20 errors in widget subclasses, every one of them on untouched code.
Deliberately out of scope: errors that appeared because an annotation became *more accurate*. The largest group follows r62529 giving wpdb::get_col() and friends precise return types, which made pre-existing call-site looseness checkable for the first time. That code is as old as 2012 and unchanged; it is technical debt, not a regression. Same for the @return never annotations that made long-standing defensive code visibly unreachable.
Of the 63 distinct symbols and sites behind the remaining errors, 9 had changed since 7.0.0. Every genuine regression traced back to a type-annotation or code-quality commit rather than to feature work.
## Committing to SVN
These commits may be landed in SVN separately rather than as one changeset. Each is self-contained: it changes one thing, regenerates the affected baseline, and leaves the tree green on its own. The table gives the revision(s) each would be a follow-up to.
| Git commit | Follow-up to | What it fixes |
|---|---|---|
d875967a93 Restore string\||void on WP_Widget::form() | r62178 | void removed from a union tightened the contract; 18 subclass form() overrides echo and return nothing. 20 errors.
|
5df03172a5 Include the array returns in WP_Block_Type::__get()'s type | r62178 | Same commit, opposite remedy: every path returns a value, so the union needed completing with the array[] from get_variations(), not void restored.
|
84c63c3362 Correct term_exists()'s conditional return for the no-taxonomy case | r62680 | The conditional type says int\||null for the empty-$taxonomy branch; that branch does return (string) $_term.
|
77dc415362 Allow an integer for WP_Comment's two ID properties | r62822 | numeric-string is contradicted by get_comment_to_edit(), which casts both to int in place. Clears 39 baselined errors across 9 files.
|
97f15444cf Call WP_Theme_JSON's private static methods through self | r62444, r62671, r62731, r62746 | 16 call sites reached 7 private statics via static::, which resolves to the runtime class where a private method is not visible.
|
5c6a81b0b9 Document get_feature_declarations_for_node()'s params as arrays | r62444, r62607 | Annotated object since 6.3.0 but only ever passed arrays. The by-reference $node propagated the wrong type back to callers. 5 errors from one docblock.
|
cbcdd48b1f Document comment_shortcuts and infinite_scrolling on WP_User | r62632 | A new user preference read through __get() without being listed among the class's @property tags, unlike rich_editing beside it.
|
cf9a4af441 Declare the $wpdb global on the Users screen | r62688 | New queries on a screen that never imported the global. |
60664f9e24 Drop isset() checks on properties that are always set | r62453, r62838 | isset() paired with ! empty() / is_array() on declared properties with non-null defaults.
|
b7a00c1796 Drop a redundant empty check in merge_properties() | r62834 | array() !== $current after ! array_is_list( $current ), which already implies non-empty.
|
b24d67b426 Stop baselining a substr_compare() report PHPStan gets wrong | r62667 | Not a code defect. PHPStan's pre-8.0 functionMap.php drops the implicit nullability of $length; moved to ignoreErrors with the reasoning recorded.
|
## Previously committed
r63296 (abc50fe6f82b2c64cb7b1555567cc9db73e7eb17, "Formatting: Document that escaping functions can take numbers") came out of the same review and is already in trunk, so it is part of this branch's history rather than under review here. It widened the esc_*() annotations to string|int|float and cleared 54 baseline entries. Worth noting it was not a regression fix: 87 of the 103 errors it resolved already existed at 7.0.0.
## Not fixed, and why
_upgrade_cron_array()(r62488) — the@phpstan-returnshape is correct about runtime behaviour, but building an array key-by-key and then settingversioncollapses PHPStan's inference tonon-empty-array<'version'|int, …>, losing the key/value correlation. Weakening a correct type to satisfy the analyzer seemed worse than leaving it baselined.WP_Postproperty reports (r62717) —$_wp_attachment_image_altis read through__get(), which accepts *any* meta key, so unlike theWP_Usercase above it cannot be resolved with a@propertytag.
## Verification
Every fix was checked with a full analysis before regenerating baselines, so that "no new errors" means the change introduced none rather than that regeneration absorbed them. Relevant suites were run per change: 316 theme.json, 794 comment, 104 block type, 78 view config, 46 block supports states, 26 term_exists.
Trac ticket: Core-65817
## Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Opus 5
Used for: Building the 7.0.0-vs-trunk comparison, classifying each error by whether the symbol behind it changed after the tag, the fixes themselves, and drafting this description. Several of its intermediate classifications were wrong and were corrected after I pushed back on them; the scope decisions, and the final read of each fix, are mine.
This ticket was mentioned in PR #13071 on WordPress/wordpress-develop by @tstokes8040.
3 weeks ago
#34
DOING_AJAX, DOING_CRON, REST_REQUEST, WP_INSTALLING, WP_INSTALLING_NETWORK, WP_REPAIRING and XMLRPC_REQUEST are each defined only as the literal true within the analyzed tree, so PHPStan concluded that the right side of the defined( 'X' ) && X idiom can never be falsy:
| Constant | Sole definition |
|---|---|
DOING_AJAX | wp-admin/admin-ajax.php:16
|
DOING_CRON | wp-cron.php:42
|
REST_REQUEST | wp-includes/rest-api.php:478
|
WP_INSTALLING | wp-admin/install.php:32
|
WP_INSTALLING_NETWORK | wp-admin/network.php:13
|
WP_REPAIRING | wp-admin/maint/repair.php:8
|
XMLRPC_REQUEST | xmlrpc.php:13
|
That conclusion is correct about the code PHPStan can see and wrong about reality. Every one of these is request-scoped: absent on most requests, defined only by the front controller handling that particular kind of request, and definable by a plugin or by wp-config.php before load. The defined( 'X' ) && X guard is doing real work, and the constants belong in dynamicConstantNames alongside the ones already listed there.
This resolves 11 booleanAnd.rightAlwaysTrue occurrences, leaving 5 in the baseline:
| File | Line(s) |
|---|---|
wp-admin/includes/schema.php | 46 |
wp-includes/functions.php | 3857 |
wp-includes/l10n.php | 962, 967, 971 |
wp-includes/load.php | 643, 1638, 1753, 1789 |
wp-includes/user.php | 387, 3827 |
The baseline is generated, not hand-edited; it was regenerated with composer phpstan:baselines -- --identifier=booleanAnd.rightAlwaysTrue.
No source file is touched, so there is no runtime behaviour to change. Follow-up to [63191], which added DB_COLLATE and WP_DEVELOPMENT_MODE for the same reason.
### Testing instructions
npm run typecheck:phpontrunkreports[OK] No errors, because the occurrences are baselined.- With this branch applied,
npm run typecheck:phpreports[OK] No errorsacross 1289 files, withtests/phpstan/baselines/booleanAnd.rightAlwaysTrue.neondown from 9 entries / 16 occurrences to 5 entries / 5 occurrences. - To confirm the change is surgical, run the analysis with all baselines suppressed both before and after and diff the results: exactly 11 errors disappear, all of them
booleanAnd.rightAlwaysTrue, and no new error of any identifier appears anywhere in the codebase. npm run test:php -- --group html-apiand the wider suite are unaffected, since no source file changes.
## Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Opus 5
Used for: grouping the remaining boolean PHPStan baselines by root cause, identifying this cluster and the constants behind it, measuring the before/after error diff across the whole codebase, regenerating the baseline, and drafting this description. The change itself and the verification runs were reviewed and confirmed by me in a local development environment.
This ticket was mentioned in PR #13069 on WordPress/wordpress-develop by @dpantazis.
3 weeks ago
#35
### Removes the below phpstan baselines and fixes the issues that they were covering:
- deadCode.unreachable.neon
- if.alwaysTrue.neon
- while.alwaysFalse.neon
- while.alwaysTrue.neon
### How ?
Marks the have_comments() and have_posts() methods as impure since they can change their return type based on the state of a speficic WP_Query instance or the global's WP_Query instance.
In the file src/wp-content/themes/twentyfourteen/inc/widgets.php moves the $tmp_more definition out of the while loop $ephemera->have_posts(). This was a bug, since it would guarantee that GLOBALS['more'] and the $tmp_more would be overwritten to 0 if there were at least 2 loops.
Loom showing the bug https://www.loom.com/share/c713c48d553b4e6abfdb3e2b9b5f2cf7
Trac ticket: https://core.trac.wordpress.org/ticket/65817
## Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code
Model(s): Opus 5
Used for: Help with suggesting how to fix the phpstan output for the specific errors we are removing the baselines for. The actual result has been reviewed and are owned by me.
This ticket was mentioned in PR #13072 on WordPress/wordpress-develop by @dpantazis.
3 weeks ago
#36
### Removes errors from the below phpstan baselines and fixes the issues that they were covering:
- staticClassAccess.privateMethod.neon
### How ?
Replaces usage of static:: with self:: in classes which were using self while trying to access API meant to be private.
This is safe to merge. Even if the static usage was aimed to be used for overwrite via inheritance, if a Child class would try to overwrite the private static methods a fatal error would be thrown.
Trac ticket: https://core.trac.wordpress.org/ticket/65817
## Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code
Model(s): Opus 5
Used for: Help with suggesting how to fix the phpstan output for the specific errors we are removing the baselines for. The actual result has been reviewed and are owned by me.
This ticket was mentioned in PR #13073 on WordPress/wordpress-develop by @dpantazis.
3 weeks ago
#37
### Removes errors from the below phpstan baselines and fixes the issues that they were covering:
- catch.neverThrown.neon
### How ?
Adds a doc-level @throws for the Exception specifically which may very well happen in all these methods since all these methods may call the method bookmark_token
Trac ticket: https://core.trac.wordpress.org/ticket/65817
## Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code
Model(s): Opus 5
Used for: Help with suggesting how to fix the phpstan output for the specific errors we are removing the baselines for. The actual result has been reviewed and are owned by me.
This ticket was mentioned in PR #13074 on WordPress/wordpress-develop by @tstokes8040.
3 weeks ago
#38
PHPStan infers current_user_can(), WP_Filesystem_Base::chmod() and WP_HTML_Processor::next_token() as pure, so it remembers what each returned and reuses that value at a later call in the same scope. All three can legitimately answer differently the second time:
| Function | Why a second call can differ |
|---|---|
current_user_can() | Reads the current user and applies the user_has_cap filter, so a plugin hooked in between can change the answer
|
WP_Filesystem_Base::chmod() | Changes the permissions that a following is_writable() reports
|
WP_HTML_Processor::next_token() | Advances the parser, so the token accessors then describe a different token |
The clearest case is wp-admin/link-manager.php. Line 11 bails out unless the user has manage_links, and line 91 checks the same capability again after admin-header.php has been required and its hooks have fired. PHPStan carried the line 11 answer forward and reported the line 91 guard as dead.
### Annotating the mutation, not the accessor
PHPStan's tips name is_writable(), is_tag_closer(), expects_closer(), get_current_depth() and set_bookmark() as candidates. Those are read-only accessors and annotating them would be inaccurate. What actually invalidates a remembered value is the mutating call sitting between the two reads, so the annotation belongs there: in WP_Upgrader::install_package() it is the chmod() between two is_writable() calls, and in WP_Block it is the next_token() loop between two is_tag_closer() calls.
WP_Filesystem_Base::copy() was annotated at first and then removed after re-running the analysis with and without it produced identical results; chmod() alone covers the copy_dir() case. WP_User::has_cap(), where the user_has_cap filter is actually applied, was also tried and cleared nothing, so the annotation sits on current_user_can() itself.
### Impact
16 errors resolved across seven identifiers, with no new error of any identifier anywhere in the codebase. Eleven are in the boolean baselines this targets:
| Baseline | Before | After |
|---|---|---|
booleanNot.alwaysFalse | 7 | 2 |
booleanNot.alwaysTrue | 8 | 4 |
booleanAnd.leftAlwaysTrue | 5 | 4 |
booleanOr.alwaysTrue | 2 | 1 |
The other five fall outside those baselines and come along for free: wp-admin/my-sites.php and wp-admin/upload.php (×2) in if.alwaysTrue, wp-admin/theme-install.php in ternary.alwaysTrue, and a deadCode.unreachable in class-wp-block.php that was only unreachable because of the boolean error above it.
The baselines are generated, not hand-edited. No baseline reaches zero here, so none is deleted and phpstan.neon.dist is unchanged.
### Testing instructions
npm run typecheck:phpontrunkreports[OK] No errors, because the occurrences are baselined.- With this branch applied,
npm run typecheck:phpreports[OK] No errorsacross 1289 files with the seven baselines regenerated. - To confirm the change is surgical, run the analysis with all baselines suppressed before and after and diff the results, comparing on file plus message rather than on line number: adding two lines to
class-wp-html-processor.phpshifts every line below it, which makes a naive line-keyed diff report roughly 40 spurious changes. Compared correctly, exactly 16 errors disappear and none appears. npm run test:php— 30871 tests, 4558975 assertions, 86 warnings, 44 skipped, and one failure:Tests_Script_Modules_WpScriptModules::test_default_script_module_files_exist, looking forsrc/wp-includes/js/dist/script-modules/a11y/index.js. That path is gitignored build output that is unbuilt in my checkout, and the test fails identically on unmodifiedtrunk, so it is unrelated to this change.
Documentation-only change; no runtime behaviour is affected.
Related: https://github.com/WordPress/wordpress-develop/pull/13057 annotates WP_HTML_Tag_Processor::parse_next_tag() for the same reason, and https://github.com/WordPress/wordpress-develop/pull/13071 clears a different cluster of the same boolean baselines. All three are independent and touch disjoint files.
## Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Opus 5
Used for: grouping the remaining boolean PHPStan baselines by root cause, identifying this cluster, testing which annotation placements were load-bearing, measuring the before/after error diff across the whole codebase, regenerating the baselines, and drafting this description. The change itself and the verification runs were reviewed and confirmed by me in a local development environment.
This ticket was mentioned in PR #13077 on WordPress/wordpress-develop by @tstokes8040.
3 weeks ago
#39
PHPStan infers current_theme_supports(), wp_installing() and is_front_page() as pure, so it remembers what each returned and reuses that value at a later call in the same scope. All three read state that can change between calls:
| Function | Mutable state it reads |
|---|---|
current_theme_supports() | The $_wp_theme_features global, and applies the current_theme_supports-{$feature} filter before returning
|
wp_installing() | A static $installing, which the same function sets when passed an argument
|
is_front_page() | The $wp_query global
|
This resolves three baselined errors:
| Site | Error |
|---|---|
Custom_Image_Header::step_1(), class-custom-image-header.php:619 | booleanNot.alwaysTrue
|
get_transient(), option.php:1459 | booleanNot.alwaysTrue
|
redirect_canonical(), canonical.php:688 | booleanAnd.leftAlwaysTrue
|
In the first, the enclosing elseif chain has already tested current_theme_supports( 'custom-header', 'flex-height' ), so PHPStan carried that answer into the inner check. In the second, get_transient() reaches the else branch only when wp_using_ext_object_cache() || wp_installing() was false, so the later ! wp_installing() looked settled.
booleanNot.alwaysTrue goes from 8 to 6 and booleanAnd.leftAlwaysTrue from 5 to 4. The baselines are generated, not hand-edited; neither reaches zero, so no baseline is deleted and phpstan.neon.dist is unchanged.
### A note on redirect_canonical()
The condition reads ( ! is_front_page() || is_front_page() && get_query_var( 'paged' ) > 1 ). Both calls sit inside one expression with nothing in between, so ! A || A && B could equally be simplified to ! A || B rather than annotated. I chose the annotation because it is independently true — is_front_page() really does read mutable global state, and PHPStan should not assume otherwise anywhere else either — and because rewriting a condition in the canonical redirect path carries behaviour risk for no analytical gain. Happy to switch to the simplification if reviewers prefer it.
### Testing instructions
npm run typecheck:phpontrunkreports[OK] No errors, because the occurrences are baselined.- With this branch applied,
npm run typecheck:phpreports[OK] No errorsacross 1289 files with both baselines regenerated. - To confirm the change is surgical, run the analysis with all baselines suppressed before and after and diff the results on file plus message rather than line number: exactly three errors disappear and none appears, anywhere in the codebase, for any identifier.
composer lintreports no errors for the three modified files.src/wp-includes/query.phpcarries four pre-existingWordPress.DB.PreparedSQL.NotPreparedwarnings at lines 1228 and 1231; the same four are present in the file ontrunkand are untouched by a docblock change at line 462.
Documentation-only change; no runtime behaviour is affected.
Related: https://github.com/WordPress/wordpress-develop/pull/13074 annotates current_user_can(), WP_Filesystem_Base::chmod() and WP_HTML_Processor::next_token() for the same reason, and https://github.com/WordPress/wordpress-develop/pull/13057 annotates WP_HTML_Tag_Processor::parse_next_tag(). This PR is independent of both and touches disjoint files.
## Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Opus 5
Used for: grouping the remaining boolean PHPStan baselines by root cause, identifying this cluster, measuring the before/after error diff across the whole codebase, regenerating the baselines, and drafting this description. The change itself and the verification runs were reviewed and confirmed by me in a local development environment.
This ticket was mentioned in PR #13078 on WordPress/wordpress-develop by @dpantazis.
3 weeks ago
#40
Removes in total 1 errors.
### Removes errors from the below phpstan baselines and fixes the issues that they were covering:
- encapsedStringPart.nonString.neon
### How ?
Better specify the shape of the return type.
Trac ticket: https://core.trac.wordpress.org/ticket/65817
## Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code
Model(s): Opus 5
Used for: Help with suggesting how to fix the phpstan output for the specific errors we are removing the baselines for. The actual result has been reviewed and are owned by me.
This ticket was mentioned in PR #13079 on WordPress/wordpress-develop by @dpantazis.
3 weeks ago
#41
Removes in total 1 errors.
### Removes errors from the below phpstan baselines and fixes the issues that they were covering:
- foreach.nonIterable
### How ?
By type casting the property to array.
Trac ticket: https://core.trac.wordpress.org/ticket/65817
## Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code
Model(s): Opus 5
Used for: Help with suggesting how to fix the phpstan output for the specific errors we are removing the baselines for. The actual result has been reviewed and are owned by me.
This ticket was mentioned in PR #13080 on WordPress/wordpress-develop by @dpantazis.
3 weeks ago
#42
Removes in total 1 errors.
### Removes errors from the below phpstan baselines and fixes the issues that they were covering:
- greater.invalid
### How ?
Removes dead else if statement.
Trac ticket: https://core.trac.wordpress.org/ticket/65817
## Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code
Model(s): Opus 5
Used for: Help with suggesting how to fix the phpstan output for the specific errors we are removing the baselines for. The actual result has been reviewed and are owned by me.
This ticket was mentioned in PR #13081 on WordPress/wordpress-develop by nomad-mystic.
3 weeks ago
#43
Good afternoon,
This PR updates the @return values in the DocBlocks with void, removes a redundant variable, and updates the neon files.
Trac ticket: https://core.trac.wordpress.org/ticket/65817
## Use of AI Tools
AI assistance: No
Tool(s): N/A
Model(s): N/A
This ticket was mentioned in PR #13082 on WordPress/wordpress-develop by @dpantazis.
3 weeks ago
#44
Removes in total 1 errors.
### Removes errors from the below phpstan baselines and fixes the issues that they were covering:
- return.missing
### How ?
Adds return types according to method docblocks.
Trac ticket: https://core.trac.wordpress.org/ticket/65817
## Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code
Model(s): Opus 5
Used for: Help with suggesting how to fix the phpstan output for the specific errors we are removing the baselines for. The actual result has been reviewed and are owned by me.
This ticket was mentioned in PR #13086 on WordPress/wordpress-develop by @tstokes8040.
3 weeks ago
#45
Each of these three conditions re-tests something the surrounding expression has already established, so PHPStan reports the redundant operand as always true.
wp_render_typography_support(), block-supports/typography.php:307 — ! empty( $x ) already requires the value to be truthy, so the following && $x can never fail.
-if ( ! empty( $block['attrs']['fitText'] ) && $block['attrs']['fitText'] && ! is_admin() ) { +if ( ! empty( $block['attrs']['fitText'] ) && ! is_admin() ) {
_get_block_templates_files(), block-template-utils.php:463 — the right operand of the || is only evaluated when ! $post_type was false, so the leading $post_type && is always true there.
! $post_type || -( $post_type && isset( $candidate['postTypes'] ) && in_array( $post_type, $candidate['postTypes'], true ) ) +( isset( $candidate['postTypes'] ) && in_array( $post_type, $candidate['postTypes'], true ) )
Walker::display_element(), class-wp-walker.php:167 — $newlevel is a local variable assigned the literal true twelve lines above and never assigned anything else, so the truthiness test adds nothing to the isset().
-if ( isset( $newlevel ) && $newlevel ) { +if ( isset( $newlevel ) ) {
All three are simplifications with no change in behaviour. booleanAnd.rightAlwaysTrue goes from 16 to 14 and booleanAnd.leftAlwaysTrue from 5 to 4. The baselines are generated, not hand-edited; neither reaches zero, so no baseline is deleted and phpstan.neon.dist is unchanged.
### One occurrence deliberately left baselined
functions.php:6784 is the same shape — 0 => ( isset( $zone[0] ) && $zone[0] ) — and PHPStan is right that the second operand is always true, because the enclosing loop continues unless $zone[0] matched an entry in $continents. It is left alone here because it is one of three parallel lines building an $exists map, and dropping the operand from only the first breaks that symmetry to remove a tautology. That seemed a change worth discussing separately rather than folding into this one.
### Testing instructions
npm run typecheck:phpontrunkreports[OK] No errors, because the occurrences are baselined.- With this branch applied,
npm run typecheck:phpreports[OK] No errorsacross 1289 files with both baselines regenerated. - To confirm the change is surgical, run the analysis with all baselines suppressed before and after and diff the results on file plus message: exactly three errors disappear and none appears, anywhere, for any identifier.
npm run test:php— 30871 tests, 4558975 assertions, 86 warnings, 44 skipped, and one failure:Tests_Script_Modules_WpScriptModules::test_default_script_module_files_exist, looking forsrc/wp-includes/js/dist/script-modules/a11y/index.js. That path is gitignored build output that is unbuilt in my checkout, and the test fails identically on unmodifiedtrunk. The counts are identical to a run without these changes.composer lintreports no errors for the three modified files.class-wp-walker.phpcarries one pre-existing filename-convention warning that is also present ontrunk.
Unlike the other PRs on this ticket, this one changes executable code rather than docblocks. Walker::display_element() in particular runs for every nav menu, category list and comment tree, which is why the full suite result above is worth checking rather than taking on the argument alone.
### Related
These are independent of one another and can land in any order, but several regenerate overlapping baseline files, so whichever lands after the first will need composer phpstan:baselines re-run against the updated trunk:
- https://github.com/WordPress/wordpress-develop/pull/13071 —
booleanAnd.rightAlwaysTrue.neon, shared with this PR - https://github.com/WordPress/wordpress-develop/pull/13074 and https://github.com/WordPress/wordpress-develop/pull/13077 —
booleanAnd.leftAlwaysTrue.neon, shared with this PR - https://github.com/WordPress/wordpress-develop/pull/13069 —
deadCode.unreachable.neonandif.alwaysTrue.neon, shared with 13074
## Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Opus 5
Used for: grouping the remaining boolean PHPStan baselines by root cause, identifying this cluster, measuring the before/after error diff across the whole codebase, regenerating the baselines, and drafting this description. The change itself and the verification runs were reviewed and confirmed by me in a local development environment.
@dmsnell commented on PR #13081:
3 weeks ago
#47
This ticket was mentioned in PR #13216 on WordPress/wordpress-develop by @swissspidy.
3 weeks ago
#49
Follow-up to [62648], which added conditional return types keyed on the fields argument to get_posts() and WP_Query::query().
'id=>parent' was missing from both conditions. Both only recognise 'ids', but 'id=>parent' also returns an array of integers — parent post IDs keyed by post ID, per the switch in WP_Query::get_posts() and the $post_parents return a few hundred lines below it. Today get_posts( [ 'fields' => 'id=>parent' ] ) is reported as WP_Post[], which is wrong in a way that is worse than no annotation at all.
WP_Term_Query::query() can be annotated the same way. Like WP_Query::query(), it takes the query arguments directly, so the mapping already documented in prose on WP_Term_Query::get_terms() can be expressed as a type:
'count'returns0|numeric-string.'ids','tt_ids'and'id=>parent'return arrays of integers.'names','slugs','id=>name'and'id=>slug'return arrays of strings.- Everything else returns an array of
WP_Termobjects.
The integer in the count branch is not a typo. WP_Term_Query::get_terms() bails out early with an integer rather than the numeric string the docblock describes when child_of or parent names a term with no descendants:
if ( ! $in_hierarchy ) { if ( 'count' === $args['fields'] ) { return 0; }
so get_terms( [ 'fields' => 'count', 'parent' => $id ] ) really can return 0. Every other count path returns $wpdb->get_var()'s numeric string. Annotating numeric-string alone would have been a promise core does not keep. (The @return line on both methods says only string for this case, and the $fields entry in WP_Term_Query::__construct() says int — that inconsistency predates this patch and is left alone here.)
WP_Query::get_posts() and WP_Term_Query::get_terms() read the query vars off the instance rather than taking them as an argument, so there is nothing to narrow their return types on and they are unchanged.
### Analysis impact
Verified with PHPStan 2.2 against extracted copies of the three annotated docblocks:
- All three docblocks parse; no
phpDoc.parseError. - Each branch resolves as intended, and unsealed shapes (
{ fields: 'ids', ... }) still match when other query vars are present. get_terms()intaxonomy.php— the only core caller ofWP_Term_Query::query()— produces no new errors before or after, including the widened count branch. PHPStan already normalises the existing@return WP_Term[]|int[]|string[]|stringinto a single array type unioned withstring, so nothing widens. (Confirmed thereturn.typerule was actually live in that harness by deliberately breaking it.)- Neither core call site of
'id=>parent'(wp_edit_posts_query()viawp(), and_get_term_hierarchy()viaget_terms()) goes through the changed conditions, so no baseline entries are affected.
The full core PHPStan run has not been executed against this branch.
### Why this matters downstream
php-stubs/wordpress-stubs carries its own functionMap.php entry for get_posts() that covers both 'ids' and 'id=>parent'. Once the stubs bump past 7.0.1, that entry becomes a duplicate of the core annotation and will be removed as obsolete — exactly what php-stubs/wordpress-stubs#475 did for WP_Theme::get(). Without this patch, that removal would silently regress 'id=>parent' to WP_Post[]. The matching stubs change is at swissspidy/wordpress-stubs#1.
Trac ticket: https://core.trac.wordpress.org/ticket/65817
## Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code
Used for: Locating the gap and writing the three docblock annotations
This ticket was mentioned in PR #13220 on WordPress/wordpress-develop by @swissspidy.
3 weeks ago
#50
Documents the types of several values that are currently only described as array, stdClass or bool[], so their contents are visible both to anyone reading the docs and to static analysis. Docblock changes only, no behavior change.
Three of these look like genuine documentation bugs rather than gaps:
get_taxonomy_labels()omits two labels its object always carries._get_custom_object_labels()always setsname_admin_bar, and both label builders addmenu_nameto the defaults before merging. Core itself reads$taxonomy->labels->menu_nameinwp-admin/menu.php.get_post_type_labels()documentsmenu_name, but notname_admin_bar, and nottemplate_nameeither, which its own@since 6.6.0line mentions.- Labels that can be
nullare documented asstring.WP_Post_Type::get_default_labels()andWP_Taxonomy::get_default_labels()are both typed(string|null)[][], and for eight taxonomy labels and one post type label the default really isnullfor one of the two hierarchies:popular_itemson a hierarchical taxonomy,parent_item_colonon a non-hierarchical post type, and so on. Those becomestring|null. get_registered_settings()omits thegroupkey.register_setting()always stores it through its defaults. The same docblock, andregister_setting()itself, describesanitize_callbackascallablewhen its default isnull.
Alongside those:
get_post_type_labels()describes its return value with the same hash notationget_taxonomy_labels()already uses, instead of a prose list, so the two read alike.WP_Taxonomy::$caplists the four capabilities it holds, mirroring thecapabilitiesargument ofregister_taxonomy().WP_Post_Type::$capgains a@see get_post_type_capabilities(), where its own list already lives.WP_User::$caps,WP_User::$allcapsandWP_Role::$capabilitiesare keyed by capability name, soarray<string, bool>rather thanbool[].
Verified with composer phpstan, at the configured level 5 with the existing baselines and reportUnmatchedIgnoredErrors: true: no new errors, and no baseline drift.
Trac ticket: https://core.trac.wordpress.org/ticket/65817
## Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code
Used for: comparing the existing docblocks against the code that builds these objects, and drafting the documentation changes in this pull request.
---
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_0122HqysjDiq2jSHebbSSu6A
---
_Generated by Claude Code_
@westonruter commented on PR #13220:
3 weeks ago
#51
Correcting the WP dev docs is good, but what would be greater is adding the PHPStan types. Or, I wonder if we could add an extension that would teach PHPStan how to interpret the WordPress flavor of phpdoc.
@swissspidy commented on PR #13220:
3 weeks ago
#52
Good point. To a degree it's possible. Normally the WP flavor is transformed into PHPStan types in the wordpress-stubs project, that's how I use it. But adding PHPStan types directly would help both internally and externally. And an extension could be reusable too and help reduce docs duplication.
This ticket was mentioned in PR #13233 on WordPress/wordpress-develop by @swissspidy.
3 weeks ago
#53
Follows up on the suggestion in #13220: rather than writing PHPStan types beside the hashes that already describe the same shapes, teach PHPStan to read the hashes.
tests/phpstan/HashNotationVisitor.php is a parser node visitor, in the same shape as the GlobalDocBlockVisitor already in that directory. It reads hash notation from @param and @return tags and appends the equivalent array shape:
/** * @param array $args { * Optional. An array of arguments. * * @type string $post_type Post type. Default 'post'. * @type int $post_author Post author ID. * } */
becomes, to PHPStan:
@phpstan-param array{post_type?: string, post_author?: int, ...} $args
Nothing is written to disk; the docblock is rewritten in the AST, so the source keeps only the hash. Across src/wp-admin, src/wp-includes and the bundled themes it derives 394 tags in 140 files, from the 465 hashes it can see that do not already carry a hand-written one.
The translation is the one php-stubs/wordpress-stubs performs when generating stubs, which is how the WordPress flavor of PHPDoc reaches PHPStan today for plugins and themes. Doing it here means core's own analysis gets the same types, and that a shape has one source rather than two that can drift apart.
## What it does not translate
A hash whose translation would be a guess is left alone, so the visitor only ever narrows a type and never contradicts one:
- A hand-written
@phpstan-paramor@phpstan-returnalways wins. Hash notation cannot express everything a type can — a function returning one shape or another, for instance — so a shape tuned in the source is never overwritten. 35 hashes are covered this way today. - The declared type must name a bare
array, alone or as one member of a union likestring|array. A type already more specific than the hash, such asarray<string, string|bool>, is left as written. - The hash must be well formed: every
{closed by a}on its own line, every@typecarrying a type and a$name. - By-reference parameters are skipped. PHPStan checks those in both directions, so a shape there is a contract every caller's variable has to satisfy before the call, which is not what the hash says.
Keys of a @param hash are optional at every level, and the shape is left open with a trailing ..., because a hash lists the keys core reads rather than the only keys a caller may pass. Keys of a @return hash are required and its shape is sealed, since they describe a value core builds — unless the description marks one Optional., which the visitor honors, as WP_Http::request() already writes for $filename.
Two kinds of hash are left for a follow-up, both noted in the README:
@varhashes on properties. A property declaration is inherited by every subclass and has to accept its own default, so a shape there says more than the hash does.objecthashes, such as the one onget_taxonomy_labels()that #13220 documents. PHPStan's object shapes are structural, so a shape derived for a value core builds as astdClassis no longer assignable to a property declaredstdClass. Covering these wants the docblocks to name the class rather thanobject, which is a docs change worth making on its own.
Hashes on hook docblocks — 170 of them — are not attached to a function, so they are outside what a node visitor sees. The value a filter passes stays typed by the existing hook extensions.
## What it found
Turning it on made the analysis check these hashes against the code for the first time. The second commit fixes what it reported:
WP_Http::processHeaders() | documents a newheaders key; the array it returns has headers
|
wp_edit_attachments_query() | documents post_mime_types and avail_post_mime_types keys; it returns the two values positionally
|
WP_List_Table::get_views_links() | documents url, label, current as keys of $link_data; they are keys of each link in it, which its own @return line says
|
wp_check_php_version() | always sets is_lower_than_future_minimum, and both callers read it, but it is not documented
|
wpdb::parse_db_host() | documents the port as string\||null, right below the absint() cast and the comment "Port cannot be a string; must be null or an integer"
|
wp_xmlrpc_server::wp_editPage() | documents its content argument as a string; it is the content struct the method writes post_type into
|
WP_Http::request() | documents headers as a CaseInsensitiveDictionary; a non-blocking request returns an empty array
|
wp_upload_bits() | documents file, url and type alongside error; only error is set when the upload fails
|
wp_font_dir() and get_avatar_data() return one shape or another rather than one shape with optional keys, which a hash cannot say, so each gains a @phpstan-return beside its hash the way wp_upload_dir() and _wp_handle_upload() already do.
What remains is 22 call sites passing an argument the documented shape does not accept, recorded in argument.type baseline rather than resolved here — each is a hash and a caller disagreeing about a key, and wants its own look. Four existing baseline entries change because the types in their messages got more precise.
## Testing
composer phpstan is green on the branch, from a cleared result cache. Worth knowing while reviewing: the result cache does not fully invalidate on a change to a registered visitor, so a run made without clearing .cache first will under-report.
This may want its own ticket rather than sharing 65817, which is about the docblocks themselves — happy to move it.
## Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code
Used for: writing the visitor and this description, measuring its effect on the analysis, and drafting the documentation corrections it surfaced.
---
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 by Claude Code_
This ticket was mentioned in PR #13235 on WordPress/wordpress-develop by @swissspidy.
3 weeks ago
#54
Several hashes describe a narrower type than the code on either side of them uses. Where the documented type is the one that is wrong, this widens it; where the caller is, it corrects the caller to what the hash already says.
These surfaced while teaching PHPStan to read hash notation in #13233, but each is a disagreement between a docblock and the code that stands on its own, so they are split out here rather than carried in that pull request's baselines.
## Documentation
WP_Ajax_Response::add() | documents $id as int\||WP_Error and $position as string. Every caller in wp-admin passes a numeric string for the first and an int for the second — which is what the $position description already says: "Accepts 1 (bottom), -1 (top)"
|
get_bookmarks() | documents $category as a comma-separated list of IDs. The links list table passes a single ID as an int, which wp_parse_id_list() accepts
|
wp_list_pages() | documents $title_li as string, in a description reading "Passing a null or empty value will result in no heading". Twenty Twenty passes false to get exactly that
|
wp_nav_menu() | documents $container as string, in a description beginning "Whether to wrap the ul". Twenty Twenty-One passes false not to
|
WP_Customize_Setting::$default | is documented string and holds whatever the setting's default is
|
$default is the interesting one: two subclasses already redeclare it as array, and one assignment of a stdClass exists. All three were reported and baselined — typing the property mixed removes those three entries. This pull request is a net reduction in the baselines.
## Call sites
WP_MS_Themes_List_Table | passes 'inline' for wp_admin_notice()'s $additional_classes, which takes a list of classes
|
| Twenty Nineteen | passes null for comment_form()'s $title_reply, where an empty string suppresses the heading just as well
|
| Twenty Twenty | passes '' for wp_nav_menu()'s $fallback_cb, which takes callable\||false
|
WP_Customize_Manager | passes 0 for get_pages()'s $hierarchical, which takes a bool
|
Each caller change is behaviourally identical to what it replaces — the values are already falsy or already accepted — so this is about saying the same thing in the documented type.
## What is not here
The same exercise surfaced call sites that no docblock change reaches, because the array reaching them has no statically known keys — five wp_insert_post() callers, get_pages() in post-template.php, register_sidebar() in widgets.php. One more, WP_Customize_Manager::get_changeset_posts(), wants get_post_stati() to say which of string[] or stdClass[] it returns for a given $output, which is a separate change. Those are left alone.
## Testing
composer phpstan is green, from a cleared result cache, and composer lint reports nothing new on the changed files. The docblock realignments are column-only where a widened type changed the width.
Shares a ticket with #13233 for now; happy to move it to its own.
## Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code
Used for: finding these disagreements by running the analysis with #13233's extension enabled, checking each against the code, and drafting the corrections and this description.
---
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 by Claude Code_
@dmsnell commented on PR #13080:
2 weeks ago
#56
This ticket was mentioned in PR #13283 on WordPress/wordpress-develop by nathanrice.
2 weeks ago
#57
## Summary
Resolves 5 method.notFound PHPStan baseline errors by adding property type annotations in installer/upgrader skin classes. Each skin now declares $upgrader with its specific upgrader type, allowing PHPStan to validate method calls.
## Changes
Language_Pack_Upgrader_Skin: Annotate$upgraderasLanguage_Pack_UpgraderPlugin_Upgrader_Skin: Annotate$upgraderasPlugin_UpgraderPlugin_Installer_Skin: Annotate$upgraderasPlugin_UpgraderTheme_Upgrader_Skin: Annotate$upgraderasTheme_UpgraderTheme_Installer_Skin: Annotate$upgraderasTheme_Upgrader- Removes
method.notFound.neonbaseline file - Removes unmatched entries from
property.notFound.neonbaseline file
## Related
Trac ticket: https://core.trac.wordpress.org/ticket/65817
## Test Plan
- Run
composer run phpstan - Confirm: No errors
- All 5 affected files pass PHPStan Level 5 validation
## Use of AI Tools
AI assistance: Yes
Tool(s): Google Gemini, Claude Code
Model(s): Gemini 3.6 (Thinking), Haiku 4.5 (medium)
Used for: Understanding the problem, identifying candidates for fixing, validating changes
This ticket was mentioned in PR #13285 on WordPress/wordpress-develop by @nathanrice.
2 weeks ago
#58
## Summary
Deletes the greater.invalid.neon baseline file that was emptied in PR #13080 but not removed from the filesystem. The file remained in the directory, causing the PHPStan baseline regeneration script to re-add it to the includes list in phpstan.neon.dist.
## Related
Follow-up to #13080 and r63344
Trac Ticket: https://core.trac.wordpress.org/ticket/65817
## Test plan
Before applying this patch, running composer phpstan:baselines sees the empty file and adds it to phpstan.neon.dist. Assuming future PRs follow the instructions, this will result in the accidental reversion of part of r63344.
With this PR, running composer phpstan:baselines should result in no change to the phpstan.neon.dist file.
## Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code
Model(s): Haiku 4.5
Used for: Chasing down PR links, revision numbers, etc.
@swissspidy commented on PR #13216:
13 days ago
#65
@westonruter commented on PR #13235:
13 days ago
#67
documents
$categoryas a comma-separated list of IDs. The links list table passes a single ID as an int, whichwp_parse_id_list()accepts
This isn't strictly correct. wp_parse_id_list() accepts mixed[]|string. However, wp_parse_list() will coerce an int into a string when it passes the value into preg_split(). I'll update the function to explicitly allow passing int (and float) since these do work and should be documented with proper casting.
This ticket was mentioned in PR #13297 on WordPress/wordpress-develop by @westonruter.
13 days ago
#68
Formalizes int as an accepted input type for wp_parse_list(), wp_parse_id_list(), and wp_parse_slug_list(), which previously documented mixed[]|string.
## Background
Passing an integer has always worked, but only by accident of type coercion. wp_parse_id_list() has called preg_split() on non-array input since it was introduced in 3.0.0, and wp_parse_list() inherited that when the three functions were consolidated in 5.1.0. Because preg_split()'s $subject parameter is a string, PHP coerces an int on the way in — wp_parse_id_list( 5 ) returns array( 5 ) and always has.
Core relies on this in several places:
WP_Tax_Querydocuments@type string|int|array $termsand passes that value straight towp_parse_id_list()inWP_Tax_Query::transform_query().get_bookmarks()reassigns$parsed_args['category']to aWP_Term::$term_id(anint) when resolvingcategory_name, then passes it towp_parse_id_list().wp_list_bookmarks()callsget_bookmarks()with'category' => $cat->term_id.
So the documented type was narrower than both the implementation and the callers. At PHPStan rule level 10 this surfaces as an argument.type error at each of those call sites, for input the functions handle correctly.
## Changes
The @param types become mixed[]|string|int, and wp_parse_list() handles the int case explicitly rather than leaning on implicit coercion of a non-string into preg_split():
if ( is_int( $input_list ) ) { $input_list = array( (string) $input_list ); } elseif ( ! is_array( $input_list ) ) { // ... }
The @phpstan-return conditional is updated so int resolves to list<string> alongside string, rather than falling through to the array branch.
There is no behavior change: the new branch produces exactly what preg_split() produced for every int, including 0, negative values, PHP_INT_MAX, and PHP_INT_MIN.
## Why not float and bool
Both are also silently coerced today, and both are deliberately left out of the documented contract.
float produces nonsense for anything ID-shaped: 1.0e25 stringifies to "1.0E+25" and absint() saturates it to PHP_INT_MAX, while 0.5 becomes 0. bool is worse — wp_parse_id_list( true ) returns array( 1 ), silently converting a flag into a query for object ID 1.
Runtime behavior for both is untouched; they are simply not promised. Since the previously declared mixed[]|string is *narrower* than the new mixed[]|string|int, this is a strict widening and no existing caller can newly fail static analysis.
## Testing
The data providers for these three functions had no non-string scalar input at all. Added coverage for a positive int, a negative int, and zero in each, plus a case pinning the contrast between a comma inside an array item and the same value passed as a string — array items are never split, so wp_parse_id_list( array( '1,2' ) ) yields array( 1 ) while wp_parse_id_list( '1,2' ) yields array( 1, 2 ).
Tests_Functions_wpParse* goes from 46 to 57 tests, all passing.
Trac ticket: Core-65817
## Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Opus 5
Used for: Reviewing the initial change, verifying the inferred PHPStan types and the runtime equivalence of the is_int() branch, and drafting the added test cases and docblock wording. The implementation was authored by me, and I have reviewed and take responsibility for all of it.
@westonruter commented on PR #13235:
13 days ago
#69
I'll update the function to explicitly allow passing
int(andfloat) since these do work and should be documented with proper casting.
See https://github.com/WordPress/wordpress-develop/pull/13297
@westonruter commented on PR #13082:
12 days ago
#74
this PR is probably too big to merge as a single change
I'm preparing to make multiple commits for this.
@westonruter commented on PR #13082:
12 days ago
#76
OK, I committed the adding of the missing return null in r63378 (5bf63817467645ae67c83c692a1b99854e89d2b1).
Proceeding to merge trunk into this branch for the remaining changes.
This ticket was mentioned in PR #13307 on WordPress/wordpress-develop by @swissspidy.
12 days ago
#81
Trac ticket: https://core.trac.wordpress.org/ticket/65817
## Use of AI Tools
@swissspidy commented on PR #13235:
9 days ago
#83
@swissspidy commented on PR #13220:
9 days ago
#85
@szepe.viktor commented on PR #13233:
9 days ago
#86
@swissspidy What makes you cling to PHP 4 heritage?
Do you think WordPress cannot let @type go?
@westonruter commented on PR #13233:
9 days ago
#87
@swissspidy What makes you cling to PHP 4 heritage? Do you think WordPress cannot let
@typego?
@szepeviktor IMO, we should transition to using PHPStan type syntax. However, this would be a LOT of churn. Also, the dev docs aren't currently equipped to parse array shapes. They need @type. So this is a good transitionary step.
@westonruter commented on PR #13079:
9 days ago
#88
Closing in favor of https://github.com/WordPress/wordpress-develop/pull/13342
This ticket was mentioned in PR #12944 on WordPress/wordpress-develop by @josephscott.
9 days ago
#89
With a long list of tests
Trac ticket: Core-65817
Follow-up to Core-65826
This will cause return values to change compared to the old absint() - https://3v4l.org/MssWC - because the old one wasn't actually limited to returning only an int
AI assistance: Yes
Tool(s): Claude
Model(s): Fable 5
Used for: Discussion on potential approaches on how to fix absint() to only return an int in a more simplified way. Claude also wrote all of the new tests.
@marian1 commented on PR #13082:
8 days ago
#90
@dpanta94 I think the resolved void unions in the functions using a $display parameter were less a type-system problem than a conceptual/API-design issue. These functions combine two responsibilities: depending on the argument, they either output a result or return one. That dual behaviour is what was causing the void unions in the first place.
Replacing void with null avoids the union, but it removed information that PHPStan otherwise could have used. Although the runtime value is still effectively null, PHPStan can no longer detect the logical error of consuming a meaningless value. And errors like Result of function function_name (void) is used. will no longer be raised, because null is now treated as a legitimate return value. See this example: https://phpstan.org/r/2c79437a-f917-4cce-8d28-1eca46bd2fa6
I understand and support the effort to get rid of the void unions but I am not sure that replacing void with null in this context is an improvement from a static-analysis perspective.
cc @westonruter
@swissspidy commented on PR #13307:
8 days ago
#92
@swissspidy commented on PR #13233:
8 days ago
#94
This ticket was mentioned in PR #13359 on WordPress/wordpress-develop by @westonruter.
7 days ago
#96
Follow-up to r63379, addressing review feedback from @IanDelMar on #13082.
The point raised there was that the void unions on the functions taking a $display parameter were less a type-system problem than a consequence of those functions having two responsibilities: depending on the argument they either print a result or return one. Replacing void with null removed the union, but it also removed information: Under string|null PHPStan treats the display-mode result as a legitimate value, so consuming a meaningless one is no longer reported.
That is correct, and it turns out to understate the problem.
## void in a plain union conveys nothing
PHPStan raises Result of function … (void) is used. only when the resolved return type is *exactly* void. A plain union never resolves to that, so @return string|void and @return void|string carry no more information than string|null does. Verified with two otherwise identical functions:
/** @return void|string */ function plain_union( string $prefix = '', bool $display = true ) { … } /** * @return void|string * * @phpstan-return ( $display is true ? void : string ) */ function with_conditional( string $prefix = '', bool $display = true ) { … } var_dump( plain_union( 'x' ) ); // no error var_dump( with_conditional( 'x' ) ); // Result of function with_conditional (void) is used. [function.void]
Only the conditional annotation does any work. So this is not a matter of putting back what r61768 and r63379 took away. A dozen or so functions that still carry void today were never getting anything from it either.
## Changes
Three commits, each independently reviewable.
1. The nine functions from r63379. wp_title(), single_post_title(), post_type_archive_title(), single_term_title(), the_date(), the_modified_date(), edit_term_link(), next_posts() and previous_posts() go back to string|void and gain a conditional @phpstan-return. The trailing return null; statements added by r63379 are removed, since void in the union licenses falling off the end.
Four of these bail before reaching the display branch, so their retrieval-mode type is string|null rather than string, and those bails go back to the bare return; they had before r61768. The conditional return type is what now distinguishes them: nothing when printing, null when retrieving. single_cat_title() and single_tag_title() delegate to single_term_title() and take the same annotation, which requires expanding their one-line body into an early return — returning the delegate's value unconditionally never returns void, and PHPStan reports the void as unused.
2. Five more with the same shape, found by sweeping core for the pattern: comment_class(), the_title(), wp_loginout(), wp_register() and wp_update_php_annotation(). The first four already documented void|string and so, per the above, were getting nothing for it. wp_update_php_annotation() needs a small body change: its trailing return null; is reached in both modes, so it moves to an early bare return; on the missing-annotation path. No behavior change.
3. An unrelated docs bug found by the same sweep. WP_Styles::print_inline_style(), WP_Scripts::print_extra_script() and the deprecated WP_Scripts::print_scripts_l10n() document their return inverted: each says the markup comes back when $display is true, but the string is returned on the ! $display branch and the printing branch returns true. Corrected, and given conditional annotations as well — not for void detection, but for narrowing:
| Call | Before | After |
|---|---|---|
print_inline_style( $h ) | string\||bool | bool
|
print_inline_style( $h, false ) | string\||bool | string\||false
|
print_extra_script( $h ) | bool\||string\||null | true\||null
|
print_extra_script( $h, false ) | bool\||string\||null | string\||null
|
This matters at the two internal call sites that pass false and then use the result as a string, where true was previously considered possible. Happy to split this commit off into its own ticket if preferred.
## Deliberately not changed
Functions that print and then return the value unconditionally, where the result is always meaningful: wp_nonce_field(), wp_referer_field(), wp_original_referer_field(), checked() / selected() / disabled() / wp_readonly() / readonly() and __checked_selected_helper(), menu_page_url(), _post_states(), _media_states(), timer_stop(), wp_popular_terms_checklist(), wp_nav_menu_disabled_check(). Also WP_Scripts::print_inline_script() and print_translations(), whose existing string|false is accurate in either mode.
Functions returning something meaningful while printing: single_month_title() returns false on failure before the display branch, so it cannot resolve to plain void.
The tags taking echo inside an $args array — wp_list_categories(), wp_list_pages(), wp_page_menu(), wp_nav_menu(), wp_list_comments(), wp_list_bookmarks(), wp_list_authors(), wp_login_form(), wp_get_archives(), the_title_attribute(), wp_tag_cloud(), paginate_comments_links(), wp_dropdown_languages(), wp_list_users(). $args accepts a query string as well as an array, so an array{echo: false} condition would resolve to the void branch for the still-common wp_list_categories( 'echo=0&title_li=' ) call style and report correct code as an error. Several also default echo to 1 rather than true, so the shape would need widening too. Expressible with nested conditionals, but not worth the false-positive risk here.
Deprecated functions: the_category_ID(), get_author_link(), get_category_rss_link(), get_author_rss_link(), get_most_active_blogs(), wp_get_links().
## Verification
- Full PHPStan runs at level 10 before and after report an identical error set, with one exception: the pre-existing
return.typereport onprint_extra_script(), which stems fromWP_Dependencies::get_data()returning mixed, restates its narrower expected type. - A temporary probe file confirmed all sixteen functions raise
function.voidwhen consumed in display mode, and none of them do in retrieval mode. tests/phpstan/baselines/return.missing.neonstays deleted —voidin the union is precisely what licenses falling off the end.- PHPCS reports no new issues.
- PHPUnit:
Tests_General_,Tests_Link_,Tests_Date_,Tests_Post_,Tests_Comment_,Tests_Functions_andTests_Dependencies_all pass.
## Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code
Model(s): Opus 5
Used for: Evaluating the review feedback, sweeping core for other instances of the pattern, and drafting the annotations and this description. The PHPStan and PHPUnit verification was run against the working tree, and the result has been reviewed and is owned by me.
@westonruter commented on PR #13082:
7 days ago
#97
@IanDelMar Thank you for following up on this. I've opened https://github.com/WordPress/wordpress-develop/pull/13359 to apply this feedback.
@westonruter commented on PR #13359:
7 days ago
#98
@IanDelMar @apermo I'd appreciate your review of this.
@westonruter commented on PR #13359:
7 days ago
#99
🤖 _Comment by Claude Opus 5_
Replying to the four suppressed notes in @Copilot's review, since none of them carry a thread to answer in.
### The next_posts() note was correct and is fixed
Addressed in 179a78c9b3, and extended to two more places with the same defect. next_posts() maps a missing link to an empty string, so on the last page a caller gets '' rather than the "link URL" the description promised. previous_posts() has the identical shape, and wp_register() promises a registration or admin link while setting the value to an empty string when registration is disabled and the visitor is logged out, or when a logged-in user cannot reach the dashboard. The types were already right, an empty string being a string, so only the descriptions changed.
### The other three stand, deliberately
Those notes all make one point: the shared bails say return null; even in printing mode, while the condition models printing mode as void, and they suggest splitting each bail so that null is returned only when retrieving.
That shape was considered and rejected in favor of the current one. Three things informed the decision.
Both spellings behave identically. PHPStan does not cross-check a conditional return type's branches against the function's actual return statements; this was verified directly, with a bail returning null on a path reachable while printing and a branch declaring void. Printing-mode calls still resolve to plain void and are still reported, and retrieval-mode calls still resolve to string|null. Nothing is gained or lost in analysis either way, so this is a question of documentation philosophy rather than of capability.
The two readings of void are both defensible. The note reads void literally, as "executes no value-returning return". The reading used here is PHPStan's own, where void means "no meaningful value" and resolves to null wherever a value is required. A failure bail reached while printing is exactly that: the markup was not produced, and the caller is not looking at the return value. Writing return null; there is not a claim that null is meaningful; it is the same statement the retrieval path needs, and the condition is what distinguishes the two modes.
The cost is real and the benefit is not. Twenty bails across twelve functions would each grow a nested branch, adding roughly sixty lines to hot template tags purely to satisfy an annotation that already resolves correctly. Diverging from the uniform shape is also what produced the inconsistency raised in the earlier review, where some branches read string|null and others string|void; that was settled by making every condition read the same way, and re-introducing a per-bail distinction would work against it.
A clarification on wp_update_php_annotation(), which may look self-contradictory: this pull request removed a return null; from it and then added one back. Those are different statements. The one removed sat at the end of the function, reachable only after the markup had been echoed, and asserted a value where the caller is not looking. The one now present sits before the print-or-return split and is reached in both modes, so when retrieving, that null genuinely is the returned value. The same distinction governs which bails were converted elsewhere: those reachable while retrieving say return null;, and the paths inside if ( $args['display'] ) in get_calendar(), which run only after the markup is echoed, keep a bare return;.
@apermo commented on PR #13359:
7 days ago
#100
I am checking all core for similar errors in "display or return" booleans. If there is a significant number i'll create a new ticket and a new pr for that.
@apermo commented on PR #13359:
7 days ago
#101
The sweep is done, and it came back clean.
Scope was every function and method in src that takes a boolean display-or-return flag: $display, $echo, $force_echo, $deprecated_echo, $display_message, plus the ones taking it as $args['echo'] or $args['display']. Around 70 symbols in wp-includes, wp-admin and the bundled themes. For each one the @return description was compared against the branch in the body that actually splits printing from returning.
The three you found in WP_Styles and WP_Scripts are the only inversions in core. Nothing else has the two branches swapped.
Two more in that group looked wrong at first, but both are already handled. wp_dropdown_languages() and twentytwenty_generate_css() both had a @return string with a bare return; on their bail, and dmsnell and nomadmystic fixed both in r63314. Your PR then sharpened the wp_dropdown_languages() description on top of that. I only saw them because I first ran the sweep against a checkout that was a few weeks behind trunk. My bad.
PHPStan agrees: on your branch with the repo config there is no return.empty and no return.missing left in src.
So from my side there is nothing else to fix here.
Disclosure: I used Claude Code (Opus 5) for the sweep and for drafting this comment. I reviewed the findings and ran the PHPStan verification in my own checkout.
@marian1 commented on PR #13359:
7 days ago
#104
I was only able to have a quick look.
- >
@return string|voidand@return void|stringcarry no more information thanstring|null
I don't think that is correct. For now, PHPStan seems to treat these as equivalent, but PHPStan evolves and that behaviour may change in the future. For users,
voidstill conveys semantic information: if the function returns the valuenull, that value has no meaning. This distinction may be irrelevant to some users, but relevant to others. But there may be other opinions on that.
wp_tag_cloud(): the$args is ''|arraybranch could be extended to include'0', although this is very unlikely to have much practical impact. Looking at the conditional return types, the same seems to apply towp_list_comments(),wp_list_bookmarks(),wp_list_authors(),paginate_comments_links(),the_title_attribute(),wp_list_pages(),wp_page_menu(), andwp_list_users().wpdb::print_error()also returnsnullat the bottom of the method. I don't think this can be changed back tovoidsimply by adding a conditional return type. It might still be worth documenting it here alongside the other related functions. Althoughvoidcannot be part of a native PHP union type, was there actually an issue with using it in the PHPDoc union?print_scripts_l10n()has long been deprecated. Why annotate it with PHPStan tags rather than ignoring any error raised for that function?wp_get_archives(): shouldn't the@return string|voidtag also includenull, given the semantic distinction betweenvoidandnull?the_date(),the_modified_date(): the return description says "String if retrieving.". This wording is inconsistent with the descriptions used for the other functions. This also applies toedit_term_link().wp_dropdown_languages(): the description is incorrect. It says that "nothing is returned when the requiredidornameargument is missing." If those arguments are missing, they are populated with the default'locale'. The function actually bails when$args['id']or$args['name']is falsy. This could be represented as something along the lines of:($args is array{id: null|0|''|'0', ...}|array{name: null|0|''|'0', ...} ? void : string)- Unrelated, but I just noticed this:
@param true $deprecated_echoontrackback_url()would result in:Parameter #1 $deprecated_echo of function trackback_url expects true, false given.. In php-stubs/wordpress-stubs, this is used deliberately to signal that a deprecated argument was supplied. See: https://phpstan.org/r/2e2aa144-9b64-4815-bf72-2c343512462d - General thought: I think the effort being put into improving code quality is very welcome.
However, I also think that some of the errors reported by PHPStan point to design questions and should prompt us to think about those questions, rather than trying to introduce workarounds solely to silence PHPStan. Once such workarounds land in core, the PHPStan error disappears and therefore no longer indicates that there may be an underlying problem. The issue has not necessarily been resolved; it may simply have been masked.
I don’t think there is anything wrong with deliberately ignoring some errors. That way, there is still an indication that something may deserve attention, without forcing the implementation or documentation into shapes that primarily exist to satisfy the analyser.
@westonruter commented on PR #13079:
7 days ago
#106
See r63442 (08f158918e6f2861f29d11a7a84ea5e230c4eb85)
This ticket was mentioned in PR #13371 on WordPress/wordpress-develop by @westonruter.
6 days ago
#107
Follow-up to r63440 and r63441, addressing review feedback from @IanDelMar on #13359.
Five commits, each independently reviewable. Four act on points raised in that review; one corrects a claim made in the previous pull request's own description.
## 1. Every falsy $args scalar belongs in the undecidable branch
The tags whose display flag lives in an $args array also accept a query string, which PHPStan cannot read, so their conditional return types end in a third branch that leaves the type a union and reports nothing. That branch tested $args is ''|array, treating the empty string as the only scalar behaving like an empty argument set.
'0' behaves identically. parse_str( '0', $r ) yields array( 0 => '' ), so wp_parse_args( '0', $defaults ) sets no key the tag reads and the flag keeps its default — the call prints and returns nothing, exactly as '' does. It was resolving to the retrieval type instead, so consuming its meaningless result went unreported.
The branch is now ''|'0'|array, matching how the flag conditions themselves already spell the falsy set as false|0|''|'0'. Ten tags are affected: wp_list_authors(), wp_list_bookmarks(), wp_tag_cloud(), wp_list_comments(), paginate_comments_links(), wp_get_archives(), the_title_attribute(), wp_list_pages(), wp_page_menu() and wp_list_users() — the nine raised in review plus wp_get_archives(), which has the same branch and the same gap.
Every other call shape is unchanged. A query string carrying a visible flag and an $args built at runtime both stay unions and are still reported in neither mode; an array setting the flag still resolves to the retrieval type; an empty array or a truthy flag still resolves to void.
## 2. wp_dropdown_languages() bails on a falsy argument, not a missing one
The return description said nothing comes back "when the required id or name argument is missing". Neither is required and neither can go missing: both default to 'locale', so a caller omitting them gets a dropdown. The function bails on a falsy value the caller supplied.
The void in the union covers exactly that bail, and until now did nothing, since a plain union never resolves to void. A conditional makes it report where the bail is visible. '' and '0' are the falsy strings the arguments are documented to hold, mirroring how the sibling tags spell the falsy set of a bool|int flag.
The two shapes have to nest rather than combine. PHPStan cannot represent a union of two unsealed array shapes with different required keys and widens array{ id: ''|'0', ... }|array{ name: ''|'0', ... } to plain non-empty-array:
/** @var array{ id: ''|'0', ... }|array{ name: ''|'0', ... } $u */ \PHPStan\dumpType( $u ); // Dumped type: non-empty-array
That would ask whether $args has any keys at all, resolving every populated array — including all seven call sites in core — to void:
| Call | Combined | Nested |
|---|---|---|
array( 'id' => '' ) | void ✔ | void ✔
|
array( 'name' => '' ) | void ✔ | void ✔
|
array( 'id' => 'x', 'name' => 'y' ) | void ✘ | string ✔
|
array( 'echo' => false ) | void ✘ | string ✔
|
array( 'selected' => 'de_DE' ) | void ✘ | string ✔
|
array() / no argument | string ✔ | string ✔
|
The two rows that look right in the combined column are coincidence: the empty array passes only by failing non-empty-array, and the two bail shapes pass only by being non-empty. src/ was swept for the same construct — no other conditional in core unions array shapes.
This also corrects the previous description, which filed wp_dropdown_languages() under "not dual-mode at all — there is no argument for a condition to switch on". There is one; it is the bail condition rather than the display flag.
## 3. the_date() and the_modified_date() say what they return
Both described their return as "String if retrieving." — naming the type rather than the content, and saying nothing about display mode, which is the one outcome the void exists to convey.
the_date() now names the date and the case where there isn't one: it builds its value only when is_new_day(), so a post sharing a date with the one before it retrieves an empty string. Naming the content without that caveat would promise a date the caller may not get, which is the correction already applied to next_posts(), previous_posts() and wp_register() in r63441. the_modified_date() always concatenates a date, so it takes the short form.
Also raised in review was edit_term_link(). As it stands in r63441 it already reads "HTML content when retrieving, null on failure or without the capability to edit the term. Nothing when displaying." — the same structure as the five title tags from single_post_title() through single_term_title(), so it is left alone.
## 4. void restored on two wpdb methods that never return a value
print_error() and check_database_version() documented void|false and void|WP_Error until r62177 replaced the void with null and added a trailing return null; to each. The premise was that void cannot belong to a union, which holds for PHP's native return types but not for PHPDoc — the premise r63441 corrected.
Both are the shape that keeps void elsewhere in core: the method either succeeds, with nothing to hand back, or reports a failure. print_error() returns false only when errors are suppressed or hidden; otherwise it prints and has no value to give. check_database_version() returns a WP_Error only when the server is too old, and its one caller, wp_check_mysql_version(), tests is_wp_error() and ignores the rest. Under null that meaningless value read as a legitimate one, which is what the void was there to deny — and what nineteen sibling functions documenting void|false, and seven documenting void|WP_Error, still say.
Neither can carry a conditional: both switch on object state rather than on an argument, so the distinction is for the reader. Removing the explicit return null; changes nothing at runtime, since falling off the end returns null anyway, and void in the union is what licenses it.
The rest of r62177 stands. prepare() and get_row() return a null their callers genuinely consume, check_connection() and bail() were corrected in the other direction because their old void branches die rather than return, and get_col_info() is documented mixed.
## 5. null named in the tags whose retrieval branch can return it
Fifteen dual-mode tags document a null in their return description while the @return tag lists only string|void, so the tag omits a value the prose promises and the conditional @phpstan-return already states.
The three-way union was rejected when those annotations were written, on the grounds that no such form appeared elsewhere in core. That was wrong. Immediately before r62177 the tree held three: WP_Block_Type::__get() as string|string[]|null|void, wpdb::get_row() as array|object|null|void, and one in WP_Theme_JSON as null|void. All three were removed by the same campaign that replaced void in unions on the mistaken premise. The idiom existed; it was erased by the error being undone here.
Thirteen tags become string|null|void — single_post_title(), post_type_archive_title(), single_cat_title(), single_tag_title(), single_term_title(), wp_get_archives(), get_calendar(), edit_term_link(), the_title(), the_title_attribute(), wp_list_comments(), wp_update_php_annotation() and twentytwenty_site_description() — and two become string|string[]|null|void: wp_tag_cloud() and paginate_comments_links(). Values first, then null, then void, following the order the tree used before r62177.
The test is the conditional's retrieval branch, not the wording, so null is named only where a caller can observe it. wp_list_pages(), wp_page_menu(), wp_list_authors(), wp_list_bookmarks() and wp_list_users() look like the same shape but are left alone: their null sits only in the undecidable branch covering a query string or an $args built at runtime, which is the union of both modes rather than a value retrieval mode can hand back. Their retrieval branch is plain string, so string|void is already complete.
## Left open
One point from the review is deliberately not acted on. Of the thirty-one dual-mode return descriptions, twenty-three name the flag ("Calendar HTML when $display is false … Nothing otherwise."), six use retrieving/displaying ("Title when retrieving … Nothing when displaying."), and two name a second dimension as well. Folding the six into the majority form is a defensible normalization, but both phrasings are accurate and it would touch five functions beyond the one raised. Happy to add it if wanted.
The suggestion to declare @param true $deprecated_echo on trackback_url(), as php-stubs/wordpress-stubs does to signal a deprecated argument, is also left for its own ticket. It conflicts with the conditional return, which then reports Condition "true is true" in conditional return type is always true. on the function itself, so adopting it means removing the conditional — and it reclassifies every existing trackback_url( false ) call as an argument-type error, which is a policy decision about deprecated arguments across core rather than a detail of this change.
## Verification
- A temporary probe file confirms each behavior claimed above:
'0'now resolves to plainvoidon all ten tags and is reported when consumed, while query strings carrying a visible flag, dynamic$args, and arrays setting the flag are unchanged in both modes. Thewp_dropdown_languages()conditional resolves correctly in all six shapes tabulated above, and the combined form was verified to fail as described. phpstan-diff --changed --staged --base=HEADis clean on every commit, as enforced by the pre-commit hook.- PHPCS reports no new errors. The warnings on
general-template.phpare the pre-existing$wpdb->prepare()ones inwp_get_archives(), on untouched lines. - PHPUnit
Tests_DBpasses for thewpdbchange: 651 tests, 985 assertions, 2 skipped. Removing the explicitreturn null;is a no-op at runtime. tests/phpstan/baselines/return.missing.neonstays deleted —voidin the union is what licenses falling off the end.
## Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code
Model(s): Opus 5
Used for: Evaluating the review feedback, verifying each claim against PHPStan, and drafting the annotations and this description. The PHPStan, PHPCS and PHPUnit verification was run against the working tree, and the result has been reviewed and is owned by me.
@westonruter commented on PR #13359:
6 days ago
#108
🤖 _Comment drafted initially by Claude Opus 5_
@IanDelMar Thank you for the review. It seems several of your points were right, and five have been acted on in a follow-up PR: https://github.com/WordPress/wordpress-develop/pull/13371
Responses in order.
## void in a plain union
@return string|voidand@return void|stringcarry no more information thanstring|null
I don't think that is correct. For now, PHPStan seems to treat these as equivalent, but PHPStan evolves and that behaviour may change in the future. For users,
voidstill conveys semantic information: if the function returns the valuenull, that value has no meaning. This distinction may be irrelevant to some users, but relevant to others. But there may be other opinions on that.
Right, so this is why we need the conditional returns. It's what allows PHPStan to catch an erroneous usage of a void non-value.
That said, the sentence you quoted was too broad on my part. It was only ever a claim about what PHPStan does with a plain union today, not about what the tag means. Your second point stands independently of the analyzer: void tells a reader the value is meaningless in a way null does not. That distinction is what the wp_get_archives() section below now rests on.
## '0' in the undecidable branch
wp_tag_cloud(): the$args is ''|arraybranch could be extended to include'0', although this is very unlikely to have much practical impact. Looking at the conditional return types, the same seems to apply towp_list_comments(),wp_list_bookmarks(),wp_list_authors(),paginate_comments_links(),the_title_attribute(),wp_list_pages(),wp_page_menu(), andwp_list_users().
Correct, and fixed. The check confirms the reasoning: parse_str( '0', $r ) yields array( 0 => '' ), so wp_parse_args( '0', $defaults ) sets no key the tag reads, the flag keeps its default, and the call prints and returns nothing — indistinguishable from ''. Before the change wp_list_authors( '0' ) resolved to string|null and went unreported while wp_list_authors( '' ) resolved to void.
The branch now reads $args is ''|'0'|array, which also matches how the flag conditions themselves already spell the falsy set as false|0|''|'0'. Ten tags were affected — the nine listed here plus wp_get_archives(), which has the same branch and the same gap. Every other call shape was re-checked and is unchanged: a query string carrying a visible flag, and an $args built at runtime, still stay unions and are reported in neither mode.
Aside: I want to work out an extension to PHPStan which would better handle typing of query strings that get passed to functions like this.
## wpdb::print_error()
wpdb::print_error()also returnsnullat the bottom of the method. I don't think this can be changed back tovoidsimply by adding a conditional return type. It might still be worth documenting it here alongside the other related functions. Althoughvoidcannot be part of a native PHP union type, was there actually an issue with using it in the PHPDoc union?
Also correct, and also fixed — with a second method alongside it.
To the question asked: no, there was no issue with void in the PHPDoc union. That premise is the one this pull request set out to correct; it holds for PHP's native return types only.
print_error() documented void|false until r62177, which replaced the void with null and added a trailing return null;. It is the shape defended under *Deliberately not changed* — the method either succeeds with nothing to hand back, or reports a failure — so leaving it converted contradicted the reasoning applied to the nineteen void|false siblings. check_database_version() in the same class and the same changeset is the void|WP_Error version of the same thing, and its only caller, wp_check_mysql_version(), tests is_wp_error() and ignores the rest. Fixing one and not the other would only have relocated the inconsistency, so both were restored.
As anticipated here, neither can carry a conditional: both switch on object state rather than on an argument, and a PHPStan conditional can only key off a parameter. The distinction is for the reader.
The rest of r62177 was reviewed and stands. prepare() and get_row() return a null their callers genuinely consume; check_connection() and bail() were corrected in the other direction, since their old void branches die rather than return; get_col_info() is documented mixed.
## print_scripts_l10n()
print_scripts_l10n()has long been deprecated. Why annotate it with PHPStan tags rather than ignoring any error raised for that function?
No error was being raised for it, so there was nothing to ignore. Given that it it has a conditional return value, I wanted to document it for posterity even if it is deprecated.
Also, what r63440 was fixing is an inverted description: all three methods said the markup comes back when $display is true, when the string is returned on the ! $display branch and the printing branch returns true. That wording had stood since r36744 and is worth correcting on a deprecated method as much as on a live one.
The conditional is one line on a pure pass-through, kept so the deprecated delegate is not documented more loosely than the method it forwards to. Removing it would leave a caller reading print_scripts_l10n() a wider type than print_extra_script() gives for the identical call. It is defensible either way and can be dropped if the preference is to leave deprecated symbols untouched.
## wp_get_archives() and null in the tag
wp_get_archives(): shouldn't the@return string|voidtag also includenull, given the semantic distinction betweenvoidandnull?
Yes it should, and it now does — for this function and for fourteen others in the same position.
The reason given in the description for not adopting string|null|void was that no such three-way union appears anywhere else in core. That was wrong, and wrong in a telling way. Immediately before r62177 the tree held three of them: WP_Block_Type::__get() as string|string[]|null|void, wpdb::get_row() as array|object|null|void, and one in WP_Theme_JSON as null|void. All three were removed by the same campaign that replaced void in union return types on the premise that a union cannot hold one — the premise r63441 set out to correct. The idiom did exist; it was erased by the mistake being undone here.
Thirteen tags therefore become string|null|void: single_post_title(), post_type_archive_title(), single_cat_title(), single_tag_title(), single_term_title(), wp_get_archives(), get_calendar(), edit_term_link(), the_title(), the_title_attribute(), wp_list_comments(), wp_update_php_annotation() and twentytwenty_site_description(). Two more become string|string[]|null|void: wp_tag_cloud() and paginate_comments_links(). Values first, then null, then void, following the order the tree used before r62177.
The test applied is the retrieval branch of the conditional rather than the wording of the description, so null is named only where a caller can actually observe it. wp_list_pages(), wp_page_menu(), wp_list_authors(), wp_list_bookmarks() and wp_list_users() look like the same shape but are deliberately left alone: their null appears only in the undecidable branch covering a query string or an $args built at runtime, which is the union of both modes rather than a value retrieval mode can return. Their retrieval branch is plain string, so string|void is already complete.
## the_date(), the_modified_date() and edit_term_link()
the_date(),the_modified_date(): the return description says "String if retrieving.". This wording is inconsistent with the descriptions used for the other functions. This also applies toedit_term_link().
Agreed on the first two, and fixed. Both said only "String if retrieving." — naming the type rather than the content, and never stating the display-mode outcome, which is the one thing the void exists to convey. the_date() now also names the case where the string is empty, since the function builds its value only when is_new_day(); naming the content without that caveat would promise a date the caller may not get.
edit_term_link() appears to have been read from an earlier commit on the branch. As it stands in r63441 it reads "HTML content when retrieving, null on failure or without the capability to edit the term. Nothing when displaying." — the same structure as the five title tags from single_post_title() through single_term_title(), so changing it alone would make it the odd one out among six.
A survey of the thirty-one dual-mode descriptions does show a genuine split, but a different one. Twenty-three name the flag ("Calendar HTML when $display is false … Nothing otherwise."), six use retrieving/displaying ("Title when retrieving … Nothing when displaying."), and two have a second dimension to account for and so name both ("Nothing when 'echo' is true and 'format' is not 'array'"). Folding the six into the majority form is a reasonable normalization; it just touches five functions beyond the one raised here, so it seemed better proposed than done unilaterally.
## wp_dropdown_languages()
wp_dropdown_languages(): the description is incorrect. It says that "nothing is returned when the requiredidornameargument is missing." If those arguments are missing, they are populated with the default'locale'. The function actually bails when$args['id']or$args['name']is falsy. This could be represented as something along the lines of:
($args is array{id: null|0|''|'0', ...}|array{name: null|0|''|'0', ...} ? void : string)
The description was wrong, exactly as described — id and name both default to 'locale', so neither can go missing, and the bail is on a falsy value the caller supplied. It now says "empty" rather than "required … missing".
The suggested conditional does not survive contact with PHPStan, though. A union of two unsealed array shapes with different required keys cannot be represented, so it is widened to plain non-empty-array:
/** @var array{ id: ''|'0', ... }|array{ name: ''|'0', ... } $u */ \PHPStan\dumpType( $u ); // Dumped type: non-empty-array
The condition therefore stops asking whether id or name is empty and starts asking whether $args has any keys at all:
| Call | Combined | Nested |
|---|---|---|
array( 'id' => '' ) | void ✔ | void ✔
|
array( 'name' => '' ) | void ✔ | void ✔
|
array( 'id' => 'x', 'name' => 'y' ) | void ✘ | string ✔
|
array( 'echo' => false ) | void ✘ | string ✔
|
array( 'selected' => 'de_DE' ) | void ✘ | string ✔
|
array() / no argument | string ✔ | string ✔
|
The two rows that look right in the combined column are coincidence — the empty array passes only by failing non-empty-array, and the two bail shapes pass only by being non-empty. Every realistic call, including all seven call sites in core, would have been typed void and reported. Nesting the two shapes is the only form PHPStan evaluates shape by shape, so that is what was committed:
* @phpstan-return (
* $args is array{ id: ''|'0', ... }
* ? void
* : ( $args is array{ name: ''|'0', ... } ? void : string )
* )
The falsy set was narrowed to ''|'0' rather than null|0|''|'0', since both arguments are documented @type string and those are the falsy strings — parallel to the sibling conditions enumerating false|0|''|'0' for a bool|int flag. Easy to widen if modeling undocumented argument types is preferred.
This also corrects a claim in the description above, which filed wp_dropdown_languages() under "not dual-mode at all — there is no argument for a condition to switch on". There is one; it is the bail condition rather than the display flag. That makes it the only function in the set whose conditional keys off a bail shape, so if the pattern is accepted, the other bail-only voids deserve the same sweep.
src/ was also swept for the same construct elsewhere — no other conditional in core unions array shapes, so nothing is carrying this bug.
## @param true $deprecated_echo on trackback_url()
Unrelated, but I just noticed this:
@param true $deprecated_echoontrackback_url()would result in:Parameter #1 $deprecated_echo of function trackback_url expects true, false given.. In php-stubs/wordpress-stubs, this is used deliberately to signal that a deprecated argument was supplied. See: https://phpstan.org/r/2e2aa144-9b64-4815-bf72-2c343512462d
A genuinely useful idea, and the wordpress-stubs precedent is a good one. It does conflict with what is there now, though: declaring the parameter true while a conditional return switches on it produces a new error on the function itself.
Condition "true is true" in conditional return type is always true. [conditionalType.alwaysTrue]
So adopting it means removing the conditional. That turns out to cost less than it appears — a call passing false still has its return resolved from the argument's own type, so trackback_url( false ) still types as string while trackback_url() still resolves to void and is reported.
The larger consideration is that it reclassifies every existing trackback_url( false ) call as an argument-type error. That is the intent, but it is a policy decision about how deprecated arguments should be signaled across core rather than a detail of this change, so it seems better raised on its own ticket.
## On workarounds versus design questions
General thought: I think the effort being put into improving code quality is very welcome.
However, I also think that some of the errors reported by PHPStan point to design questions and should prompt us to think about those questions, rather than trying to introduce workarounds solely to silence PHPStan. Once such workarounds land in core, the PHPStan error disappears and therefore no longer indicates that there may be an underlying problem. The issue has not necessarily been resolved; it may simply have been masked.
I don’t think there is anything wrong with deliberately ignoring some errors. That way, there is still an indication that something may deserve attention, without forcing the implementation or documentation into shapes that primarily exist to satisfy the analyser.
Good point. Yeah, there are definitely some design issues here. One example that stands out to me is that WP_Term_Query::get_terms() cannot have its return type narrowed since it takes no argument, but the WP_Term_Query::query() wrapper function _does_ take an argument, and so it _can_ have its return type conditionally narrowed.
Every conditional added here *creates* reports that did not exist before — a call consuming the result of a printing tag is now an error where previously the plain union made it silent. Nothing was ignored and no baseline entry was added. The one baseline removed, return.missing, went because void in the union genuinely licenses falling off the end, which is the accurate description of what those functions do rather than a way around the rule.
Where the underlying design question was reached rather than papered over, it was left alone and written down. The functions listed under *Deliberately not changed* are there precisely because the annotation cannot express what they do — single_month_title(), wp_list_categories(), wp_nav_menu() and twentytwenty_site_logo() each return a meaningful value on a path shared by both modes, and making them fit would have meant changing what callers receive. That is the dual-responsibility problem originally raised, and it is still open; an annotation was not treated as its resolution.
Where that leaves things: updates are ready for review in a follow-up PR: https://github.com/WordPress/wordpress-develop/pull/13371
@marian1 commented on PR #13371:
6 days ago
#109
Folding the six into the majority form is a defensible normalization, but both phrasings are accurate and it would touch five functions beyond the one raised.
Yes, that's what I meant. It does not follow the "under this condition, the return type is ..." pattern. "when retrieving" requires looking up when retrieval actually happens.
@param true $deprecated_echoontrackback_url(), as php-stubs/wordpress-stubs does to signal a deprecated argument, is also left for its own ticket. It conflicts with the conditional return
There is no other way to express this in the DocBlock. Doing it differently would require a PHPStan rule. And because it is a "conflict" the example added @phpstan-ignore conditionalType.alwaysTrue. There is no need to remove the conditional.
@marian1 commented on PR #13359:
6 days ago
#110
print_scripts_l10n(): Neither core nor anyone else should use this function - it is deprecated. Using it should not be rewarded with a narrowed return type.- On workarounds versus design questions: This was a general remark, as I had the impression that the initial replacement of
voidwithnullwas an attempt to resolve the design issue that these functions conditionally either return or echo. If you oppose documenting this behaviour - which is tempting - then I think the alternative should be to consider how that behaviour could be removed. For example, by havingget_search_form()andprint_search_form()instead of using$args['echo'] = trueto turnget_search_form()into a "convenience" wrapper forecho get_search_form().
@westonruter commented on PR #13359:
6 days ago
#111
print_scripts_l10n(): Neither core nor anyone else should use this function - it is deprecated. Using it should not be rewarded with a narrowed return type.
I'm including it because the absence of a conditional return it would come up again when scanning for places where it is missing. The method remains deprecated, so it's not suddenly encouraged to be used.
- On workarounds versus design questions: This was a general remark, as I had the impression that the initial replacement of
voidwithnullwas an attempt to resolve the design issue that these functions conditionally either return or echo. If you oppose documenting this behaviour - which is tempting - then I think the alternative should be to consider how that behaviour could be removed. For example, by havingget_search_form()andprint_search_form()instead of using$args['echo'] = trueto turnget_search_form()into a "convenience" wrapper forecho get_search_form().
My goal currently is to try to document the behavior of the functions as they exist today, and hopefully add static analysis hints to catch incorrect usage as much as possible. Refactoring to address the fundamental inability to have deterministic return types for a given input is something which should be done, but at a later stage.
@marian1 commented on PR #13359:
6 days ago
#112
My goal currently is to try to document the behavior of the functions as they exist today, and hopefully add static analysis hints to catch incorrect usage as much as possible. Refactoring to address the fundamental inability to have deterministic return types for a given input is something which should be done, but at a later stage.
Yes, I just wanted to raise awareness of the broader implications of introducing workarounds merely to silence PHPStan or to avoid documenting behaviour that seems odd from a native type perspective. In the case of the functions discussed here, those workarounds have already been reverted. Thank you for taking this up.
@westonruter commented on PR #13371:
6 days ago
#113
@param true $deprecated_echoontrackback_url(), as php-stubs/wordpress-stubs does to signal a deprecated argument, is also left for its own ticket. It conflicts with the conditional return
There is no other way to express this in the DocBlock. Doing it differently would require a PHPStan rule. And because it is a "conflict" the example added
@phpstan-ignore conditionalType.alwaysTrue. There is no need to remove the conditional.
I've applied your suggestion in c8fdc2b.
@marian1 commented on PR #13371:
5 days ago
#115
I just noticed that the if condition inside the function body also produces a "If condition is always true.". Don't know if you want to clutter the code with ignore statements.
@westonruter commented on PR #13371:
5 days ago
#116
I just noticed that the if condition inside the function body also produces a "If condition is always true.". Don't know if you want to clutter the code with ignore statements.
@IanDelMar I don't see that. Without the comment in the function docblock I see:
1252 Condition "true is true" in conditional return type is always true.
🪪 conditionalType.alwaysTrue
When I restore @phpstan-ignore conditionalType.alwaysTrue to the function docblock, there are no PHPStan errors for any line in the function. I'm checking with rule level 10.
@marian1 commented on PR #13371:
5 days ago
#117
@westonruter commented on PR #13371:
5 days ago
#118
@IanDelMar Ah, this is because you configured with treatPhpDocTypesAsCertain enabled. This is set to false in core:
The error goes away in the playground when turning that option off: https://phpstan.org/r/f2392eb0-9dca-445c-91c8-bfb62e2e32a8
This ticket was mentioned in PR #13395 on WordPress/wordpress-develop by @westonruter.
5 days ago
#119
PHPStan's cache holds more than analysis results. It also stores what PHPStan read out of each source file — the docblocks and signatures it found — keyed by that file's contents and nothing else. The extensions in tests/phpstan change what reading a file yields without changing the file: HashNotationVisitor rewrites a docblock in the syntax tree, and the bytes on disk stay as they were.
That rewriting only reaches the files PHPStan routes to its rich parser, which is the set of files being analyzed. PathRoutingParser sends everything else to the simple parser, where the visitors never run. So a run narrowed to a few paths — an editor analyzing the file being typed in, or one scoped to a diff — caches the unrewritten reading of every file outside those paths.
Because generate-baselines.php shared the tmpDir that those runs write to, a later regeneration restored that reading and recorded messages derived from types the extensions would have replaced.
## How it showed up
While an unrelated docblock was being edited, regenerating the baselines produced a one-line change to assign.propertyType.neon that had nothing to do with the edit:
-message: '#^Static property WP_Widget_Media\:\:\$l10n_defaults \(array\<string\>\) does not accept array\<string, array\<int\|string, string\|null\>\|string\>\.$#' +message: '#^Static property WP_Widget_Media\:\:\$l10n_defaults \(array\<string\>\) does not accept array\<string, array\|string\>\.$#'
WP_Widget_Media::get_l10n_defaults() assigns the return of _n_noop(), whose @return is written in hash notation. With HashNotationVisitor applied, that resolves to the array shape the notation describes, which generalizes to array<int|string, string|null>. Without it, _n_noop() returns a bare array — the second line above.
Reverting the docblock edit did not clear it, because the entry was never caused by the edit. It came from a cached reading of l10n.php left behind by an earlier narrowed run.
## The fix
Point the analysis at .cache/baselines, a directory only this script writes to and only ever from a full run, so no narrowed run can leave anything in it.
tmpDir is set in a wrapper that includes the stripped copy of the configuration rather than in the copy itself, because NEON rejects a duplicated key (Duplicated key 'parameters') and the copy already carries the parameters section it was made from. A file's own parameters win over those of the files it includes, so the wrapper's tmpDir is what takes effect.
## Verification
- Poisoning a shared
.cachewith a narrowed run reproduces the degraded message, and a regeneration immediately afterwards now leaves the baselines unchanged. Before this change, that same sequence produced the diff above. - A full
composer phpstan:baselinesregenerating every identifier leavestests/phpstan/baselinesandphpstan.neon.distbyte-identical. - A generator run no longer touches
.cache/resultCache.phpor.cache/cache.
tests/phpstan/README.md already documented this hazard in both directions; it now notes that baseline generation is exempt from the narrowed-run half of it.
The commit also adds the trailing newline generate-baselines.php was missing, which every sibling in the directory has.
## Not addressed here
This makes baseline generation immune to the cache being poisoned. It does not make narrowed runs themselves correct — a run scoped to a subset still reports against a bare array wherever a hash-notation type crosses a file boundary, whether or not it shares a cache with anything. That is a separate problem and worth its own ticket.
Trac ticket: Core-65817
## Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Opus 5
Used for: Diagnosing the cache interaction, and drafting the change and its comments. The diagnosis was confirmed by reproducing the degraded baseline entry from a poisoned cache and confirming a wiped cache regenerates it identically to trunk; the implementation and verification were reviewed by me.
---
🤖 Generated with Claude Code
This ticket was mentioned in PR #13396 on WordPress/wordpress-develop by @westonruter.
5 days ago
#120
Follow-up to #13233 (r63420, f3cc113c59b6ec052f8f9a0b2416f647b068c94b), which taught PHPStan to read hash notation.
HashNotationVisitor is tagged phpstan.parser.richParserNodeVisitor, and a visitor tagged that way runs in PHPStan's RichParser. PathRoutingParser sends a file to that parser only when it is one of the files being analyzed. A file merely *read* so that something it declares can be reflected goes to the simple parser instead, where the visitors never run.
PHPStan can route that way because its own two parsers agree about everything reflection exposes. The simple one is wrapped in a CleaningParser, whose CleaningVisitor empties function, method and closure bodies and touches nothing else, so docblocks and signatures come out of either parser identically. What is reflected out of a file does not depend on whether that file was analyzed — which is what makes it correct for PHPStan to key its per-file reflection cache on the file's contents alone.
HashNotationVisitor breaks that agreement. It rewrites @param, @return and @var docblocks, which are exactly what reflection exposes.
## What that costs today
Analyzing a subset of the tree reports against types the visitor would have replaced, wherever one crosses a file boundary. WP_Widget_Media::get_l10n_defaults() assigns the return of _n_noop(), whose @return is hash notation, so it is such a boundary:
# phpstan analyse src/wp-includes/widgets/class-wp-widget-media.php Static property WP_Widget_Media::$l10n_defaults (array<string>) does not accept array<string, array|string>. # phpstan analyse (whole configured tree) Static property WP_Widget_Media::$l10n_defaults (array<string>) does not accept array<string, array<int|string, string|null>|string>.
The narrowed run is the wrong one. It is also the shape most tooling runs in: an editor analyzing the file being typed in, or a run scoped to a diff. Both under-report, against a type wider than the documentation describes, and can equally report errors a full run does not.
The reading is then stored in the shared cache, so the next *full* run restores it — which is how this surfaced. Core-65817 has a companion PR, #13395, isolating baseline generation from that. This addresses the cause rather than that one consequence, and the two are independent.
## The change
Have the simple parser wrap the rich one. Every parse then rewrites the same docblocks, the cleaning still happens on top of it, and the reflection cache is keyed on contents correctly again.
This redefines currentPhpVersionSimpleParser in terms of currentPhpVersionRichParser, both PHPStan's own service names from its conf/parsers.neon, and neither a documented extension point. Should a release rename either, the definition stops being wired into anything and today's behavior returns silently. That tradeoff is recorded beside the definition together with what to check, and it is the reason the comment there is as long as it is. Worth a reviewer's opinion on whether it is acceptable — the alternative is leaving subset analysis wrong.
## Results are unchanged
- Analyzing the whole configured tree before and after reports the same 1544 errors, with no textual difference between the two outputs.
- Every baseline still matches, with
reportUnmatchedIgnoredErrors: true. - Regenerating every baseline leaves
tests/phpstan/baselinesbyte-identical. - Builds and analyzes at level 10 with
bleedingEdge, and completes at the 2G limit CI uses.
## What it costs
Parsing that a subset run used to avoid is now work it does, so a cold cache pays for it. Three runs per cell, spread within each under 0.1s:
| Target | Cache | Before | After | Delta |
|---|---|---|---|---|
| One file | cold | 3.39 / 3.42 / 3.50 | 3.90 / 3.93 / 3.96 | +0.5s (+14%) |
| One file | warm | 2.23 / 2.23 / 2.25 | 2.11 / 2.12 / 2.24 | none |
src/wp-includes/widgets (14 files) | cold | 5.19 / 5.21 / 5.34 | 6.53 / 6.53 / 6.54 | +1.3s (+24%) |
src/wp-includes/widgets | warm | 1.50 / 1.51 / 1.60 | 1.53 / 1.53 / 1.53 | none |
| Whole tree | cold | 29.0s | 29.6s | +0.6s (+2%) |
A warm cache reads the same stored result either way, so it is unchanged — and that is the case an editor and a diff-scoped run are in after their first analysis. src/wp-includes/widgets is the worst ratio because it analyzes fourteen small files while reflecting most of wp-includes: the denominator is fixed and the numerator barely grows.
Memory moves from 174.5 MB to 176.5 MB. The cache on disk is the same size, 8.5 MB either way, because CleaningParser still wraps the result and the stored trees are stripped exactly as before.
CI analyzes the whole tree, so it pays the 2%, and only on a cold cache — which this change causes once anyway, its cache being keyed on the files in tests/phpstan.
Trac ticket: Core-65817
## Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Opus 5
Used for: Tracing the behavior through PHPStan's parser routing, and drafting the change, its comment and the benchmarks. Every claim above was measured rather than reasoned: the differing messages were reproduced from the two run shapes, the full-run outputs diffed, and each timing repeated three times. Reviewed by me.
---
🤖 Generated with Claude Code
This ticket was mentioned in PR #13417 on WordPress/wordpress-develop by @westonruter.
2 days ago
#127
Narrows the documented types of wp_array_slice_assoc(), and adds PHPStan generics so that the keys of the returned slice are inferred from the $keys argument rather than being lost.
Trac ticket: Core-65817
## Background
wp_array_slice_assoc() was introduced in r15574 (September 2010, See Core-14572) to back wp_dropdown_users(), and shipped in 3.1.0. Its docblock has carried a bare array for both parameters and the return value ever since. The touches it has had since — r18448 for typo fixes, r54929 for the reserved-keyword parameter rename — adjusted prose and names but never the types, so the function still describes itself no more precisely than it did in 2010.
That matters more than it looks, because the function is a pure key filter: everything about what it returns is determined by what it was passed. Documented as array, all of that is discarded at every one of its ~40 call sites.
## What this changes
Two commits, each self-contained.
1. Narrow the plain docblock types. @param array<string, mixed> $input_array, @param string[] $keys, @return array<string, mixed>.
2. Add generics so the returned key set follows $keys. Bound with @phpstan--prefixed tags below the plain ones, so the human-readable types in the developer reference are untouched:
* @phpstan-template TKey of string * @phpstan-template TValue * @phpstan-param array<string, TValue> $input_array * @phpstan-param array<TKey> $keys * @phpstan-return array<TKey, TValue>
A call with a literal list of keys now resolves to array<'age'|'name', ...> rather than array<string, ...>, and the input's value types carry through to the slice. A call whose keys are not known statically falls back to array<string, ...> as before.
## Measured impact
Measured with PHPStan 2.2.13, comparing each commit against trunk.
Against the CI configuration (phpstan.neon.dist, level 5 + baselines): no errors, and no baseline count changes. Nothing here reaches CI.
At the stricter level 10 the picture is mixed, and worth stating plainly. Over the whole src/ tree, trunk reports 27,106 errors; this branch reports 27,118, a net +12.
| Δ | Resolved | Introduced | |
| --- | ---: | ---: | ---: |
| Commit 1 (narrow types) | +9 | 6 | 15 |
| Commit 2 (generics) | +4 | 1 | 5 |
Resolved by commit 1 — all inside the function itself: three missingType.iterableValue for its own untyped signature, and three offsetAccess.invalidOffset ("Possibly invalid array key type mixed") now that $key is known to be a string.
Introduced by commit 1 — fifteen argument.type errors, every one a caller passing an untyped array into the newly narrowed array<string, mixed>. These are pre-existing gaps in the callers' own documentation that the narrowing surfaces; they are not defects created here, and each is fixed by typing the caller rather than by widening this function back:
wp-includes/author-template.php:484wp-includes/class-wp-comment-query.php:455,:853,:1051wp-includes/class-wp-customize-panel.php:227wp-includes/class-wp-customize-section.php:238wp-includes/class-wp-site-query.php:352wp-includes/class-wp-term-query.php:1183wp-includes/comment.php:2981wp-includes/theme.php:2572,:3557,:3558wp-includes/user.php:938,:1773wp-includes/widgets/class-wp-widget-media.php:341
Resolved by commit 2 — one return.type in wp-includes/class-wp-script-modules.php:738, where get_marked_for_enqueue() no longer returns something broader than the shape it documents.
Introduced by commit 2 — five argument.templateType, "Unable to resolve the template type TValue". All five are call sites that pass mixed as $input_array, so there is nothing for TValue to bind to:
wp-includes/class-wp-customize-manager.php:1395wp-includes/widgets/class-wp-widget-media-audio.php:157wp-includes/widgets/class-wp-widget-media-gallery.php:145wp-includes/widgets/class-wp-widget-media-image.php:320wp-includes/widgets/class-wp-widget-media-video.php:196
These are the same five lines that already report expects array<string, mixed>, mixed given, so each of those call sites now reports twice for one underlying defect. Typing those five callers clears both errors at each. If reviewers would rather not take that cost, commit 2 can be dropped independently of commit 1, at the price of the class-wp-script-modules.php fix.
## Follow-up
The twenty call sites listed above are the natural next step, and are deliberately left out of this PR to keep it reviewable and scoped to the function itself.
🤖 Generated with Claude Code
This ticket was mentioned in PR #13430 on WordPress/wordpress-develop by @westonruter.
33 hours ago
#129
Documents the types of wp_parse_args(), which has described its two parameters and its return value as a bare array since it was introduced.
Trac ticket: Core-65817, Core-65860
## Background
wp_parse_args() arrived in r5397 for 2.2, and its docblock has carried untyped arrays ever since. The touches it has had since — r38670 improving a parameter description, r47429 correcting the documented default of $defaults — adjusted prose and defaults but never the types.
That matters because the function is used throughout core to normalize arguments, so every one of its ~240 call sites in src/ reads its result as an untyped array, and everything downstream of those reads is mixed.
## What this changes
@param string|array<string, mixed>|object $args@param array<string, mixed> $defaults@return array<string, mixed>, plus a native: arrayreturn type on the signature.
The documented return and the analysis return deliberately differ, which is worth explaining rather than leaving to be rediscovered:
* @return array<string, mixed> Merged user defined values with defaults. * @phpstan-return array<array-key, mixed>
String keys are what every caller means, but they are not something the function can promise, and integer keys stay reachable through arguments that satisfy the documented parameter types exactly:
wp_parse_args( json_decode( '{"0":"a","name":"b"}' ), array( 'name' => '' ) ); // array( 'name' => 'b', 0 => 'a' ) wp_parse_args( '0=a&name=b', array( 'name' => '' ) ); // array( 'name' => 'b', 0 => 'a' )
get_object_vars() reports a numeric-string property under an integer key, and parse_str() reads 0=a as one. Both of those are in-contract inputs, so narrowing callers further cannot close the gap — only changing the body could. Documenting array<string, mixed> alone leaves two return.type errors on the function's own return statements, which no docblock elsewhere can pay off; the prefixed tag records what can actually come back so the documented type can still describe the intent.
## Measured impact
Measured with PHPStan 2.2.13 against trunk.
Against the CI configuration (phpstan.neon.dist, level 5 + baselines): no errors.
At the stricter level 10, over the whole src/ tree, trunk reports 27,115 errors and this branch reports 27,233 — a net +118, from 3 resolved and 121 surfaced.
The three resolved are the missingType.iterableValue errors on the function's own two parameters and return value.
The 121 surfaced are all argument.type, spread across 65 files, and every one is a caller handing wp_parse_args() an array it has not documented the contents of:
Parameter #1 $args of function wp_parse_args expects array<string, mixed>|object|string, array given.
These are pre-existing gaps in the callers' own documentation that the narrowing makes visible; they are not defects introduced here, and each is fixed by typing the caller rather than by widening this function back. They are deliberately left for follow-up so that this change stays reviewable — per the guidance on Core-65817, they are best worked through by component rather than all at once.
## One baselined error
One of the surfaced errors is not caller documentation and cannot be fixed by adding types:
src/wp-includes/rest-api/endpoints/class-wp-rest-block-types-controller.php:335 Parameter #1 $args of function wp_parse_args expects array<string, mixed>|object|string, list<array> given.
WP_REST_Block_Types_Controller::prepare_item_for_response() runs the registered block styles through array_values() and then merges them with wp_parse_args(). Both operands are lists, so the call reaches array_merge() and simply concatenates them — none of what wp_parse_args() adds over array_merge() applies, and a list can never satisfy array<string, mixed>.
It is baselined here rather than fixed, to keep this change to the docblock. A follow-up replaces the call with array_merge() and removes the baseline entry.
## Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Opus 5
Used for: Two of the five commits — "Add missing typing for the wp_parse_args() return value" and "Update the argument.type baseline after typing wp_parse_args()" — along with the PHPStan measurements reported above and drafting this description. The other commits are my own work. Directed, reviewed and verified by me.
Session transcript: https://claude.ai/code/session_01NtH5qFmG4hJcGWDCx5tMY7
This ticket was mentioned in PR #13432 on WordPress/wordpress-develop by @westonruter.
28 hours ago
#131
Merges the registered block styles with array_merge() instead of wp_parse_args(), and documents the shape of a registered block style so that the values coming out of WP_Block_Styles_Registry are no longer mixed.
Trac ticket: Core-65817, Core-65860
## The call
WP_REST_Block_Types_Controller::prepare_item_for_response() has combined the registered block styles with the ones already prepared for the response by calling wp_parse_args() since the endpoint was introduced in r48173:
$styles = $this->style_registry->get_registered_styles_for_block( $block_type->name ); $styles = array_values( $styles ); $data['styles'] = wp_parse_args( $styles, $data['styles'] );
Both operands are lists — $styles is run through array_values() on the line above, and the styles schema is an array of objects — so the call reaches array_merge() inside wp_parse_args() and concatenates them. Nothing wp_parse_args() adds over array_merge() applies to two lists: there are no string keys to merge on, and nothing is being defaulted.
Calling array_merge() directly means saying what the existing value is, which wp_parse_args() was absorbing. The key is assigned in the loop above from rest_sanitize_value_from_schema(), documented as returning mixed|WP_Error, so neither its presence nor its type is established where it is read. The WP_Error is why the value is checked rather than cast, and the previous behavior — returning the registered styles unchanged when the value is absent or not an array — is preserved exactly.
## The shape
WP_Block_Styles_Registry hands out block style properties from three getters, all documented as a bare array or array[], so every consumer reads the properties off a mixed. This documents them in two registers: the plain tags gain the array generics, which is as much as the developer reference and an editor can use, and a Block_Style_Properties alias carries the shape itself for static analysis.
The alias says more than the register() hash it derives from, because it describes what is stored rather than what is accepted: name is required for a style to be registered at all, and label is filled in from the name when the caller omits it, so both are always present on the way back out even though a caller may pass neither. It is left open, since the array is stored verbatim and a caller's own keys survive — including into the REST response, which never sets additionalProperties to false.
It goes in base.neon beside Maybe_Callable rather than on the registry class, because the styles are read from script-loader.php and block-supports/layout.php as well as from classes, and a type alias declared on a class can only be imported into another class docblock. Only WP_REST_Block_Types_Controller needs to name the alias; everything else picks the shape up through the return types.
$style_data is documented as array<string, mixed> to match, which realigns the hash. Theme.json-like data is string-keyed, and a bare array inside the alias makes the whole alias invalid — it is reported as having no value type specified, and every tag referring to it is then silently ignored.
## The baseline entry
Narrowing wp_parse_args() in r63522 surfaced one error that no caller documentation could fix, precisely because the argument is a list rather than a keyed array. It was baselined there so the typing could land on its own, with the note that a follow-up would rewrite the call. This is that follow-up, so the entry now matches nothing and is removed.
## Measured impact
Measured with PHPStan 2.2.13 against trunk at r63522.
Against the CI configuration (phpstan.neon.dist, level 5 + baselines): no errors.
At rule level 10 over the whole src/ tree, trunk reports 27,233 errors and this branch reports 27,206 — 27 resolved and none introduced:
| File | Resolved |
| --- | ---: |
script-loader.php | 8 |
class-wp-block-styles-registry.php | 7 |
class-wp-theme-json.php | 6 |
class-wp-theme-json-resolver.php | 3 |
class-wp-rest-block-types-controller.php | 2 |
block-supports/layout.php | 1 |
The spread across four subsystems is the point of declaring the alias globally: nothing outside the registry names it, and every one of those files picks the shape up through a return type.
## Note on test coverage
No test changes accompany this, deliberately. The array_merge() replacement is behavior-preserving in every case — wp_parse_args() reduces to array_merge( $defaults, $args ) when the defaults are a non-empty array and returns the args otherwise, which is what the explicit check reproduces. That was verified by writing a test for the one uncovered path (a block type registered with 'styles' => false alongside a register_block_style() call) and running it against the code as it stands on trunk, where it also passes. There is no input that distinguishes the two implementations, so there is no assertion that could fail before this change and pass after it.
## Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Opus 5
Used for: All three commits, along with the PHPStan measurements reported above and drafting this description. Directed, reviewed and verified by me.
Session transcript: https://claude.ai/code/session_01NtH5qFmG4hJcGWDCx5tMY7
@westonruter commented on PR #13041:
27 hours ago
#132
Closing as this was fixed independently in r63485 (2fbc03a4948160e319c0870a24b035a5996c957a).
This ticket was mentioned in PR #13433 on WordPress/wordpress-develop by @westonruter.
27 hours ago
#133
Adds a PHPStan dynamic return type extension that types the result of wp_parse_args() from the arguments it was called with.
Trac ticket: Core-65817
## Why
In r63522 the types of wp_parse_args() were documented, which says what the function promises. This is the other half: @return array<string, mixed> still discards everything the body works out for a *particular* call. The body ends in array_merge( $defaults, $parsed_args ), so a call whose defaults are written out at the call site returns an array whose keys are known too — and that is roughly four calls in five across src/, 193 of the 240 call sites.
wp_parse_args( $query, array( 'orderby' => 'name', 'number' => 10 ) )
resolves to array{orderby: mixed, number: mixed, ...<string, mixed>} rather than array<array-key, mixed>. The values stay mixed there because an unshaped $args may hold anything at any key, which is the honest answer rather than a limitation.
Where $args does carry a shape the value types survive the merge, and that is where most of the benefit comes from. It also arrives for free: HashNotationVisitor already derives a shape from the @type list documenting an $args parameter, and the @return was throwing it away again a line later. The two now compound, so documenting a hash on a function that parses its arguments types the parsed result as well.
## How
Nothing here assumes a caller honors the types of the defaults it overrides. All three branches the body takes for $args are modeled as written — an array is used as it is, an object goes through get_object_vars(), and anything else through wp_parse_str(), whose keys are not narrowed to strings since parse_str() reads 0=a as an integer key and the wp_parse_str filter it ends with documents a plain array.
The merge itself is handed back to PHPStan's own array_merge() support rather than reimplemented, so it stays correct about integer-key renumbering and about how two shapes combine. A call whose $args is mixed constrains nothing and keeps the documented type.
Verified against each branch:
$args | Resolves to |
|---|---|
array<string, mixed> | array{orderby: mixed, number: mixed, echo: mixed, ...<string, mixed>}
|
array<string, mixed>\||string | array{orderby: mixed, number: mixed, echo: mixed, ...}
|
string | array{orderby: mixed, number: mixed, echo: mixed, ...}
|
object | array{orderby: mixed, number: mixed, echo: mixed, ...<string, mixed>}
|
array{orderby: string, number: int, echo: bool} | array{orderby: string, number: int, echo: bool}
|
no $defaults, or empty | array<string, mixed>
|
mixed | array<mixed> — declines, keeping the documented type
|
## Measured impact
Measured with PHPStan 2.2.13 against trunk at r63522.
Against the CI configuration (phpstan.neon.dist, level 5 + baselines): no errors.
At rule level 10 over the whole src/ tree, trunk reports 27,233 errors and this branch reports 27,087 — a net −146, from 164 resolved and 18 introduced.
| Resolved | Introduced | ||
| --- | ---: | --- | ---: |
argument.type | 119 | class-wp-widget-media-image.php | 14 |
binaryOp.invalid | 13 | post-template.php | 2 |
echo.nonString | 7 | schema.php | 1 |
offsetAccess.nonOffsetAccessible | 6 | script-loader.php | 1 |
encapsedStringPart.nonString | 5 | ||
offsetAccess.notFound | 5 | ||
| 5 others | 9 |
Fourteen of the eighteen are in one file. WP_Widget_Media_Image::render_media() merges its schema defaults in through wp_list_pluck(), whose own @return array hides the keys it just added; the result is narrowed to the one key this extension can see, and every other key read is then reported as possibly missing. They are false positives inherited from that rather than from the merge modeled here, and none of the eighteen are reported at the rule level the test suite enforces. Typing wp_list_pluck() would clear them, and is worth doing separately.
## The two other commits
The star rating cast is a prerequisite. wp_star_rating() documents @type int|float $rating and hands that value straight to str_replace(), whose $subject takes a string. The line's own comment says the rating may be "coming from a string", so the value it is written for is not the value the docblock describes, and neither is what str_replace() wants. PHP coerces it silently, which is why it has gone unnoticed; the extension makes the documented type visible at that line. Casting makes the conversion the code already relies on explicit, and mirrors the (float) cast the result is fed into.
The baseline regeneration moves one entry's wording. The $parsed_args that wp_list_pages() hands to get_pages() comes from wp_parse_args(), so anything changing how that call is typed changes how the value is described in the message. In r63522 it moved from non-empty-array to non-empty-array<mixed>; resolving the call to a shape moves it back. The error itself is untouched and still needs fixing — it is a genuine pre-existing problem, and fixing it properly rather than re-baselining it a third time would be a reasonable ask.
## Related
Core-66049 proposes PHPStan extensions for a different gap in the same function — the query-string $args form that a conditional return type cannot decide. This does not address that; it types what wp_parse_args() returns, not how X is Y resolves for a string argument.
## Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Opus 5
Used for: All three commits, along with the PHPStan measurements reported above and drafting this description. Directed, reviewed and verified by me.
Session transcript: https://claude.ai/code/session_01NtH5qFmG4hJcGWDCx5tMY7
This ticket was mentioned in PR #13433 on WordPress/wordpress-develop by @westonruter.
27 hours ago
#134
Adds a PHPStan dynamic return type extension that types the result of wp_parse_args() from the arguments it was called with.
Trac ticket: Core-65817
## Why
In r63522 the types of wp_parse_args() were documented, which says what the function promises. This is the other half: @return array<string, mixed> still discards everything the body works out for a *particular* call. The body ends in array_merge( $defaults, $parsed_args ), so a call whose defaults are written out at the call site returns an array whose keys are known too — and that is roughly four calls in five across src/, 193 of the 240 call sites.
wp_parse_args( $query, array( 'orderby' => 'name', 'number' => 10 ) )
resolves to array{orderby: mixed, number: mixed, ...<string, mixed>} rather than array<array-key, mixed>. The values stay mixed there because an unshaped $args may hold anything at any key, which is the honest answer rather than a limitation.
Where $args does carry a shape the value types survive the merge, and that is where most of the benefit comes from. It also arrives for free: HashNotationVisitor already derives a shape from the @type list documenting an $args parameter, and the @return was throwing it away again a line later. The two now compound, so documenting a hash on a function that parses its arguments types the parsed result as well.
## How
Nothing here assumes a caller honors the types of the defaults it overrides. All three branches the body takes for $args are modeled as written — an array is used as it is, an object goes through get_object_vars(), and anything else through wp_parse_str(), whose keys are not narrowed to strings since parse_str() reads 0=a as an integer key and the wp_parse_str filter it ends with documents a plain array.
The merge itself is handed back to PHPStan's own array_merge() support rather than reimplemented, so it stays correct about integer-key renumbering and about how two shapes combine. A call whose $args is mixed constrains nothing and keeps the documented type.
Verified against each branch:
$args | Resolves to |
|---|---|
array<string, mixed> | array{orderby: mixed, number: mixed, echo: mixed, ...<string, mixed>}
|
array<string, mixed>\||string | array{orderby: mixed, number: mixed, echo: mixed, ...}
|
string | array{orderby: mixed, number: mixed, echo: mixed, ...}
|
object | array{orderby: mixed, number: mixed, echo: mixed, ...<string, mixed>}
|
array{orderby: string, number: int, echo: bool} | array{orderby: string, number: int, echo: bool}
|
no $defaults, or empty | array<string, mixed>
|
mixed | array<mixed> — declines, keeping the documented type
|
## Measured impact
Measured with PHPStan 2.2.13 against trunk at r63522.
Against the CI configuration (phpstan.neon.dist, level 5 + baselines): no errors.
At rule level 10 over the whole src/ tree, trunk reports 27,233 errors and this branch reports 27,087 — a net −146, from 164 resolved and 18 introduced.
| Resolved | Introduced | ||
| --- | ---: | --- | ---: |
argument.type | 119 | class-wp-widget-media-image.php | 14 |
binaryOp.invalid | 13 | post-template.php | 2 |
echo.nonString | 7 | schema.php | 1 |
offsetAccess.nonOffsetAccessible | 6 | script-loader.php | 1 |
encapsedStringPart.nonString | 5 | ||
offsetAccess.notFound | 5 | ||
| 5 others | 9 |
Fourteen of the eighteen are in one file. WP_Widget_Media_Image::render_media() merges its schema defaults in through wp_list_pluck(), whose own @return array hides the keys it just added; the result is narrowed to the one key this extension can see, and every other key read is then reported as possibly missing. They are false positives inherited from that rather than from the merge modeled here, and none of the eighteen are reported at the rule level the test suite enforces. Typing wp_list_pluck() would clear them, and is worth doing separately.
## The two other commits
The star rating cast is a prerequisite. wp_star_rating() documents @type int|float $rating and hands that value straight to str_replace(), whose $subject takes a string. The line's own comment says the rating may be "coming from a string", so the value it is written for is not the value the docblock describes, and neither is what str_replace() wants. PHP coerces it silently, which is why it has gone unnoticed; the extension makes the documented type visible at that line. Casting makes the conversion the code already relies on explicit, and mirrors the (float) cast the result is fed into.
The baseline regeneration moves one entry's wording. The $parsed_args that wp_list_pages() hands to get_pages() comes from wp_parse_args(), so anything changing how that call is typed changes how the value is described in the message. In r63522 it moved from non-empty-array to non-empty-array<mixed>; resolving the call to a shape moves it back. The error itself is untouched and still needs fixing — it is a genuine pre-existing problem, and fixing it properly rather than re-baselining it a third time would be a reasonable ask.
## Related
Core-66049 proposes PHPStan extensions for a different gap in the same function — the query-string $args form that a conditional return type cannot decide. This does not address that; it types what wp_parse_args() returns, not how X is Y resolves for a string argument.
## Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Opus 5
Used for: All three commits, along with the PHPStan measurements reported above and drafting this description. Directed, reviewed and verified by me.
Session transcript: https://claude.ai/code/session_01NtH5qFmG4hJcGWDCx5tMY7
@swissspidy commented on PR #13433:
27 hours ago
#135
@westonruter commented on PR #13433:
26 hours ago
#137
🤖 _Comment by Claude Opus 5_
szepeviktor/phpstan-wordpress#309 was read against this branch, and two things it does better have been adopted in 3c90c89871.
get_object_vars() is now synthesized rather than described. The object branch here had been modeled as array<string, mixed>, which is unsound: a numeric-string property name produces an integer key, and json_decode( '{"0":"a"}' ) reaches that readily — the very case cited in wp_parse_args()'s own docblock as of r63522. Copilot flagged the same thing independently. Handing the call back to PHPStan corrects the key types and is more precise about the values as well, since the object's declared properties become known.
The class-scope caveat came with it. wp_parse_args() is a global function, so the get_object_vars() inside it sees public properties only, while a synthesized call resolved in the caller's scope would see the private and protected ones too. Inside a class the extension now declines. That subtlety was not arrived at here independently.
Unpacked arguments are declined, which was missing as well.
Both are credited in the file docblock and in the README, following how ApplyFiltersDynamicFunctionReturnTypeExtension credits the same project.
Two differences remain, deliberately, in the direction of covering more call sites:
- The
wp_parse_str()branch is modeled rather than declined. Where #309 returns null unless$argsis provably an array or object, the string branch is modeled here as a plain array, with its keys *not* narrowed —parse_str()reads0=aas an integer key, and thewp_parse_strfilter it ends with documents a plain array. Sincearray_merge( $defaults, <any array> )still guarantees the defaults' keys are present, the result stays sound, and it reaches the ~36 call sites insrc/whose$argsis documentedarray|string. - Defaults that may or may not be an array are unioned rather than declined. Where #309 returns null unless
$defaultsis provably an array, both branches are combined here: a non-array$defaultsreturns the parsed arguments unmerged and an array one merges them, so the union describes exactly those two outcomes.
The expectations in tests/data/wp_parse_args.php from #309 were checked against this implementation, which agrees on every case — shaped arguments, constant arrays, and absent, empty, non-array and maybe-empty defaults — except the two above and the object cases that this change brings into line.
Measured against trunk at r63524, the branch takes rule level 10 from 27,208 to 27,062: 164 resolved, 18 introduced. The object handling accounted for 25 of that on its own, beyond what the extension already resolved.
@swissspidy commented on PR #13433:
25 hours ago
#138
There's a ton of other useful extensions in phpstan-wordpress, maybe we could find a way to leverage that instead of reinventing the wheel? Avoids drift between the two implementations too.
@westonruter commented on PR #13433:
25 hours ago
#139
Yes, and vice versa too, as I think a few of the extensions written specifically for core were augmenting what was done for phpstan-wordpress or else creating new extensions.
Should we migrate the extensions over to phpstan-wordpress where possible? My concern would be just the additional delay in landing the extensions in new releases.
@swissspidy commented on PR #13433:
24 hours ago
#140
@szepeviktor summoning you! What do you think? :)
@szepe.viktor commented on PR #13433:
23 hours ago
#141
Thank you for summing me.
- Please get me monthly sponsors (agencies) to pay for the food I digest while struggling with WordPress types - now I have to work something else to pay for WordPress stuff
- I do not support using URL query string-like parameters
- Do you want me to add broader array keys to WpParseArgsDynamicFunctionReturnTypeExtension?
This ticket was mentioned in PR #13437 on WordPress/wordpress-develop by @swissspidy.
23 hours ago
#142
Explores the question raised on #13433: rather than adapting extensions from szepeviktor/phpstan-wordpress into tests/phpstan/ one at a time, can core depend on the package and load its extensions directly, without the stubs it bundles?
Short answer: yes, and this branch does it, but only three of its extensions earn a place in core. The rest are either already outdone by core's own versions, or do what a @phpstan-return in the function's docblock does, and a docblock is the better fix because it also types the function for every plugin through the generated stubs. The package has been moving in exactly that direction itself.
## How it is wired
szepeviktor/phpstan-wordpressis added torequire-dev(^2.0.4).- Its
extension.neonis not included. That file bootstrapsphp-stubs/wordpress-stubs, a declaration of every core function and class, which would declare the code under analysis a second time. Composer still installs the stubs as a transitive dependency; they are not read. tests/phpstan/base.neonregisters the extensions that apply to core by class name, with a note on each. The README gets a section recording what was taken, what was not, and why.composer run phpstanon the CI configuration is green. The baselines gain 20 hook docblock findings and oneget_posts()argument now visible throughshortcode_atts().
## What is loaded, and what is not
Measured with PHPStan 2.2.13 and phpstan-wordpress 2.0.4 against trunk at r63526. At rule level 10 over the whole src/ tree, trunk reports 29,381 errors; each extension was registered on its own and the report compared. The CI configuration (level 5 plus baselines) was run with the rules as well, since that is what would land in the baselines.
| Extension | Level 10 | Verdict |
|---|---|---|
ShortcodeAttsDynamicFunctionReturnTypeExtension | −21 (41 resolved, 20 introduced, all in media.php) | Loaded. Types shortcode_atts() from its defaults, the same merge wp_parse_args() performs; a docblock cannot express it. The 20 introduced are precise: the video shortcode multiplies width, typed 360\||640\||string, and get_posts() is handed an include string where it documents int[].
|
HookCallbackRule | +55 (3 $accepted_args mismatches, 52 action callbacks that return a value) | Loaded, one check ignored. The three mismatches were real and are fixed in the second commit. The 52 are functions like wp_save_post_revision() and redirect_canonical() registered on actions on purpose; WordPress discards the value. That message is ignored in phpstan.neon.dist with the reason recorded.
|
HookDocsRule | +661 at level 10, +19 at level 5 | Loaded. Checks that the type a hook docblock documents accepts the value passed, which is what apply_filters() is typed from. The 19 at the CI level are genuine documentation defects: @param int $tt_id passed a numeric-string, @param string $audio passed WP_Post\||null, @param stdClass $details passed a WP_Site. Only docblocks written at the call are checked; a "This filter is documented in" reference is not.
|
EscSqlDynamicFunctionReturnTypeExtension | −13 (15 resolved, 2 introduced) | Not loaded. #12975 gets the same result with a conditional @phpstan-return.
|
WpSlashDynamicFunctionReturnTypeExtension | −3 (7 resolved, 4 introduced) | Not loaded. Core's wp_slash() docblock already carries a conditional type; the extension keeps array shapes through the call, which @phpstan-return ( T is string ? string : T ), as stripslashes_from_strings_only() is written, would also do.
|
WpParseUrlFunctionDynamicReturnTypeExtension | +2 (2 resolved, 4 introduced) | Not loaded. Core's docblock already covers it. The extension is right on one point the docblock misses: the component form can return false, and the introduced errors in pluggable.php are real. That is a one-line docblock fix.
|
SlashitFunctionsDynamicFunctionReturnTypeExtension | 0 (8 messages reworded string → non-falsy-string) | Not loaded. @phpstan-return non-falsy-string on trailingslashit() is the whole effect.
|
NormalizeWhitespace…, StripslashesFromStringsOnly… | 0 | Not loaded. No effect at all; core already types the latter. |
WpConstantFetchRule | +70 | Not loaded. It discourages reading MULTISITE, WP_NETWORK_ADMIN and the like where a function exists, but core is where those functions read them.
|
HookDocsVisitor, HookDocBlock, ApplyFiltersDynamicFunctionReturnTypeExtension | — | Not loaded. Core's versions were adapted from these and go further: they resolve reference comments, bound a docblock's reach to the node it documents, and fold inherited docblocks into the result cache key. |
AssertWpErrorTypeSpecifyingExtension | — | Not loaded. Only relevant once tests/phpunit is analyzed, and then @phpstan-assert on WP_UnitTestCase_Base::assertWPError() itself is the way to express it.
|
## Two things worth knowing before deciding
The package's branches. phpstan-wordpress develops on two lines. Its 2.x branch (v2.0.4, PHPStan ^2.0) is the one core can depend on, since core is on PHPStan 2.2. Its master branch is the PHPStan 1.x line, and that is where szepeviktor/phpstan-wordpress#309, the wp_parse_args() extension, was merged. So depending on the package today does not bring that extension; the version in #13433 is what core has for PHPStan 2, and porting it to 2.x would be the direction of travel.
Where the other extensions went. The 1.x line had extensions for get_post(), get_posts(), get_terms(), get_sites(), current_time(), mysql2date(), wp_die(), has_filter(), term_exists(), is_wp_error(), the $echo parameter, _get_list_table() and more. The 2.x branch removed them in favor of conditional types that php-stubs/wordpress-stubs applies on top of core's docblocks in its `functionMap.php`. That file is, in effect, the list of docblocks core could adopt: each entry there is a @phpstan-return or @phpstan-param core does not carry yet, and moving one into core removes the need for it in the stubs as well as for any extension. Core already has a few of them (wp_parse_url(), wp_slash(), is_wp_error(), wp_die(), check_ajax_referer()).
## What this suggests for upstream
Loading the package surfaced a few things that would make it a better fit for core, and probably for others:
HookCallbackRulereports under PHPStan's own identifiers (arguments.count,return.void,return.missing), andHookDocsRuleunderparameter.phpDocType. Core's baselines are split by identifier, so these now share files with PHPStan's own errors, and the action-return check can only be switched off by matching its message. Identifiers of its own, asWpConstantFetchRulehas, would fix both; a parameter to disable the action-return check would be nicer still.HookDocBlockcould resolve "This filter is documented in" reference comments the way core's does. Core's implementation is intests/phpstan/HookDocBlock.phpand could move upstream.- The
wp_parse_args()extension from #13433 could be contributed to the2.xbranch.
## Follow-ups in core
- Fix the 19 hook docblocks now baselined under
parameter.phpDocType. - Add
falseto the component form ofwp_parse_url()'s@phpstan-return, andnon-falsy-stringtotrailingslashit(). - The
get_posts()call in the gallery shortcode passesincludeas a string. - The
$accepted_argschange toblock-style-variations.phpis also due in Gutenberg, where that file lives.
## Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Fable 5.1
Used for: The survey of both branches of phpstan-wordpress and of the wordpress-stubs function map, the per-extension measurements above, the configuration and README changes, the three $accepted_args fixes, and drafting this description. Directed and reviewed by me.
Session transcript: https://claude.ai/code/session_013QJ9RphuttTR7cS5G4mU21
---
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_013QJ9RphuttTR7cS5G4mU21
---
_Generated by Claude Code_
@westonruter commented on PR #13433:
16 hours ago
#143
@szepeviktor:
- I do not support using URL query string-like parameters
In regards to this, see that Core-66049 has two parts, which are essentially alternatives:
- Support for static query strings like
'echo=0'. - A rule forbidding non-array
$argslike this.
In core it doesn't really matter because there are hardly any instances of query strings being used. So a forbidding rule would primarily help with keeping core away from introducing query strings in the future. (Maybe an extension isn't even necessary here.)
For themes and plugins, there are quite a few which do pass a query string. For them, is it too heavy handed to introduce a rule that causes an error? Naturally they could decide to not adopt the rule.
I'm inclined to agree that we should be strongly encouraging devs to move away from query strings, since they cannot be typed reliably.
@westonruter commented on PR #13433:
11 hours ago
#146
- Please get me monthly sponsors (agencies) to pay for the food I digest while struggling with WordPress types - now I have to work something else to pay for WordPress stuff
@szepeviktor I understand the struggle, but it's not something I can really help with. But we can help with the maintenance of this project. In fact, maybe it makes sense to promote this project as an official repo in the WordPress organization directly as WordPress/phpstan-wordpress? Given the traction that PHPStan has been getting in core, I think this would make sense. And it would lessen your load.
@szepe.viktor commented on PR #13433:
11 hours ago
#147
but it's not something I can really help with
@westonruter I only have a computer at home, you have all those connections to hundreds of people.
I'm not looking for real, visible amount of sponsor money, just the cost of ordering food from an average restaurant.
@westonruter commented on PR #13433:
10 hours ago
#148
@szepeviktor I restored my GitHub sponsorship of you. I had reset all my sponsors after I was laid off last year, but I didn't resume after I got a new job.
What do you think about this repo being adopted into the WordPress org?
@szepe.viktor commented on PR #13433:
10 hours ago
#149
I restored my GitHub sponsorship of you.
Thank you ❤️ I'm looking for agencies, definitely not your personal money.
What do you think about your repo being adopted into the WordPress org?
All I can think of in place of this highly popular package:
- depression
- staring into infinity
so I'd rather keep it.
@westonruter commented on PR #13433:
8 hours ago
#150
@szepeviktor I guess I'm just trying to think about what would be best for WordPress core and Gutenberg for long-term maintenance, collaboration, and an increased pace of development. I see that you published a new release of your package 2 weeks ago, but the previous release was last year. For WordPress core to be able to aggressively work through the baselines as we have been, and to increase the error rule levels, we'll want to be able to quickly land new extensions and make new releases which can be leveraged by both core and Gutenberg.
Are you the sole person who is a maintainer of szepeviktor/phpstan-wordpress who can approve PRs and manage releases? That may not make your project suitable for WordPress core and we may need to fork it into the WordPress org. As it stands right now, your readme includes a statement risking that you may abandon the package without sponsorship. This raises concerns as to whether focus should be placed on adding and refining the extensions in your repo, or else to add them to WordPress core directly, or to a fork of your repo in the WordPress org.
All this being said, I thank you for the work you have done and continue to do on this!
@szepe.viktor commented on PR #13433:
5 hours ago
#151
but the previous release was last year
Yes. Because there was not change, no new issues, no new PRs in the repo.
The latest release includes only a single commit.
I maintain this package with due diligence.
@szepe.viktor commented on PR #13433:
5 hours ago
#152
@westonruter Please do not take away my package with force.
All I can think of in place of this highly popular package:
depression
staring into infinity
@szepe.viktor commented on PR #13433:
4 hours ago
#153
I've replace my FU-style warning message with this.
[!IMPORTANT]
## Need help?
I build and maintain reliable web applications, with a focus on PHP, WordPress, and software quality.
If this package helps your team, feel free to reach out for consulting, development work, or sponsorship.
![(please configure the [header_logo] section in trac.ini)](/chrome/site/your_project_logo.png)
Corrects the
@globalannotation for$l10ninWP_Locale_Switcher::load_translations(), which documented the array asMo[].The class is
MO, defined inwp-includes/pomo/mo.php; no class namedMoexists. Every other
@globalannotation for$l10nin core already usesMO[], so this was the sole outlier. The description is also clarified to notethat the array is keyed by text domain.
This was the only remaining
class.nameCaseoccurrence, so the change emptiestests/phpstan/baselines/class.nameCase.neon. As the baseline header directs,the file is deleted along with its
includesentry inphpstan.neon.dist. Thebaseline was regenerated with
composer phpstan:baselines -- --identifier=class.nameCase, not edited by hand.### Background
git blamedates the annotation to [38961] (2016-10-26), the changeset thatintroduced
WP_Locale_Switcher. It has been present in every revision of thefile since — roughly nine years — and was never copied elsewhere, which is why
the rest of core is already consistent.
### Testing instructions
npm run typecheck:phpontrunkreports[OK] No errors, because the occurrence is baselined.tests/phpstan/baselines/class.nameCase.neonand itsincludesentry, then re-run: PHPStan reportsClass MO referenced with incorrect case: Mo.atsrc/wp-includes/class-wp-locale-switcher.php:241.npm run typecheck:phpreports[OK] No errorswith the baseline gone.npm run test:phpis unchanged: 30774 tests, 4559286 assertions, 86 warnings, 44 skipped, exit 0 — identical before and after.Documentation-only change; no runtime behaviour is affected.
## Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Opus 5
Used for: locating the occurrence via the PHPStan
class.nameCasebaseline,git blame/git log -Sarchaeology to date the annotation, and drafting thisdescription. The change itself, the baseline regeneration, and verification
against the full PHPUnit and PHPStan runs were reviewed and confirmed by me in a
local development environment.