fix(php85): eliminate every deprecation, and fail the build on a new one - #197
Merged
Conversation
The suite reported 15 deprecations on PHP 8.5. They were tolerated rather than fixed, which meant
new ones could accumulate unnoticed — and two of these classes are not cosmetic: implicitly
nullable parameters become a FATAL in PHP 9.
Fixed at the source, never suppressed:
- **Implicit nullable params** — `array $Block = null` -> `?array $Block = null` in the vendored
Parsedown (2 signatures). `?array` is exactly what the implicit form already meant, so behaviour
is identical. Parsedown is third-party, so the change is documented in a LOCAL MODIFICATIONS
header block; re-apply it if the library is ever re-vendored. The matching Zend_View one shipped
separately as TigerZF v1.32.2 (the floor here moves to ^1.32.2 so consumers actually get it).
- **`curl_close()`** — a no-op since PHP 8.0 (the handle is an object freed by refcount),
deprecated in 8.5. Removed at ALL 12 call sites, not only the one the tests happened to reach:
Recaptcha, Google/Analytics (x2), Location/Adapter/Aws, Agent/Provider/{Gemini,OpenAiCompatible,
Anthropic} (x2 each), register/Registration, and bin/mcp-bridge. Every one read curl_getinfo /
curl_error BEFORE the call, so removal is behaviour-preserving.
- **`imagedestroy()`** — same story (GdImage is an object since 8.0). Removed from Media/Image,
with a note that the memory is released at exactly the points it always was: `$dst` when the next
loop iteration reassigns it, `$src` when the scope ends.
- **`ReflectionProperty/Method::setAccessible()`** — a no-op since PHP 8.1. Removed from 9 test
sites AND from Application/Bootstrap, which the suite never exercised — most of the codebase had
already been cleaned, these were the stragglers.
Then the ratchet: `failOnDeprecation="true"` in phpunit.xml, so a NEW deprecation fails the build
by exit code instead of being reported and ignored. Verified both ways — reintroducing the
Parsedown signature makes the run exit 1, restoring it exits 0.
Full suite: OK (2111 tests, 21490 assertions) — clean green, zero deprecations.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The suite reported 15 deprecations on PHP 8.5, tolerated rather than fixed. Two of these classes are not cosmetic — implicitly nullable parameters become a fatal in PHP 9.
Fixed at the source. Nothing suppressed.
array $Block = null→?array $Block = nullZend_View::renderString)^1.32.2curl_close()— no-op since 8.0imagedestroy()— no-op since 8.0setAccessible()— no-op since 8.1Beyond the 15 reported
The suite only reaches the paths it covers. I fixed the whole class, not the sample:
curl_close()— the tests hit 1 of 12 call sites. The other 11 (Recaptcha, Aws location, all three agent providers, register, the MCP bridge) would have fired in production.setAccessible()— 9 were in tests, but one was inApplication/Bootstrap, production code the suite never exercises.Every
curl_close()removal was checked to readcurl_getinfo/curl_errorbefore the call, so removal is behaviour-preserving.?arrayis exactly what implicit nullable already meant.Vendored code
Parsedown is third-party, so the patch is recorded in a
LOCAL MODIFICATIONSheader block naming both signatures — otherwise the fix silently disappears on a re-vendor.The ratchet
failOnDeprecation="true"inphpunit.xml, so a new deprecation fails the build by exit code rather than being printed and ignored.Verified it actually bites rather than assuming: reintroducing the Parsedown signature → exit 1; restoring it → exit 0. (The summary line still reads "OK, but there were issues!" — the signal is the exit code, which is what CI gates on.)
Result
OK (2111 tests, 21490 assertions)— clean green, zero deprecations.