illogical oneOf detection missing

[adamaltman]

Describe the bug

I found an impossible oneOf definition.

I'm constantly trying to educate people about oneOf vs anyOf. In any case, it would be better if the lint tool could catch impossible definitions.

To Reproduce

Most simple reproduction:

oneOf:
  - type: 'object'
  - type: 'object'

Here is the actual place I spotted it:

1. Given this https://github.com/Rebilly/api-definitions/blob/main/openapi/components/schemas/Subscription.yaml#L119-L132 with a oneOf: $ref to InvoiceTimeShift object, null.
2. The problem is that the InvoiceTimeShift object itself has a type of object or null.
3. See error (that there are no lint catch of this problem).

Given that Rebilly is likely to change that before we finish this, here is how to reproduce it.

Excerpt from a schema:

  invoiceTimeShift:
    description: reduced to shorten example
    example: null
    oneOf:
      - $ref: ./InvoiceTimeShift.yaml
      - type: 'null'

Now, let's look at InvoiceTimeShift.yaml (only the first couple of lines needed):

type:
  - 'object'
  - 'null'

Why is this impossible? oneOf needs to match one and only only option. In this case, InvoiceTimeShift can be null, and the second option is also null (both the same). There is no way to distinguish which one was intended. Therefore, this is an impossible oneOf.

Expected behavior

I expected to be alerted to an incorrect oneOf definition. oneOf requires each item must be able to be distinguished from the other items (as opposed to anyOf).

Logs

n/a

OpenAPI description

See link above

Redocly Version(s)

latest (n/a)

Node.js Version(s)

n/a

Additional context

Noticed while debugging something unrelated.

[adamaltman]

no-illogical-schemas could be a potential name for the rule.

We could also check allOf for illogical behavior:

https://json-schema.org/understanding-json-schema/reference/combining#illogicalschemas

[tatomyr]

Thanks for reporting! This looks more like an enhancement though.

[tatomyr]

@adamaltman a question based on the next example from the link you provided:

{
  "oneOf": [
    { "type": "number", "multipleOf": 5 },
    { "type": "number", "multipleOf": 3 }
  ]
}

Wouldn't it be also considered an illogical use of oneOf? The number 15 will be technically valid against both of the subschemas. Is there a way to distinguish an intentional use of oneOf to exclude certain numbers from an example of illogical use?

[adamaltman]

It is indeed illogical too.

If possible values intersect, they are illogical in the context of oneOf.

I don't know what you mean by the intentional use of oneOf to exclude certain numbers? oneOf would not be the appropriate tool to exclude certain numbers. Instead, more detailed schema criteria should be used.

I suspect there is a good amount of the nullable case in the real world (at least for people who adopted 3.1 with nullable). I'm not sure if there are many of this multiple of 3 and 5 type cases in the real world. And I suspect if there is, it would mostly be something like greater or equal to 0 and also something else that includes 0 where 0 is the culprit and could match two schemas.

[tatomyr]

I don't know what you mean by the intentional use of oneOf to exclude certain numbers?

The same example - all multiples of 3 and 5 will be valid against that oneOf except for multiples of 3 AND 5 at the same time. (e.g. 3, 5 - valid, 15 - invalid). So technically a similar trick could be used intentionally.

I'm not sure if there are many of this multiple of 3 and 5 type cases in the real world

Most likely not too much. And the vast majority should be using anyOf instead. But how about the following:

oneOf:
- type: object
  properties: 
    foo: { type: string }
- type: object
  properties:
    bar: { type: number }

Technically any object is invalid against this schema since additionalProperties are true by default and any field will be valid against both schemas. However, I suppose it's pretty common to write schemas like that. Do we want to error on such cases as well?

[adamaltman]

@tatomyr I think the schema is written incorrectly for "I want 15 to be invalid" and the user would need to compose of more complex objects with not multiple of 15. So, I do believe it is still illogical. I'm not sure we need to check for all aspects of illogicality.

Your example with additionalProperties is a common illogical situation. I believe that I would want to know it is illogical, but there are likely people who would like to assume additionalProperties was false even though it isn't false.

I think the rule should take into consideration required properties. Sometimes a missing required property can make the case logical even without additionalProperties set to false.

[tatomyr]

Makes sense. Thanks for the explanation!

[jeremyfiel]

This is a big black hole of linting rules. It would be amazing!! Certainly, there are a million configurable rules you can create for these types of checks, similar to #1318

There has never really existed a good JSON Schema linter, which is something the JSON Schema core group has discussed many times, but without the budget to produce something. I would definitely consider discussing at least some of the bigger schema authoring mistakes they encounter to try and build some rules around them.