Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/available-rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -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. |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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);
}
Expand Down
26 changes: 26 additions & 0 deletions tests/Analyser/AnalyserTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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' => '<?php namespace App\\Tests;'
. ' class CIUnitTestCase extends \\PHPUnit\\Framework\\TestCase {}',
'tests/FormatRulesTest.php' => '<?php namespace App\\Tests;'
. ' class FormatRulesTest extends CIUnitTestCase {}',
'tests/StrictFormatRulesTest.php' => '<?php namespace App\\Tests;'
. ' final class StrictFormatRulesTest extends FormatRulesTest {}',
]);

$architecture = Architecture::define()
->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 = '<?php namespace App;' . "\n"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@
#[CoversClass(AddAbstractClassVisitor::class)]
final class ExtendedClassMustBeAbstractOrInstantiatedRuleTest extends TestCase
{
/**
* @param list<string> $parentClasses
*/
private function makeNode(
string $className = 'App\\Domain\\BaseRepository',
string $layer = 'Domain',
Expand All @@ -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,
Expand Down Expand Up @@ -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(
Expand Down