This repository was archived by the owner on Aug 23, 2026. It is now read-only.
Repository navigation
refactor(quality): Rector wave 1 — declare(strict_types=1) in app/ (AID-529) - #189
Merged
Merged
Conversation
…ID-529)
Enable DeclareStrictTypesRector and apply it via composer refactor: 73
files in app/ gain declare(strict_types=1), bringing app/ to 182/182.
This is the only semantic wave: strict_types changes runtime type
coercion, so a file that today accepts "5" where it declares int starts
throwing TypeError. Every other wave only changed shape.
Scope decision — Pint stays out of it. The ticket floated adding
declare_strict_types to pint.json to own the convention repo-wide, on
the premise that Rector alone would leave new files uncovered. That
premise was wrong: with the rule in the gate, a new app/ file without
the declare fails refactor:dry (proven by violation probe below), so the
gate already owns app/. Pint would only extend reach OUTSIDE app/ —
measured at 44 files in tests/, 2 in routes/, 6 in bootstrap/ — which is
a different mechanism with a different risk profile and belongs to its
own wave, not smuggled into a semantic PR.
Wave checklist (per rector.php header):
- clean tree at start; applied via composer refactor (rector + pint)
- diff shape-verified pure: 146 added lines = 73 declares + 73 blank
lines, zero others, zero deletions (insert-only)
- rule proven to bite, attributably: a new file without the declare ->
exit 2 naming app/ProbeStrict.php:1
- idempotence proven: second composer refactor pass is a byte-level
no-op (ordered checksum identical, rector reports zero changes)
- gates: check-full green, 1442 passed / 16 skipped, coverage 89.8%,
PHPStan clean, refactor:dry exit 0
Semantic risk reviewed by hand, since a green suite does not prove
coercion is safe on uncovered paths. The 73 files break down as 32
Filament, 10 artisan commands, 8 models, 7 exceptions, 3 services, 2
each of controllers/middleware/Livewire/traits. Artisan commands are the
classic hazard — argument() and option() always return strings — so
every raw-input site was traced to its destination: Host::where('id',
$hostId) and Host::find($hostId) pass strings to Eloquent (accepted);
UnblockIpCommand already casts with (int) $hostId before the typed call;
the remaining option() uses are boolean flags and request()->ip() is
string. No string reaches a typed int/float parameter uncast.
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.
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 only semantic wave:
strict_typeschanges runtime type coercion, so a file that today accepts"5"where it declaresintstarts throwingTypeError. Every other wave only changed shape.73 files in app/ gain the declare → app/ is now 182/182, matching openmizan's convention.
Scope decision: Pint stays out
The ticket floated adding
declare_strict_typestopint.jsonto own the convention repo-wide, on the premise that Rector alone would leave new files uncovered.That premise was wrong. With the rule in the gate, a new
app/file without the declare failsrefactor:dry— proven by the violation probe below. The gate already owns app/.Pint would only extend reach outside app/. Measured (the ticket's "175 files in tests/" was also wrong — that counted files that exist, not files that would change):
tests/routes/bootstrap/That is a different mechanism with a different risk profile. It belongs to its own wave, not smuggled into a semantic PR.
Diff is shape-verified pure
Semantic risk reviewed by hand
A green suite does not prove coercion is safe on uncovered paths, so every raw-input site among the 73 was traced to its destination.
Breakdown: 32 Filament · 10 artisan commands · 8 models · 7 exceptions · 3 services · 2 each of controllers/middleware/Livewire/traits.
Artisan commands are the classic hazard —
argument()andoption()always return strings:AddHostKeyCommand:35Host::where('id', $hostId)UnblockIpCommand:47$action->handle($ip, (int) $hostId, ...)TestHostConnectionCommand:40Host::find($hostId)TestSpatieMultiplexerCommand:29Host::find($hostId)Remaining
option()uses are boolean flags;request()->ip()returns string. No string reaches a typed int/float parameter uncast.Wave checklist
composer refactorrc=2namingapp/ProbeStrict.php:1composer check-full→ exit 0, 1442 passed / 16 skipped, coverage 89.8%, PHPStan clean,refactor:dryexit 0