Skip to content

Add oneOf + discriminator support - #1249

Open
LiddleDev wants to merge 2 commits into
dedoc:mainfrom
LiddleDev:feat/oneof-discriminator
Open

Add oneOf + discriminator support#1249
LiddleDev wants to merge 2 commits into
dedoc:mainfrom
LiddleDev:feat/oneof-discriminator

Conversation

@LiddleDev

Copy link
Copy Markdown

Add oneOf + discriminator support

Scramble can document allOf and anyOf, but there is currently no way to document a polymorphic (oneOf) schema, and no support for the discriminator object at all. This PR adds both.

Usage

Add #[Discriminator] to the base class (or interface) of the polymorphic type, mapping the discriminator property values to the types they resolve to:

use Dedoc\Scramble\Attributes\Discriminator;

#[Discriminator('petType', ['cat' => Cat::class, 'dog' => Dog::class])]
abstract class Pet
{
    public string $petType;
}
class PetController
{
    public function show(): Pet
    {
        return $this->pets->find(...);
    }
}
Pet:
  oneOf:
    - $ref: '#/components/schemas/Cat'
    - $ref: '#/components/schemas/Dog'
  discriminator:
    propertyName: petType
    mapping:
      cat: '#/components/schemas/Cat'
      dog: '#/components/schemas/Dog'

The mapped types are documented as usual, so this works the same for plain objects, JSON resources, enums-carrying DTOs, etc. The JSON resource case:

#[Discriminator('petType', ['cat' => CatResource::class, 'dog' => DogResource::class])]
abstract class PetResource extends JsonResource {}

When the mapping is given as a list, the mapping key is omitted and the discriminator values are implicitly resolved to the schema names, which is the behavior the specification defines:

#[Discriminator('petType', [Cat::class, Dog::class])]
abstract class Pet {}

A class with #[Discriminator] but no mapped types is documented the usual way, so the attribute is never destructive.

Implementation

  • Support\Generator\Combined\OneOf — the new combined type.
  • Support\Generator\Combined\CombinedTypeAnyOf and AllOf are extracted into this shared base class (their behavior is unchanged), so every combined schema supports a discriminator. This removes the duplication that already existed between the two and 10 baseline entries with it.
  • Support\Generator\Discriminator — the discriminator object. Its mapping holds Reference objects, so mapped $refs are resolved at serialization time and stay in sync with the unique schema names.
  • Support\TypeToSchemaExtensions\DiscriminatedObjectToSchema — the extension building the schema. It is appended to the extensions after the user ones, so an explicit #[Discriminator] attribute wins over inference (see the precedence note below).
  • Attributes\Discriminator — the attribute.
  • Diagnostics\Schema\Se002InvalidDiscriminatorMappingDiagnostic — a mapped class that doesn't exist is skipped and reported instead of silently disappearing from the document. Happy to drop this if you'd rather not spend a diagnostic code on it.

Extension precedence (worth your call)

Type to schema extensions resolve last-registered-first, and user extensions are merged after the built-in ones. An extension that documents a whole class hierarchy — Laravel Data objects, for example — therefore claims an annotated base class, and the discriminator is silently dropped from the document.

Appending DiscriminatedObjectToSchema to the extensions after that merge makes the explicit attribute win, which seemed like the least surprising rule: annotating a class as polymorphic is a deliberate statement, whereas the extension is doing inference. The mapped types are unaffected — they are transformed normally, so a oneOf of Laravel Data objects still gets its member schemas from the extension that owns them.

The trade-off is that an extension can no longer override how an annotated class is documented. If you'd rather keep "last registered always wins", moving the registration into the built-in list is a one-line change, and tests/Attributes/DiscriminatorTest.php covers the behavior either way.

The reason the registration doesn't simply sit in the built-in list with its peers is that array position is the only way to express precedence today, and that list is merged before the user extensions. If you'd rather make precedence explicit, resolution could understand it directly:

$extension = collect($this->typeToSchemaExtensions)
    ->filter(fn ($ext) => ... && $ext->shouldHandle($type))
    ->sortBy(fn ($ext) => $ext instanceof HasPriority ? $ext->priority() : 0)
    ->last();

PHP sorts are stable, so registration order would still decide ties. That is a marker interface plus a line in each of handleUsingExtensions() and handleResponseUsingExtensions(), and the discriminator extension would move back into the list as its first consumer. I left it out because it adds public API that every extension author can then rely on, which is your call rather than something to slip into a feature PR. Happy to add it if you want it.

Notes / open questions

  • Only types resolved to component schemas end up in mapping (an inline schema has no $ref to point at); they still appear in oneOf.
  • The discriminator property is not injected into the mapped schemas — the specification expects it to be a required property of each of them, and that stays the responsibility of the documented types.
  • Union return types (Cat|Dog) still produce anyOf as before. Making them resolve to their discriminated parent is possible, but it felt like something to decide separately from this PR.

Tests

vendor/bin/pest (1214 passing), vendor/bin/pint --test, and vendor/bin/phpstan analyse (no new errors) all pass. New coverage:

  • tests/Support/Generator/CombinedTypesTest.php — serialization of the combined types and the discriminator.
  • tests/Support/TypeToSchemaExtensions/DiscriminatedObjectToSchemaTest.php — explicit and implicit mappings, interfaces, and the no-mapping fallback.
  • tests/Attributes/DiscriminatorTest.php — end-to-end documents for a plain object and a JSON resource hierarchy, plus precedence over an extension registered after the built-in ones.
  • tests/Diagnostics/Schema/Se002InvalidDiscriminatorMappingDiagnosticTest.php.

AI disclosure

I used Claude Code to look over what I changed, write the tests, and help me write this PR body. It also did the Se002InvalidDiscriminatorMappingDiagnostic

Thanks

@LiddleDev
LiddleDev marked this pull request as ready for review August 13, 2026 09:21
@romalytvynenko

Copy link
Copy Markdown
Member

Hey @LiddleDev

Thanks for the PR!

It would be great to use consts for a property that is used by discriminator, since discriminator itself is rather a metadata, not something that affects JSON schema validation:

oneOf:
  - $ref: '#/components/schemas/Cat'
  - $ref: '#/components/schemas/Dog'

Cat:
  type: object
  required: [petType]
  properties:
    petType:
      const: cat

Dog:
  type: object
  required: [petType]
  properties:
    petType:
      const: dog

Will this work for your case?

I would also add warning in case the attribute describes a property that is not present on a referenced type.

Not a request change, just trying to get a better understanding of your use case here.

@LiddleDev

Copy link
Copy Markdown
Author

Hi @romalytvynenko, thanks for looking at this so quickly. I have to admit I don't know loads about openapi, but I've done some more reading on the validation.

Our issue came from our app developer generating code from the API schema, so that's what I've sorted for him here by adding support for a discriminator and oneOf.

I think you're right though, it will fail validation without the consts as a payload could match more than one branch of the oneOf. I've added them in the latest commit. The mapping key gets documented as a const on the mapped type and the property marked required. Worth mentioning that Scramble only infers that const on its own when toArray() returns a literal value, so a plain typed property or a backed enum had nothing to go on, which is why the mapping supplies it. Enum properties get the const alongside the $ref, which is fine for 3.1 I think, would be worth you taking a look though.

Happy to add the warning for a mapped type that's missing the property too if needed :)

Thanks, Scramble makes our lives a lot easier!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants