Make @internal tag configurable

Some projects, like MediaWiki, use the @internal tag with a different semantics than the one imposed by deptrac. In the case of MediaWiki, the intended semantics is "not for use by extensions", i.e. "internal to the core repo". To work around this issue, allow the config file to specify which tag deptrac will interpret as "layer-internal". The @deptrac-internal tag will continue to always work. NOTE: This is a breaking change: the "@internal" tag will will now be ignored unless explicitly configured to be used.
qossmic · Dec 20, 2023 · 0fa1193 · 0fa1193
1 parent 3ab320b
commit 0fa1193
Show file tree

Hide file tree

Showing 7 changed files with 123 additions and 38 deletions.
diff --git a/config/services.php b/config/services.php
@@ -340,7 +340,10 @@
         ->tag('kernel.event_subscriber');
     $services
         ->set(DependsOnInternalToken::class)
-        ->tag('kernel.event_subscriber');
+        ->tag('kernel.event_subscriber')
+        ->args([
+            '$config' => param('analyser'),
+        ]);
     $services
         ->set(UnmatchedSkippedViolations::class)
         ->tag('kernel.event_subscriber');

diff --git a/docs/configuration.md b/docs/configuration.md
@@ -20,6 +20,23 @@ The following table shows the available config keys for Deptrac.
 </thead>
 <tbody>
 <tr>
+<td>analyser.internal_tag</td>
+<td>
+Specifies which doc block tag deptrac should use to identify layer-internal class-like tokens.
+The default is <code>"@internal"</code>. May be set to <code>null</code> to disable.
+Note that the tag <code>@deptrac-internal</code> will always be used to identify
+layer-internal class-like tokens, even if this <code>internal_tag</code> is set to <code>null</code>.
+</td>
+<td>
+
+```yaml
+deptrac:
+  analyser:
+    internal_tag: ~
+```
+</td>
+</tr>
+<tr>
 <td>analyser.types</td>
 <td>
 

diff --git a/src/Contract/Config/DeptracConfig.php b/src/Contract/Config/DeptracConfig.php
@@ -21,13 +21,22 @@ final class DeptracConfig implements ConfigBuilderInterface
     private array $formatters = [];
     /** @var array<Ruleset> */
     private array $rulesets = [];
+    /** @var ?string */
+    private ?string $internalTag = null;
     /** @var array<string, EmitterType> */
     private array $analyser = [];
     /** @var array<string, array<string>> */
     private array $skipViolations = [];
     /** @var array<string> */
     private array $excludeFiles = [];
 
