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
3 changes: 2 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,5 @@ phparkitect.phar
composer.lock
.php-version
composer.phar
.phpunit.cache/
.phpunit.cache/
.vscode
3 changes: 1 addition & 2 deletions .php-cs-fixer.dist.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
->exclude('vendor/')
->notPath('tests/E2E/_fixtures/parse_error/Services/CartService.php');


return (new PhpCsFixer\Config())
->setFinder($finder)
->setRiskyAllowed(true)
Expand All @@ -24,7 +23,7 @@
'modernize_types_casting' => true, // Replaces intval, floatval, doubleval, strval and boolval function calls with according type casting operator.
'multiline_whitespace_before_semicolons' => true, // Forbid multi-line whitespace before the closing semicolon or move the semicolon to the new line for chained calls.
'no_unreachable_default_argument_value' => true, // In function arguments there must not be arguments with default values before non-default ones.
'no_superfluous_phpdoc_tags' => ['allow_mixed' => true],// To avoid problems of compatibility with the old php-cs-fixer version used on PHP 7.3
'no_superfluous_phpdoc_tags' => ['allow_mixed' => true], // To avoid problems of compatibility with the old php-cs-fixer version used on PHP 7.3
'no_useless_else' => true,
'no_useless_return' => true,
'ordered_class_elements' => true, // Orders the elements of classes/interfaces/traits.
Expand Down
9 changes: 0 additions & 9 deletions src/Analyzer/FileVisitor.php
Original file line number Diff line number Diff line change
Expand Up @@ -224,15 +224,6 @@ public function enterNode(Node $node): void
->addDependency(new ClassDependency($returnType->toString(), $returnType->getLine()));
}
}

if ($node instanceof Node\Attribute) {
$nodeName = $node->name;

if ($nodeName instanceof Node\Name\FullyQualified) {
$this->classDescriptionBuilder
->addDependency(new ClassDependency(implode('\\', $nodeName->getParts()), $node->getLine()));
}
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this snippet was adding the dependency derived by attributes twice

}

public function getClassDescriptions(): array
Expand Down
1 change: 1 addition & 0 deletions src/CLI/PhpArkitectApplication.php
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
<?php

declare(strict_types=1);

namespace Arkitect\CLI;
Expand Down
11 changes: 11 additions & 0 deletions src/Expression/Expression.php
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
<?php

declare(strict_types=1);

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

/**
* Checks if the expression applies to the class passed as parameter.
* If the current expression does not apply to the class, this method should return false.
*
* eg: IsAbstract does not applies to interfaces, traits, readonly classes
*
* Not included directly in the interface to allow incremental implementation of it in the rules.
*/
//public function appliesTo(ClassDescription $theClass): bool;

