From 967598cda239bd8ed3d95885f1168f871a99d002 Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Mon, 7 Sep 2026 21:12:20 +0700 Subject: [PATCH 1/7] fix: Skip PHPUnit-run *Test classes in ExtendedClassMustBeAbstractOrInstantiatedRule --- docs/available-rules.md | 2 +- docs/presets.md | 2 +- ...dClassMustBeAbstractOrInstantiatedRule.php | 16 ++++++ tests/Analyser/AnalyserTest.php | 26 +++++++++ ...ssMustBeAbstractOrInstantiatedRuleTest.php | 56 ++++++++++++++++++- 5 files changed, 99 insertions(+), 3 deletions(-) diff --git a/docs/available-rules.md b/docs/available-rules.md index 8ab531ee..afd69859 100644 --- a/docs/available-rules.md +++ b/docs/available-rules.md @@ -84,7 +84,7 @@ Namespace: `Boundwize\StructArmed\Rule\Rules\Class_`. | `EnumCaseNameMustBePascalCaseRule` | `new EnumCaseNameMustBePascalCaseRule(layer: 'Source')` | Enum case names use PascalCase, per [PER Coding Style](https://www.php-fig.org/per/coding-style/#9-enumerations). | | `EnumConstantMayNotBeProtectedRule` | `new EnumConstantMayNotBeProtectedRule(layer: 'Source')` | Enum constants are not declared `protected` — enums cannot be extended, so `private` is used instead, per [PER Coding Style](https://www.php-fig.org/per/coding-style/#9-enumerations). Supports `--fix` by changing `protected` to `private`. | | `EnumMethodMayNotBeProtectedRule` | `new EnumMethodMayNotBeProtectedRule(layer: 'Source')` | Enum methods are not declared `protected` — enums cannot be extended, so `private` is used instead, per [PER Coding Style](https://www.php-fig.org/per/coding-style/#9-enumerations). Supports `--fix` by changing `protected` to `private`. | -| `ExtendedClassMustBeAbstractOrInstantiatedRule` | `new ExtendedClassMustBeAbstractOrInstantiatedRule(layer: 'Source')` | Classes another scanned class extends are declared `abstract` unless they are also instantiated (`new X`, a `new self`/`new static`/`new parent` resolving to them, a constant class expression such as `new (X::class)` or `new ('App\X')`, or a chained `(new ReflectionClass(X::class))->newInstance*()`). Type hints, `instanceof`, and `::class` keep working on an abstract class, so they do not count. Runtime-fed construction (`new $class` from a parameter, `unserialize()`, container factories) is outside the scanned-code boundary — exclude such factories' targets with rule-scoped `skip()` or `skipRule()`. Supports `--fix` by adding the `abstract` modifier. | +| `ExtendedClassMustBeAbstractOrInstantiatedRule` | `new ExtendedClassMustBeAbstractOrInstantiatedRule(layer: 'Source')` | Classes another scanned class extends are declared `abstract` unless they are also instantiated (`new X`, a `new self`/`new static`/`new parent` resolving to them, a constant class expression such as `new (X::class)` or `new ('App\X')`, or a chained `(new ReflectionClass(X::class))->newInstance*()`). Type hints, `instanceof`, and `::class` keep working on an abstract class, so they do not count. Runtime-fed construction (`new $class` from a parameter, `unserialize()`, container factories) is outside the scanned-code boundary — exclude such factories' targets with rule-scoped `skip()` or `skipRule()`. `*Test` classes extending `PHPUnit\Framework\TestCase` are skipped: the PHPUnit runner instantiates them, so a test another test extends must stay concrete (`*TestCase` base classes are still checked). Supports `--fix` by adding the `abstract` modifier. | | `MaxDependencyCountRule` | `new MaxDependencyCountRule(layer: 'Controller', maxCount: 5)` | Constructor dependency count stays below the configured limit. | | `MayNotExtendClassRule` | `new MayNotExtendClassRule(layer: 'Domain', class: 'Illuminate\\Database\\Eloquent\\Model')` | Classes in a layer do not extend a forbidden class, directly or through any parent class. | | `MayNotImplementInterfaceRule` | `new MayNotImplementInterfaceRule(layer: 'Domain', interface: JsonSerializable::class)` | Classes in a layer do not implement a forbidden interface. | diff --git a/docs/presets.md b/docs/presets.md index f3a7295f..ce876911 100644 --- a/docs/presets.md +++ b/docs/presets.md @@ -26,7 +26,7 @@ StructArmed ships with presets for common PHP standards and architecture styles. | `Preset::PSR15()` | `*Middleware` classes must implement PSR-15 `MiddlewareInterface`; `*Handler` classes must implement PSR-15 `RequestHandlerInterface`; StructArmed also enforces matching `Middleware`/`Handler` suffixes for implementations of those interfaces | | `Preset::DDD()` | Layer isolation, entity/VO/repository/event/service conventions, including keeping Doctrine ORM repository inheritance out of the Domain layer | | `Preset::MVC()` | Layer isolation, thin controllers, model/view/service rules, return types for helper functions | -| `Preset::YAGNI()` | Speculative-abstraction cleanup: interfaces must be implemented by a class or extended by another interface, abstract classes must be extended, traits must be used, and extended classes that are never instantiated must be abstract — a dependency reference (type hint, `instanceof`, `::class`, static call, a class-name string, ...) also counts as usage within the scanned paths, while only instantiation (`new X`, `new self`/`static`/`parent`, or a constant class expression such as `new (X::class)`) keeps an extended class concrete. All rules support `--fix`, removing the unused declaration or adding the `abstract` modifier | +| `Preset::YAGNI()` | Speculative-abstraction cleanup: interfaces must be implemented by a class or extended by another interface, abstract classes must be extended, traits must be used, and extended classes that are never instantiated must be abstract — a dependency reference (type hint, `instanceof`, `::class`, static call, a class-name string, ...) also counts as usage within the scanned paths, while only instantiation (`new X`, `new self`/`static`/`parent`, or a constant class expression such as `new (X::class)`) keeps an extended class concrete; `*Test` classes PHPUnit runs are skipped because the runner instantiates them. All rules support `--fix`, removing the unused declaration or adding the `abstract` modifier | | `Preset::CODEQUALITY()` | General readability conventions independent of any architecture style: closures and arrow functions that do not read `$this` must be declared `static`, and plain decimal numeric literals of `1_000_000` or more must group their digits with `_` separators (`1000500` becomes `1_000_500`). Both rules support `--fix`. Tune the literal threshold with `replaceRule(CodeQualityPreset::LARGE_NUMERIC_LITERALS_MUST_USE_SEPARATOR, new LargeNumericLiteralMustUseSeparatorRule(minimum: 1_000))` | ## Initialize Presets diff --git a/src/Rule/Rules/Class_/ExtendedClassMustBeAbstractOrInstantiatedRule.php b/src/Rule/Rules/Class_/ExtendedClassMustBeAbstractOrInstantiatedRule.php index c605ce22..7c7fb1ac 100644 --- a/src/Rule/Rules/Class_/ExtendedClassMustBeAbstractOrInstantiatedRule.php +++ b/src/Rule/Rules/Class_/ExtendedClassMustBeAbstractOrInstantiatedRule.php @@ -9,12 +9,23 @@ use Boundwize\StructArmed\Rule\Fixer\PhpParser\AbstractPhpParserFixableRule; use Boundwize\StructArmed\Rule\Fixer\PhpParser\Class_\AddAbstractClassVisitor; use Boundwize\StructArmed\Rule\RuleViolation; +use PHPUnit\Framework\TestCase; use function sprintf; final readonly class ExtendedClassMustBeAbstractOrInstantiatedRule extends AbstractPhpParserFixableRule implements ExtendedClassAwareRuleInterface { + private const PHPUNIT_TEST_CASE = TestCase::class; + + /** + * PHPUnit runs every `*Test` class it discovers, instantiating it outside + * the scanned code, so such a class must stay concrete even when another + * test extends it. Base test cases (`*TestCase`) are never run by + * themselves and may become abstract like any other extended class. + */ + private const PHPUNIT_TEST_SUFFIX = 'Test'; + public function __construct( private string $layer, private ?string $classNamePattern = null, @@ -31,6 +42,11 @@ public function appliesTo(ClassNode $classNode): bool return false; } + if ($classNode->nameEndsWith(self::PHPUNIT_TEST_SUFFIX) + && $classNode->extendsClass(self::PHPUNIT_TEST_CASE)) { + return false; + } + if ($this->classNamePattern !== null) { return $classNode->nameMatches($this->classNamePattern, isFullName: true); } diff --git a/tests/Analyser/AnalyserTest.php b/tests/Analyser/AnalyserTest.php index 1c7f6d89..c0b4c2cf 100644 --- a/tests/Analyser/AnalyserTest.php +++ b/tests/Analyser/AnalyserTest.php @@ -1023,6 +1023,32 @@ public function testExtendedClassMustBeAbstractOrInstantiatedRuleFlagsUninstanti $this->assertSame('App\BaseRepository', $violations[0]->className); } + public function testExtendedClassMustBeAbstractOrInstantiatedRuleSkipsPhpUnitTestCases(): void + { + // PHPUnit's TestCase is outside the scan; the transitive parent chain + // still records it, so the extended `*Test` class is recognised as a + // runner-instantiated test and kept concrete, while the concrete + // `*TestCase` base it extends is still reported. + $basePath = $this->makeTempProject([ + 'tests/CIUnitTestCase.php' => ' ' 'withPreset(Preset::YAGNI(sourcePaths: ['tests/'])); + + $violations = (new Analyser($basePath)) + ->analyse($architecture, [], null, AnalyserOptions::sequential()) + ->forRule(YagniPreset::EXTENDED_CLASS_MUST_BE_ABSTRACT_OR_INSTANTIATED); + + $this->assertCount(1, $violations); + $this->assertSame('App\\Tests\\CIUnitTestCase', $violations[0]->className); + } + public function testExtendedClassMustBeAbstractOrInstantiatedRuleFlagsTypeHintedButUninstantiatedParent(): void { $consumer = ' $parentClasses + */ private function makeNode( string $className = 'App\\Domain\\BaseRepository', string $layer = 'Domain', @@ -28,19 +31,22 @@ private function makeNode( bool $isExtended = false, bool $isReferenced = false, bool $isInstantiated = false, + ?string $extends = null, + array $parentClasses = [], ): ClassNode { return new ClassNode( className: $className, file: '/src/Domain/BaseRepository.php', line: 1, layer: $layer, - extends: null, + extends: $extends, isAbstract: $isAbstract, isFinal: false, isInterface: $isInterface, isReadonly: false, isTrait: $isTrait, isEnum: $isEnum, + parentClasses: $parentClasses, isExtended: $isExtended, isReferenced: $isReferenced, isInstantiated: $isInstantiated, @@ -161,6 +167,54 @@ public function testDoesNotApplyToAbstractClasses(): void $this->assertFalse($extendedClassMustBeAbstractOrInstantiatedRule->appliesTo($classNode)); } + public function testDoesNotApplyToPhpUnitTestCases(): void + { + // PHPUnit instantiates test classes at runtime, so a test another + // test extends is never `new`-ed in scanned code yet must stay concrete. + $extendedClassMustBeAbstractOrInstantiatedRule = new ExtendedClassMustBeAbstractOrInstantiatedRule( + layer: 'Domain' + ); + $classNode = $this->makeNode( + className: 'App\\Tests\\FormatRulesTest', + isExtended: true, + extends: 'App\\Tests\\CIUnitTestCase', + parentClasses: ['App\\Tests\\CIUnitTestCase', TestCase::class], + ); + + $this->assertFalse($extendedClassMustBeAbstractOrInstantiatedRule->appliesTo($classNode)); + } + + public function testAppliesToPhpUnitBaseTestCases(): void + { + // PHPUnit only runs `*Test` classes; a `*TestCase` base is never + // instantiated by the runner and may become abstract. + $extendedClassMustBeAbstractOrInstantiatedRule = new ExtendedClassMustBeAbstractOrInstantiatedRule( + layer: 'Domain' + ); + $classNode = $this->makeNode( + className: 'App\\Tests\\CIUnitTestCase', + isExtended: true, + extends: TestCase::class, + parentClasses: [TestCase::class], + ); + + $this->assertTrue($extendedClassMustBeAbstractOrInstantiatedRule->appliesTo($classNode)); + } + + public function testAppliesToExtendedClassOutsidePhpUnit(): void + { + $extendedClassMustBeAbstractOrInstantiatedRule = new ExtendedClassMustBeAbstractOrInstantiatedRule( + layer: 'Domain' + ); + $classNode = $this->makeNode( + isExtended: true, + extends: 'App\\Domain\\AbstractRepository', + parentClasses: ['App\\Domain\\AbstractRepository'], + ); + + $this->assertTrue($extendedClassMustBeAbstractOrInstantiatedRule->appliesTo($classNode)); + } + public function testDoesNotApplyToInterfaces(): void { $extendedClassMustBeAbstractOrInstantiatedRule = new ExtendedClassMustBeAbstractOrInstantiatedRule( From d9638c6335a49ab6923a82b1a59c4e23d6aa01d6 Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Mon, 7 Sep 2026 21:14:55 +0700 Subject: [PATCH 2/7] clean up --- docs/presets.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/presets.md b/docs/presets.md index ce876911..f3a7295f 100644 --- a/docs/presets.md +++ b/docs/presets.md @@ -26,7 +26,7 @@ StructArmed ships with presets for common PHP standards and architecture styles. | `Preset::PSR15()` | `*Middleware` classes must implement PSR-15 `MiddlewareInterface`; `*Handler` classes must implement PSR-15 `RequestHandlerInterface`; StructArmed also enforces matching `Middleware`/`Handler` suffixes for implementations of those interfaces | | `Preset::DDD()` | Layer isolation, entity/VO/repository/event/service conventions, including keeping Doctrine ORM repository inheritance out of the Domain layer | | `Preset::MVC()` | Layer isolation, thin controllers, model/view/service rules, return types for helper functions | -| `Preset::YAGNI()` | Speculative-abstraction cleanup: interfaces must be implemented by a class or extended by another interface, abstract classes must be extended, traits must be used, and extended classes that are never instantiated must be abstract — a dependency reference (type hint, `instanceof`, `::class`, static call, a class-name string, ...) also counts as usage within the scanned paths, while only instantiation (`new X`, `new self`/`static`/`parent`, or a constant class expression such as `new (X::class)`) keeps an extended class concrete; `*Test` classes PHPUnit runs are skipped because the runner instantiates them. All rules support `--fix`, removing the unused declaration or adding the `abstract` modifier | +| `Preset::YAGNI()` | Speculative-abstraction cleanup: interfaces must be implemented by a class or extended by another interface, abstract classes must be extended, traits must be used, and extended classes that are never instantiated must be abstract — a dependency reference (type hint, `instanceof`, `::class`, static call, a class-name string, ...) also counts as usage within the scanned paths, while only instantiation (`new X`, `new self`/`static`/`parent`, or a constant class expression such as `new (X::class)`) keeps an extended class concrete. All rules support `--fix`, removing the unused declaration or adding the `abstract` modifier | | `Preset::CODEQUALITY()` | General readability conventions independent of any architecture style: closures and arrow functions that do not read `$this` must be declared `static`, and plain decimal numeric literals of `1_000_000` or more must group their digits with `_` separators (`1000500` becomes `1_000_500`). Both rules support `--fix`. Tune the literal threshold with `replaceRule(CodeQualityPreset::LARGE_NUMERIC_LITERALS_MUST_USE_SEPARATOR, new LargeNumericLiteralMustUseSeparatorRule(minimum: 1_000))` | ## Initialize Presets From 836b1a01c247e3b7e85ef41ec60727e95fd5970d Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Mon, 7 Sep 2026 21:19:13 +0700 Subject: [PATCH 3/7] clean up --- ...xtendedClassMustBeAbstractOrInstantiatedRule.php | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/src/Rule/Rules/Class_/ExtendedClassMustBeAbstractOrInstantiatedRule.php b/src/Rule/Rules/Class_/ExtendedClassMustBeAbstractOrInstantiatedRule.php index 7c7fb1ac..74e7a77f 100644 --- a/src/Rule/Rules/Class_/ExtendedClassMustBeAbstractOrInstantiatedRule.php +++ b/src/Rule/Rules/Class_/ExtendedClassMustBeAbstractOrInstantiatedRule.php @@ -17,13 +17,6 @@ ExtendedClassAwareRuleInterface { private const PHPUNIT_TEST_CASE = TestCase::class; - - /** - * PHPUnit runs every `*Test` class it discovers, instantiating it outside - * the scanned code, so such a class must stay concrete even when another - * test extends it. Base test cases (`*TestCase`) are never run by - * themselves and may become abstract like any other extended class. - */ private const PHPUNIT_TEST_SUFFIX = 'Test'; public function __construct( @@ -42,8 +35,10 @@ public function appliesTo(ClassNode $classNode): bool return false; } - if ($classNode->nameEndsWith(self::PHPUNIT_TEST_SUFFIX) - && $classNode->extendsClass(self::PHPUNIT_TEST_CASE)) { + $hasTestSuffix = $classNode->nameEndsWith(self::PHPUNIT_TEST_SUFFIX); + $isExtendsPHPUnitTestCase = $classNode->extendsClass(self::PHPUNIT_TEST_CASE); + + if ($hasTestSuffix && $isExtendsPHPUnitTestCase) { return false; } From 6c835c793566cc46eefbab770a4c475c5c3e543d Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Mon, 7 Sep 2026 21:21:32 +0700 Subject: [PATCH 4/7] cs fix --- .../Class_/ExtendedClassMustBeAbstractOrInstantiatedRule.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Rule/Rules/Class_/ExtendedClassMustBeAbstractOrInstantiatedRule.php b/src/Rule/Rules/Class_/ExtendedClassMustBeAbstractOrInstantiatedRule.php index 74e7a77f..c8211de0 100644 --- a/src/Rule/Rules/Class_/ExtendedClassMustBeAbstractOrInstantiatedRule.php +++ b/src/Rule/Rules/Class_/ExtendedClassMustBeAbstractOrInstantiatedRule.php @@ -16,7 +16,7 @@ final readonly class ExtendedClassMustBeAbstractOrInstantiatedRule extends AbstractPhpParserFixableRule implements ExtendedClassAwareRuleInterface { - private const PHPUNIT_TEST_CASE = TestCase::class; + private const PHPUNIT_TEST_CASE = TestCase::class; private const PHPUNIT_TEST_SUFFIX = 'Test'; public function __construct( From edd37fc853ec76f4526eb0af049d27a978b062b3 Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Mon, 7 Sep 2026 21:22:30 +0700 Subject: [PATCH 5/7] clean up --- .../ExtendedClassMustBeAbstractOrInstantiatedRule.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Rule/Rules/Class_/ExtendedClassMustBeAbstractOrInstantiatedRule.php b/src/Rule/Rules/Class_/ExtendedClassMustBeAbstractOrInstantiatedRule.php index c8211de0..eaa9edbd 100644 --- a/src/Rule/Rules/Class_/ExtendedClassMustBeAbstractOrInstantiatedRule.php +++ b/src/Rule/Rules/Class_/ExtendedClassMustBeAbstractOrInstantiatedRule.php @@ -35,10 +35,10 @@ public function appliesTo(ClassNode $classNode): bool return false; } - $hasTestSuffix = $classNode->nameEndsWith(self::PHPUNIT_TEST_SUFFIX); - $isExtendsPHPUnitTestCase = $classNode->extendsClass(self::PHPUNIT_TEST_CASE); + $hasTestSuffix = $classNode->nameEndsWith(self::PHPUNIT_TEST_SUFFIX); + $isTestCase = $classNode->extendsClass(self::PHPUNIT_TEST_CASE); - if ($hasTestSuffix && $isExtendsPHPUnitTestCase) { + if ($hasTestSuffix && $isTestCase) { return false; } From 5fb668c5fc64dfc7ee74d394b6112be8605f894e Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Mon, 7 Sep 2026 21:24:16 +0700 Subject: [PATCH 6/7] rectify --- .../Class_/ExtendedClassMustBeAbstractOrInstantiatedRule.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Rule/Rules/Class_/ExtendedClassMustBeAbstractOrInstantiatedRule.php b/src/Rule/Rules/Class_/ExtendedClassMustBeAbstractOrInstantiatedRule.php index eaa9edbd..5836adcd 100644 --- a/src/Rule/Rules/Class_/ExtendedClassMustBeAbstractOrInstantiatedRule.php +++ b/src/Rule/Rules/Class_/ExtendedClassMustBeAbstractOrInstantiatedRule.php @@ -17,6 +17,7 @@ ExtendedClassAwareRuleInterface { private const PHPUNIT_TEST_CASE = TestCase::class; + private const PHPUNIT_TEST_SUFFIX = 'Test'; public function __construct( From 5e6d072590fa0db39d8ad0f804068d9d12611698 Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Mon, 7 Sep 2026 21:24:23 +0700 Subject: [PATCH 7/7] rectify --- .../Class_/ExtendedClassMustBeAbstractOrInstantiatedRule.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Rule/Rules/Class_/ExtendedClassMustBeAbstractOrInstantiatedRule.php b/src/Rule/Rules/Class_/ExtendedClassMustBeAbstractOrInstantiatedRule.php index 5836adcd..4ee23af0 100644 --- a/src/Rule/Rules/Class_/ExtendedClassMustBeAbstractOrInstantiatedRule.php +++ b/src/Rule/Rules/Class_/ExtendedClassMustBeAbstractOrInstantiatedRule.php @@ -16,7 +16,7 @@ final readonly class ExtendedClassMustBeAbstractOrInstantiatedRule extends AbstractPhpParserFixableRule implements ExtendedClassAwareRuleInterface { - private const PHPUNIT_TEST_CASE = TestCase::class; + private const PHPUNIT_TEST_CASE = TestCase::class; private const PHPUNIT_TEST_SUFFIX = 'Test';