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/src/Rule/Rules/Class_/ExtendedClassMustBeAbstractOrInstantiatedRule.php b/src/Rule/Rules/Class_/ExtendedClassMustBeAbstractOrInstantiatedRule.php index c605ce22..4ee23af0 100644 --- a/src/Rule/Rules/Class_/ExtendedClassMustBeAbstractOrInstantiatedRule.php +++ b/src/Rule/Rules/Class_/ExtendedClassMustBeAbstractOrInstantiatedRule.php @@ -9,12 +9,17 @@ 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; + + private const PHPUNIT_TEST_SUFFIX = 'Test'; + public function __construct( private string $layer, private ?string $classNamePattern = null, @@ -31,6 +36,13 @@ public function appliesTo(ClassNode $classNode): bool return false; } + $hasTestSuffix = $classNode->nameEndsWith(self::PHPUNIT_TEST_SUFFIX); + $isTestCase = $classNode->extendsClass(self::PHPUNIT_TEST_CASE); + + if ($hasTestSuffix && $isTestCase) { + 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(