Skip to content

re #96 burn down PHPStan level-6 baseline (72 -> 11)#111

Merged
tonydspaniard merged 2 commits into
masterfrom
chore/96-burndown-level6
May 28, 2026
Merged

re #96 burn down PHPStan level-6 baseline (72 -> 11)#111
tonydspaniard merged 2 commits into
masterfrom
chore/96-burndown-level6

Conversation

@tonydspaniard
Copy link
Copy Markdown
Member

Summary

Continues #96 toward an empty baseline. Fixes 69 of the 80 grandfathered level-6 findings at root cause — no @phpstan-ignore, no type-widening-to-silence, no baseline tricks. phpstan-baseline.neon shrinks 72 → 11 entries.

Work was parallelized: sub-agents fixed the independent packages (Http, Validation, Security, Session, Sanitation, Courier, Doctor, Events, Happen, Introspection, Scaffold); the coupled/design-heavy slice (Cache, Common, Container, Structure) was done directly. Every change verified centrally.

Notable real bugs fixed (not just annotations)

  • HttpStatusCollection int/string confusion (substr on an int, impossible instanceof, unreachable code).
  • FilesystemCacheItemStorage: the Windows path-length guard was '\\' === '/' — always false, so it never fired. Now DIRECTORY_SEPARATOR === '\\'.
  • Predis: cluster classes moved to Predis\Connection\Cluster\* in predis 2.x (code imported the removed Aggregate\*); also removed dead 0 >= $lifespan branches.
  • CreditCard/Ip/Isbn rules did arithmetic on strings; Session gc() returned void/true instead of int|false.
  • Executable narrowed to ReflectionFunction|ReflectionMethod (only concrete subtypes), fixing invokeArgs()/invokeClosure().
  • Arr::getValue typed mixed (it genuinely handles array|object|other and recurses on mixed) — also fixes a latent nested-access TypeError.

Dead code / precision

Redundant is_array/is_string/method_exists guards, always-false branches, redundant array_values, unnecessary nullsafe removed. SequenceTrait::adjustCapacity() is now abstract instead of method_exists-guarded; PriorityNode uses native int + constructor promotion.

Remaining 11 (decision-gated, follow-up)

  • 8 trait.unused — public-API mixins (Data entity traits, etc.) exercised only by test fixtures, which PHPStan doesn't analyze (adding tests/ to paths surfaces 1006 test-code findings — out of scope).
  • 2 Cookie collection genericsCookieCollection/SetCookieCollection diverge from Map<string, Cookie> (values() returns string cookie values), so no @extends binding is LSP-valid without a compose-vs-extend refactor.
  • 1 Cycle getRepository template — third-party class-string<T>|non-empty-string|T union prevents template inference; result is pinned via @var.

Test plan

  • composer cs — clean
  • composer stan (PHPStan 2.1, baseline 11) — [OK] No errors
  • vendor/bin/rector process --dry-run (full) — clean
  • composer test — 5555 tests; only pre-existing local MongoSessionHandlerTest env errors (CI loads ext-mongodb)

Antonio Ramirez added 2 commits May 28, 2026 08:49
Fix 69 of the 80 grandfathered level-6 findings at root cause across ~25
files in 12 packages (no @PHPStan-Ignore, no widening, no baseline tricks).
Parallel sub-agents handled the independent packages (Http, Validation,
Security, Session, Sanitation, Courier, Doctor, Events, Happen,
Introspection, Scaffold); the coupled/design-heavy slice (Cache, Common,
Container, Structure) was done directly. Full test suite green (only the
pre-existing ext-mongodb-absent MongoSessionHandlerTest errors remain).

Highlights:
- Real bugs fixed: HttpStatusCollection int/string confusion; the Windows
  path-length guard in FilesystemCacheItemStorage ('\\' === '/' was always
  false); Predis cluster classes moved to Predis\Connection\Cluster\* in
  predis 2.x; CreditCard/Ip/Isbn arithmetic on strings; Session gc() return
  types (void/true -> int|false); the unused validateToken expiry gap.
- Type precision: Executable narrowed to ReflectionFunction|ReflectionMethod;
  Arr::getValue typed mixed (it genuinely handles array|object|other);
  Courier middleware array shapes; Sanitizer/Validator $payload -> interface.
- Dead code removed: redundant is_array/is_string/method_exists guards,
  always-false branches, redundant array_values, unnecessary nullsafe.
- SequenceTrait declares adjustCapacity() abstract instead of guarding with
  method_exists; PriorityNode priority/stamp are native int.

Remaining 11 are decision-gated (tracked for follow-up): 8 trait.unused
(public-API mixins exercised only by tests), 2 Cookie collection generics
(divergent-contract LSP), 1 Cycle getRepository template (third-party
loose-union generic).
@tonydspaniard tonydspaniard merged commit dd957d8 into master May 28, 2026
3 checks passed
@tonydspaniard tonydspaniard deleted the chore/96-burndown-level6 branch May 28, 2026 07:11
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