Refactor annotation root comparisons (#1431)

zircote · Mar 17, 2023 · 1661f99 · 1661f99
1 parent e1b1754
commit 1661f99
Show file tree

Hide file tree

Showing 13 changed files with 335 additions and 48 deletions.
diff --git a/src/Analysers/AttributeAnnotationFactory.php b/src/Analysers/AttributeAnnotationFactory.php
@@ -109,7 +109,7 @@ public function build(\Reflector $reflector, Context $context): array
         // merge backwards into parents...
         $isParent = function (OA\AbstractAnnotation $annotation, OA\AbstractAnnotation $possibleParent): bool {
             // regular annotation hierarchy
-            $explicitParent = null !== $possibleParent::matchNested(get_class($annotation)) && !$annotation instanceof OA\Attachable;
+            $explicitParent = null !== $possibleParent->matchNested($annotation) && !$annotation instanceof OA\Attachable;
 
             $isParentAllowed = false;
             // support Attachable subclasses
@@ -126,7 +126,7 @@ public function build(\Reflector $reflector, Context $context): array
             }
 
             // Property can be nested...
-            return get_class($annotation) != get_class($possibleParent)
+            return $annotation->getRoot() != $possibleParent->getRoot()
                 && ($explicitParent || ($isAttachable && $isParentAllowed));
         };
 

