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

Better Violations #1105

Merged
merged 6 commits into from
Mar 10, 2023
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
15 changes: 15 additions & 0 deletions baseline.xml
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,19 @@
<code>list&lt;RuleInterface&gt;</code>
</MixedReturnTypeCoercion>
</file>
<file src="src/Core/Analyser/EventHandler/DependsOnDisallowedLayer.php">
<MissingImmutableAnnotation occurrences="1">
<code>DependsOnDisallowedLayer</code>
</MissingImmutableAnnotation>
</file>
<file src="src/Core/Analyser/EventHandler/DependsOnInternalToken.php">
<MissingImmutableAnnotation occurrences="1">
<code>DependsOnInternalToken</code>
</MissingImmutableAnnotation>
</file>
<file src="src/Core/Analyser/EventHandler/DependsOnPrivateLayer.php">
<MissingImmutableAnnotation occurrences="1">
<code>DependsOnPrivateLayer</code>
</MissingImmutableAnnotation>
</file>
</files>
24 changes: 19 additions & 5 deletions config/services.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,16 @@
use PhpParser\Parser;
use PhpParser\ParserFactory;
use Psr\EventDispatcher\EventDispatcherInterface;
use Qossmic\Deptrac\Contract\Analyser\EventHelper;
use Qossmic\Deptrac\Contract\Layer\LayerProvider;
use Qossmic\Deptrac\Core\Analyser\DependencyLayersAnalyser;
use Qossmic\Deptrac\Core\Analyser\EventHandler\AllowDependencyHandler;
use Qossmic\Deptrac\Core\Analyser\EventHandler\DependsOnDisallowedLayer;
use Qossmic\Deptrac\Core\Analyser\EventHandler\DependsOnInternalToken;
use Qossmic\Deptrac\Core\Analyser\EventHandler\DependsOnPrivateLayer;
use Qossmic\Deptrac\Core\Analyser\EventHandler\MatchingLayersHandler;
use Qossmic\Deptrac\Core\Analyser\EventHandler\UncoveredDependentHandler;
use Qossmic\Deptrac\Core\Analyser\EventHandler\ViolationHandler;
use Qossmic\Deptrac\Core\Analyser\EventHandler\UnmatchedSkippedViolations;
use Qossmic\Deptrac\Core\Analyser\LayerForTokenAnalyser;
use Qossmic\Deptrac\Core\Analyser\TokenInLayerAnalyser;
use Qossmic\Deptrac\Core\Analyser\UnassignedTokenAnalyser;
Expand Down Expand Up @@ -65,7 +70,6 @@
use Qossmic\Deptrac\Core\Layer\Collector\SuperglobalCollector;
use Qossmic\Deptrac\Core\Layer\Collector\TraitCollector;
use Qossmic\Deptrac\Core\Layer\Collector\UsesCollector;
use Qossmic\Deptrac\Core\Layer\LayerProvider;
use Qossmic\Deptrac\Core\Layer\LayerResolver;
use Qossmic\Deptrac\Core\Layer\LayerResolverInterface;
use Qossmic\Deptrac\Supportive\Console\Command\AnalyseCommand;
Expand Down Expand Up @@ -316,11 +320,21 @@
->set(AllowDependencyHandler::class)
->tag('kernel.event_subscriber');
$services
->set(ViolationHandler::class)
->set(DependsOnDisallowedLayer::class)
->tag('kernel.event_subscriber');
$services
->set(DependsOnPrivateLayer::class)
->tag('kernel.event_subscriber');
$services
->set(DependsOnInternalToken::class)
->tag('kernel.event_subscriber');
$services
->set(UnmatchedSkippedViolations::class)
->tag('kernel.event_subscriber');
$services->set(EventHelper::class)
->args([
'$skippedViolations' => param('skip_violations'),
])
->tag('kernel.event_subscriber');
]);
$services
->set(DependencyLayersAnalyser::class);
$services->set(TokenInLayerAnalyser::class)
Expand Down
19 changes: 10 additions & 9 deletions docs/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,14 @@ You can analyse projects that require an older PHP version as long as

