Skip to content

Chore: Add Rector with safe rule bucket and CI refactor check#247

Merged
loks0n merged 1 commit intomainfrom
chore/rector
Apr 24, 2026
Merged

Chore: Add Rector with safe rule bucket and CI refactor check#247
loks0n merged 1 commit intomainfrom
chore/rector

Conversation

@loks0n
Copy link
Copy Markdown
Contributor

@loks0n loks0n commented Apr 24, 2026

Summary

  • Adds Rector with a conservative config (rector.php) targeting src/ and tests/, using PHP 8.3 level set plus CODE_QUALITY, DEAD_CODE, EARLY_RETURN, and INSTANCEOF rule sets. Rules with BC or behavioural risk are explicitly skipped — see config for the full list and rationale.
  • Adds composer refactor (apply) and composer refactor:check (dry-run) scripts.
  • Adds a Checks / Refactor CI job that runs rector in dry-run on PHP 8.3.
  • Applies the safe-bucket refactor across the codebase: declare(strict_types=1), #[\Override], str_contains/str_starts_with, null-coalescing, match (where safe), dead-code removal, docblock cleanup.

Skipped rules (worth revisiting in follow-ups)

  • ReadOnlyPropertyRector / ReadOnlyClassRector — BC break for library consumers.
  • ExplicitBoolCompareRector, SimplifyEmptyCheckOnEmptyArrayRector, DisallowedEmptyRuleFixerRector — change truthy/empty semantics for "0", null, "".
  • TypedPropertyFromAssignsRector, TypedPropertyFromStrictConstructorRector, ParamTypeByParentCallTypeRector, NullToStrictStringFuncCallArgRector — can throw TypeError on previously-working callers.
  • ClassPropertyAssignToConstructorPromotionRector, RestoreDefaultNullToNullableTypePropertyRector — shape/reflection changes.
  • RandomFunctionRector — different distribution and failure mode.
  • RecastingRemovalRector, FlipTypeControlToUseExclusiveTypeRector, SwitchNegatedTernaryRector, StringClassNameToClassConstantRector — subtle casting / control-flow shifts; apply manually.

Test plan

  • composer format clean
  • composer analyze (PHPStan level 7) clean
  • composer test unit suite passes (84 tests, 248 assertions)
  • CI Checks / Refactor job green
  • E2E suite green

Introduces Rector for automated refactoring, wired up via:

- rector.php config targeting src/ and tests/ with PHP 8.3 level set
  plus CODE_QUALITY, DEAD_CODE, EARLY_RETURN, and INSTANCEOF sets.
  Rules with BC risk or behavioural shifts (readonly, explicit bool
  compare, empty() rewrites, type-hint additions, random_int swaps)
  are explicitly skipped.
- composer refactor / refactor:check scripts.
- Checks / Refactor CI job running rector in dry-run.
- Safe-bucket refactor applied across the codebase: strict_types
  declarations, #[\Override] attributes, str_contains / str_starts_with,
  null-coalescing, match over switch where safe, dead code removal,
  and docblock cleanup.
@github-actions
Copy link
Copy Markdown

k6 benchmark

Throughput Requests Fail rate p50 p95
8089 req/s 566352 0% 4.61 ms 12.13 ms

@loks0n loks0n merged commit 938e3bc into main Apr 24, 2026
10 checks passed
@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented Apr 24, 2026

Greptile Summary

This PR introduces Rector with a conservative PHP 8.3 rule set and applies the resulting safe refactors across src/ and tests/: declare(strict_types=1) (partial), #[\Override], str_contains/str_starts_with, null-coalescing, match, arrow functions, and dead-code/docblock cleanup. The skipped rules are all well-justified. The only gap is that declare(strict_types=1) was not added to several core files — but since DeclareStrictTypesRector is not in the Rector config, the new rector:check CI job will not catch this inconsistency.

Confidence Score: 5/5

Safe to merge — all changes are mechanical refactors with no logic or BC impact.

All findings are P2 style/completeness issues. No logic bugs, security issues, or BC breaks were found. PHPStan level 7 and the unit suite both pass.

No files require special attention; the partial declare(strict_types=1) coverage is a minor follow-up item.

Important Files Changed

Filename Overview
.github/workflows/ci.yml Adds Checks / Refactor job that runs Rector dry-run on PHP 8.3; consistent with other jobs' action versions.
rector.php New Rector config targeting src/ and tests/ with PHP 8.3 level set; skipped rules are well-justified with explanations in comments.
composer.json Adds rector/rector ^2.0 to require-dev and composer refactor/refactor:check scripts.
src/Http/Http.php Clean mechanical refactors: arrow functions, match expressions, strict === comparisons, unused $key removed from injection loop; missing declare(strict_types=1) which was not in the base file either.
src/Http/Request.php Docblock cleanup, null coalescing, str_starts_with, first-class callable syntax; still missing declare(strict_types=1).
src/Http/Adapter/FPM/Request.php Docblock cleanup, null coalescing, #[\Override], match expression, str_starts_with; still missing declare(strict_types=1).
src/Http/View.php Arrow functions, str_contains replacing strpos, instanceof check replacing !empty() on typed nullable property; all changes are safe.
src/Http/Adapter/SwooleCoroutine/Server.php Removed explicit = null initializer from untyped property; safe since untyped properties default to null in PHP.
tests/e2e/Client.php Strict === comparisons, removed unused $responseType variable; clean dead-code removal.

Comments Outside Diff (1)

  1. src/Http/Http.php, line 1-3 (link)

    P2 declare(strict_types=1) partially applied

    The PR description lists declare(strict_types=1) as one of the refactors applied across the codebase, but several core files are still missing it after this PR: src/Http/Http.php, src/Http/Request.php, src/Http/Response.php, src/Http/Route.php, src/Http/Router.php, src/Http/View.php, src/Http/Files.php, src/Http/Adapter/FPM/Request.php, src/Http/Adapter/Swoole/Request.php, src/Http/Adapter/Swoole/Response.php, and tests/HttpTest.php / tests/RequestTest.php. Since DeclareStrictTypesRector does not appear to be enabled in rector.php, the rector:check CI job won't flag this gap — but it's worth completing the pass for consistency.

Reviews (1): Last reviewed commit: "Chore: Add Rector with safe rule bucket ..." | Re-trigger Greptile

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant