Skip to content

Commit

Permalink
Fix: Reject rule sets referencing unknown layers
Browse files Browse the repository at this point in the history
  • Loading branch information
@localheinz
localheinz committed Oct 20, 2020
1 parent 0148a64 commit fe79d6b
Show file tree
Hide file tree
Showing 5 changed files with 177 additions and 8 deletions.
16 changes: 16 additions & 0 deletions src/Configuration/Configuration.php
View file
Expand Up @@ -68,6 +68,22 @@ private function __construct(array $options)
throw Exception\InvalidConfigurationException::fromDuplicateLayerNames(...$duplicateLayerNames);
}

$layerNamesUsedInRuleset = array_unique(array_merge(
array_keys($options['ruleset']),
...array_values(array_map(static function (?array $rules): array {
return (array) $rules;
}, $options['ruleset']))
));

$unknownLayerNames = array_diff(
$layerNamesUsedInRuleset,
$layerNames
);

if ([] !== $unknownLayerNames) {
throw Exception\InvalidConfigurationException::fromUnknownLayerNames(...$unknownLayerNames);
}

$this->ruleset = ConfigurationRuleset::fromArray($options['ruleset']);
$this->paths = $options['paths'];
$this->skipViolations = ConfigurationSkippedViolation::fromArray($options['skip_violations']);
Expand Down
10 changes: 10 additions & 0 deletions src/Configuration/Exception/InvalidConfigurationException.php
View file
Expand Up @@ -15,4 +15,14 @@ public static function fromDuplicateLayerNames(string ...$layerNames): self
implode('", "', $layerNames)
));
}

public static function fromUnknownLayerNames(string ...$layerNames): self
{
natsort($layerNames);

return new self(sprintf(
'Configuration can not reference rule sets with unknown layer names, got "%s" as unknown.',
implode('", "', $layerNames)
));
}
}
51 changes: 46 additions & 5 deletions tests/Configuration/ConfigurationTest.php
View file
Expand Up @@ -57,6 +57,43 @@ public function testFromArrayRejectsLayersWithDuplicateNames(): void
]);
}

public function testFromArrayRejectsRulesetUsingUnknownLayerNames(): void
{
$this->expectException(Exception\InvalidConfigurationException::class);
$this->expectExceptionMessage('Configuration can not reference rule sets with unknown layer names, got "quux", "qux" as unknown.');

Configuration::fromArray([
'layers' => [
[
'name' => 'foo',
'collectors' => [],
],
[
'name' => 'bar',
'collectors' => [],
],
[
'name' => 'baz',
'collectors' => [],
],
],
'paths' => [
'src',
],
'ruleset' => [
'foo' => [
'bar',
],
'bar' => null,
'baz' => [
'bar',
'qux',
],
'quux' => null,
],
]);
}

public function testFromArray(): void
{
$configuration = Configuration::fromArray([
Expand All @@ -66,7 +103,11 @@ public function testFromArray(): void
'collectors' => [],
],
[
'name' => 'some_other_name',
'name' => 'xx',
'collectors' => [],
],
[
'name' => 'yy',
'collectors' => [],
],
],
Expand All @@ -79,15 +120,15 @@ public function testFromArray(): void
'bar2',
],
'ruleset' => [
'lala' => ['xx', 'yy'],
'some_name' => ['xx', 'yy'],
],
]);

static::assertCount(2, $configuration->getLayers());
static::assertCount(3, $configuration->getLayers());
static::assertEquals('some_name', $configuration->getLayers()[0]->getName());
static::assertEquals(['foo', 'bar'], $configuration->getPaths());
static::assertEquals(['foo2', 'bar2'], $configuration->getExcludeFiles());
static::assertEquals(['xx', 'yy'], $configuration->getRuleset()->getAllowedDependencies('lala'));
static::assertEquals(['xx', 'yy'], $configuration->getRuleset()->getAllowedDependencies('some_name'));
static::assertTrue($configuration->ignoreUncoveredInternalClasses());
}

Expand All @@ -109,7 +150,7 @@ public function testExcludedFilesAreOptional(): void
'bar',
],
'ruleset' => [
'lala' => ['xx', 'yy'],
'some_name' => ['some_other_name'],
],
]);

Expand Down
19 changes: 19 additions & 0 deletions tests/Configuration/Exception/InvalidConfigurationExceptionTest.php
View file
Expand Up @@ -37,4 +37,23 @@ public function testFromDuplicateLayerNamesReturnsException(): void

