Repository navigation
Media: Use set_param() to assign REST request parameters - #86110
noruzzamans wants to merge 1 commit into
Conversation
🤖 PR meta 🤖🎉 PropsIf you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. Props Bot commandsYou can trigger Props Bot actions by commenting on this PR:
Updated as activity occurs, without notifying anyone named here. Add the |
westonruter
left a comment
There was a problem hiding this comment.
It's good that this was opened because it highlights a problem with the underlying WP_REST_Request typing.
So I'm marking with "request changes" so we can fix the underlying problem.
🤖 Analysis by Claude Opus 5.5 · medium effort:
The switch from $response_request['context'] = … to $response_request->set_param( 'context', … ) avoids the errors, but it looks like a workaround for a PHPStan limitation that the stub signature triggers. Nothing is wrong with the code itself. With the baseline entries removed and the change reverted, PHPStan 2.3.4 with php-stubs/wordpress-stubs 7.2.0-beta3 reports:
- line 1737:
Cannot assign offset 'context' to WP_REST_Request<array<string, mixed>>.(offsetAssign.dimType) andWP_REST_Request<array<string, mixed>> does not accept string.(offsetAssign.valueType) - line 1740:
Cannot assign offset '_fields' to WP_REST_Request<array<string, mixed>>.(offsetAssign.dimType)
Cause
In the stubs, WP_REST_Request::offsetSet() has a method-level template, the same as set_param():
/**
* @phpstan-template TOffset of key-of<T>
* @phpstan-param TOffset $offset
* @phpstan-param T[TOffset] $value
*/
public function offsetSet($offset, $value)The two forms are checked differently:
->set_param( 'context', 'edit' )is analyzed as a method call.TOffsetis inferred from the argument as'context', soT['context']resolves tomixedand the call passes.$request['context'] = 'edit'is analyzed byObjectType::setOffsetValueType(). That method readsoffsetSet()'s parameters throughgetOnlyVariant()and never resolves the method template against the actual offset and value. The accepted offset type stays as the unresolvedTOffset, and the accepted value type stays asT[TOffset].isSuperTypeOf()on an unresolved template type returns maybe, not yes, so the assignment is rejected. Line 1740 gets only thedimTypeerror because its value ($request['_fields']) ismixed, and the value check skipsmixed.
The same behavior shows up in a minimal reproduction without WordPress. A class implementing ArrayAccess<key-of<T>, value-of<T>> whose offsetSet() has @template TOffset of key-of<T> gives the same three errors, while its identically typed set_param() passes. The same class with offsetSet() typed as plain @param key-of<T> $offset / @param value-of<T> $value (no method template) gives no errors.
Suggested direction
Array-access assignment on WP_REST_Request is an idiom used throughout core, so it seems better to fix this upstream than to change call sites. Either of these would work:
- PHPStan: resolve
offsetSet()'s method template inObjectType::setOffsetValueType()(and probably the matching read path), for example withParametersAcceptorSelector::selectFromTypes()and[ $offsetType, $valueType ]instead ofgetOnlyVariant(), so array-access writes are inferred like a method call. - wordpress-stubs: drop the method-level template from
offsetSet()and usekey-of<T>/value-of<T>directly. This gives up per-key value precision on array-access writes, but it works with PHPStan today.
Until then, it may be preferable to keep the baseline entries, or a targeted ignore that links to the upstream issue, and leave the array-access syntax unchanged.
Separately, the WP_REST_Request class docblock in the stubs has two @phpstan-implements tags (ArrayAccess<string, mixed> and ArrayAccess<key-of<T>, value-of<T>>), and they appear before the @phpstan-template declaration. That doesn't cause these errors, but it's worth tidying up in the stubs at the same time.
| $response_request['context'] = 'edit'; | ||
| $response_request->set_param( 'context', 'edit' ); | ||
|
|
||
| if ( isset( $request['_fields'] ) ) { | ||
| $response_request['_fields'] = $request['_fields']; | ||
| $response_request->set_param( '_fields', $request['_fields'] ); |
There was a problem hiding this comment.
While these changes are fine and resolve the errors, they're ultimately a workaround for a deeper problem.
|
🤖 Comment by Claude Opus 5.5 · medium effort The switch from
CauseIn the stubs, /**
* @phpstan-template TOffset of key-of<T>
* @phpstan-param TOffset $offset
* @phpstan-param T[TOffset] $value
*/
public function offsetSet($offset, $value)The two forms are checked differently:
This is a long-standing PHPStan bug, already reported as phpstan/phpstan#8549 with the same Upstream fixA fix has been proposed in phpstan/phpstan-src#6903. It makes array-access writes resolve The fix also lets array-access writes be checked per key once SuggestionArray access on |
|
Thank you for looking into this and linking the upstream PHPStan fix (phpstan/phpstan-src#6903). Agreed that preserving the idiomatic array-access syntax and waiting for the upstream PHPStan release is the right approach here. Closing in favor of the upstream fix. |
|
For a stop-gap solution: |
What?
See #86088
Replaces array offset assignments on a WP_REST_Request instance with set_param() in WP_REST_Attachments_Gutenberg_Controller, and removes the resolved entries from the PHPStan baseline.
Why?
WP_REST_Request implements ArrayAccess by delegating offsetSet() directly to set_param(). However, PHPStan treats direct array offset assignment on WP_REST_Request as invalid (offsetAssign.dimType and offsetAssign.valueType). Using set_param() directly resolves 3 errors initially baselined in #66693.
How?
lib/media/class-gutenberg-rest-attachments-controller.php, replaced$response_request['context'] = 'edit'and$response_request['_fields'] = ...with$response_request->set_param(...).offsetAssignentries fromtools/phpstan/baseline.neon.Testing Instructions
composer phpstanand confirm it passes with no errors.composer phpstan:baselinesand confirmtools/phpstan/baseline.neonhas no further changes.Testing Instructions for Keyboard
Not applicable. There are no UI changes.
Screenshots or screencast
Not applicable.
Use of AI Tools
I used Claude Code to identify the affected occurrences and draft the change. I reviewed the diff and take responsibility for it.