Add rector, and split lint off test - #73
Merged
Merged
Conversation
DerManoMann
force-pushed
the
chore/rector
branch
from
September 20, 2026 04:16
2f4e478 to
3940f11
Compare
Running the linters as part of `composer test` couples a local test run to tooling no CI job invokes that way: code-style runs `composer lint`, static-analysis runs `composer analyse`, and the adapter jobs call phpunit directly. `test` is phpunit alone now.
`rector.php` matches the repo's floor (PHP 8.1) and enables the prepared sets plus TypedPropertyFromStrictConstructorRector; `composer rector` applies, and `composer analyse` now ends with a dry run. That placement is deliberate. Both frameworks are composer suggests, so rector only sees the adapters when something installs them first — which static-analysis already does for phpstan and code-style, where `lint` runs, does not. Run without Laravel it reads `tests/Adapters/LaravelAdapterTest` against a base class it cannot resolve and proposes to declare `$app` and delete `parent::setUp()`; neither is a change this repo wants. The `src` rewrites are that first run. One of its suggestions is dropped: typing the loader property in `VerifiesOpenApi` also dropped its `= null`, which leaves a typed property uninitialised and fatals on the trait's own lazy-init path — the one the Laravel and Slim adapters reach and the test suite overrides.
rector/rector ^1.2 requires phpstan ^1.11, which pulled the analyser back to 1.12.34 while main resolves 2.2.14 — that, not the rewrites, is why static-analysis reported six unmatched ignore patterns and one new error. Rector 2's code-quality set also wants to sort named arguments alphabetically, which would reorder the swagger-php attributes that decide the generated specification's own order; skip those two rules. The two remaining rewrites it proposes are applied. The typed $openapiSpecificationLoader makes one of the baselined "always false" negations in the adapter tests report differently, so two baseline entries follow it.
DerManoMann
force-pushed
the
chore/rector
branch
from
September 21, 2026 04:47
08d719c to
fccc3e0
Compare
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Two commits, both of which were sitting uncommitted in a working tree.
Split the lint step off
test.composer testranphpunitand then@lint. No CI jobuses it that way —
code-stylerunscomposer lint,static-analysisrunscomposer analyse,and the adapter jobs call
vendor/bin/phpunitdirectly — so the coupling only slowed down alocal test run.
testis phpunit alone now.Add rector.
rector.phpusesPhpVersion::PHP_81(matching"php": ">=8.1"and the8.1–8.4 CI matrix), the prepared sets, and
TypedPropertyFromStrictConstructorRector.composer rectorapplies it;composer analyseends with@rector --dry-run.Why the dry run lives with
analyseand not withlintlaravel/laravelandslim/slimare composersuggests, so nothing installs them by default.static-analysis.ymlalready doescomposer requirefor both beforecomposer analyse, becausephpstan is meaningless on the adapters without them.
code-style.yml, wherelintruns, installsneither.
Run without Laravel, rector reads
tests/Adapters/LaravelAdapterTestagainst a base class itcannot resolve and proposes two changes that are both wrong:
$appcomes fromIlluminate\Foundation\Testing\TestCaseand droppingparent::setUp()wouldskip Laravel's application bootstrap. Putting the check in the job that installs the frameworks
is what stops that being proposed on every run.
What the first rector run produced, and the one thing corrected
Kept: typed properties on
PSR17Middleware::$psrHttpFactoryandSlim\OpenApiVerifierMiddleware::$container, nestedif/else→elseif,'/' != $path[0]→!==,[0-9]+→\d+,if ($path)→if ($path !== []), andsetUp()visibility on theLaravel adapter test.
Corrected: typing the loader property in
VerifiesOpenApialso dropped its= null, leaving atyped property uninitialised.
getOpenApiSpecificationLoader()opens by reading it, so the firstcall fatals with
Typed property … must not be accessed before initialization. The suite staysgreen either way —
VerifiesOpenApiTestoverrides that accessor, and the code that does reach it(
Adapters\AbstractOpenApiResponseVerifier) is only exercised by the adapter tests, which skipunless the frameworks are installed. Restored to
= null.Verified
vendor/bin/phpunit— 28 tests, 8 skipped (Laravel and Slim absent locally), green.php-cs-fixer --dry-run— clean.rector --dry-run— clean apart from the twoframework-absent proposals above, which is what CI's framework-installing job is there to settle.