This repository was archived by the owner on Aug 23, 2026. It is now read-only.
Repository navigation
refactor(quality): Rector wave 8 — route/dispatch/compact helpers; ThrowWithPrevious proven harmful (AID-543) - #195
Merged
Conversation
SimpleUnblockRateLimitExceeded declares int for $attempts and $maxAttempts, but rateLimitResponse() fed it RateLimiter::attempts() raw — whatever the cache store holds, a string under the database driver — and an uncast config() read that returns the raw env string whenever the variable is set. It only ever worked by accident: the event was built through Dispatchable::dispatch(), whose call site lives in vendor without strict_types, so PHP coerced '3' to 3 silently. checkRateLimit() in the same middleware already casts its config read; this path was the exception. Surfaced by the wave in the next commit, which moves the constructor call site into this file — where declare(strict_types=1) (AID-529) turns the hidden coercion into a TypeError, caught by the suite (5 failures in ThrottleSimpleUnblockTest). The bug predates the wave; the cast belongs at the data source either way. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…rowWithPrevious proven harmful (AID-543)
Enable RedirectRouteToToRouteHelperRector, DispatchToHelperFunctionsRector
and CompactToVariablesRector; apply with cleared cache: 13 app/ files
(9 to_route, 11 dispatch(new ...), 6 event(new ...), 1 compact).
Equivalence is literal in vendor, verified per transform as the ticket
required: to_route() IS redirect()->route() (alias in
Foundation/helpers.php; the ->with() chaining lands on the same
RedirectResponse); event Dispatchable::dispatch() IS
event(new static(...)); Bus Dispatchable::dispatch() returns
newPendingDispatch(new static(...)) = new PendingDispatch($job),
identical to the dispatch() helper — and no converted site chains
->onQueue()/->onConnection(). compact('report') -> ['report' => $report]
with the variable provably in scope.
ThrowWithPreviousExceptionRector is PERMANENTLY skipped, proven harmful
rather than assumed safe: all 9 domain throws already pass previous:,
so what the rule actually adds here is $e->getCode() as the SECOND
POSITIONAL argument, assuming the stock \Exception(message, code,
previous) signature — but the domain exceptions have custom
constructors (FirewallException's 2nd parameter is ?string $hostName,
CsfServiceException's is ?string $message). Under app/'s strict_types
that is a runtime TypeError on the firewall error paths, reproduced in
an isolated strict-mode script: "Argument #2 ($hostName) must be of
type ?string, int given". Without strict_types it would be silent data
corruption (hostName='0'). The skip is config, not prose; a
counter-probe confirms the applied wave touches zero throw sites and
adds zero getCode() calls.
The dispatch conversion surfaced a latent type bug fixed in the
previous commit: moving the event constructor's call site from vendor
(coercive) into app/ (strict) exposed RateLimiter::attempts()
returning a string into an int parameter — the suite caught it
(5 failures), exactly the strict_types safety net working as intended.
Wave checklist:
- resized with --clear-cache: 4 + 8 + 1 files, overlap zero, 13 total
- diff shape-verified: every changed line classified, none unexplained
- rules bite, attributably: a probe file with all three old forms ->
exit 2 naming each rule
- idempotence: second rector+pint pass is a byte-level no-op
- gates: check-full green, 1442 passed / 16 skipped, coverage 89.9%,
refactor:dry (cleared cache) exit 0
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Eighth and final wave of the split set. 13 app/ files (9
to_route, 11dispatch(new …), 6event(new …), 1 compact). Resized with--clear-cache: 4 + 8 + 1, overlap zero.Equivalence is literal in vendor — verified per transform
redirect()->route()→to_route()to_route()isredirect()->route()(alias inFoundation/helpers.php);->with()chains on the sameRedirectResponseEvent::dispatch()→event(new …)Events\Dispatchable::dispatch()isevent(new static(...))Job::dispatch()→dispatch(new …)Bus\Dispatchable::dispatch()→new PendingDispatch($job), identical to the helper; no converted site chains->onQueue()/->onConnection()compact('report')→['report' => $report]ThrowWithPreviousExceptionRector: permanently skipped, proven harmfulAll 9 domain throws already pass
previous:— the rule's nominal purpose is satisfied. What it actually adds here is$e->getCode()as the second positional argument, assuming the stock\Exception(message, code, previous)signature. The domain exceptions have custom constructors:FirewallException's 2nd parameter is?string $hostName,CsfServiceException's is?string $message.Under app/'s
strict_types(wave 1) that is a runtime TypeError on the firewall error paths — reproduced in an isolated strict-mode script:Argument #2 ($hostName) must be of type ?string, int given. Without strict_types it would be silent data corruption (hostName='0'). The skip lives in config with the reason; a counter-probe confirms the applied wave touches zero throw sites and adds zerogetCode()calls.First commit: latent type bug surfaced and fixed at source
Moving the event constructor's call site from vendor (coercive) into app/ (strict) exposed
RateLimiter::attempts()returning a string into anintparameter — it had only ever worked becauseDispatchable::dispatch()built the event inside vendor, where PHP coerced'3'→ 3 silently. The suite caught it (5 failures inThrottleSimpleUnblockTest): exactly the strict_types safety net working. Fixed by casting at the data source ((int) RateLimiter::attempts()+ the one uncastconfig()read in the same method —checkRateLimit()already casts its own).Gates
rc=2naming each rule ✓getCode()added ✓refactor:dry(cleared cache) exit 0 ✓composer check-full→ exit 0, 1442 passed / 16 skipped, coverage 89.9% ✓