Coding Standards: Use array_last() to get the last array element. - #12417
Coding Standards: Use array_last() to get the last array element.#12417Soean wants to merge 7 commits into
Conversation
Replace `$array[ count( $array ) - 1 ]` constructs with the `array_last()` function (added in PHP 8.5, polyfilled in compat.php) for improved readability.
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
There was a problem hiding this comment.
Pull request overview
This PR updates several core code paths to use PHP’s array_last() helper (polyfilled via wp-includes/compat.php) instead of manual count( $array ) - 1 indexing, improving readability and aligning with modern PHP idioms.
Changes:
- Replaced manual “last element” indexing with
array_last()across multiple files. - Updated trackback parsing, Theme JSON element-path handling, WP_Query author slug parsing, block parser stack access, and Ajax tag search parsing to use
array_last().
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/wp-trackback.php | Uses array_last() when deriving the post ID segment from the request URI. |
| src/wp-includes/class-wp-theme-json.php | Uses array_last() to obtain the current element from metadata paths. |
| src/wp-includes/class-wp-query.php | Uses array_last() for the last author slug segment when parsing nice URLs. |
| src/wp-includes/class-wp-block-parser.php | Uses array_last() to retrieve the current parent from the parsing stack. |
| src/wp-admin/includes/ajax-actions.php | Uses array_last() to grab the final comma-delimited search token. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if ( array_last( $query_vars['author_name'] ) ) { | ||
| $query_vars['author_name'] = array_last( $query_vars['author_name'] ); // No trailing slash. |
|
Nice thing about this is it is resilient against the possibility of not getting the actual last item in the array if the array is not a |
Store the result of `array_last()` in a temporary variable instead of calling it twice within the same branch when parsing the author slug from a nice URL. This avoids the repeated (potentially polyfilled) function call and makes the intent clearer.
Co-authored-by: Weston Ruter <westonruter@gmail.com>
| $parent = array_last( $this->stack ); | ||
| if ( ! $parent ) { | ||
| throw new LogicException( 'The parser stack must not be empty when adding an inner block.' ); | ||
| } |
There was a problem hiding this comment.
A fatal error would have ensued anyway when attempting to do $parent->block->innerBlocks[] = (array) $block when $parent is null in PHP 8: https://3v4l.org/J1qGg#veol
So this just makes PHP 7 behave consistently with PHP 8.
There was a problem hiding this comment.
Nevertheless. There is no @throws tag being added. I guess simpler to just use array_key_last() for now.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
src/wp-includes/class-wp-block-parser.php:348
- This refactor introduces a new explicit exception path (
throw new LogicException) inadd_inner_block(), which changes runtime behavior and goes beyond the PR’s stated “coding standards / readability” scope. It also makes the block parser less “best-effort” (seeparse()docblock noting it does not return an error on invalid inputs). Since the previous code already relied on callers ensuring a non-empty stack, consider keeping the change strictly to usingarray_last()and preserving the prior behavior (letting PHP handle the invariant violation).
$parent = array_last( $this->stack );
if ( ! $parent ) {
throw new LogicException( 'The parser stack must not be empty when adding an inner block.' );
}
…block(). Replace the `array_last()` call and its accompanying `LogicException` guard with `$this->stack[ array_key_last( $this->stack ) ]`. Both internal call sites are already guarded against an empty stack, so the exception was unreachable from core. Since `add_inner_block()` is a public method of a class that can be replaced via the `block_parser_class` filter, throwing a new exception type would have changed behavior for third-party callers, which is out of scope for a coding standards change. Indexing by `array_key_last()` keeps the readability improvement without introducing a nullable into the expression. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Replace the two remaining `$stack[ count( $stack ) - 1 ]` reads in `Plural_Forms::parse()` with `array_last( $stack )`. Both sites sit inside `while ( ! empty( $stack ) )` loops, so the stack is guaranteed non-empty and the value read is unchanged. `$stack` is only ever appended to with `$stack[] = ...` and shortened with `array_pop()`, so its keys stay sequential and `array_last()` returns the same element as indexing by `count() - 1`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…posts(). Read the exploded author name into `$author_name_parts` instead of temporarily storing the array in `$query_vars['author_name']`, so that query var only ever holds a string, and both branches read from the same local. Behavior is unchanged. `$query_vars` is a reference to `$this->query_vars` and feeds `generate_cache_key()`, so the value it ends up with matters: both branches still assign it exactly as before, and only the intervening array assignment -- which every path overwrote two lines later -- is gone. The loose `if ( $last_part )` check is kept deliberately; a strict `'' !== $last_part` would change the result for an author name such as `a/0`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
src/wp-includes/class-wp-block-parser.php:345
- This line switches to
array_key_last()+ indexing, but the PR’s stated goal is to replace “get last element” patterns witharray_last(). Sincearray_last()is already polyfilled inwp-includes/compat.php, this can be simplified and made consistent with the rest of the PR by usingarray_last( $this->stack )directly.
$parent = $this->stack[ array_key_last( $this->stack ) ];
Replace
$array[ count( $array ) - 1 ]constructs with thearray_last()function (added in PHP 8.5, polyfilled in compat.php) for improved readability.Trac ticket: https://core.trac.wordpress.org/ticket/64897
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.