Skip to content
This repository was archived by the owner on Aug 23, 2026. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
},
"require-dev": {
"barryvdh/laravel-debugbar": "^4.0",
"driftingly/rector-laravel": "^2.5",
"fakerphp/faker": "^1.23",
"infection/infection": "^0.32.0",
"larastan/larastan": "^3.6",
Expand All @@ -35,7 +36,8 @@
"pestphp/pest-plugin-browser": "^4.0",
"pestphp/pest-plugin-laravel": "^4.0",
"pestphp/pest-plugin-livewire": "^4.0",
"phpstan/phpstan": "^2.1"
"phpstan/phpstan": "^2.1",
"rector/rector": "^2.5"
},
"autoload": {
"psr-4": {
Expand Down Expand Up @@ -88,6 +90,15 @@
"syntax-check": [
"bash ./scripts/check-syntax.sh"
],
"refactor": [
"Composer\\Config::disableProcessTimeout",
"./vendor/bin/rector process",
"./vendor/bin/pint --dirty"
],
"refactor:dry": [
"Composer\\Config::disableProcessTimeout",
"./vendor/bin/rector process --dry-run"
],
"test": [
"@php artisan config:clear --ansi",
"@php artisan test"
Expand Down Expand Up @@ -116,13 +127,15 @@
"@validate-lock",
"@syntax-check",
"@pint-test",
"@refactor:dry",
"@phpstan",
"@test"
],
"check-full": [
"@validate-lock",
"@syntax-check",
"@pint-test",
"@refactor:dry",
"@phpstan",
"@test-coverage"
],
Expand Down
98 changes: 97 additions & 1 deletion composer.lock
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
"Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies",
"This file is @generated automatically"
],
"content-hash": "d19acfc6961bac096ede3f0b8c93eaad",
"content-hash": "d3b212e3be291c8f448f9f6ce7a84233",
"packages": [
{
"name": "anourvalar/eloquent-serialize",
Expand Down Expand Up @@ -11083,6 +11083,42 @@
},
"time": "2026-02-07T07:09:04+00:00"
},
{
"name": "driftingly/rector-laravel",
"version": "2.5.0",
"source": {
"type": "git",
"url": "https://github.com/driftingly/rector-laravel.git",
"reference": "0dbdd842dfdbc821cfe5f47ca7326e2502b2a107"
},
"dist": {
"type": "zip",
"url": "https://api.github.com/repos/driftingly/rector-laravel/zipball/0dbdd842dfdbc821cfe5f47ca7326e2502b2a107",
"reference": "0dbdd842dfdbc821cfe5f47ca7326e2502b2a107",
"shasum": ""
},
"require": {
"php": "^7.4 || ^8.0",
"rector/rector": "^2.2.7",
"webmozart/assert": "^1.11 || ^2.0"
},
"type": "rector-extension",
"autoload": {
"psr-4": {
"RectorLaravel\\": "src"
}
},
"notification-url": "https://packagist.org/downloads/",
"license": [
"MIT"
],
"description": "Rector upgrades rules for Laravel Framework",
"support": {
"issues": "https://github.com/driftingly/rector-laravel/issues",
"source": "https://github.com/driftingly/rector-laravel/tree/2.5.0"
},
"time": "2026-06-02T16:22:14+00:00"
},
{
"name": "ergebnis/agent-detector",
"version": "1.2.0",
Expand Down Expand Up @@ -15402,6 +15438,66 @@
],
"time": "2024-06-11T12:45:25+00:00"
},
{
"name": "rector/rector",
"version": "2.5.7",
"source": {
"type": "git",
"url": "https://github.com/rectorphp/rector.git",
"reference": "ba22f8c087848278fed6b4910d4cf1108096d8d3"
},
"dist": {
"type": "zip",
"url": "https://api.github.com/repos/rectorphp/rector/zipball/ba22f8c087848278fed6b4910d4cf1108096d8d3",
"reference": "ba22f8c087848278fed6b4910d4cf1108096d8d3",
"shasum": ""
},
"require": {
"php": "^7.4|^8.0",
"phpstan/phpstan": "^2.2.2"
},
"conflict": {
"rector/rector-doctrine": "*",
"rector/rector-downgrade-php": "*",
"rector/rector-phpunit": "*",
"rector/rector-symfony": "*"
},
"suggest": {
"ext-dom": "To manipulate phpunit.xml via the custom-rule command"
},
"bin": [
"bin/rector"
],
"type": "library",
"autoload": {
"files": [
"bootstrap.php"
]
},
"notification-url": "https://packagist.org/downloads/",
"license": [
"MIT"
],
"description": "Instant Upgrade and Automated Refactoring of any PHP code",
"homepage": "https://getrector.com/",
"keywords": [
"automation",
"dev",
"migration",
"refactoring"
],
"support": {
"issues": "https://github.com/rectorphp/rector/issues",
"source": "https://github.com/rectorphp/rector/tree/2.5.7"
},
"funding": [
{
"url": "https://github.com/tomasvotruba",
"type": "github"
}
],
"time": "2026-07-13T15:24:18+00:00"
},
{
"name": "revolt/event-loop",
"version": "v1.0.9",
Expand Down
113 changes: 113 additions & 0 deletions docs/superpowers/specs/2026-07-17-rector-laravel-design.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
# Rector adoption — gate infrastructure (PR 0)

- **Ticket:** AID-528
- **Date:** 2026-07-17
- **Status:** approved, pending implementation

## Goal

Adopt `rector/rector` + `driftingly/rector-laravel` in unblock as a code-quality support tool, wired to a gate that actually blocks bad merges. Follow the wave-based adoption pattern proven in openmizan (AID-520), **adapted to unblock's measured reality** rather than copied.

Wave-based means: PR 0 ships infrastructure only, with a rule set verified zero-diff. Each subsequent wave enables curated rules in its own mechanical PR, backed by the full suite.

## Measured baseline (pilot, 2026-07-17)

Run in an isolated worktree, zero writes to the repo. `rector/rector 2.5.7` + `driftingly/rector-laravel 2.5.0`, over `app/` (182 PHP files), target `PHP_83`:

| Rule / Set | Files |
|---|---|
| `RemoveDumpDataDeadCodeRector` | **0** ← Wave 0 |
| `ReadOnlyClassRector` | 0 |
| `LARAVEL_TYPE_DECLARATIONS` | 0 |
| `deadCode` (50) | 3 |
| `LARAVEL_COLLECTION` | 4 |
| `codeQuality` (50) | 22 |
| `LARAVEL_CODE_QUALITY` | 28 |
| `AddOverrideAttributeToOverriddenMethodsRector` | 48 |
| `DeclareStrictTypesRector` | 73 |

`DeclareStrictTypesRector`'s 73 is cross-checked independently: `app/` has 182 PHP files, 109 of which already carry `declare(strict_types=1)` — 182 − 109 = 73.

## Why this is not a copy of openmizan's config

Two premises behind openmizan's `rector.php` do not hold here, and both were measured:

1. **`DeclareStrictTypesRector` is not zero-diff in unblock.** openmizan's `pint.json` sets `declare_strict_types: true`, so Pint owns strict-types insertion repo-wide and `app/` sits at 597/597 — the Rector rule there is an inert second lock. unblock's `pint.json` does not set it, and `app/` sits at 109/182. Enabling the same rule here would rewrite 73 files with a **semantic** change (runtime type coercion), not a cosmetic one. Deferred to its own wave.

2. **unblock already has the enforcement openmizan lacks.** openmizan's `main` has no branch protection (its AID-527), which is why its `rector.php` documents enforcement as "ship-workflow convention — a red job does not block a merge by itself". unblock's `main` has `required_status_checks: ["quality"]` with `strict: true` (verified via `gh api`), and the `quality` job runs `composer check-full`. Placing the gate inside `check-full` therefore inherits real enforcement for free: a pending diff turns CI red and blocks the merge.

## Design

### `rector.php`

```php
->withPaths([__DIR__.'/app'])
->withPhpVersion(PhpVersion::PHP_83)
->withRules([RemoveDumpDataDeadCodeRector::class])
->withSkip([
ThrowIfRector::class,
ThrowIfAndThrowUnlessExceptionsToUseClassStringRector::class,
AppToResolveRector::class,
])
```

**Path scope.** `app/` only. `routes/`, `bootstrap/`, `config/` and `tests/` are wave candidates, not oversights. `database/` must never be added: migrations are append-only history.

**PHP version.** `PHP_83` follows the `composer.json` floor (`^8.3|^8.4`). Bump both together so gate semantics never drift implicitly. The CI runtime is PHP 8.4; `withPhpVersion` sets the transformation target, not the runtime, so the two are independent by design.

**Skips**, justified against unblock's own code rather than inherited:

- `ThrowIfRector` — `app/` has 52 explicit `throw new` and **zero** `throw_if()`. The explicit guard *is* the convention here; `throw_if()` would also flatten guards and evaluate its exception argument eagerly even when nothing throws.
- `ThrowIfAndThrowUnlessExceptionsToUseClassStringRector` — inert today (no `throw_if()` exists), armed in advance so the decision lives in config rather than in remembered prose. Rector warns such rules are "never registered" until a set enables them; that warning is accepted deliberately.
- `AppToResolveRector` — `app()` 2 vs `resolve()` 2. Naming churn with no semantic value.

### Wave 0 carries exactly one rule

Three rules measured zero-diff, but only `RemoveDumpDataDeadCodeRector` ships. The other two are excluded on purpose:

- Neither has been proven to bite. A rule that reports nothing and has not been shown to detect anything is noise in the gate, not protection.
- `ReadOnlyClassRector` is a forward risk in a Filament/Livewire codebase, where components mutate properties by design.

`RemoveDumpDataDeadCodeRector` earns its place: it stops a forgotten `dd()` from reaching main, and it is proven to catch one.

### Gate wiring

```
"refactor": rector process && pint --dirty # write path
"refactor:dry": rector process --dry-run # gate path

"check" += @refactor:dry
"check-full" += @refactor:dry # job `quality` (required + strict) => red blocks merge
scripts/pre-commit += rector --dry-run step
```

The pre-commit hook does **not** call `check-full` — it runs its own steps — so the gate must be added there explicitly or the hook will not inherit it. The hook runs Rector over all of `app/`, matching how Pint and PHPStan already behave in that hook; at 182 files the cost is negligible next to the test run it already performs.

Ordering inside `check-full`: after `@pint-test`, before `@phpstan`. Style is settled first, so a Rector failure is never a formatting artifact.

## Verification

**Expected code diff: 0 files.** `composer refactor:dry` must exit 0 on the untouched tree.

**Violation probe — the gate must be proven to bite, or it is decorative:**

1. Clean baseline → `rc=0`.
2. Add a new isolated file containing a `dd()` → `rc=2`, and Rector must **name the file and the rule** (`app/ProbeVictim.php:6` / `* RemoveDumpDataDeadCodeRector`).
3. Remove the probe → `rc=0`, tree clean.

The probe must be attributable. During the pilot, a first attempt reported a false "bites" result: the script had corrupted a file with stray whitespace and the `rc=2` came from that, not from the `dd()` — the injection had in fact failed. Asserting on the exit code alone is insufficient; assert that Rector names the file and the rule, and confirm the tree is clean afterwards.

**Full gate:** `composer check-full` green before opening the PR.

## Waves (separate tickets)

| Wave | Content | Files | Nature |
|---|---|---|---|
| 1 | `strict_types` + `pint.json` | 73 in `app/` | **Semantic** — changes runtime coercion |
| 2 | `AddOverrideAttribute` | 48 | Mechanical |
| 3 | `LARAVEL_CODE_QUALITY` + `codeQuality` 50 | 28 + 22 | Mechanical |
| 4 | `LARAVEL_COLLECTION` + `deadCode` 50 | 4 + 3 | Manual review of public signatures |

Wave 1 carries a scope trap worth recording now: unblock's `pint.json` excludes `database`, `elk`, `config` and `notas`, but **not** `tests/`. Setting `declare_strict_types: true` in Pint would therefore rewrite the 175 files under `tests/` and `routes/` as well, not just the 73 under `app/`. Size that wave before starting it.

Each wave must re-prove its new rules bite before landing.
61 changes: 61 additions & 0 deletions rector.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
<?php

declare(strict_types=1);

use Rector\Config\RectorConfig;
use Rector\ValueObject\PhpVersion;
use RectorLaravel\Rector\FuncCall\{AppToResolveRector, RemoveDumpDataDeadCodeRector, ThrowIfAndThrowUnlessExceptionsToUseClassStringRector};
use RectorLaravel\Rector\If_\ThrowIfRector;

/**
* Rector gate — wave-based adoption (AID-528).
*
* CI runs `composer refactor:dry` as part of `composer check-full`, which is
* what the `quality` job executes. That job is a required status check on
* main with strict mode, so a pending diff turns CI red and blocks the merge.
* Rules are enabled in curated waves — each wave is its own mechanical PR
* verified by the full suite, and each wave must re-prove its new rules bite
* (violation probe → non-zero exit) before the wave PR lands.
*
* Path scope (deliberate, per AID-528):
* - app/ only for now. routes/, bootstrap/, config/ and tests/ are wave
* candidates, not oversights — extending scope is a wave decision.
* - database/ must NEVER be added: migrations are append-only history.
*
* Wave 0 carries exactly one rule. Three rules measured zero-diff in the
* pilot (RemoveDumpDataDeadCodeRector, ReadOnlyClassRector and the
* LARAVEL_TYPE_DECLARATIONS set), but only this one is proven to bite; a rule
* that reports nothing and has not been shown to detect anything is noise in
* the gate, not protection. ReadOnlyClassRector is additionally a forward
* risk here: Filament/Livewire components mutate properties by design.
*
* Convention ownership: unlike openmizan, this repo's pint.json does NOT set
* `declare_strict_types`, and app/ sits at 109/182. DeclareStrictTypesRector
* is therefore deliberately absent — enabling it is a semantic change over 73
* files (runtime type coercion) and belongs to its own wave, not here.
*/
return RectorConfig::configure()
->withPaths([
__DIR__.'/app',
])
// Follows the composer.json "php" floor (^8.3|^8.4). Bump together with the
// composer constraint so gate semantics never drift implicitly. This sets
// the transformation target, not the runtime — CI runs PHP 8.4.
->withPhpVersion(PhpVersion::PHP_83)
->withRules([
RemoveDumpDataDeadCodeRector::class,
])
->withSkip([
// Excluded by design decision (AID-528), armed BEFORE any wave enables
// a set that would register them — config, not remembered prose.
// Rector warns these rules are "never registered" until such a set
// lands; that warning is accepted deliberately.
//
// app/ holds 52 explicit `throw new` and zero `throw_if()`: the
// explicit guard is the convention. throw_if() also flattens guards and
// evaluates its exception argument eagerly even when nothing is thrown.
ThrowIfRector::class,
ThrowIfAndThrowUnlessExceptionsToUseClassStringRector::class,
// app() vs resolve() is naming churn with no semantic value (2 vs 2).
AppToResolveRector::class,
]);
20 changes: 17 additions & 3 deletions scripts/pre-commit
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#!/bin/bash
# Pre-commit hook for unblock project
# Install: composer run hook:install
# Checks: lock file sync, syntax, lint, PHPStan, tests
# Checks: lock file sync, syntax, lint, Rector, PHPStan, tests

set -e

Expand Down Expand Up @@ -91,7 +91,21 @@ else
pass "No PHP files to lint"
fi

# ── 4. PHPStan static analysis ──────────────────────────────────────
# ── 4. Rector (refactoring gate) ────────────────────────────────────
#
# Runs over all of app/, matching how Pint and PHPStan already behave here.
# Rules live in rector.php and are enabled in curated waves (AID-528).
# `composer refactor` applies the pending changes.

echo "Checking pending refactorings..."

vendor/bin/rector process --dry-run --no-progress-bar --no-diffs > /dev/null 2>&1 || {
fail "Rector found pending refactorings. Run 'composer refactor:dry' to review, 'composer refactor' to apply."
}

pass "Rector OK"

# ── 5. PHPStan static analysis ──────────────────────────────────────

echo "Running static analysis..."

Expand All @@ -101,7 +115,7 @@ vendor/bin/phpstan analyse --memory-limit=1G --no-progress --quiet 2>/dev/null |

pass "PHPStan OK"

# ── 5. Tests ─────────────────────────────────────────────────────────
# ── 6. Tests ─────────────────────────────────────────────────────────

echo "Running tests..."

Expand Down