self::assertSame($message, $exception->getMessage());
}

public function testFromUnknownLayerNamesReturnsException(): void
{
$layerNames = [
'foo',
'bar',
];

$exception = InvalidConfigurationException::fromUnknownLayerNames(...$layerNames);

natsort($layerNames);

$message = sprintf(
'Configuration can not reference rule sets with unknown layer names, got "%s" as unknown.',
implode('", "', $layerNames)
);

self::assertSame($message, $exception->getMessage());
}
}
89 changes: 86 additions & 3 deletions tests/RulesetEngineTest.php
View file
Expand Up @@ -13,6 +13,9 @@
use SensioLabs\Deptrac\Dependency\Result;
use SensioLabs\Deptrac\RulesetEngine;

/**
* @covers \SensioLabs\Deptrac\RulesetEngine
*/
class RulesetEngineTest extends TestCase
{
private function createDependencies(array $fromTo): iterable
Expand All @@ -38,6 +41,16 @@ public function dependencyProvider(): iterable
'ClassA' => ['LayerA'],
'ClassB' => ['LayerB'],
],
[
[
'name' => 'LayerA',
'collectors' => [],
],
[
'name' => 'LayerB',
'collectors' => [],
],
],
[
'LayerA' => [
'LayerB',
Expand All @@ -54,6 +67,16 @@ public function dependencyProvider(): iterable
'ClassA' => ['LayerA'],
'ClassB' => ['LayerB'],
],
[
[
'name' => 'LayerA',
'collectors' => [],
],
[
'name' => 'LayerB',
'collectors' => [],
],
],
[
'LayerA' => [],
'LayerB' => [],
Expand All @@ -69,6 +92,16 @@ public function dependencyProvider(): iterable
'ClassA' => ['LayerA'],
'ClassB' => ['LayerB'],
],
[
[
'name' => 'LayerA',
'collectors' => [],
],
[
'name' => 'LayerB',
'collectors' => [],
],
],
[],
1,
];
Expand All @@ -82,6 +115,7 @@ public function dependencyProvider(): iterable
'ClassB' => [],
],
[],
[],
0,
];

Expand All @@ -93,6 +127,16 @@ public function dependencyProvider(): iterable
'ClassA' => ['LayerA'],
'ClassB' => ['LayerB'],
],
[
[
'name' => 'LayerA',
'collectors' => [],
],
[
'name' => 'LayerB',
'collectors' => [],
],
],
[
'LayerA' => ['LayerB'],
],
Expand All @@ -107,6 +151,16 @@ public function dependencyProvider(): iterable
'ClassA' => ['LayerA'],
'ClassB' => ['LayerB'],
],
[
[
'name' => 'LayerA',
'collectors' => [],
],
[
'name' => 'LayerB',
'collectors' => [],
],
],
[
'LayerB' => ['LayerA'],
],
Expand All @@ -125,6 +179,24 @@ public function dependencyProvider(): iterable
'ClassC' => ['LayerC'],
'ClassD' => ['LayerD'],
],
[
[
'name' => 'LayerA',
'collectors' => [],
],
[
'name' => 'LayerB',
'collectors' => [],
],
[
'name' => 'LayerC',
'collectors' => [],
],
[
'name' => 'LayerD',
'collectors' => [],
],
],
[],
3,
];
Expand All @@ -136,6 +208,12 @@ public function dependencyProvider(): iterable
[
'ClassA' => ['LayerA'],
],
[
[
'name' => 'LayerA',
'collectors' => [],
],
],
[],
0,
];
Expand All @@ -144,8 +222,13 @@ public function dependencyProvider(): iterable
/**
* @dataProvider dependencyProvider
*/
public function testProcess(array $dependenciesAsArray, array $classesInLayers, array $rulesetConfiguration, int $expectedCount): void
{
public function testProcess(
array $dependenciesAsArray,
array $classesInLayers,
array $layersConfiguration,
array $rulesetConfiguration,
int $expectedCount
): void {
$dependencyResult = new Result();
foreach ($this->createDependencies($dependenciesAsArray) as $dep) {
$dependencyResult->addDependency($dep);
Expand All @@ -157,7 +240,7 @@ public function testProcess(array $dependenciesAsArray, array $classesInLayers,
}

$configuration = Configuration::fromArray([
'layers' => [],
'layers' => $layersConfiguration,
'paths' => [],
'ruleset' => $rulesetConfiguration,
]);
Expand Down

0 comments on commit fe79d6b

Please sign in to comment.