Make WordPress Core

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 westonruter)

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:

  1. r63019
  2. r63020
  3. r63021 & r63022
  4. r63023
  5. r63024

👉 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)

#1 @westonruter
5 weeks ago

  • Description modified (diff)

#2 @westonruter
5 weeks ago

  • Description modified (diff)

#3 @westonruter
5 weeks ago

  • Keywords needs-patch good-first-bug added
  • Owner set to westonruter
  • Status newreviewing

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

Corrects the @global annotation for $l10n in
WP_Locale_Switcher::load_translations(), which documented the array as Mo[].
The class is MO, defined in wp-includes/pomo/mo.php; no class named Mo
exists. Every other @global annotation for $l10n in core already uses
MO[], so this was the sole outlier. The description is also clarified to note
that the array is keyed by text domain.

This was the only remaining class.nameCase occurrence, so the change empties
tests/phpstan/baselines/class.nameCase.neon. As the baseline header directs,
the file is deleted along with its includes entry in phpstan.neon.dist. The
baseline was regenerated with
composer phpstan:baselines -- --identifier=class.nameCase, not edited by hand.

### Background

git blame dates the annotation to [38961] (2016-10-26), the changeset that
introduced WP_Locale_Switcher. It has been present in every revision of the
file since — roughly nine years — and was never copied elsewhere, which is why
the rest of core is already consistent.

### Testing instructions

  1. npm run typecheck:php on trunk reports [OK] No errors, because the occurrence is baselined.
  2. Delete tests/phpstan/baselines/class.nameCase.neon and its includes entry, then re-run: PHPStan reports Class MO referenced with incorrect case: Mo. at src/wp-includes/class-wp-locale-switcher.php:241.
  3. With this branch applied, npm run typecheck:php reports [OK] No errors with the baseline gone.
  4. npm run test:php is 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.nameCase baseline,
git blame/git log -S archaeology to date the annotation, and drafting this
description. 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.

