From 2d804ec9fa36d4521a2b4f46cb7789be5d730c72 Mon Sep 17 00:00:00 2001 From: DerManoMann Date: Mon, 29 Jun 2026 16:47:45 +1200 Subject: [PATCH 1/2] refactor: add Analysis::mergeAnnotations() to ensure registry completeness Processors that create annotations via merge() previously needed to separately call addAnnotation() to register them. This was error-prone and left the registries incomplete. mergeAnnotations() combines both operations, closing the gap between the annotation tree and the flat registry index. Migrated processors: MergeIntoOpenApi, MergeIntoComponents, AugmentSchemas, AugmentParameters, AugmentTags, BuildPaths. Also fixes incorrect 'nested' context value in AugmentParameters ($this -> null). Co-Authored-By: Claude Opus 4.6 --- ...-analysis-registries-and-tree-structure.md | 79 +++++++++++++++++++ src/Analysis.php | 25 ++++++ src/Processors/AugmentParameters.php | 4 +- src/Processors/AugmentSchemas.php | 2 +- src/Processors/AugmentTags.php | 2 +- src/Processors/BuildPaths.php | 2 +- src/Processors/MergeIntoComponents.php | 2 +- src/Processors/MergeIntoOpenApi.php | 2 +- 8 files changed, 111 insertions(+), 7 deletions(-) create mode 100644 docs/adr/002-analysis-registries-and-tree-structure.md diff --git a/docs/adr/002-analysis-registries-and-tree-structure.md b/docs/adr/002-analysis-registries-and-tree-structure.md new file mode 100644 index 000000000..6d93b9f0d --- /dev/null +++ b/docs/adr/002-analysis-registries-and-tree-structure.md @@ -0,0 +1,79 @@ +# ADR-002: Analysis Registries and Tree Structure + +## Status + +Accepted + +## Context + +The OpenAPI specification is represented as a tree of annotation objects rooted at `OpenApi`. Processors need to both query annotations by type ("find all Schemas") and traverse structural relationships ("what refs does this subtree contain"). The system maintains two flat registries alongside the tree to support efficient queries. + +## The Two Registries + +### `$analysis->annotations` (SplObjectStorage) + +A flat index of all annotations keyed by object identity, with Context as the attached value. + +**Populated by:** `Analysis::addAnnotation()`, which recursively registers the annotation and all its nested children at the time of the call. + +**Read by:** +- `getAnnotationsOfType()` — query all annotations of a given class (used by ~15 processors) +- Direct iteration in early-pipeline processors (`MergeIntoOpenApi`, `MergeIntoComponents`, `DocBlockDescriptions`, `CleanUnmerged`) +- Late-pipeline processors (`AugmentRefs`, `CleanUnusedComponents`) +- `Analysis::merged()` / `Analysis::unmerged()` / `Analysis::split()` + +### `$context->annotations` (array) + +A per-source-location list of annotations declared at that context. + +**Populated by:** `Analysis::addAnnotation()`, which appends to the context's annotations array. + +**Read by:** +- `getAnnotationForSource()` — finds the source-declared annotation for a FQDN. Critical for `ExpandClasses`, `ExpandTraits`, `ExpandInterfaces`, `AugmentSchemas`, `AugmentDiscriminators`, and type resolvers. + +## The Annotation Tree + +The annotation tree rooted at `$analysis->openapi` is the source of truth for what appears in the output. Annotations reachable from this root via non-blacklisted properties are serialized and validated. + +## Registry Completeness + +The registries are **complete** — every annotation in the tree is also in the registry. This is guaranteed by the combination of: + +1. **Scanning phase:** The analyser calls `addAnnotation()` on all discovered annotations, which recursively registers their children. +2. **Processing phase:** Processors use `mergeAnnotations()` (or `addAnnotation()` directly) when creating or placing annotations, ensuring new annotations are always registered. + +Since `addAnnotation()` is **idempotent** (early-returns if the annotation already exists), calling it on already-registered annotations is a safe no-op. This eliminates the need for processors to track whether an annotation is "new" or "already known." + +## The `mergeAnnotations` Method + +### Problem + +Previously, processors that created new annotations needed two separate operations: +1. `$parent->merge([$annotation])` — place the annotation in the tree +2. `$analysis->addAnnotation($annotation, ...)` — register it in the index + +This split was error-prone: some processors forgot step 2. + +### Solution + +`Analysis::mergeAnnotations($parent, $annotations, $ignore)` combines both: +1. Calls `$parent->merge($annotations, $ignore)` to place annotations in the tree +2. Calls `addAnnotation()` for each annotation to ensure registration + +### When to use + +| Situation | Method | +|-----------|--------| +| Processor merges annotations into a parent | `$analysis->mergeAnnotations($parent, [...])` | +| Processor creates an annotation and places it directly (not via merge) | `$analysis->addAnnotation($annotation, $context)` + direct assignment | +| Processor relocates an annotation (removes from old parent, adds to new) | `$analysis->removeAnnotation()` + direct assignment + `addAnnotation()` | + +## Decision + +1. Processors must use `$analysis->mergeAnnotations()` when merging annotations into the tree. This guarantees registry completeness without requiring callers to remember a separate registration step. + +2. The registries are complete and can be relied upon for type-based queries and ref scanning. Iterating `$analysis->annotations` is sufficient to find all annotations, including those created by processors. + +3. Processors that need type-based queries should use `getAnnotationsOfType()` — it's correct and efficient. + +4. `AbstractAnnotation::merge()` should not be called directly by processors when `Analysis` is available. Direct `merge()` is acceptable only in contexts where annotations are already registered (e.g., trait methods operating on scan-time annotations without access to Analysis). \ No newline at end of file diff --git a/src/Analysis.php b/src/Analysis.php index d50522e9c..86bce9c24 100644 --- a/src/Analysis.php +++ b/src/Analysis.php @@ -122,6 +122,31 @@ public function addAnnotations(array $annotations, Context $context): void } } + /** + * Merge annotations into a parent and register any new ones in the analysis. + * + * Combines the tree-structural operation (AbstractAnnotation::merge) with + * registry registration (addAnnotation) so callers don't need to handle + * both separately. + * + * @param list $annotations + * @param bool $ignore Ignore unmerged annotations + * + * @return list The unmerged annotations + */ + public function mergeAnnotations(OA\AbstractAnnotation $parent, array $annotations, bool $ignore = false): array + { + $unmerged = $parent->merge($annotations, $ignore); + + foreach ($annotations as $annotation) { + if ($annotation instanceof OA\AbstractAnnotation) { + $this->addAnnotation($annotation, $annotation->_context); + } + } + + return $unmerged; + } + public function addClassDefinition(array $definition): void { $class = $definition['context']->fullyQualifiedName($definition['class']); diff --git a/src/Processors/AugmentParameters.php b/src/Processors/AugmentParameters.php index e13586dac..623bd07f1 100644 --- a/src/Processors/AugmentParameters.php +++ b/src/Processors/AugmentParameters.php @@ -77,7 +77,7 @@ protected function augmentParameters(Analysis $analysis): void $this->generator->getTypeResolver()->augmentSchemaType($analysis, $schema); - $parameter->merge([new OA\Schema([ + $analysis->mergeAnnotations($parameter, [new OA\Schema([ 'type' => $schema->type, 'format' => $schema->format, 'items' => $schema->items, @@ -86,7 +86,7 @@ protected function augmentParameters(Analysis $analysis): void 'anyOf' => $schema->anyOf, 'ref' => $schema->ref, '_context' => new Context([ - 'nested' => $this, + 'nested' => null, 'comment' => null, 'reflector' => $context->reflector, ], $context)]), diff --git a/src/Processors/AugmentSchemas.php b/src/Processors/AugmentSchemas.php index bf3f8a1ce..a8095a3d6 100644 --- a/src/Processors/AugmentSchemas.php +++ b/src/Processors/AugmentSchemas.php @@ -75,7 +75,7 @@ protected function mergeUnmergedProperties(Analysis $analysis): void continue; } - $annotation->merge([$property], true); + $analysis->mergeAnnotations($annotation, [$property], true); break; } } diff --git a/src/Processors/AugmentTags.php b/src/Processors/AugmentTags.php index e9b256648..ac3ab3dbf 100644 --- a/src/Processors/AugmentTags.php +++ b/src/Processors/AugmentTags.php @@ -76,7 +76,7 @@ public function __invoke(Analysis $analysis): void $declatedTagNames = array_keys($declaredTags); foreach ($usedTagNames as $tagName) { if (!in_array($tagName, $declatedTagNames)) { - $analysis->openapi->merge([new OA\Tag([ + $analysis->mergeAnnotations($analysis->openapi, [new OA\Tag([ 'name' => $tagName, 'description' => $this->withDescription ? $tagName diff --git a/src/Processors/BuildPaths.php b/src/Processors/BuildPaths.php index cd9777995..68470869e 100644 --- a/src/Processors/BuildPaths.php +++ b/src/Processors/BuildPaths.php @@ -45,7 +45,7 @@ public function __invoke(Analysis $analysis): void ]); $analysis->addAnnotation($pathItem, $pathItem->_context); } - if ($paths[$operation->path]->merge([$operation])) { + if ($analysis->mergeAnnotations($paths[$operation->path], [$operation])) { $operation->_context->logger->warning('Unable to merge ' . $operation->identity() . ' in ' . $operation->_context); } } diff --git a/src/Processors/MergeIntoComponents.php b/src/Processors/MergeIntoComponents.php index 1a7eb58a2..83591cb97 100644 --- a/src/Processors/MergeIntoComponents.php +++ b/src/Processors/MergeIntoComponents.php @@ -29,7 +29,7 @@ public function __invoke(Analysis $analysis): void && in_array(OA\Components::class, $annotation::$_parents) && false === $annotation->_context->is('nested')) { // A top level annotation. - $components->merge([$annotation], true); + $analysis->mergeAnnotations($components, [$annotation], true); $analysis->openapi->components = $components; } } diff --git a/src/Processors/MergeIntoOpenApi.php b/src/Processors/MergeIntoOpenApi.php index e13909df2..1ad2a6592 100644 --- a/src/Processors/MergeIntoOpenApi.php +++ b/src/Processors/MergeIntoOpenApi.php @@ -106,6 +106,6 @@ public function __invoke(Analysis $analysis): void } } - $openapi->merge($merge, true); + $analysis->mergeAnnotations($openapi, $merge, true); } } From 471cd914c2ae2c6f82b16d01fc65e2c28eb71f3b Mon Sep 17 00:00:00 2001 From: DerManoMann Date: Mon, 29 Jun 2026 16:49:29 +1200 Subject: [PATCH 2/2] fix: standardize nested context value to null in AttributeAnnotationFactory The 'nested' context flag uses null to mean "no parent" and an AbstractAnnotation instance to mean "has parent". The value false was inconsistent with this convention. Co-Authored-By: Claude Opus 4.6 --- src/Analysers/AttributeAnnotationFactory.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Analysers/AttributeAnnotationFactory.php b/src/Analysers/AttributeAnnotationFactory.php index 724714d21..20bb5cc9c 100644 --- a/src/Analysers/AttributeAnnotationFactory.php +++ b/src/Analysers/AttributeAnnotationFactory.php @@ -73,7 +73,7 @@ public function build(\Reflector $reflector, Context $context): array /** @var OA\Property|OAT\Parameter|OA\RequestBody $instance */ $instance = $attribute->newInstance(); $instance->_context = new Context([ - 'nested' => false, + 'nested' => null, 'property' => $rp->getName(), 'reflector' => $rp, ], $context);