Skip to content

Commit 92dad93

Browse files
micheleorsellimicheleorselli
authored
Adds appliesTo to verify if a given rule should be applied (#454)
1 parent 3adbcec commit 92dad93

32 files changed

+647
-245
lines changed

Diff for: ‎.gitignore

+2-1
Original file line numberDiff line numberDiff line change
@@ -11,4 +11,5 @@ phparkitect.phar
1111
composer.lock
1212
.php-version
1313
composer.phar
14-
.phpunit.cache/
14+
.phpunit.cache/
15+
.vscode

Diff for: ‎.php-cs-fixer.dist.php

+1-2
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
->exclude('vendor/')
66
->notPath('tests/E2E/_fixtures/parse_error/Services/CartService.php');
77

8-
98
return (new PhpCsFixer\Config())
109
->setFinder($finder)
1110
->setRiskyAllowed(true)
@@ -24,7 +23,7 @@
2423
'modernize_types_casting' => true, // Replaces intval, floatval, doubleval, strval and boolval function calls with according type casting operator.
2524
'multiline_whitespace_before_semicolons' => true, // Forbid multi-line whitespace before the closing semicolon or move the semicolon to the new line for chained calls.
2625
'no_unreachable_default_argument_value' => true, // In function arguments there must not be arguments with default values before non-default ones.
27-
'no_superfluous_phpdoc_tags' => ['allow_mixed' => true],// To avoid problems of compatibility with the old php-cs-fixer version used on PHP 7.3
26+
'no_superfluous_phpdoc_tags' => ['allow_mixed' => true], // To avoid problems of compatibility with the old php-cs-fixer version used on PHP 7.3
2827
'no_useless_else' => true,
2928
'no_useless_return' => true,
3029
'ordered_class_elements' => true, // Orders the elements of classes/interfaces/traits.

Diff for: ‎src/Analyzer/FileVisitor.php

-9
Original file line numberDiff line numberDiff line change
@@ -224,15 +224,6 @@ public function enterNode(Node $node): void
224224
->addDependency(new ClassDependency($returnType->toString(), $returnType->getLine()));
225225
}
226226
}
227-
228-
if ($node instanceof Node\Attribute) {
229-
$nodeName = $node->name;
230-
231-
if ($nodeName instanceof Node\Name\FullyQualified) {
232-
$this->classDescriptionBuilder
233-
->addDependency(new ClassDependency(implode('\\', $nodeName->getParts()), $node->getLine()));
234-
}
235-
}
236227
}
237228

238229
public function getClassDescriptions(): array

Diff for: ‎src/CLI/PhpArkitectApplication.php

+1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
<?php
2+
23
declare(strict_types=1);
34

45
namespace Arkitect\CLI;

Diff for: ‎src/Expression/Expression.php

+11
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
<?php
2+
23
declare(strict_types=1);
34

45
namespace Arkitect\Expression;
@@ -20,6 +21,16 @@ interface Expression
2021
*/
2122
public function describe(ClassDescription $theClass, string $because): Description;
2223