You can install Deptrac via Composer. We recommend using the [deptrac-shim](https://github.com/qossmic/deptrac-shim) package for this.
Alternatively, you can also use [PHIVE](#phive) or download the
[PHAR](#phar) attached to each release on GitHub.
[PHAR](#phar) attached to each release on GitHub.
This will ensure that Deptrac and its dependencies are
bundled together and will not interfere with any of your project's dependencies.

### Composer

We strongly advise against using the deptrac package directly as a composer dependency.
We update dependencies regularly, which might cause disruptions in your project.
We strongly advise against using the deptrac package directly as a composer dependency.
We update dependencies regularly, which might cause disruptions in your project.
Instead, please use the dedicated distribution repository <https://github.com/qossmic/deptrac-shim>.

When you install Deptrac using the qossmic/deptrac-shim package, you will get
Expand Down Expand Up @@ -200,12 +200,13 @@ This can be disabled with the `--no-cache` option.
The generated output will roughly look like this:

```bash
----------- --------------------------------------------------------------------------------------------------------------------------------
Reason Repository
----------- --------------------------------------------------------------------------------------------------------------------------------
Violation examples\MyNamespace\Repository\SomeRepository must not depend on examples\MyNamespace\Controllers\SomeController (Controller)
/Users/dbr/workspace/qossmic/deptrac/examples/ControllerServiceRepository1/SomeRepository.php:5
----------- --------------------------------------------------------------------------------------------------------------------------------
-------------------------- --------------------------------------------------------------------------------------------------------------------------------
Reason Repository
-------------------------- --------------------------------------------------------------------------------------------------------------------------------
DependsOnDisallowedLayer examples\MyNamespace\Repository\SomeRepository must not depend on examples\MyNamespace\Controllers\SomeController
You are depending on token that is a part of a layer that you are not allowed to depend on. (Controller)
/Users/dbr/workspace/qossmic/deptrac/examples/ControllerServiceRepository1/SomeRepository.php:5
-------------------------- --------------------------------------------------------------------------------------------------------------------------------


-------------------- -----
Expand Down
61 changes: 61 additions & 0 deletions src/Contract/Analyser/EventHelper.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
<?php

declare(strict_types=1);

namespace Qossmic\Deptrac\Contract\Analyser;

use Qossmic\Deptrac\Contract\Layer\LayerProvider;
use Qossmic\Deptrac\Contract\Result\SkippedViolation;
use Qossmic\Deptrac\Contract\Result\Violation;

class EventHelper
{
/**
* @var array<string, string[]>
*/
private array $unmatchedSkippedViolation;

/**
* @param array<string, string[]> $skippedViolations
*/
public function __construct(
private readonly array $skippedViolations,
public readonly LayerProvider $layerProvider,
) {
$this->unmatchedSkippedViolation = $skippedViolations;
}

public function isViolationSkipped(string $depender, string $dependent): bool
{
$matched = isset($this->skippedViolations[$depender]) && in_array($dependent, $this->skippedViolations[$depender], true);

if ($matched && false !== ($key = array_search($dependent, $this->unmatchedSkippedViolation[$depender], true))) {
unset($this->unmatchedSkippedViolation[$depender][$key]);
}

return $matched;
}

/**
* @return array<string, string[]>
*/
public function unmatchedSkippedViolations(): array
{
return array_filter($this->unmatchedSkippedViolation);
}

public function addSkippableViolation(ProcessEvent $event, AnalysisResult $result, string $dependentLayer, ViolationCreatingInterface $violationCreatingRule): void
{
if ($this->isViolationSkipped(
$event->dependency->getDepender()
->toString(),
$event->dependency->getDependent()
->toString()
)
) {
$result->addRule(new SkippedViolation($event->dependency, $event->dependerLayer, $dependentLayer));
} else {
$result->addRule(new Violation($event->dependency, $event->dependerLayer, $dependentLayer, $violationCreatingRule));
}
}
}
20 changes: 20 additions & 0 deletions src/Contract/Analyser/ViolationCreatingInterface.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
<?php

declare(strict_types=1);

namespace Qossmic\Deptrac\Contract\Analyser;

use Symfony\Component\EventDispatcher\EventSubscriberInterface;

interface ViolationCreatingInterface extends EventSubscriberInterface
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great idea! I really like this interface and the naming around it 👍

{
/**
* @psalm-pure
*/
public function ruleName(): string;

/**
* @psalm-pure
*/
public function ruleDescription(): string;
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

declare(strict_types=1);

namespace Qossmic\Deptrac\Core\Layer\Exception;
namespace Qossmic\Deptrac\Contract\Layer;

use Qossmic\Deptrac\Contract\ExceptionInterface;
use RuntimeException;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,7 @@

declare(strict_types=1);

namespace Qossmic\Deptrac\Core\Layer;

use Qossmic\Deptrac\Core\Layer\Exception\CircularReferenceException;
namespace Qossmic\Deptrac\Contract\Layer;

class LayerProvider
{
Expand Down
14 changes: 13 additions & 1 deletion src/Contract/Result/Violation.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

namespace Qossmic\Deptrac\Contract\Result;

use Qossmic\Deptrac\Contract\Analyser\ViolationCreatingInterface;
use Qossmic\Deptrac\Contract\Dependency\DependencyInterface;

/**
Expand All @@ -14,7 +15,8 @@ final class Violation implements CoveredRuleInterface
public function __construct(
private readonly DependencyInterface $dependency,
private readonly string $dependerLayer,
private readonly string $dependentLayer
private readonly string $dependentLayer,
private readonly ViolationCreatingInterface $violationCreatingRule
) {
}

Expand All @@ -32,4 +34,14 @@ public function getDependentLayer(): string
{
return $this->dependentLayer;
}

public function ruleName(): string
{
return $this->violationCreatingRule->ruleName();
}

public function ruleDescription(): string
{
return $this->violationCreatingRule->ruleDescription();
}
}
4 changes: 2 additions & 2 deletions src/Core/Analyser/DependencyLayersAnalyser.php
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,9 @@ public function analyse(): AnalysisResult
$dependentRef = $this->tokenResolver->resolve($dependent, $astMap);
$dependentLayers = $this->layerResolver->getLayersForReference($dependentRef);

foreach ($dependerLayers as $dependentLayer) {
foreach ($dependerLayers as $dependerLayer) {
$event = new ProcessEvent(
$dependency, $dependerRef, $dependentLayer, $dependentRef, $dependentLayers, $result
$dependency, $dependerRef, $dependerLayer, $dependentRef, $dependentLayers, $result
);
$this->eventDispatcher->dispatch($event);

Expand Down
39 changes: 2 additions & 37 deletions src/Core/Analyser/EventHandler/AllowDependencyHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,61 +6,26 @@

use Qossmic\Deptrac\Contract\Analyser\ProcessEvent;
use Qossmic\Deptrac\Contract\Result\Allowed;
use Qossmic\Deptrac\Contract\Result\Error;
use Qossmic\Deptrac\Core\Ast\AstMap\ClassLike\ClassLikeReference;
use Qossmic\Deptrac\Core\Layer\Exception\CircularReferenceException;
use Qossmic\Deptrac\Core\Layer\LayerProvider;
use Symfony\Component\EventDispatcher\EventSubscriberInterface;

use function in_array;

/**
* @internal
*/
class AllowDependencyHandler implements EventSubscriberInterface
{
public function __construct(private readonly LayerProvider $layerProvider)
{
}

public static function getSubscribedEvents()
{
return [
ProcessEvent::class => ['invoke', 4],
ProcessEvent::class => ['invoke', -100],
];
}

public function invoke(ProcessEvent $event): void
{
$ruleset = $event->getResult();

foreach ($event->dependentLayers as $dependentLayer => $isPublic) {
try {
$allowedLayers = $this->layerProvider->getAllowedLayers($event->dependerLayer);
} catch (CircularReferenceException $circularReferenceException) {
$ruleset->addError(new Error($circularReferenceException->getMessage()));
$event->stopPropagation();

return;
}

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

if (!in_array($dependentLayer, $allowedLayers, true)) {
return;
}

if ($event->dependerLayer !== $dependentLayer
&& $event->dependentReference instanceof ClassLikeReference
&& $event->dependentReference->isInternal
) {
return;
}

foreach ($event->dependentLayers as $dependentLayer => $_) {
$ruleset->addRule(new Allowed($event->dependency, $event->dependerLayer, $dependentLayer));

$event->stopPropagation();
}
}
Expand Down
61 changes: 61 additions & 0 deletions src/Core/Analyser/EventHandler/DependsOnDisallowedLayer.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
<?php

declare(strict_types=1);

namespace Qossmic\Deptrac\Core\Analyser\EventHandler;

use Qossmic\Deptrac\Contract\Analyser\EventHelper;
use Qossmic\Deptrac\Contract\Analyser\ProcessEvent;
use Qossmic\Deptrac\Contract\Analyser\ViolationCreatingInterface;
use Qossmic\Deptrac\Contract\Layer\CircularReferenceException;
use Qossmic\Deptrac\Contract\Result\Error;

use function in_array;

/**
* @internal
*/
class DependsOnDisallowedLayer implements ViolationCreatingInterface
{
public function __construct(private readonly EventHelper $eventHelper)
{
}

public static function getSubscribedEvents()
{
return [
ProcessEvent::class => ['invoke', -1],
];
}

public function invoke(ProcessEvent $event): void
{
$ruleset = $event->getResult();

try {
$allowedLayers = $this->eventHelper->layerProvider->getAllowedLayers($event->dependerLayer);
} catch (CircularReferenceException $circularReferenceException) {
$ruleset->addError(new Error($circularReferenceException->getMessage()));
$event->stopPropagation();

return;
}

foreach ($event->dependentLayers as $dependentLayer => $_) {
if (!in_array($dependentLayer, $allowedLayers, true)) {
$this->eventHelper->addSkippableViolation($event, $ruleset, $dependentLayer, $this);
$event->stopPropagation();
}
}
}

public function ruleName(): string
{
return 'DependsOnDisallowedLayer';
}

public function ruleDescription(): string
{
return 'You are depending on token that is a part of a layer that you are not allowed to depend on.';
}
}
Loading