Skip to content

Fix implicit nullable parameters deprecated in PHP 8.4 - #4

Merged
matasarei merged 5 commits into
mainfrom
fix/php84-implicit-nullables
Sep 26, 2026
Merged

matasarei merged 5 commits into
mainfrom
fix/php84-implicit-nullables

Conversation

@matasarei

@matasarei matasarei commented Sep 26, 2026 •

Copy link
Copy Markdown
Contributor

What and why

ScopesMapper::__construct(Scopes $defaultScopes = null) and ItemsListResponse::__construct(array $items = [], int $total = null) declare implicitly nullable parameters. PHP 8.4 reports this as deprecated every time either class loads.

This library's suite never saw it, because PHPUnit 9 does not fail on deprecations by default. Downstream it does fail: grinchenkoedu/registries prints the notice from inside its tests, and its PHP 8.4 CI job fails under failOnRisky. That is the last cause left in registries#64.

How

  • Both parameters are now explicitly nullable (?Scopes, ?int). No behaviour change.
  • phpunit.xml sets convertDeprecationsToExceptions="true", so the next deprecation fails this CI instead of a downstream one. The file keeps its CRLF line endings; the change is one line.

Testing

  • With only the guard in place, the suite on PHP 8.4.10 (composer:lts image) gave Tests: 46, Assertions: 121, Errors: 15, all from ScopesMapper.php:15.
  • With the fix:
    • PHP 8.4 and 8.5 (stock images): OK (46 tests, 297 assertions).
    • PHP 7.4.3 (Composer install inside 7.4): OK (46 tests, 297 assertions).
  • No implicit nullable is left in src/ or tests/ (grep).

Notes

  • CHANGELOG has a 2.0.1 — unreleased entry. Once this is merged, tag 2.0.1 so registries can require ^2.0.1 when it moves to 2.0 (its Phase B, step 6).

Update after review

  • The guard now fires in CI (ad97ede). setup-php loads the production php.ini (ini-file: production in the job logs), whose error_reporting leaves out E_DEPRECATED, and PHPUnit 9 only converts errors that error_reporting includes. So convertDeprecationsToExceptions had no effect in CI. With the ScopesMapper fix reverted, the suite at CI's level passed: OK (46 tests, 297 assertions). phpunit.xml now sets error_reporting to -1. At CI's level, the unfixed code now gives Errors: 15 and the fixed code OK (46 tests, 297 assertions).

    • Trade-off: the guard also covers dependencies, and CI resolves them fresh on every run, including the monthly one. A future dependency release that triggers a deprecation would turn CI red with no change here.
  • .gitignore (218a24b): ignores .tasks/, .claude/repo-profile.json and .gku/, which are local plans and tool state. CRLF, like the rest of the file.

  • First catch (55ecd8a): once the guard was active, the PHP 7.4 job failed with 5 errors. ClientFactoryTest cast ReflectionType to a string, which is deprecated on 7.4 only. It now uses getName(). All 7 PHP jobs pass on 55ecd8a.

matasarei and others added 5 commits September 27, 2026 01:39
ScopesMapper::__construct(Scopes $defaultScopes = null) and
ItemsListResponse::__construct(..., int $total = null) are implicitly
nullable, which PHP 8.4 reports as deprecated whenever the class loads.
The suite never noticed: PHPUnit 9 does not fail on deprecations by
default. Downstream, grinchenkoedu/registries prints the notice from
inside its tests and fails its PHP 8.4 CI job on it.

Both are now explicitly nullable (?Scopes, ?int) — no behaviour change.
phpunit.xml sets convertDeprecationsToExceptions="true": before the fix
the suite errored 15 times on PHP 8.4; after it, OK (46 tests, 297
assertions) on PHP 7.4, 8.4 and 8.5.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
setup-php loads php.ini-production (`ini-file: production` in the job
logs), whose error_reporting leaves out E_DEPRECATED, and PHPUnit 9 only
converts errors that error_reporting includes. So the guard never fired
in CI: with ScopesMapper's fix reverted, the suite at CI's level
(E_ALL & ~E_DEPRECATED & ~E_STRICT) gave OK (46 tests, 297 assertions).
With error_reporting=-1 set in phpunit.xml it gives Errors: 15, and the
fixed code stays OK (46 tests, 297 assertions).

Co-Authored-By: Claude Opus 5.5 <[email protected]>
.tasks/ holds local plans and briefs, .claude/repo-profile.json one
developer's machine profile, and .gku/ local review reports and notes.
None of them belongs in the repository. The new lines use CRLF, like
the rest of the file.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
ReflectionType::__toString() is deprecated on PHP 7.4 (8.0 lifted the
deprecation). With the deprecation guard now active in CI, the PHP 7.4
job failed with 5 errors from ClientFactoryTest.php:96. getName()
compares the class names themselves, which is what the test is about.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Tagged and released on 2026-09-27.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@matasarei
matasarei merged commit b881f0a into main Sep 26, 2026
9 checks passed
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