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

report unmatched skipped violations #403

Merged
merged 1 commit into from
Oct 27, 2020
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
1 change: 1 addition & 0 deletions examples/SkipViolations.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -14,4 +14,5 @@ ruleset:
skip_violations:
Library\LibClass:
- Core\CoreClass
- Core\Unmatched

10 changes: 5 additions & 5 deletions src/Configuration/ConfigurationSkippedViolation.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@

namespace SensioLabs\Deptrac\Configuration;

use SensioLabs\Deptrac\AstRunner\AstMap\ClassLikeName;

/**
* @author Dmitry Balabka <dmitry.balabka@gmail.com>
*/
Expand All @@ -30,9 +28,11 @@ private function __construct(array $classesDeps)
$this->classesDeps = $classesDeps;
}

public function isViolationSkipped(ClassLikeName $classLikeNameA, ClassLikeName $classLikeNameB): bool
/**
* @return array<string, string[]>
*/
public function all(): array
{
return isset($this->classesDeps[$classLikeNameA->toString()])
&& \in_array($classLikeNameB->toString(), $this->classesDeps[$classLikeNameA->toString()], true);
return $this->classesDeps;
}
}
2 changes: 1 addition & 1 deletion src/Console/Command/AnalyzeCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
return 1;
}

return $context->hasViolations() ? 1 : 0;
return $context->hasViolations() || $context->hasErrors() ? 1 : 0;
}

protected function printCollectViolations(SymfonyOutput $output): void
Expand Down
14 changes: 13 additions & 1 deletion src/OutputFormatter/ConsoleOutputFormatter.php
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ public function finish(
$output->writeLineFormatted('');
}

