From a627ece7803d9cbbed7095cfcaebe57868053928 Mon Sep 17 00:00:00 2001 From: Patrick Luca Fazzi Date: Tue, 22 Nov 2022 23:41:54 +0100 Subject: [PATCH 1/2] Implement no class feature --- src/CLI/TargetPhpVersion.php | 7 ++- src/Expression/NegateDecorator.php | 57 +++++++++++++++++++++ src/Rules/NoClass.php | 42 ++++++++++++++++ src/Rules/Rule.php | 5 ++ src/Rules/RuleBuilder.php | 14 +++++- src/Rules/Specs.php | 3 +- tests/Expression/NegateDecoratorTest.php | 56 +++++++++++++++++++++ tests/Unit/Rules/NoClassRulesTest.php | 64 ++++++++++++++++++++++++ 8 files changed, 243 insertions(+), 5 deletions(-) create mode 100644 src/Expression/NegateDecorator.php create mode 100644 src/Rules/NoClass.php create mode 100644 tests/Expression/NegateDecoratorTest.php create mode 100644 tests/Unit/Rules/NoClassRulesTest.php diff --git a/src/CLI/TargetPhpVersion.php b/src/CLI/TargetPhpVersion.php index 1da49ab5..09a1bb39 100644 --- a/src/CLI/TargetPhpVersion.php +++ b/src/CLI/TargetPhpVersion.php @@ -25,7 +25,12 @@ private function __construct(?string $version) $this->version = $version; } - public static function create(?string $version): self + /** + * @param ?string $version + * + * @throws PhpVersionNotValidException + */ + public static function create(?string $version = null): self { if (null === $version) { return new self(phpversion()); diff --git a/src/Expression/NegateDecorator.php b/src/Expression/NegateDecorator.php new file mode 100644 index 00000000..ce7a5774 --- /dev/null +++ b/src/Expression/NegateDecorator.php @@ -0,0 +1,57 @@ +expression = $expression; + } + + public function describe(ClassDescription $theClass, string $because): Description + { + $description = $this->expression->describe($theClass, $because)->toString(); + + $description = str_replace( + ['should not'], + ['should'], + $description, + $count + ); + + if (0 === $count) { + $description = str_replace( + ['should'], + ['should not'], + $description, + $count + ); + } + + return new Description($description, ''); + } + + public function evaluate(ClassDescription $theClass, Violations $violations, string $because): void + { + $this->expression->evaluate($theClass, $currentViolations = new Violations(), $because); + + if (0 === $currentViolations->count()) { + $violations->add( + Violation::create( + $theClass->getFQCN(), + ViolationMessage::selfExplanatory($this->describe($theClass, $because)) + ) + ); + } + } +} diff --git a/src/Rules/NoClass.php b/src/Rules/NoClass.php new file mode 100644 index 00000000..3c2bc180 --- /dev/null +++ b/src/Rules/NoClass.php @@ -0,0 +1,42 @@ +ruleBuilder = (new RuleBuilder())->negateShoulds(); + } + + public function should(Expression $expression): BecauseParser + { + $this->ruleBuilder->addShould(new NegateDecorator($expression)); + + return new Because($this->ruleBuilder); + } + + public function that(Expression $expression): AndThatShouldParser + { + $this->ruleBuilder->addThat($expression); + + return new AndThatShould($this->ruleBuilder); + } + + public function except(string ...$classesToBeExcluded): ThatParser + { + $this->ruleBuilder->classesToBeExcluded(...$classesToBeExcluded); + + return $this; + } +} diff --git a/src/Rules/Rule.php b/src/Rules/Rule.php index 6562da7e..7908aa9f 100644 --- a/src/Rules/Rule.php +++ b/src/Rules/Rule.php @@ -9,4 +9,9 @@ public static function allClasses(): AllClasses { return new AllClasses(); } + + public static function noClass(): NoClass + { + return new NoClass(); + } } diff --git a/src/Rules/RuleBuilder.php b/src/Rules/RuleBuilder.php index e6f12bf2..d8e2eca1 100644 --- a/src/Rules/RuleBuilder.php +++ b/src/Rules/RuleBuilder.php @@ -4,6 +4,7 @@ namespace Arkitect\Rules; use Arkitect\Expression\Expression; +use Arkitect\Expression\NegateDecorator; class RuleBuilder { @@ -18,9 +19,13 @@ class RuleBuilder /** @var array */ private $classesToBeExcluded; + /** @var bool */ private $runOnlyThis; + /** @var bool */ + private $negateShoulds; + public function __construct() { $this->thats = new Specs(); @@ -28,6 +33,7 @@ public function __construct() $this->because = ''; $this->classesToBeExcluded = []; $this->runOnlyThis = false; + $this->negateShoulds = false; } public function addThat(Expression $that): self @@ -39,6 +45,10 @@ public function addThat(Expression $that): self public function addShould(Expression $should): self { + if ($this->negateShoulds) { + $should = new NegateDecorator($should); + } + $this->shoulds->add($should); return $this; @@ -69,9 +79,9 @@ public function classesToBeExcluded(string ...$classesToBeExcluded): self return $this; } - public function setRunOnlyThis(): self + public function negateShoulds(): self { - $this->runOnlyThis = true; + $this->negateShoulds = true; return $this; } diff --git a/src/Rules/Specs.php b/src/Rules/Specs.php index c131d78f..ccb40006 100644 --- a/src/Rules/Specs.php +++ b/src/Rules/Specs.php @@ -8,7 +8,7 @@ class Specs { - /** @var array */ + /** @var list */ private $expressions = []; public function add(Expression $expression): void @@ -18,7 +18,6 @@ public function add(Expression $expression): void public function allSpecsAreMatchedBy(ClassDescription $classDescription, string $because): bool { - /** @var Expression $spec */ foreach ($this->expressions as $spec) { $violations = new Violations(); $spec->evaluate($classDescription, $violations, $because); diff --git a/tests/Expression/NegateDecoratorTest.php b/tests/Expression/NegateDecoratorTest.php new file mode 100644 index 00000000..cab22b2c --- /dev/null +++ b/tests/Expression/NegateDecoratorTest.php @@ -0,0 +1,56 @@ +setFinal(true) + ->get(); + + $isFinal = new IsFinal(); + + $isFinal->evaluate($finalClass, $violations = new Violations(), 'of some reason'); + self::assertEquals('FinalClass should be final because of some reason', $isFinal->describe($finalClass, 'of some reason')->toString()); + + self::assertEquals(0, $violations->count()); + + $isNotFinal = new NegateDecorator($isFinal); + + $isNotFinal->evaluate($finalClass, $violations = new Violations(), 'of some reason'); + self::assertEquals('FinalClass should not be final because of some reason', $isNotFinal->describe($finalClass, 'of some reason')->toString()); + + self::assertEquals(1, $violations->count()); + } + + public function test_negative_decoration(): void + { + $finalClass = ClassDescription::build('Tests\FinalClass') + ->setFinal(true) + ->get(); + + $isNotFinal = new IsNotFinal(); + + $isNotFinal->evaluate($finalClass, $violations = new Violations(), ''); + + self::assertEquals(1, $violations->count()); + self::assertEquals('FinalClass should not be final because of some reason', $isNotFinal->describe($finalClass, 'of some reason')->toString()); + + $isFinal = new NegateDecorator($isNotFinal); + + $isFinal->evaluate($finalClass, $violations = new Violations(), ''); + + self::assertEquals(0, $violations->count()); + self::assertEquals('FinalClass should be final because of some reason', $isFinal->describe($finalClass, 'of some reason')->toString()); + } +} diff --git a/tests/Unit/Rules/NoClassRulesTest.php b/tests/Unit/Rules/NoClassRulesTest.php new file mode 100644 index 00000000..1fc7273e --- /dev/null +++ b/tests/Unit/Rules/NoClassRulesTest.php @@ -0,0 +1,64 @@ +should(new NotResideInTheseNamespaces('App\Services')) + ->because('this namespace has been deprecated in favor of the modular architecture'); + + $classSet = ClassSet::fromDir(__DIR__.'/../../E2E/_fixtures/mvc'); + + $runner = new Runner(); + + $runner->check( + ClassSetRules::create($classSet, $rule), + new VoidProgress(), + FileParserFactory::createFileParser(TargetPhpVersion::create()), + $violations = new Violations(), + new ParsingErrors() + ); + + self::assertNotEmpty($violations->toArray()); + } + + public function test_no_class_dsl_works(): void + { + $rule = Rule::noClass() + ->that(new ResideInOneOfTheseNamespaces('App\Services')) + ->should(new HaveNameMatching('*Pippo')) + ->because('of our naming convention'); + + $classSet = ClassSet::fromDir(__DIR__.'/../../E2E/_fixtures/mvc'); + + $runner = new Runner(); + + $runner->check( + ClassSetRules::create($classSet, $rule), + new VoidProgress(), + FileParserFactory::createFileParser(TargetPhpVersion::create()), + $violations = new Violations(), + new ParsingErrors() + ); + + self::assertEmpty($violations->toArray()); + } +} From cd1fda22017b6f3488be8859f7411f7e8c2e5297 Mon Sep 17 00:00:00 2001 From: Patrick Luca Fazzi Date: Tue, 22 Nov 2022 23:45:00 +0100 Subject: [PATCH 2/2] Refactor --- tests/Unit/Rules/NoClassRulesTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/Unit/Rules/NoClassRulesTest.php b/tests/Unit/Rules/NoClassRulesTest.php index 1fc7273e..8dc2b742 100644 --- a/tests/Unit/Rules/NoClassRulesTest.php +++ b/tests/Unit/Rules/NoClassRulesTest.php @@ -43,8 +43,8 @@ public function test_no_class_without_that_clause_dsl_works(): void public function test_no_class_dsl_works(): void { $rule = Rule::noClass() - ->that(new ResideInOneOfTheseNamespaces('App\Services')) - ->should(new HaveNameMatching('*Pippo')) + ->that(new ResideInOneOfTheseNamespaces('App\Entity')) + ->should(new HaveNameMatching('*Service')) ->because('of our naming convention'); $classSet = ClassSet::fromDir(__DIR__.'/../../E2E/_fixtures/mvc');