24+
/**
25+
* Checks if the expression applies to the class passed as parameter.
26+
* If the current expression does not apply to the class, this method should return false.
27+
*
28+
* eg: IsAbstract does not applies to interfaces, traits, readonly classes
29+
*
30+
* Not included directly in the interface to allow incremental implementation of it in the rules.
31+
*/
32+
//public function appliesTo(ClassDescription $theClass): bool;
33+
2334
/**
2435
* Evaluates the expression for the class passed as parameter.
2536
* It should adds violations if rule is violated.

Diff for: ‎src/Expression/ForClasses/IsAbstract.php

+6-2
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,14 @@ public function describe(ClassDescription $theClass, string $because): Descripti
1818
return new Description("{$theClass->getName()} should be abstract", $because);
1919
}
2020

21+
public function appliesTo(ClassDescription $theClass): bool
22+
{
23+
return !($theClass->isInterface() || $theClass->isTrait() || $theClass->isEnum() || $theClass->isFinal());
24+
}
25+
2126
public function evaluate(ClassDescription $theClass, Violations $violations, string $because): void
2227
{
23-
if ($theClass->isAbstract() || $theClass->isInterface() || $theClass->isTrait() || $theClass->isEnum()
24-
|| $theClass->isFinal()) {
28+
if ($theClass->isAbstract()) {
2529
return;
2630
}
2731

Diff for: ‎src/Expression/ForClasses/IsFinal.php

+6-2
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,14 @@ public function describe(ClassDescription $theClass, string $because): Descripti
1818
return new Description("{$theClass->getName()} should be final", $because);
1919
}
2020

21+
public function appliesTo(ClassDescription $theClass): bool
22+
{
23+
return !($theClass->isInterface() || $theClass->isTrait() || $theClass->isEnum() || $theClass->isAbstract());
24+
}
25+
2126
public function evaluate(ClassDescription $theClass, Violations $violations, string $because): void
2227
{
23-
if ($theClass->isAbstract() || $theClass->isInterface() || $theClass->isFinal() || $theClass->isTrait()
24-
|| $theClass->isEnum()) {
28+
if ($theClass->isFinal()) {
2529
return;
2630
}
2731

Diff for: ‎src/Expression/ForClasses/IsNotAbstract.php

+5
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,11 @@ public function describe(ClassDescription $theClass, string $because): Descripti
1818
return new Description("{$theClass->getName()} should not be abstract", $because);
1919
}
2020

21+
public function appliesTo(ClassDescription $theClass): bool
22+
{
23+
return !($theClass->isInterface() || $theClass->isTrait() || $theClass->isEnum() || $theClass->isFinal());
24+
}
25+
2126
public function evaluate(ClassDescription $theClass, Violations $violations, string $because): void
2227
{
2328
if (!$theClass->isAbstract()) {

Diff for: ‎src/Expression/ForClasses/IsNotFinal.php

+5
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,11 @@ public function describe(ClassDescription $theClass, string $because): Descripti
1818
return new Description("{$theClass->getName()} should not be final", $because);
1919
}
2020

21+
public function appliesTo(ClassDescription $theClass): bool
22+
{
23+
return !($theClass->isInterface() || $theClass->isTrait() || $theClass->isEnum() || $theClass->isAbstract());
24+
}
25+
2126
public function evaluate(ClassDescription $theClass, Violations $violations, string $because): void
2227
{
2328
if (!$theClass->isFinal()) {

Diff for: ‎src/Expression/ForClasses/IsNotReadonly.php

+5
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,11 @@ public function describe(ClassDescription $theClass, string $because): Descripti
1818
return new Description("{$theClass->getName()} should not be readonly", $because);
1919
}
2020

21+
public function appliesTo(ClassDescription $theClass): bool
22+
{
23+
return !($theClass->isInterface() || $theClass->isTrait() || $theClass->isEnum());
24+
}
25+
2126
public function evaluate(ClassDescription $theClass, Violations $violations, string $because): void
2227
{
2328
if (!$theClass->isReadonly()) {

Diff for: ‎src/Expression/ForClasses/IsReadonly.php

+6-1
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,14 @@ public function describe(ClassDescription $theClass, string $because): Descripti
1818
return new Description("{$theClass->getName()} should be readonly", $because);
1919
}
2020

21+
public function appliesTo(ClassDescription $theClass): bool
22+
{
23+
return !($theClass->isInterface() || $theClass->isTrait() || $theClass->isEnum());
24+
}
25+
2126
public function evaluate(ClassDescription $theClass, Violations $violations, string $because): void
2227
{
23-
if ($theClass->isReadonly() || $theClass->isInterface() || $theClass->isTrait() || $theClass->isEnum()) {
28+
if ($theClass->isReadonly()) {
2429
return;
2530
}
2631

Diff for: ‎src/PHPUnit/ArchRuleCheckerConstraintAdapter.php

+8-1
Original file line numberDiff line numberDiff line change
@@ -66,11 +66,18 @@ protected function matches(
6666
$this->parsingErrors
6767
);
6868

69-
return 0 === $this->violations->count();
69+
$violationsCount = $this->violations->count();
70+
$parsingErrorsCount = $this->parsingErrors->count();
71+
72+
return 0 === $violationsCount && 0 === $parsingErrorsCount;
7073
}
7174

7275
protected function failureDescription($other): string
7376
{
77+
if ($this->parsingErrors->count() > 0) {
78+
return "\n".$this->parsingErrors->toString();
79+
}
80+
7481
return "\n".$this->violations->toString();
7582
}
7683
}

Diff for: ‎src/Rules/ArchRule.php

+1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
<?php
2+
23
declare(strict_types=1);
34

45
namespace Arkitect\Rules;

Diff for: ‎src/Rules/Constraints.php

+1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
<?php
2+
23
declare(strict_types=1);
34

45
namespace Arkitect\Rules;

Diff for: ‎src/Rules/Specs.php

+10
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
<?php
2+
23
declare(strict_types=1);
34

45
namespace Arkitect\Rules;
@@ -20,6 +21,15 @@ public function allSpecsAreMatchedBy(ClassDescription $classDescription, string
2021
{
2122
/** @var Expression $spec */
2223
foreach ($this->expressions as $spec) {
24+
// incremental way to introduce this method
25+
if (method_exists($spec, 'appliesTo')) {
26+
$canApply = $spec->appliesTo($classDescription);
27+
28+
if (false === $canApply) {
29+
return false;
30+
}
31+
}
32+
2333
$violations = new Violations();
2434
$spec->evaluate($classDescription, $violations, $because);
2535

Diff for: ‎tests/E2E/PHPUnit/ArchRuleTestCase.php

+1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
<?php
2+
23
declare(strict_types=1);
34

45
namespace Arkitect\Tests\E2E\PHPUnit;

Diff for: ‎tests/E2E/PHPUnit/CheckAttributeDependencyTest.php

-35
This file was deleted.

Diff for: ‎tests/E2E/PHPUnit/CheckClassHaveAttributeTest.php

+17-16
Original file line numberDiff line numberDiff line change
@@ -1,61 +1,62 @@
11
<?php
2+
23
declare(strict_types=1);
34

45
namespace Arkitect\Tests\E2E\PHPUnit;
56

6-
use Arkitect\ClassSet;
77
use Arkitect\Expression\ForClasses\HaveAttribute;
88
use Arkitect\Expression\ForClasses\HaveNameMatching;
99
use Arkitect\Expression\ForClasses\ResideInOneOfTheseNamespaces;
1010
use Arkitect\Rules\Rule;
11-
use PHPUnit\Framework\ExpectationFailedException;
11+
use Arkitect\Tests\Utils\TestRunner;
1212
use PHPUnit\Framework\TestCase;
1313

14-
/**
15-
* @requires PHP >= 8.0
16-
*/
1714
final class CheckClassHaveAttributeTest extends TestCase
1815
{
1916
public function test_entities_should_reside_in_app_model(): void
2017
{
21-
$set = ClassSet::fromDir(__DIR__.'/../_fixtures/mvc');
18+
$runner = TestRunner::create('8.4');
2219

2320
$rule = Rule::allClasses()
2421
->that(new HaveAttribute('Entity'))
2522
->should(new ResideInOneOfTheseNamespaces('App\Model'))
2623
->because('we use an ORM');
2724

28-
ArchRuleTestCase::assertArchRule($rule, $set);
25+
$runner->run(__DIR__.'/../_fixtures/mvc', $rule);
26+
27+
$this->assertCount(0, $runner->getViolations());
28+
$this->assertCount(0, $runner->getParsingErrors());
2929
}
3030

3131
public function test_controllers_should_have_name_ending_in_controller(): void
3232
{
33-
$set = ClassSet::fromDir(__DIR__.'/../_fixtures/mvc');
33+
$runner = TestRunner::create('8.4');
3434

3535
$rule = Rule::allClasses()
3636
->that(new HaveAttribute('AsController'))
3737
->should(new HaveNameMatching('*Controller'))
3838
->because('its a symfony thing');
3939

40-
$expectedExceptionMessage = '
41-
App\Controller\Foo has 1 violations
42-
should have a name that matches *Controller because its a symfony thing';
40+
$runner->run(__DIR__.'/../_fixtures/mvc', $rule);
4341

44-
$this->expectException(ExpectationFailedException::class);
45-
$this->expectExceptionMessage($expectedExceptionMessage);
42+
$this->assertCount(1, $runner->getViolations());
43+
$this->assertCount(0, $runner->getParsingErrors());
4644

47-
ArchRuleTestCase::assertArchRule($rule, $set);
45+
$this->assertEquals('App\Controller\Foo', $runner->getViolations()->get(0)->getFqcn());
4846
}
4947

5048
public function test_controllers_should_have_controller_attribute(): void
5149
{
52-
$set = ClassSet::fromDir(__DIR__.'/../_fixtures/mvc');
50+
$runner = TestRunner::create('8.4');
5351

5452
$rule = Rule::allClasses()
5553
->that(new HaveNameMatching('*Controller'))
5654
->should(new HaveAttribute('AsController'))
5755
->because('it configures the service container');
5856

59-
ArchRuleTestCase::assertArchRule($rule, $set);
57+
$runner->run(__DIR__.'/../_fixtures/mvc', $rule);
58+
59+
$this->assertCount(0, $runner->getViolations());
60+
$this->assertCount(0, $runner->getParsingErrors());
6061
}
6162
}

Diff for: ‎tests/E2E/PHPUnit/CheckClassWithMultipleExpressionsTest.php

-36
This file was deleted.

Diff for: ‎tests/E2E/_fixtures/attributes/Foo.php

-13
This file was deleted.

Diff for: ‎tests/E2E/_fixtures/attributes/Invalid/Attr.php

-12
This file was deleted.

0 commit comments

Comments
 (0)