/**
* Evaluates the expression for the class passed as parameter.
* It should adds violations if rule is violated.
Expand Down
8 changes: 6 additions & 2 deletions src/Expression/ForClasses/IsAbstract.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,14 @@ public function describe(ClassDescription $theClass, string $because): Descripti
return new Description("{$theClass->getName()} should be abstract", $because);
}

public function appliesTo(ClassDescription $theClass): bool
{
return !($theClass->isInterface() || $theClass->isTrait() || $theClass->isEnum() || $theClass->isFinal());
}

public function evaluate(ClassDescription $theClass, Violations $violations, string $because): void
{
if ($theClass->isAbstract() || $theClass->isInterface() || $theClass->isTrait() || $theClass->isEnum()
|| $theClass->isFinal()) {
if ($theClass->isAbstract()) {
return;
}

Expand Down
8 changes: 6 additions & 2 deletions src/Expression/ForClasses/IsFinal.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,14 @@ public function describe(ClassDescription $theClass, string $because): Descripti
return new Description("{$theClass->getName()} should be final", $because);
}

public function appliesTo(ClassDescription $theClass): bool
{
return !($theClass->isInterface() || $theClass->isTrait() || $theClass->isEnum() || $theClass->isAbstract());
}

public function evaluate(ClassDescription $theClass, Violations $violations, string $because): void
{
if ($theClass->isAbstract() || $theClass->isInterface() || $theClass->isFinal() || $theClass->isTrait()
|| $theClass->isEnum()) {
if ($theClass->isFinal()) {
return;
}

Expand Down
5 changes: 5 additions & 0 deletions src/Expression/ForClasses/IsNotAbstract.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,11 @@ public function describe(ClassDescription $theClass, string $because): Descripti
return new Description("{$theClass->getName()} should not be abstract", $because);
}

public function appliesTo(ClassDescription $theClass): bool
{
return !($theClass->isInterface() || $theClass->isTrait() || $theClass->isEnum() || $theClass->isFinal());
}

public function evaluate(ClassDescription $theClass, Violations $violations, string $because): void
{
if (!$theClass->isAbstract()) {
Expand Down
5 changes: 5 additions & 0 deletions src/Expression/ForClasses/IsNotFinal.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,11 @@ public function describe(ClassDescription $theClass, string $because): Descripti
return new Description("{$theClass->getName()} should not be final", $because);
}

public function appliesTo(ClassDescription $theClass): bool
{
return !($theClass->isInterface() || $theClass->isTrait() || $theClass->isEnum() || $theClass->isAbstract());
}

public function evaluate(ClassDescription $theClass, Violations $violations, string $because): void
{
if (!$theClass->isFinal()) {
Expand Down
5 changes: 5 additions & 0 deletions src/Expression/ForClasses/IsNotReadonly.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,11 @@ public function describe(ClassDescription $theClass, string $because): Descripti
return new Description("{$theClass->getName()} should not be readonly", $because);
}

public function appliesTo(ClassDescription $theClass): bool
{
return !($theClass->isInterface() || $theClass->isTrait() || $theClass->isEnum());
}

public function evaluate(ClassDescription $theClass, Violations $violations, string $because): void
{
if (!$theClass->isReadonly()) {
Expand Down
7 changes: 6 additions & 1 deletion src/Expression/ForClasses/IsReadonly.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,14 @@ public function describe(ClassDescription $theClass, string $because): Descripti
return new Description("{$theClass->getName()} should be readonly", $because);
}

public function appliesTo(ClassDescription $theClass): bool
{
return !($theClass->isInterface() || $theClass->isTrait() || $theClass->isEnum());
}

public function evaluate(ClassDescription $theClass, Violations $violations, string $because): void
{
if ($theClass->isReadonly() || $theClass->isInterface() || $theClass->isTrait() || $theClass->isEnum()) {
if ($theClass->isReadonly()) {
return;
}

Expand Down
9 changes: 8 additions & 1 deletion src/PHPUnit/ArchRuleCheckerConstraintAdapter.php
Original file line number Diff line number Diff line change
Expand Up @@ -66,11 +66,18 @@
$this->parsingErrors
);

return 0 === $this->violations->count();
$violationsCount = $this->violations->count();
$parsingErrorsCount = $this->parsingErrors->count();

return 0 === $violationsCount && 0 === $parsingErrorsCount;
}

protected function failureDescription($other): string
{
if ($this->parsingErrors->count() > 0) {
return "\n".$this->parsingErrors->toString();

Check warning on line 78 in src/PHPUnit/ArchRuleCheckerConstraintAdapter.php

View check run for this annotation

Codecov / codecov/patch

src/PHPUnit/ArchRuleCheckerConstraintAdapter.php#L78

Added line #L78 was not covered by tests
}

return "\n".$this->violations->toString();
}
}
1 change: 1 addition & 0 deletions src/Rules/ArchRule.php
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
<?php

declare(strict_types=1);

namespace Arkitect\Rules;
Expand Down
1 change: 1 addition & 0 deletions src/Rules/Constraints.php
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
<?php

declare(strict_types=1);

namespace Arkitect\Rules;
Expand Down
10 changes: 10 additions & 0 deletions src/Rules/Specs.php
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
<?php

declare(strict_types=1);

namespace Arkitect\Rules;
Expand All @@ -20,6 +21,15 @@ public function allSpecsAreMatchedBy(ClassDescription $classDescription, string
{
/** @var Expression $spec */
foreach ($this->expressions as $spec) {
// incremental way to introduce this method
if (method_exists($spec, 'appliesTo')) {
$canApply = $spec->appliesTo($classDescription);

if (false === $canApply) {
return false;
}
}

$violations = new Violations();
$spec->evaluate($classDescription, $violations, $because);

Expand Down
1 change: 1 addition & 0 deletions tests/E2E/PHPUnit/ArchRuleTestCase.php
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
<?php

declare(strict_types=1);

namespace Arkitect\Tests\E2E\PHPUnit;
Expand Down
35 changes: 0 additions & 35 deletions tests/E2E/PHPUnit/CheckAttributeDependencyTest.php

This file was deleted.

33 changes: 17 additions & 16 deletions tests/E2E/PHPUnit/CheckClassHaveAttributeTest.php
Original file line number Diff line number Diff line change
@@ -1,61 +1,62 @@
<?php

declare(strict_types=1);

namespace Arkitect\Tests\E2E\PHPUnit;

use Arkitect\ClassSet;
use Arkitect\Expression\ForClasses\HaveAttribute;
use Arkitect\Expression\ForClasses\HaveNameMatching;
use Arkitect\Expression\ForClasses\ResideInOneOfTheseNamespaces;
use Arkitect\Rules\Rule;
use PHPUnit\Framework\ExpectationFailedException;
use Arkitect\Tests\Utils\TestRunner;
use PHPUnit\Framework\TestCase;

/**
* @requires PHP >= 8.0
*/
final class CheckClassHaveAttributeTest extends TestCase
{
public function test_entities_should_reside_in_app_model(): void
{
$set = ClassSet::fromDir(__DIR__.'/../_fixtures/mvc');
$runner = TestRunner::create('8.4');

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

ArchRuleTestCase::assertArchRule($rule, $set);
$runner->run(__DIR__.'/../_fixtures/mvc', $rule);

$this->assertCount(0, $runner->getViolations());
$this->assertCount(0, $runner->getParsingErrors());
}

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

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

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

$this->expectException(ExpectationFailedException::class);
$this->expectExceptionMessage($expectedExceptionMessage);
$this->assertCount(1, $runner->getViolations());
$this->assertCount(0, $runner->getParsingErrors());

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

public function test_controllers_should_have_controller_attribute(): void
{
$set = ClassSet::fromDir(__DIR__.'/../_fixtures/mvc');
$runner = TestRunner::create('8.4');

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

ArchRuleTestCase::assertArchRule($rule, $set);
$runner->run(__DIR__.'/../_fixtures/mvc', $rule);

$this->assertCount(0, $runner->getViolations());
$this->assertCount(0, $runner->getParsingErrors());
}
}
36 changes: 0 additions & 36 deletions tests/E2E/PHPUnit/CheckClassWithMultipleExpressionsTest.php

This file was deleted.

13 changes: 0 additions & 13 deletions tests/E2E/_fixtures/attributes/Foo.php

This file was deleted.

12 changes: 0 additions & 12 deletions tests/E2E/_fixtures/attributes/Invalid/Attr.php

This file was deleted.

Loading