diff --git a/src/Annotations/AbstractAnnotation.php b/src/Annotations/AbstractAnnotation.php
@@ -186,7 +186,7 @@ public function merge(array $annotations, bool $ignore = false): array
 
         foreach ($annotations as $annotation) {
             $mapped = false;
-            if ($details = static::matchNested(get_class($annotation))) {
+            if ($details = $this->matchNested($annotation)) {
                 $property = $details->value;
                 if (is_array($property)) {
                     $property = $property[0];
@@ -416,7 +416,7 @@ public function validate(array $stack = [], array $skip = [], string $ref = '',
 
             /** @var class-string<AbstractAnnotation> $class */
             $class = get_class($annotation);
-            if ($details = static::matchNested($class)) {
+            if ($details = $this->matchNested($annotation)) {
                 $property = $details->value;
                 if (is_array($property)) {
                     $this->_context->logger->warning('Only one ' . Util::shorten(get_class($annotation)) . '() allowed for ' . $this->identity() . ' multiple found, skipped: ' . $annotation->_context);
@@ -574,46 +574,50 @@ public function identity(): string
     }
 
     /**
-     * Find matching nested details.
+     * Check if `$other` can be nested and if so return details about where/how.
      *
-     * @param class-string $class the class to match
+     * @param AbstractAnnotation $other the other annotation
      *
      * @return null|object key/value object or `null`
      */
-    public static function matchNested(string $class)
+    public function matchNested($other)
     {
-        if (array_key_exists($class, static::$_nested)) {
-            return (object) ['key' => $class, 'value' => static::$_nested[$class]];
-        }
-
-        $parent = $class;
-        // only consider the immediate OpenApi parent
-        while (0 !== strpos($parent, 'OpenApi\\Annotations\\') && $parent = get_parent_class($parent)) {
-            if ($kvp = static::matchNested($parent)) {
-                return $kvp;
-            }
+        if ($other instanceof AbstractAnnotation && array_key_exists($root = $other->getRoot(), static::$_nested)) {
+            return (object) ['key' => $root, 'value' => static::$_nested[$root]];
         }
 
         return null;
     }
 
     /**
-     * Match the annotation root.
+     * Get the root annotation.
      *
-     * @param class-string $rootClass the root class to match
+     * This is used for resolving type equality and nesting rules to allow those rules to also work for custom,
+     * derived annotation classes.
+     *
+     * @return class-string the root annotation class in the `OpenApi\\Annotations` namespace
      */
-    public function isRoot(string $rootClass): bool
+    public function getRoot(): string
     {
-        $parent = get_class($this);
+        $class = get_class($this);
 
-        // only consider the immediate OpenApi parent
         do {
-            if ($parent == $rootClass) {
-                return true;
+            if (0 === strpos($class, 'OpenApi\\Annotations\\')) {
+                break;
             }
-        } while (0 !== strpos($parent, 'OpenApi\\Annotations\\')  && $parent = get_parent_class($parent));
+        } while ($class = get_parent_class($class));
 
-        return false;
+        return $class;
+    }
+
+    /**
+     * Match the annotation root.
+     *
+     * @param class-string $rootClass the root class to match
+     */
+    public function isRoot(string $rootClass): bool
+    {
+        return $this->getRoot() == $rootClass;
     }
 
     /**

diff --git a/src/Processors/ExpandClasses.php b/src/Processors/ExpandClasses.php
@@ -8,7 +8,6 @@
 
 use OpenApi\Analysis;
 use OpenApi\Annotations as OA;
-use OpenApi\Attributes as OAT;
 use OpenApi\Generator;
 
 /**
@@ -25,7 +24,7 @@ class ExpandClasses implements ProcessorInterface
     public function __invoke(Analysis $analysis)
     {
         /** @var OA\Schema[] $schemas */
-        $schemas = $analysis->getAnnotationsOfType([OA\Schema::class, OAT\Schema::class], true);
+        $schemas = $analysis->getAnnotationsOfType(OA\Schema::class, true);
 
         foreach ($schemas as $schema) {
             if ($schema->_context->is('class')) {

diff --git a/src/Processors/ExpandEnums.php b/src/Processors/ExpandEnums.php
@@ -8,7 +8,6 @@
 
 use OpenApi\Analysis;
 use OpenApi\Annotations as OA;
-use OpenApi\Attributes as OAT;
 use OpenApi\Generator;
 
 /**
@@ -33,7 +32,7 @@ public function __invoke(Analysis $analysis)
     protected function expandContextEnum(Analysis $analysis): void
     {
         /** @var OA\Schema[] $schemas */
-        $schemas = $analysis->getAnnotationsOfType([OA\Schema::class, OAT\Schema::class], true);
+        $schemas = $analysis->getAnnotationsOfType(OA\Schema::class, true);
 
         foreach ($schemas as $schema) {
             if ($schema->_context->is('enum')) {
@@ -63,12 +62,7 @@ protected function expandContextEnum(Analysis $analysis): void
     protected function expandSchemaEnum(Analysis $analysis): void
     {
         /** @var OA\Schema[] $schemas */
-        $schemas = $analysis->getAnnotationsOfType([
-            OA\Schema::class,
-            OAT\Schema::class,
-            OA\ServerVariable::class,
-            OAT\ServerVariable::class,
-        ]);
+        $schemas = $analysis->getAnnotationsOfType([OA\Schema::class, OA\ServerVariable::class]);
 
         foreach ($schemas as $schema) {
             if (Generator::isDefault($schema->enum)) {

diff --git a/src/Processors/ExpandInterfaces.php b/src/Processors/ExpandInterfaces.php
@@ -8,7 +8,6 @@
 
 use OpenApi\Analysis;
 use OpenApi\Annotations as OA;
-use OpenApi\Attributes as OAT;
 use OpenApi\Generator;
 
 /**
@@ -23,7 +22,7 @@ class ExpandInterfaces
     public function __invoke(Analysis $analysis)
     {
         /** @var OA\Schema[] $schemas */
-        $schemas = $analysis->getAnnotationsOfType([OA\Schema::class, OAT\Schema::class], true);
+        $schemas = $analysis->getAnnotationsOfType(OA\Schema::class, true);
 
         foreach ($schemas as $schema) {
             if ($schema->_context->is('class')) {

diff --git a/src/Processors/ExpandTraits.php b/src/Processors/ExpandTraits.php
@@ -8,7 +8,6 @@
 
 use OpenApi\Analysis;
 use OpenApi\Annotations as OA;
-use OpenApi\Attributes as OAT;
 use OpenApi\Generator;
 
 /**
@@ -23,7 +22,7 @@ class ExpandTraits implements ProcessorInterface
     public function __invoke(Analysis $analysis)
     {
         /** @var OA\Schema[] $schemas */
-        $schemas = $analysis->getAnnotationsOfType([OA\Schema::class, OAT\Schema::class], true);
+        $schemas = $analysis->getAnnotationsOfType(OA\Schema::class, true);
 
         // do regular trait inheritance / merge
         foreach ($schemas as $schema) {

diff --git a/src/Processors/MergeIntoComponents.php b/src/Processors/MergeIntoComponents.php
@@ -23,8 +23,11 @@ public function __invoke(Analysis $analysis)
             $components = new OA\Components(['_context' => new Context(['generated' => true], $analysis->context)]);
         }
 
+        /** @var OA\AbstractAnnotation $annotation */
         foreach ($analysis->annotations as $annotation) {
-            if (OA\Components::matchNested(get_class($annotation)) && $annotation->_context->is('nested') === false) {
+            if ($annotation instanceof OA\AbstractAnnotation
+                && in_array(OA\Components::class, $annotation::$_parents)
+                && false === $annotation->_context->is('nested')) {
                 // A top level annotation.
                 $components->merge([$annotation], true);
                 $analysis->openapi->components = $components;

diff --git a/src/Processors/MergeIntoOpenApi.php b/src/Processors/MergeIntoOpenApi.php
@@ -45,7 +45,11 @@ public function __invoke(Analysis $analysis)
                         $openapi->paths[] = $path;
                     }
                 }
-            } elseif (OA\OpenApi::matchNested(get_class($annotation)) && property_exists($annotation, '_context') && $annotation->_context->is('nested') === false) {
+            } elseif (
+                $annotation instanceof OA\AbstractAnnotation
+                && in_array(OA\OpenApi::class, $annotation::$_parents)
+                && property_exists($annotation, '_context')
+                && false === $annotation->_context->is('nested')) {
                 // A top level annotation.
                 $merge[] = $annotation;
             }

diff --git a/tests/Analysers/ReflectionAnalyserTest.php b/tests/Analysers/ReflectionAnalyserTest.php
@@ -14,7 +14,6 @@
 use OpenApi\Analysers\TokenAnalyser;
 use OpenApi\Analysis;
 use OpenApi\Annotations as OA;
-use OpenApi\Attributes as OAT;
 use OpenApi\Context;
 use OpenApi\Generator;
 use OpenApi\Processors\CleanUnusedComponents;
@@ -136,7 +135,7 @@ public function testApiAttributesBasic(AnalyserInterface $analyser): void
 
         // check CustomAttachable is only attached to @OA\Get
         /** @var OA\Get[] $gets */
-        $gets = $analysis->getAnnotationsOfType(OAT\Get::class, true);
+        $gets = $analysis->getAnnotationsOfType(OA\Get::class, true);
         $this->assertCount(2, $gets);
         $this->assertTrue(is_array($gets[0]->attachables), 'Attachables not set');
         $this->assertCount(1, $gets[0]->attachables);

diff --git a/tests/Annotations/AbstractAnnotationTest.php b/tests/Annotations/AbstractAnnotationTest.php
@@ -113,7 +113,6 @@ public function nestedMatches(): iterable
         $parameterMatch = (object) ['key' => OA\Parameter::class, 'value' => ['parameters']];
 
         return [
-            'unknown' => [self::class, null],
             'simple-match' => [OA\Parameter::class, $parameterMatch],
             'invalid-annotation' => [OA\Schema::class, null],
             'sub-annotation' => [SubParameter::class, $parameterMatch],
@@ -127,7 +126,7 @@ public function nestedMatches(): iterable
      */
     public function testMatchNested(string $class, $expected): void
     {
-        $this->assertEquals($expected, OA\Get::matchNested($class));
+        $this->assertEquals($expected, (new OA\Get([]))->matchNested(new $class([])));
     }
 
     public function testDuplicateOperationIdValidation(): void