+    public function internalTag(?string $tag): self
+    {
+        $this->internalTag = $tag;
+
+        return $this;
+    }
+
     public function analysers(EmitterType ...$types): self
     {
         foreach ($types as $type) {
@@ -109,6 +118,8 @@ public function toArray(): array
             $config['paths'] = $this->paths;
         }
 
+        $config['analyser']['internal_tag'] = $this->internalTag;
+
         if ([] !== $this->analyser) {
             $config['analyser']['types'] = array_map(static fn (EmitterType $emitterType) => $emitterType->value, $this->analyser);
         }

diff --git a/src/Core/Analyser/EventHandler/DependsOnInternalToken.php b/src/Core/Analyser/EventHandler/DependsOnInternalToken.php
@@ -14,7 +14,17 @@
  */
 class DependsOnInternalToken implements ViolationCreatingInterface
 {
-    public function __construct(private readonly EventHelper $eventHelper) {}
+    private ?string $internalTag;
+
+    /**
+     * @param array{internal_tag:string|null, ...} $config
+     */
+    public function __construct(
+        private readonly EventHelper $eventHelper,
+        array $config)
+    {
+        $this->internalTag = $config['internal_tag'];
+    }
 
     public static function getSubscribedEvents()
     {
@@ -29,11 +39,17 @@ public function invoke(ProcessEvent $event): void
         foreach ($event->dependentLayers as $dependentLayer => $_) {
             if ($event->dependerLayer !== $dependentLayer
                 && $event->dependentReference instanceof ClassLikeReference
-                && ($event->dependentReference->hasTag('@deptrac-internal')
-                    || $event->dependentReference->hasTag('@internal'))
             ) {
-                $this->eventHelper->addSkippableViolation($event, $ruleset, $dependentLayer, $this);
-                $event->stopPropagation();
+                $isInternal = $event->dependentReference->hasTag('@deptrac-internal');
+
+                if (!$isInternal && $this->internalTag) {
+                    $isInternal = $event->dependentReference->hasTag($this->internalTag);
+                }
+
+                if ($isInternal) {
+                    $this->eventHelper->addSkippableViolation($event, $ruleset, $dependentLayer, $this);
+                    $event->stopPropagation();
+                }
             }
         }
     }

diff --git a/src/Supportive/DependencyInjection/Configuration.php b/src/Supportive/DependencyInjection/Configuration.php
@@ -207,8 +207,10 @@ private function appendEmitterTypes(ArrayNodeDefinition $node): void
                 ->arrayNode('analyser')
                     ->addDefaultsIfNotSet()
                     ->children()
+                        ->scalarNode('internal_tag')
+                            ->defaultNull()
+                            ->end()
                         ->arrayNode('types')
-                            ->isRequired()
                             ->defaultValue([
                                 EmitterType::CLASS_TOKEN->value,
                                 EmitterType::FUNCTION_TOKEN->value,

diff --git a/tests/Core/Analyser/EventHandler/DependsOnInternalTokenTest.php b/tests/Core/Analyser/EventHandler/DependsOnInternalTokenTest.php
@@ -53,41 +53,41 @@ private function makeEvent(
         return $event;
     }
 
-    public function testInvokeEnabled(): void
+    public function testInvoke(): void
     {
         $helper = new EventHelper([], new LayerProvider([]));
-        $handler = new DependsOnInternalToken($helper);
+        $handler = new DependsOnInternalToken($helper, ['internal_tag' => '@layer-internal']);
 
         $event = $this->makeEvent([], []);
         $handler->invoke($event);
 
         $this->assertFalse(
             $event->isPropagationStopped(),
-            'Propagation should continue if neither reference has the "internal" tag'
+            'Propagation should continue if neither reference has the "layer-internal" tag'
         );
 
-        $event = $this->makeEvent(['@internal' => ['']], []);
+        $event = $this->makeEvent(['@layer-internal' => ['']], []);
         $handler->invoke($event);
 
         $this->assertFalse(
             $event->isPropagationStopped(),
-            'Propagation should continue if only the depender is marked @internal'
+            'Propagation should continue if only the depender is marked @layer-internal'
         );
 
-        $event = $this->makeEvent([], ['@internal' => ['']]);
+        $event = $this->makeEvent([], ['@layer-internal' => ['']]);
         $handler->invoke($event);
 
         $this->assertTrue(
             $event->isPropagationStopped(),
-            'Propagation should be stopped if the dependent is marked @internal'
+            'Propagation should be stopped if the dependent is marked @layer-internal'
         );
 
-        $event = $this->makeEvent([], ['@internal' => ['']], 'DependerLayer');
+        $event = $this->makeEvent([], ['@layer-internal' => ['']], 'DependerLayer');
         $handler->invoke($event);
 
         $this->assertFalse(
             $event->isPropagationStopped(),
-            'Propagation should not be stopped if the dependent is marked @internal '.
+            'Propagation should not be stopped if the dependent is marked @layer-internal '.
             'but dependent is in the same layer'
         );
 
@@ -99,4 +99,26 @@ public function testInvokeEnabled(): void
             'Propagation should be stopped if the dependent is marked @deptrac-internal'
         );
     }
+
+    public function testDefaultInternalTag(): void
+    {
+        $helper = new EventHelper([], new LayerProvider([]));
+        $handler = new DependsOnInternalToken($helper, ['internal_tag' => null]);
+
+        $event = $this->makeEvent([], ['@internal' => ['']]);
+        $handler->invoke($event);
+
+        $this->assertFalse(
+            $event->isPropagationStopped(),
+            'The @internal tag should not be used per default'
+        );
+
+        $event = $this->makeEvent([], ['@deptrac-internal' => ['']]);
+        $handler->invoke($event);
+
+        $this->assertTrue(
+            $event->isPropagationStopped(),
+            'The @deptrac-internal tag should be used per default'
+        );
+    }
 }
diff --git a/tests/Supportive/DependencyInjection/DeptracExtensionTest.php b/tests/Supportive/DependencyInjection/DeptracExtensionTest.php
@@ -17,7 +17,8 @@ final class DeptracExtensionTest extends TestCase
 {
     private ContainerBuilder $container;
     private DeptracExtension $extension;
-    private array $formatterDefaults = [
+
+    private const FORMATTER_DEFAULTS = [
         'graphviz' => [
             'hidden_layers' => [],
             'groups' => [],
@@ -32,6 +33,14 @@ final class DeptracExtensionTest extends TestCase
         ],
     ];
 
+    private const ANALYSER_DEFAULTS = [
+        'internal_tag' => null,
+        'types' => [
+            'class',
+            'function',
+        ],
+    ];
+
     protected function setUp(): void
     {
         parent::setUp();
@@ -51,8 +60,8 @@ public function testDefaults(): void
         self::assertSame([], $this->container->getParameter('layers'));
         self::assertSame([], $this->container->getParameter('ruleset'));
         self::assertSame([], $this->container->getParameter('skip_violations'));
-        self::assertSame($this->formatterDefaults, $this->container->getParameter('formatters'));
-        self::assertSame(['types' => [EmitterType::CLASS_TOKEN->value, EmitterType::FUNCTION_TOKEN->value]], $this->container->getParameter('analyser'));
+        self::assertSame(self::FORMATTER_DEFAULTS, $this->container->getParameter('formatters'));
+        self::assertSame(self::ANALYSER_DEFAULTS, $this->container->getParameter('analyser'));
         self::assertSame(true, $this->container->getParameter('ignore_uncovered_internal_classes'));
     }
 
@@ -69,8 +78,8 @@ public function testDefaultsWithEmptyRoot(): void
         self::assertSame([], $this->container->getParameter('layers'));
         self::assertSame([], $this->container->getParameter('ruleset'));
         self::assertSame([], $this->container->getParameter('skip_violations'));
-        self::assertSame($this->formatterDefaults, $this->container->getParameter('formatters'));
-        self::assertSame(['types' => [EmitterType::CLASS_TOKEN->value, EmitterType::FUNCTION_TOKEN->value]], $this->container->getParameter('analyser'));
+        self::assertSame(self::FORMATTER_DEFAULTS, $this->container->getParameter('formatters'));
+        self::assertSame(self::ANALYSER_DEFAULTS, $this->container->getParameter('analyser'));
         self::assertSame(true, $this->container->getParameter('ignore_uncovered_internal_classes'));
     }
 
@@ -307,10 +316,12 @@ public function testNullAnalyser(): void
             ],
         ];
 
-        $this->expectException(InvalidConfigurationException::class);
-        $this->expectExceptionMessage('The child config "types" under "deptrac.analyser" must be configured.');
-
         $this->extension->load($configs, $this->container);
+
+        self::assertSame(
+            self::ANALYSER_DEFAULTS,
+            $this->container->getParameter('analyser')
+        );
     }
 
     public function testEmptyAnalyser(): void
