Shared definition of refactoring (legacy → modern architecture). Applies to refactoring skills and code review whenever refactoring is in scope.
php artisan boost:add-skill pekral/arch-app-services
Save this content to: .cursor/rules/refactoring/general.mdc
## What Refactoring Is
- Refactoring is a **behavior-preserving change of code structure**.
- The application keeps doing the same thing; only how it does it changes.
- Goals: readability, structure, and readiness for further development.
- Refactoring is **technical debt reduction**, not a feature.
## Symptoms of Legacy Code
- Large methods (e.g. controllers doing everything).
- Mixed validation, business logic, and persistence in one place.
- Code duplication.
- Poor testability.
- Hidden bugs and race conditions.
## What Refactoring Brings
- Faster delivery of new features.
- Fewer bugs and regressions.
- Better readability.
- Easier onboarding.
- The ability to scale both the application and the team.
## Core Rule
- **Never** do a big-bang rewrite.
- **Always** migrate incrementally, one piece at a time.
## Recommended Process
1. **Stabilize** — add at least smoke tests, fix critical bugs first.
2. **Identify entry points** — start at controllers, jobs, commands, listeners, Livewire components.
3. **Introduce the Action pattern** — keep entry points thin; delegate orchestration to Actions.
4. **Split responsibilities** — Action (orchestration), Service (business logic), Repository (read), ModelManager (write).
5. **Modernize code**:
- raw arrays → DTOs (e.g. Spatie Laravel Data)
- static helpers → dependency injection
- fat models → service layer
- inline validation → Data Validators / FormRequests
6. **Remove duplication** — apply DRY pragmatically, not dogmatically.
7. **Address concurrency** — DB transactions, `lockForUpdate`, atomic operations instead of read-modify-write on shared state.
8. **Split oversized HTML / Blade layouts** — when the refactor touches a Livewire / Blade view, analyze its HTML as a tree of UI concerns and split it into the smallest set of reusable Livewire and Blade components per `@rules/laravel/livewire.mdc` *HTML / Blade Layout Splitting*. Treat the layout split as a structural refactor: the **Test Coverage Contract** below applies in spirit — every rendered branch of the touched view must be exercised by a Livewire / Blade feature test that stays green through the refactor commit unchanged, since PHP `--coverage-clover` does not measure `.blade.php` line-by-line. See the rule's *Process for splitting an existing view* step 6 for the binding wording.
## When to Refactor
- Changes are slow and painful.
- The code is unreadable.
- The team is afraid to touch it.
- The same bugs keep coming back.
## When Not to Refactor
- Just to make the code "look nice".
- Without a business reason.
## Reality Check
- Refactoring does not deliver immediate business value.
- Without it, however:
- delivery slows down,
- technical debt grows,
- error rate increases.
- Refactoring is an investment in future delivery speed and system stability.
## Test Coverage Contract (mandatory — issue #493)
Refactoring is behavior-preserving by definition, so behavior preservation must be **proven** by a test suite that exists **before** the structural change lands. Every refactor must respect the following three-step contract:
1. **Before the refactor commit — verify 100% coverage of the target lines.** Verify coverage of the *current* code that is about to be refactored, using the project's available coverage tooling scoped to those files (see `@rules/php/core-standards.mdc` Testing section). Every line, branch, and condition the refactor will touch must already be exercised by tests. If coverage is below 100% on those lines, **stop and write the missing tests first**.
2. **Add missing tests in a dedicated commit before the refactor commit.** The new tests live in their own commit using the `test(scope): cover <area> before refactor` form per `@rules/git/general.mdc` Allowed Types. Never mix new test code with the refactor change in the same commit — the separation is what makes the safety net auditable.
3. **The refactor commit must not modify pre-existing tests.** Once the safety net is in place, the refactor commit changes structure only. Renaming, restructuring, or rewriting tests that existed before the refactor invalidates the proof that behavior was preserved. The only test changes allowed in the refactor commit are those forced by mechanical renames the refactor itself introduces (e.g. moving a class to a new namespace), and even those must be flagged in the commit body. New tests that *cover newly introduced code paths in the refactor* belong in yet another commit after the refactor — not in the refactor commit and not in the pre-refactor coverage commit.
4. **After the refactor commit — re-verify coverage of the changed lines is still 100%.** Run the project's coverage tooling again, scoped to the refactored files, and confirm every line, branch, and condition the refactor touched is still exercised. The pre-existing assertions must pass unchanged *and* coverage must not have regressed. If a refactored line is no longer covered, the refactor introduced an untested path (typically dead code or a new branch) — fix it before finishing, do not paper over it by editing the pre-refactor tests. This proactive re-verification is part of the apply step, not only a code-review check.
Code review must reject any refactor PR that violates this contract — see the **Code Review Application** section below for the enforcement rules.
## Code Review Application
- Treat this definition as authoritative when reviewing or proposing refactoring.
- Flag big-bang rewrites and mixed-responsibility entry points as findings.
- Prefer incremental migration over wholesale rewrites in suggested fixes.
- **Enforce the Test Coverage Contract above on every refactor PR.** Walk the PR commit history and verify: (a) the refactor commit is preceded by a `test(scope): ...` commit that brings the refactored lines to 100% coverage, (b) no pre-existing test file is modified inside the refactor commit (mechanical-rename exemption only, and only when the commit body documents it), (c) coverage of the refactor commit alone — verified with the project's available coverage tooling scoped to the changed files — reports 100% for changed lines. Missing pre-refactor coverage commit, modified tests inside the refactor commit, or sub-100% coverage on the changed lines are each **Critical** findings.
- For Laravel projects, combine this rule with `@rules/laravel/architecture.mdc` (Action pattern, Repositories, ModelManagers, Data Validators, DTOs).
lockForUpdate, atomic operations instead of read-modify-write on shared state.@rules/laravel/livewire.mdc HTML / Blade Layout Splitting. Treat the layout split as a structural refactor: the Test Coverage Contract below applies in spirit — every rendered branch of the touched view must be exercised by a Livewire / Blade feature test that stays green through the refactor commit unchanged, since PHP --coverage-clover does not measure .blade.php line-by-line. See the rule's Process for splitting an existing view step 6 for the binding wording.Refactoring is behavior-preserving by definition, so behavior preservation must be proven by a test suite that exists before the structural change lands. Every refactor must respect the following three-step contract:
@rules/php/core-standards.mdc Testing section). Every line, branch, and condition the refactor will touch must already be exercised by tests. If coverage is below 100% on those lines, stop and write the missing tests first.test(scope): cover <area> before refactor form per @rules/git/general.mdc Allowed Types. Never mix new test code with the refactor change in the same commit — the separation is what makes the safety net auditable.Code review must reject any refactor PR that violates this contract — see the Code Review Application section below for the enforcement rules.
test(scope): ... commit that brings the refactored lines to 100% coverage, (b) no pre-existing test file is modified inside the refactor commit (mechanical-rename exemption only, and only when the commit body documents it), (c) coverage of the refactor commit alone — verified with the project's available coverage tooling scoped to the changed files — reports 100% for changed lines. Missing pre-refactor coverage commit, modified tests inside the refactor commit, or sub-100% coverage on the changed lines are each Critical findings.@rules/laravel/architecture.mdc (Action pattern, Repositories, ModelManagers, Data Validators, DTOs).How can I help you explore Laravel packages today?