Skip to content

Add rector, and split lint off test - #73

Merged
DerManoMann merged 3 commits into
mainfrom
chore/rector
Sep 21, 2026
Merged

DerManoMann merged 3 commits into
mainfrom
chore/rector

Conversation

@DerManoMann

Copy link
Copy Markdown
Owner

Two commits, both of which were sitting uncommitted in a working tree.

Split the lint step off test. composer test ran phpunit and then @lint. No CI job
uses it that way — code-style runs composer lint, static-analysis runs composer analyse,
and the adapter jobs call vendor/bin/phpunit directly — so the coupling only slowed down a
local test run. test is phpunit alone now.

Add rector. rector.php uses PhpVersion::PHP_81 (matching "php": ">=8.1" and the
8.1–8.4 CI matrix), the prepared sets, and TypedPropertyFromStrictConstructorRector.
composer rector applies it; composer analyse ends with @rector --dry-run.

Why the dry run lives with analyse and not with lint

laravel/laravel and slim/slim are composer suggests, so nothing installs them by default.
static-analysis.yml already does composer require for both before composer analyse, because
phpstan is meaningless on the adapters without them. code-style.yml, where lint runs, installs
neither.

Run without Laravel, rector reads tests/Adapters/LaravelAdapterTest against a base class it
cannot resolve and proposes two changes that are both wrong:

+    public $app;                 // CompleteDynamicPropertiesRector
-        parent::setUp();         // RemoveParentCallWithoutParentRector

$app comes from Illuminate\Foundation\Testing\TestCase and dropping parent::setUp() would
skip 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::$psrHttpFactory and
Slim\OpenApiVerifierMiddleware::$container, nested if/else → elseif, '/' != $path[0] →
!==, [0-9]+ → \d+, if ($path) → if ($path !== []), and setUp() visibility on the
Laravel adapter test.

Corrected: typing the loader property in VerifiesOpenApi also dropped its = null, leaving a
typed property uninitialised. getOpenApiSpecificationLoader() opens by reading it, so the first
call fatals with Typed property … must not be accessed before initialization. The suite stays
green either way — VerifiesOpenApiTest overrides that accessor, and the code that does reach it
(Adapters\AbstractOpenApiResponseVerifier) is only exercised by the adapter tests, which skip
unless 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 two
framework-absent proposals above, which is what CI's framework-installing job is there to settle.

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
DerManoMann merged commit 09c29d3 into main Sep 21, 2026
27 of 28 checks passed
@DerManoMann
DerManoMann deleted the chore/rector branch September 21, 2026 05:22
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