Skip to content

Commit cc81afc

Browse files
authored
Add ability to check for multiple extends (phparkitect#432)
* Add ability to check for multiple Extends * Add ability to check for multiple NotExtends
1 parent 26bc759 commit cc81afc

File tree

6 files changed

+82
-31
lines changed

6 files changed

+82
-31
lines changed

README.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,8 @@ $rules[] = Rule::allClasses()
186186
->that(new ResideInOneOfTheseNamespaces('App\Controller'))
187187
->should(new Extend('App\Controller\AbstractController'))
188188
->because('we want to be sure that all controllers extend AbstractController');
189+
190+
You can add multiple parameters, the violation will happen when none of them match
189191
```
190192

191193
### Has an attribute (requires PHP >= 8.0)
@@ -348,6 +350,8 @@ $rules[] = Rule::allClasses()
348350
->that(new ResideInOneOfTheseNamespaces('App\Controller\Admin'))
349351
->should(new NotExtend('App\Controller\AbstractController'))
350352
->because('we want to be sure that all admin controllers not extend AbstractController for security reasons');
353+
354+
You can add multiple parameters, the violation will happen when one of them match
351355
```
352356

353357
### Don't have dependency outside a namespace

src/Expression/ForClasses/Extend.php

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -13,25 +13,30 @@
1313

1414
class Extend implements Expression
1515
{
16-
/** @var string */
17-
private $className;
16+
/** @var string[] */
17+
private $classNames;
1818

19-
public function __construct(string $className)
19+
public function __construct(string ...$classNames)
2020
{
21-
$this->className = $className;
21+
$this->classNames = $classNames;
2222
}
2323

2424
public function describe(ClassDescription $theClass, string $because): Description
2525
{
26-
return new Description("should extend {$this->className}", $because);
26+
$desc = implode(', ', $this->classNames);
27+
28+
return new Description("should extend one of these classes: {$desc}", $because);
2729
}
2830

2931
public function evaluate(ClassDescription $theClass, Violations $violations, string $because): void
3032
{
3133
$extends = $theClass->getExtends();
3234

33-
if (null !== $extends && $extends->matches($this->className)) {
34-
return;
35+
/** @var string $className */
36+
foreach ($this->classNames as $className) {
37+
if (null !== $extends && $extends->matches($className)) {
38+
return;
39+
}
3540
}
3641

3742
$violation = Violation::create(

src/Expression/ForClasses/NotExtend.php

Lines changed: 18 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -13,36 +13,37 @@
1313

1414
class NotExtend implements Expression
1515
{
16-
/** @var string */
17-
private $className;
16+
/** @var string[] */
17+
private $classNames;
1818

19-
public function __construct(string $className)
19+
public function __construct(string ...$classNames)
2020
{
21-
$this->className = $className;
21+
$this->classNames = $classNames;
2222
}
2323

2424
public function describe(ClassDescription $theClass, string $because): Description
2525
{
26-
return new Description("should not extend {$this->className}", $because);
26+
$desc = implode(', ', $this->classNames);
27+
28+
return new Description("should not extend one of these classes: {$desc}", $because);
2729
}
2830

2931
public function evaluate(ClassDescription $theClass, Violations $violations, string $because): void
3032
{
3133
$extends = $theClass->getExtends();
3234

33-
if (null === $extends) {
34-
return;
35-
}
35+
/** @var string $className */
36+
foreach ($this->classNames as $className) {
37+
if (null !== $extends && $extends->matches($className)) {
38+
$violation = Violation::create(
39+
$theClass->getFQCN(),
40+
ViolationMessage::selfExplanatory($this->describe($theClass, $because))
41+
);
3642

37-
if ($extends->toString() !== $this->className) {
38-
return;
39-
}
43+
$violations->add($violation);
4044

41-
$violation = Violation::create(
42-
$theClass->getFQCN(),
43-
ViolationMessage::selfExplanatory($this->describe($theClass, $because))
44-
);
45-
46-
$violations->add($violation);
45+
return;
46+
}
47+
}
4748
}
4849
}

tests/E2E/Cli/DebugExpressionCommandTest.php

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,16 +36,16 @@ public function test_some_classes_found(): void
3636
public function test_meaningful_errors_for_too_few_arguments_for_the_expression(): void
3737
{
3838
$appTester = $this->createAppTester();
39-
$appTester->run(['debug:expression', 'expression' => 'NotExtend', 'arguments' => [], '--from-dir' => __DIR__.'/../_fixtures/mvc/Domain']);
40-
$this->assertEquals("Error: Too few arguments for 'NotExtend'.\n", $appTester->getDisplay());
39+
$appTester->run(['debug:expression', 'expression' => 'NotImplement', 'arguments' => [], '--from-dir' => __DIR__.'/../_fixtures/mvc/Domain']);
40+
$this->assertEquals("Error: Too few arguments for 'NotImplement'.\n", $appTester->getDisplay());
4141
$this->assertEquals(2, $appTester->getStatusCode());
4242
}
4343

4444
public function test_meaningful_errors_for_too_many_arguments_for_the_expression(): void
4545
{
4646
$appTester = $this->createAppTester();
47-
$appTester->run(['debug:expression', 'expression' => 'NotExtend', 'arguments' => ['First', 'Second'], '--from-dir' => __DIR__.'/../_fixtures/mvc/Domain']);
48-
$this->assertEquals("Error: Too many arguments for 'NotExtend'.\n", $appTester->getDisplay());
47+
$appTester->run(['debug:expression', 'expression' => 'NotImplement', 'arguments' => ['First', 'Second'], '--from-dir' => __DIR__.'/../_fixtures/mvc/Domain']);
48+
$this->assertEquals("Error: Too many arguments for 'NotImplement'.\n", $appTester->getDisplay());
4949
$this->assertEquals(2, $appTester->getStatusCode());
5050
}
5151

tests/Unit/Expressions/ForClasses/ExtendTest.php

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ public function test_it_should_return_violation_error_when_class_not_extend(): v
7070
$extend->evaluate($classDescription, $violations, 'we want to add this rule for our software');
7171

7272
self::assertEquals(1, $violations->count());
73-
self::assertEquals('should extend My\BaseClass because we want to add this rule for our software', $violations->get(0)->getError());
73+
self::assertEquals('should extend one of these classes: My\BaseClass because we want to add this rule for our software', $violations->get(0)->getError());
7474
}
7575

7676
public function test_it_should_return_violation_error_if_extend_is_null(): void
@@ -97,6 +97,21 @@ public function test_it_should_return_violation_error_if_extend_is_null(): void
9797
$extend->evaluate($classDescription, $violations, $because);
9898

9999
self::assertEquals(1, $violations->count());
100-
self::assertEquals('should extend My\BaseClass because we want to add this rule for our software', $violationError);
100+
self::assertEquals('should extend one of these classes: My\BaseClass because we want to add this rule for our software', $violationError);
101+
}
102+
103+
public function test_it_should_accept_multiple_extends(): void
104+
{
105+
$extend = new Extend('My\FirstExtend', 'My\SecondExtend');
106+
107+
$classDescription = (new ClassDescriptionBuilder())
108+
->setClassName('My\Class')
109+
->setExtends('My\SecondExtend', 10)
110+
->build();
111+
112+
$violations = new Violations();
113+
$extend->evaluate($classDescription, $violations, 'because');
114+
115+
self::assertEquals(0, $violations->count());
101116
}
102117
}

tests/Unit/Expressions/ForClasses/NotExtendTest.php

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ public function test_it_should_return_violation_error(): void
3535
$notExtend->evaluate($classDescription, $violations, $because);
3636

3737
self::assertEquals(1, $violations->count());
38-
self::assertEquals('should not extend My\BaseClass because we want to add this rule for our software', $violationError);
38+
self::assertEquals('should not extend one of these classes: My\BaseClass because we want to add this rule for our software', $violationError);
3939
}
4040

4141
public function test_it_should_not_return_violation_error_if_extends_another_class(): void
@@ -62,4 +62,30 @@ public function test_it_should_not_return_violation_error_if_extends_another_cla
6262

6363
self::assertEquals(0, $violations->count());
6464
}
65+
66+
public function test_it_should_return_violation_error_for_multiple_extends(): void
67+
{
68+
$notExtend = new NotExtend('My\FirstExtend', 'My\SecondExtend');
69+
70+
$classDescription = new ClassDescription(
71+
FullyQualifiedClassName::fromString('HappyIsland'),
72+
[],
73+
[],
74+
FullyQualifiedClassName::fromString('My\SecondExtend'),
75+
false,
76+
false,
77+
false,
78+
false,
79+
false,
80+
false
81+
);
82+
$because = 'we want to add this rule for our software';
83+
$violationError = $notExtend->describe($classDescription, $because)->toString();
84+
85+
$violations = new Violations();
86+
$notExtend->evaluate($classDescription, $violations, $because);
87+
88+
self::assertEquals(1, $violations->count());
89+
self::assertEquals('should not extend one of these classes: My\FirstExtend, My\SecondExtend because we want to add this rule for our software', $violationError);
90+
}
6591
}

0 commit comments

Comments
 (0)