Skip to content
Merged
Show file tree
Hide file tree
Changes from 7 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
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 @@
return new Description("{$theClass->getName()} should not be final", $because);
}

public function appliesTo(ClassDescription $theClass): bool

Check warning on line 21 in src/Expression/ForClasses/IsNotFinal.php

View check run for this annotation

Codecov / codecov/patch

src/Expression/ForClasses/IsNotFinal.php#L21

Added line #L21 was not covered by tests
{
return !($theClass->isInterface() || $theClass->isTrait() || $theClass->isEnum() || $theClass->isFinal());

Check warning on line 23 in src/Expression/ForClasses/IsNotFinal.php

View check run for this annotation

Codecov / codecov/patch

src/Expression/ForClasses/IsNotFinal.php#L23

Added line #L23 was not covered by tests
Copy link
Collaborator

Choose a reason for hiding this comment

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

this isFinal should not be there?

}

public function evaluate(ClassDescription $theClass, Violations $violations, string $because): void
{
if (!$theClass->isFinal()) {
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.

32 changes: 20 additions & 12 deletions tests/E2E/PHPUnit/CheckClassHaveAttributeTest.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 All @@ -8,54 +9,61 @@
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
{
$runner = TestRunner::create('8.4');

$set = ClassSet::fromDir(__DIR__.'/../_fixtures/mvc');

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

ArchRuleTestCase::assertArchRule($rule, $set);
$runner->run($set, $rule);

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

public function test_controllers_should_have_name_ending_in_controller(): void
{
$runner = TestRunner::create('8.4');

$set = ClassSet::fromDir(__DIR__.'/../_fixtures/mvc');

$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($set, $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
{
$runner = TestRunner::create('8.4');

$set = ClassSet::fromDir(__DIR__.'/../_fixtures/mvc');

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

ArchRuleTestCase::assertArchRule($rule, $set);
$runner->run($set, $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.

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

This file was deleted.

Loading