Skip to content

Fix silently-swallowed CI failures + resolve pre-existing PHPStan errors - #4

Merged
miguelcolmenares merged 2 commits into
mainfrom
fix/phpstan-errors-and-makefile-enforcement
Aug 17, 2026
Merged

Fix silently-swallowed CI failures + resolve pre-existing PHPStan errors#4
miguelcolmenares merged 2 commits into
mainfrom
fix/phpstan-errors-and-makefile-enforcement

Conversation

@miguelcolmenares

Copy link
Copy Markdown
Member

Summary

Two related fixes found while investigating why 3 real PHPStan errors in community-listings never failed CI:

  1. The 3 PHPStan errors themselves, resolved via real refactors (no @phpstan-ignore suppressions):

    • SettingsPage::render_settings_page(): removed a dead-code truthy check — wp_count_posts() always returns an object per its own stub, never false.
    • CptRegistrar::register_graphql_meta_fields(): added the missing function_exists('register_graphql_field') guard (only register_graphql_object_type was checked, even though both are called — WPGraphQL always registers both together in practice, so this was never a runtime risk, but PHPStan can't know that without its own explicit check).
    • Same method: refactored the PHP-type → GraphQL-scalar mapping from an array + ?? fallback to a match expression. The array fallback was provably unreachable (every current META_FIELDS value is a mapped key) but had real defensive value for a future entry using an unmapped type — a match default arm preserves that behavior without PHPStan being able to prove it dead.
  2. The Makefile bug that hid them. phpcs/phpstan/install/install-dev/build all looped for plugin in $(PLUGINS) without checking each iteration's exit status — only the last plugin's result reached Make. Since graphql-shortcode-support (last in $(PLUGINS)) always passed, community-listings' real PHPStan failures were silently swallowed. Confirmed via a CI log from before this fix: the PHPStan step printed Found 3 errors and returned with error code 1 for community-listings, then kept going and the job still reported success. Same root-cause class as the run-quality-checks.sh errexit bug found during the wp-coding-standards portfolio rollout.

    While fixing this, also found make phpcs (via composer run phpcs) disagreed with CI's own PHPCS step (which passes --warning-severity=0) — once exit codes stopped being swallowed, make phpcs started failing on pre-existing warnings CI is configured to ignore. make phpcs now calls vendor/bin/phpcs --warning-severity=0 directly, matching CI exactly.

Verified

  • make test (install-dev + phpcs + phpstan) and make build both exit 0 across all three sub-plugins
  • PHPCS: 0 errors, 0 warnings-that-CI-cares-about in all 3 sub-plugins
  • PHPStan: 0 errors in all 3 sub-plugins (previously 3 in community-listings)
  • A pre-PR core-review pass (read-only subagent) confirmed no dangling references to the removed $type_map variable, semantic equivalence of the match refactor against every META_FIELDS value, and valid Make/shell semantics for the exit-status propagation — it's what caught install/install-dev/build sharing the same bug, since my first pass only touched phpcs/phpstan

CHANGELOG

Also backfills [Unreleased] entries for #2 (Provider Listings meta box) and #3 (coding-standards adoption), which hadn't been documented yet, alongside this fix.

- SettingsPage::render_settings_page(): remove dead truthy-check —
  wp_count_posts() always returns an object per its own stub, never
  false, so the guard could never be false.
- CptRegistrar::register_graphql_meta_fields(): add the missing
  function_exists('register_graphql_field') guard (only
  register_graphql_object_type was checked, even though both are
  called) — WPGraphQL always registers both together, so this was
  never a runtime risk, but PHPStan can't know that without its own
  explicit check.
- Same method: refactor the PHP-type -> GraphQL-scalar mapping from an
  array + `??` fallback to a match expression. The array fallback was
  provably unreachable (every current META_FIELDS value is a mapped
  key) but still had real defensive value for a future entry using an
  unmapped type — a match's default arm preserves that behavior
  without PHPStan being able to prove it dead, so no ignore comment is
  needed either way.

Verified: 0 PHPCS and 0 PHPStan errors in community-listings; the
other two sub-plugins already reported 0.
- install/install-dev/build targets had the same for-loop-swallows-
  failure bug just fixed in phpcs/phpstan (found by a pre-PR review
  pass) — same fix applied.
- phpcs target now calls vendor/bin/phpcs --warning-severity=0
  directly instead of composer run phpcs. The two behaved differently:
  composer's bare `phpcs` script has no --warning-severity flag, so
  once exit codes were no longer swallowed, make phpcs started failing
  on pre-existing warnings (discouraged json_encode()/file_get_contents()
  calls, a missing nonce-verification annotation) that CI's own PHPCS
  step is explicitly configured to ignore. Local and CI now agree.

Verified: make test (install-dev + phpcs + phpstan) and make build
both exit 0 across all three sub-plugins.
@miguelcolmenares
miguelcolmenares merged commit 61973e9 into main Aug 17, 2026
3 checks passed
@miguelcolmenares
miguelcolmenares deleted the fix/phpstan-errors-and-makefile-enforcement branch August 17, 2026 07:43
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