foreach ($context->all() as $rule) {
foreach ($context->rules() as $rule) {
if (!$rule instanceof Violation && !$rule instanceof SkippedViolation) {
continue;
}
Expand All @@ -70,6 +70,10 @@ public function finish(
$this->printUncovered($context, $output);
}

if ($context->hasErrors()) {
$this->printErrors($context, $output);
}

$this->printSummary($context, $output);
}

Expand Down Expand Up @@ -178,4 +182,12 @@ private function printFileOccurrence(Output $output, FileOccurrence $fileOccurre
{
$output->writeLineFormatted($fileOccurrence->getFilepath().'::'.$fileOccurrence->getLine());
}

private function printErrors(Context $context, Output $output): void
{
$output->writeLineFormatted('');
foreach ($context->errors() as $error) {
$output->writeLineFormatted(sprintf('<fg=red>[ERROR]</> %s', $error->toString()));
}
}
}
13 changes: 12 additions & 1 deletion src/OutputFormatter/GithubActionsOutputFormatter.php
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ public function finish(Context $context, Output $output, OutputFormatterInput $o
$output->writeLineFormatted('');
}

foreach ($context->all() as $rule) {
foreach ($context->rules() as $rule) {
if (!$rule instanceof Violation && !$rule instanceof SkippedViolation) {
continue;
}
Expand Down Expand Up @@ -92,6 +92,10 @@ public function finish(Context $context, Output $output, OutputFormatterInput $o
|| $outputFormatterInput->getOptionAsBoolean(AnalyzeCommand::OPTION_REPORT_UNCOVERED)) {
$this->printUncovered($context, $output);
}

if ($context->hasErrors()) {
$this->printErrors($context, $output);
}
}

private function determineLogLevel(Rule $rule): string
Expand Down Expand Up @@ -145,4 +149,11 @@ private function inheritPathMessage(InheritDependency $dependency): string

return implode(' ->%0A', $buffer);
}

private function printErrors(Context $context, Output $output): void
{
foreach ($context->errors() as $error) {
$output->writeLineFormatted('::error ::'.$error->toString());
}
}
}
2 changes: 1 addition & 1 deletion src/OutputFormatter/GraphVizOutputFormatter.php
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ public function finish(
$this->reportDeprecation(!empty($legacyDumpHtml), self::LEGACY_DUMP_HTML, self::DUMP_HTML, $output);

$layerViolations = $this->calculateViolations($context->violations());
$layersDependOnLayers = $this->calculateLayerDependencies($context->all());
$layersDependOnLayers = $this->calculateLayerDependencies($context->rules());

$graph = new Graph();

Expand Down
2 changes: 1 addition & 1 deletion src/OutputFormatter/JUnitOutputFormatter.php
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ private function addTestSuites(Context $context, \DOMDocument $xmlDoc): void
private function addTestSuite(Context $context, \DOMDocument $xmlDoc, \DOMElement $testSuites): void
{
$layers = [];
foreach ($context->all() as $rule) {
foreach ($context->rules() as $rule) {
if ($rule instanceof Allowed || $rule instanceof Violation || $rule instanceof SkippedViolation) {
$layers[$rule->getLayerA()][] = $rule;
} elseif ($rule instanceof Uncovered) {
Expand Down
20 changes: 19 additions & 1 deletion src/OutputFormatter/TableOutputFormatter.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
use SensioLabs\Deptrac\Dependency\InheritDependency;
use SensioLabs\Deptrac\RulesetEngine\Allowed;
use SensioLabs\Deptrac\RulesetEngine\Context;
use SensioLabs\Deptrac\RulesetEngine\Error;
use SensioLabs\Deptrac\RulesetEngine\Rule;
use SensioLabs\Deptrac\RulesetEngine\SkippedViolation;
use SensioLabs\Deptrac\RulesetEngine\Uncovered;
Expand Down Expand Up @@ -40,7 +41,7 @@ public function finish(
$groupedRules = [];
$reportUncovered = $outputFormatterInput->getOptionAsBoolean(AnalyzeCommand::OPTION_REPORT_UNCOVERED);

foreach ($context->all() as $rule) {
foreach ($context->rules() as $rule) {
if ($rule instanceof Allowed) {
continue;
}
Expand All @@ -67,6 +68,10 @@ public function finish(
$style->table(['Reason', $layer], $rows);
}

if ($context->hasErrors()) {
$this->printErrors($context, $output);
}

$this->printSummary($context, $output);
}

Expand Down Expand Up @@ -156,4 +161,17 @@ private function uncoveredRow(Uncovered $rule): array
$message,
];
}

private function printErrors(Context $context, Output $output): void
{
$output->getStyle()->table(
['<fg=red>Errors</>'],
array_map(
static function (Error $error) {
return [$error->toString()];
},
$context->errors()
)
);
}
}
15 changes: 12 additions & 3 deletions src/RulesetEngine.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@
use SensioLabs\Deptrac\Dependency\Result;
use SensioLabs\Deptrac\RulesetEngine\Allowed;
use SensioLabs\Deptrac\RulesetEngine\Context;
use SensioLabs\Deptrac\RulesetEngine\Error;
use SensioLabs\Deptrac\RulesetEngine\SkippedViolation;
use SensioLabs\Deptrac\RulesetEngine\SkippedViolationHelper;
use SensioLabs\Deptrac\RulesetEngine\Uncovered;
use SensioLabs\Deptrac\RulesetEngine\Violation;

Expand All @@ -24,7 +26,7 @@ public function process(
$rules = [];

$configurationRuleset = $configuration->getRuleset();
$configurationSkippedViolation = $configuration->getSkipViolations();
$skippedViolationHelper = new SkippedViolationHelper($configuration->getSkipViolations());

foreach ($dependencyResult->getDependenciesAndInheritDependencies() as $dependency) {
$layerNames = $classNameLayerResolver->getLayersByClassName($dependency->getClassLikeNameA());
Expand All @@ -51,7 +53,7 @@ public function process(
continue;
}

if ($configurationSkippedViolation->isViolationSkipped($dependency->getClassLikeNameA(), $dependency->getClassLikeNameB())) {
if ($skippedViolationHelper->isViolationSkipped($dependency->getClassLikeNameA(), $dependency->getClassLikeNameB())) {
$rules[] = new SkippedViolation($dependency, $layerName, $layerNameOfDependency);
continue;
}
Expand All @@ -61,7 +63,14 @@ public function process(
}
}

return new Context($rules);
$errors = [];
foreach ($skippedViolationHelper->unmatchedSkippedViolations() as $classLikeNameA => $classLikes) {
foreach ($classLikes as $classLikeNameB) {
$errors[] = new Error(sprintf('Skipped violation "%s" for "%s" was not matched.', $classLikeNameB, $classLikeNameA));
}
}

return new Context($rules, $errors);
}

private function ignoreUncoveredInternalClass(Configuration $configuration, ClassLikeName $classLikeName): bool
Expand Down
25 changes: 22 additions & 3 deletions src/RulesetEngine/Context.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,19 +10,25 @@ final class Context
* @var Rule[]
*/
private $rules;
/**
* @var Error[]
*/
private $errors;

/**
* @param Rule[] $rules
* @param Rule[] $rules
* @param Error[] $errors
*/
public function __construct(array $rules)
public function __construct(array $rules, array $errors)
{
$this->rules = $rules;
$this->errors = $errors;
}

/**
* @return Rule[]
*/
public function all(): array
public function rules(): array
{
return $this->rules;
}
Expand Down Expand Up @@ -76,4 +82,17 @@ public function allowed(): array
return $rule instanceof Allowed;
});
}

public function hasErrors(): bool
{
return \count($this->errors) > 0;
}

/**
* @return Error[]
*/
public function errors(): array
{
return $this->errors;
}
}
23 changes: 23 additions & 0 deletions src/RulesetEngine/Error.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<?php

declare(strict_types=1);

namespace SensioLabs\Deptrac\RulesetEngine;

class Error
{
/**
* @var string
*/
private $message;

public function __construct(string $message)
{
$this->message = $message;
}

public function toString(): string
{
return $this->message;
}
}
46 changes: 46 additions & 0 deletions src/RulesetEngine/SkippedViolationHelper.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
<?php

declare(strict_types=1);

namespace SensioLabs\Deptrac\RulesetEngine;

use SensioLabs\Deptrac\AstRunner\AstMap\ClassLikeName;
use SensioLabs\Deptrac\Configuration\ConfigurationSkippedViolation;

final class SkippedViolationHelper
{
/**
* @var array<string, string[]>
*/
private $skippedViolation;

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

public function __construct(ConfigurationSkippedViolation $configuration)
{
$this->skippedViolation = $configuration->all();
$this->unmatchedSkippedViolation = $configuration->all();
}

public function isViolationSkipped(ClassLikeName $classLikeNameA, ClassLikeName $classLikeNameB): bool
{
$a = $classLikeNameA->toString();
$b = $classLikeNameB->toString();

$matched = isset($this->skippedViolation[$a]) && \in_array($b, $this->skippedViolation[$a], true);

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

return $matched;
}

public function unmatchedSkippedViolations(): array
{
return array_filter($this->unmatchedSkippedViolation);
}
}
43 changes: 0 additions & 43 deletions tests/Configuration/ConfigurationSkippedViolationTest.php

This file was deleted.

9 changes: 6 additions & 3 deletions tests/Configuration/ConfigurationTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
namespace Tests\SensioLabs\Deptrac\Configuration;

use PHPUnit\Framework\TestCase;
use SensioLabs\Deptrac\AstRunner\AstMap\ClassLikeName;
use SensioLabs\Deptrac\Configuration\Configuration;
use SensioLabs\Deptrac\Configuration\Exception;

Expand Down Expand Up @@ -184,8 +183,12 @@ public function testSkipViolations(): void
],
]);

self::assertTrue($configuration->getSkipViolations()->isViolationSkipped(ClassLikeName::fromFQCN('FooClass'), ClassLikeName::fromFQCN('BarClass')));
self::assertTrue($configuration->getSkipViolations()->isViolationSkipped(ClassLikeName::fromFQCN('FooClass'), ClassLikeName::fromFQCN('AnotherClass')));
self::assertSame([
'FooClass' => [
'BarClass',
'AnotherClass',
],
], $configuration->getSkipViolations()->all());
}

public function testIgnoreUncoveredInternalClassesSetToFalse(): void
Expand Down
Loading