From 6892b72f997e8690d4aba83b39cb1c0f3310492d Mon Sep 17 00:00:00 2001 From: DerManoMann Date: Sat, 13 Jun 2026 18:26:30 +1200 Subject: [PATCH 1/3] fix: remove consumed annotations from analysis registry Add Analysis::removeAnnotation() and use it in all processors that consume/merge annotations. MergeJsonContent and MergeXmlContent now properly remove the original JsonContent/XmlContent from the analysis after transforming them into MediaType + Schema, preventing orphan annotations from appearing in validation. Replaces all direct $analysis->annotations->offsetUnset() calls with the new method for consistency. Co-Authored-By: Claude Opus 4.6 --- src/Analysis.php | 15 +++++++++++++++ src/Processors/AugmentRefs.php | 2 +- src/Processors/AugmentTags.php | 2 +- src/Processors/BuildPaths.php | 2 +- src/Processors/MergeIntoOpenApi.php | 2 +- src/Processors/MergeJsonContent.php | 1 + src/Processors/MergeXmlContent.php | 1 + 7 files changed, 21 insertions(+), 4 deletions(-) diff --git a/src/Analysis.php b/src/Analysis.php index 1e0d7787a..d50522e9c 100644 --- a/src/Analysis.php +++ b/src/Analysis.php @@ -97,6 +97,21 @@ public function addAnnotation(OA\AbstractAnnotation $annotation, Context $contex } } + public function removeAnnotation(OA\AbstractAnnotation $annotation): void + { + if ($this->annotations->offsetExists($annotation)) { + $context = $this->annotations->offsetGet($annotation); + $this->annotations->offsetUnset($annotation); + + if ($context->is('annotations') !== false) { + $index = array_search($annotation, $context->annotations, true); + if ($index !== false) { + array_splice($context->annotations, $index, 1); + } + } + } + } + /** * @param list $annotations */ diff --git a/src/Processors/AugmentRefs.php b/src/Processors/AugmentRefs.php index cd5080880..f719d7075 100644 --- a/src/Processors/AugmentRefs.php +++ b/src/Processors/AugmentRefs.php @@ -88,7 +88,7 @@ protected function removeDuplicateRefs(Analysis $analysis): void if (!Generator::isDefault($allOfSchema->ref)) { if (in_array($allOfSchema->ref, $refs)) { $dupes[] = $allOfSchema->ref; - $analysis->annotations->offsetUnset($allOfSchema); + $analysis->removeAnnotation($allOfSchema); unset($schema->allOf[$ii]); continue; } diff --git a/src/Processors/AugmentTags.php b/src/Processors/AugmentTags.php index 4a4a8c423..e88b05129 100644 --- a/src/Processors/AugmentTags.php +++ b/src/Processors/AugmentTags.php @@ -104,7 +104,7 @@ private function removeUnusedTags(array $usedTagNames, array $declaredTags, Anal foreach ($declaredTags as $tag) { if (!in_array($tag->name, $tagsToKeep)) { if (false !== $index = array_search($tag, $analysis->openapi->tags, true)) { - $analysis->annotations->offsetUnset($tag); + $analysis->removeAnnotation($tag); unset($analysis->openapi->tags[$index]); $analysis->openapi->tags = array_values($analysis->openapi->tags); } diff --git a/src/Processors/BuildPaths.php b/src/Processors/BuildPaths.php index 1397f4651..cd9777995 100644 --- a/src/Processors/BuildPaths.php +++ b/src/Processors/BuildPaths.php @@ -26,7 +26,7 @@ public function __invoke(Analysis $analysis): void $annotation->_context->logger->warning($annotation->identity() . ' is missing required property "path" in ' . $annotation->_context); } elseif (isset($paths[$annotation->path])) { $paths[$annotation->path]->mergeProperties($annotation); - $analysis->annotations->offsetUnset($annotation); + $analysis->removeAnnotation($annotation); } else { $paths[$annotation->path] = $annotation; } diff --git a/src/Processors/MergeIntoOpenApi.php b/src/Processors/MergeIntoOpenApi.php index 7fafefd2c..e13909df2 100644 --- a/src/Processors/MergeIntoOpenApi.php +++ b/src/Processors/MergeIntoOpenApi.php @@ -98,7 +98,7 @@ public function __invoke(Analysis $analysis): void } } - $analysis->annotations->offsetUnset($components); + $analysis->removeAnnotation($components); } $merge = array_filter($merge, static fn (OA\AbstractAnnotation $annotation): bool => !$annotation instanceof OA\Components); diff --git a/src/Processors/MergeJsonContent.php b/src/Processors/MergeJsonContent.php index 6507f6bb3..726490e85 100644 --- a/src/Processors/MergeJsonContent.php +++ b/src/Processors/MergeJsonContent.php @@ -53,6 +53,7 @@ public function __invoke(Analysis $analysis): void if ($index !== false) { array_splice($parent->_unmerged, $index, 1); } + $analysis->removeAnnotation($jsonContent); } } } diff --git a/src/Processors/MergeXmlContent.php b/src/Processors/MergeXmlContent.php index 76b88bec3..0d7050359 100644 --- a/src/Processors/MergeXmlContent.php +++ b/src/Processors/MergeXmlContent.php @@ -50,6 +50,7 @@ public function __invoke(Analysis $analysis): void if ($index !== false) { array_splice($parent->_unmerged, $index, 1); } + $analysis->removeAnnotation($xmlContent); } } } From be4ac3e8548df5f9db5302230fa50c4d71d2d9bb Mon Sep 17 00:00:00 2001 From: DerManoMann Date: Sat, 13 Jun 2026 19:04:11 +1200 Subject: [PATCH 2/3] fix: update context when relocating JsonContent/XmlContent into MediaType After merge, the consumed annotation is reused as MediaType->schema but its _context->nested still pointed at the old parent. Validation walks the tree via collectAnnotations() and would find the annotation in an unexpected location. Update _context to reflect the new parent. Also adds ADR-001 documenting context keys and the annotation lifecycle contract for processors. Co-Authored-By: Claude Opus 4.6 --- ...1-context-keys-and-annotation-lifecycle.md | 137 ++++++++++++++++++ src/Processors/MergeJsonContent.php | 1 + src/Processors/MergeXmlContent.php | 1 + 3 files changed, 139 insertions(+) create mode 100644 docs/adr/001-context-keys-and-annotation-lifecycle.md diff --git a/docs/adr/001-context-keys-and-annotation-lifecycle.md b/docs/adr/001-context-keys-and-annotation-lifecycle.md new file mode 100644 index 000000000..29b11241e --- /dev/null +++ b/docs/adr/001-context-keys-and-annotation-lifecycle.md @@ -0,0 +1,137 @@ +# ADR-001: Context Keys and Annotation Lifecycle + +## Status + +Accepted + +## Context + +The `Context` class uses dynamic properties (via `__get` with prototypical inheritance) to carry metadata about where an annotation was found or created. Understanding these keys is essential for writing processors that create, move, or remove annotations. + +## Context Keys + +### Source location keys + +Set by the analyser, inherited via the parent chain. + +| Key | Type | Meaning | +|-----|------|---------| +| `filename` | `string` | Absolute path to the PHP file | +| `line` | `int` | Line number | +| `character` | `int` | Column offset | +| `namespace` | `string` | PHP namespace | +| `uses` | `array` | Import aliases (`['Alias' => 'Full\\Class']`) | +| `class` | `string` | Enclosing class name | +| `interface` | `string` | Enclosing interface name | +| `trait` | `string` | Enclosing trait name | +| `enum` | `string` | Enclosing enum name | +| `method` | `string` | Enclosing method name | +| `property` | `string` | Enclosing property name | +| `static` | `bool` | Whether the method/property is static | +| `extends` | `string\|array` | Parent class or interfaces extended | +| `implements` | `array` | Interfaces implemented | +| `comment` | `string` | The raw PHP DocComment | +| `reflector` | `\Reflector` | Reflection object for the element | +| `scanned` | `array` | Details from file scanner (ReflectionAnalyser) | + +### Annotation relationship keys + +| Key | Type | Meaning | +|-----|------|---------| +| `nested` | `AbstractAnnotation\|null` | The parent annotation this one is nested inside. `null` means explicitly not nested (top-level for merge purposes). Absent (not set) means the same as null but via inheritance — `is('nested')` returns `false`. | +| `annotations` | `list` | All annotations registered on this context. Shared by annotations at the same source location. | + +### Processing keys + +| Key | Type | Meaning | +|-----|------|---------| +| `generated` | `bool` | The annotation/context was created by a processor, type resolver, or serializer (not from source scan). | +| `version` | `string` | The OpenAPI version in use (set on root context). | +| `logger` | `LoggerInterface` | PSR logger (guaranteed set when using Generator). | +| `other` | `list` | Non-OpenApi annotations found at this location. | + +## The `nested` Key + +### How `is('nested')` works + +`Context::is()` calls `property_exists()` — it checks whether the property is set directly on this context instance (not inherited from parent). This distinction drives processor behaviour: + +- `is('nested') === true`: The property exists on this context. The annotation has an explicit nesting declaration. +- `is('nested') === false`: The property is not set. MergeIntoOpenApi/MergeIntoComponents treat this as "top-level, merge into root". + +### Values + +| Value | `is('nested')` | Meaning | +|-------|----------------|---------| +| `AbstractAnnotation` instance | `true` | This annotation is a child of that parent | +| `null` | `true` | Explicitly marked as having no parent (e.g. parameter-level attributes that should not be merged into root) | +| *(not set)* | `false` | Top-level — eligible for merge into OpenApi/Components | + +### Where `nested` is set + +1. **`AbstractAnnotation::__construct()`** (line 110): Creates a child context `['nested' => $this]` for annotations passed as constructor properties. + +2. **`AbstractAnnotation::merge()`** (line 156): Same pattern for annotations merged into `_unmerged`. + +3. **`AttributeAnnotationFactory`** (line 76): Sets `'nested' => null` for parameter-level attributes (`#[Property]`, `#[Parameter]`, `#[RequestBody]` on method parameters) that should not be merged into root. + +4. **Processors** (e.g. MergeJsonContent): Should update `_context` when relocating an annotation to reflect its new parent. + +### How processors use `nested` + +- **MergeJsonContent/MergeXmlContent**: Read `$annotation->_context->nested` to find the parent (Response/RequestBody/Parameter) and check `instanceof`. +- **MergeIntoOpenApi/MergeIntoComponents**: Check `$annotation->_context->is('nested') === false` to find top-level annotations eligible for merging into root. + +## Annotation Lifecycle + +### Registration + +`Analysis::addAnnotation($annotation, $context)` registers in two places: +1. `$this->annotations` (`SplObjectStorage`) — keyed by annotation, value is context +2. `$context->annotations[]` — array on the context object + +### Removal + +`Analysis::removeAnnotation($annotation)` removes from both: +1. The `SplObjectStorage` +2. The context's annotations array (context is retrieved from the SplObjectStorage before removal) + +### When processors relocate annotations + +When a processor transforms an annotation (e.g., JsonContent becomes a Schema inside a MediaType), it must: + +1. **Update `_context`** — set a new context with `nested` pointing to the new parent, so tree-walking validation sees it in the correct location. +2. **Remove from parent's `_unmerged`** — so the old parent's validation doesn't warn about unexpected children. +3. **Remove from analysis registry** — via `$analysis->removeAnnotation()` so it's no longer discoverable via `getAnnotationsOfType()` and the old context's annotations array is cleaned up. + +Example (from MergeJsonContent): +```php +// Create the new parent +$mediaType = new OA\MediaType(['schema' => $jsonContent, ...]); + +// Update context to reflect new position in tree +$jsonContent->_context = new Context(['nested' => $mediaType, 'generated' => true], $mediaType->_context); + +// Remove from old parent's _unmerged +array_splice($parent->_unmerged, $index, 1); + +// Remove from analysis registry (cleans up SplObjectStorage + old context->annotations) +$analysis->removeAnnotation($jsonContent); +``` + +## Validation and Tree Walking + +`Analysis::validate()` does NOT iterate the `SplObjectStorage`. It calls `collectAnnotations()` which walks the annotation tree starting from `$this->openapi`, following all non-blacklisted object properties recursively. This means: + +- An annotation removed from the registry but still reachable via the tree **will** be validated. +- The `_context->nested` value determines whether validation considers the annotation correctly placed. +- Annotations with `$_parents = []` (like `JsonContent`) have no valid parent — if still reachable during tree walking, their context must point to a valid parent or they should be transformed into a type that has valid parents. + +## Decision + +Processors that consume/transform annotations must perform full cleanup: +1. Update `_context` to reflect the annotation's new position in the tree +2. Remove from old parent's `_unmerged` +3. Remove from analysis registry via `removeAnnotation()` + +The `nested` context key should use `null` (not `false`) to indicate "explicitly no parent" — this matches the declared `@property OA\AbstractAnnotation|null` type. \ No newline at end of file diff --git a/src/Processors/MergeJsonContent.php b/src/Processors/MergeJsonContent.php index 726490e85..7fb826062 100644 --- a/src/Processors/MergeJsonContent.php +++ b/src/Processors/MergeJsonContent.php @@ -48,6 +48,7 @@ public function __invoke(Analysis $analysis): void $jsonContent->examples = Generator::UNDEFINED; /* @phpstan-ignore assign.propertyType */ $jsonContent->encoding = Generator::UNDEFINED; + $jsonContent->_context = new Context(['nested' => $mediaType, 'generated' => true], $mediaType->_context); $index = array_search($jsonContent, $parent->_unmerged, true); if ($index !== false) { diff --git a/src/Processors/MergeXmlContent.php b/src/Processors/MergeXmlContent.php index 0d7050359..0deef480a 100644 --- a/src/Processors/MergeXmlContent.php +++ b/src/Processors/MergeXmlContent.php @@ -45,6 +45,7 @@ public function __invoke(Analysis $analysis): void } $xmlContent->example = Generator::UNDEFINED; $xmlContent->examples = Generator::UNDEFINED; + $xmlContent->_context = new Context(['nested' => $mediaType, 'generated' => true], $mediaType->_context); $index = array_search($xmlContent, $parent->_unmerged, true); if ($index !== false) { From e17ce5daa55fd231286f552b5f066c064960b5e7 Mon Sep 17 00:00:00 2001 From: DerManoMann Date: Sat, 13 Jun 2026 19:58:40 +1200 Subject: [PATCH 3/3] docs: add guidance on creating annotations in processors to ADR-001 Co-Authored-By: Claude Opus 4.6 --- ...1-context-keys-and-annotation-lifecycle.md | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/docs/adr/001-context-keys-and-annotation-lifecycle.md b/docs/adr/001-context-keys-and-annotation-lifecycle.md index 29b11241e..438dd91f4 100644 --- a/docs/adr/001-context-keys-and-annotation-lifecycle.md +++ b/docs/adr/001-context-keys-and-annotation-lifecycle.md @@ -96,6 +96,26 @@ Set by the analyser, inherited via the parent chain. 1. The `SplObjectStorage` 2. The context's annotations array (context is retrieved from the SplObjectStorage before removal) +### Creating annotations in a processor + +When a processor creates a new annotation, it must register it with the analysis via `addAnnotation()`. This ensures the annotation is discoverable by subsequent processors (via `getAnnotationsOfType()`) and that its context is properly linked into the tree. + +```php +$context = new Context(['nested' => $parent, 'generated' => true], $parent->_context); +$annotation = new OA\Schema(['_context' => $context, ...]); +$analysis->addAnnotation($annotation, $context); +``` + +Key points: + +- **Create a new `Context` for each annotation** — never share a single context instance across multiple annotations. Each annotation needs its own context because `addAnnotation()` appends to `$context->annotations[]`. Sharing a context causes unrelated annotations to appear in each other's context, which confuses validation and cleanup. The context's parent chain provides inheritance of location keys (file, class, method), so per-annotation contexts are lightweight. +- **Always pass `_context`** with `'generated' => true` and `'nested' => $parent` so the annotation is correctly positioned for validation. +- **Call `addAnnotation()`** — this registers in both the `SplObjectStorage` (making it findable by type) and `$context->annotations` (linking it to its source location). +- **Use `$parent->merge([$annotation])`** if the annotation should be nested into the parent's `$_nested` mapping or end up in `_unmerged`. This is the normal path for annotations that the parent "owns". +- **Call `addAnnotation()` directly** (without merge) when the annotation will be consumed by a later processor (e.g., creating a `JsonContent` that `MergeJsonContent` will transform). The later processor is responsible for cleanup. + +If you only call `addAnnotation()` without placing the annotation into the tree (i.e., it's not reachable from the root `OpenApi` object via properties or `_unmerged`), it will be findable via `getAnnotationsOfType()` but won't be validated or serialized. + ### When processors relocate annotations When a processor transforms an annotation (e.g., JsonContent becomes a Schema inside a MediaType), it must: