diff --git a/README.md b/README.md index 882dc41a..860b670d 100644 --- a/README.md +++ b/README.md @@ -217,6 +217,15 @@ $rules[] = Rule::allClasses() ->because('we want uniform naming for services'); ``` +### Use a trait + +```php +$rules[] = Rule::allClasses() + ->that(new ResideInOneOfTheseNamespaces('Tests\Feature')) + ->should(new HaveTrait('Illuminate\Foundation\Testing\DatabaseTransactions')) + ->because('we want all Feature tests to run transactions'); +``` + ### Implements an interface ```php @@ -235,6 +244,15 @@ $rules[] = Rule::allClasses() ->because('all public controllers should not be container aware'); ``` +### Not use a trait + +```php +$rules[] = Rule::allClasses() + ->that(new ResideInOneOfTheseNamespaces('Tests\Feature')) + ->should(new NotHaveTrait('Illuminate\Foundation\Testing\RefreshDatabase')) + ->because('we want all Feature tests to never refresh the database for performance reasons'); +``` + ### Is abstract ```php diff --git a/src/Analyzer/ClassDescription.php b/src/Analyzer/ClassDescription.php index 93545a98..3d4127d0 100644 --- a/src/Analyzer/ClassDescription.php +++ b/src/Analyzer/ClassDescription.php @@ -25,6 +25,9 @@ class ClassDescription /** @var list */ private array $attributes; + /** @var list */ + private array $traits; + private bool $final; private bool $readonly; @@ -41,8 +44,9 @@ class ClassDescription * @param list $dependencies * @param list $interfaces * @param list $extends - * @param list $attributes * @param list $docBlock + * @param list $attributes + * @param list $traits */ public function __construct( FullyQualifiedClassName $FQCN, @@ -57,6 +61,7 @@ public function __construct( bool $enum, array $docBlock, array $attributes, + array $traits, string $filePath ) { $this->FQCN = $FQCN; @@ -69,6 +74,7 @@ public function __construct( $this->abstract = $abstract; $this->docBlock = $docBlock; $this->attributes = $attributes; + $this->traits = $traits; $this->interface = $interface; $this->trait = $trait; $this->enum = $enum; @@ -202,4 +208,23 @@ static function (bool $carry, FullyQualifiedClassName $attribute) use ($pattern) false ); } + + /** + * @return list + */ + public function getTraits(): array + { + return $this->traits; + } + + public function hasTrait(string $pattern): bool + { + return array_reduce( + $this->traits, + static function (bool $carry, FullyQualifiedClassName $trait) use ($pattern): bool { + return $carry || $trait->matches($pattern); + }, + false + ); + } } diff --git a/src/Analyzer/ClassDescriptionBuilder.php b/src/Analyzer/ClassDescriptionBuilder.php index fb137d3e..bc61bb2e 100644 --- a/src/Analyzer/ClassDescriptionBuilder.php +++ b/src/Analyzer/ClassDescriptionBuilder.php @@ -30,6 +30,9 @@ class ClassDescriptionBuilder /** @var list */ private array $attributes = []; + /** @var list */ + private array $traits = []; + private bool $interface = false; private bool $trait = false; @@ -49,6 +52,7 @@ public function clear(): void $this->abstract = false; $this->docBlock = []; $this->attributes = []; + $this->traits = []; $this->interface = false; $this->trait = false; $this->enum = false; @@ -148,6 +152,14 @@ public function addAttribute(string $FQCN, int $line): self return $this; } + public function addTrait(string $FQCN, int $line): self + { + $this->addDependency(new ClassDependency($FQCN, $line)); + $this->traits[] = FullyQualifiedClassName::fromString($FQCN); + + return $this; + } + public function build(): ClassDescription { Assert::notNull($this->FQCN, 'You must set an FQCN'); @@ -166,6 +178,7 @@ public function build(): ClassDescription $this->enum, $this->docBlock, $this->attributes, + $this->traits, $this->filePath ); } diff --git a/src/Analyzer/FileVisitor.php b/src/Analyzer/FileVisitor.php index 7ffedda9..b831d516 100644 --- a/src/Analyzer/FileVisitor.php +++ b/src/Analyzer/FileVisitor.php @@ -41,6 +41,9 @@ public function enterNode(Node $node): void // handles trait definition like trait MyTrait {} $this->handleTraitNode($node); + // handles trait usage like use MyTrait; + $this->handleTraitUseNode($node); + // handles code like $constantValue = StaticClass::constant; $this->handleStaticClassConstantNode($node); @@ -302,6 +305,18 @@ private function handleTraitNode(Node $node): void $this->classDescriptionBuilder->setTrait(true); } + private function handleTraitUseNode(Node $node): void + { + if (!($node instanceof Node\Stmt\TraitUse)) { + return; + } + + foreach ($node->traits as $trait) { + $this->classDescriptionBuilder + ->addTrait($trait->toString(), $trait->getLine()); + } + } + private function handleReturnTypeDependency(Node $node): void { if (!($node instanceof Node\Stmt\ClassMethod)) { diff --git a/src/Expression/ForClasses/HaveTrait.php b/src/Expression/ForClasses/HaveTrait.php new file mode 100644 index 00000000..a77c6b9d --- /dev/null +++ b/src/Expression/ForClasses/HaveTrait.php @@ -0,0 +1,42 @@ +trait = $trait; + } + + public function describe(ClassDescription $theClass, string $because): Description + { + return new Description("should use the trait {$this->trait}", $because); + } + + public function evaluate(ClassDescription $theClass, Violations $violations, string $because): void + { + if ($theClass->hasTrait($this->trait)) { + return; + } + + $violations->add( + Violation::create( + $theClass->getFQCN(), + ViolationMessage::selfExplanatory($this->describe($theClass, $because)), + $theClass->getFilePath() + ) + ); + } +} diff --git a/src/Expression/ForClasses/NotHaveTrait.php b/src/Expression/ForClasses/NotHaveTrait.php new file mode 100644 index 00000000..3f0c8c30 --- /dev/null +++ b/src/Expression/ForClasses/NotHaveTrait.php @@ -0,0 +1,56 @@ +trait = $trait; + } + + public function describe(ClassDescription $theClass, string $because): Description + { + return new Description("should not use the trait {$this->trait}", $because); + } + + public function appliesTo(ClassDescription $theClass): bool + { + return !($theClass->isInterface() || $theClass->isTrait()); + } + + public function evaluate(ClassDescription $theClass, Violations $violations, string $because): void + { + if ($theClass->isInterface() || $theClass->isTrait()) { + return; + } + + $trait = $this->trait; + $traits = $theClass->getTraits(); + $usesTrait = function (FullyQualifiedClassName $FQCN) use ($trait): bool { + return $FQCN->matches($trait); + }; + + if (\count(array_filter($traits, $usesTrait)) > 0) { + $violation = Violation::create( + $theClass->getFQCN(), + ViolationMessage::selfExplanatory($this->describe($theClass, $because)), + $theClass->getFilePath() + ); + $violations->add($violation); + } + } +} diff --git a/tests/Integration/CheckClassHaveTraitTest.php b/tests/Integration/CheckClassHaveTraitTest.php new file mode 100644 index 00000000..eb166e89 --- /dev/null +++ b/tests/Integration/CheckClassHaveTraitTest.php @@ -0,0 +1,141 @@ +createDirStructure())->url(); + + $runner = TestRunner::create('8.4'); + + $rule = Rule::allClasses() + ->that(new ResideInOneOfTheseNamespaces('Tests\Feature')) + ->should(new HaveTrait('DatabaseTransactions')) + ->because('we want all Feature tests to run transactions'); + + $runner->run($dir, $rule); + + self::assertCount(1, $runner->getViolations()); + self::assertCount(0, $runner->getParsingErrors()); + + self::assertEquals('Tests\Feature\UserFeatureTest', $runner->getViolations()->get(0)->getFqcn()); + } + + public function test_feature_tests_should_not_use_refresh_database_trait(): void + { + $dir = vfsStream::setup('root', null, $this->createDirStructure())->url(); + + $runner = TestRunner::create('8.4'); + + $rule = Rule::allClasses() + ->that(new ResideInOneOfTheseNamespaces('Tests\Feature')) + ->should(new NotHaveTrait('RefreshDatabase')) + ->because('we want all Feature tests to never refresh the database for performance reasons'); + + $runner->run($dir, $rule); + + self::assertCount(1, $runner->getViolations()); + self::assertCount(0, $runner->getParsingErrors()); + + self::assertEquals('Tests\Feature\ProductFeatureTest', $runner->getViolations()->get(0)->getFqcn()); + } + + public function test_classes_with_uuid_should_have_uuid_trait(): void + { + $dir = vfsStream::setup('root', null, $this->createDirStructure())->url(); + + $runner = TestRunner::create('8.4'); + + $rule = Rule::allClasses() + ->that(new ResideInOneOfTheseNamespaces('App\Models')) + ->should(new HaveTrait('HasUuid')) + ->because('all models should use UUID'); + + $runner->run($dir, $rule); + + self::assertCount(0, $runner->getViolations()); + self::assertCount(0, $runner->getParsingErrors()); + } + + public function createDirStructure(): array + { + return [ + 'Feature' => [ + 'OrderFeatureTest.php' => <<<'EOT' + <<<'EOT' + <<<'EOT' + [ + 'User.php' => <<<'EOT' + <<<'EOT' + setFilePath('src/Foo.php') + ->setClassName($FQCN) + ->addTrait('TraitClass', 15) + ->build(); + + self::assertEquals( + [FullyQualifiedClassName::fromString('TraitClass')], + $classDescription->getTraits() + ); + } + public function test_it_should_create_interface(): void { $FQCN = 'HappyIsland'; diff --git a/tests/Unit/Analyzer/ClassDescriptionTest.php b/tests/Unit/Analyzer/ClassDescriptionTest.php index 5a87a274..9d405acc 100644 --- a/tests/Unit/Analyzer/ClassDescriptionTest.php +++ b/tests/Unit/Analyzer/ClassDescriptionTest.php @@ -85,4 +85,23 @@ public function test_should_return_false_if_not_has_attribute(): void self::assertFalse($cd->hasAttribute('Bar')); } + + public function test_should_return_true_if_has_trait(): void + { + $cd = $this->builder + ->addTrait('FooTrait', 15) + ->build(); + + self::assertTrue($cd->hasTrait('FooTrait')); + self::assertTrue($cd->hasTrait('Foo*')); + } + + public function test_should_return_false_if_not_has_trait(): void + { + $cd = $this->builder + ->addTrait('FooTrait', 15) + ->build(); + + self::assertFalse($cd->hasTrait('Bar')); + } } diff --git a/tests/Unit/Expressions/ForClasses/HaveTraitTest.php b/tests/Unit/Expressions/ForClasses/HaveTraitTest.php new file mode 100644 index 00000000..76b1598d --- /dev/null +++ b/tests/Unit/Expressions/ForClasses/HaveTraitTest.php @@ -0,0 +1,71 @@ +setFilePath('src/Foo.php') + ->setClassName('HappyIsland\Myclass') + ->addTrait('MyTrait', 1) + ->build(); + + $because = 'we want to add this rule for our software'; + $violations = new Violations(); + $expression->evaluate($classDescription, $violations, $because); + + self::assertEquals(0, $violations->count()); + self::assertEquals( + 'should use the trait MyTrait because we want to add this rule for our software', + $expression->describe($classDescription, $because)->toString() + ); + } + + public function test_it_should_return_true_if_class_uses_trait_without_because(): void + { + $expression = new HaveTrait('MyTrait'); + + $classDescription = (new ClassDescriptionBuilder()) + ->setFilePath('src/Foo.php') + ->setClassName('HappyIsland\Myclass') + ->addTrait('MyTrait', 1) + ->build(); + + $violations = new Violations(); + $expression->evaluate($classDescription, $violations, ''); + + self::assertEquals(0, $violations->count()); + self::assertEquals( + 'should use the trait MyTrait', + $expression->describe($classDescription, '')->toString() + ); + } + + public function test_it_should_return_false_if_class_does_not_use_trait(): void + { + $expression = new HaveTrait('AnotherTrait'); + + $classDescription = (new ClassDescriptionBuilder()) + ->setFilePath('src/Foo.php') + ->setClassName('HappyIsland\Myclass') + ->addTrait('MyTrait', 1) + ->build(); + + $because = 'we want to add this rule for our software'; + $violations = new Violations(); + $expression->evaluate($classDescription, $violations, $because); + + self::assertEquals(1, $violations->count()); + } +} diff --git a/tests/Unit/Expressions/ForClasses/NotHaveTraitTest.php b/tests/Unit/Expressions/ForClasses/NotHaveTraitTest.php new file mode 100644 index 00000000..4ac8ddc0 --- /dev/null +++ b/tests/Unit/Expressions/ForClasses/NotHaveTraitTest.php @@ -0,0 +1,103 @@ +setFilePath('src/Foo.php') + ->setClassName('HappyIsland') + ->build(); + + $because = 'we want to add this rule for our software'; + $violations = new Violations(); + $traitConstraint->evaluate($classDescription, $violations, $because); + + self::assertEquals(0, $violations->count()); + } + + public function test_it_should_return_no_violation_if_class_uses_different_trait(): void + { + $traitConstraint = new NotHaveTrait('MyTrait'); + + $classDescription = (new ClassDescriptionBuilder()) + ->setFilePath('src/Foo.php') + ->setClassName('HappyIsland') + ->addTrait('AnotherTrait', 1) + ->build(); + + $because = 'we want to add this rule for our software'; + $violations = new Violations(); + $traitConstraint->evaluate($classDescription, $violations, $because); + + self::assertEquals(0, $violations->count()); + } + + public function test_it_should_return_violation_if_class_uses_trait(): void + { + $traitConstraint = new NotHaveTrait('MyTrait'); + + $classDescription = (new ClassDescriptionBuilder()) + ->setFilePath('src/Foo.php') + ->setClassName('HappyIsland') + ->addTrait('MyTrait', 1) + ->build(); + + $because = 'we want to add this rule for our software'; + $violations = new Violations(); + $traitConstraint->evaluate($classDescription, $violations, $because); + + $violationError = $traitConstraint->describe($classDescription, $because)->toString(); + + self::assertNotEquals(0, $violations->count()); + self::assertEquals( + 'should not use the trait MyTrait because we want to add this rule for our software', + $violationError + ); + } + + public function test_it_should_return_no_violation_if_is_an_interface(): void + { + $traitConstraint = new NotHaveTrait('MyTrait'); + + $classDescription = (new ClassDescriptionBuilder()) + ->setFilePath('src/Foo.php') + ->setClassName('HappyIsland') + ->setInterface(true) + ->build(); + + $because = 'we want to add this rule for our software'; + $violations = new Violations(); + $traitConstraint->evaluate($classDescription, $violations, $because); + + self::assertEquals(0, $violations->count()); + } + + public function test_it_should_return_no_violation_if_is_a_trait(): void + { + $traitConstraint = new NotHaveTrait('MyTrait'); + + $classDescription = (new ClassDescriptionBuilder()) + ->setFilePath('src/Foo.php') + ->setClassName('HappyIsland') + ->setTrait(true) + ->build(); + + $because = 'we want to add this rule for our software'; + $violations = new Violations(); + $traitConstraint->evaluate($classDescription, $violations, $because); + + self::assertEquals(0, $violations->count()); + } +}