@@ -321,10 +332,12 @@ public function testEmptyAnalyser(): void
             ],
         ];
 
-        $this->expectException(InvalidConfigurationException::class);
-        $this->expectExceptionMessage('The child config "types" under "deptrac.analyser" must be configured.');
-
         $this->extension->load($configs, $this->container);
+
+        self::assertSame(
+            self::ANALYSER_DEFAULTS,
+            $this->container->getParameter('analyser')
+        );
     }
 
     public function testInvalidAnalyserTypes(): void
@@ -333,7 +346,7 @@ public function testInvalidAnalyserTypes(): void
             'deptrac' => [
                 'analyser' => [
                     'types' => ['invalid'],
-                ],
+                ] + self::ANALYSER_DEFAULTS,
             ],
         ];
 
@@ -349,14 +362,14 @@ public function testNullAnalyserTypes(): void
             'deptrac' => [
                 'analyser' => [
                     'types' => null,
-                ],
+                ] + self::ANALYSER_DEFAULTS,
             ],
         ];
 
         $this->extension->load($configs, $this->container);
 
         self::assertSame(
-            ['types' => []],
+            ['types' => []] + self::ANALYSER_DEFAULTS,
             $this->container->getParameter('analyser')
         );
     }
@@ -367,14 +380,14 @@ public function testEmptyAnalyserTypes(): void
             'deptrac' => [
                 'analyser' => [
                     'types' => [],
-                ],
+                ] + self::ANALYSER_DEFAULTS,
             ],
         ];
 
         $this->extension->load($configs, $this->container);
 
         self::assertSame(
-            ['types' => []],
+            ['types' => []] + self::ANALYSER_DEFAULTS,
             $this->container->getParameter('analyser')
         );
     }
