Redocly / Redocly/redoc

oneOfs inside allOf produces incorrect "Incompatible types" warning, and lacks location

Open
#2,045 2 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

openapi p3 Type: Bug
Dominant language
TypeScript
Stars
25.9k
Forks
2.4k
Avg merge
13h 10m
Merged PRs (30d)
4

Description

Describe the bug
An allOf that contains overlapping oneOfs produces a confusing and incorrect warning about incompatible types:

Incompatible types in allOf at "undefined": "string" and "object"

Expected behavior
Should not generate warnings for valid schemas

Minimal reproducible OpenAPI snippet

openapi: 3.1.0

info:
  title: bug demo

paths:
  /:
    get:
      responses:
        '200':
          content:
            application/json:
              schema:
                allOf:
                  - oneOf:
                      - type: string
                      - type: object
                  - oneOf:
                      - type: string
                      - type: object

Screenshots
N/A

Additional context

After a brutal debugging session (I was unable to find a way to test this using a non-minified version of redoc, and the minified version is worse than most, because most of the code seems to be inside eval strings or something?) I traced this down from a much more complex real schema to this minimal test case.

It seems like the way hoistOneOfs and mergeAllOf work together is ... not correct. It ends up checking that every possible combination of each oneOf in the allOf is valid, and that is not correct. What I think it should be checking is that some possible combination is valid. The schema is only invalid if no combination is valid, not if any combination is invalid.

Note that, if the allOf branches are replaced with $refs, this sometimes goes away, I think due to short circuiting? It only seems to be happening with "dereferenced" schemas like the example above.

Also, the lack of a proper pointer to where in the schema this came from (at "undefined") drastically increased the time it took to trace this down. While the reformatted schemas it's generating may not have an exact pointer, there's at least a "greatest common ancestor" type value that could be given here I think.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Reproduce the warning with the minimal OpenAPI 3.1 snippet in the issue, then inspect the interaction between hoistOneOfs and mergeAllOf. The fix is done when valid overlapping oneOf branches inside allOf produce no incompatible-types warning and the warning reports a meaningful schema location instead of "undefined".

Written by the indexing model from the issue text.

Assessment

Tech stack
openapi, typescript
Domain
documentation
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.