Skip to content

Harden type handling for PHP 8.5 compatibility#27

Open
rhoerr wants to merge 1 commit intomainfrom
rhoerr/php85-null-safety
Open

Harden type handling for PHP 8.5 compatibility#27
rhoerr wants to merge 1 commit intomainfrom
rhoerr/php85-null-safety

Conversation

@rhoerr
Copy link
Copy Markdown
Contributor

@rhoerr rhoerr commented Apr 26, 2026

Summary

PHP 8.5 promotes several previously-deprecated coercions to errors — notably casting non-scalar values to string. This PR hardens type handling in places where mixed / untyped values flow into typed APIs, so the module is forward-compatible.

  • ViewModel/SpeculationRules::getConfigValue — guard the mixed scopeConfig->getValue() result with is_scalar before casting to string. Previously a non-scalar config value (rare, but possible via custom backends) would fatal in PHP 8.5.
  • ViewModel/SpeculationRules::getSpeculationRulesJson — handle SerializerInterface::serialize() returning false. The interface return is string|false; the method signature promised string.
  • ViewModel/ViewTransitions::getConfigValue — same scalar guard as above.
  • Plugin/Framework/App/Response/Http — narrow getHeader() result with instanceof checks. Laminas Headers::get() returns false|HeaderInterface|ArrayIterator; the previous if (!$header) only ruled out false, leaving ArrayIterator callable as if it were a HeaderInterface. removeDirective() lives on CacheControl, not HeaderInterface, so the second narrowing uses that concrete class. Also dropped a redundant second getHeader() call in afterSetNoCacheHeaders.
  • Plugin/Framework/App/Response/Http::getConfig — same scalar guard before casting to string.
  • Setup/Patch/Data/UpdateSpeculationRulesConfigPathPatch::apply — return $this to match PatchInterface docblock (@return $this). Not a PHP 8.5 issue per se, but caught by phpstan and trivial to align.

Verification

  • phpstan level 9 against the module — clean (was 8 errors before)
  • phpunit against app/code/MageOS/ThemeOptimization/phpunit.xml.dist — 41/41 pass

Test plan

  • CI green
  • No regression on bfcache header behavior in built-in FPC mode
  • Speculation rules JSON still emitted for stores with non-default mode/eagerness/exclude_* config
  • setup:upgrade still applies UpdateSpeculationRulesConfigPathPatch cleanly

🤖 Generated with Claude Code

PHP 8.5 promotes several previously-deprecated coercions to errors,
notably casting non-scalar values to string. This hardens type
handling in places where mixed/untyped values flow into typed APIs.

- ViewModel/SpeculationRules::getConfigValue: guard mixed scopeConfig
  return with is_scalar before casting to string
- ViewModel/SpeculationRules::getSpeculationRulesJson: handle
  SerializerInterface::serialize returning false
- ViewModel/ViewTransitions::getConfigValue: same guard as above
- Plugin/Framework/App/Response/Http: narrow getHeader() result with
  instanceof checks (Laminas returns false|HeaderInterface|ArrayIterator;
  removeDirective is on CacheControl, not HeaderInterface)
- Plugin/Framework/App/Response/Http::getConfig: same scalar guard
- Setup/Patch/Data/UpdateSpeculationRulesConfigPathPatch::apply:
  return \$this to match PatchInterface contract

Verified with phpstan level 9 (clean) and existing unit tests (41/41).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@rhoerr rhoerr requested a review from a team as a code owner April 26, 2026 17:54
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