@@ -385,14 +398,15 @@ public function testAnalyserWithDuplicateTypes(): void
             'deptrac' => [
                 'analyser' => [
                     'types' => [EmitterType::CLASS_TOKEN->value, EmitterType::CLASS_TOKEN->value],
-                ],
+                ] + self::ANALYSER_DEFAULTS,
             ],
         ];
 
         $this->extension->load($configs, $this->container);
 
         self::assertSame(
-            ['types' => [EmitterType::CLASS_TOKEN->value, EmitterType::CLASS_TOKEN->value]],
+            ['types' => [EmitterType::CLASS_TOKEN->value, EmitterType::CLASS_TOKEN->value]]
+                + self::ANALYSER_DEFAULTS,
             $this->container->getParameter('analyser')
         );
     }
@@ -422,7 +436,7 @@ public function testGraphvizFormatterWithEmptyNodes(): void
 
         $this->extension->load($configs, $this->container);
 
-        $expectedFormatterConfig = $this->formatterDefaults;
+        $expectedFormatterConfig = self::FORMATTER_DEFAULTS;
         $actualFormatterConfig = $this->container->getParameter('formatters');
 
         krsort($expectedFormatterConfig);
@@ -445,7 +459,7 @@ public function testGraphvizFormatterWithOldPointToGroupsConfig(): void
 
         $this->extension->load($configs, $this->container);
 
-        $expectedFormatterConfig = $this->formatterDefaults;
+        $expectedFormatterConfig = self::FORMATTER_DEFAULTS;
         $expectedFormatterConfig['graphviz'] = [
             'point_to_groups' => true,
             'hidden_layers' => [],
@@ -474,7 +488,7 @@ public function testGraphvizFormattersWithHiddenLayers(): void
 
         $this->extension->load($configs, $this->container);
 
-        $expectedFormatterConfig = $this->formatterDefaults;
+        $expectedFormatterConfig = self::FORMATTER_DEFAULTS;
         $expectedFormatterConfig['graphviz'] = [
             'hidden_layers' => ['Utils'],
             'groups' => [
@@ -499,7 +513,7 @@ public function testCodeclimateFormatterWithEmptyNodes(): void
 
         $this->extension->load($configs, $this->container);
 
-        $expectedFormatterConfig = $this->formatterDefaults;
+        $expectedFormatterConfig = self::FORMATTER_DEFAULTS;
         $actualFormatterConfig = $this->container->getParameter('formatters');
 
         krsort($expectedFormatterConfig);