Redocly / redocly-cli

⚒️ Redocly CLI makes OpenAPI easy. Lint/validate to any standard, generate beautiful docs, and more.
https://redocly.com/docs/cli/
MIT License
908 stars 138 forks source link

lint: False negative for operation-4xx-problem-details-rfc7807 when using allOf #932

Open MHarutunian opened 1 year ago

MHarutunian commented 1 year ago

Describe the bug The operation-4xx-problem-details-rfc7807 rule reports properties type and title as missing when they are inherited using allOf.

To Reproduce Steps to reproduce the behavior:

  1. Given this openapi.yaml file:
    components:
    schemas:
    ProblemDetails:
      description: Problem details based on https://datatracker.ietf.org/doc/html/rfc7807.
      required:
        - title
        - status
      properties:
        type:
          type: string
        title:
          type: string
        status:
          type: integer
          example: 409
    responses:
    # produces "SchemaProperties object should contain `type`/`title` field.", even though the properties will be rendered
    BadRequestResponse:
      content:
        application/problem+json:
          schema:
            allOf:
              - $ref: "#/components/schemas/ProblemDetails"
              - properties:
                  validationErrors:
                    additionalProperties:
                      type: string
    # the following does not produce any errors - using only a $ref without allOf works
    ErrorResponse:
      content:
        application/problem+json:
          schema:
            $ref: "#/components/schemas/ProblemDetails"
  2. Run redocly lint with rule operation-4xx-problem-details-rfc7807 enabled

Expected behavior No error should be produced when using allOf if type and title are present in the resulting schema.

OpenAPI definition See above, using OpenAPI version 3.0.3

Redocly Version(s) 1.0.0-beta.112

Node.js Version(s) Node v16

Hope the information provided helps to narrow down the issue. Please let me know if I can provide any additional details. Thanks!

tatomyr commented 1 year ago

Thanks for reporting this!

adamaltman commented 1 year ago

Shouldn't it be like this though (added a missing "type: object"):

            allOf:
              - $ref: "#/components/schemas/ProblemDetails"
              - type: object
                 properties:
                  validationErrors:
                    additionalProperties:
                      type: string
MHarutunian commented 1 year ago

Thank you for the quick feedback! ~I can see that the error disappears if I add type: object~ (EDIT: this is not true), but up until now my assumption was that type: object would be implied in such cases, e.g. through the presence of properties. I have no idea how I reached that assumption, but I can see that in our specification there are several models that don't explicitly define type: object with a list of properties, and no linter or renderer (including Redocly, Swagger and Microsoft Hidi) had any complaints thus far. So at the very least I think it's a bit confusing that this doesn't cause issues anywhere else, but then produces a somewhat misleading error message on this linting rule.

Looking at the official specification it seems that the type is indeed optional, but it doesn't mention that object would be the default. I checked both OpenAPI and the underlying JSON Schema spec, and they are not particularly clear about this, but it seems that the default would rather be any (allowing any type), so I guess it's not a good idea to leave it out anyway.

EDIT: Actually I messed up, sorry. The error still comes up, even if I add type: object, so while it might be a good idea to add it, it doesn't solve the issue.