Make WordPress Core

Opened 6 weeks ago

Closed 6 weeks ago

#66051 closed enhancement (fixed)

Update PHPStan

Reported by: markusstaab Owned by: westonruter
Priority: normal Milestone: 7.2
Component: General Version:
Severity: normal Keywords: has-patch has-unit-tests
Cc: Focuses:

Description

update PHPStan to unlock latest performance improvements

Change History (6)

This ticket was mentioned in ​PR #13410 on ​WordPress/wordpress-develop by ​staabm.


6 weeks ago
#1

  • Keywords has-patch has-unit-tests added

at PHPStan we worked hard to improve performance on recent releases. Updating the tooling makes sure Wordpress can use latest additions and get the best possible PHPStan experience

before this PR (PHPStan 2.2.5)

time composer phpstan
197.13s user 10.28s system 667% cpu 31.084 total

after this PR (PHPStan 2.2.13)

time composer phpstan
112.30s user 5.09s system 981% cpu 11.964 total

running on Macbook M4-Pro, PHP 8.5.10 (cli)

https://core.trac.wordpress.org/ticket/66051

​@markusstaab commented on ​PR #13410:


6 weeks ago
#2

Contributors, please ​read how to link your accounts to ensure your work is properly credited in WordPress releases.

done

#3 @SergeyBiryukov
6 weeks ago

  • Milestone Awaiting Review → 7.2

#4 @westonruter
6 weeks ago

  • Owner set to westonruter
  • Status new → reviewing

​@westonruter commented on ​PR #13410:


6 weeks ago
#5

🤖 _Comment by Claude Fable 5.1_

The baseline changes were analyzed to understand why each error appeared or disappeared between PHPStan 2.2.5 and 2.2.13. Both versions were run on the affected files with the baselines stripped, the output was diffed, each difference was bisected across 2.2.6 through 2.2.12, and each was reduced to a standalone repro.

## Summary

Baseline change Release Cause Which side is right?
Removed: if.alwaysFalse in load.php 2.2.6 Negative function_exists() narrowing is now forgotten after require/include New (the old report was a false positive)
Added: 6th WP_Theme::$version in class-wp-debug-data.php 2.2.6 2.2.5 bug: a remembered narrowing on a variable hid a later fetch of the same expression New
Removed: is_wp_error() always true in ajax-actions.php 2.2.7 Impossible-check rules now honor treatPhpDocTypesAsCertain: false New
Removed: is_string() always true in block-editor.php 2.2.7 Same New
Added: 4 × is_string()/is_scalar() in functions.php 2.2.7 list() destructuring makes a phpdoc-only type look native Debatable, likely an upstream bug
Removed: empty($title) in plugin.php 2.2.8 Same phpdoc-certainty fix, for empty() New
Code change: esc_xml() guard 2.2.8 isset() inside callbacks now reaches the impossible-check rules New
Added: curl_getinfo()/finfo_file() argument.type 2.2.10 Two functionMap entries were deleted, so stub signatures with a native resource parameter now apply Inconsistent rather than wrong

## Removed errors

load.php line 863. The check is function_exists( 'wp_cache_init' ) right after require_once of the object-cache drop-in, inside a block guarded by the negated check. 2.2.5 carried the negative narrowing across the require and reported the inner check as always false. 2.2.6 forgets function_exists()/class_exists() narrowings when global state may change (​https://github.com/phpstan/phpstan-src/pull/5998) and invalidates volatile expressions on include (​https://github.com/phpstan/phpstan-src/pull/6009). The repro confirms the report vanishes only when a require sits between the two checks.

The three phpdoc-only checks. Core sets treatPhpDocTypesAsCertain: false, which is supposed to suppress "always true" reports that rest solely on docblock types. All three qualify: the term update result is typed only by the wp_update_term() docblock, the preload paths come from the hook docblock via the apply_filters() return type extension, and $title is a @global string. 2.2.5 reported them anyway. With the setting flipped to true, all three come back under 2.2.13, which confirms the mechanism. The 2.2.7 and 2.2.8 release notes describe the rework of the constant-condition and impossible-check machinery that fixed this.

## Added errors

The sixth WP_Theme::$version report. $theme is WP_Theme at both sites under both versions, so this is not a type change. A structural bisect showed 2.2.5 hid the report only when both the earlier read into $theme_version and the ! empty( $theme_version ) checks were present. Minimal repro:

$v = $t->version;
if ( ! empty( $v ) ) { }
$w = $t->version; // silent in 2.2.5, reported in 2.2.13; reported in both without the empty() check

Narrowing the variable also narrowed the remembered expression, and the later fetch used that entry without reaching the undefined-property rule. This is not called out in the 2.2.6 notes; it fell out of the expression-handling fixes.

The four wp_die() handler checks. Each follows list( $message, ... ) = _wp_die_process_input(), whose hash-notation return becomes array{string, string, array} through core's HashNotationVisitor. That is a phpdoc-only type, so under core's settings it should not be reported, and a direct $message = $result[0] indeed is not. Only list() destructuring triggers it, in every version from 2.2.7 on. That is the opposite direction from the certainty fix above and looks like an upstream bug worth filing.

curl_getinfo() and finfo_file(). In 2.2.10, ​https://github.com/phpstan/phpstan-src/pull/6243 deleted exactly these two functions from PHPStan's functionMap. Their signatures now come from the PhpStorm stubs, whose version-aware parameter resolves to a native resource on the PHP 7.4 floor, and that rejects the false half of what curl_init() and finfo_open() return. The functionMap-sourced resource parameter accepted the same union in both versions, even with treatPhpDocTypesAsCertain: true. That is why the sibling calls on the same handle, such as curl_exec(), curl_setopt() and finfo_close(), stay silent: they still have functionMap entries. So the reporting is inconsistent, but the underlying observation is real, since core never checks either function for failure. The benevolent-union argument in ​https://github.com/phpstan/phpstan/issues/15185 would not cover finfo_open(), whose return type is a plain resource|false rather than a benevolent union.

esc_xml(). The $matches shape is identical in 2.2.5, so what changed in 2.2.8 is that isset() inside a callback closure now reaches the rules, via the new isset descriptors (​https://github.com/phpstan/phpstan-src/pull/6144) and running the impossible-check rules on post-processing virtual nodes (​https://github.com/phpstan/phpstan-src/pull/6157). Removing the guard is correct; offset 0 always exists in a preg_replace_callback() callback.

#6 @westonruter
6 weeks ago

  • Resolution → fixed
  • Status reviewing → closed

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.

Note: See TracTickets for help on using tickets.