From 2876f34b870858dbf87c8ebb4f7f0e0df548cd73 Mon Sep 17 00:00:00 2001 From: atournayre Date: Sun, 3 Dec 2023 18:40:40 +0100 Subject: [PATCH 01/10] Remove Rule.php & Sets.php due to refactoring These two files have been removed as part of a project refactoring. The content of these files has been distributed across the project in a more organized manner, which increases maintainability and readability of code. Also, this helps to better adhere to the single responsibility principle. --- README.md | 23 +- src/Rule.php | 614 ------------------------------------ src/Rule/ApiPlaformRule.php | 84 +++++ src/Rule/ApiRule.php | 23 ++ src/Rule/DoctrineRule.php | 40 +++ src/Rule/DomainRule.php | 55 ++++ src/Rule/LogRule.php | 55 ++++ src/Rule/MiscRule.php | 269 ++++++++++++++++ src/Rule/Rule.php | 38 +++ src/Rule/SymfonyRule.php | 174 ++++++++++ src/Set/Sets.php | 294 +++++++++++++++++ src/Sets.php | 355 --------------------- 12 files changed, 1049 insertions(+), 975 deletions(-) delete mode 100644 src/Rule.php create mode 100644 src/Rule/ApiPlaformRule.php create mode 100644 src/Rule/ApiRule.php create mode 100644 src/Rule/DoctrineRule.php create mode 100644 src/Rule/DomainRule.php create mode 100644 src/Rule/LogRule.php create mode 100644 src/Rule/MiscRule.php create mode 100644 src/Rule/Rule.php create mode 100644 src/Rule/SymfonyRule.php create mode 100644 src/Set/Sets.php delete mode 100644 src/Sets.php diff --git a/README.md b/README.md index ea16067..9299b64 100644 --- a/README.md +++ b/README.md @@ -18,15 +18,26 @@ Here is an example of how to use it: // phparkitect.php use Arkitect\ClassSet; use Arkitect\CLI\Config; +use Atournayre\PHPArkitect\Rule\LogRule; +use Atournayre\PHPArkitect\Rule\MiscRule; +use Atournayre\PHPArkitect\Set\Sets; return static function (Config $config): void { $classSet = ClassSet::fromDir(__DIR__ . '/src'); - - $rules = \Atournayre\PHPArkitect\Sets::symfonyApiPlatform(); - $rules[] = \Atournayre\PHPArkitect\Rule::uniformNamingForService(); - - $config - ->add($classSet, ...$rules); + + // Add all rules for API Platform + $config->add($classSet, ...Sets::apiPlatform()); + // Add all rules for Symfony + $config->add($classSet, ...Sets::symfony()); + // Add subset of rules for Doctrine + $config->add($classSet, ...Sets::doctrineUniformNaming()); + // Add specific rules + $config->add( + $classSet, + LogRule::listenerMustBeLoggable(), + MiscRule::dtoMustNotEndWithDto(), + MiscRule::traits(), + ); }; ``` You can use sets or rules individually. diff --git a/src/Rule.php b/src/Rule.php deleted file mode 100644 index ae6768a..0000000 --- a/src/Rule.php +++ /dev/null @@ -1,614 +0,0 @@ -that(new ResideInOneOfTheseNamespaces('App\\' . $nameMatching)) - ->should(new HaveNameMatching('*' . $nameMatching)) - ->because('Classes should have a name matching ' . $nameMatching); - } - - public static function apiDocumentationOpenApi(): ArchRule - { - return BaseRule::allClasses() - ->that(new ResideInOneOfTheseNamespaces('App\\Api\\Documentation')) - ->should(new HaveNameMatching('/OpenApi$/')) - ->because('API Documentation should have a name with "OpenApi" inside'); - } - - public static function uniformNamingForApiResource(): ArchRule - { - return self::uniformNaming('ApiResource'); - } - - public static function apiResourceShouldBeFinal(): ArchRule - { - return BaseRule::allClasses() - ->that(new ResideInOneOfTheseNamespaces('App\\ApiResource')) - ->should(new IsFinal()) - ->because('ApiResource are not meant to be extended'); - } - - public static function uniformNamingForCollection(): ArchRule - { - return self::uniformNaming('Collection'); - } - - public static function collectionDependencies(): ArchRule - { - return BaseRule::allClasses() - ->that(new ResideInOneOfTheseNamespaces('App\\Collection')) - ->should(new DependsOnlyOnTheseNamespaces( - 'App\\Entity', - 'Carbon\\Carbon', - 'DateTimeInterface', - 'Doctrine\\Common\\Collections\\Collection', - 'Doctrine\\Common\\Collections\\Criteria', - 'Doctrine\\Common\\Collections\\ArrayCollection', - 'Doctrine\\Common\\Collections\\ReadableCollection', - 'Doctrine\\ORM\PersistentCollection', - Assert::class, - 'Symfony\\Contracts\\EventDispatcher\\Event' - )) - ->because('Collection should only depend on Entity, Carbon, DateTimeInterface, Doctrine Collection, Doctrine Criteria, Doctrine ReadableCollection, Doctrine PersistentCollection, Webmozart Assert and Symfony EventDispatcher Event'); - } - - public static function uniformNamingForCommand(): ArchRule - { - return self::uniformNaming('Command'); - } - - public static function commandMustUseSymfonyAsCommandAttribute(): ArchRule - { - return BaseRule::allClasses() - ->that(new ResideInOneOfTheseNamespaces('App\\Command')) - ->should(new DependsOnTheseNamespaces('Symfony\\Component\\Console\\Attribute\\AsCommand')) - ->because('Command should have dependency on AsCommand attribute from Symfony Console'); - } - - public static function commandMustExtendSymfonyCommand(): ArchRule - { - return BaseRule::allClasses() - ->that(new ResideInOneOfTheseNamespaces('App\\Command')) - ->should(new DependsOnTheseNamespaces('Symfony\\Component\\Console\\Command\\Command')) - ->because('Command should have dependency on Symfony Console Command'); - } - - public static function commandMustUseSymfonyStopwatch(): ArchRule - { - return BaseRule::allClasses() - ->that(new ResideInOneOfTheseNamespaces('App\\Command')) - ->should(new DependsOnTheseNamespaces('Symfony\\Component\\Stopwatch\\Stopwatch')) - ->because('We use Stopwatch to measure time of command execution'); - } - - public static function dtoMustNotEndWithDto(): ArchRule - { - return BaseRule::allClasses() - ->that(new ResideInOneOfTheseNamespaces('App\\DTO')) - ->should(new NotHaveNameMatching('*DTO')) - ->because('DTO should not be suffixed by DTO'); - } - - public static function dtoMustBeFinal(): ArchRule - { - return BaseRule::allClasses() - ->that(new ResideInOneOfTheseNamespaces('App\\DTO')) - ->should(new IsFinal()) - ->because('Extending a DTO is not a good practice'); - } - - public static function uniformNamingForContracts(): ArchRule - { - return self::uniformNaming('Interface'); - } - - public static function contractsDependencies(): ArchRule - { - return BaseRule::allClasses() - ->that(new ResideInOneOfTheseNamespaces('App\\Contracts')) - ->should(new DependsOnlyOnTheseNamespaces( - 'App\\Entity', - 'App\\Entity', - 'App\\DTO', - 'AppcontrollerMustUseSymfonRouteAttributeVO', - 'DateTimeInterface', - )) - ->because('Interface should only depend on Entity, DTO, VO and DateTimeInterface'); - } - - public static function uniformNamingForController(): ArchRule - { - return self::uniformNaming('Controller'); - } - - public static function controllerMustUseSymfonRouteAttribute(): ArchRule - { - return BaseRule::allClasses() - ->that(new ResideInOneOfTheseNamespaces('App\\Controller')) - ->should(new DependsOnTheseNamespaces('Symfony\\Component\\Routing\\Annotation\\Route')) - ->because('Controller should have dependency on Route attribute from Symfony Routing'); - } - - public static function controllerDependencies(): ArchRule - { - return BaseRule::allClasses() - ->that(new ResideInOneOfTheseNamespaces('App\\Controller')) - ->should(new DependsOnTheseNamespaces( - 'App\\Entity', - 'App\\Form', - 'App\\Repository', - 'App\\Service', - 'Symfony\\Bundle\\FrameworkBundle\\Controller\\AbstractController', - 'Symfony\\Component\\HttpFoundation\\JsonResponse', - 'Symfony\\Component\\HttpFoundation\\RedirectResponse', - 'Symfony\\Component\\HttpFoundation\\Request', - 'Symfony\\Component\\HttpFoundation\\Response', - 'Symfony\\Component\\Security\\Http\\Attribute\\IsGranted', - )) - ->because('Controller should have dependency on AbstractController, Response, Request, RedirectResponse, JsonResponse, IsGranted attribute from Symfony Security, Entity, Form, Repository and Service'); - } - - public static function uniformNamingForDecorator(): ArchRule - { - return self::uniformNaming('Decorator'); - } - - public static function decoratorMustBeFinal(): ArchRule - { - return BaseRule::allClasses() - ->that(new ResideInOneOfTheseNamespaces('App\\Decorator')) - ->should(new IsFinal()) - ->because('Decorators are not meant to be extended but composed'); - } - - public static function uniformNamingForDoctrineExtension(): ArchRule - { - return self::uniformNaming('DoctrineExtension'); - } - - public static function doctrineExtensionShouldImplementQueryCollectionExtensionInterface(): ArchRule - { - return BaseRule::allClasses() - ->that(new ResideInOneOfTheseNamespaces('App\\DoctrineExtension')) - ->should(new Implement('ApiPlatform\\Doctrine\\Orm\\Extension\\QueryCollectionExtensionInterface',)) - ->because('DoctrineExtension should implement ApiPlatform Doctrine QueryCollectionExtensionInterface'); - } - - public static function doctrineExtensionShouldImplementQueryItemExtensionInterface(): ArchRule - { - return BaseRule::allClasses() - ->that(new ResideInOneOfTheseNamespaces('App\\DoctrineExtension')) - ->should(new Implement('ApiPlatform\\Doctrine\\Orm\\Extension\\QueryItemExtensionInterface',)) - ->because('DoctrineExtension should implement ApiPlatform Doctrine QueryItemExtensionInterface'); - } - - public static function doctrineExtensionMustBeFinal(): ArchRule - { - return BaseRule::allClasses() - ->that(new ResideInOneOfTheseNamespaces('App\\DoctrineExtension')) - ->should(new IsFinal()) - ->because('DoctrineExtension are not meant to be extended'); - } - - public static function uniformNamingForEngine(): ArchRule - { - return self::uniformNaming('Engine'); - } - - public static function engineMustNotDependOnTime(): ArchRule - { - return BaseRule::allClasses() - ->that(new ResideInOneOfTheseNamespaces('App\\Engine')) - ->should(new NotDependsOnTheseNamespaces( - 'Psr\\Clock\\ClockInterface', - 'Symfony\\Component\\Clock\\ClockInterface', - )) - ->because('time should be injected in Engine instead of using ClockInterface or Psr ClockInterface directly'); - } - - public static function uniformNamingForEngineRule(): ArchRule - { - return self::uniformNaming('EngineRule'); - } - - public static function engineRuleMustNotDependOnTime(): ArchRule - { - return BaseRule::allClasses() - ->that(new ResideInOneOfTheseNamespaces('App\\EngineRule')) - ->should(new NotDependsOnTheseNamespaces( - 'Psr\\Clock\\ClockInterface', - 'Symfony\\Component\\Clock\\ClockInterface', - )) - ->because('time should be injected in EngineRule instead of using ClockInterface or Psr ClockInterface directly'); - } - - public static function uniformNamingForEvent(): ArchRule - { - return self::uniformNaming('Event'); - } - - public static function eventDependencies(): ArchRule - { - return BaseRule::allClasses() - ->that(new ResideInOneOfTheseNamespaces('App\\Event')) - ->should(new DependsOnTheseNamespaces( - 'App\\Entity', - 'Symfony\\Contracts\\EventDispatcher\\Event', - )) - ->because('Event should have dependency on Event from Symfony EventDispatcher and Entity'); - } - - public static function uniformNamingForFormType(): ArchRule - { - return self::uniformNaming('FormType'); - } - - public static function formMustExtendSymfonyAbstractType(): ArchRule - { - return BaseRule::allClasses() - ->that(new ResideInOneOfTheseNamespaces('App\\Form')) - ->should(new Extend('Symfony\\Component\\Form\\AbstractType')) - ->because('Form should extend Symfony Form AbstractType'); - } - - public static function formTypeMustBeFinal(): ArchRule - { - return BaseRule::allClasses() - ->that(new ResideInOneOfTheseNamespaces('App\\Form')) - ->should(new IsFinal()) - ->because('Form are not meant to be extended, except using Symfony best practices'); - } - - public static function uniformNamingForGenerator(): ArchRule - { - return self::uniformNaming('Generator'); - } - - public static function uniformNamingForHttp(): ArchRule - { - return self::uniformNaming('Http'); - } - - public static function httpMustBeLoggable(): ArchRule - { - return BaseRule::allClasses() - ->that(new ResideInOneOfTheseNamespaces('App\\Http')) - ->should(new DependsOnTheseNamespaces('Psr\\Log\\LoggerInterface')) - ->because('Http should have dependency on LoggerInterface'); - } - - public static function httpMustBeFinal(): ArchRule - { - return BaseRule::allClasses() - ->that(new ResideInOneOfTheseNamespaces('App\\Http')) - ->should(new IsFinal()) - ->because('Extending a Http is not a good practice and should be decorated using Symfony Decorator Pattern'); - } - - public static function uniformNamingForListener(): ArchRule - { - return self::uniformNaming('Listener'); - } - - public static function listenerDependencies(): ArchRule - { - return BaseRule::allClasses() - ->that(new ResideInOneOfTheseNamespaces('App\\Listener')) - ->should(new DependsOnTheseNamespaces( - 'Doctrine\\Bundle\\DoctrineBundle\\Attribute\\AsEntityListener', - 'App\\Entity', - 'App\\Service', - 'Symfony\Component\Clock\ClockInterface', - )) - ->because('Listener should have dependency on AsEntityListener attribute from Doctrine'); - } - - public static function listenerShouldNotThrowDoctrineException(): ArchRule - { - return BaseRule::allClasses() - ->that(new ResideInOneOfTheseNamespaces('App\\Listener')) - ->should(new NotDependsOnTheseNamespaces( - 'Doctrine\\ORM\\NonUniqueResultException', - 'Doctrine\\ORM\\NoResultException', - )) - ->because('Doctrine Exception should not be used in Listener, catch them in Service instead, log them and rethrow a specific Exception'); - } - - public static function listenerShouldBeLoggable(): ArchRule - { - return BaseRule::allClasses() - ->that(new ResideInOneOfTheseNamespaces('App\\Listener')) - ->should(new DependsOnTheseNamespaces('Psr\\Log\\LoggerInterface')) - ->because('Listener should have dependency on LoggerInterface'); - } - - public static function uniformNamingForLogger(): ArchRule - { - return self::uniformNaming('Logger'); - } - - public static function loggerShouldDependOnLoggerInterface(): ArchRule - { - return BaseRule::allClasses() - ->that(new ResideInOneOfTheseNamespaces('App\\Logger')) - ->should(new DependsOnTheseNamespaces('Psr\\Log\\LoggerInterface')) - ->because('Logger should have dependency on LoggerInterface'); - } - - public static function uniformNamingForMigration(): ArchRule - { - return self::uniformNaming('Migration'); - } - - public static function uniformNamingForNormalizer(): ArchRule - { - return self::uniformNaming('Normalizer'); - } - - public static function normalizerMustBeLoggable(): ArchRule - { - return BaseRule::allClasses() - ->that(new ResideInOneOfTheseNamespaces('App\\Normalizer')) - ->should(new DependsOnTheseNamespaces('Psr\\Log\\LoggerInterface')) - ->because('Normalizer should have dependency on LoggerInterface'); - } - - public static function uniformNamingForProfiler(): ArchRule - { - return self::uniformNaming('Profiler'); - } - - public static function uniformNamingForProfilerDataCollector(): ArchRule - { - return self::uniformNaming('ProfilerDataCollector'); - } - - public static function uniformNamingForRepository(): ArchRule - { - return self::uniformNaming('Repository'); - } - - public static function repositoryMustHaveAtournayreDoctrineTraits(): ArchRule - { - return BaseRule::allClasses() - ->that(new ResideInOneOfTheseNamespaces('App\\Repository')) - ->should(new DependsOnTheseNamespaces( - 'Atournayre\\Component\\Doctrine\\Traits\\SaveTrait', - 'Atournayre\\Component\\Doctrine\\Traits\\RemoveTrait', - 'Atournayre\\Component\\Doctrine\\Traits\\SaveAndRemoveTrait', - )) - ->because('Repository should have dependency on SaveTrait, RemoveTrait or SaveAndRemoveTrait, to avoid code duplication and provide useful methods. If not installed, use composer require atournayre/doctrine-component.'); - } - - public static function uniformNamingForSearch(): ArchRule - { - return self::uniformNaming('Search'); - } - - public static function uniformNamingForSecurity(): ArchRule - { - return self::uniformNaming('Security'); - } - - public static function securityDependencies(): ArchRule - { - return BaseRule::allClasses() - ->that(new ResideInOneOfTheseNamespaces('App\\Security')) - ->should(new DependsOnlyOnTheseNamespaces( - 'App\\Entity', - 'App\\Service', - 'App\\Repository', - 'Symfony\\Component\\HttpFoundation', - 'Symfony\\Component\\Routing', - 'Symfony\\Component\\Security', - )) - ->because('Security should have dependency on Entity, Service, Repository and Symfony Security'); - } - - public static function uniformNamingForService(): ArchRule - { - return self::uniformNaming('Service'); - } - - public static function serviceMustBeFinal(): ArchRule - { - return BaseRule::allClasses() - ->that(new ResideInOneOfTheseNamespaces('App\\Service')) - ->should(new IsFinal()) - ->because('Service are not meant to be extended but decorated using Symfony Decorator Pattern'); - } - - public static function serviceMustBeLoggable(): ArchRule - { - return BaseRule::allClasses() - ->that(new ResideInOneOfTheseNamespaces('App\\Service')) - ->should(new DependsOnTheseNamespaces('Psr\\Log\\LoggerInterface')) - ->because('Service should have dependency on LoggerInterface'); - } - - public static function uniformNamingForServiceListener(): ArchRule - { - return self::uniformNaming('ServiceListener'); - } - - public static function uniformNamingForServiceSubscriber(): ArchRule - { - return self::uniformNaming('ServiceSubscriber'); - } - - public static function uniformNamingForServiceState(): ArchRule - { - return self::uniformNaming('ServiceState'); - } - - public static function uniformNamingForServiceStateProcessor(): ArchRule - { - return self::uniformNaming('ServiceStateProcessor'); - } - - public static function uniformNamingForServiceStateProvider(): ArchRule - { - return self::uniformNaming('ServiceStateProvider'); - } - - public static function uniformNamingForState(): ArchRule - { - return self::uniformNaming('State'); - } - - public static function uniformNamingForStateProvider(): ArchRule - { - return self::uniformNaming('StateProvider'); - } - - public static function stateProviderMustBeFinal(): ArchRule - { - return BaseRule::allClasses() - ->that(new ResideInOneOfTheseNamespaces('App\\State\\Provider')) - ->should(new IsFinal()) - ->because('StateProvider are not meant to be extended but decorated using Symfony Decorator Pattern'); - } - - public static function uniformNamingForStateProcessor(): ArchRule - { - return self::uniformNaming('StateProcessor'); - } - - public static function stateProcessorMustBeFinal(): ArchRule - { - return BaseRule::allClasses() - ->that(new ResideInOneOfTheseNamespaces('App\\State\\Processor')) - ->should(new IsFinal()) - ->because('StateProcessor are not meant to be extended but decorated using Symfony Decorator Pattern'); - } - - public static function uniformNamingForSubscriber(): ArchRule - { - return self::uniformNaming('Subscriber'); - } - - public static function uniformNamingForTrait(): ArchRule - { - return self::uniformNaming('Trait'); - } - - public static function entityDependencies(): ArchRule - { - return BaseRule::allClasses() - ->that(new ResideInOneOfTheseNamespaces('App\\Entity')) - ->should(new DependsOnlyOnTheseNamespaces( - 'ApiPlatform\\Metadata\\ApiResource', - 'ApiPlatform\\Metadata\\Delete', - 'ApiPlatform\\Metadata\\Get', - 'ApiPlatform\\Metadata\\GetCollection', - 'ApiPlatform\\Metadata\\Patch', - 'ApiPlatform\\Metadata\\Post', - 'ApiPlatform\\OpenApi\\Model', - 'App\\ApiPlatform\\Endpoint', - 'App\\ApiResource\\Groupes', - 'App\\Enum', - 'ArrayObject', - 'Doctrine\\ORM\\Mapping', - 'Symfony\\Component\\HttpFoundation\\File\\File', - 'Symfony\\Component\\Validator\\Constraints', - 'Vich\\UploaderBundle\\Mapping\\Annotation', - )) - ->because('Entity should have dependency on ApiPlatform, ArrayObject, Doctrine Mapping, Symfony File, Symfony Validator and Vich UploaderBundle'); - } - - public static function entitiesShouldImplementIsEntityInterface(): ArchRule - { - return BaseRule::allClasses() - ->that(new ResideInOneOfTheseNamespaces('App\\Entity')) - ->should(new Implement('Atournayre\\Component\\Doctrine\\Contracts\\IsEntityInterface')) - ->because('it simplifies the check of the type of an object, and will be useful in repositories. If not installed, use composer require atournayre/doctrine-component.'); - } - - public static function entitiesShouldNotThrowDoctrineException(): ArchRule - { - return BaseRule::allClasses() - ->that(new ResideInOneOfTheseNamespaces('App\\Entity')) - ->should(new NotDependsOnTheseNamespaces( - 'Doctrine\\ORM\\NonUniqueResultException', - 'Doctrine\\ORM\\NoResultException', - )) - ->because('Doctrine Exception should not be used in Entity, catch them in Repository instead, log them and rethrow a specific Exception'); - } - - public static function entityExceptionNameMustNotEndWithException(): ArchRule - { - return BaseRule::allClasses() - ->that(new ResideInOneOfTheseNamespaces('App\\Entity\\Exception')) - ->should(new NotHaveNameMatching('*Exception')) - ->because('Exception should not be suffixed by Exception in order to be meaningful when suffixed by named constructor'); - } - - public static function entityExceptionDependencies(): ArchRule - { - return BaseRule::allClasses() - ->that(new ResideInOneOfTheseNamespaces('App\\Entity\\Exception')) - ->should(new DependsOnlyOnTheseNamespaces( - 'App\\Entity', - 'App\\Enum', - 'App\\VO', - 'DateTimeInterface', - 'RuntimeException', - )) - ->because('Exception should only depend on Entity, Enum, VO, DateTimeInterface and RuntimeException'); - } - - public static function enumShouldImplementEnumInterface(): ArchRule - { - return BaseRule::allClasses() - ->that(new ResideInOneOfTheseNamespaces('App\\Enum')) - ->should(new Implement('App\\Contracts\\EnumInterface',)) - ->because('Enum should implement EnumInterface'); - } - - public static function voMustNotEndWithVo(): ArchRule - { - return BaseRule::allClasses() - ->that(new ResideInOneOfTheseNamespaces('App\\VO')) - ->should(new NotHaveNameMatching('*VO')) - ->because('VO should not be suffixed by VO'); - } - - public static function voMustBeFinal(): ArchRule - { - return BaseRule::allClasses() - ->that(new ResideInOneOfTheseNamespaces('App\\VO')) - ->should(new IsFinal()) - ->because('Extending a VO is not a good practice'); - } - - public static function exceptionMustNotEndWithException(): ArchRule - { - return BaseRule::allClasses() - ->that(new ResideInOneOfTheseNamespaces('App\\Exception')) - ->should(new NotHaveNameMatching('*Exception')) - ->because('Exception should not be suffixed by Exception in order to be meaningful when suffixed by named constructor'); - } -} diff --git a/src/Rule/ApiPlaformRule.php b/src/Rule/ApiPlaformRule.php new file mode 100644 index 0000000..8c97544 --- /dev/null +++ b/src/Rule/ApiPlaformRule.php @@ -0,0 +1,84 @@ +that(new ResideInOneOfTheseNamespaces('App\DoctrineExtension')) + ->should(new Implement('ApiPlatform\Doctrine\Orm\Extension\QueryItemExtensionInterface',)) + ->because('DoctrineExtension should implement ApiPlatform Doctrine QueryItemExtensionInterface'); + } + + public static function doctrineExtensionShouldImplementQueryCollectionExtensionInterface(): ArchRule + { + return self::allClasses() + ->that(new ResideInOneOfTheseNamespaces('App\DoctrineExtension')) + ->should(new Implement('ApiPlatform\Doctrine\Orm\Extension\QueryCollectionExtensionInterface',)) + ->because('DoctrineExtension should implement ApiPlatform Doctrine QueryCollectionExtensionInterface'); + } + + public static function doctrineExtensionMustBeFinal(): ArchRule + { + return self::allClasses() + ->that(new ResideInOneOfTheseNamespaces('App\DoctrineExtension')) + ->should(new IsFinal()) + ->because('DoctrineExtension are not meant to be extended'); + } + + public static function classImplementingProviderInterfaceShouldResideInAppState(): ArchRule + { + return self::allClasses() + ->that(new Implement('ApiPlatform\State\ProviderInterface')) + ->should(new ResideInOneOfTheseNamespaces( + 'App\State', + 'App\State\Provider', + )) + ->because('it\'s an ApiPlatform convention'); + } + + public static function classImplementingProcessorInterfaceShouldResideInAppState(): ArchRule + { + return self::allClasses() + ->that(new Implement('ApiPlatform\State\ProcessorInterface')) + ->should(new ResideInOneOfTheseNamespaces( + 'App\State', + 'App\State\Processor', + )) + ->because('it\'s an ApiPlatform convention'); + } + + public static function providersAndProcessorsShouldBeFinal(): ArchRule + { + return self::allClasses() + ->that(new ResideInOneOfTheseNamespaces( + 'App\State', + 'App\State\Provider', + 'App\State\Processor', + )) + ->should(new IsFinal()) + ->because('extending a provider should be done using the decorator pattern'); + } + + public static function uniformNamingForStateProcessor(): ArchRule + { + return self::uniformNaming('Processor'); + } + + public static function uniformNamingForStateProvider(): ArchRule + { + return self::uniformNaming('Provider'); + } +} diff --git a/src/Rule/ApiRule.php b/src/Rule/ApiRule.php new file mode 100644 index 0000000..f3a8291 --- /dev/null +++ b/src/Rule/ApiRule.php @@ -0,0 +1,23 @@ +that(new ResideInOneOfTheseNamespaces('App\ApiResource')) + ->should(new IsFinal()) + ->because('ApiResource are not meant to be extended'); + } +} diff --git a/src/Rule/DoctrineRule.php b/src/Rule/DoctrineRule.php new file mode 100644 index 0000000..eb7d2b7 --- /dev/null +++ b/src/Rule/DoctrineRule.php @@ -0,0 +1,40 @@ +that(new ResideInOneOfTheseNamespaces('App\Listener')) + ->should(new DependsOnlyOnTheseNamespaces( + 'Doctrine\Bundle\DoctrineBundle\Attribute\AsEntityListener', + 'App\Entity', + 'App\Service', + 'Symfony\Component\Clock\ClockInterface', + )) + ->because('Listener should have dependency on AsEntityListener attribute from Doctrine'); + } + + public static function listenerShouldNotThrowDoctrineException(): ArchRule + { + return self::allClasses() + ->that(new ResideInOneOfTheseNamespaces('App\Listener')) + ->should(new NotDependsOnTheseNamespaces( + 'Doctrine\ORM\NonUniqueResultException', + 'Doctrine\ORM\NoResultException', + )) + ->because('Doctrine Exception should not be used in Listener, catch them in Service instead, log them and rethrow a specific Exception'); + } +} diff --git a/src/Rule/DomainRule.php b/src/Rule/DomainRule.php new file mode 100644 index 0000000..842e831 --- /dev/null +++ b/src/Rule/DomainRule.php @@ -0,0 +1,55 @@ +that(new ResideInOneOfTheseNamespaces('App\Entity')) + ->should(new NotHaveDependencyOutsideNamespace('App\Entity', [ + 'ApiPlatform\Metadata\ApiResource', + 'ApiPlatform\Metadata\Delete', + 'ApiPlatform\Metadata\Get', + 'ApiPlatform\Metadata\GetCollection', + 'ApiPlatform\Metadata\Patch', + 'ApiPlatform\Metadata\Post', + 'ApiPlatform\OpenApi\Model', + 'App\ApiPlatform\Endpoint', + 'App\ApiResource\Groupes', + 'App\Enum', + 'ArrayObject', + 'Doctrine\ORM\Mapping', + 'Symfony\Component\HttpFoundation\File\File', + 'Symfony\Component\Validator\Constraints', + 'Vich\UploaderBundle\Mapping\Annotation', + ])) + ->because('Entities should not have dependency outside App\Entity namespace, except for ApiPlatform, ArrayObject, Doctrine Mapping, Symfony File, Symfony Validator and Vich UploaderBundle'); + } + + public static function entityExceptionNameMustNotEndWithException(): ArchRule + { + return self::allClasses() + ->that(new ResideInOneOfTheseNamespaces('App\Entity\Exception')) + ->should(new NotHaveNameMatching('*Exception')) + ->because('Exception should not be suffixed by Exception in order to be meaningful when suffixed by named constructor'); + } + + public static function entitiesShouldNotThrowDoctrineException(): ArchRule + { + return self::allClasses() + ->that(new ResideInOneOfTheseNamespaces('App\Entity')) + ->should(new NotDependsOnTheseNamespaces( + 'Doctrine\ORM\NonUniqueResultException', + 'Doctrine\ORM\NoResultException', + )) + ->because('Doctrine Exception should not be used in Entity, catch them in Repository instead, log them and rethrow a specific Exception'); + } +} diff --git a/src/Rule/LogRule.php b/src/Rule/LogRule.php new file mode 100644 index 0000000..c36983f --- /dev/null +++ b/src/Rule/LogRule.php @@ -0,0 +1,55 @@ +that(new ResideInOneOfTheseNamespaces('App\Normalizer')) + ->should(new DependsOnTheseNamespaces('Psr\Log\LoggerInterface')) + ->because('Normalizer must have dependency on LoggerInterface'); + } + + public static function listenerMustBeLoggable(): ArchRule + { + return self::allClasses() + ->that(new ResideInOneOfTheseNamespaces('App\Listener')) + ->should(new DependsOnTheseNamespaces('Psr\Log\LoggerInterface')) + ->because('Listener must have dependency on LoggerInterface'); + } + + public static function loggerMustDependOnLoggerInterface(): ArchRule + { + return self::allClasses() + ->that(new ResideInOneOfTheseNamespaces('App\Logger')) + ->should(new DependsOnTheseNamespaces('Psr\Log\LoggerInterface')) + ->because('Logger must have dependency on LoggerInterface'); + } + + public static function serviceMustBeLoggable(): ArchRule + { + return self::allClasses() + ->that(new ResideInOneOfTheseNamespaces('App\Service')) + ->should(new DependsOnTheseNamespaces('Psr\Log\LoggerInterface')) + ->because('Service must have dependency on LoggerInterface'); + } + + public static function httpMustBeLoggable(): ArchRule + { + return self::allClasses() + ->that(new ResideInOneOfTheseNamespaces('App\Http')) + ->should(new DependsOnTheseNamespaces('Psr\Log\LoggerInterface')) + ->because('Http must have dependency on LoggerInterface'); + } +} diff --git a/src/Rule/MiscRule.php b/src/Rule/MiscRule.php new file mode 100644 index 0000000..47bcc26 --- /dev/null +++ b/src/Rule/MiscRule.php @@ -0,0 +1,269 @@ +that(new ResideInOneOfTheseNamespaces('App\Collection')) + ->should(new DependsOnlyOnTheseNamespaces( + 'App\Entity', + 'Carbon\Carbon', + 'DateTimeInterface', + 'Doctrine\Common\Collections\Collection', + 'Doctrine\Common\Collections\Criteria', + 'Doctrine\Common\Collections\ArrayCollection', + 'Doctrine\Common\Collections\ReadableCollection', + 'Doctrine\ORM\PersistentCollection', + Assert::class, + )) + ->because('Collection should only depend on Entity, Carbon, DateTimeInterface, Doctrine Collection, Doctrine Criteria, Doctrine ReadableCollection, Doctrine PersistentCollection and Webmozart Assert'); + } + + public static function eventCollectionDependencies(): ArchRule + { + return self::allClasses() + ->that(new ResideInOneOfTheseNamespaces('App\Event\Collection')) + ->should(new DependsOnlyOnTheseNamespaces( + 'App\Entity', + 'Carbon\Carbon', + 'DateTimeInterface', + 'Doctrine\Common\Collections\Collection', + 'Doctrine\Common\Collections\Criteria', + 'Doctrine\Common\Collections\ArrayCollection', + 'Doctrine\Common\Collections\ReadableCollection', + 'Doctrine\ORM\PersistentCollection', + Assert::class, + 'Symfony\Contracts\EventDispatcher\Event' + )) + ->because('Collection should only depend on Entity, Carbon, DateTimeInterface, Doctrine Collection, Doctrine Criteria, Doctrine ReadableCollection, Doctrine PersistentCollection, Webmozart Assert and Symfony EventDispatcher Event'); + } + + public static function uniformNamingForCollection(): ArchRule + { + return self::uniformNaming('Collection'); + } + + public static function dtoMustNotEndWithDto(): ArchRule + { + return self::allClasses() + ->that(new ResideInOneOfTheseNamespaces('App\DTO')) + ->should(new NotHaveNameMatching('*DTO')) + ->because('DTO should not be suffixed by DTO'); + } + + public static function dtoMustBeFinal(): ArchRule + { + return self::allClasses() + ->that(new ResideInOneOfTheseNamespaces('App\DTO')) + ->should(new IsFinal()) + ->because('Extending a DTO is not a good practice'); + } + + public static function voMustNotEndWithVo(): ArchRule + { + return self::allClasses() + ->that(new ResideInOneOfTheseNamespaces('App\VO')) + ->should(new NotHaveNameMatching('*VO')) + ->because('VO should not be suffixed by VO'); + } + + public static function voMustBeFinal(): ArchRule + { + return self::allClasses() + ->that(new ResideInOneOfTheseNamespaces('App\VO')) + ->should(new IsFinal()) + ->because('Extending a VO is not a good practice'); + } + + public static function uniformNamingForDecorator(): ArchRule + { + return self::uniformNaming('Decorator'); + } + + public static function decoratorMustBeFinal(): ArchRule + { + return self::allClasses() + ->that(new ResideInOneOfTheseNamespaces('App\Decorator')) + ->should(new IsFinal()) + ->because('Decorators are not meant to be extended but composed'); + } + + public static function uniformNamingForApiDocumentation(): ArchRule + { + return self::uniformNaming('ApiDocumentation'); + } + + public static function apiDocumentationOpenApi(): ArchRule + { + return self::allClasses() + ->that(new ResideInOneOfTheseNamespaces('App\Api\Documentation')) + ->should(new HaveNameMatching('/OpenApi$/')) + ->because('API Documentation should have a name with "OpenApi" inside'); + } + + public static function commandMustUseSymfonyStopwatch(): ArchRule + { + return self::allClasses() + ->that(new ResideInOneOfTheseNamespaces('App\Command')) + ->should(new DependsOnTheseNamespaces('Symfony\Component\Stopwatch\Stopwatch')) + ->because('We use Stopwatch to measure time of command execution'); + } + + public static function uniformNamingForContracts(): ArchRule + { + return self::uniformNaming('Interface'); + } + + public static function contractsDependencies(): ArchRule + { + return self::allClasses() + ->that(new ResideInOneOfTheseNamespaces('App\Contracts')) + ->should(new DependsOnlyOnTheseNamespaces( + 'App\Entity', + 'App\DTO', + 'App\VO', + 'DateTimeInterface', + )) + ->because('Interface should only depend on Entity, DTO, VO and DateTimeInterface'); + } + + public static function engineMustNotDependOnTime(): ArchRule + { + return self::allClasses() + ->that(new ResideInOneOfTheseNamespaces('App\Engine')) + ->should(new NotDependsOnTheseNamespaces( + 'Psr\Clock\ClockInterface', + 'Symfony\Component\Clock\ClockInterface', + )) + ->because('time should be injected in Engine instead of using ClockInterface or Psr ClockInterface directly'); + } + + public static function engineRuleMustNotDependOnTime(): ArchRule + { + return self::allClasses() + ->that(new ResideInOneOfTheseNamespaces('App\EngineRule')) + ->should(new NotDependsOnTheseNamespaces( + 'Psr\Clock\ClockInterface', + 'Symfony\Component\Clock\ClockInterface', + )) + ->because('time should be injected in EngineRule instead of using ClockInterface or Psr ClockInterface directly'); + } + + public static function uniformNamingForEngine(): ArchRule + { + return self::uniformNaming('Engine'); + } + + public static function uniformNamingForTrait(): ArchRule + { + return self::uniformNaming('Trait'); + } + + public static function uniformNamingForGenerator(): ArchRule + { + return self::uniformNaming('Generator'); + } + + public static function uniformNamingForHttp(): ArchRule + { + return self::uniformNaming('Http'); + } + + public static function httpMustBeFinal(): ArchRule + { + return self::allClasses() + ->that(new ResideInOneOfTheseNamespaces('App\Http')) + ->should(new IsFinal()) + ->because('Extending a Http is not a good practice and should be decorated using Symfony Decorator Pattern'); + } + + public static function uniformNamingForSearch(): ArchRule + { + return self::uniformNaming('Search'); + } + + public static function uniformNamingForEngineRule(): ArchRule + { + return self::uniformNaming('EngineRule'); + } + + public static function enum(): ArchRule + { + return self::allClasses() + ->that(new ResideInOneOfTheseNamespaces('App\Enum')) + ->should(new IsEnum()) + ->because('we want to be sure that all classes are enum'); + } + + public static function enumShouldImplementEnumInterface(): ArchRule + { + return self::allClasses() + ->that(new ResideInOneOfTheseNamespaces('App\Enum')) + ->should(new Implement('App\Contracts\EnumInterface',)) + ->because('Enum should implement EnumInterface'); + } + + public static function entityExceptionDependencies(): ArchRule + { + return self::allClasses() + ->that(new ResideInOneOfTheseNamespaces('App\Entity\Exception')) + ->should(new DependsOnlyOnTheseNamespaces( + 'App\Entity', + 'App\Enum', + 'App\VO', + 'DateTimeInterface', + 'RuntimeException', + )) + ->because('Exception should only depend on Entity, Enum, VO, DateTimeInterface and RuntimeException'); + } + + public static function exceptionMustNotEndWithException(): ArchRule + { + return self::allClasses() + ->that(new ResideInOneOfTheseNamespaces('App\Exception')) + ->should(new NotHaveNameMatching('*Exception')) + ->because('Exception should not be suffixed by Exception in order to be meaningful when suffixed by named constructor'); + } + + public static function entitiesShouldImplementIsEntityInterface(): ArchRule + { + return self::allClasses() + ->that(new ResideInOneOfTheseNamespaces('App\Entity')) + ->should(new Implement('Atournayre\Component\Doctrine\Contracts\IsEntityInterface')) + ->because('it simplifies the check of the type of an object, and will be useful in repositories. If not installed, use composer require atournayre/doctrine-component.'); + } + + public static function repositoryMustHaveAtournayreDoctrineTraits(): ArchRule + { + return self::allClasses() + ->that(new ResideInOneOfTheseNamespaces('App\Repository')) + ->should(new DependsOnTheseNamespaces( + 'Atournayre\Component\Doctrine\Traits', + )) + ->because('Repository should have dependency on Atournayre\Component\Doctrine\Traits, to avoid code duplication and provide useful methods. If not installed, use composer require atournayre/doctrine-component.'); + } + + public static function traits(): ArchRule + { + return self::allClasses() + ->that(new ResideInOneOfTheseNamespaces('App\Traits')) + ->should(new IsTrait()) + ->because('we want to be sure that there are only traits in a specific namespace'); + } +} diff --git a/src/Rule/Rule.php b/src/Rule/Rule.php new file mode 100644 index 0000000..9fae3e4 --- /dev/null +++ b/src/Rule/Rule.php @@ -0,0 +1,38 @@ +that(new ResideInOneOfTheseNamespaces('App\\' . $nameMatching)) + ->should(new HaveNameMatching('*' . $nameMatching)) + ->because('Classes should have a name matching ' . $nameMatching); + } + + public static function all(): array + { + $reflection = new \ReflectionClass(self::class); + $methods = $reflection->getMethods(\ReflectionMethod::IS_STATIC | \ReflectionMethod::IS_PUBLIC); + $rules = []; + foreach ($methods as $method) { + $methodName = $method->getName(); + if ($methodName !== 'all') { + $rules[] = self::$methodName(); + } + } + return $rules; + } +} diff --git a/src/Rule/SymfonyRule.php b/src/Rule/SymfonyRule.php new file mode 100644 index 0000000..0ca4511 --- /dev/null +++ b/src/Rule/SymfonyRule.php @@ -0,0 +1,174 @@ +that(new ResideInOneOfTheseNamespaces('App\Command')) + ->should(new Extend('Symfony\Component\Console\Command\Command')) + ->because('Command should have dependency on Symfony Console Command'); + } + + public static function commandMustUseSymfonyAsCommandAttribute(): ArchRule + { + return self::allClasses() + ->that(new ResideInOneOfTheseNamespaces('App\Command')) + ->should(new HaveAttribute('AsCommand')) + ->because('Command should have dependency on AsCommand attribute from Symfony Console'); + } + + public static function uniformNamingForController(): ArchRule + { + return self::uniformNaming('Controller'); + } + + public static function controllerMustUseSymfonRouteAttribute(): ArchRule + { + return self::allClasses() + ->that(new ResideInOneOfTheseNamespaces('App\Controller')) + ->should(new HaveAttribute('Route')) + ->because('Controller should have dependency on Route attribute from Symfony Routing'); + } + + public static function controllerDependencies(): ArchRule + { + return self::allClasses() + ->that(new ResideInOneOfTheseNamespaces('App\Controller')) + ->should(new DependsOnlyOnTheseNamespaces( + 'App\Entity', + 'App\Form', + 'App\Service', + 'Symfony\Bundle\FrameworkBundle\Controller\AbstractController', + 'Symfony\Component\HttpFoundation\JsonResponse', + 'Symfony\Component\HttpFoundation\RedirectResponse', + 'Symfony\Component\HttpFoundation\Request', + 'Symfony\Component\HttpFoundation\Response', + 'Symfony\Component\Security\Http\Attribute\IsGranted', + )) + ->because('Controller should have dependency on AbstractController, Response, Request, RedirectResponse, JsonResponse, IsGranted attribute from Symfony Security, Entity, Form and Service'); + } + + public static function eventDependencies(): ArchRule + { + return self::allClasses() + ->that(new ResideInOneOfTheseNamespaces('App\Event')) + ->should(new DependsOnlyOnTheseNamespaces( + 'App\Entity', + 'Symfony\Contracts\EventDispatcher\Event', + )) + ->because('Event should depends only on Entity and Symfony Event'); + } + + public static function formMustExtendSymfonyAbstractType(): ArchRule + { + return self::allClasses() + ->that(new ResideInOneOfTheseNamespaces('App\Form\Type')) + ->should(new Extend('Symfony\Component\Form\AbstractType')) + ->because('Form should extend Symfony Form AbstractType'); + } + + public static function formTypesShoudResideInAppFormType(): ArchRule + { + return self::allClasses() + ->that(new Extend('Symfony\Component\Form\AbstractType')) + ->should(new ResideInOneOfTheseNamespaces('App\Form\Type')) + ->because('Form that extend Symfony Form AbstractType should reside in App\Form\Type'); + } + + public static function formTypeMustBeFinal(): ArchRule + { + return self::allClasses() + ->that(new ResideInOneOfTheseNamespaces('App\Form\Type')) + ->should(new IsFinal()) + ->because('Form are not meant to be extended, except using Symfony best practices'); + } + + public static function uniformNamingForProfilerDataCollector(): ArchRule + { + return self::uniformNaming('ProfilerDataCollector'); + } + + public static function uniformNamingForProfiler(): ArchRule + { + return self::uniformNaming('Profiler'); + } + + public static function uniformNamingForFormType(): ArchRule + { + return self::uniformNaming('Type'); + } + + public static function uniformNamingForRepository(): ArchRule + { + return self::uniformNaming('Repository'); + } + + public static function uniformNamingForNormalizer(): ArchRule + { + return self::uniformNaming('Normalizer'); + } + + public static function securityDependencies(): ArchRule + { + return self::allClasses() + ->that(new ResideInOneOfTheseNamespaces('App\Security')) + ->should(new DependsOnlyOnTheseNamespaces( + 'App\Entity', + 'App\Service', + 'App\Repository', + 'Symfony\Component\HttpFoundation', + 'Symfony\Component\Routing', + 'Symfony\Component\Security', + )) + ->because('Security should have dependency on Entity, Service, Repository and Symfony Security'); + } + + public static function uniformNamingForService(): ArchRule + { + return self::uniformNaming('Service'); + } + + public static function uniformNamingForListener(): ArchRule + { + return self::uniformNaming('Listener'); + } + + public static function uniformNamingForEvent(): ArchRule + { + return self::uniformNaming('Event'); + } + + public static function uniformNamingForSecurity(): ArchRule + { + return self::uniformNaming('Security'); + } + + public static function serviceMustBeFinal(): ArchRule + { + return self::allClasses() + ->that(new ResideInOneOfTheseNamespaces('App\Service')) + ->should(new IsFinal()) + ->because('Service are not meant to be extended but decorated using Symfony Decorator Pattern'); + } + + public static function uniformNamingForSubscriber(): ArchRule + { + return self::uniformNaming('Subscriber'); + } +} diff --git a/src/Set/Sets.php b/src/Set/Sets.php new file mode 100644 index 0000000..f68cb61 --- /dev/null +++ b/src/Set/Sets.php @@ -0,0 +1,294 @@ + Date: Sun, 3 Dec 2023 18:45:29 +0100 Subject: [PATCH 02/10] Refactor Rule.php to exclude specific methods Included a filter in Rule.php to exclude all and uniformNaming static, public methods from the rules. This change removes the 'all' condition further down in the code, streamlining and simplifying the process generation of rules. The refactoring enhances code readability and efficiency. --- src/Rule/Rule.php | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/Rule/Rule.php b/src/Rule/Rule.php index 9fae3e4..a36957c 100644 --- a/src/Rule/Rule.php +++ b/src/Rule/Rule.php @@ -27,11 +27,14 @@ public static function all(): array $reflection = new \ReflectionClass(self::class); $methods = $reflection->getMethods(\ReflectionMethod::IS_STATIC | \ReflectionMethod::IS_PUBLIC); $rules = []; + $methods = array_filter($methods, function ($method) { + $methodName = $method->getName(); + return !in_array($methodName, ['all', 'uniformNaming']); + }); + foreach ($methods as $method) { $methodName = $method->getName(); - if ($methodName !== 'all') { - $rules[] = self::$methodName(); - } + $rules[] = self::$methodName(); } return $rules; } From 7519e7843bf560789a872f9559a8dc2c7e3c2d53 Mon Sep 17 00:00:00 2001 From: atournayre Date: Sun, 3 Dec 2023 20:34:49 +0100 Subject: [PATCH 03/10] Add contracts method in MiscRule.php for namespace check Added a new method named 'contracts' in MiscRule.php file. This method ensures that there are only interfaces in a specific namespace 'App\Contracts'. This change is done to enforce a rule to place interfaces in the designated namespace, enhancing organization and readability of the codebase. --- src/Rule/MiscRule.php | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/Rule/MiscRule.php b/src/Rule/MiscRule.php index 47bcc26..3f8c08f 100644 --- a/src/Rule/MiscRule.php +++ b/src/Rule/MiscRule.php @@ -266,4 +266,12 @@ public static function traits(): ArchRule ->should(new IsTrait()) ->because('we want to be sure that there are only traits in a specific namespace'); } + + public static function contracts(): ArchRule + { + return self::allClasses() + ->that(new HaveNameMatching('*Interface')) + ->should(new ResideInOneOfTheseNamespaces('App\Contracts')) + ->because('we want to be sure that there are only interfaces in a specific namespace'); + } } From 8a90ebf40e6200d250d877875a6ccaea2928ae2e Mon Sep 17 00:00:00 2001 From: atournayre Date: Sun, 3 Dec 2023 20:35:17 +0100 Subject: [PATCH 04/10] Refactor Rule::all to use late static binding Refactored `Rule::all` method to use late static binding and array_map function. Late static binding is used instead of self::class for future-proofing the framework accommodating new static method rules without needing to modify the all() method. Array_map is used to simplify the process of applying each rule, making the code cleaner and more efficient. --- src/Rule/Rule.php | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/src/Rule/Rule.php b/src/Rule/Rule.php index a36957c..687dcea 100644 --- a/src/Rule/Rule.php +++ b/src/Rule/Rule.php @@ -24,18 +24,15 @@ protected static function uniformNaming(string $nameMatching): ArchRule public static function all(): array { - $reflection = new \ReflectionClass(self::class); - $methods = $reflection->getMethods(\ReflectionMethod::IS_STATIC | \ReflectionMethod::IS_PUBLIC); - $rules = []; + $reflection = new \ReflectionClass(static::class); + $methods = $reflection->getMethods(\ReflectionMethod::IS_STATIC); $methods = array_filter($methods, function ($method) { - $methodName = $method->getName(); - return !in_array($methodName, ['all', 'uniformNaming']); + return $method->isPublic() && $method->getName() !== 'all'; }); - foreach ($methods as $method) { - $methodName = $method->getName(); - $rules[] = self::$methodName(); - } - return $rules; + return array_map( + fn ($method) => static::{$method->getName()}(), + $methods + ); } } From caaa89163286aba788b604bf6b8e84a845f5c9d2 Mon Sep 17 00:00:00 2001 From: atournayre Date: Sun, 3 Dec 2023 22:13:03 +0100 Subject: [PATCH 05/10] Enhanced rule validation and error handling logic Reworked rule validation logic to improve code readability and error handling. With this change, rules now use the IsNotInterface and IsNotTrait to ensure that the classes implementing the ProviderInterface and ProcessorInterface are neither interfaces nor traits. This helps enforce our API Platform convention of using concrete classes for providers and processors. Also, the DependsOnTheseNamespaces class was renamed to DependsOnTheseNamespace and refactored to validate dependencies on a single namespace for improved accuracy of dependency checks. Additional namespaces were added to dependencies checks for various classes to enforce structural organization and reduce improper cross dependencies within application. Expanded dependency checks to enforce architectural patterns and organization. Entities are now prohibited from having dependencies outside the App\Entity namespace, exceptions were added for specific classes. Renamed class 'DependsOnTheseNamespaces' to 'DependsOnTheseNamespace' to better match its functionality and updated all its uses. --- .../ForClasses/DependsOnTheseNamespace.php | 54 ++++++++++++++++++ .../ForClasses/DependsOnTheseNamespaces.php | 56 ------------------- src/Rule/ApiPlaformRule.php | 10 +++- src/Rule/DoctrineRule.php | 9 +++ src/Rule/DomainRule.php | 30 ++++++++-- src/Rule/LogRule.php | 12 ++-- src/Rule/MiscRule.php | 6 +- src/Rule/SymfonyRule.php | 1 - .../DependsOnTheseNamespacesTest.php | 23 +++----- 9 files changed, 112 insertions(+), 89 deletions(-) create mode 100644 src/Expression/ForClasses/DependsOnTheseNamespace.php delete mode 100644 src/Expression/ForClasses/DependsOnTheseNamespaces.php diff --git a/src/Expression/ForClasses/DependsOnTheseNamespace.php b/src/Expression/ForClasses/DependsOnTheseNamespace.php new file mode 100644 index 0000000..fe338ec --- /dev/null +++ b/src/Expression/ForClasses/DependsOnTheseNamespace.php @@ -0,0 +1,54 @@ +namespace = $namespace; + } + + public function describe(ClassDescription $theClass, string $because): Description + { + return new Description("should depend on class in these namespace: $this->namespace", $because); + } + + /** + * @throws FailOnFirstViolationException + */ + public function evaluate(ClassDescription $theClass, Violations $violations, string $because): void + { + $dependencies = $theClass->getDependencies(); + + $filteredDependencies = array_filter($dependencies, function ($dependency) { + return $dependency->matches($this->namespace); + }); + + if (count($filteredDependencies) > 0) { + return; + } + + $violation = Violation::create( + $theClass->getFQCN(), + ViolationMessage::withDescription( + $this->describe($theClass, $because), + "depends on {$this->namespace}" + ) + ); + + $violations->add($violation); + } +} diff --git a/src/Expression/ForClasses/DependsOnTheseNamespaces.php b/src/Expression/ForClasses/DependsOnTheseNamespaces.php deleted file mode 100644 index 565992f..0000000 --- a/src/Expression/ForClasses/DependsOnTheseNamespaces.php +++ /dev/null @@ -1,56 +0,0 @@ -namespaces = $namespace; - } - - public function describe(ClassDescription $theClass, string $because): Description - { - $desc = implode(', ', $this->namespaces); - - return new Description("should depend on classes in one of these namespaces: $desc", $because); - } - - public function evaluate(ClassDescription $theClass, Violations $violations, string $because): void - { - $dependencies = $theClass->getDependencies(); - - foreach ($this->namespaces as $namespace) { - $found = false; - foreach ($dependencies as $dependency) { - if ($dependency->getFQCN()->namespace() === $namespace) { - $found = true; - break; - } - } - if (!$found) { - $violation = Violation::create( - $theClass->getFQCN(), - ViolationMessage::withDescription( - $this->describe($theClass, $because), - "depends on {$namespace}" - ) - ); - - $violations->add($violation); - } - } - } -} diff --git a/src/Rule/ApiPlaformRule.php b/src/Rule/ApiPlaformRule.php index 8c97544..49b8f33 100644 --- a/src/Rule/ApiPlaformRule.php +++ b/src/Rule/ApiPlaformRule.php @@ -4,6 +4,8 @@ use Arkitect\Expression\ForClasses\Implement; use Arkitect\Expression\ForClasses\IsFinal; +use Arkitect\Expression\ForClasses\IsNotInterface; +use Arkitect\Expression\ForClasses\IsNotTrait; use Arkitect\Expression\ForClasses\ResideInOneOfTheseNamespaces; use Arkitect\Rules\DSL\ArchRule; @@ -42,22 +44,26 @@ public static function classImplementingProviderInterfaceShouldResideInAppState( { return self::allClasses() ->that(new Implement('ApiPlatform\State\ProviderInterface')) + ->andThat(new IsNotInterface()) + ->andThat(new IsNotTrait()) ->should(new ResideInOneOfTheseNamespaces( 'App\State', 'App\State\Provider', )) - ->because('it\'s an ApiPlatform convention'); + ->because('it\'s an ApiPlatform convention for Providers'); } public static function classImplementingProcessorInterfaceShouldResideInAppState(): ArchRule { return self::allClasses() ->that(new Implement('ApiPlatform\State\ProcessorInterface')) + ->andThat(new IsNotInterface()) + ->andThat(new IsNotTrait()) ->should(new ResideInOneOfTheseNamespaces( 'App\State', 'App\State\Processor', )) - ->because('it\'s an ApiPlatform convention'); + ->because('it\'s an ApiPlatform convention for Processors'); } public static function providersAndProcessorsShouldBeFinal(): ArchRule diff --git a/src/Rule/DoctrineRule.php b/src/Rule/DoctrineRule.php index eb7d2b7..4c3d26f 100644 --- a/src/Rule/DoctrineRule.php +++ b/src/Rule/DoctrineRule.php @@ -19,10 +19,19 @@ public static function listenerDependencies(): ArchRule return self::allClasses() ->that(new ResideInOneOfTheseNamespaces('App\Listener')) ->should(new DependsOnlyOnTheseNamespaces( + 'Doctrine\Bundle\DoctrineBundle\Attribute\AsDoctrineListener', 'Doctrine\Bundle\DoctrineBundle\Attribute\AsEntityListener', + 'Doctrine\ORM\Event', + 'Doctrine\ORM\Events', + 'App\Decorator', + 'App\DTO', 'App\Entity', + 'App\Enum', 'App\Service', + 'App\Repository', + 'App\VO', 'Symfony\Component\Clock\ClockInterface', + 'Webmozart\Assert\Assert', )) ->because('Listener should have dependency on AsEntityListener attribute from Doctrine'); } diff --git a/src/Rule/DomainRule.php b/src/Rule/DomainRule.php index 842e831..8d224e7 100644 --- a/src/Rule/DomainRule.php +++ b/src/Rule/DomainRule.php @@ -15,21 +15,39 @@ public static function entityDependencies(): ArchRule return self::allClasses() ->that(new ResideInOneOfTheseNamespaces('App\Entity')) ->should(new NotHaveDependencyOutsideNamespace('App\Entity', [ - 'ApiPlatform\Metadata\ApiResource', - 'ApiPlatform\Metadata\Delete', - 'ApiPlatform\Metadata\Get', - 'ApiPlatform\Metadata\GetCollection', - 'ApiPlatform\Metadata\Patch', - 'ApiPlatform\Metadata\Post', + 'ApiPlatform\Doctrine', + 'ApiPlatform\Metadata', 'ApiPlatform\OpenApi\Model', 'App\ApiPlatform\Endpoint', 'App\ApiResource\Groupes', + 'App\Collection', + 'App\Contracts', + 'App\DTO', 'App\Enum', + 'App\Repository', + 'App\State', + 'App\State\Provider', + 'App\State\Processor', + 'App\VO', 'ArrayObject', + 'Carbon', + 'DateTimeImmutable', + 'DateTimeInterface', + 'Doctrine\Common\Collections\Collection', + 'Doctrine\Common\Collections\Criteria', + 'Doctrine\Common\Collections\ArrayCollection', + 'Doctrine\DBAL\Types\Types', 'Doctrine\ORM\Mapping', + 'Doctrine\ORM\PersistentCollection', + 'InvalidArgumentException', + 'RuntimeException', + 'Symfony\Bridge\Doctrine\Validator\Constraints', 'Symfony\Component\HttpFoundation\File\File', 'Symfony\Component\Validator\Constraints', + 'Symfony\Component\Uid\Uuid', + 'SymfonyCasts\Bundle\ResetPassword\Model\ResetPasswordRequestInterface', 'Vich\UploaderBundle\Mapping\Annotation', + 'Webmozart\Assert\Assert', ])) ->because('Entities should not have dependency outside App\Entity namespace, except for ApiPlatform, ArrayObject, Doctrine Mapping, Symfony File, Symfony Validator and Vich UploaderBundle'); } diff --git a/src/Rule/LogRule.php b/src/Rule/LogRule.php index c36983f..e9b3a3f 100644 --- a/src/Rule/LogRule.php +++ b/src/Rule/LogRule.php @@ -4,7 +4,7 @@ use Arkitect\Expression\ForClasses\ResideInOneOfTheseNamespaces; use Arkitect\Rules\DSL\ArchRule; -use Atournayre\PHPArkitect\Expression\ForClasses\DependsOnTheseNamespaces; +use Atournayre\PHPArkitect\Expression\ForClasses\DependsOnTheseNamespace; final class LogRule extends Rule { @@ -17,7 +17,7 @@ public static function normalizerMustBeLoggable(): ArchRule { return self::allClasses() ->that(new ResideInOneOfTheseNamespaces('App\Normalizer')) - ->should(new DependsOnTheseNamespaces('Psr\Log\LoggerInterface')) + ->should(new DependsOnTheseNamespace('Psr\Log\LoggerInterface')) ->because('Normalizer must have dependency on LoggerInterface'); } @@ -25,7 +25,7 @@ public static function listenerMustBeLoggable(): ArchRule { return self::allClasses() ->that(new ResideInOneOfTheseNamespaces('App\Listener')) - ->should(new DependsOnTheseNamespaces('Psr\Log\LoggerInterface')) + ->should(new DependsOnTheseNamespace('Psr\Log\LoggerInterface')) ->because('Listener must have dependency on LoggerInterface'); } @@ -33,7 +33,7 @@ public static function loggerMustDependOnLoggerInterface(): ArchRule { return self::allClasses() ->that(new ResideInOneOfTheseNamespaces('App\Logger')) - ->should(new DependsOnTheseNamespaces('Psr\Log\LoggerInterface')) + ->should(new DependsOnTheseNamespace('Psr\Log\LoggerInterface')) ->because('Logger must have dependency on LoggerInterface'); } @@ -41,7 +41,7 @@ public static function serviceMustBeLoggable(): ArchRule { return self::allClasses() ->that(new ResideInOneOfTheseNamespaces('App\Service')) - ->should(new DependsOnTheseNamespaces('Psr\Log\LoggerInterface')) + ->should(new DependsOnTheseNamespace('Psr\Log\LoggerInterface')) ->because('Service must have dependency on LoggerInterface'); } @@ -49,7 +49,7 @@ public static function httpMustBeLoggable(): ArchRule { return self::allClasses() ->that(new ResideInOneOfTheseNamespaces('App\Http')) - ->should(new DependsOnTheseNamespaces('Psr\Log\LoggerInterface')) + ->should(new DependsOnTheseNamespace('Psr\Log\LoggerInterface')) ->because('Http must have dependency on LoggerInterface'); } } diff --git a/src/Rule/MiscRule.php b/src/Rule/MiscRule.php index 3f8c08f..be254f8 100644 --- a/src/Rule/MiscRule.php +++ b/src/Rule/MiscRule.php @@ -12,7 +12,7 @@ use Arkitect\Expression\ForClasses\NotHaveNameMatching; use Arkitect\Expression\ForClasses\ResideInOneOfTheseNamespaces; use Arkitect\Rules\DSL\ArchRule; -use Atournayre\PHPArkitect\Expression\ForClasses\DependsOnTheseNamespaces; +use Atournayre\PHPArkitect\Expression\ForClasses\DependsOnTheseNamespace; use Webmozart\Assert\Assert; final class MiscRule extends Rule @@ -121,7 +121,7 @@ public static function commandMustUseSymfonyStopwatch(): ArchRule { return self::allClasses() ->that(new ResideInOneOfTheseNamespaces('App\Command')) - ->should(new DependsOnTheseNamespaces('Symfony\Component\Stopwatch\Stopwatch')) + ->should(new DependsOnTheseNamespace('Symfony\Component\Stopwatch\Stopwatch')) ->because('We use Stopwatch to measure time of command execution'); } @@ -253,7 +253,7 @@ public static function repositoryMustHaveAtournayreDoctrineTraits(): ArchRule { return self::allClasses() ->that(new ResideInOneOfTheseNamespaces('App\Repository')) - ->should(new DependsOnTheseNamespaces( + ->should(new DependsOnTheseNamespace( 'Atournayre\Component\Doctrine\Traits', )) ->because('Repository should have dependency on Atournayre\Component\Doctrine\Traits, to avoid code duplication and provide useful methods. If not installed, use composer require atournayre/doctrine-component.'); diff --git a/src/Rule/SymfonyRule.php b/src/Rule/SymfonyRule.php index 0ca4511..46f1cc5 100644 --- a/src/Rule/SymfonyRule.php +++ b/src/Rule/SymfonyRule.php @@ -8,7 +8,6 @@ use Arkitect\Expression\ForClasses\IsFinal; use Arkitect\Expression\ForClasses\ResideInOneOfTheseNamespaces; use Arkitect\Rules\DSL\ArchRule; -use Atournayre\PHPArkitect\Expression\ForClasses\DependsOnTheseNamespaces; final class SymfonyRule extends Rule { diff --git a/tests/Unit/Expressions/ForClasses/DependsOnTheseNamespacesTest.php b/tests/Unit/Expressions/ForClasses/DependsOnTheseNamespacesTest.php index aa97090..4689dab 100644 --- a/tests/Unit/Expressions/ForClasses/DependsOnTheseNamespacesTest.php +++ b/tests/Unit/Expressions/ForClasses/DependsOnTheseNamespacesTest.php @@ -5,21 +5,17 @@ use Arkitect\Analyzer\ClassDependency; use Arkitect\Analyzer\ClassDescription; use Arkitect\Rules\Violations; -use Atournayre\PHPArkitect\Expression\ForClasses\DependsOnTheseNamespaces; +use Atournayre\PHPArkitect\Expression\ForClasses\DependsOnTheseNamespace; use PHPUnit\Framework\TestCase; class DependsOnTheseNamespacesTest extends TestCase { - public function test_it_should_return_true_when_class_depends_all_namespaces(): void + public function test_it_should_return_true_when_class_depends_on_namespace(): void { - $dependOnClasses = new DependsOnTheseNamespaces( - 'myNamespace', - 'anotherNamespace' - ); + $dependOnClasses = new DependsOnTheseNamespace('myNamespace\Banana'); $classDescription = ClassDescription::getBuilder('HappyIsland\Myclass') ->addDependency(new ClassDependency('myNamespace\Banana', 0)) - ->addDependency(new ClassDependency('anotherNamespace\Banana', 1)) ->build(); $because = 'we want to add this rule for our software'; $violations = new Violations(); @@ -27,20 +23,17 @@ public function test_it_should_return_true_when_class_depends_all_namespaces(): self::assertEquals(0, $violations->count()); self::assertEquals( - 'should depend on classes in one of these namespaces: myNamespace, anotherNamespace because we want to add this rule for our software', + 'should depend on class in these namespace: myNamespace\Banana because we want to add this rule for our software', $dependOnClasses->describe($classDescription, $because)->toString() ); } - public function test_it_should_return_false_when_class_not_depends_all_namespaces(): void + public function test_it_should_return_false_when_class_not_depends_on_namespace(): void { - $dependOnClasses = new DependsOnTheseNamespaces( - 'myNamespace', - 'anotherNamespace' - ); + $dependOnClasses = new DependsOnTheseNamespace('myNamespace\Banana'); $classDescription = ClassDescription::getBuilder('HappyIsland\Myclass') - ->addDependency(new ClassDependency('myNamespace\Banana', 0)) + ->addDependency(new ClassDependency('anotherNamespace\Banana', 0)) ->build(); $because = 'we want to add this rule for our software'; $violations = new Violations(); @@ -48,7 +41,7 @@ public function test_it_should_return_false_when_class_not_depends_all_namespace self::assertEquals(1, $violations->count()); self::assertEquals( - 'should depend on classes in one of these namespaces: myNamespace, anotherNamespace because we want to add this rule for our software', + 'should depend on class in these namespace: myNamespace\Banana because we want to add this rule for our software', $dependOnClasses->describe($classDescription, $because)->toString() ); } From 166df8906ece7b96b70e95122556a9e1e282f5a8 Mon Sep 17 00:00:00 2001 From: atournayre Date: Sun, 3 Dec 2023 22:27:56 +0100 Subject: [PATCH 06/10] Add new namespace dependencies and refactor validation logic Introduced new namespace dependencies in 'MiscRule.php' to improve structural organization and enforce code cleanliness. The updated rules now help to ensure that classes only depend on suitable namespaces for better application design. Made grammatical corrections on a comment for better readability and understanding. Refactoring of the validity checks provides a more precise monitoring of cross dependencies within application components. --- src/Rule/MiscRule.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/Rule/MiscRule.php b/src/Rule/MiscRule.php index be254f8..2bbc9cc 100644 --- a/src/Rule/MiscRule.php +++ b/src/Rule/MiscRule.php @@ -22,6 +22,7 @@ public static function collectionDependencies(): ArchRule return self::allClasses() ->that(new ResideInOneOfTheseNamespaces('App\Collection')) ->should(new DependsOnlyOnTheseNamespaces( + 'App\Contracts', 'App\Entity', 'Carbon\Carbon', 'DateTimeInterface', @@ -30,6 +31,7 @@ public static function collectionDependencies(): ArchRule 'Doctrine\Common\Collections\ArrayCollection', 'Doctrine\Common\Collections\ReadableCollection', 'Doctrine\ORM\PersistentCollection', + 'Symfony\Contracts', Assert::class, )) ->because('Collection should only depend on Entity, Carbon, DateTimeInterface, Doctrine Collection, Doctrine Criteria, Doctrine ReadableCollection, Doctrine PersistentCollection and Webmozart Assert'); @@ -122,7 +124,7 @@ public static function commandMustUseSymfonyStopwatch(): ArchRule return self::allClasses() ->that(new ResideInOneOfTheseNamespaces('App\Command')) ->should(new DependsOnTheseNamespace('Symfony\Component\Stopwatch\Stopwatch')) - ->because('We use Stopwatch to measure time of command execution'); + ->because('we use Stopwatch to measure time of command execution'); } public static function uniformNamingForContracts(): ArchRule @@ -139,6 +141,7 @@ public static function contractsDependencies(): ArchRule 'App\DTO', 'App\VO', 'DateTimeInterface', + 'Symfony\Component\DependencyInjection\Attribute', )) ->because('Interface should only depend on Entity, DTO, VO and DateTimeInterface'); } @@ -229,6 +232,7 @@ public static function entityExceptionDependencies(): ArchRule 'App\VO', 'DateTimeInterface', 'RuntimeException', + 'Symfony\Component\Uid\Uuid', )) ->because('Exception should only depend on Entity, Enum, VO, DateTimeInterface and RuntimeException'); } From c8801a316de63f486cc8e239820b0992a8168d35 Mon Sep 17 00:00:00 2001 From: atournayre Date: Mon, 4 Dec 2023 23:00:58 +0100 Subject: [PATCH 07/10] Update namespace usage and correct typos in Rule classes Expanded the use of full namespaces in the SymfonyRule.php class to ensure accurate attribute declaration and improved application design. Also introduced new namespace dependencies for better code organization. Corrected several typographical errors in method names and comments to enhance code readability. These changes provide better precision and usability in validation checks for dependencies across various application components. --- src/Rule/SymfonyRule.php | 13 +++++++++---- src/Set/Sets.php | 4 ++-- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/src/Rule/SymfonyRule.php b/src/Rule/SymfonyRule.php index 46f1cc5..9dab160 100644 --- a/src/Rule/SymfonyRule.php +++ b/src/Rule/SymfonyRule.php @@ -28,7 +28,7 @@ public static function commandMustUseSymfonyAsCommandAttribute(): ArchRule { return self::allClasses() ->that(new ResideInOneOfTheseNamespaces('App\Command')) - ->should(new HaveAttribute('AsCommand')) + ->should(new HaveAttribute('Symfony\Component\Console\Attribute\AsCommand')) ->because('Command should have dependency on AsCommand attribute from Symfony Console'); } @@ -37,11 +37,11 @@ public static function uniformNamingForController(): ArchRule return self::uniformNaming('Controller'); } - public static function controllerMustUseSymfonRouteAttribute(): ArchRule + public static function controllerMustUseSymfonyRouteAttribute(): ArchRule { return self::allClasses() ->that(new ResideInOneOfTheseNamespaces('App\Controller')) - ->should(new HaveAttribute('Route')) + ->should(new HaveAttribute('Symfony\Component\Routing\Annotation\Route')) ->because('Controller should have dependency on Route attribute from Symfony Routing'); } @@ -53,12 +53,14 @@ public static function controllerDependencies(): ArchRule 'App\Entity', 'App\Form', 'App\Service', + 'Psr\Log\LoggerInterface', 'Symfony\Bundle\FrameworkBundle\Controller\AbstractController', 'Symfony\Component\HttpFoundation\JsonResponse', 'Symfony\Component\HttpFoundation\RedirectResponse', 'Symfony\Component\HttpFoundation\Request', 'Symfony\Component\HttpFoundation\Response', 'Symfony\Component\Security\Http\Attribute\IsGranted', + 'Symfony\Component\Routing\RouterInterface', )) ->because('Controller should have dependency on AbstractController, Response, Request, RedirectResponse, JsonResponse, IsGranted attribute from Symfony Security, Entity, Form and Service'); } @@ -68,7 +70,9 @@ public static function eventDependencies(): ArchRule return self::allClasses() ->that(new ResideInOneOfTheseNamespaces('App\Event')) ->should(new DependsOnlyOnTheseNamespaces( + 'App\DTO', 'App\Entity', + 'App\VO', 'Symfony\Contracts\EventDispatcher\Event', )) ->because('Event should depends only on Entity and Symfony Event'); @@ -82,7 +86,7 @@ public static function formMustExtendSymfonyAbstractType(): ArchRule ->because('Form should extend Symfony Form AbstractType'); } - public static function formTypesShoudResideInAppFormType(): ArchRule + public static function formTypesShouldResideInAppFormType(): ArchRule { return self::allClasses() ->that(new Extend('Symfony\Component\Form\AbstractType')) @@ -131,6 +135,7 @@ public static function securityDependencies(): ArchRule 'App\Entity', 'App\Service', 'App\Repository', + 'Symfony\Bundle\SecurityBundle\Security', 'Symfony\Component\HttpFoundation', 'Symfony\Component\Routing', 'Symfony\Component\Security', diff --git a/src/Set/Sets.php b/src/Set/Sets.php index f68cb61..38d7c8f 100644 --- a/src/Set/Sets.php +++ b/src/Set/Sets.php @@ -159,7 +159,7 @@ public static function symfonyController(): array { return self::buildArrayOfRules([ SymfonyRule::uniformNamingForController(), - SymfonyRule::controllerMustUseSymfonRouteAttribute(), + SymfonyRule::controllerMustUseSymfonyRouteAttribute(), SymfonyRule::controllerDependencies(), ]); } @@ -177,7 +177,7 @@ public static function symfonyForm(): array return self::buildArrayOfRules([ SymfonyRule::uniformNamingForFormType(), SymfonyRule::formMustExtendSymfonyAbstractType(), - SymfonyRule::formTypesShoudResideInAppFormType(), + SymfonyRule::formTypesShouldResideInAppFormType(), SymfonyRule::formTypeMustBeFinal(), ]); } From 99466ebe6e2fc26f4f821e49ad3042092beecca0 Mon Sep 17 00:00:00 2001 From: atournayre Date: Tue, 5 Dec 2023 23:53:50 +0100 Subject: [PATCH 08/10] Remove detailed architectural rules from PHPArkitekt The architectural rules were too detailed and specific which reduced codebase flexibility. They have been removed to emphasize higher-level design principles and avoid unnecessary constraints. It would also make the codebase more maintainable and it would improve the readability of the PHPArkitekt codebase. --- README.md | 38 ++- src/Builder/RuleBuilder.php | 69 +++++ src/Contracts/AndThatInterface.php | 14 + src/Contracts/ExceptInterface.php | 9 + src/Contracts/RulesInterface.php | 15 + .../ForClasses/DependsOnTheseNamespace.php | 1 - src/Rule/ApiPlaformRule.php | 90 ------ src/Rule/ApiRule.php | 23 -- src/Rule/DoctrineRule.php | 49 --- src/Rule/DomainRule.php | 73 ----- src/Rule/LogRule.php | 55 ---- src/Rule/MiscRule.php | 281 ------------------ src/Rule/Rule.php | 38 --- src/Rule/SymfonyRule.php | 178 ----------- src/Rules/ApiDocumentationOpenApiMisc.php | 27 ++ src/Rules/ApiResourceShouldBeFinalApi.php | 27 ++ ...rfaceShouldResideInAppStateApiPlatform.php | 38 +++ ...rfaceShouldResideInAppStateApiPlatform.php | 38 +++ src/Rules/CollectionDependenciesMisc.php | 28 ++ src/Rules/CommandMustExtendSymfonyCommand.php | 27 ++ ...ommandMustUseSymfonyAsCommandAttribute.php | 27 ++ .../CommandMustUseSymfonyStopwatchMisc.php | 27 ++ src/Rules/ContractsDependenciesMisc.php | 27 ++ src/Rules/ContractsMisc.php | 27 ++ src/Rules/ControllerDependenciesSymfony.php | 27 ++ ...ControllerMustUseSymfonyRouteAttribute.php | 27 ++ src/Rules/DecoratorMustBeFinalMisc.php | 27 ++ ...octrineExtensionMustBeFinalApiPlatform.php | 27 ++ ...ollectionExtensionInterfaceApiPlatform.php | 27 ++ ...QueryItemExtensionInterfaceApiPlatform.php | 27 ++ src/Rules/DtoMustBeFinalMisc.php | 27 ++ src/Rules/DtoMustNotEndWithDtoMisc.php | 27 ++ src/Rules/EngineMustNotDependOnTimeMisc.php | 27 ++ .../EngineRuleMustNotDependOnTimeMisc.php | 27 ++ ...esShouldImplementIsEntityInterfaceMisc.php | 27 ++ ...sShouldNotThrowDoctrineExceptionDomain.php | 27 ++ src/Rules/EntityDependenciesDomain.php | 27 ++ src/Rules/EntityExceptionDependenciesMisc.php | 27 ++ ...ptionNameMustNotEndWithExceptionDomain.php | 27 ++ src/Rules/EnumMisc.php | 27 ++ .../EnumShouldImplementEnumInterfaceMisc.php | 27 ++ src/Rules/EventCollectionDependenciesMisc.php | 28 ++ src/Rules/EventDependenciesSymfony.php | 27 ++ .../ExceptionMustNotEndWithExceptionMisc.php | 27 ++ .../FormMustExtendSymfonyAbstractType.php | 27 ++ src/Rules/FormTypeMustBeFinalSymfony.php | 27 ++ ...mTypesShouldResideInAppFormTypeSymfony.php | 27 ++ src/Rules/HttpMustBeFinalMisc.php | 27 ++ src/Rules/HttpMustBeLoggableLog.php | 27 ++ src/Rules/ListenerDependenciesDoctrine.php | 27 ++ src/Rules/ListenerMustBeLoggableLog.php | 27 ++ ...houldNotThrowDoctrineExceptionDoctrine.php | 27 ++ .../LoggerMustDependOnLoggerInterfaceLog.php | 27 ++ src/Rules/NormalizerMustBeLoggableLog.php | 27 ++ ...sAndProcessorsShouldBeFinalApiPlatform.php | 27 ++ ...ryMustHaveAtournayreDoctrineTraitsMisc.php | 27 ++ src/Rules/SecurityDependenciesSymfony.php | 27 ++ src/Rules/ServiceMustBeFinalSymfony.php | 27 ++ src/Rules/ServiceMustBeLoggableLog.php | 27 ++ src/Rules/TraitsMisc.php | 27 ++ src/Rules/UniformNaming.php | 29 ++ .../UniformNamingForApiDocumentationMisc.php | 11 + src/Rules/UniformNamingForApiResourceApi.php | 11 + src/Rules/UniformNamingForCollectionMisc.php | 11 + src/Rules/UniformNamingForContractsMisc.php | 11 + src/Rules/UniformNamingForDecoratorMisc.php | 11 + ...mNamingForDoctrineExtensionApiPlatform.php | 11 + src/Rules/UniformNamingForEngineMisc.php | 11 + src/Rules/UniformNamingForEngineRuleMisc.php | 11 + src/Rules/UniformNamingForGeneratorMisc.php | 11 + src/Rules/UniformNamingForHttpMisc.php | 11 + src/Rules/UniformNamingForLoggerLog.php | 11 + .../UniformNamingForMigrationDoctrine.php | 11 + src/Rules/UniformNamingForSearchMisc.php | 11 + ...formNamingForStateProcessorApiPlatform.php | 11 + ...iformNamingForStateProviderApiPlatform.php | 11 + src/Rules/UniformNamingForSymfonyCommand.php | 11 + .../UniformNamingForSymfonyController.php | 11 + src/Rules/UniformNamingForSymfonyEvent.php | 11 + src/Rules/UniformNamingForSymfonyFormType.php | 11 + src/Rules/UniformNamingForSymfonyListener.php | 11 + .../UniformNamingForSymfonyNormalizer.php | 11 + src/Rules/UniformNamingForSymfonyProfiler.php | 11 + ...mNamingForSymfonyProfilerDataCollector.php | 11 + .../UniformNamingForSymfonyRepository.php | 11 + src/Rules/UniformNamingForSymfonySecurity.php | 11 + src/Rules/UniformNamingForSymfonyService.php | 11 + .../UniformNamingForSymfonySubscriber.php | 11 + src/Rules/UniformNamingForTraitMisc.php | 11 + src/Rules/VoMustBeFinalMisc.php | 27 ++ src/Rules/VoMustNotEndWithVoMisc.php | 27 ++ src/Set/Sets.php | 257 ++++++++-------- 92 files changed, 1919 insertions(+), 928 deletions(-) create mode 100644 src/Builder/RuleBuilder.php create mode 100644 src/Contracts/AndThatInterface.php create mode 100644 src/Contracts/ExceptInterface.php create mode 100644 src/Contracts/RulesInterface.php delete mode 100644 src/Rule/ApiPlaformRule.php delete mode 100644 src/Rule/ApiRule.php delete mode 100644 src/Rule/DoctrineRule.php delete mode 100644 src/Rule/DomainRule.php delete mode 100644 src/Rule/LogRule.php delete mode 100644 src/Rule/MiscRule.php delete mode 100644 src/Rule/Rule.php delete mode 100644 src/Rule/SymfonyRule.php create mode 100644 src/Rules/ApiDocumentationOpenApiMisc.php create mode 100644 src/Rules/ApiResourceShouldBeFinalApi.php create mode 100644 src/Rules/ClassImplementingProcessorInterfaceShouldResideInAppStateApiPlatform.php create mode 100644 src/Rules/ClassImplementingProviderInterfaceShouldResideInAppStateApiPlatform.php create mode 100644 src/Rules/CollectionDependenciesMisc.php create mode 100644 src/Rules/CommandMustExtendSymfonyCommand.php create mode 100644 src/Rules/CommandMustUseSymfonyAsCommandAttribute.php create mode 100644 src/Rules/CommandMustUseSymfonyStopwatchMisc.php create mode 100644 src/Rules/ContractsDependenciesMisc.php create mode 100644 src/Rules/ContractsMisc.php create mode 100644 src/Rules/ControllerDependenciesSymfony.php create mode 100644 src/Rules/ControllerMustUseSymfonyRouteAttribute.php create mode 100644 src/Rules/DecoratorMustBeFinalMisc.php create mode 100644 src/Rules/DoctrineExtensionMustBeFinalApiPlatform.php create mode 100644 src/Rules/DoctrineExtensionShouldImplementQueryCollectionExtensionInterfaceApiPlatform.php create mode 100644 src/Rules/DoctrineExtensionShouldImplementQueryItemExtensionInterfaceApiPlatform.php create mode 100644 src/Rules/DtoMustBeFinalMisc.php create mode 100644 src/Rules/DtoMustNotEndWithDtoMisc.php create mode 100644 src/Rules/EngineMustNotDependOnTimeMisc.php create mode 100644 src/Rules/EngineRuleMustNotDependOnTimeMisc.php create mode 100644 src/Rules/EntitiesShouldImplementIsEntityInterfaceMisc.php create mode 100644 src/Rules/EntitiesShouldNotThrowDoctrineExceptionDomain.php create mode 100644 src/Rules/EntityDependenciesDomain.php create mode 100644 src/Rules/EntityExceptionDependenciesMisc.php create mode 100644 src/Rules/EntityExceptionNameMustNotEndWithExceptionDomain.php create mode 100644 src/Rules/EnumMisc.php create mode 100644 src/Rules/EnumShouldImplementEnumInterfaceMisc.php create mode 100644 src/Rules/EventCollectionDependenciesMisc.php create mode 100644 src/Rules/EventDependenciesSymfony.php create mode 100644 src/Rules/ExceptionMustNotEndWithExceptionMisc.php create mode 100644 src/Rules/FormMustExtendSymfonyAbstractType.php create mode 100644 src/Rules/FormTypeMustBeFinalSymfony.php create mode 100644 src/Rules/FormTypesShouldResideInAppFormTypeSymfony.php create mode 100644 src/Rules/HttpMustBeFinalMisc.php create mode 100644 src/Rules/HttpMustBeLoggableLog.php create mode 100644 src/Rules/ListenerDependenciesDoctrine.php create mode 100644 src/Rules/ListenerMustBeLoggableLog.php create mode 100644 src/Rules/ListenerShouldNotThrowDoctrineExceptionDoctrine.php create mode 100644 src/Rules/LoggerMustDependOnLoggerInterfaceLog.php create mode 100644 src/Rules/NormalizerMustBeLoggableLog.php create mode 100644 src/Rules/ProvidersAndProcessorsShouldBeFinalApiPlatform.php create mode 100644 src/Rules/RepositoryMustHaveAtournayreDoctrineTraitsMisc.php create mode 100644 src/Rules/SecurityDependenciesSymfony.php create mode 100644 src/Rules/ServiceMustBeFinalSymfony.php create mode 100644 src/Rules/ServiceMustBeLoggableLog.php create mode 100644 src/Rules/TraitsMisc.php create mode 100644 src/Rules/UniformNaming.php create mode 100644 src/Rules/UniformNamingForApiDocumentationMisc.php create mode 100644 src/Rules/UniformNamingForApiResourceApi.php create mode 100644 src/Rules/UniformNamingForCollectionMisc.php create mode 100644 src/Rules/UniformNamingForContractsMisc.php create mode 100644 src/Rules/UniformNamingForDecoratorMisc.php create mode 100644 src/Rules/UniformNamingForDoctrineExtensionApiPlatform.php create mode 100644 src/Rules/UniformNamingForEngineMisc.php create mode 100644 src/Rules/UniformNamingForEngineRuleMisc.php create mode 100644 src/Rules/UniformNamingForGeneratorMisc.php create mode 100644 src/Rules/UniformNamingForHttpMisc.php create mode 100644 src/Rules/UniformNamingForLoggerLog.php create mode 100644 src/Rules/UniformNamingForMigrationDoctrine.php create mode 100644 src/Rules/UniformNamingForSearchMisc.php create mode 100644 src/Rules/UniformNamingForStateProcessorApiPlatform.php create mode 100644 src/Rules/UniformNamingForStateProviderApiPlatform.php create mode 100644 src/Rules/UniformNamingForSymfonyCommand.php create mode 100644 src/Rules/UniformNamingForSymfonyController.php create mode 100644 src/Rules/UniformNamingForSymfonyEvent.php create mode 100644 src/Rules/UniformNamingForSymfonyFormType.php create mode 100644 src/Rules/UniformNamingForSymfonyListener.php create mode 100644 src/Rules/UniformNamingForSymfonyNormalizer.php create mode 100644 src/Rules/UniformNamingForSymfonyProfiler.php create mode 100644 src/Rules/UniformNamingForSymfonyProfilerDataCollector.php create mode 100644 src/Rules/UniformNamingForSymfonyRepository.php create mode 100644 src/Rules/UniformNamingForSymfonySecurity.php create mode 100644 src/Rules/UniformNamingForSymfonyService.php create mode 100644 src/Rules/UniformNamingForSymfonySubscriber.php create mode 100644 src/Rules/UniformNamingForTraitMisc.php create mode 100644 src/Rules/VoMustBeFinalMisc.php create mode 100644 src/Rules/VoMustNotEndWithVoMisc.php diff --git a/README.md b/README.md index 9299b64..2384cd4 100644 --- a/README.md +++ b/README.md @@ -18,26 +18,32 @@ Here is an example of how to use it: // phparkitect.php use Arkitect\ClassSet; use Arkitect\CLI\Config; -use Atournayre\PHPArkitect\Rule\LogRule; -use Atournayre\PHPArkitect\Rule\MiscRule; -use Atournayre\PHPArkitect\Set\Sets; +use Arkitect\Expression\ForClasses\IsFinal; +use Arkitect\Expression\ForClasses\ResideInOneOfTheseNamespaces; +use Arkitect\Rules\Rule; +use Atournayre\PHPArkitect\Builder\RulesBuilder; +use Atournayre\PHPArkitect\Rules; +use Atournayre\PHPArkitect\Set; return static function (Config $config): void { $classSet = ClassSet::fromDir(__DIR__ . '/src'); - // Add all rules for API Platform - $config->add($classSet, ...Sets::apiPlatform()); - // Add all rules for Symfony - $config->add($classSet, ...Sets::symfony()); - // Add subset of rules for Doctrine - $config->add($classSet, ...Sets::doctrineUniformNaming()); - // Add specific rules - $config->add( - $classSet, - LogRule::listenerMustBeLoggable(), - MiscRule::dtoMustNotEndWithDto(), - MiscRule::traits(), - ); + $rules = RulesBuilder::create + ->add(new ListenerMustBeLoggableLog) + // Add all rules for Symfony + ->set(Sets::symfony()) + // Add subset of rules for Doctrine + ->set(Sets::doctrineUniformNaming()) + // Add regular rules + ->add( + Rule::allClasses() + ->that(new ResideInOneOfTheseNamespaces('App')) + ->should(new IsFinal()) + ->because('All classes in App namespace must be final') + ) + ->getRules(); + + $config->add($classSet, $rules); }; ``` You can use sets or rules individually. diff --git a/src/Builder/RuleBuilder.php b/src/Builder/RuleBuilder.php new file mode 100644 index 0000000..f43a980 --- /dev/null +++ b/src/Builder/RuleBuilder.php @@ -0,0 +1,69 @@ +rules[] = $this->buildArchRule($rule); + return $this; + } + + public function set(array $rules): self + { + Assert::allIsInstanceOf($rules, RulesInterface::class); + + foreach ($rules as $rule) { + $this->add($rule); + } + return $this; + } + + public function rules(): array + { + return $this->rules; + } + + private function buildArchRule(RulesInterface|ExceptInterface|AndThatInterface $rule): ArchRule + { + Assert::isInstanceOf($rule, RulesInterface::class); + + $allClasses = Rule::allClasses(); + + if ($rule instanceof ExceptInterface) { + $allClasses = $allClasses->except(...$rule->except()); + } + + $allClasses = $allClasses->that($rule->that()); + + if ($rule instanceof AndThatInterface) { + foreach ($rule->andThat() as $andThatItem) { + $allClasses = $allClasses->andThat($andThatItem); + } + } + + return $allClasses + ->should($rule->should()) + ->because($rule->because()); + } +} diff --git a/src/Contracts/AndThatInterface.php b/src/Contracts/AndThatInterface.php new file mode 100644 index 0000000..f079814 --- /dev/null +++ b/src/Contracts/AndThatInterface.php @@ -0,0 +1,14 @@ +that(new ResideInOneOfTheseNamespaces('App\DoctrineExtension')) - ->should(new Implement('ApiPlatform\Doctrine\Orm\Extension\QueryItemExtensionInterface',)) - ->because('DoctrineExtension should implement ApiPlatform Doctrine QueryItemExtensionInterface'); - } - - public static function doctrineExtensionShouldImplementQueryCollectionExtensionInterface(): ArchRule - { - return self::allClasses() - ->that(new ResideInOneOfTheseNamespaces('App\DoctrineExtension')) - ->should(new Implement('ApiPlatform\Doctrine\Orm\Extension\QueryCollectionExtensionInterface',)) - ->because('DoctrineExtension should implement ApiPlatform Doctrine QueryCollectionExtensionInterface'); - } - - public static function doctrineExtensionMustBeFinal(): ArchRule - { - return self::allClasses() - ->that(new ResideInOneOfTheseNamespaces('App\DoctrineExtension')) - ->should(new IsFinal()) - ->because('DoctrineExtension are not meant to be extended'); - } - - public static function classImplementingProviderInterfaceShouldResideInAppState(): ArchRule - { - return self::allClasses() - ->that(new Implement('ApiPlatform\State\ProviderInterface')) - ->andThat(new IsNotInterface()) - ->andThat(new IsNotTrait()) - ->should(new ResideInOneOfTheseNamespaces( - 'App\State', - 'App\State\Provider', - )) - ->because('it\'s an ApiPlatform convention for Providers'); - } - - public static function classImplementingProcessorInterfaceShouldResideInAppState(): ArchRule - { - return self::allClasses() - ->that(new Implement('ApiPlatform\State\ProcessorInterface')) - ->andThat(new IsNotInterface()) - ->andThat(new IsNotTrait()) - ->should(new ResideInOneOfTheseNamespaces( - 'App\State', - 'App\State\Processor', - )) - ->because('it\'s an ApiPlatform convention for Processors'); - } - - public static function providersAndProcessorsShouldBeFinal(): ArchRule - { - return self::allClasses() - ->that(new ResideInOneOfTheseNamespaces( - 'App\State', - 'App\State\Provider', - 'App\State\Processor', - )) - ->should(new IsFinal()) - ->because('extending a provider should be done using the decorator pattern'); - } - - public static function uniformNamingForStateProcessor(): ArchRule - { - return self::uniformNaming('Processor'); - } - - public static function uniformNamingForStateProvider(): ArchRule - { - return self::uniformNaming('Provider'); - } -} diff --git a/src/Rule/ApiRule.php b/src/Rule/ApiRule.php deleted file mode 100644 index f3a8291..0000000 --- a/src/Rule/ApiRule.php +++ /dev/null @@ -1,23 +0,0 @@ -that(new ResideInOneOfTheseNamespaces('App\ApiResource')) - ->should(new IsFinal()) - ->because('ApiResource are not meant to be extended'); - } -} diff --git a/src/Rule/DoctrineRule.php b/src/Rule/DoctrineRule.php deleted file mode 100644 index 4c3d26f..0000000 --- a/src/Rule/DoctrineRule.php +++ /dev/null @@ -1,49 +0,0 @@ -that(new ResideInOneOfTheseNamespaces('App\Listener')) - ->should(new DependsOnlyOnTheseNamespaces( - 'Doctrine\Bundle\DoctrineBundle\Attribute\AsDoctrineListener', - 'Doctrine\Bundle\DoctrineBundle\Attribute\AsEntityListener', - 'Doctrine\ORM\Event', - 'Doctrine\ORM\Events', - 'App\Decorator', - 'App\DTO', - 'App\Entity', - 'App\Enum', - 'App\Service', - 'App\Repository', - 'App\VO', - 'Symfony\Component\Clock\ClockInterface', - 'Webmozart\Assert\Assert', - )) - ->because('Listener should have dependency on AsEntityListener attribute from Doctrine'); - } - - public static function listenerShouldNotThrowDoctrineException(): ArchRule - { - return self::allClasses() - ->that(new ResideInOneOfTheseNamespaces('App\Listener')) - ->should(new NotDependsOnTheseNamespaces( - 'Doctrine\ORM\NonUniqueResultException', - 'Doctrine\ORM\NoResultException', - )) - ->because('Doctrine Exception should not be used in Listener, catch them in Service instead, log them and rethrow a specific Exception'); - } -} diff --git a/src/Rule/DomainRule.php b/src/Rule/DomainRule.php deleted file mode 100644 index 8d224e7..0000000 --- a/src/Rule/DomainRule.php +++ /dev/null @@ -1,73 +0,0 @@ -that(new ResideInOneOfTheseNamespaces('App\Entity')) - ->should(new NotHaveDependencyOutsideNamespace('App\Entity', [ - 'ApiPlatform\Doctrine', - 'ApiPlatform\Metadata', - 'ApiPlatform\OpenApi\Model', - 'App\ApiPlatform\Endpoint', - 'App\ApiResource\Groupes', - 'App\Collection', - 'App\Contracts', - 'App\DTO', - 'App\Enum', - 'App\Repository', - 'App\State', - 'App\State\Provider', - 'App\State\Processor', - 'App\VO', - 'ArrayObject', - 'Carbon', - 'DateTimeImmutable', - 'DateTimeInterface', - 'Doctrine\Common\Collections\Collection', - 'Doctrine\Common\Collections\Criteria', - 'Doctrine\Common\Collections\ArrayCollection', - 'Doctrine\DBAL\Types\Types', - 'Doctrine\ORM\Mapping', - 'Doctrine\ORM\PersistentCollection', - 'InvalidArgumentException', - 'RuntimeException', - 'Symfony\Bridge\Doctrine\Validator\Constraints', - 'Symfony\Component\HttpFoundation\File\File', - 'Symfony\Component\Validator\Constraints', - 'Symfony\Component\Uid\Uuid', - 'SymfonyCasts\Bundle\ResetPassword\Model\ResetPasswordRequestInterface', - 'Vich\UploaderBundle\Mapping\Annotation', - 'Webmozart\Assert\Assert', - ])) - ->because('Entities should not have dependency outside App\Entity namespace, except for ApiPlatform, ArrayObject, Doctrine Mapping, Symfony File, Symfony Validator and Vich UploaderBundle'); - } - - public static function entityExceptionNameMustNotEndWithException(): ArchRule - { - return self::allClasses() - ->that(new ResideInOneOfTheseNamespaces('App\Entity\Exception')) - ->should(new NotHaveNameMatching('*Exception')) - ->because('Exception should not be suffixed by Exception in order to be meaningful when suffixed by named constructor'); - } - - public static function entitiesShouldNotThrowDoctrineException(): ArchRule - { - return self::allClasses() - ->that(new ResideInOneOfTheseNamespaces('App\Entity')) - ->should(new NotDependsOnTheseNamespaces( - 'Doctrine\ORM\NonUniqueResultException', - 'Doctrine\ORM\NoResultException', - )) - ->because('Doctrine Exception should not be used in Entity, catch them in Repository instead, log them and rethrow a specific Exception'); - } -} diff --git a/src/Rule/LogRule.php b/src/Rule/LogRule.php deleted file mode 100644 index e9b3a3f..0000000 --- a/src/Rule/LogRule.php +++ /dev/null @@ -1,55 +0,0 @@ -that(new ResideInOneOfTheseNamespaces('App\Normalizer')) - ->should(new DependsOnTheseNamespace('Psr\Log\LoggerInterface')) - ->because('Normalizer must have dependency on LoggerInterface'); - } - - public static function listenerMustBeLoggable(): ArchRule - { - return self::allClasses() - ->that(new ResideInOneOfTheseNamespaces('App\Listener')) - ->should(new DependsOnTheseNamespace('Psr\Log\LoggerInterface')) - ->because('Listener must have dependency on LoggerInterface'); - } - - public static function loggerMustDependOnLoggerInterface(): ArchRule - { - return self::allClasses() - ->that(new ResideInOneOfTheseNamespaces('App\Logger')) - ->should(new DependsOnTheseNamespace('Psr\Log\LoggerInterface')) - ->because('Logger must have dependency on LoggerInterface'); - } - - public static function serviceMustBeLoggable(): ArchRule - { - return self::allClasses() - ->that(new ResideInOneOfTheseNamespaces('App\Service')) - ->should(new DependsOnTheseNamespace('Psr\Log\LoggerInterface')) - ->because('Service must have dependency on LoggerInterface'); - } - - public static function httpMustBeLoggable(): ArchRule - { - return self::allClasses() - ->that(new ResideInOneOfTheseNamespaces('App\Http')) - ->should(new DependsOnTheseNamespace('Psr\Log\LoggerInterface')) - ->because('Http must have dependency on LoggerInterface'); - } -} diff --git a/src/Rule/MiscRule.php b/src/Rule/MiscRule.php deleted file mode 100644 index 2bbc9cc..0000000 --- a/src/Rule/MiscRule.php +++ /dev/null @@ -1,281 +0,0 @@ -that(new ResideInOneOfTheseNamespaces('App\Collection')) - ->should(new DependsOnlyOnTheseNamespaces( - 'App\Contracts', - 'App\Entity', - 'Carbon\Carbon', - 'DateTimeInterface', - 'Doctrine\Common\Collections\Collection', - 'Doctrine\Common\Collections\Criteria', - 'Doctrine\Common\Collections\ArrayCollection', - 'Doctrine\Common\Collections\ReadableCollection', - 'Doctrine\ORM\PersistentCollection', - 'Symfony\Contracts', - Assert::class, - )) - ->because('Collection should only depend on Entity, Carbon, DateTimeInterface, Doctrine Collection, Doctrine Criteria, Doctrine ReadableCollection, Doctrine PersistentCollection and Webmozart Assert'); - } - - public static function eventCollectionDependencies(): ArchRule - { - return self::allClasses() - ->that(new ResideInOneOfTheseNamespaces('App\Event\Collection')) - ->should(new DependsOnlyOnTheseNamespaces( - 'App\Entity', - 'Carbon\Carbon', - 'DateTimeInterface', - 'Doctrine\Common\Collections\Collection', - 'Doctrine\Common\Collections\Criteria', - 'Doctrine\Common\Collections\ArrayCollection', - 'Doctrine\Common\Collections\ReadableCollection', - 'Doctrine\ORM\PersistentCollection', - Assert::class, - 'Symfony\Contracts\EventDispatcher\Event' - )) - ->because('Collection should only depend on Entity, Carbon, DateTimeInterface, Doctrine Collection, Doctrine Criteria, Doctrine ReadableCollection, Doctrine PersistentCollection, Webmozart Assert and Symfony EventDispatcher Event'); - } - - public static function uniformNamingForCollection(): ArchRule - { - return self::uniformNaming('Collection'); - } - - public static function dtoMustNotEndWithDto(): ArchRule - { - return self::allClasses() - ->that(new ResideInOneOfTheseNamespaces('App\DTO')) - ->should(new NotHaveNameMatching('*DTO')) - ->because('DTO should not be suffixed by DTO'); - } - - public static function dtoMustBeFinal(): ArchRule - { - return self::allClasses() - ->that(new ResideInOneOfTheseNamespaces('App\DTO')) - ->should(new IsFinal()) - ->because('Extending a DTO is not a good practice'); - } - - public static function voMustNotEndWithVo(): ArchRule - { - return self::allClasses() - ->that(new ResideInOneOfTheseNamespaces('App\VO')) - ->should(new NotHaveNameMatching('*VO')) - ->because('VO should not be suffixed by VO'); - } - - public static function voMustBeFinal(): ArchRule - { - return self::allClasses() - ->that(new ResideInOneOfTheseNamespaces('App\VO')) - ->should(new IsFinal()) - ->because('Extending a VO is not a good practice'); - } - - public static function uniformNamingForDecorator(): ArchRule - { - return self::uniformNaming('Decorator'); - } - - public static function decoratorMustBeFinal(): ArchRule - { - return self::allClasses() - ->that(new ResideInOneOfTheseNamespaces('App\Decorator')) - ->should(new IsFinal()) - ->because('Decorators are not meant to be extended but composed'); - } - - public static function uniformNamingForApiDocumentation(): ArchRule - { - return self::uniformNaming('ApiDocumentation'); - } - - public static function apiDocumentationOpenApi(): ArchRule - { - return self::allClasses() - ->that(new ResideInOneOfTheseNamespaces('App\Api\Documentation')) - ->should(new HaveNameMatching('/OpenApi$/')) - ->because('API Documentation should have a name with "OpenApi" inside'); - } - - public static function commandMustUseSymfonyStopwatch(): ArchRule - { - return self::allClasses() - ->that(new ResideInOneOfTheseNamespaces('App\Command')) - ->should(new DependsOnTheseNamespace('Symfony\Component\Stopwatch\Stopwatch')) - ->because('we use Stopwatch to measure time of command execution'); - } - - public static function uniformNamingForContracts(): ArchRule - { - return self::uniformNaming('Interface'); - } - - public static function contractsDependencies(): ArchRule - { - return self::allClasses() - ->that(new ResideInOneOfTheseNamespaces('App\Contracts')) - ->should(new DependsOnlyOnTheseNamespaces( - 'App\Entity', - 'App\DTO', - 'App\VO', - 'DateTimeInterface', - 'Symfony\Component\DependencyInjection\Attribute', - )) - ->because('Interface should only depend on Entity, DTO, VO and DateTimeInterface'); - } - - public static function engineMustNotDependOnTime(): ArchRule - { - return self::allClasses() - ->that(new ResideInOneOfTheseNamespaces('App\Engine')) - ->should(new NotDependsOnTheseNamespaces( - 'Psr\Clock\ClockInterface', - 'Symfony\Component\Clock\ClockInterface', - )) - ->because('time should be injected in Engine instead of using ClockInterface or Psr ClockInterface directly'); - } - - public static function engineRuleMustNotDependOnTime(): ArchRule - { - return self::allClasses() - ->that(new ResideInOneOfTheseNamespaces('App\EngineRule')) - ->should(new NotDependsOnTheseNamespaces( - 'Psr\Clock\ClockInterface', - 'Symfony\Component\Clock\ClockInterface', - )) - ->because('time should be injected in EngineRule instead of using ClockInterface or Psr ClockInterface directly'); - } - - public static function uniformNamingForEngine(): ArchRule - { - return self::uniformNaming('Engine'); - } - - public static function uniformNamingForTrait(): ArchRule - { - return self::uniformNaming('Trait'); - } - - public static function uniformNamingForGenerator(): ArchRule - { - return self::uniformNaming('Generator'); - } - - public static function uniformNamingForHttp(): ArchRule - { - return self::uniformNaming('Http'); - } - - public static function httpMustBeFinal(): ArchRule - { - return self::allClasses() - ->that(new ResideInOneOfTheseNamespaces('App\Http')) - ->should(new IsFinal()) - ->because('Extending a Http is not a good practice and should be decorated using Symfony Decorator Pattern'); - } - - public static function uniformNamingForSearch(): ArchRule - { - return self::uniformNaming('Search'); - } - - public static function uniformNamingForEngineRule(): ArchRule - { - return self::uniformNaming('EngineRule'); - } - - public static function enum(): ArchRule - { - return self::allClasses() - ->that(new ResideInOneOfTheseNamespaces('App\Enum')) - ->should(new IsEnum()) - ->because('we want to be sure that all classes are enum'); - } - - public static function enumShouldImplementEnumInterface(): ArchRule - { - return self::allClasses() - ->that(new ResideInOneOfTheseNamespaces('App\Enum')) - ->should(new Implement('App\Contracts\EnumInterface',)) - ->because('Enum should implement EnumInterface'); - } - - public static function entityExceptionDependencies(): ArchRule - { - return self::allClasses() - ->that(new ResideInOneOfTheseNamespaces('App\Entity\Exception')) - ->should(new DependsOnlyOnTheseNamespaces( - 'App\Entity', - 'App\Enum', - 'App\VO', - 'DateTimeInterface', - 'RuntimeException', - 'Symfony\Component\Uid\Uuid', - )) - ->because('Exception should only depend on Entity, Enum, VO, DateTimeInterface and RuntimeException'); - } - - public static function exceptionMustNotEndWithException(): ArchRule - { - return self::allClasses() - ->that(new ResideInOneOfTheseNamespaces('App\Exception')) - ->should(new NotHaveNameMatching('*Exception')) - ->because('Exception should not be suffixed by Exception in order to be meaningful when suffixed by named constructor'); - } - - public static function entitiesShouldImplementIsEntityInterface(): ArchRule - { - return self::allClasses() - ->that(new ResideInOneOfTheseNamespaces('App\Entity')) - ->should(new Implement('Atournayre\Component\Doctrine\Contracts\IsEntityInterface')) - ->because('it simplifies the check of the type of an object, and will be useful in repositories. If not installed, use composer require atournayre/doctrine-component.'); - } - - public static function repositoryMustHaveAtournayreDoctrineTraits(): ArchRule - { - return self::allClasses() - ->that(new ResideInOneOfTheseNamespaces('App\Repository')) - ->should(new DependsOnTheseNamespace( - 'Atournayre\Component\Doctrine\Traits', - )) - ->because('Repository should have dependency on Atournayre\Component\Doctrine\Traits, to avoid code duplication and provide useful methods. If not installed, use composer require atournayre/doctrine-component.'); - } - - public static function traits(): ArchRule - { - return self::allClasses() - ->that(new ResideInOneOfTheseNamespaces('App\Traits')) - ->should(new IsTrait()) - ->because('we want to be sure that there are only traits in a specific namespace'); - } - - public static function contracts(): ArchRule - { - return self::allClasses() - ->that(new HaveNameMatching('*Interface')) - ->should(new ResideInOneOfTheseNamespaces('App\Contracts')) - ->because('we want to be sure that there are only interfaces in a specific namespace'); - } -} diff --git a/src/Rule/Rule.php b/src/Rule/Rule.php deleted file mode 100644 index 687dcea..0000000 --- a/src/Rule/Rule.php +++ /dev/null @@ -1,38 +0,0 @@ -that(new ResideInOneOfTheseNamespaces('App\\' . $nameMatching)) - ->should(new HaveNameMatching('*' . $nameMatching)) - ->because('Classes should have a name matching ' . $nameMatching); - } - - public static function all(): array - { - $reflection = new \ReflectionClass(static::class); - $methods = $reflection->getMethods(\ReflectionMethod::IS_STATIC); - $methods = array_filter($methods, function ($method) { - return $method->isPublic() && $method->getName() !== 'all'; - }); - - return array_map( - fn ($method) => static::{$method->getName()}(), - $methods - ); - } -} diff --git a/src/Rule/SymfonyRule.php b/src/Rule/SymfonyRule.php deleted file mode 100644 index 9dab160..0000000 --- a/src/Rule/SymfonyRule.php +++ /dev/null @@ -1,178 +0,0 @@ -that(new ResideInOneOfTheseNamespaces('App\Command')) - ->should(new Extend('Symfony\Component\Console\Command\Command')) - ->because('Command should have dependency on Symfony Console Command'); - } - - public static function commandMustUseSymfonyAsCommandAttribute(): ArchRule - { - return self::allClasses() - ->that(new ResideInOneOfTheseNamespaces('App\Command')) - ->should(new HaveAttribute('Symfony\Component\Console\Attribute\AsCommand')) - ->because('Command should have dependency on AsCommand attribute from Symfony Console'); - } - - public static function uniformNamingForController(): ArchRule - { - return self::uniformNaming('Controller'); - } - - public static function controllerMustUseSymfonyRouteAttribute(): ArchRule - { - return self::allClasses() - ->that(new ResideInOneOfTheseNamespaces('App\Controller')) - ->should(new HaveAttribute('Symfony\Component\Routing\Annotation\Route')) - ->because('Controller should have dependency on Route attribute from Symfony Routing'); - } - - public static function controllerDependencies(): ArchRule - { - return self::allClasses() - ->that(new ResideInOneOfTheseNamespaces('App\Controller')) - ->should(new DependsOnlyOnTheseNamespaces( - 'App\Entity', - 'App\Form', - 'App\Service', - 'Psr\Log\LoggerInterface', - 'Symfony\Bundle\FrameworkBundle\Controller\AbstractController', - 'Symfony\Component\HttpFoundation\JsonResponse', - 'Symfony\Component\HttpFoundation\RedirectResponse', - 'Symfony\Component\HttpFoundation\Request', - 'Symfony\Component\HttpFoundation\Response', - 'Symfony\Component\Security\Http\Attribute\IsGranted', - 'Symfony\Component\Routing\RouterInterface', - )) - ->because('Controller should have dependency on AbstractController, Response, Request, RedirectResponse, JsonResponse, IsGranted attribute from Symfony Security, Entity, Form and Service'); - } - - public static function eventDependencies(): ArchRule - { - return self::allClasses() - ->that(new ResideInOneOfTheseNamespaces('App\Event')) - ->should(new DependsOnlyOnTheseNamespaces( - 'App\DTO', - 'App\Entity', - 'App\VO', - 'Symfony\Contracts\EventDispatcher\Event', - )) - ->because('Event should depends only on Entity and Symfony Event'); - } - - public static function formMustExtendSymfonyAbstractType(): ArchRule - { - return self::allClasses() - ->that(new ResideInOneOfTheseNamespaces('App\Form\Type')) - ->should(new Extend('Symfony\Component\Form\AbstractType')) - ->because('Form should extend Symfony Form AbstractType'); - } - - public static function formTypesShouldResideInAppFormType(): ArchRule - { - return self::allClasses() - ->that(new Extend('Symfony\Component\Form\AbstractType')) - ->should(new ResideInOneOfTheseNamespaces('App\Form\Type')) - ->because('Form that extend Symfony Form AbstractType should reside in App\Form\Type'); - } - - public static function formTypeMustBeFinal(): ArchRule - { - return self::allClasses() - ->that(new ResideInOneOfTheseNamespaces('App\Form\Type')) - ->should(new IsFinal()) - ->because('Form are not meant to be extended, except using Symfony best practices'); - } - - public static function uniformNamingForProfilerDataCollector(): ArchRule - { - return self::uniformNaming('ProfilerDataCollector'); - } - - public static function uniformNamingForProfiler(): ArchRule - { - return self::uniformNaming('Profiler'); - } - - public static function uniformNamingForFormType(): ArchRule - { - return self::uniformNaming('Type'); - } - - public static function uniformNamingForRepository(): ArchRule - { - return self::uniformNaming('Repository'); - } - - public static function uniformNamingForNormalizer(): ArchRule - { - return self::uniformNaming('Normalizer'); - } - - public static function securityDependencies(): ArchRule - { - return self::allClasses() - ->that(new ResideInOneOfTheseNamespaces('App\Security')) - ->should(new DependsOnlyOnTheseNamespaces( - 'App\Entity', - 'App\Service', - 'App\Repository', - 'Symfony\Bundle\SecurityBundle\Security', - 'Symfony\Component\HttpFoundation', - 'Symfony\Component\Routing', - 'Symfony\Component\Security', - )) - ->because('Security should have dependency on Entity, Service, Repository and Symfony Security'); - } - - public static function uniformNamingForService(): ArchRule - { - return self::uniformNaming('Service'); - } - - public static function uniformNamingForListener(): ArchRule - { - return self::uniformNaming('Listener'); - } - - public static function uniformNamingForEvent(): ArchRule - { - return self::uniformNaming('Event'); - } - - public static function uniformNamingForSecurity(): ArchRule - { - return self::uniformNaming('Security'); - } - - public static function serviceMustBeFinal(): ArchRule - { - return self::allClasses() - ->that(new ResideInOneOfTheseNamespaces('App\Service')) - ->should(new IsFinal()) - ->because('Service are not meant to be extended but decorated using Symfony Decorator Pattern'); - } - - public static function uniformNamingForSubscriber(): ArchRule - { - return self::uniformNaming('Subscriber'); - } -} diff --git a/src/Rules/ApiDocumentationOpenApiMisc.php b/src/Rules/ApiDocumentationOpenApiMisc.php new file mode 100644 index 0000000..75610d6 --- /dev/null +++ b/src/Rules/ApiDocumentationOpenApiMisc.php @@ -0,0 +1,27 @@ +naming); + } + + public function should(): Expression + { + return new HaveNameMatching('*'.$this->naming); + } + + public function because(): string + { + return 'Classes should have a name matching '.$this->naming; + } +} diff --git a/src/Rules/UniformNamingForApiDocumentationMisc.php b/src/Rules/UniformNamingForApiDocumentationMisc.php new file mode 100644 index 0000000..30eb33a --- /dev/null +++ b/src/Rules/UniformNamingForApiDocumentationMisc.php @@ -0,0 +1,11 @@ + self::buildArrayOfRules($rule), + $rules + ); Assert::allIsInstanceOf($arrayOfRules, ArchRule::class, 'The object passed in parameter is not an ArchRule.'); return $arrayOfRules; @@ -220,75 +229,75 @@ public static function buildArrayOfRules(array $rules): array public static function collection(): array { return self::buildArrayOfRules([ - MiscRule::uniformNamingForCollection(), - MiscRule::collectionDependencies(), - MiscRule::eventCollectionDependencies(), + new UniformNamingForCollectionMisc, + new CollectionDependenciesMisc, + new EventCollectionDependenciesMisc, ]); } public static function dto(): array { return self::buildArrayOfRules([ - MiscRule::dtoMustNotEndWithDto(), - MiscRule::dtoMustBeFinal(), + new DtoMustNotEndWithDtoMisc, + new DtoMustBeFinalMisc, ]); } public static function vo(): array { return self::buildArrayOfRules([ - MiscRule::voMustNotEndWithVo(), - MiscRule::voMustBeFinal(), + new VoMustNotEndWithVoMisc, + new VoMustBeFinalMisc, ]); } public static function decorator(): array { return self::buildArrayOfRules([ - MiscRule::uniformNamingForDecorator(), - MiscRule::decoratorMustBeFinal(), + new UniformNamingForDecoratorMisc, + new DecoratorMustBeFinalMisc, ]); } public static function contracts(): array { return self::buildArrayOfRules([ - MiscRule::uniformNamingForContracts(), - MiscRule::contractsDependencies(), + new UniformNamingForContractsMisc, + new ContractsDependenciesMisc, ]); } public static function engine(): array { return self::buildArrayOfRules([ - MiscRule::uniformNamingForEngine(), - MiscRule::engineMustNotDependOnTime(), - MiscRule::uniformNamingForEngineRule(), - MiscRule::engineRuleMustNotDependOnTime(), + new UniformNamingForEngineMisc, + new EngineMustNotDependOnTimeMisc, + new UniformNamingForEngineRuleMisc, + new EngineRuleMustNotDependOnTimeMisc, ]); } public static function http(): array { return self::buildArrayOfRules([ - MiscRule::uniformNamingForHttp(), - MiscRule::httpMustBeFinal(), + new UniformNamingForHttpMisc, + new HttpMustBeFinalMisc, ]); } public static function enum(): array { return self::buildArrayOfRules([ - MiscRule::enum(), - MiscRule::enumShouldImplementEnumInterface(), + new EnumMisc, + new EnumShouldImplementEnumInterfaceMisc, ]); } public static function exception(): array { return self::buildArrayOfRules([ - MiscRule::entityExceptionDependencies(), - MiscRule::exceptionMustNotEndWithException(), + new EntityExceptionDependenciesMisc, + new ExceptionMustNotEndWithExceptionMisc, ]); } } From 6a5cb86e19f2e67fc92f5dfa6574196eda1207c3 Mon Sep 17 00:00:00 2001 From: atournayre Date: Tue, 5 Dec 2023 23:59:44 +0100 Subject: [PATCH 09/10] Update RuleBuilder to accept RulesInterface and ArchRule The add method in RuleBuilder now accepts both RulesInterface and ArchRule instances. This update provides higher flexibility by allowing different rule types. Additionally, corresponding changes have been made in the README.md as well. --- README.md | 12 ++++++------ src/Builder/RuleBuilder.php | 4 ++-- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index 2384cd4..6bfa85a 100644 --- a/README.md +++ b/README.md @@ -21,9 +21,9 @@ use Arkitect\CLI\Config; use Arkitect\Expression\ForClasses\IsFinal; use Arkitect\Expression\ForClasses\ResideInOneOfTheseNamespaces; use Arkitect\Rules\Rule; -use Atournayre\PHPArkitect\Builder\RulesBuilder; -use Atournayre\PHPArkitect\Rules; -use Atournayre\PHPArkitect\Set; +use Atournayre\PHPArkitect\Builder\RuleBuilder; +use Atournayre\PHPArkitect\Rules\ListenerMustBeLoggableLog; +use Atournayre\PHPArkitect\Set\Sets; return static function (Config $config): void { $classSet = ClassSet::fromDir(__DIR__ . '/src'); @@ -31,7 +31,7 @@ return static function (Config $config): void { $rules = RulesBuilder::create ->add(new ListenerMustBeLoggableLog) // Add all rules for Symfony - ->set(Sets::symfony()) + ->set(Sets::symfonyCommand()) // Add subset of rules for Doctrine ->set(Sets::doctrineUniformNaming()) // Add regular rules @@ -41,9 +41,9 @@ return static function (Config $config): void { ->should(new IsFinal()) ->because('All classes in App namespace must be final') ) - ->getRules(); + ->rules(); - $config->add($classSet, $rules); + $config->add($classSet, ...$rules); }; ``` You can use sets or rules individually. diff --git a/src/Builder/RuleBuilder.php b/src/Builder/RuleBuilder.php index f43a980..d1ace5f 100644 --- a/src/Builder/RuleBuilder.php +++ b/src/Builder/RuleBuilder.php @@ -23,9 +23,9 @@ public static function create(): self return new self(); } - public function add(RulesInterface $rule): self + public function add(RulesInterface|ArchRule $rule): self { - $this->rules[] = $this->buildArchRule($rule); + $this->rules[] = $rule instanceof ArchRule ? $rule : $this->buildArchRule($rule); return $this; } From 48f52d0942654def5d74c6a02b7b1fde514a8b53 Mon Sep 17 00:00:00 2001 From: atournayre Date: Wed, 6 Dec 2023 00:01:43 +0100 Subject: [PATCH 10/10] Refine comments for Symfony and Doctrine rule sets in README Clarifications made in README.md about the rules being added for Symfony Command and Doctrine Naming. This improves understanding of which specific rules are being adopted. --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 6bfa85a..29eb4a3 100644 --- a/README.md +++ b/README.md @@ -30,9 +30,9 @@ return static function (Config $config): void { $rules = RulesBuilder::create ->add(new ListenerMustBeLoggableLog) - // Add all rules for Symfony + // Add rules for Symfony Command ->set(Sets::symfonyCommand()) - // Add subset of rules for Doctrine + // Add rules for Doctrine Naming ->set(Sets::doctrineUniformNaming()) // Add regular rules ->add(