From 72028a3df9d5e3e37296ef4401d635fa875e1414 Mon Sep 17 00:00:00 2001 From: patrickkusebauch Date: Tue, 7 Jun 2022 20:06:16 +0200 Subject: [PATCH 1/5] first steps to provide private collectors --- src/Analyser/DependencyLayersAnalyser.php | 4 ++-- src/Analyser/LayerForTokenAnalyser.php | 2 +- src/Analyser/TokenInLayerAnalyser.php | 6 ++--- src/Layer/Collector/AttributeCollector.php | 2 +- src/Layer/Collector/BoolCollector.php | 4 ++-- src/Layer/Collector/Collectable.php | 2 +- src/Layer/Collector/CollectorInterface.php | 2 +- .../ConditionalCollectorInterface.php | 2 +- src/Layer/Collector/ExtendsCollector.php | 2 +- src/Layer/Collector/FunctionNameCollector.php | 2 +- src/Layer/Collector/ImplementsCollector.php | 5 +--- src/Layer/Collector/InheritsCollector.php | 5 +--- src/Layer/Collector/RegexCollector.php | 4 ++-- src/Layer/Collector/SuperglobalCollector.php | 2 +- src/Layer/Collector/UsesCollector.php | 2 +- src/Layer/LayerResolver.php | 24 ++++++++++--------- src/Layer/LayerResolverInterface.php | 2 +- tests/Layer/LayerResolverTest.php | 2 +- 18 files changed, 35 insertions(+), 39 deletions(-) diff --git a/src/Analyser/DependencyLayersAnalyser.php b/src/Analyser/DependencyLayersAnalyser.php index 0e2532c24..56391af99 100644 --- a/src/Analyser/DependencyLayersAnalyser.php +++ b/src/Analyser/DependencyLayersAnalyser.php @@ -51,7 +51,7 @@ public function process(): Result foreach ($dependencies->getDependenciesAndInheritDependencies() as $dependency) { $depender = $dependency->getDepender(); $dependerRef = $this->tokenResolver->resolve($depender, $astMap); - $dependerLayers = $this->dependerLayerResolver->getLayersForReference($dependerRef, $astMap); + $dependerLayers = array_keys($this->dependerLayerResolver->getLayersForReference($dependerRef, $astMap)); if (!isset($warnings[$depender->toString()]) && count($dependerLayers) > 1) { $warnings[$depender->toString()] = Warning::tokenIsInMoreThanOneLayer($depender, $dependerLayers); @@ -59,7 +59,7 @@ public function process(): Result $dependent = $dependency->getDependent(); $dependentRef = $this->tokenResolver->resolve($dependent, $astMap); - $dependentLayers = $this->dependentLayerResolver->getLayersForReference($dependentRef, $astMap); + $dependentLayers = array_keys($this->dependentLayerResolver->getLayersForReference($dependentRef, $astMap)); foreach ($dependerLayers as $dependentLayer) { $event = new ProcessEvent($dependency, $dependerRef, $dependentLayer, $dependentRef, $dependentLayers, $result); diff --git a/src/Analyser/LayerForTokenAnalyser.php b/src/Analyser/LayerForTokenAnalyser.php index c62a1c4f1..9e88eb35f 100644 --- a/src/Analyser/LayerForTokenAnalyser.php +++ b/src/Analyser/LayerForTokenAnalyser.php @@ -66,7 +66,7 @@ private function findLayersForReferences(array $references, string $tokenName, A continue; } $token = $this->tokenResolver->resolve($reference->getToken(), $astMap); - $matchingLayers = $this->layerResolver->getLayersForReference($token, $astMap); + $matchingLayers = array_keys($this->layerResolver->getLayersForReference($token, $astMap)); natcasesort($matchingLayers); diff --git a/src/Analyser/TokenInLayerAnalyser.php b/src/Analyser/TokenInLayerAnalyser.php index d1add860e..bf71098dc 100644 --- a/src/Analyser/TokenInLayerAnalyser.php +++ b/src/Analyser/TokenInLayerAnalyser.php @@ -59,7 +59,7 @@ public function findTokensInLayer(string $layer): array if (in_array(TokenType::CLASS_LIKE, $this->tokenTypes, true)) { foreach ($astMap->getClassLikeReferences() as $classReference) { $classToken = $this->tokenResolver->resolve($classReference->getToken(), $astMap); - if (in_array($layer, $this->layerResolver->getLayersForReference($classToken, $astMap), true)) { + if (array_key_exists($layer, $this->layerResolver->getLayersForReference($classToken, $astMap))) { $matchingTokens[] = $classToken->getToken() ->toString(); } @@ -69,7 +69,7 @@ public function findTokensInLayer(string $layer): array if (in_array(TokenType::FUNCTION, $this->tokenTypes, true)) { foreach ($astMap->getFunctionLikeReferences() as $functionReference) { $functionToken = $this->tokenResolver->resolve($functionReference->getToken(), $astMap); - if (in_array($layer, $this->layerResolver->getLayersForReference($functionToken, $astMap), true)) { + if (array_key_exists($layer, $this->layerResolver->getLayersForReference($functionToken, $astMap))) { $matchingTokens[] = $functionToken->getToken() ->toString(); } @@ -79,7 +79,7 @@ public function findTokensInLayer(string $layer): array if (in_array(TokenType::FILE, $this->tokenTypes, true)) { foreach ($astMap->getFileReferences() as $fileReference) { $fileToken = $this->tokenResolver->resolve($fileReference->getToken(), $astMap); - if (in_array($layer, $this->layerResolver->getLayersForReference($fileToken, $astMap), true)) { + if (array_key_exists($layer, $this->layerResolver->getLayersForReference($fileToken, $astMap))) { $matchingTokens[] = $fileToken->getToken() ->toString(); } diff --git a/src/Layer/Collector/AttributeCollector.php b/src/Layer/Collector/AttributeCollector.php index 9e535488b..50c6cd286 100644 --- a/src/Layer/Collector/AttributeCollector.php +++ b/src/Layer/Collector/AttributeCollector.php @@ -41,7 +41,7 @@ public function satisfy(array $config, TokenReferenceInterface $reference, AstMa } /** - * @param array> $config + * @param array> $config */ private function getSearchedSubstring(array $config): string { diff --git a/src/Layer/Collector/BoolCollector.php b/src/Layer/Collector/BoolCollector.php index f801b3bc9..c475c3812 100644 --- a/src/Layer/Collector/BoolCollector.php +++ b/src/Layer/Collector/BoolCollector.php @@ -76,9 +76,9 @@ public function resolvable(array $config): bool } /** - * @param array> $configuration + * @param array> $configuration * - * @return array> + * @return array> */ private function normalizeConfiguration(array $configuration): array { diff --git a/src/Layer/Collector/Collectable.php b/src/Layer/Collector/Collectable.php index 0564018ec..bbfe3dcda 100644 --- a/src/Layer/Collector/Collectable.php +++ b/src/Layer/Collector/Collectable.php @@ -28,7 +28,7 @@ public function getCollector(): CollectorInterface } /** - * @return array> + * @return array> */ public function getAttributes(): array { diff --git a/src/Layer/Collector/CollectorInterface.php b/src/Layer/Collector/CollectorInterface.php index 678015aae..96a41494d 100644 --- a/src/Layer/Collector/CollectorInterface.php +++ b/src/Layer/Collector/CollectorInterface.php @@ -13,7 +13,7 @@ interface CollectorInterface { /** - * @param array> $config + * @param array> $config */ public function satisfy(array $config, TokenReferenceInterface $reference, AstMap $astMap): bool; } diff --git a/src/Layer/Collector/ConditionalCollectorInterface.php b/src/Layer/Collector/ConditionalCollectorInterface.php index 2ac524a7e..458c196b3 100644 --- a/src/Layer/Collector/ConditionalCollectorInterface.php +++ b/src/Layer/Collector/ConditionalCollectorInterface.php @@ -7,7 +7,7 @@ interface ConditionalCollectorInterface extends CollectorInterface { /** - * @param array> $config + * @param array> $config */ public function resolvable(array $config): bool; } diff --git a/src/Layer/Collector/ExtendsCollector.php b/src/Layer/Collector/ExtendsCollector.php index 224b87c39..d561b0cd3 100644 --- a/src/Layer/Collector/ExtendsCollector.php +++ b/src/Layer/Collector/ExtendsCollector.php @@ -30,7 +30,7 @@ public function satisfy(array $config, TokenReferenceInterface $reference, AstMa } /** - * @param array> $config + * @param array> $config */ private function getInterfaceName(array $config): ClassLikeToken { diff --git a/src/Layer/Collector/FunctionNameCollector.php b/src/Layer/Collector/FunctionNameCollector.php index 39c2e3c70..fccd5b4b2 100644 --- a/src/Layer/Collector/FunctionNameCollector.php +++ b/src/Layer/Collector/FunctionNameCollector.php @@ -25,7 +25,7 @@ public function satisfy(array $config, TokenReferenceInterface $reference, AstMa } /** - * @param array> $config + * @param array> $config */ private function getPattern(array $config): string { diff --git a/src/Layer/Collector/ImplementsCollector.php b/src/Layer/Collector/ImplementsCollector.php index 9fc68e7bf..a5928a7e4 100644 --- a/src/Layer/Collector/ImplementsCollector.php +++ b/src/Layer/Collector/ImplementsCollector.php @@ -12,9 +12,6 @@ final class ImplementsCollector implements CollectorInterface { - /** - * {@inheritdoc} - */ public function satisfy(array $config, TokenReferenceInterface $reference, AstMap $astMap): bool { if (!$reference instanceof ClassLikeReference) { @@ -33,7 +30,7 @@ public function satisfy(array $config, TokenReferenceInterface $reference, AstMa } /** - * @param array> $config + * @param array> $config */ private function getInterfaceName(array $config): ClassLikeToken { diff --git a/src/Layer/Collector/InheritsCollector.php b/src/Layer/Collector/InheritsCollector.php index 1178c333c..befe271f4 100644 --- a/src/Layer/Collector/InheritsCollector.php +++ b/src/Layer/Collector/InheritsCollector.php @@ -12,9 +12,6 @@ final class InheritsCollector implements CollectorInterface { - /** - * {@inheritdoc} - */ public function satisfy(array $config, TokenReferenceInterface $reference, AstMap $astMap): bool { if (!$reference instanceof ClassLikeReference) { @@ -33,7 +30,7 @@ public function satisfy(array $config, TokenReferenceInterface $reference, AstMa } /** - * @param array> $config + * @param array> $config */ private function getClassLikeName(array $config): ClassLikeToken { diff --git a/src/Layer/Collector/RegexCollector.php b/src/Layer/Collector/RegexCollector.php index 2b668fb24..f67db6888 100644 --- a/src/Layer/Collector/RegexCollector.php +++ b/src/Layer/Collector/RegexCollector.php @@ -9,12 +9,12 @@ abstract class RegexCollector implements CollectorInterface { /** - * @param array> $config + * @param array> $config */ abstract protected function getPattern(array $config): string; /** - * @param array> $config + * @param array> $config */ protected function getValidatedPattern(array $config): string { diff --git a/src/Layer/Collector/SuperglobalCollector.php b/src/Layer/Collector/SuperglobalCollector.php index 3b9819b58..30c36a74c 100644 --- a/src/Layer/Collector/SuperglobalCollector.php +++ b/src/Layer/Collector/SuperglobalCollector.php @@ -21,7 +21,7 @@ public function satisfy(array $config, TokenReferenceInterface $reference, AstMa } /** - * @param array> $config + * @param array> $config * * @return string[] */ diff --git a/src/Layer/Collector/UsesCollector.php b/src/Layer/Collector/UsesCollector.php index a9f40d0c1..9a65bb2da 100644 --- a/src/Layer/Collector/UsesCollector.php +++ b/src/Layer/Collector/UsesCollector.php @@ -30,7 +30,7 @@ public function satisfy(array $config, TokenReferenceInterface $reference, AstMa } /** - * @param array> $config + * @param array> $config */ private function getTraitName(array $config): ClassLikeToken { diff --git a/src/Layer/LayerResolver.php b/src/Layer/LayerResolver.php index d9c47398e..2e3b6de75 100644 --- a/src/Layer/LayerResolver.php +++ b/src/Layer/LayerResolver.php @@ -11,8 +11,6 @@ use Qossmic\Deptrac\Layer\Collector\ConditionalCollectorInterface; use Qossmic\Deptrac\Layer\Exception\InvalidLayerDefinitionException; use function array_key_exists; -use function array_unique; -use function array_values; use function in_array; class LayerResolver implements LayerResolverInterface @@ -25,7 +23,7 @@ class LayerResolver implements LayerResolverInterface private array $layers; /** - * @var array + * @var array> */ private array $resolved = []; @@ -39,9 +37,6 @@ public function __construct(CollectorResolverInterface $collectorResolver, array $this->initializeLayers($layers); } - /** - * @return string[] - */ public function getLayersForReference(TokenReferenceInterface $reference, AstMap $astMap): array { $tokenName = $reference->getToken()->toString(); @@ -50,22 +45,29 @@ public function getLayersForReference(TokenReferenceInterface $reference, AstMap } foreach ($this->layers as $layer => $collectables) { + $this->resolved[$tokenName] = []; foreach ($collectables as $collectable) { $collector = $collectable->getCollector(); + $attributes = $collectable->getAttributes(); if ($collector instanceof ConditionalCollectorInterface - && !$collector->resolvable($collectable->getAttributes()) + && !$collector->resolvable($attributes) ) { continue; } - if ($collectable->getCollector()->satisfy($collectable->getAttributes(), $reference, $astMap)) { - $this->resolved[$tokenName][] = $layer; + if ($collectable->getCollector()->satisfy($attributes, $reference, $astMap)) { + if (array_key_exists($layer, $this->resolved[$tokenName]) && true === $this->resolved[$tokenName][$layer]) { + continue; + } + if (array_key_exists('private', $attributes) && true === $attributes['private']) { + $this->resolved[$tokenName][$layer] = false; + } else { + $this->resolved[$tokenName][$layer] = true; + } } } } - $this->resolved[$tokenName] = array_values(array_unique($this->resolved[$tokenName] ?? [])); - return $this->resolved[$tokenName]; } diff --git a/src/Layer/LayerResolverInterface.php b/src/Layer/LayerResolverInterface.php index 217256032..1ecd7310b 100644 --- a/src/Layer/LayerResolverInterface.php +++ b/src/Layer/LayerResolverInterface.php @@ -10,7 +10,7 @@ interface LayerResolverInterface { /** - * @return string[] + * @return array layer name and whether the dependency is public(true) or private(false) */ public function getLayersForReference(TokenReferenceInterface $reference, AstMap $astMap): array; diff --git a/tests/Layer/LayerResolverTest.php b/tests/Layer/LayerResolverTest.php index 57616e844..69ea8e999 100644 --- a/tests/Layer/LayerResolverTest.php +++ b/tests/Layer/LayerResolverTest.php @@ -157,7 +157,7 @@ public function testGetLayersForReference(): void ); self::assertSame( - ['test'], + ['test' => true], $resolver->getLayersForReference($reference, new AstMap([])) ); } From c86719f556bef32a1242e03c8c4a129879448387 Mon Sep 17 00:00:00 2001 From: patrickkusebauch Date: Tue, 7 Jun 2022 20:20:19 +0200 Subject: [PATCH 2/5] Propagation to analyser --- src/Analyser/DependencyLayersAnalyser.php | 2 +- src/Analyser/Event/ProcessEvent.php | 6 +++--- src/Analyser/EventHandler/AllowDependencyHandler.php | 6 +++++- src/Analyser/EventHandler/MatchingLayersHandler.php | 2 +- 4 files changed, 10 insertions(+), 6 deletions(-) diff --git a/src/Analyser/DependencyLayersAnalyser.php b/src/Analyser/DependencyLayersAnalyser.php index 56391af99..0d3346f98 100644 --- a/src/Analyser/DependencyLayersAnalyser.php +++ b/src/Analyser/DependencyLayersAnalyser.php @@ -59,7 +59,7 @@ public function process(): Result $dependent = $dependency->getDependent(); $dependentRef = $this->tokenResolver->resolve($dependent, $astMap); - $dependentLayers = array_keys($this->dependentLayerResolver->getLayersForReference($dependentRef, $astMap)); + $dependentLayers = $this->dependentLayerResolver->getLayersForReference($dependentRef, $astMap); foreach ($dependerLayers as $dependentLayer) { $event = new ProcessEvent($dependency, $dependerRef, $dependentLayer, $dependentRef, $dependentLayers, $result); diff --git a/src/Analyser/Event/ProcessEvent.php b/src/Analyser/Event/ProcessEvent.php index 886070056..12b4a462b 100644 --- a/src/Analyser/Event/ProcessEvent.php +++ b/src/Analyser/Event/ProcessEvent.php @@ -20,14 +20,14 @@ class ProcessEvent extends Event private TokenReferenceInterface $dependentReference; /** - * @var string[] + * @var array */ private array $dependentLayers; private Result $result; /** - * @param string[] $dependentLayers + * @param array $dependentLayers */ public function __construct( DependencyInterface $dependency, @@ -66,7 +66,7 @@ public function getDependentReference(): TokenReferenceInterface } /** - * @return string[] + * @return array */ public function getDependentLayers(): array { diff --git a/src/Analyser/EventHandler/AllowDependencyHandler.php b/src/Analyser/EventHandler/AllowDependencyHandler.php index 7261cb512..08ab54e6a 100644 --- a/src/Analyser/EventHandler/AllowDependencyHandler.php +++ b/src/Analyser/EventHandler/AllowDependencyHandler.php @@ -26,7 +26,7 @@ public function __invoke(ProcessEvent $event): void $dependerLayer = $event->getDependerLayer(); $ruleset = $event->getResult(); - foreach ($event->getDependentLayers() as $dependentLayer) { + foreach ($event->getDependentLayers() as $dependentLayer => $isPublic) { try { $allowedLayers = $this->layerProvider->getAllowedLayers($dependerLayer); } catch (CircularReferenceException $circularReferenceException) { @@ -36,6 +36,10 @@ public function __invoke(ProcessEvent $event): void return; } + if (!$isPublic && $dependerLayer !== $dependentLayer) { + return; + } + if (!in_array($dependentLayer, $allowedLayers, true)) { return; } diff --git a/src/Analyser/EventHandler/MatchingLayersHandler.php b/src/Analyser/EventHandler/MatchingLayersHandler.php index 91998dbe7..40dace6c3 100644 --- a/src/Analyser/EventHandler/MatchingLayersHandler.php +++ b/src/Analyser/EventHandler/MatchingLayersHandler.php @@ -13,7 +13,7 @@ public function __invoke(ProcessEvent $event): void $dependerLayer = $event->getDependerLayer(); $dependentLayers = $event->getDependentLayers(); - foreach ($dependentLayers as $dependeeLayer) { + foreach ($dependentLayers as $dependeeLayer => $_) { if ($dependerLayer !== $dependeeLayer) { return; } From 3d305a4ef3b2812fb8636d310c9bfe0cb55abce4 Mon Sep 17 00:00:00 2001 From: patrickkusebauch Date: Tue, 7 Jun 2022 20:23:04 +0200 Subject: [PATCH 3/5] Fix --- src/Analyser/EventHandler/ViolationHandler.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Analyser/EventHandler/ViolationHandler.php b/src/Analyser/EventHandler/ViolationHandler.php index b9a576071..d3c4f5aa0 100644 --- a/src/Analyser/EventHandler/ViolationHandler.php +++ b/src/Analyser/EventHandler/ViolationHandler.php @@ -32,14 +32,14 @@ public function handleViolation(ProcessEvent $event): void $dependentLayers = $event->getDependentLayers(); $ruleset = $event->getResult(); - foreach ($dependentLayers as $dependeeLayer) { + foreach ($dependentLayers as $dependentLayer => $_) { if ($this->skippedViolationHelper->isViolationSkipped($depender->toString(), $dependent->toString())) { - $ruleset->add(new SkippedViolation($dependency, $dependerLayer, $dependeeLayer)); + $ruleset->add(new SkippedViolation($dependency, $dependerLayer, $dependentLayer)); continue; } - $ruleset->add(new Violation($dependency, $dependerLayer, $dependeeLayer)); + $ruleset->add(new Violation($dependency, $dependerLayer, $dependentLayer)); } } From b4c66489b32b7f2ceb8c3b3ddad83fb0c8d76b29 Mon Sep 17 00:00:00 2001 From: patrickkusebauch Date: Wed, 8 Jun 2022 18:44:41 +0200 Subject: [PATCH 4/5] Tests via `make deptrac` --- deptrac.yaml | 3 +++ src/Layer/LayerResolver.php | 3 ++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/deptrac.yaml b/deptrac.yaml index ca7037fe5..bfade17de 100644 --- a/deptrac.yaml +++ b/deptrac.yaml @@ -47,10 +47,13 @@ deptrac: value: ^Qossmic\\Deptrac\\Layer\\LayerResolverInterface$ - type: implements value: Qossmic\Deptrac\Layer\LayerResolverInterface + private: true - type: implements value: Qossmic\Deptrac\Layer\Collector\CollectorResolverInterface + private: true - type: className value: ^Qossmic\\Deptrac\\Layer\\Collector\\Collectable$ + private: true - name: Analyser collectors: - type: className diff --git a/src/Layer/LayerResolver.php b/src/Layer/LayerResolver.php index 2e3b6de75..6995b7bed 100644 --- a/src/Layer/LayerResolver.php +++ b/src/Layer/LayerResolver.php @@ -44,8 +44,9 @@ public function getLayersForReference(TokenReferenceInterface $reference, AstMap return $this->resolved[$tokenName]; } + $this->resolved[$tokenName] = []; + foreach ($this->layers as $layer => $collectables) { - $this->resolved[$tokenName] = []; foreach ($collectables as $collectable) { $collector = $collectable->getCollector(); $attributes = $collectable->getAttributes(); From 25fc5bc8087f58e8d838fc9ec09fc54dad904b61 Mon Sep 17 00:00:00 2001 From: patrickkusebauch Date: Wed, 8 Jun 2022 18:50:56 +0200 Subject: [PATCH 5/5] Docs --- docs/collectors.md | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/docs/collectors.md b/docs/collectors.md index 2117d4021..9ea8a0c8b 100644 --- a/docs/collectors.md +++ b/docs/collectors.md @@ -305,3 +305,23 @@ project. With this you can create collectors specific for your use case. If you would like to make your collector available to others, feel free to [contribute](contributing.md) it by making a pull request. + +## Extra collector configuration + +Any collector can also specify parameter `private:true` like this: + +```yaml +deptrac: + layers: + - name: Foo + collectors: + - type: uses + value: 'App\SomeTrait' + private: true +``` + +This means that tokens collected by this specific collector can be referenced only by other tokens in the same layer. References from other layers will be considered violations, even if they would normally be allowed by configured ruleset. + +This can be useful at least in 2 cases: + - **External library that should be used only by one particular layer** - In this case, you might via vendor include a library that should be used only by this particular layer and nobody else. + - **Layer that has a public API and private implementation** - You might want to provide only a few classes to be available to use by other layers (public API) that call the internal implementation of the layer that on the other hand should not be available to anybody else other than the public API of the layer.