From 14715d782794ada45a0828f9b9d3a7fcc16e49fe Mon Sep 17 00:00:00 2001 From: Gerard Oosterhof Date: Thu, 16 Jul 2026 09:37:18 +0200 Subject: [PATCH] =?UTF-8?q?refactor(rules):=20migrate=20deprecated=20isSub?= =?UTF-8?q?classOf(string)=20=E2=86=92=20isSubclassOfClass=20(queue=20#112?= =?UTF-8?q?)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit isSubclassOf(string) is @deprecated in PHPStan 2.2.2 and removed in 3.x — a latent break for every consumer's static analysis. Resolve the configured/known base FQCN via ReflectionProvider->hasClass()/getClass() and call isSubclassOfClass(ClassReflection), preserving the unknown-base-class no-op that keeps non-Laravel consumers unaffected. Pinned by a base-class-absent fixture. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_017kMpRQSwwg8zM5kYzbNArQ --- CHANGELOG.md | 4 ++ .../EnforceAuditModelProtectionsRule.php | 20 +++++++- src/Rules/EnforceFormRequestToDtoRule.php | 21 ++++++++- .../EnforceResourceDataValidatorOptInRule.php | 20 +++++++- .../EnforceAuditModelProtectionsRuleTest.php | 31 ++++++++++-- .../Rules/EnforceFormRequestToDtoRuleTest.php | 47 +++++++++++++++++-- ...orceResourceDataValidatorOptInRuleTest.php | 39 ++++++++++++++- 7 files changed, 171 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6366dbd..289efa2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and ## [Unreleased] +### Changed + +- `EnforceResourceDataValidatorOptInRule` + `EnforceFormRequestToDtoRule` + `EnforceAuditModelProtectionsRule` — migrated the internal inheritance gate off the deprecated `ClassReflection::isSubclassOf(string)` API onto `isSubclassOfClass(ClassReflection)` (war-room enforcement queue #112). `isSubclassOf(string)` is `@deprecated Use isSubclassOfClass instead.` in PHPStan 2.2+ and **removed in PHPStan 3.x** — a latent break for every consumer's static analysis the day this package (the fleet's canonical static-analysis backbone) targets PHPStan 3. Each of the three rules now injects `PHPStan\Reflection\ReflectionProvider` (constructor DI, autowired by the PHPStan/Nette container — no `extension.neon` argument wiring needed, proven by each rule's container-resolution test) and resolves its configured/known base FQCN via `reflectionProvider->hasClass()/getClass()` before calling `isSubclassOfClass()`. **Behaviour is byte-for-byte preserved:** the migration inlines the deprecated method's own body (`if (!hasClass($fqcn)) return false; return isSubclassOfClass(getClass($fqcn));`), so the load-bearing unknown-base-class no-op — a tree lacking the configured base class (or `Illuminate\Database\Eloquent\Model` for the audit rule) silently does not fire, the "consumers analysing non-Laravel trees are unaffected" guarantee — is reproduced exactly. Pinned by a new base-class-absent no-op test per rule (a configured base FQCN absent from the analysed tree ⇒ zero errors); all existing positive tests stay green. **Versioning: PATCH per ADR-0021 §Versioning** (internal API migration / future-proofing — adds no errors, removes none, no consumer-visible behaviour change). Seed: war-room enforcement queue #112. + ## [0.8.0] — 2026-07-13 **Release-as-a-whole: candidate MAJOR** — two entries. Both `EnforceActionResultDtoRule` (war-room enforcement queue #136) and `ForbidInlineArrayJsonResponseInControllersRule` (queue #137) surface new errors in already-clean consumer code (an `array`-returning `execute()`; an inline-array `JsonResponse` in a controller — see their bullets), so the release as a whole classifies as candidate MAJOR. Per the pre-1.0 caret convention `^0.7` excludes the next minor, so tagging auto-adopts nobody — each consumer adopts on its own pin-bump PR. Seed: kendo PR #1653 (KD-0220 central-user 2FA — queue #136 + #137). diff --git a/src/Rules/EnforceAuditModelProtectionsRule.php b/src/Rules/EnforceAuditModelProtectionsRule.php index a204d40..33b2630 100644 --- a/src/Rules/EnforceAuditModelProtectionsRule.php +++ b/src/Rules/EnforceAuditModelProtectionsRule.php @@ -12,6 +12,7 @@ use PHPStan\Analyser\Scope; use PHPStan\Node\InClassNode; use PHPStan\Reflection\ClassReflection; +use PHPStan\Reflection\ReflectionProvider; use PHPStan\Rules\IdentifierRuleError; use PHPStan\Rules\Rule; use PHPStan\Rules\RuleErrorBuilder; @@ -97,6 +98,7 @@ final class EnforceAuditModelProtectionsRule implements Rule * `AuditLog`) */ public function __construct( + private ReflectionProvider $reflectionProvider, private array $auditModelNamespacePrefixes = ['App\Models\Audit'], private array $auditModelNameSuffixes = ['AuditLog'], ) {} @@ -127,7 +129,23 @@ public function processNode(Node $node, Scope $scope): array // Type gate: a class merely NAMED like an audit log (a DTO, a service, // an enum) is not an audit model. Only Eloquent models carry the // trait / timestamp surface these protections govern. - if (!$classReflection->isSubclassOf(Model::class)) { + // + // Resolves `Model` to a `ClassReflection` and calls + // `isSubclassOfClass()` (the non-deprecated form; `isSubclassOf(string)` + // is `@deprecated` in PHPStan 2.2+ and removed in 3.x). The two guards + // reproduce the deprecated string form's body exactly + // (`if (!hasClass($fqcn)) return false; return isSubclassOfClass(...)`), + // preserving the no-op: a `Model`-absent tree, or a class that is not a + // Model subtype, silently does not fire (the non-Laravel-consumer + // guarantee). The `hasClass` guard is belt-and-suspenders — this + // package requires `illuminate/database`, so `Model` is resolvable in + // every real consumer env, and the reachable no-op is the + // not-a-subclass branch below. + if (!$this->reflectionProvider->hasClass(Model::class)) { + return []; + } + + if (!$classReflection->isSubclassOfClass($this->reflectionProvider->getClass(Model::class))) { return []; } diff --git a/src/Rules/EnforceFormRequestToDtoRule.php b/src/Rules/EnforceFormRequestToDtoRule.php index 35da9be..a797af6 100644 --- a/src/Rules/EnforceFormRequestToDtoRule.php +++ b/src/Rules/EnforceFormRequestToDtoRule.php @@ -10,6 +10,7 @@ use PHPStan\Analyser\Scope; use PHPStan\Node\InClassNode; use PHPStan\Reflection\ClassReflection; +use PHPStan\Reflection\ReflectionProvider; use PHPStan\Rules\Rule; use PHPStan\Rules\RuleErrorBuilder; @@ -99,6 +100,7 @@ final class EnforceFormRequestToDtoRule implements Rule * supplied only from consumer config */ public function __construct( + private ReflectionProvider $reflectionProvider, private string $formRequestBaseClass = FormRequest::class, private array $exemptClasses = [], ) {} @@ -157,9 +159,26 @@ public function processNode(Node $node, Scope $scope): array * abstract layers and namespace-relative `extends` clauses. Short-name * collisions in unrelated namespaces do not match, and the base class * itself is not a subclass of itself. + * + * Resolves the configured base FQCN to a `ClassReflection` and calls + * `isSubclassOfClass()` (the non-deprecated form; `isSubclassOf(string)` + * is `@deprecated` in PHPStan 2.2+ and removed in 3.x). When the base + * class is absent from the analysed tree (`hasClass()` false) the method + * returns `false` — reproducing the deprecated string form's own no-op + * exactly (its body is `if (!hasClass($fqcn)) return false;`). This is the + * load-bearing "consumers analysing non-Laravel trees are unaffected" + * guarantee: a tree with no `Illuminate\Foundation\Http\FormRequest` + * (the default base is stub-only, not a standalone Composer package) + * never fires the rule. */ private function extendsFormRequestBase(ClassReflection $classReflection): bool { - return $classReflection->isSubclassOf($this->formRequestBaseClass); + if (!$this->reflectionProvider->hasClass($this->formRequestBaseClass)) { + return false; + } + + return $classReflection->isSubclassOfClass( + $this->reflectionProvider->getClass($this->formRequestBaseClass), + ); } } diff --git a/src/Rules/EnforceResourceDataValidatorOptInRule.php b/src/Rules/EnforceResourceDataValidatorOptInRule.php index 1eaf24b..4550cb7 100644 --- a/src/Rules/EnforceResourceDataValidatorOptInRule.php +++ b/src/Rules/EnforceResourceDataValidatorOptInRule.php @@ -15,6 +15,7 @@ use PHPStan\Analyser\Scope; use PHPStan\Node\InClassNode; use PHPStan\Reflection\ClassReflection; +use PHPStan\Reflection\ReflectionProvider; use PHPStan\Rules\Rule; use PHPStan\Rules\RuleErrorBuilder; @@ -76,6 +77,7 @@ final class EnforceResourceDataValidatorOptInRule implements Rule private const string VALIDATOR_METHOD_NAME = 'validateRelationsLoaded'; public function __construct( + private ReflectionProvider $reflectionProvider, private string $resourceDataBaseClass = 'App\Http\Resources\ResourceData', ) {} @@ -129,6 +131,16 @@ public function processNode(Node $node, Scope $scope): array * configured base FQCN. Uses PHPStan reflection — handles intermediate * abstract layers and namespace-relative `extends` clauses. Short-name * collisions in unrelated namespaces do not match. + * + * Resolves the configured base FQCN to a `ClassReflection` and calls + * `isSubclassOfClass()` (the non-deprecated form; `isSubclassOf(string)` + * is `@deprecated` in PHPStan 2.2+ and removed in 3.x). When the base + * class is absent from the analysed tree (`hasClass()` false) the method + * returns `false` — reproducing the deprecated string form's own no-op + * exactly (its body is `if (!hasClass($fqcn)) return false;`). This is the + * load-bearing "consumers analysing non-Laravel trees are unaffected" + * guarantee: a tree with no `App\Http\Resources\ResourceData` never fires + * the rule. */ private function extendsResourceDataBase(ClassReflection $classReflection): bool { @@ -136,7 +148,13 @@ private function extendsResourceDataBase(ClassReflection $classReflection): bool return false; } - return $classReflection->isSubclassOf($this->resourceDataBaseClass); + if (!$this->reflectionProvider->hasClass($this->resourceDataBaseClass)) { + return false; + } + + return $classReflection->isSubclassOfClass( + $this->reflectionProvider->getClass($this->resourceDataBaseClass), + ); } /** diff --git a/tests/Rules/EnforceAuditModelProtectionsRuleTest.php b/tests/Rules/EnforceAuditModelProtectionsRuleTest.php index b6defcd..d54c715 100644 --- a/tests/Rules/EnforceAuditModelProtectionsRuleTest.php +++ b/tests/Rules/EnforceAuditModelProtectionsRuleTest.php @@ -231,7 +231,10 @@ public function testCustomSuffixParameterBringsModelIntoScope(): void // Configure `auditModelNameSuffixes: ['Trail']` — the model now matches // and its HasFactory violation fires. Proves the suffix parameter is // honoured end-to-end (default namespace prefixes retained). - $this->ruleOverride = new EnforceAuditModelProtectionsRule(auditModelNameSuffixes: ['Trail']); + $this->ruleOverride = new EnforceAuditModelProtectionsRule( + $this->createReflectionProvider(), + auditModelNameSuffixes: ['Trail'], + ); $this->analyse( [__DIR__ . '/../Fixtures/AuditModelProtections/LifecycleTrail.php'], @@ -258,7 +261,10 @@ public function testCustomNamespaceParameterBringsModelIntoScope(): void // Configure `auditModelNamespacePrefixes: ['App\Ledger']` — the model now // matches and its HasFactory violation fires. Proves the namespace-prefix // parameter is honoured end-to-end (default suffixes retained). - $this->ruleOverride = new EnforceAuditModelProtectionsRule(auditModelNamespacePrefixes: ['App\Ledger']); + $this->ruleOverride = new EnforceAuditModelProtectionsRule( + $this->createReflectionProvider(), + auditModelNamespacePrefixes: ['App\Ledger'], + ); $this->analyse( [__DIR__ . '/../Fixtures/AuditModelProtections/PaymentRecord.php'], @@ -306,6 +312,25 @@ public function testRuleResolvesFromExtensionNeonAndFires(): void ); } + public function testBaseModelResolutionIsNoOpForNonModelClass(): void + { + // The unknown-base no-op path for the migrated `isSubclassOfClass()` + // resolution (queue #112). `App\Support\FakeAuditLog` matches the + // `AuditLog` suffix signal but is NOT a subtype of Eloquent `Model`, so + // the migrated Model-resolution gate resolves `Model` (always present — + // this package requires `illuminate/database`), finds FakeAuditLog is + // not a subclass, and silently no-ops — reproducing the deprecated + // `isSubclassOf(Model::class)` false return exactly. The complementary + // `hasClass(Model::class)` false sub-branch (a genuinely Model-absent + // tree) mirrors the deprecated method body 1:1 but is unreachable in + // this package's analysis env because `Model` is a hard vendor + // dependency; the reachable no-op is this not-a-subclass path. + $this->analyse( + [__DIR__ . '/../Fixtures/AuditModelProtections/FakeAuditLog.php'], + [], + ); + } + /** * Load the shipped extension.neon so testRuleResolvesFromExtensionNeonAndFires * can pull the rule out of the container with its NEON-configured discovery @@ -322,6 +347,6 @@ public static function getAdditionalConfigFiles(): array protected function getRule(): Rule { - return $this->ruleOverride ?? new EnforceAuditModelProtectionsRule; + return $this->ruleOverride ?? new EnforceAuditModelProtectionsRule($this->createReflectionProvider()); } } diff --git a/tests/Rules/EnforceFormRequestToDtoRuleTest.php b/tests/Rules/EnforceFormRequestToDtoRuleTest.php index 05e7155..be51e54 100644 --- a/tests/Rules/EnforceFormRequestToDtoRuleTest.php +++ b/tests/Rules/EnforceFormRequestToDtoRuleTest.php @@ -173,7 +173,10 @@ public function testCustomBaseClassParameterMatchesAlternativeFqcn(): void // Re-run the same fixture with the parameter overridden to point at // the alternative base FQCN — must now fire. Proves the // `formRequestBaseClass` parameter is honored end-to-end. - $this->ruleOverride = new EnforceFormRequestToDtoRule('App\Unrelated\FormRequest'); + $this->ruleOverride = new EnforceFormRequestToDtoRule( + $this->createReflectionProvider(), + 'App\Unrelated\FormRequest', + ); $this->analyse( [__DIR__ . '/../Fixtures/FormRequestToDto/UnrelatedShortNameCollision.php'], @@ -190,7 +193,10 @@ public function testViolatorWithEmptyExemptListIsFlagged(): void { // Regression: the new exemptClasses param defaults to empty; default // behaviour must be unchanged (violator still fires). - $this->ruleOverride = new EnforceFormRequestToDtoRule(exemptClasses: []); + $this->ruleOverride = new EnforceFormRequestToDtoRule( + $this->createReflectionProvider(), + exemptClasses: [], + ); $this->analyse( [ @@ -211,6 +217,7 @@ public function testExemptClassByFqcnIsNotFlagged(): void // The violator's exact FQCN is in the exempt list — the class-keyed // consumer exemption path. No error. $this->ruleOverride = new EnforceFormRequestToDtoRule( + $this->createReflectionProvider(), exemptClasses: ['App\Http\Requests\ViolatorRequest'], ); @@ -229,6 +236,7 @@ public function testExemptionIsPreciseNotGlobalOffSwitch(): void // analysed in the same run — the exemption is precise, not a global // off-switch. $this->ruleOverride = new EnforceFormRequestToDtoRule( + $this->createReflectionProvider(), exemptClasses: ['App\Http\Requests\ViolatorRequest'], ); @@ -253,6 +261,7 @@ public function testExemptMatchIsExactFqcnNotShortNameOrOtherNamespace(): void // unrelated-namespace class of the same short name exempts the real // `App\Http\Requests\ViolatorRequest` — it must still fire. $this->ruleOverride = new EnforceFormRequestToDtoRule( + $this->createReflectionProvider(), exemptClasses: ['ViolatorRequest', 'App\Other\ViolatorRequest'], ); @@ -270,6 +279,38 @@ public function testExemptMatchIsExactFqcnNotShortNameOrOtherNamespace(): void ); } + public function testBaseClassAbsentFromTreeIsNoOp(): void + { + // The unknown-base-class no-op (queue #112). The rule is configured with + // a base FQCN that is absent from every analysed tree. The violator + // fixture structurally extends the (stubbed) real FormRequest base, but + // because the CONFIGURED base `App\Absent\NonExistentFormRequestBase` + // does not exist, the migrated `isSubclassOfClass()` resolution path + // takes the `hasClass()`-false branch and silently no-ops — reproducing + // the deprecated `isSubclassOf(string)` false return exactly (its body: + // `if (!hasClass($fqcn)) return false;`). This is the load-bearing + // "consumers analysing non-Laravel trees are unaffected" guarantee: a + // tree lacking the configured base never fires the rule. + // + // A bogus-base FQCN is used rather than omitting the framework stub: + // RuleTestCase analyses share ONE PHP process, so a stub required by an + // earlier test leaks `Illuminate\Foundation\Http\FormRequest` into + // runtime reflection for every later test, making "omit the stub" + // order-dependent and unsound. An always-absent FQCN is deterministic. + $this->ruleOverride = new EnforceFormRequestToDtoRule( + $this->createReflectionProvider(), + 'App\Absent\NonExistentFormRequestBase', + ); + + $this->analyse( + [ + __DIR__ . '/../Fixtures/FormRequestToDto/_stubs.php', + __DIR__ . '/../Fixtures/FormRequestToDto/ViolatorRequest.php', + ], + [], + ); + } + /** * Load the shipped extension.neon so testRuleResolvesFromExtensionNeonAndFires * can pull the rule out of the container with its NEON-configured @@ -286,6 +327,6 @@ public static function getAdditionalConfigFiles(): array protected function getRule(): Rule { - return $this->ruleOverride ?? new EnforceFormRequestToDtoRule; + return $this->ruleOverride ?? new EnforceFormRequestToDtoRule($this->createReflectionProvider()); } } diff --git a/tests/Rules/EnforceResourceDataValidatorOptInRuleTest.php b/tests/Rules/EnforceResourceDataValidatorOptInRuleTest.php index 0ab72d8..b1c9b1c 100644 --- a/tests/Rules/EnforceResourceDataValidatorOptInRuleTest.php +++ b/tests/Rules/EnforceResourceDataValidatorOptInRuleTest.php @@ -123,7 +123,10 @@ public function testCustomBaseClassParameterMatchesAlternativeFqcn(): void // Re-run the same fixture with the parameter overridden to point at // the alternative base FQCN — must now fire. Proves the // `resourceDataBaseClass` parameter is honored end-to-end. - $this->ruleOverride = new EnforceResourceDataValidatorOptInRule('App\Unrelated\ResourceData'); + $this->ruleOverride = new EnforceResourceDataValidatorOptInRule( + $this->createReflectionProvider(), + 'App\Unrelated\ResourceData', + ); $this->analyse( [__DIR__ . '/../Fixtures/ResourceDataValidatorOptIn/UnrelatedShortNameCollision.php'], @@ -161,6 +164,38 @@ public function testRuleResolvesFromExtensionNeonAndFires(): void ); } + public function testBaseClassAbsentFromTreeIsNoOp(): void + { + // The unknown-base-class no-op (queue #112). The rule is configured with + // a base FQCN that is absent from every analysed tree. The violator + // fixture structurally extends the (stubbed) real ResourceData base, but + // because the CONFIGURED base `App\Absent\NonExistentResourceDataBase` + // does not exist, the migrated `isSubclassOfClass()` resolution path + // takes the `hasClass()`-false branch and silently no-ops — reproducing + // the deprecated `isSubclassOf(string)` false return exactly (its body: + // `if (!hasClass($fqcn)) return false;`). This is the load-bearing + // "consumers analysing non-Laravel trees are unaffected" guarantee: a + // tree lacking the configured base never fires the rule. + // + // A bogus-base FQCN is used rather than omitting the stub: RuleTestCase + // analyses share ONE PHP process, so a stub required by an earlier test + // leaks `App\Http\Resources\ResourceData` into runtime reflection for + // every later test, making "omit the stub" order-dependent and unsound. + // An always-absent FQCN is deterministic. + $this->ruleOverride = new EnforceResourceDataValidatorOptInRule( + $this->createReflectionProvider(), + 'App\Absent\NonExistentResourceDataBase', + ); + + $this->analyse( + [ + __DIR__ . '/../Fixtures/ResourceDataValidatorOptIn/_stubs.php', + __DIR__ . '/../Fixtures/ResourceDataValidatorOptIn/ViolatorResource.php', + ], + [], + ); + } + /** * Load the shipped extension.neon so testRuleResolvesFromExtensionNeonAndFires * can pull the rule out of the container with its NEON-configured @@ -177,6 +212,6 @@ public static function getAdditionalConfigFiles(): array protected function getRule(): Rule { - return $this->ruleOverride ?? new EnforceResourceDataValidatorOptInRule; + return $this->ruleOverride ?? new EnforceResourceDataValidatorOptInRule($this->createReflectionProvider()); } }