#5 @callumbw95
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_COLLATE comes from wp-config-sample.php, pulled in via scanFiles, where it is define( 'DB_COLLATE', '' );.
  • WP_DEVELOPMENT_MODE comes from tests/phpstan/bootstrap.php, where it is define( '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

  1. On trunk, npm run typecheck:php reports [OK] No errors, because all three occurrences are baselined.
  2. Delete tests/phpstan/baselines/ternary.alwaysFalse.neon and tests/phpstan/baselines/booleanAnd.rightAlwaysFalse.neon along with their includes entries, then run it again. PHPStan reports:
    • Ternary operator condition is always false. twice in src/wp-admin/includes/class-wp-debug-data.php
    • Right side of && is always false. in src/wp-includes/class-wpdb.php
  3. With this branch applied, npm run typecheck:php reports [OK] No errors with both baselines gone and no new errors elsewhere.
  4. npm run test:php is 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 @callumbw95
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 in class-wp-debug-data.php (lines 1548 and 1634)
  • booleanAnd.rightAlwaysFalse, in class-wpdb.php at defined( '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.

#9 @SergeyBiryukov
4 weeks ago

In 63169:

Code Quality: Correct the case of the MO class in a WP_Locale_Switcher docblock.

Corrects the @global annotation for $l10n in WP_Locale_Switcher::load_translations(), which documented the array as Mo[]. The class is MO, defined in wp-includes/pomo/mo.php; no class named Mo exists. Every other @global annotation for $l10n in core already uses MO[], so this was the sole outlier. The description is also clarified to note that the array is keyed by text domain.

This was the only remaining class.nameCase occurrence, so the change empties tests/phpstan/baselines/class.nameCase.neon. As the baseline header directs, the file is deleted along with its includes entry in phpstan.neon.dist. The baseline was regenerated with:

composer phpstan:baselines -- --identifier=class.nameCase

Developed in https://github.com/WordPress/wordpress-develop/pull/12938.

Follow-up to r38961.

Props callumbw95, westonruter, mindctrl.
See #65817.

@SergeyBiryukov commented on PR #12938:


4 weeks ago
#10

Thanks for the PR! Merged in r63169.

#11 @westonruter
4 weeks ago

In 63170:

Build/Test Tools: Add @phpstan-assert on assertWPError and assertNotWPError.

Follow-up to r63005.

See #65817.

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

  1. On trunk, npm run typecheck:php reports [OK] No errors, because the occurrence is baselined.
  2. Delete tests/phpstan/baselines/parameter.unresolvableType.neon and its includes entry, then run it again. PHPStan reports PHPDoc tag @param for parameter $type contains unresolvable type. in src/wp-includes/class-wp-feed-cache-transient.php.
  3. With this branch applied, npm run typecheck:php reports [OK] No errors with the baseline gone and nothing new elsewhere. The baseline directory goes from 73 files to 72.
  4. 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.
  5. composer lint is clean on the changed file.
  6. npm run test:php passes: 30853 tests, 4559514 assertions, 86 warnings, 44 skipped, exit 0, no failures or errors. Run against this branch at 996c6d6864.

## 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.

#13 @westonruter
4 weeks ago

In 63181:

Code Quality: Fully qualify the SimplePie cache type in a docblock.

The @param annotation for $type in WP_Feed_Cache_Transient::__construct() documented the type as Base::TYPE_FEED|Base::TYPE_IMAGE. The class is declared in the global namespace and imports nothing—it writes implements SimplePie\Cache\Base out in full—so the bare Base resolved to \Base, which exists nowhere in core. The shorthand was carried over in r59141 from SimplePie\Cache\Base itself, where it is correct because that file declares namespace SimplePie\Cache;. Qualifying both constants makes the annotation consistent with the rest of the docblock, which already spells the interface out in full.

This was the only parameter.unresolvableType occurrence, so the change empties tests/phpstan/baselines/parameter.unresolvableType.neon. As the baseline header directs, the file is deleted along with its includes entry in phpstan.neon.dist. The baseline was regenerated with:

composer phpstan:baselines -- --identifier=parameter.unresolvableType

Developed in https://github.com/WordPress/wordpress-develop/pull/12970.
Follow-up to r59141, r63020.

Props callumbw95.
See #65817.

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

#15 @westonruter
4 weeks ago

In 63191:

Build/Test Tools: Treat DB_COLLATE and WP_DEVELOPMENT_MODE as dynamic in PHPStan.

The DB_COLLATE and WP_DEVELOPMENT_MODE constants both vary per installation, but neither was listed in dynamicConstantNames, so PHPStan resolved each to the empty string it is handed during analysis. Every conditional guarding either constant was therefore reported as having a constant result, accounting for two ternary.alwaysFalse errors in class-wp-debug-data.php and one booleanAnd.rightAlwaysFalse error in class-wpdb.php. Listing both constants as dynamic resolves all three errors and empties their baselines, which are deleted along with their includes entries in phpstan.neon.dist.

Developed in https://github.com/WordPress/wordpress-develop/pull/12958.
Follow-up to r61699, r63023.

Props callumbw95.
See #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() gates s only with is_scalar(), so ints, floats and bools pass through untouched. Tests_Query_ParseQuery::test_parse_query_s_type asserts exactly that — 3, 3.5 and true all survive a round trip unchanged. Hence scalar, not string, and hence the cast that esc_attr() now receives (behavior-preserving, since it already coerces).
  • It cost more than it saved. An unsealed array shape is *stricter* than a plain array for 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, across WP_Query itself, WP_Media_List_Table and three REST controllers, plus a further 12 in the parseQuery test 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

  1. On trunk, npm run typecheck:php reports [OK] No errors, because the occurrence is baselined.
  2. Delete tests/phpstan/baselines/property.onlyWritten.neon and its includes entry, then run it again. PHPStan reports Property WP_REST_Template_Autosaves_Controller::$parent_post_type is never read, only written.
  3. With this branch applied, npm run typecheck:php reports [OK] No errors with the baseline gone and nothing new elsewhere. The baseline directory goes from 70 files to 69.
  4. 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.
  5. composer lint is clean on the changed file, including the realigned assignments.
  6. 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.php was added alongside the class in [56819] and has 23 test methods, all of which construct the controller.
  7. 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.

#21 @westonruter
4 weeks ago

In 63294:

Build/Test Tools: Suppress every PHPStan baseline when regenerating.

The strip_baseline_includes() function in generate-baselines.php dropped an includes entry only when it resolved inside the output directory. With the default output directory, that test covers the managed region exactly, so nothing appeared amiss. When --output-dir was used to point the output elsewhere, however, every baseline already in force stayed active for the analysis, which then reported only the errors those baselines did not already cover: a differential baseline where a complete one was requested.

Strip the region between the # phpstan:baselines markers whatever it points at, since that region is generated and lists every baseline in force. The output directory test is kept as well, so a baseline listed by hand outside the markers is still suppressed when it is about to be rewritten.

Found while generating a baseline from the 7.0.0 source tree to identify which errors have been introduced since: with the managed region left in place, PHPStan refused to start at all, as two of the entries named files which do not exist in 7.0.0.

Follow-up to r63019.

See #64680, #65817.

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:193data-state="<?php echo esc_attr( has_site_icon() ); ?>". This works, because site-icon.js compares against the literal '1' and writes back '1'/''. The convention is real but implicit.
  • wp-admin/customize.php:291aria-pressed="<?php echo esc_attr( $active ); ?>". This renders aria-pressed="1" for the desktop button and aria-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 until controls.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.

#23 @westonruter
4 weeks ago

In 63296:

Formatting: Document that escaping functions can take numbers.

The esc_attr(), esc_html(), esc_js(), esc_textarea(), and esc_xml() functions were each annotated @param string $text, but none of them has ever required a string at runtime. Both wp_check_invalid_utf8() and _wp_specialchars() open by casting to string, and that coercion dates to r10298 and r11380, the same year esc_attr() and esc_html() were introduced in 2.8.0. Only the annotation ever claimed otherwise, which is why passing an integer, overwhelmingly a post ID, a term ID, or a count, has always worked while still registering as a static analysis error.

Widen the annotation to string|int|float and cast at the top of each function. That cast is not redundant with the ones downstream: it is what makes the $text argument handed to the attribute_escape, esc_html, and js_escape filters match its documented string type, where callbacks previously received the raw integer or float. For esc_textarea() it also affects the output, since that function passed $text straight to htmlspecialchars(), so esc_textarea( null ) emitted a deprecation notice on PHP 8.1 and later.

The bool type is deliberately excluded. Casting true yields '1' while false yields the empty string, and an empty string is indistinguishable from a missing value in any output context. The two call sites in core which pass one stay baselined rather than being permitted by the signature; one of the two is an a11y defect being addressed separately.

Regenerating the baselines drops 54 entries covering 101 errors from tests/phpstan/baselines/argument.type.neon, where esc_attr() was the largest single cluster.

Developed in https://github.com/WordPress/wordpress-develop/pull/13044.
Follow-up to r11380, r63024.

See #65817.

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

  1. npm run typecheck:php on trunk reports [OK] No errors, because the occurrences are baselined.
  2. Delete tests/phpstan/baselines/booleanAnd.alwaysTrue.neon and its includes entry, then re-run: PHPStan reports Result of && is always true. twice at src/wp-includes/html-api/class-wp-html-tag-processor.php:1004.
  3. With this branch applied, npm run typecheck:php reports [OK] No errors across 1289 files with both baselines gone. composer lint is also clean for the modified file.
  4. 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 for src/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 unmodified trunk, so it is unrelated to this change. npm run test:php -- --group html-api passes 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.

#26 @westonruter
4 weeks ago

In 63301:

Code Quality: Annotate HTML API's parse_next_tag() as "impure" for PHPStan.

PHPStan infers the WP_HTML_Tag_Processor::parse_next_tag() method as pure, so the STATE_READY value narrowed onto $parser_state before the call survives across it, even though the method reassigns that property throughout its body. Every state comparison that follows in the base_class_next_token() method is then decided at analysis time rather than at runtime: both && operands resolve to true, the early return true is treated as unconditional, and the remainder of the method is analyzed as unreachable. This in turn leaves skip_rawtext() and skip_script_data() reported as uncalled and $skip_newline_at as never assigned an int. Adding @phpstan-impure, the annotation PHPStan's own tip recommends, resolves ten baselined errors across six identifiers; next_tag() in the same class already carries it.

This empties the booleanAnd.alwaysTrue and property.unusedType baselines, so both files are removed along with their includes entries in phpstan.neon.dist.

Developed in https://github.com/WordPress/wordpress-develop/pull/13057.
Follow-up to r61934, r63023.

Props tstokes8040.
See #65817.

@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, because customize-controls.js sets 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-return shape is correct about runtime behaviour, but building an array key-by-key and then setting version collapses PHPStan's inference to non-empty-array<'version'|int, …>, losing the key/value correlation. Weakening a correct type to satisfy the analyzer seemed worse than leaving it baselined.
  • WP_Post property reports (r62717) — $_wp_attachment_image_alt is read through __get(), which accepts *any* meta key, so unlike the WP_User case above it cannot be resolved with a @property tag.

## 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

  1. npm run typecheck:php on trunk reports [OK] No errors, because the occurrences are baselined.
  2. With this branch applied, npm run typecheck:php reports [OK] No errors across 1289 files, with tests/phpstan/baselines/booleanAnd.rightAlwaysTrue.neon down from 9 entries / 16 occurrences to 5 entries / 5 occurrences.
  3. 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.
  4. npm run test:php -- --group html-api and 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

  1. npm run typecheck:php on trunk reports [OK] No errors, because the occurrences are baselined.
  2. With this branch applied, npm run typecheck:php reports [OK] No errors across 1289 files with the seven baselines regenerated.
  3. 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.php shifts 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.
  4. 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 for src/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 unmodified trunk, 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

  1. npm run typecheck:php on trunk reports [OK] No errors, because the occurrences are baselined.
  2. With this branch applied, npm run typecheck:php reports [OK] No errors across 1289 files with both baselines regenerated.
  3. 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.
  4. composer lint reports no errors for the three modified files. src/wp-includes/query.php carries four pre-existing WordPress.DB.PreparedSQL.NotPrepared warnings at lines 1228 and 1231; the same four are present in the file on trunk and 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

  1. npm run typecheck:php on trunk reports [OK] No errors, because the occurrences are baselined.
  2. With this branch applied, npm run typecheck:php reports [OK] No errors across 1289 files with both baselines regenerated.
  3. 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.
  4. 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 for src/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 unmodified trunk. The counts are identical to a run without these changes.
  5. composer lint reports no errors for the three modified files. class-wp-walker.php carries one pre-existing filename-convention warning that is also present on trunk.

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:

## 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.

#46 @dmsnell
3 weeks ago

In 63314:

Code Quality: Resolve return.empty PHPStan errors in baseline.

Fixes a couple of functions with return-type problems. One is resolved by adding the missing void type, while the other is resolved by updating the function to conform to the existing string return type contract, a likely oversight in the original commit.

This change was part of Contributor Day at WordCamp US 2026.

Developed in: https://github.com/WordPress/wordpress-develop/pull/13081
Discussed in: https://core.trac.wordpress.org/ticket/65817

Props dmsnell, nomadmystic.
See #65817.

#48 @westonruter
3 weeks ago

In 63328:

Code Quality: Delete empty return.empty.neon PHPStan baseline file.

Follow-up to r63314.

See #65817.

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' returns 0|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_Term objects.

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() in taxonomy.php — the only core caller of WP_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[]|string into a single array type unioned with string, so nothing widens. (Confirmed the return.type rule was actually live in that harness by deliberately breaking it.)
  • Neither core call site of 'id=>parent' (wp_edit_posts_query() via wp(), and _get_term_hierarchy() via get_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 sets name_admin_bar, and both label builders add menu_name to the defaults before merging. Core itself reads $taxonomy->labels->menu_name in wp-admin/menu.php. get_post_type_labels() documents menu_name, but not name_admin_bar, and not template_name either, which its own @since 6.6.0 line mentions.
  • Labels that can be null are documented as string. WP_Post_Type::get_default_labels() and WP_Taxonomy::get_default_labels() are both typed (string|null)[][], and for eight taxonomy labels and one post type label the default really is null for one of the two hierarchies: popular_items on a hierarchical taxonomy, parent_item_colon on a non-hierarchical post type, and so on. Those become string|null.
  • get_registered_settings() omits the group key. register_setting() always stores it through its defaults. The same docblock, and register_setting() itself, describe sanitize_callback as callable when its default is null.

Alongside those:

  • get_post_type_labels() describes its return value with the same hash notation get_taxonomy_labels() already uses, instead of a prose list, so the two read alike.
  • WP_Taxonomy::$cap lists the four capabilities it holds, mirroring the capabilities argument of register_taxonomy(). WP_Post_Type::$cap gains a @see get_post_type_capabilities(), where its own list already lives.
  • WP_User::$caps, WP_User::$allcaps and WP_Role::$capabilities are keyed by capability name, so array<string, bool> rather than bool[].

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-param or @phpstan-return always 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 like string|array. A type already more specific than the hash, such as array<string, string|bool>, is left as written.
  • The hash must be well formed: every { closed by a } on its own line, every @type carrying 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:

  • @var hashes 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.
  • object hashes, such as the one on get_taxonomy_labels() that #13220 documents. PHPStan's object shapes are structural, so a shape derived for a value core builds as a stdClass is no longer assignable to a property declared stdClass. Covering these wants the docblocks to name the class rather than object, 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_

#55 @dmsnell
2 weeks ago

In 63344:

Code Quality: Resolve greater.invalid PHPStan errors in baseline.

Resolves type issues in upgrade_430_fix_comments(), which checks for a string return from $wpdb->get_col_length(), which was removed from Core in r32364.

This change was part of Contributor Day at WordCamp US 2026.

Developed in: https://github.com/WordPress/wordpress-develop/pull/13080
Discussed in: https://core.trac.wordpress.org/ticket/65817

Follow-up to [32364].

Props dpantazis, dmsnell.
See #65817.

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 $upgrader as Language_Pack_Upgrader
  • Plugin_Upgrader_Skin: Annotate $upgrader as Plugin_Upgrader
  • Plugin_Installer_Skin: Annotate $upgrader as Plugin_Upgrader
  • Theme_Upgrader_Skin: Annotate $upgrader as Theme_Upgrader
  • Theme_Installer_Skin: Annotate $upgrader as Theme_Upgrader
  • Removes method.notFound.neon baseline file
  • Removes unmatched entries from property.notFound.neon baseline 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.

#59 @westonruter
2 weeks ago

In 63351:

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 https://github.com/WordPress/wordpress-develop/pull/13023.
Follow-up to r28407, r32298, r41721, r48789, r60351, r61504, r61699, r63019.

Props westonruter, apermo.
See #65817.

#60 @westonruter
2 weeks ago

In 63352:

Customize: Emit valid aria-pressed on device buttons.

The responsive preview buttons in the Customizer rendered aria-pressed by passing a bool through esc_attr(), which PHP casts to 1 for the default device and to an empty string for the other two. Neither is valid for a tristate attribute whose only accepted values are true, false, mixed, and undefined, and an invalid value is treated as undefined, so on initial render none of the three buttons was exposed as a toggle button at all. Emitting the literals directly makes the server render agree with the true/false strings that customize-controls.js already writes on the first interaction, matching the existing pattern for aria-expanded in wp-admin/includes/template.php.

The Site Icon button on the general settings screen relied on the same cast to produce the 1 and empty-string sentinels that site-icon.js compares against and writes back. That value was already correct; it is now written out explicitly so the contract is stated rather than resting on PHP's cast rules. With both call sites resolved, no esc_*() entries remain in the argument.type PHPStan baseline.

Developed in https://github.com/WordPress/wordpress-develop/pull/13046.
Follow-up to r36532, r57602, r63296.

Props westonruter, afercia.
See #65817.

#61 @westonruter
2 weeks ago

In 63353:

Code Quality: Remove empty greater.invalid PHPStan baseline file.

Follow-up to r63344.

Props nathanrice.
See #65817.

#62 @westonruter
13 days ago

In 63356:

Build/Test Tools: Treat request-scoped constants as dynamic in PHPStan.

The constants 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 tree PHPStan analyzes, so it concluded that the right side of the defined( 'X' ) && X idiom can never be falsy. That 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 guard is doing real work, so the constants belong in dynamicConstantNames alongside those already listed there.

This resolves 11 booleanAnd.rightAlwaysTrue occurrences, in wp-admin/includes/schema.php, wp-includes/functions.php, wp-includes/l10n.php, wp-includes/load.php and wp-includes/user.php, leaving 5 in the baseline, which was regenerated with composer phpstan:baselines. No source file is touched, so runtime behavior is unchanged.

Developed in https://github.com/WordPress/wordpress-develop/pull/13071.
Follow-up to r63023, r63191.

Props tstokes8040.
See #65817.

#63 @westonruter
13 days ago

In 63357:

Code Quality: Mark have_posts() and have_comments() as "impure" for PHPStan.

Both WP_Query::have_posts() and WP_Query::have_comments() advance the loop as a side effect, so two identical calls do not return the same value. PHPStan treated them as pure, concluded that a while ( have_posts() ) condition could never change, and reported the loops as always-true or always-false with the code past them unreachable. Marking those methods and their procedural wrappers in wp-includes/query.php as @phpstan-impure empties the while.alwaysTrue and while.alwaysFalse baselines, which are deleted along with their entries in phpstan.neon.dist, and trims deadCode.unreachable and if.alwaysTrue.

Two of the removed while.alwaysTrue entries were in Twenty_Fourteen_Ephemera_Widget::widget(), whose loop carries a real bug: $tmp_more was assigned inside the loop whose effects it exists to undo, so on every iteration after the first it captured the value the widget itself had just written, and the restore afterwards put back the secondary query's 0 rather than the caller's value. The assignment moves above the loop, while also guarding against the variable possibly being undefined.

Developed in https://github.com/WordPress/wordpress-develop/pull/13069.
Follow-up to r27124, r30085, r63023.

Props dpantazis, westonruter.
See #65817.

#64 @swissspidy
13 days ago

In 63358:

Code Quality: Narrow post and term query return types by fields.

get_posts(), WP_Query::query() and WP_Term_Query::query() take the query arguments directly, so static analysis can resolve their result from the fields value. Only 'ids' was mapped; 'id=>parent' and every WP_Term_Query field are now covered too.

Two divergences in the cached WP_Query path had to be fixed for the 'id=>parent' type to hold: a query served from the post-queries cache returned parent IDs keyed by their post_parent:{$post_id} cache keys rather than by post ID, and left WP_Query::$posts null when the cached query matched nothing.

Developed in: https://github.com/WordPress/wordpress-develop/pull/13216

Props westonruter, swissspidy.
See #65817.

#66 @swissspidy
13 days ago

In 63360:

Code Quality: Update PHPStan baseline files after r63358.

See #65817.

@westonruter commented on PR #13235:


13 days ago
#67

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

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_Query documents @type string|int|array $terms and passes that value straight to wp_parse_id_list() in WP_Tax_Query::transform_query().
  • get_bookmarks() reassigns $parsed_args['category'] to a WP_Term::$term_id (an int) when resolving category_name, then passes it to wp_parse_id_list().
  • wp_list_bookmarks() calls get_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 (and float) since these do work and should be documented with proper casting.

See https://github.com/WordPress/wordpress-develop/pull/13297

#70 @westonruter
13 days ago

In 63366:

Code Quality: Support passing an int to wp_parse_list() et al.

Passing an integer has worked since r44546 consolidated this parsing on preg_split(), which coerces it to a string, but the declared mixed[]|string parameter type never allowed for it. The type is widened to mixed[]|string|int for wp_parse_list(), wp_parse_id_list(), and wp_parse_slug_list(). The wp_parse_list() function now converts an integer explicitly instead of relying on that coercion. Behavior is unchanged, and tests are added for integer input. A float or a bool is also coerced today, but neither is meaningful for a list of IDs or slugs, so both are left undocumented.

Developed in https://github.com/WordPress/wordpress-develop/pull/13297.
Follow-up to r44546, r62797, r62835.

See #65817.

#71 @westonruter
13 days ago

In 63367:

Code Quality: Narrow $upgrader type in upgrader skins.

Each WP_Upgrader_Skin subclass is only ever constructed with one specific upgrader, but $upgrader was declared only on the parent as WP_Upgrader. Static analysis therefore flagged the subclass-specific calls to Plugin_Upgrader::plugin_info(), Theme_Upgrader::theme_info() and Language_Pack_Upgrader::get_name_for_update() as calls to undefined methods. Redeclaring the property in Language_Pack_Upgrader_Skin, Plugin_Upgrader_Skin, Plugin_Installer_Skin, Theme_Upgrader_Skin and Theme_Installer_Skin with its narrower type documents what was already true at runtime.

The @since tag on each redeclaration is the version in which that skin class was introduced rather than the current release, since the property has always been available by inheritance and is not newly added here.

With those five occurrences resolved, tests/phpstan/baselines/method.notFound.neon is empty and is removed along with its includes entry in phpstan.neon.dist, and two entries in property.notFound.neon that no longer match are dropped.

Developed in https://github.com/WordPress/wordpress-develop/pull/13283.
Follow-up to r63020.

Props nathanrice, westonruter.
See #65817.

#72 @westonruter
13 days ago

In 63368:

Code Quality: Call private static methods via self::.

The classes WP_Classic_To_Block_Menu_Converter, WP_Navigation_Fallback, WP_Theme_JSON_Resolver, and WP_Font_Face_Resolver reached their own private static helpers through static::. Late static binding is problematic here. Because the helpers are private they are not an extension point, yet static:: still resolves against the called class, so a subclass that declares a method of the same name either hijacks the call when that method is protected or public, or triggers a fatal error when it is private. With self:: the binding of the call goes to the declaring class, which is what the visibility already promises.

In practice the resolution is unchanged, since core always invokes these classes by their own name and none of them is subclassed in core. Only calls to private methods are converted; static:: is retained where it targets a protected or public member and late static binding is meaningful. This clears the 20 remaining staticClassAccess.privateMethod entries, covering 26 call sites, from the PHPStan baseline.

Developed in https://github.com/WordPress/wordpress-develop/pull/13072.
Follow-up to r63020.

Props dpantazis, dhruvang21.
See #65817.

#73 @jorbin
12 days ago

In 63369:

Media: Document PDF alpha channel return value.

Document WP_Image_Editor_Imagick::remove_pdf_alpha_channel() as returning null|WP_Error and explicitly return null on success.

Developed in: https://github.com/WordPress/wordpress-develop/pull/13200

Follow-up to r56271.

Props mazharul78, westonruter.
Fixes #65916. See #39216, #65817.

@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.

#75 @westonruter
12 days ago

In 63378:

Code Quality: Add the documented return null; to 14 functions.

PHPStan reports return.missing where a documented return type obliges a function to return a value on every path, but one path falls off the end instead. Adding the return null; that the annotation already implies resolves the report without changing behavior, since PHP was returning null implicitly anyway. This is the shape r61716 used on get_category_by_path(), which replaced a bare return; in that same function; its remaining path is now handled too.

Covered here are get_category_by_path(), wp_list_users(), single_month_title(), WP_Customize_Manager::get_setting(), get_panel(), get_section() and get_control(), WP_Customize_Widgets::get_setting_type(), WP_Customize_Header_Image_Control::get_current_image_src(), WP_Image_Editor_Imagick::set_imagick_time_limit(), and the four pagination helpers in link-template.php.

Every change is a pure append, leaving the diff additive at 30 insertions and no deletions. Two further groups of return.missing fixes from the same pull request carry more risk and will follow separately.

The baseline tests/phpstan/baselines/return.missing.neon drops from 41 entries covering 44 errors to 27 covering 29, so the file and its entry in phpstan.neon.dist both remain for now.

Developed in https://github.com/WordPress/wordpress-develop/pull/13082.
Follow-up to r61716, r63020.

Props dpantazis, dmsnell.
See #65817.

@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.

#77 @westonruter
12 days ago

In 63379:

Code Quality: Resolve the remaining return.missing PHPStan errors.

Nine template functions take a $display parameter and either echo their result or return it. Each is documented as returning string|null, but the echoing branch fell off the end without returning, which is what PHPStan reports as return.missing. Inverting the condition lets the value-returning path exit first and the echoing path fall through to an explicit return null;, the same shape single_month_title() received in r63378. Affected are 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(). No behavior changes.

WP_Widget::form() is documented as string|void again. r59336 added the void deliberately, noting that unlike its siblings it is the child classes which return nothing when the method is correctly implemented. r62178 replaced it with null on the premise that void cannot belong to a union, which holds for PHP's native return types but not for PHPDoc, where PHPStan reads void in a union as "may not return at all". Under string|null the annotation instead obliged every override to return, putting all 18 of them in breach, two of which live in bundled themes that r62178 did not touch.

With no occurrences left, tests/phpstan/baselines/return.missing.neon is deleted along with its entry in phpstan.neon.dist, completing the work begun in r63378.

Developed in https://github.com/WordPress/wordpress-develop/pull/13082.
Follow-up to r59336, r62178, r63020, r63378.

Props dpantazis, dmsnell.
See #65817, #64704.

#78 @westonruter
12 days ago

In 63381:

Code Quality: Resolve the encapsedStringPart.nonString error.

The get_attachment_fields_to_edit() function documented every value of its returned array as a field definition array, but the attachment_fields_to_edit filter may also add a _final entry holding raw HTML, which get_media_item() interpolates into a string. Declare that structure in a @phpstan-return tag so the _final entry is typed as a string while the remaining keys stay typed as field definition arrays, and widen the plain @return to a union so the array shape remains a proper subtype of it.

The menu_order key is listed explicitly in the shape because get_media_item() unsets it. Unsetting a key that an unsealed array shape does not declare collapses the shape into a plain union of its value types, which would reintroduce the imprecision for every other field.

Developed in https://github.com/WordPress/wordpress-develop/pull/13078.
Follow-up to r61594, r63020.

Props dpantazis.
See #65817.

#79 @westonruter
12 days ago

In 63383:

Users: Document comment_shortcuts and infinite_scrolling properties on WP_User.

Both are user preferences read straight off a WP_User through the magic __get(), exactly like rich_editing and syntax_highlighting beside them, but neither was listed among the class's @property tags, so nothing could resolve them. The infinite_scrolling property arrived in r62632 with the Media Library opt-out option; comment_shortcuts has gone undocumented since r9217 added the comment hotkeys opt-in, and was missing for the same reason. Regenerating the baselines drops both entries from tests/phpstan/baselines/property.nonObject.neon.

Developed as subset of https://github.com/WordPress/wordpress-develop/pull/13064.
Follow-up to r9217, r62632, r63020.

See #65817.

#80 @westonruter
12 days ago

In 63384:

Code Quality: Call WP_Theme_JSON's private statics via self::.

In r52744 this class's internal calls were rewritten from self:: to static:: so that the subclasses it was opening the class up to could override them. For a private method that buys nothing, since a subclass cannot supply one, and PHPStan reports it as unsafe: late static binding resolves static:: to the runtime class, where the private method is not visible. Thirty such call sites were left, reaching fourteen private statics. All fourteen are declared private static on WP_Theme_JSON itself, so self:: is behavior-identical. A docblock example on compute_spacing_sizes() showed the same form and is updated along with the code it documents.

This now eliminates the tests/phpstan/baselines/staticClassAccess.privateMethod.neon baseline. In r63368 the identifier was cleared from the four other classes reporting it, and WP_Theme_JSON was the last.

Developed as subset of https://github.com/WordPress/wordpress-develop/pull/13064.
Follow-up to r52744, r63020, r63368.

See #65817.

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

#82 @swissspidy
9 days ago

In 63404:

Docs: Align hash notation with the values callers pass.

Several @type entries documented a narrower type than what was actually passed.

Developed in: https://github.com/WordPress/wordpress-develop/pull/13235

Props swissspidy, westonruter, mukesh27.
See #65817.

#84 @swissspidy
9 days ago

In 63405:

Docs: Describe the shape of label, capability and setting data.

Several values are documented only as array, stdClass or bool[]. This spells out what they hold: the taxonomy capability object, the capability lists on WP_User and WP_Role, and the setting arrays in register_setting() and get_registered_settings(). The post type labels move from a prose list into the same hash notation the taxonomy labels already use.

Also adds missing keys such as name_admin_bar and template_name, or group for register_setting().

Developed in: https://github.com/WordPress/wordpress-develop/pull/13220

Props swissspidy, westonruter.
See #65817.

@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 @type go?

@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.

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

#91 @swissspidy
8 days ago

In 63419:

Code Quality: Narrow return types for field-plucking functions.

get_post_stati(), get_taxonomies(), get_object_taxonomies(), get_attachment_taxonomies() and get_taxonomies_for_attachments() each return names or objects depending on $output, so this adds a @phpstan-return conditional accordingly.

Developed in: https://github.com/WordPress/wordpress-develop/pull/13307

Props swissspidy, westonruter.
See #65817.

#93 @swissspidy
8 days ago

In 63420:

Build/Test Tools: Teach PHPStan to read hash notation.

Core describes what an array or object holds with hash notation, the nested @type list that PHPStan reads as free text. A parser node visitor now translates each one into the array or object shape it stands for, so the analysis gets the types the documentation already carries. Existing @phpstan-param or @phpstan-return tags take precedence.

Some incorrect/unclear docblocks were fixed in the process.

The GitHub Actions workflow also keys its PHPStan cache on phpstan.neon.dist and the sources in tests/phpstan now to avoid stale results.

Developed in: https://github.com/WordPress/wordpress-develop/pull/13233

Props swissspidy, westonruter, szepeviktor.
See #65817.

#95 @westonruter
8 days ago

In 63426:

Code Quality: Fix PHPStan-identified static analysis regressions from the 7.1 cycle.

These are the errors trunk reports that a baseline generated from 7.0.0's src does not, restricted to those where the symbol whose type provoked the error was itself changed after the 7.0 tag.

Four annotations are corrected:

  • On WP_Comment, $comment_ID and $comment_post_ID widen to numeric-string|int, since the numeric-string added in r62640 is contradicted by get_comment_to_edit(), which has replaced both with integers in place since long before.
  • On term_exists(), the conditional return added in r62680 promises int|null for the empty-$taxonomy branch, which actually returns a cast string.
  • On WP_Block_Type::__get(), the union narrowed in r62178 omits the arrays returned for the variations and uses_context names.
  • On WP_Theme_JSON::get_feature_declarations_for_node(), both parameters have been annotated object since r56058 but only ever receive arrays, and the by-reference $node propagated the wrong type back to callers.

The remaining four are code rather than annotations:

  • The users screen gains the $wpdb global that the queries added in r62688 read without importing.
  • An isset() check on $block_type->selectors is dropped, since the property is declared with a non-null default in r62453.
  • An isset() check paired with ! empty() is dropped in the posts list table, where r62838 left both guarding the same property.
  • In WP_View_Config_Data::merge_properties(), an array() !== $current check is redundant after ! array_is_list( $current ), which already implies non-empty.

One report is suppressed rather than fixed: PHPStan reads substr_compare()'s $length from its pre-8.0 functionMap.php, which dropped the parameter's implicit nullability, so a correct call is reported as invalid on every version WordPress supports. That belongs in ignoreErrors rather than a baseline, since a baseline entry records work still to be done and there is none.

Baselines are regenerated, clearing 49 entries and 81 errors across six files.

Developed in https://github.com/WordPress/wordpress-develop/pull/13064.
Follow-up to r56058, r62178, r62453, r62640, r62680, r62688, r62834, r62838, r63383, r63384.

See #65817.

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 arraywp_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.type report on print_extra_script(), which stems from WP_Dependencies::get_data() returning mixed, restates its narrower expected type.
  • A temporary probe file confirmed all sixteen functions raise function.void when consumed in display mode, and none of them do in retrieval mode.
  • tests/phpstan/baselines/return.missing.neon stays deleted — void in 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_ and Tests_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.

#102 @westonruter
7 days ago

In 63440:

Code Quality: Correct three inverted $display return docs.

WP_Styles::print_inline_style(), WP_Scripts::print_extra_script() and the deprecated WP_Scripts::print_scripts_l10n() each document their return the wrong way round. All three say the markup comes back when $display is true, but the string is returned on the ! $display branch and the printing branch returns true. The wording has read this way since r36744, so anyone consulting it to decide which argument to pass was told the opposite of what the code does.

Swapping true for false in the three descriptions corrects that, and a conditional @phpstan-return pins the two behaviors apart, since the plain unions collapse the distinction. print_inline_style() now resolves to bool when printing and string|false when retrieving, rather than string|bool either way, and print_extra_script() to true|null and string|null rather than bool|string|null. The narrower retrieval types matter at the two internal call sites that pass false and then use the result as a string.

WP_Scripts::print_inline_script() and print_translations() are left alone. Both print and then return the same value, so their existing string|false is accurate in either mode and there is nothing for a condition to separate.

Developed as subset of https://github.com/WordPress/wordpress-develop/pull/13359.
Follow-up to r36744, r62178.

Props apermo, westonruter.
See #65817.

#103 @westonruter
7 days ago

In 63441:

Code Quality: Restore void on the dual-mode template tag.

Many template tag functions either print their result or return it, depending on a display or echo param. Their @return carried void in a union until r61766, r61768 and r62178 replaced it with null, on the premise that void cannot belong to a union type. That premise holds for PHP's native return types but not for PHPDoc, where PHPStan reads void in a union as "may not return at all".

Restoring it is not enough on its own, though. PHPStan raises the "Result of function … (void) is used." error only when the resolved return type is exactly void, which a union never is, so the tags that kept void were getting no more out of it than the ones converted to null. What carries the distinction is a conditional @phpstan-return, resolving to plain void when the tag prints and to the type it returns otherwise. Thirty-one functions gain one, including one in the bundled Twenty Twenty theme, each with a single void branch and a nullable retrieval branch. The trailing return null; statements added in r63378 and r63379 are removed; they existed only to satisfy return.missing, which does not apply once void is in the union. Where the flag lives in an $args array the condition has to match every falsy spelling of it, since these tags variously default it to true or to 1. A call whose flag PHPStan cannot see, such as a query string or an array built at runtime, does not resolve to plain void, so it is never reported.

Four descriptions are corrected alongside: next_posts(), previous_posts() and wp_register() promised a link where an empty string is possible, and wp_dropdown_languages() prints and then returns the markup rather than choosing between the two. Tags returning a meaningful value on a path shared by both modes cannot resolve to plain void and are left alone, among them single_month_title(), wp_list_categories() and wp_nav_menu().

Developed as subset of https://github.com/WordPress/wordpress-develop/pull/13359.
Follow-up to r32568, r61766, r61768, r62178, r63378, r63379, r63440.

Props marian1, westonruter, apermo.
See #65817, #64704.

@marian1 commented on PR #13359:


7 days ago
#104

I was only able to have a quick look.

  • > @return string|void and @return void|string carry no more information than string|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, void still conveys semantic information: if the function returns the value null, 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 ''|array branch 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 to wp_list_comments(), wp_list_bookmarks(), wp_list_authors(), paginate_comments_links(), the_title_attribute(), wp_list_pages(), wp_page_menu(), and wp_list_users().
  • wpdb::print_error() also returns null at the bottom of the method. I don't think this can be changed back to void simply by adding a conditional return type. It might still be worth documenting it here alongside the other related functions. Although void cannot 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|void tag also include null, given the semantic distinction between void and null?
  • 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 to edit_term_link().
  • wp_dropdown_languages(): the description is incorrect. It says that "nothing is returned when the required id or name argument 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_echo on trackback_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.

#105 @westonruter
7 days ago

In 63442:

Posts, Post Types: Rebuild meta capabilities on unregistration.

The $post_type_meta_caps registry is keyed by custom capability name, so a single entry may be owed to any number of registered post types. The WP_Post_Type::remove_rewrite_rules() method removed entries by subtracting every value of the unregistered post type's own $cap object. That deleted mappings which other, still-registered post types sharing a capability type continue to depend on. It also deleted entries for post types registered with map_meta_cap set to false, which never stored any to begin with. Finally, it treated primitive capabilities as meta capabilities, since only the read, delete and edit capabilities are ever stored.

Once a mapping is gone, map_meta_cap() returns the meta capability verbatim rather than mapping it down to primitives. The ownership and post status checks (edit_others_*, edit_published_*, edit_private_*) do not become stricter, they disappear from the result entirely, so a post's own author can lose edit access to it.

Subtraction cannot work against a shared registry, as nothing records which post types an entry is owed to. Remove the cleanup from remove_rewrite_rules(), leaving that method to do only what its name says, and rebuild the registry from the post types that remain in unregister_post_type(), once the post type has been removed from $wp_post_types. Rebuilding inherits the rules of _post_type_meta_capabilities() instead of trying to invert them, so all three defects are addressed at once: entries are written only for post types with map_meta_cap set to true, only for the three meta capabilities, and an entry shared by several post types survives as long as any one of them is still registered.

Removing the loop also empties the foreach.nonIterable PHPStan baseline, so that file and its includes entry are deleted. The docblock of _post_type_meta_capabilities() is corrected alongside: in r36316 the branch that returned the list when the function was called with no arguments was removed, but the null parameter default and a summary describing that return value were left behind.

Developed in https://github.com/WordPress/wordpress-develop/pull/13342.
Follow-up to r15890, r36316, r37890.

Props westonruter, dmsnell, dpantazis.
See #65817.
Fixes #66008.

@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|voidsingle_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 plain void on 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. The wp_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=HEAD is clean on every commit, as enforced by the pre-commit hook.
  • PHPCS reports no new errors. The warnings on general-template.php are the pre-existing $wpdb->prepare() ones in wp_get_archives(), on untouched lines.
  • PHPUnit Tests_DB passes for the wpdb change: 651 tests, 985 assertions, 2 skipped. Removing the explicit return null; is a no-op at runtime.
  • tests/phpstan/baselines/return.missing.neon stays deleted — void in 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|void and @return void|string carry no more information than string|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, void still conveys semantic information: if the function returns the value null, 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 ''|array branch 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 to wp_list_comments(), wp_list_bookmarks(), wp_list_authors(), paginate_comments_links(), the_title_attribute(), wp_list_pages(), wp_page_menu(), and wp_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 returns null at the bottom of the method. I don't think this can be changed back to void simply by adding a conditional return type. It might still be worth documenting it here alongside the other related functions. Although void cannot 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|void tag also include null, given the semantic distinction between void and null?

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 to edit_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 required id or name argument 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_echo on trackback_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_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

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 void with null was 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 having get_search_form() and print_search_form() instead of using $args['echo'] = true to turn get_search_form() into a "convenience" wrapper for echo 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 void with null was 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 having get_search_form() and print_search_form() instead of using $args['echo'] = true to turn get_search_form() into a "convenience" wrapper for echo 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_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

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.

#114 @westonruter
6 days ago

In 63451:

Code Quality: Tighten the remaining documented types for hooks.

Introduce a Maybe_Callable PHPStan type alias for a callback that may or may not be defined in the current scope, and use it for the $callback parameters of has_filter(), remove_filter(), has_action(), remove_action(), _wp_filter_build_unique_id(), and the corresponding WP_Hook methods. This captures in one named type what these functions have always accepted, which is deliberately wider than callable.

Document the collected hook arguments as list<mixed>, narrowed to non-empty-list<mixed> for apply_filters_ref_array() and apply_filters_deprecated(), both of which read index 0 unconditionally. The do_action variants stay list<mixed>, since nothing on that path reads index 0 and an empty array is a legitimate argument: wp_schedule_event() and wp_schedule_single_event() both default $args to an empty array, and every such event reaches do_action_ref_array() with one.

Add @no-named-arguments to apply_filters() and do_action() so that spreading an associative array into them is reported. As of PHP 8.1 that syntax passes named arguments, which collect into an associative $args and lose the positional mapping the callbacks expect; in PHP 7 the keys were simply ignored.

In WP_Hook::has_filter(), pass an integer priority to _wp_filter_build_unique_id() rather than false, matching its documented int $priority parameter. The value is unused when building the key, so this is behavior-neutral, and it retires the corresponding entry from the PHPStan baseline. In WP_Hook::apply_filters(), assign the result of current() to a local variable and bail from the loop in the impossible case that it returns false, before storing it. This keeps $current_priority honestly typed as a list of integers.

Developed in https://github.com/WordPress/wordpress-develop/pull/12443.
Follow-up to r38571, r52300, r62733.

Props westonruter, johnbillion, sergeybiryukov, swissspidy.
See #64896, #65817.

@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.

@westonruter commented on PR #13371:


5 days ago
#118

https://phpstan.org/r/9d3e9efd-95ea-4eb0-8067-ecdb9b158fcf

@IanDelMar Ah, this is because you configured with treatPhpDocTypesAsCertain enabled. This is set to false in core:

https://github.com/WordPress/wordpress-develop/blob/f19cdb15409d7dcaa04f51d476538eb4d9bb248b/tests/phpstan/base.neon#L74-L75

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 .cache with 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:baselines regenerating every identifier leaves tests/phpstan/baselines and phpstan.neon.dist byte-identical.
  • A generator run no longer touches .cache/resultCache.php or .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

https://claude.ai/code/session_01EdFn9fuiUhJMXzacuhmdGJ

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/baselines byte-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

https://claude.ai/code/session_01EdFn9fuiUhJMXzacuhmdGJ

#121 @westonruter
5 days ago

In 63460:

Build/Test Tools: Isolate the PHPStan baseline generation cache.

What a baseline records has to follow from core and the committed dist configuration alone, which is why the phpstan:baselines script pins the configuration it analyzes rather than reading whatever local phpstan.neon override a developer keeps beside phpstan.neon.dist. Its analysis nevertheless ran in the cache directory every other PHPStan run uses, and what a run leaves there depends on how that run was scoped (e.g. analyzing the entire project versus a single file or directory).

What PHPStan stores for a file it has read is not settled by that file's contents: the extensions in tests/phpstan apply to the files an analysis is scoped to rather than to every file it reads, so a run over a subset stores readings a full run would not. Sharing a directory with those runs, a regeneration read them back and recorded entries a clean analysis does not report.

Give the analysis .cache/baselines, a directory only this script writes to and only ever from a full run, suffixed with the configuration whenever --config names one other than the default.

Developed in https://github.com/WordPress/wordpress-develop/pull/13395.
Follow-up to r63019, r63420.

See #65817.

#122 @westonruter
5 days ago

In 63462:

Build/Test Tools: Run the PHPStan node visitors on every parse.

A visitor tagged phpstan.parser.richParserNodeVisitor runs in PHPStan's RichParser, and PathRoutingParser sends a file there 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. That routing is safe for PHPStan's own parsers, which agree about everything reflection exposes, but the HashNotationVisitor added in r63420 rewrites @param, @return and @var docblocks, which are exactly what reflection exposes. Analyzing a subset of the tree therefore reports against types the visitor would have replaced wherever one crosses a file boundary, and writes that reading into the shared cache for the next full run to restore.

Have the simple parser wrap the rich one, so every parse rewrites the same docblocks, the cleaning still happens on top of it, and the reflection cache is keyed on contents correctly again. This redefines PHPStan's own currentPhpVersionSimpleParser service, which is not a documented extension point; the definition records what to check should a future release rename it. Full runs report exactly what they did before, and every baseline still matches. A run over a subset now does the parsing it previously skipped, so a cold cache pays a few percent for it, while a warm one is unchanged.

Developed in https://github.com/WordPress/wordpress-develop/pull/13396.
Follow-up to r63420, r63460.

Props westonruter, swissspidy.
See #65817.

#123 @westonruter
4 days ago

In 63486:

Build/Test Tools: Regenerate PHPStan baselines for property.onlyWritten.

The sole property.onlyWritten error identified by PHPStan was fixed without the baselines being regenerated.

Follow-up to r63485.

See #65817, #65818.

#124 @westonruter
4 days ago

In 63487:

Code Quality: Refine the dual-mode template tag annotations.

  • Restore void to the @return unions of wpdb::print_error() and wpdb::check_database_version(), which r62177 replaced with null.
  • Add the null that fifteen tags describe in prose but omit from a @return listing only string|void.
  • Match every falsy spelling of a display flag, and every argument shape reaching a bail, so those calls resolve to void; a query string still cannot be read, tracked in #66049.
  • Narrow the deprecated param for trackback_url() so that static analysis catches the obsolete usage.

Developed in https://github.com/WordPress/wordpress-develop/pull/13371.
Follow-up to r32568, r61766, r61768, r62177, r62178, r63440, r63441.

Props marian1, westonruter.
See #64703, #64704, #65817, #66049.

#125 @westonruter
4 days ago

In 63488:

Taxonomy: Fix wp_get_object_terms() when requesting a count.

Requesting a count passed the numeric string get_terms() returns straight to array_merge(), which warns and yields null on PHP 7.4 and throws a TypeError on PHP 8.0 and later. The count is now cast before it is merged, and summed once every taxonomy has been queried, so the function returns the numeric string its documentation has described since r49947. An empty object or taxonomy list returns '0' for the same reason, rather than the empty array that contradicted the documented type.

A taxonomy registered with an args array is queried by a separate recursive call, and those results were merged with array_merge() unconditionally. For the id=> values of fields the term IDs are integer array keys, which array_merge() renumbers, so such a taxonomy came back keyed from zero. The recursive branch now uses the union operator for those, as the merge for the remaining taxonomies has done since r41809.

The functions get_terms(), wp_get_object_terms(), wp_count_terms(), wp_get_post_categories(), wp_get_post_tags() and wp_get_post_terms() each gain a conditional return type that resolves the result from the fields value, falling back to what that function's own default answers with, as was done for WP_Term_Query::query() in r63358.

Developed in https://github.com/WordPress/wordpress-develop/pull/7278.
Follow-up to r38667, r40513, r41809, r49947, r63358.

Props marian1, westonruter, swissspidy.
See #65817, #66049.
Fixes #61936.

#126 @westonruter
2 days ago

In 63510:

Build/Test Tools: Update PHPStan to 2.2.13.

Recent PHPStan releases bring substantial performance improvements. The baselines are regenerated for the new version.

The docblock of _wp_die_process_input() is also corrected to admit the integer message that wp_die() accepts for the legacy Ajax responses, which the helper passes through untouched. This removes a baseline entry for a guard in _default_wp_die_handler() that the incomplete type made look unreachable.

Developed in https://github.com/WordPress/wordpress-develop/pull/13410.
Follow-up to r44666, r53144, r62703, r62798.

Props markusstaab, westonruter.
See #65817.
Fixes #66051.

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:484
  • wp-includes/class-wp-comment-query.php:455, :853, :1051
  • wp-includes/class-wp-customize-panel.php:227
  • wp-includes/class-wp-customize-section.php:238
  • wp-includes/class-wp-site-query.php:352
  • wp-includes/class-wp-term-query.php:1183
  • wp-includes/comment.php:2981
  • wp-includes/theme.php:2572, :3557, :3558
  • wp-includes/user.php:938, :1773
  • wp-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:1395
  • wp-includes/widgets/class-wp-widget-media-audio.php:157
  • wp-includes/widgets/class-wp-widget-media-gallery.php:145
  • wp-includes/widgets/class-wp-widget-media-image.php:320
  • wp-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

https://claude.ai/code/session_01NtH5qFmG4hJcGWDCx5tMY7

#128 @westonruter
2 days ago

In 63511:

Code Quality: Improve typing for wp_array_slice_assoc().

The @param and @return tags have documented a bare array since the function was introduced in r15574, so nothing was known about the slice it returns. They now describe the string-keyed arrays the function accepts, PHPStan generics bind the returned key set to the $keys argument so that a call passing a known list of keys resolves to those keys rather than to string. The signature also gains a native array return type.

Developed in https://github.com/WordPress/wordpress-develop/pull/13417.
Follow-up to r15574.

See #65817, #65860.

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 : array return 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

#130 @westonruter
33 hours ago

In 63522:

Code Quality: Improve typing for wp_parse_args().

Both the parameters and the return value have been documented as a bare array since the function entered trunk in r5234. They now describe the string-keyed arrays it is used with, and the signature gains a native array return type. Every caller treats those keys as strings, but the function cannot guarantee it, since get_object_vars() reports a numeric-string property under an integer key and parse_str() reads 0=a as one, so the @phpstan-return records what can actually come back.

Developed in https://github.com/WordPress/wordpress-develop/pull/13430.
Follow-up to r5234, r47429.

See #65817, #65860.

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

#136 @westonruter
26 hours ago

In 63524:

Code Quality: Combine registered block styles with array_merge().

In WP_REST_Block_Types_Controller the block styles registered for a block are combined with array_merge(), where wp_parse_args() had been used erroneously on what are two lists. In support of this, a global Block_Style_Properties type alias is introduced for the shape of a registered block style.

Developed in https://github.com/WordPress/wordpress-develop/pull/13432.
Follow-up to r48173, r59760, r63522.

Props westonruter, swissspidy.
See #65817, #65860.

@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 $args is provably an array or object, the string branch is modeled here as a plain array, with its keys *not* narrowed — parse_str() reads 0=a as an integer key, and the wp_parse_str filter it ends with documents a plain array. Since array_merge( $defaults, <any array> ) still guarantees the defaults' keys are present, the result stays sound, and it reaches the ~36 call sites in src/ whose $args is documented array|string.
  • Defaults that may or may not be an array are unioned rather than declined. Where #309 returns null unless $defaults is provably an array, both branches are combined here: a non-array $defaults returns 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.

  1. 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
  2. I do not support using URL query string-like parameters
  3. 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-wordpress is added to require-dev (^2.0.4).
  • Its extension.neon is not included. That file bootstraps php-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.neon registers 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 phpstan on the CI configuration is green. The baselines gain 20 hook docblock findings and one get_posts() argument now visible through shortcode_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 stringnon-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:

  1. HookCallbackRule reports under PHPStan's own identifiers (arguments.count, return.void, return.missing), and HookDocsRule under parameter.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, as WpConstantFetchRule has, would fix both; a parameter to disable the action-return check would be nicer still.
  2. HookDocBlock could resolve "This filter is documented in" reference comments the way core's does. Core's implementation is in tests/phpstan/HookDocBlock.php and could move upstream.
  3. The wp_parse_args() extension from #13433 could be contributed to the 2.x branch.

## Follow-ups in core

  • Fix the 19 hook docblocks now baselined under parameter.phpDocType.
  • Add false to the component form of wp_parse_url()'s @phpstan-return, and non-falsy-string to trailingslashit().
  • The get_posts() call in the gallery shortcode passes include as a string.
  • The $accepted_args change to block-style-variations.php is 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:

  1. Support for static query strings like 'echo=0'.
  2. A rule forbidding non-array $args like 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.

#144 @westonruter
13 hours ago

In 63540:

Code Quality: Narrow the esc_sql() parameter and return types.

Narrow the documented types of esc_sql() and wpdb::_escape() from string|array to string|string[], since neither function is meant to operate on anything other than strings and flat arrays of strings.

Add @phpstan- prefixed annotations with a key template and a conditional return type, so static analysis knows that a string yields a string and that an array yields an array of strings with its keys preserved.

Developed in https://github.com/WordPress/wordpress-develop/pull/12975.
Follow-up to r24986, r62672.

Props johnbillion, westonruter, irozum.
See #65817.

#145 @westonruter
12 hours ago

In 63542:

Code Quality: Remove redundant boolean sub-expressions.

Each of three conditions re-tests something the surrounding expression has already established, so PHPStan reports the redundant operand as always true:

  1. In wp_render_typography_support(), ! empty() already requires the fitText attribute to be truthy, so a following truthiness check on the same value can never fail.
  2. In _get_block_templates_files(), the right operand of an || is only evaluated when ! $post_type was false, so a leading $post_type && there is always true.
  3. In Walker::display_element(), $newlevel is a local variable that is only ever assigned the literal true, so the truthiness test adds nothing to the isset().

All three are simplifications with no change in behavior. This resolves two booleanAnd.rightAlwaysTrue occurrences and one booleanAnd.leftAlwaysTrue occurrence, and the corresponding PHPStan baselines are regenerated.

Developed in https://github.com/WordPress/wordpress-develop/pull/13086.
Follow-up to r55687, r61246, r63023.

Props tstokes8040.
See #65817.

@westonruter commented on PR #13433:


11 hours ago
#146

  1. 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.

Note: See TracTickets for help on using tickets.