Skip to content
This repository was archived by the owner on Aug 23, 2026. It is now read-only.

refactor(quality): Rector wave 3 — equivalence-safe cosmetics only (AID-531) - #190

Merged
abkrim merged 1 commit into
mainfrom
abdelkarim/aid-531-rector-wave-code-quality
Jul 17, 2026
Merged

abkrim merged 1 commit into
mainfrom
abdelkarim/aid-531-rector-wave-code-quality

Conversation

@abkrim

@abkrim abkrim commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Third wave. 16 app/ files, five equivalence-preserving cosmetic rules from LARAVEL_CODE_QUALITY + codeQuality level 50.

The set is NOT mechanical — it was split by risk profile

Re-measured with --clear-cache, the full set touches 40 files and mixes three risk profiles. This PR ships only the equivalence-safe cosmetics; everything with a semantic or public-surface footprint went to its own ticket, so each PR is revertible without dragging another risk profile:

Rule(s) Risk Lands in
MakeModelAttributesAndScopesProtected 27 scopes public→protected (public surface) AID-540
CarbonToDateFacade + Now→Today runtime class + temporal semantics AID-541
ApplyDefaultInsteadOfNullCoalesce ?? vs ?: — differs on false/0/'' AID-542
DispatchToHelper / RedirectRouteToToRoute / Compact / ThrowWithPrevious equivalence must be shown per transform AID-543
5 cosmetic rules (this PR) shape-only, equivalence-verified AID-531

Every landed transform verified equivalence-preserving

  • ExplicitBoolCompare: if ($s) → $s !== '' && $s !== '0' fires only on string-typed vars. The 3 sites (keyName, keyPath, companyName) all originate from '' or a string-returning call. Rector left the ?string option() path untouched (UserCreateCommand:139) and rewrote only the text()-sourced companyName at :272 — confirmed by inspecting the exact transformed line.
  • ObjectExplicitBoolCompare: if ($obj) → instanceof — equivalent for the declared nullable-object type (12 occurrences).
  • strlen($s)===0 → $s==='', CombineIf (short-circuit preserved), SimplifyIfReturnBool — shape-only.

Shape-verify of the applied diff: 0 scopes, 0 Carbon, 0 ??, 0 to_route leaked; patterns are 12 instanceof + 5 === '' + 3 !== '' && !== '0'.

Two Rector-cache traps hit (and worked around)

  1. Measuring configs back-to-back without --clear-cache returns stale zeros (wave-2 lesson).
  2. composer refactor under-applied: 16 files measured, only 3 written, because it does not pass --clear-cache and the cache from the measuring session marked the other 13 as processed. Cleared the cache → all 16 applied. Noted in the rector.php header for the remaining waves.

Gates

composer check-full → exit 0, coverage 89.7%, PHPStan clean, refactor:dry (cleared cache) exit 0. Violation probe → rc=2 naming app/ProbeCq.php:10.

…ID-531)

Enable the five equivalence-preserving cosmetic rules from
LARAVEL_CODE_QUALITY + codeQuality level 50, applied via composer
refactor across 16 app/ files: ObjectExplicitBoolCompare,
ExplicitBoolCompare, SimplifyIfReturnBool, StrlenZeroToIdenticalEmptyString,
CombineIf.

The set is NOT mechanical as the ticket assumed. Measured with
--clear-cache, the full set touches 40 files and mixes three risk
profiles: 27 model scopes flipped public->protected (public surface),
Carbon->Date facade + now()->startOfDay()->today() (runtime class and
temporal semantics), and ApplyDefaultInsteadOfNullCoalesce (?? vs ?:,
which differs on false/0/''). Those are split out by risk profile into
AID-540 (scopes), AID-541 (Carbon/temporal), AID-542 (?? default —
touches the recorded ??/?: lesson), AID-543 (helpers/exceptions, each
transform to prove equivalence). A PR must be revertible without
dragging another risk profile with it.

Every transform that landed was diff-verified equivalence-preserving:
- ExplicitBoolCompare's `if ($s)` -> `$s !== '' && $s !== '0'` fires only
  on string-typed vars; the three sites (keyName, keyPath, companyName)
  all originate from '' or a string-returning call. Rector left the
  ?string option() path (UserCreateCommand:139) untouched and only
  rewrote the text()-sourced companyName at :272.
- ObjectExplicitBoolCompare's `if ($obj)` -> `instanceof` is equivalent
  for the declared nullable-object type.
- strlen($s)===0 -> $s==='' , CombineIf (short-circuit preserved),
  SimplifyIfReturnBool: all shape-only.

Two Rector-cache traps hit and worked around, both from the wave-2
lesson: measuring several configs back-to-back without --clear-cache
returns stale zeros; and `composer refactor` (no --clear-cache) after a
measuring session under-applied — 16 files measured, only 3 written —
until the cache was cleared and it re-applied all 16. Noted in the
rector.php header for the remaining waves.

Wave checklist:
- clean tree at start; applied via composer refactor with cleared cache
- shape-verified: zero non-cosmetic transforms leaked (0 scopes, 0
  Carbon, 0 ??, 0 to_route); patterns are 12 instanceof + 5 === '' + 3
  !== '' && !== '0'
- rule bites, attributably: a new if/return-bool file -> exit 2 naming
  app/ProbeCq.php:10 (SimplifyIfReturnBoolRector)
- gates: check-full green, coverage 89.7%, PHPStan clean, refactor:dry
  (cleared cache) exit 0

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@abkrim
abkrim merged commit d90e01b into main Jul 17, 2026
2 checks passed
@abkrim
abkrim deleted the abdelkarim/aid-531-rector-wave-code-quality branch July 17, 2026 16:18
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant