fix(cache): restore PSR-16 v1/v2 LSP compatibility (#989)#1014
fix(cache): restore PSR-16 v1/v2 LSP compatibility (#989)#1014serbanghita merged 3 commits into4.xfrom
Conversation
Widen Cache public method parameters to omit scalar type declarations so the class is LSP-compatible with any psr/simple-cache major. Previously, when another plugin's autoloader (common in WordPress) registered an older Psr\SimpleCache\CacheInterface before ours, PHP fatal'd at class load because our narrower signatures violated variance rules. - Drop param types on $key, $ttl, $keys, $values in public methods; validate at runtime via checkKey/checkTtl/checkIterable helpers. - Return types are preserved (covariance-safe). - Broaden composer.json to "^1.0 || ^2.0 || ^3.0" so transitive deps can resolve any supported major without conflict. - Add CI matrix job that runs the test suite against psr/simple-cache ^1.0, ^2.0, and ^3.0 so a future interface change that breaks LSP against our widened signatures is caught automatically. - Add reflection-based regression test asserting the listed parameters stay untyped.
PSR-16 LSP fix loosened get/set/*Multiple parameter types, so passing non-string/non-iterable values is no longer a static type error. PHPStan flags the five @phpstan-ignore-next-line directives as ignore.unmatchedLine, failing the quality job.
There was a problem hiding this comment.
Pull request overview
Restores LSP compatibility for Detection\Cache\Cache across PSR-16 psr/simple-cache majors (v1/v2/v3) by widening public method parameter signatures and adding regression coverage to prevent future signature narrowing that would cause class-load fatals in mixed-autoloader environments (e.g., WordPress).
Changes:
- Removed scalar/union parameter type declarations from PSR-16 public methods and added runtime validation helpers (
checkKey,checkTtl,checkIterable) while preserving return types. - Broadened
psr/simple-cachedependency constraint to allow^1.0 || ^2.0 || ^3.0. - Added reflection-based regression tests and a CI matrix job to run the test suite against PSR-16 v1/v2/v3.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/Cache/Cache.php |
Widens PSR-16 method parameter signatures and adds runtime validation helpers to maintain LSP compatibility across PSR-16 majors. |
tests/CacheTest.php |
Adds reflection-based regression test ensuring selected public parameters remain untyped, plus runtime-validation tests for widened signatures. |
composer.json |
Broadens psr/simple-cache version constraint to support v1/v2/v3. |
.github/workflows/4.8.x-test.yml |
Adds a matrix CI job to execute the test suite against psr/simple-cache v1/v2/v3 across PHP versions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - name: Install composer dependencies | ||
| run: composer install --no-interaction | ||
|
|
There was a problem hiding this comment.
In psr16-compat, composer install is run and then composer require psr/simple-cache:... --update-with-dependencies is run, which will resolve/install dependencies again. This adds significant time across the matrix; consider removing the initial composer install step and letting the composer require ... --update-with-dependencies perform the install (or alternatively change the order so the pin happens before installing).
| - name: Install composer dependencies | |
| run: composer install --no-interaction |
composer require --update-with-dependencies performs a full resolve and install on a fresh checkout, so the prior composer install just did the work twice. Rename the remaining step to be honest about installing.
Widen Cache public method parameters to omit scalar type declarations so the class is LSP-compatible with any psr/simple-cache major. Previously, when another plugin's autoloader (common in WordPress) registered an older Psr\SimpleCache\CacheInterface before ours, PHP fatal'd at class load because our narrower signatures violated variance rules.