From 6e59919810aa8fd66aa29d26c04e98d69aee6c4d Mon Sep 17 00:00:00 2001 From: smoench Date: Fri, 20 Sep 2019 11:40:45 +0200 Subject: [PATCH] POC: track uncovered dependencies --- src/Analyser.php | 21 +--- src/Console/Command/AnalyzeCommand.php | 6 +- src/DependencyContext.php | 111 ----------------- .../ConsoleOutputFormatter.php | 86 +++++++------- .../GraphVizOutputFormatter.php | 83 ++++++------- src/OutputFormatter/JUnitOutputFormatter.php | 72 ++++++----- .../OutputFormatterInterface.php | 4 +- src/RulesetEngine.php | 64 +++++----- .../{RulesetViolation.php => Allowed.php} | 2 +- src/RulesetEngine/Context.php | 54 +++++++++ src/RulesetEngine/Rule.php | 12 ++ src/RulesetEngine/SkippedViolation.php | 36 ++++++ src/RulesetEngine/Uncovered.php | 29 +++++ src/RulesetEngine/Violation.php | 36 ++++++ tests/DependencyContextTest.php | 99 ---------------- .../ConsoleOutputFormatterTest.php | 30 ++--- .../GraphVizOutputFormatterTest.php | 34 ++++++ .../JUnitOutputFormatterTest.php | 50 ++------ .../data/graphviz-expected.dot | 4 + tests/RuleEngine/RulesetViolationTest.php | 25 ---- tests/RulesetEngineTest.php | 112 +++++++++--------- 21 files changed, 443 insertions(+), 527 deletions(-) delete mode 100644 src/DependencyContext.php rename src/RulesetEngine/{RulesetViolation.php => Allowed.php} (95%) create mode 100644 src/RulesetEngine/Context.php create mode 100644 src/RulesetEngine/Rule.php create mode 100644 src/RulesetEngine/SkippedViolation.php create mode 100644 src/RulesetEngine/Uncovered.php create mode 100644 src/RulesetEngine/Violation.php delete mode 100644 tests/DependencyContextTest.php create mode 100644 tests/OutputFormatter/data/graphviz-expected.dot delete mode 100644 tests/RuleEngine/RulesetViolationTest.php diff --git a/src/Analyser.php b/src/Analyser.php index 02a0b93a8..776f83292 100644 --- a/src/Analyser.php +++ b/src/Analyser.php @@ -6,6 +6,7 @@ use SensioLabs\Deptrac\Collector\Registry; use SensioLabs\Deptrac\Configuration\Configuration; use SensioLabs\Deptrac\Dependency\Resolver; +use SensioLabs\Deptrac\RulesetEngine\Context; class Analyser { @@ -29,7 +30,7 @@ public function __construct( $this->rulesetEngine = $rulesetEngine; } - public function analyse(Configuration $configuration): DependencyContext + public function analyse(Configuration $configuration): Context { $astMap = $this->astRunner->createAstMapByFiles($this->fileResolver->resolve($configuration)); $dependencyResult = $this->resolver->resolve($astMap); @@ -38,24 +39,10 @@ public function analyse(Configuration $configuration): DependencyContext new ClassNameLayerResolver($configuration, $astMap, $this->collectorRegistry) ); - /** @var RulesetEngine\RulesetViolation[] $violations */ - $violations = $this->rulesetEngine->getViolations( + return $this->rulesetEngine->process( $dependencyResult, $classNameLayerResolver, - $configuration->getRuleset() - ); - - $skippedViolations = $this->rulesetEngine->getSkippedViolations( - $violations, - $configuration->getSkipViolations() - ); - - return new DependencyContext( - $astMap, - $dependencyResult, - $classNameLayerResolver, - $violations, - $skippedViolations + $configuration ); } } diff --git a/src/Console/Command/AnalyzeCommand.php b/src/Console/Command/AnalyzeCommand.php index 0541b27d4..808dd3300 100644 --- a/src/Console/Command/AnalyzeCommand.php +++ b/src/Console/Command/AnalyzeCommand.php @@ -71,14 +71,14 @@ protected function execute(InputInterface $input, OutputInterface $output): int } $this->printCollectViolations($output); - $dependencyContext = $this->analyser->analyse($configuration); + $context = $this->analyser->analyse($configuration); $this->printFormattingStart($output); foreach ($this->formatterFactory->getActiveFormatters($input) as $formatter) { try { $formatter->finish( - $dependencyContext, + $context, $output, $this->formatterFactory->getOutputFormatterInput($formatter, $input) ); @@ -87,7 +87,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int } } - return $dependencyContext->hasViolations() ? 1 : 0; + return $context->hasViolations() ? 1 : 0; } protected function printBanner(OutputInterface $output): void diff --git a/src/DependencyContext.php b/src/DependencyContext.php deleted file mode 100644 index 7269b947c..000000000 --- a/src/DependencyContext.php +++ /dev/null @@ -1,111 +0,0 @@ -astMap = $astMap; - $this->dependencyResult = $dependencyResult; - $this->classNameLayerResolver = $classNameLayerResolver; - $this->violations = $violations; - $this->skippedViolations = $skippedViolations; - } - - public function getAstMap(): AstMap - { - return $this->astMap; - } - - /** - * @return RulesetViolation[] - */ - public function getViolations(): array - { - return $this->violations; - } - - /** - * @return RulesetViolation[] - */ - public function getViolationsByLayerName(string $layerName): array - { - return array_filter($this->violations, static function (RulesetViolation $violation) use ($layerName): bool { - return $violation->getLayerA() === $layerName; - }); - } - - public function getDependencyResult(): Result - { - return $this->dependencyResult; - } - - public function getClassNameLayerResolver(): ClassNameLayerResolverInterface - { - return $this->classNameLayerResolver; - } - - /** - * @return RulesetViolation[] - */ - public function getSkippedViolationsByLayerName(string $layerName): array - { - return array_values( - array_filter( - $this->skippedViolations, - static function (RulesetViolation $violation) use ($layerName): bool { - return $violation->getLayerA() === $layerName; - } - ) - ); - } - - /** - * @return RulesetViolation[] - */ - public function getSkippedViolations(): array - { - return $this->skippedViolations; - } - - public function isViolationSkipped(RulesetViolation $violation): bool - { - return \in_array($violation, $this->skippedViolations, true); - } - - public function hasViolations(): bool - { - return (count($this->violations) - count($this->skippedViolations)) > 0; - } -} diff --git a/src/OutputFormatter/ConsoleOutputFormatter.php b/src/OutputFormatter/ConsoleOutputFormatter.php index a283af543..1f9d2402c 100644 --- a/src/OutputFormatter/ConsoleOutputFormatter.php +++ b/src/OutputFormatter/ConsoleOutputFormatter.php @@ -5,12 +5,14 @@ namespace SensioLabs\Deptrac\OutputFormatter; use SensioLabs\Deptrac\AstRunner\AstMap\AstInherit; -use SensioLabs\Deptrac\DependencyContext; use SensioLabs\Deptrac\Dependency\InheritDependency; -use SensioLabs\Deptrac\RulesetEngine\RulesetViolation; +use SensioLabs\Deptrac\RulesetEngine\Context; +use SensioLabs\Deptrac\RulesetEngine\Rule; +use SensioLabs\Deptrac\RulesetEngine\SkippedViolation; +use SensioLabs\Deptrac\RulesetEngine\Violation; use Symfony\Component\Console\Output\OutputInterface; -class ConsoleOutputFormatter implements OutputFormatterInterface +final class ConsoleOutputFormatter implements OutputFormatterInterface { public function getName(): string { @@ -28,73 +30,77 @@ public function enabledByDefault(): bool } public function finish( - DependencyContext $dependencyContext, + Context $context, OutputInterface $output, OutputFormatterInput $outputFormatterInput ): void { - foreach ($dependencyContext->getViolations() as $violation) { - if ($violation->getDependency() instanceof InheritDependency) { - $this->handleInheritDependency($violation, $output, $dependencyContext->isViolationSkipped($violation)); + foreach ($context->all() as $rule) { + if (!$rule instanceof Violation && !$rule instanceof SkippedViolation) { continue; } - $this->handleDependency($violation, $output, $dependencyContext->isViolationSkipped($violation)); - } + if ($rule->getDependency() instanceof InheritDependency) { + $this->handleInheritDependency($rule, $output); + continue; + } - $violationCount = \count($dependencyContext->getViolations()); - $skippedViolationCount = \count($dependencyContext->getSkippedViolations()); - if ($violationCount > $skippedViolationCount) { - $output->writeln( - sprintf( - 'Found %s Violations'.($skippedViolationCount ? ' and %s Violations skipped' : ''), - $violationCount - $skippedViolationCount, - $skippedViolationCount - ) - ); - } else { - $output->writeln( - sprintf( - 'Found %s Violations'.($skippedViolationCount ? ' and %s Violations skipped' : ''), - $violationCount - $skippedViolationCount, - $skippedViolationCount - ) - ); + $this->handleDependency($rule, $output); } + + $violationCount = \count($context->violations()); + $skippedViolationCount = \count($context->skippedViolations()); + + $output->writeln( + sprintf( + 'Found %s Violations'.($skippedViolationCount ? ' and %s Violations skipped' : ''), + $violationCount, + $skippedViolationCount + ) + ); } - private function handleInheritDependency(RulesetViolation $violation, OutputInterface $output, bool $isSkipped) + /** + * @param Violation|SkippedViolation $rule + */ + private function handleInheritDependency(Rule $rule, OutputInterface $output): void { /** @var InheritDependency $dependency */ - $dependency = $violation->getDependency(); + $dependency = $rule->getDependency(); + $output->writeln( sprintf( "%s%s must not depend on %s (%s on %s) \n%s", - $isSkipped ? '[SKIPPED] ' : '', + $rule instanceof SkippedViolation ? '[SKIPPED] ' : '', $dependency->getClassA(), $dependency->getClassB(), - $violation->getLayerA(), - $violation->getLayerB(), + $rule->getLayerA(), + $rule->getLayerB(), $this->formatPath($dependency->getPath(), $dependency) ) ); } - private function handleDependency(RulesetViolation $violation, OutputInterface $output, bool $isSkipped) + /** + * @param Violation|SkippedViolation $rule + */ + private function handleDependency(Rule $rule, OutputInterface $output): void { + $dependency = $rule->getDependency(); + $output->writeln( sprintf( '%s%s::%s must not depend on %s (%s on %s)', - $isSkipped ? '[SKIPPED] ' : '', - $violation->getDependency()->getClassA(), - $violation->getDependency()->getClassALine(), - $violation->getDependency()->getClassB(), - $violation->getLayerA(), - $violation->getLayerB() + $rule instanceof SkippedViolation ? '[SKIPPED] ' : '', + $dependency->getClassA(), + $dependency->getClassALine(), + $dependency->getClassB(), + $rule->getLayerA(), + $rule->getLayerB() ) ); } - private function formatPath(AstInherit $astInherit, InheritDependency $dependency) + private function formatPath(AstInherit $astInherit, InheritDependency $dependency): string { $buffer = []; foreach ($astInherit->getPath() as $p) { diff --git a/src/OutputFormatter/GraphVizOutputFormatter.php b/src/OutputFormatter/GraphVizOutputFormatter.php index 1814e6706..cf2dfb3e7 100644 --- a/src/OutputFormatter/GraphVizOutputFormatter.php +++ b/src/OutputFormatter/GraphVizOutputFormatter.php @@ -4,19 +4,19 @@ namespace SensioLabs\Deptrac\OutputFormatter; +use Fhaculty\Graph\Graph; use Fhaculty\Graph\Vertex; use Graphp\GraphViz\GraphViz; -use SensioLabs\Deptrac\AstRunner\AstMap; -use SensioLabs\Deptrac\ClassNameLayerResolverInterface; -use SensioLabs\Deptrac\Dependency\Result; -use SensioLabs\Deptrac\DependencyContext; -use SensioLabs\Deptrac\RulesetEngine\RulesetViolation; +use SensioLabs\Deptrac\RulesetEngine\Allowed; +use SensioLabs\Deptrac\RulesetEngine\Context; +use SensioLabs\Deptrac\RulesetEngine\Rule; +use SensioLabs\Deptrac\RulesetEngine\SkippedViolation; +use SensioLabs\Deptrac\RulesetEngine\Uncovered; +use SensioLabs\Deptrac\RulesetEngine\Violation; use Symfony\Component\Console\Output\OutputInterface; class GraphVizOutputFormatter implements OutputFormatterInterface { - protected $eventDispatcher; - private static $argument_display = 'display'; private static $argument_dump_image = 'dump-image'; private static $argument_dump_dot = 'dump-dot'; @@ -46,19 +46,14 @@ public function configureOptions(): array } public function finish( - DependencyContext $dependencyContext, + Context $context, OutputInterface $output, OutputFormatterInput $outputFormatterInput ): void { - $layerViolations = $this->calculateViolations($dependencyContext->getViolations()); - - $layersDependOnLayers = $this->calculateLayerDependencies( - $dependencyContext->getAstMap(), - $dependencyContext->getDependencyResult(), - $dependencyContext->getClassNameLayerResolver() - ); + $layerViolations = $this->calculateViolations($context->violations()); + $layersDependOnLayers = $this->calculateLayerDependencies($context->all()); - $graph = new \Fhaculty\Graph\Graph(); + $graph = new Graph(); /** @var Vertex[] $vertices */ $vertices = []; @@ -112,7 +107,7 @@ public function finish( } /** - * @param RulesetViolation[] $violations + * @param Violation[] $violations */ private function calculateViolations(array $violations): array { @@ -132,47 +127,35 @@ private function calculateViolations(array $violations): array return $layerViolations; } - private function calculateLayerDependencies( - AstMap $astMap, - Result $dependencyResult, - ClassNameLayerResolverInterface $classNameLayerResolver - ): array { + /** + * @param Rule[] $rules + */ + private function calculateLayerDependencies(array $rules): array + { $layersDependOnLayers = []; - // all classes - foreach ($astMap->getAstClassReferences() as $classReferences) { - foreach ($classNameLayerResolver->getLayersByClassName( - $classReferences->getClassName() - ) as $classReferenceLayer) { - $layersDependOnLayers[$classReferenceLayer] = []; - } - } - - // dependencies - foreach ($dependencyResult->getDependenciesAndInheritDependencies() as $dependency) { - $layersA = $classNameLayerResolver->getLayersByClassName($dependency->getClassA()); - $layersB = $classNameLayerResolver->getLayersByClassName($dependency->getClassB()); - - if (empty($layersB)) { - continue; - } + foreach ($rules as $rule) { + if ($rule instanceof Violation + || $rule instanceof SkippedViolation + || $rule instanceof Allowed + ) { + $layerA = $rule->getLayerA(); + $layerB = $rule->getLayerB(); - foreach ($layersA as $layerA) { if (!isset($layersDependOnLayers[$layerA])) { $layersDependOnLayers[$layerA] = []; } - foreach ($layersB as $layerB) { - if ($layerA === $layerB) { - continue; - } - - if (!isset($layersDependOnLayers[$layerA][$layerB])) { - $layersDependOnLayers[$layerA][$layerB] = 1; - continue; - } + if (!isset($layersDependOnLayers[$layerA][$layerB])) { + $layersDependOnLayers[$layerA][$layerB] = 1; + continue; + } - ++$layersDependOnLayers[$layerA][$layerB]; + ++$layersDependOnLayers[$layerA][$layerB]; + } elseif ($rule instanceof Uncovered) { + $layer = $rule->getLayer(); + if (!isset($layersDependOnLayers[$layer])) { + $layersDependOnLayers[$layer] = []; } } } diff --git a/src/OutputFormatter/JUnitOutputFormatter.php b/src/OutputFormatter/JUnitOutputFormatter.php index 883a2f148..935ca47ce 100644 --- a/src/OutputFormatter/JUnitOutputFormatter.php +++ b/src/OutputFormatter/JUnitOutputFormatter.php @@ -4,8 +4,10 @@ namespace SensioLabs\Deptrac\OutputFormatter; -use SensioLabs\Deptrac\DependencyContext; -use SensioLabs\Deptrac\RulesetEngine\RulesetViolation; +use SensioLabs\Deptrac\RulesetEngine\Context; +use SensioLabs\Deptrac\RulesetEngine\Rule; +use SensioLabs\Deptrac\RulesetEngine\SkippedViolation; +use SensioLabs\Deptrac\RulesetEngine\Violation; use Symfony\Component\Console\Output\OutputInterface; final class JUnitOutputFormatter implements OutputFormatterInterface @@ -38,11 +40,11 @@ public function enabledByDefault(): bool * @throws \Exception */ public function finish( - DependencyContext $dependencyContext, + Context $context, OutputInterface $output, OutputFormatterInput $outputFormatterInput ): void { - $xml = $this->createXml($dependencyContext); + $xml = $this->createXml($context); if ($dumpXmlPath = $outputFormatterInput->getOption(static::$argument_dump_xml)) { file_put_contents($dumpXmlPath, $xml); @@ -53,7 +55,7 @@ public function finish( /** * @throws \Exception */ - private function createXml(DependencyContext $dependencyContext): string + private function createXml(Context $context): string { if (!class_exists(\DOMDocument::class)) { throw new \Exception('Unable to create xml file (php-xml needs to be installed)'); @@ -62,68 +64,82 @@ private function createXml(DependencyContext $dependencyContext): string $xmlDoc = new \DOMDocument('1.0', 'UTF-8'); $xmlDoc->formatOutput = true; - $this->addTestSuites($dependencyContext, $xmlDoc); + $this->addTestSuites($context, $xmlDoc); return $xmlDoc->saveXML(); } - private function addTestSuites(DependencyContext $dependencyContext, \DOMDocument $xmlDoc): void + private function addTestSuites(Context $context, \DOMDocument $xmlDoc): void { $testSuites = $xmlDoc->createElement('testsuites'); $xmlDoc->appendChild($testSuites); - $this->addTestSuite($dependencyContext, $xmlDoc, $testSuites); + $this->addTestSuite($context, $xmlDoc, $testSuites); } - private function addTestSuite(DependencyContext $dependencyContext, \DOMDocument $xmlDoc, \DOMElement $testSuites): void + private function addTestSuite(Context $context, \DOMDocument $xmlDoc, \DOMElement $testSuites): void { - $layers = $dependencyContext->getClassNameLayerResolver()->getLayers(); + $layers = []; + foreach ($context->all() as $rule) { + if (!$rule instanceof Violation && !$rule instanceof SkippedViolation) { + continue; + } + + $layers[$rule->getLayerA()][] = $rule; + } $layerIndex = 0; - foreach ($layers as $layer) { - $violationsByLayer = $dependencyContext->getViolationsByLayerName($layer); - if (0 === count($violationsByLayer)) { + foreach ($layers as $layer => $rules) { + /** @var Violation[] $violationsByLayer */ + $violationsByLayer = array_filter($rules, static function (Rule $rule) { + return $rule instanceof Violation; + }); + + /** @var SkippedViolation[] $skippedViolationsByLayer */ + $skippedViolationsByLayer = array_filter($rules, static function (Rule $rule) { + return $rule instanceof SkippedViolation; + }); + + if (0 === count($violationsByLayer) && 0 === count($skippedViolationsByLayer)) { continue; } - $violationsByClassName = []; - foreach ($violationsByLayer as $violation) { - $violationsByClassName[$violation->getDependency()->getClassA()][] = $violation; + $rulesByClassName = []; + foreach ($rules as $rule) { + $rulesByClassName[$rule->getDependency()->getClassA()][] = $rule; } - $skippedViolationsByLayer = $dependencyContext->getSkippedViolationsByLayerName($layer); - $testSuite = $xmlDoc->createElement('testsuite'); $testSuite->appendChild(new \DOMAttr('id', (string) ++$layerIndex)); $testSuite->appendChild(new \DOMAttr('package', '')); $testSuite->appendChild(new \DOMAttr('name', $layer)); $testSuite->appendChild(new \DOMAttr('hostname', 'localhost')); - $testSuite->appendChild(new \DOMAttr('tests', (string) count($violationsByClassName))); - $testSuite->appendChild(new \DOMAttr('failures', (string) (count($violationsByLayer) - count($skippedViolationsByLayer)))); + $testSuite->appendChild(new \DOMAttr('tests', (string) count($rulesByClassName))); + $testSuite->appendChild(new \DOMAttr('failures', (string) count($violationsByLayer))); $testSuite->appendChild(new \DOMAttr('skipped', (string) count($skippedViolationsByLayer))); $testSuite->appendChild(new \DOMAttr('errors', '0')); $testSuite->appendChild(new \DOMAttr('time', '0')); $testSuites->appendChild($testSuite); - $this->addTestCase($layer, $violationsByClassName, $xmlDoc, $testSuite, $skippedViolationsByLayer); + $this->addTestCase($layer, $rulesByClassName, $xmlDoc, $testSuite); } } - private function addTestCase(string $layer, array $violationsByClassName, \DOMDocument $xmlDoc, \DOMElement $testSuite, array $skippedViolationsByLayer): void + private function addTestCase(string $layer, array $rulesByClassName, \DOMDocument $xmlDoc, \DOMElement $testSuite): void { - foreach ($violationsByClassName as $className => $violations) { + foreach ($rulesByClassName as $className => $rules) { $testCase = $xmlDoc->createElement('testcase'); $testCase->appendChild(new \DOMAttr('name', $layer.' - '.$className)); $testCase->appendChild(new \DOMAttr('classname', $className)); $testCase->appendChild(new \DOMAttr('time', '0')); - foreach ($violations as $violation) { - if (\in_array($violation, $skippedViolationsByLayer, true)) { + foreach ($rules as $rule) { + if ($rule instanceof SkippedViolation) { $this->addSkipped($xmlDoc, $testCase); - } else { - $this->addFailure($violation, $xmlDoc, $testCase); + } elseif ($rule instanceof Violation) { + $this->addFailure($rule, $xmlDoc, $testCase); } } @@ -131,7 +147,7 @@ private function addTestCase(string $layer, array $violationsByClassName, \DOMDo } } - private function addFailure(RulesetViolation $violation, \DOMDocument $xmlDoc, \DOMElement $testCase): void + private function addFailure(Violation $violation, \DOMDocument $xmlDoc, \DOMElement $testCase): void { $dependency = $violation->getDependency(); diff --git a/src/OutputFormatter/OutputFormatterInterface.php b/src/OutputFormatter/OutputFormatterInterface.php index c9ad1715f..75f874ae5 100644 --- a/src/OutputFormatter/OutputFormatterInterface.php +++ b/src/OutputFormatter/OutputFormatterInterface.php @@ -4,7 +4,7 @@ namespace SensioLabs\Deptrac\OutputFormatter; -use SensioLabs\Deptrac\DependencyContext; +use SensioLabs\Deptrac\RulesetEngine\Context; use Symfony\Component\Console\Output\OutputInterface; interface OutputFormatterInterface @@ -28,7 +28,7 @@ public function enabledByDefault(): bool; * Renders the final result. */ public function finish( - DependencyContext $dependencyContext, + Context $context, OutputInterface $output, OutputFormatterInput $outputFormatterInput ): void; diff --git a/src/RulesetEngine.php b/src/RulesetEngine.php index ba9d31e55..a3287ae56 100644 --- a/src/RulesetEngine.php +++ b/src/RulesetEngine.php @@ -4,19 +4,25 @@ namespace SensioLabs\Deptrac; -use SensioLabs\Deptrac\Configuration\ConfigurationRuleset; -use SensioLabs\Deptrac\Configuration\ConfigurationSkippedViolation; +use SensioLabs\Deptrac\Configuration\Configuration; use SensioLabs\Deptrac\Dependency\Result; -use SensioLabs\Deptrac\RulesetEngine\RulesetViolation; +use SensioLabs\Deptrac\RulesetEngine\Allowed; +use SensioLabs\Deptrac\RulesetEngine\Context; +use SensioLabs\Deptrac\RulesetEngine\SkippedViolation; +use SensioLabs\Deptrac\RulesetEngine\Uncovered; +use SensioLabs\Deptrac\RulesetEngine\Violation; class RulesetEngine { - /** - * @return RulesetViolation[] - */ - public function getViolations(Result $dependencyResult, ClassNameLayerResolverInterface $classNameLayerResolver, ConfigurationRuleset $configurationRuleset): array - { - $violations = []; + public function process( + Result $dependencyResult, + ClassNameLayerResolverInterface $classNameLayerResolver, + Configuration $configuration + ): Context { + $rules = []; + + $configurationRuleset = $configuration->getRuleset(); + $configurationSkippedViolation = $configuration->getSkipViolations(); foreach ($dependencyResult->getDependenciesAndInheritDependencies() as $dependency) { $layerNames = $classNameLayerResolver->getLayersByClassName($dependency->getClassA()); @@ -24,41 +30,33 @@ public function getViolations(Result $dependencyResult, ClassNameLayerResolverIn foreach ($layerNames as $layerName) { $allowedDependencies = $configurationRuleset->getAllowedDependencies($layerName); - foreach ($classNameLayerResolver->getLayersByClassName($dependency->getClassB()) as $layerNameOfDependency) { + $layersNamesClassB = $classNameLayerResolver->getLayersByClassName($dependency->getClassB()); + + if (0 === count($layersNamesClassB)) { + $rules[] = new Uncovered($dependency, $layerName); + continue; + } + + foreach ($layersNamesClassB as $layerNameOfDependency) { if ($layerName === $layerNameOfDependency) { continue; } if (in_array($layerNameOfDependency, $allowedDependencies, true)) { + $rules[] = new Allowed($dependency, $layerName, $layerNameOfDependency); + continue; + } + + if ($configurationSkippedViolation->isViolationSkipped($dependency->getClassA(), $dependency->getClassB())) { + $rules[] = new SkippedViolation($dependency, $layerName, $layerNameOfDependency); continue; } - $violations[] = new RulesetViolation( - $dependency, - $layerName, - $layerNameOfDependency - ); + $rules[] = new Violation($dependency, $layerName, $layerNameOfDependency); } } } - return $violations; - } - - /** - * @param RulesetViolation[] $violations - * - * @return RulesetViolation[] - */ - public function getSkippedViolations(array $violations, ConfigurationSkippedViolation $configurationSkipViolation): array - { - return \array_values( - \array_filter($violations, static function ($violation) use ($configurationSkipViolation): bool { - /** @var RulesetViolation $violation */ - $dep = $violation->getDependency(); - - return $configurationSkipViolation->isViolationSkipped($dep->getClassA(), $dep->getClassB()); - }) - ); + return new Context($rules); } } diff --git a/src/RulesetEngine/RulesetViolation.php b/src/RulesetEngine/Allowed.php similarity index 95% rename from src/RulesetEngine/RulesetViolation.php rename to src/RulesetEngine/Allowed.php index 3f7d07d9b..c35bc8372 100644 --- a/src/RulesetEngine/RulesetViolation.php +++ b/src/RulesetEngine/Allowed.php @@ -6,7 +6,7 @@ use SensioLabs\Deptrac\Dependency\DependencyInterface; -class RulesetViolation +final class Allowed implements Rule { private $dependency; private $layerA; diff --git a/src/RulesetEngine/Context.php b/src/RulesetEngine/Context.php new file mode 100644 index 000000000..484d3097e --- /dev/null +++ b/src/RulesetEngine/Context.php @@ -0,0 +1,54 @@ +rules = $rules; + } + + /** + * @return Rule[] + */ + public function all(): array + { + return $this->rules; + } + + /** + * @return Violation[] + */ + public function violations(): array + { + return array_filter($this->rules, static function (Rule $rule) { + return $rule instanceof Violation; + }); + } + + public function hasViolations(): bool + { + return count($this->violations()) > 0; + } + + /** + * @return SkippedViolation[] + */ + public function skippedViolations(): array + { + return array_filter($this->rules, static function (Rule $rule) { + return $rule instanceof SkippedViolation; + }); + } +} diff --git a/src/RulesetEngine/Rule.php b/src/RulesetEngine/Rule.php new file mode 100644 index 000000000..f18555876 --- /dev/null +++ b/src/RulesetEngine/Rule.php @@ -0,0 +1,12 @@ +dependency = $dependency; + $this->layerA = $layerA; + $this->layerB = $layerB; + } + + public function getDependency(): DependencyInterface + { + return $this->dependency; + } + + public function getLayerA(): string + { + return $this->layerA; + } + + public function getLayerB(): string + { + return $this->layerB; + } +} diff --git a/src/RulesetEngine/Uncovered.php b/src/RulesetEngine/Uncovered.php new file mode 100644 index 000000000..fbf60b32b --- /dev/null +++ b/src/RulesetEngine/Uncovered.php @@ -0,0 +1,29 @@ +dependency = $dependency; + $this->layer = $layer; + } + + public function getDependency(): DependencyInterface + { + return $this->dependency; + } + + public function getLayer(): string + { + return $this->layer; + } +} diff --git a/src/RulesetEngine/Violation.php b/src/RulesetEngine/Violation.php new file mode 100644 index 000000000..82450b473 --- /dev/null +++ b/src/RulesetEngine/Violation.php @@ -0,0 +1,36 @@ +dependency = $dependency; + $this->layerA = $layerA; + $this->layerB = $layerB; + } + + public function getDependency(): DependencyInterface + { + return $this->dependency; + } + + public function getLayerA(): string + { + return $this->layerA; + } + + public function getLayerB(): string + { + return $this->layerB; + } +} diff --git a/tests/DependencyContextTest.php b/tests/DependencyContextTest.php deleted file mode 100644 index 76d41bb5a..000000000 --- a/tests/DependencyContextTest.php +++ /dev/null @@ -1,99 +0,0 @@ -prophesize(AstMap::class)->reveal(), - $dependencyResult = $this->prophesize(Result::class)->reveal(), - $classNameLayerResolver = $this->prophesize(ClassNameLayerResolverInterface::class)->reveal(), - [1, 2, 3] - ); - - static::assertSame($astMap, $context->getAstMap()); - static::assertEquals([1, 2, 3], $context->getViolations()); - static::assertSame($dependencyResult, $context->getDependencyResult()); - static::assertSame($classNameLayerResolver, $context->getClassNameLayerResolver()); - } - - public function testIsViolationSkipped(): void - { - $skippedViolations = [ - new RulesetViolation( - new Dependency('ClassA', 12, 'ClassB'), - 'LayerA', - 'LayerB' - ), - ]; - $context = new DependencyContext( - $astMap = $this->prophesize(AstMap::class)->reveal(), - $dependencyResult = $this->prophesize(Result::class)->reveal(), - $classNameLayerResolver = $this->prophesize(ClassNameLayerResolverInterface::class)->reveal(), - [], - $skippedViolations - ); - $this->assertSame($skippedViolations, $context->getSkippedViolations()); - } - - public function testGetSkippedViolationsByLayerName(): void - { - $skippedViolations = [ - new RulesetViolation( - new Dependency('ClassA', 12, 'ClassB'), - 'LayerA', - 'LayerB' - ), - $matchedViolation = new RulesetViolation( - new Dependency('ClassA', 12, 'ClassB'), - 'LayerC', - 'LayerD' - ), - ]; - $context = new DependencyContext( - $astMap = $this->prophesize(AstMap::class)->reveal(), - $dependencyResult = $this->prophesize(Result::class)->reveal(), - $classNameLayerResolver = $this->prophesize(ClassNameLayerResolverInterface::class)->reveal(), - [], - $skippedViolations - ); - $this->assertSame([$matchedViolation], $context->getSkippedViolationsByLayerName('LayerC')); - $this->assertSame([], $context->getSkippedViolationsByLayerName('LayerB')); - } - - public function testGetSkippedViolations(): void - { - $skippedViolations = [ - new RulesetViolation( - new Dependency('ClassA', 12, 'ClassB'), - 'LayerA', - 'LayerB' - ), - $matchedViolation = new RulesetViolation( - new Dependency('ClassA', 12, 'ClassB'), - 'LayerC', - 'LayerD' - ), - ]; - $context = new DependencyContext( - $astMap = $this->prophesize(AstMap::class)->reveal(), - $dependencyResult = $this->prophesize(Result::class)->reveal(), - $classNameLayerResolver = $this->prophesize(ClassNameLayerResolverInterface::class)->reveal(), - [], - $skippedViolations - ); - $this->assertSame($skippedViolations, $context->getSkippedViolations()); - } -} diff --git a/tests/OutputFormatter/ConsoleOutputFormatterTest.php b/tests/OutputFormatter/ConsoleOutputFormatterTest.php index acc371538..e785401db 100644 --- a/tests/OutputFormatter/ConsoleOutputFormatterTest.php +++ b/tests/OutputFormatter/ConsoleOutputFormatterTest.php @@ -5,16 +5,14 @@ namespace Tests\SensioLabs\Deptrac\OutputFormatter; use PHPUnit\Framework\TestCase; -use SensioLabs\Deptrac\AstRunner\AstMap; use SensioLabs\Deptrac\AstRunner\AstMap\AstInherit; -use SensioLabs\Deptrac\ClassNameLayerResolverInterface; -use SensioLabs\Deptrac\Dependency\Result; -use SensioLabs\Deptrac\DependencyContext; use SensioLabs\Deptrac\Dependency\Dependency; use SensioLabs\Deptrac\Dependency\InheritDependency; use SensioLabs\Deptrac\OutputFormatter\ConsoleOutputFormatter; use SensioLabs\Deptrac\OutputFormatter\OutputFormatterInput; -use SensioLabs\Deptrac\RulesetEngine\RulesetViolation; +use SensioLabs\Deptrac\RulesetEngine\Context; +use SensioLabs\Deptrac\RulesetEngine\SkippedViolation; +use SensioLabs\Deptrac\RulesetEngine\Violation; use Symfony\Component\Console\Output\BufferedOutput; class ConsoleOutputFormatterTest extends TestCase @@ -28,7 +26,7 @@ public function basicDataProvider(): iterable { yield [ [ - new RulesetViolation( + new Violation( new InheritDependency( 'ClassA', 'ClassB', @@ -43,7 +41,6 @@ public function basicDataProvider(): iterable 'LayerB' ), ], - [], ' ClassA must not depend on ClassB (LayerA on LayerB) ClassInheritD::6 -> @@ -58,13 +55,12 @@ public function basicDataProvider(): iterable yield [ [ - new RulesetViolation( + new Violation( new Dependency('OriginalA', 12, 'OriginalB'), 'LayerA', 'LayerB' ), ], - [], ' OriginalA::12 must not depend on OriginalB (LayerA on LayerB) @@ -73,7 +69,6 @@ public function basicDataProvider(): iterable ]; yield [ - [], [], ' @@ -83,15 +78,12 @@ public function basicDataProvider(): iterable yield [ [ - $violation = new RulesetViolation( + new SkippedViolation( new Dependency('OriginalA', 12, 'OriginalB'), 'LayerA', 'LayerB' ), ], - [ - $violation, - ], '[SKIPPED] OriginalA::12 must not depend on OriginalB (LayerA on LayerB) Found 0 Violations and 1 Violations skipped ', @@ -101,19 +93,13 @@ public function basicDataProvider(): iterable /** * @dataProvider basicDataProvider */ - public function testBasic(array $violations, array $skippedViolations, string $expectedOutput): void + public function testBasic(array $rules, string $expectedOutput): void { $output = new BufferedOutput(); $formatter = new ConsoleOutputFormatter(); $formatter->finish( - new DependencyContext( - $this->prophesize(AstMap::class)->reveal(), - $this->prophesize(Result::class)->reveal(), - $this->prophesize(ClassNameLayerResolverInterface::class)->reveal(), - $violations, - $skippedViolations - ), + new Context($rules), $output, new OutputFormatterInput([]) ); diff --git a/tests/OutputFormatter/GraphVizOutputFormatterTest.php b/tests/OutputFormatter/GraphVizOutputFormatterTest.php index ea11181d8..192a35f0a 100644 --- a/tests/OutputFormatter/GraphVizOutputFormatterTest.php +++ b/tests/OutputFormatter/GraphVizOutputFormatterTest.php @@ -5,7 +5,14 @@ namespace Tests\SensioLabs\Deptrac\OutputFormatter; use PHPUnit\Framework\TestCase; +use SensioLabs\Deptrac\Dependency\Dependency; use SensioLabs\Deptrac\OutputFormatter\GraphVizOutputFormatter; +use SensioLabs\Deptrac\OutputFormatter\OutputFormatterInput; +use SensioLabs\Deptrac\RulesetEngine\Allowed; +use SensioLabs\Deptrac\RulesetEngine\Context; +use SensioLabs\Deptrac\RulesetEngine\Uncovered; +use SensioLabs\Deptrac\RulesetEngine\Violation; +use Symfony\Component\Console\Output\BufferedOutput; class GraphVizOutputFormatterTest extends TestCase { @@ -13,4 +20,31 @@ public function testGetName(): void { static::assertEquals('graphviz', (new GraphVizOutputFormatter())->getName()); } + + public function testFinish(): void + { + $dotFile = __DIR__.'/data/graphviz.dot'; + + $context = new Context([ + new Violation(new Dependency('ClassA', 0, 'ClassB'), 'LayerA', 'LayerB'), + new Violation(new Dependency('ClassAB', 1, 'ClassBA'), 'LayerA', 'LayerB'), + new Allowed(new Dependency('ClassA', 0, 'ClassC'), 'LayerA', 'LayerC'), + new Uncovered(new Dependency('ClassA', 0, 'ClassD'), 'LayerC'), + ]); + + $output = new BufferedOutput(); + $input = new OutputFormatterInput([ + 'display' => false, + 'dump-image' => false, + 'dump-dot' => $dotFile, + 'dump-html' => false, + ]); + + (new GraphVizOutputFormatter())->finish($context, $output, $input); + + static::assertSame(sprintf("Script dumped to %s\n", $dotFile), $output->fetch()); + static::assertFileEquals(__DIR__.'/data/graphviz-expected.dot', $dotFile); + + unlink($dotFile); + } } diff --git a/tests/OutputFormatter/JUnitOutputFormatterTest.php b/tests/OutputFormatter/JUnitOutputFormatterTest.php index d7705debb..264c69738 100644 --- a/tests/OutputFormatter/JUnitOutputFormatterTest.php +++ b/tests/OutputFormatter/JUnitOutputFormatterTest.php @@ -5,16 +5,14 @@ namespace Tests\SensioLabs\Deptrac\OutputFormatter; use PHPUnit\Framework\TestCase; -use SensioLabs\Deptrac\AstRunner\AstMap; use SensioLabs\Deptrac\AstRunner\AstMap\AstInherit; -use SensioLabs\Deptrac\ClassNameLayerResolverInterface; -use SensioLabs\Deptrac\Dependency\Result; -use SensioLabs\Deptrac\DependencyContext; use SensioLabs\Deptrac\Dependency\Dependency; use SensioLabs\Deptrac\Dependency\InheritDependency; use SensioLabs\Deptrac\OutputFormatter\JUnitOutputFormatter; use SensioLabs\Deptrac\OutputFormatter\OutputFormatterInput; -use SensioLabs\Deptrac\RulesetEngine\RulesetViolation; +use SensioLabs\Deptrac\RulesetEngine\Context; +use SensioLabs\Deptrac\RulesetEngine\SkippedViolation; +use SensioLabs\Deptrac\RulesetEngine\Violation; use Symfony\Component\Console\Output\BufferedOutput; class JUnitOutputFormatterTest extends TestCase @@ -37,11 +35,7 @@ public function basicDataProvider(): iterable { yield [ [ - 'LayerA', - 'LayerB', - ], - [ - new RulesetViolation( + new Violation( new InheritDependency( 'ClassA', 'ClassB', @@ -56,42 +50,28 @@ public function basicDataProvider(): iterable 'LayerB' ), ], - [], 'expected-junit-report_1.xml', ]; yield [ [ - 'LayerA', - 'LayerB', - ], - [ - new RulesetViolation( + new Violation( new Dependency('OriginalA', 12, 'OriginalB'), 'LayerA', 'LayerB' ), ], - [], 'expected-junit-report_2.xml', ]; yield [ - [ - ], - [ - ], [], 'expected-junit-report_3.xml', ]; yield [ [ - 'LayerA', - 'LayerB', - ], - [ - $violations = new RulesetViolation( + new SkippedViolation( new InheritDependency( 'ClassA', 'ClassB', @@ -105,7 +85,7 @@ public function basicDataProvider(): iterable 'LayerA', 'LayerB' ), - new RulesetViolation( + new Violation( new InheritDependency( 'ClassC', 'ClassD', @@ -120,9 +100,6 @@ public function basicDataProvider(): iterable 'LayerB' ), ], - [ - $violations, - ], 'expected-junit-report-with-skipped-violations.xml', ]; } @@ -130,22 +107,13 @@ public function basicDataProvider(): iterable /** * @dataProvider basicDataProvider */ - public function testBasic(array $layers, array $violations, array $skippedViolations, $expectedOutputFile): void + public function testBasic(array $rules, string $expectedOutputFile): void { - $classNameResolver = $this->prophesize(ClassNameLayerResolverInterface::class); - $classNameResolver->getLayers()->willReturn($layers); - $output = new BufferedOutput(); $formatter = new JUnitOutputFormatter(); $formatter->finish( - new DependencyContext( - $this->prophesize(AstMap::class)->reveal(), - $this->prophesize(Result::class)->reveal(), - $classNameResolver->reveal(), - $violations, - $skippedViolations - ), + new Context($rules), $output, new OutputFormatterInput(['dump-xml' => __DIR__.'/data/'.static::$actual_junit_report_file]) ); diff --git a/tests/OutputFormatter/data/graphviz-expected.dot b/tests/OutputFormatter/data/graphviz-expected.dot new file mode 100644 index 000000000..608e20761 --- /dev/null +++ b/tests/OutputFormatter/data/graphviz-expected.dot @@ -0,0 +1,4 @@ +digraph G { + "LayerA" -> "LayerB" [label=2 color="red"] + "LayerA" -> "LayerC" +} diff --git a/tests/RuleEngine/RulesetViolationTest.php b/tests/RuleEngine/RulesetViolationTest.php deleted file mode 100644 index 7dd31011a..000000000 --- a/tests/RuleEngine/RulesetViolationTest.php +++ /dev/null @@ -1,25 +0,0 @@ -prophesize(DependencyInterface::class)->reveal(), - 'layerA', - 'layerB' - ); - - static::assertSame($dep, $ruleViolation->getDependency()); - static::assertEquals('layerA', $ruleViolation->getLayerA()); - static::assertEquals('layerB', $ruleViolation->getLayerB()); - } -} diff --git a/tests/RulesetEngineTest.php b/tests/RulesetEngineTest.php index c028eab13..d2229bbdd 100644 --- a/tests/RulesetEngineTest.php +++ b/tests/RulesetEngineTest.php @@ -6,8 +6,7 @@ use PHPUnit\Framework\TestCase; use SensioLabs\Deptrac\ClassNameLayerResolverInterface; -use SensioLabs\Deptrac\Configuration\ConfigurationRuleset; -use SensioLabs\Deptrac\Configuration\ConfigurationSkippedViolation; +use SensioLabs\Deptrac\Configuration\Configuration; use SensioLabs\Deptrac\Dependency\Result; use SensioLabs\Deptrac\Dependency\Dependency; use SensioLabs\Deptrac\RulesetEngine; @@ -37,7 +36,6 @@ public function dependencyProvider(): iterable 'LayerA' => [ 'LayerB', ], - 'LayerC' => [], ], 0, ]; @@ -140,9 +138,9 @@ public function dependencyProvider(): iterable /** * @dataProvider dependencyProvider */ - public function testGetViolationsButNoViolations(array $dependenciesAsArray, array $classesInLayers, array $rulesetConfiguration, int $expectedCount): void + public function testProcess(array $dependenciesAsArray, array $classesInLayers, array $rulesetConfiguration, int $expectedCount): void { - $dependencyResult = (new Result()); + $dependencyResult = new Result(); foreach ($this->createDependencies($dependenciesAsArray) as $dep) { $dependencyResult->addDependency($dep); } @@ -152,82 +150,86 @@ public function testGetViolationsButNoViolations(array $dependenciesAsArray, arr $classNameLayerResolver->getLayersByClassName($classInLayer)->willReturn($layers); } - static::assertCount( - $expectedCount, - (new RulesetEngine())->getViolations( - $dependencyResult, - $classNameLayerResolver->reveal(), - ConfigurationRuleset::fromArray($rulesetConfiguration) - ) + $configuration = Configuration::fromArray([ + 'layers' => [], + 'paths' => [], + 'ruleset' => $rulesetConfiguration, + ]); + + $context = (new RulesetEngine())->process( + $dependencyResult, + $classNameLayerResolver->reveal(), + $configuration ); + + static::assertCount($expectedCount, $context->violations()); } public function provideTestGetSkippedViolations(): array { return [ - 'empty violations' => [ - [], + 'not skipped violations' => [ [ - 'ClassA' => [ - 'ClassB', - 'ClassC', - ], + 'ClassA' => 'ClassB', + 'ClassB' => 'ClassA', ], - [], - ], - 'not matched violations' => [ [ - new RulesetEngine\RulesetViolation( - new Dependency('ClassA', 12, 'ClassD'), - 'LayerA', - 'LayerB' - ), - ], - [ - 'ClassA' => [ - 'ClassB', - 'ClassC', - ], + 'ClassA' => ['LayerA'], + 'ClassB' => ['LayerB'], ], [], + 0, ], - 'has matched violations' => [ + 'has skipped violations' => [ + [ + 'ClassA' => 'ClassB', + 'ClassB' => 'ClassA', + ], [ - new RulesetEngine\RulesetViolation( - new Dependency('ClassA', 12, 'ClassD'), - 'LayerA', - 'LayerB' - ), - $matchedViolation = new RulesetEngine\RulesetViolation( - new Dependency('ClassA', 12, 'ClassB'), - 'LayerA', - 'LayerB' - ), + 'ClassA' => ['LayerA'], + 'ClassB' => ['LayerB'], ], [ 'ClassA' => [ 'ClassB', - 'ClassC', + ], + 'ClassB' => [ + 'ClassA', ], ], - [ - $matchedViolation, - ], - ], + 2, + ] ]; } /** * @dataProvider provideTestGetSkippedViolations */ - public function testGetSkippedViolations(array $violations, array $skippedViolationsConfig, array $expectedSkippedViolations): void + public function testGetSkippedViolations(array $dependenciesAsArray, array $classesInLayers, array $skippedViolationsConfig, int $expectedSkippedViolationCount): void { - static::assertSame( - $expectedSkippedViolations, - (new RulesetEngine())->getSkippedViolations( - $violations, - ConfigurationSkippedViolation::fromArray($skippedViolationsConfig) - ) + $dependencyResult = new Result(); + foreach ($this->createDependencies($dependenciesAsArray) as $dep) { + $dependencyResult->addDependency($dep); + } + + $classNameLayerResolver = $this->prophesize(ClassNameLayerResolverInterface::class); + foreach ($classesInLayers as $classInLayer => $layers) { + $classNameLayerResolver->getLayersByClassName($classInLayer)->willReturn($layers); + } + + $configuration = Configuration::fromArray([ + 'layers' => [], + 'paths' => [], + 'ruleset' => [], + 'skip_violations' => $skippedViolationsConfig, + ]); + + $context = (new RulesetEngine())->process( + $dependencyResult, + $classNameLayerResolver->reveal(), + $configuration ); + + static::assertCount($expectedSkippedViolationCount, $context->skippedViolations()); } }