Skip to content

Code Quality: Remove three redundant boolean sub-expressions - #13086

Open
tstokes8040 wants to merge 3 commits into
WordPress:trunkfrom
tstokes8040:phpstan/redundant-subexpressions
Open

Code Quality: Remove three redundant boolean sub-expressions#13086
tstokes8040 wants to merge 3 commits into
WordPress:trunkfrom
tstokes8040:phpstan/redundant-subexpressions

Conversation

@tstokes8040

@tstokes8040 tstokes8040 commented Aug 16, 2026

Copy link
Copy Markdown

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:

Trac ticket: https://core.trac.wordpress.org/ticket/65817

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.

Count of removed errors: 2

Each of these conditions re-tests something the surrounding expression has already established. In `wp_render_typography_support()`, `! empty( $block['attrs']['fitText'] )` already requires the value to be truthy, so the following `&& $block['attrs']['fitText']` can never fail. In `_get_block_templates_files()`, the right operand of the `||` is only evaluated when `! $post_type` was false, so the leading `$post_type &&` is likewise always true. In `Walker::display_element()`, `$newlevel` is a local assigned the literal `true` and nothing else, so `isset( $newlevel ) && $newlevel` reduces to the `isset()`.

All three are simplifications with no change in behaviour. This resolves two `booleanAnd.rightAlwaysTrue` and one `booleanAnd.leftAlwaysTrue` occurrence. The baselines were regenerated with:
{{{
composer phpstan:baselines -- --identifier=booleanAnd.rightAlwaysTrue,booleanAnd.leftAlwaysTrue
}}}

Props tstokes8040.
See #65817.
@github-actions

Copy link
Copy Markdown

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 props-bot label.

Core Committers: Use this line as a base for the props when committing in SVN:

Props tstokes8040.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@github-actions

Copy link
Copy Markdown

Test using WordPress Playground

The 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

  • All changes will be lost when closing a tab with a Playground instance.
  • All changes will be lost when refreshing the page.
  • A fresh instance is created each time the link below is clicked.
  • Every time this pull request is updated, a new ZIP file containing all changes is created. If changes are not reflected in the Playground instance,
    it's possible that the most recent build failed, or has not completed. Check the list of workflow runs to be sure.

For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation.

Test this pull request with WordPress Playground.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant