Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Private collectors #905

Merged
merged 5 commits into from
Jun 10, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions deptrac.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
20 changes: 20 additions & 0 deletions docs/collectors.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
2 changes: 1 addition & 1 deletion src/Analyser/DependencyLayersAnalyser.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
6 changes: 3 additions & 3 deletions src/Analyser/Event/ProcessEvent.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,14 @@ class ProcessEvent extends Event
private TokenReferenceInterface $dependentReference;

/**
* @var string[]
* @var array<string, bool>
*/
private array $dependentLayers;

private Result $result;

/**
* @param string[] $dependentLayers
* @param array<string, bool> $dependentLayers
*/
public function __construct(
DependencyInterface $dependency,
Expand Down Expand Up @@ -66,7 +66,7 @@ public function getDependentReference(): TokenReferenceInterface
}

/**
* @return string[]
* @return array<string, bool>
*/
public function getDependentLayers(): array
{
Expand Down
6 changes: 5 additions & 1 deletion src/Analyser/EventHandler/AllowDependencyHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -36,6 +36,10 @@ public function __invoke(ProcessEvent $event): void
return;
}

if (!$isPublic && $dependerLayer !== $dependentLayer) {
return;
}

if (!in_array($dependentLayer, $allowedLayers, true)) {
return;
}
Expand Down
2 changes: 1 addition & 1 deletion src/Analyser/EventHandler/MatchingLayersHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
6 changes: 3 additions & 3 deletions src/Analyser/EventHandler/ViolationHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/Analyser/LayerForTokenAnalyser.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
6 changes: 3 additions & 3 deletions src/Analyser/TokenInLayerAnalyser.php
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand All @@ -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();
}
Expand All @@ -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();
}
Expand Down
2 changes: 1 addition & 1 deletion src/Layer/Collector/AttributeCollector.php
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public function satisfy(array $config, TokenReferenceInterface $reference, AstMa
}

/**
* @param array<string, string|array<string, string>> $config
* @param array<string, bool|string|array<string, string>> $config
*/
private function getSearchedSubstring(array $config): string
{
Expand Down
4 changes: 2 additions & 2 deletions src/Layer/Collector/BoolCollector.php
Original file line number Diff line number Diff line change
Expand Up @@ -76,9 +76,9 @@ public function resolvable(array $config): bool
}

/**
* @param array<string, string|array<string, string>> $configuration
* @param array<string, bool|string|array<string, string>> $configuration
*
* @return array<string, string|array<string, string>>
* @return array<string, bool|string|array<string, string>>
*/
private function normalizeConfiguration(array $configuration): array
{
Expand Down
2 changes: 1 addition & 1 deletion src/Layer/Collector/Collectable.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ public function getCollector(): CollectorInterface
}

/**
* @return array<string, string|array<string, string>>
* @return array<string, bool|string|array<string, string>>
*/
public function getAttributes(): array
{
Expand Down
2 changes: 1 addition & 1 deletion src/Layer/Collector/CollectorInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
interface CollectorInterface
{
/**
* @param array<string, string|array<string, string>> $config
* @param array<string, bool|string|array<string, string>> $config
*/
public function satisfy(array $config, TokenReferenceInterface $reference, AstMap $astMap): bool;
}
2 changes: 1 addition & 1 deletion src/Layer/Collector/ConditionalCollectorInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
interface ConditionalCollectorInterface extends CollectorInterface
{
/**
* @param array<string, string|array<string, string>> $config
* @param array<string, bool|string|array<string, string>> $config
*/
public function resolvable(array $config): bool;
}
2 changes: 1 addition & 1 deletion src/Layer/Collector/ExtendsCollector.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ public function satisfy(array $config, TokenReferenceInterface $reference, AstMa
}

/**
* @param array<string, string|array<string, string>> $config
* @param array<string, bool|string|array<string, string>> $config
*/
private function getInterfaceName(array $config): ClassLikeToken
{
Expand Down
2 changes: 1 addition & 1 deletion src/Layer/Collector/FunctionNameCollector.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ public function satisfy(array $config, TokenReferenceInterface $reference, AstMa
}

/**
* @param array<string, string|array<string, string>> $config
* @param array<string, bool|string|array<string, string>> $config
*/
private function getPattern(array $config): string
{
Expand Down
5 changes: 1 addition & 4 deletions src/Layer/Collector/ImplementsCollector.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,6 @@

final class ImplementsCollector implements CollectorInterface
{
/**
* {@inheritdoc}
*/
public function satisfy(array $config, TokenReferenceInterface $reference, AstMap $astMap): bool
{
if (!$reference instanceof ClassLikeReference) {
Expand All @@ -33,7 +30,7 @@ public function satisfy(array $config, TokenReferenceInterface $reference, AstMa
}

/**
* @param array<string, string|array<string, string>> $config
* @param array<string, bool|string|array<string, string>> $config
*/
private function getInterfaceName(array $config): ClassLikeToken
{
Expand Down
5 changes: 1 addition & 4 deletions src/Layer/Collector/InheritsCollector.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,6 @@

final class InheritsCollector implements CollectorInterface
{
/**
* {@inheritdoc}
*/
public function satisfy(array $config, TokenReferenceInterface $reference, AstMap $astMap): bool
{
if (!$reference instanceof ClassLikeReference) {
Expand All @@ -33,7 +30,7 @@ public function satisfy(array $config, TokenReferenceInterface $reference, AstMa
}

/**
* @param array<string, string|array<string, string>> $config
* @param array<string, bool|string|array<string, string>> $config
*/
private function getClassLikeName(array $config): ClassLikeToken
{
Expand Down
4 changes: 2 additions & 2 deletions src/Layer/Collector/RegexCollector.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,12 @@
abstract class RegexCollector implements CollectorInterface
{
/**
* @param array<string, string|array<string, string>> $config
* @param array<string, bool|string|array<string, string>> $config
*/
abstract protected function getPattern(array $config): string;

/**
* @param array<string, string|array<string, string>> $config
* @param array<string, bool|string|array<string, string>> $config
*/
protected function getValidatedPattern(array $config): string
{
Expand Down
2 changes: 1 addition & 1 deletion src/Layer/Collector/SuperglobalCollector.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ public function satisfy(array $config, TokenReferenceInterface $reference, AstMa
}

/**
* @param array<string, string|array<string, string>> $config
* @param array<string, bool|string|array<string, string>> $config
*
* @return string[]
*/
Expand Down
2 changes: 1 addition & 1 deletion src/Layer/Collector/UsesCollector.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ public function satisfy(array $config, TokenReferenceInterface $reference, AstMa
}

/**
* @param array<string, string|array<string, string>> $config
* @param array<string, bool|string|array<string, string>> $config
*/
private function getTraitName(array $config): ClassLikeToken
{
Expand Down
25 changes: 14 additions & 11 deletions src/Layer/LayerResolver.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -25,7 +23,7 @@ class LayerResolver implements LayerResolverInterface
private array $layers;

/**
* @var array<string, string[]>
* @var array<string, array<string, bool>>
*/
private array $resolved = [];

Expand All @@ -39,33 +37,38 @@ public function __construct(CollectorResolverInterface $collectorResolver, array
$this->initializeLayers($layers);
}

/**
* @return string[]
*/
public function getLayersForReference(TokenReferenceInterface $reference, AstMap $astMap): array
{
$tokenName = $reference->getToken()->toString();
if (array_key_exists($tokenName, $this->resolved)) {
return $this->resolved[$tokenName];
}

$this->resolved[$tokenName] = [];

foreach ($this->layers as $layer => $collectables) {
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];
}

Expand Down
2 changes: 1 addition & 1 deletion src/Layer/LayerResolverInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
interface LayerResolverInterface
{
/**
* @return string[]
* @return array<string, bool> layer name and whether the dependency is public(true) or private(false)
*/
public function getLayersForReference(TokenReferenceInterface $reference, AstMap $astMap): array;

Expand Down
2 changes: 1 addition & 1 deletion tests/Layer/LayerResolverTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ public function testGetLayersForReference(): void
);

self::assertSame(
['test'],
['test' => true],
$resolver->getLayersForReference($reference, new AstMap([]))
);
}
Expand Down