guardrail-dev / guardrail

Principled code generation from OpenAPI specifications
https://guardrail.dev
MIT License
522 stars 132 forks source link

Traits with discriminator are not sealed #447

Open gaspb opened 4 years ago

gaspb commented 4 years ago

Hello !

When using the following schema :

  Strategy:
    required:
      - type
    discriminator: type
    properties:
      type:
        type: string
  SomeStrategy:
    allOf:
      - $ref: '#/definitions/Strategy'
      - required:
          - type
          - description
        properties:
          type:
            type: string
            enum:
              - SomeStrategy
          description:
            type: string
  AnotherStrategy:
    allOf:
      - $ref: '#/definitions/Strategy'
      - required:
          - type
          - description
        properties:
          type:
            type: string
            enum:
              - AnotherStrategy
          description:
            type: string

I get the generated code :

trait Strategy
object Strategy {
  val discriminator: String = "type"
  implicit val encoder: Encoder[Strategy] = Encoder.instance({
    case e: SomeStrategy =>
      e.asJsonObject.add(discriminator, "SomeStrategy".asJson).asJson
    case e: AnotherStrategy =>
      e.asJsonObject.add(discriminator, "AnotherStrategy".asJson).asJson
  })

which is incorrect, the trait should be sealed or the pattern matching should include case _ =>

blast-hardcheese commented 4 years ago

@gaspb Thanks for the report, this is definitely not great. I think sealed was removed to support allOf.

The work required to re-introduce the sealed constraint would likely involve a mapping function that describes file membership as well as definition order.

Is this something you would be interested in helping with? I can provide some support on getting started, if so.

gaspb commented 4 years ago

@blast-hardcheese I would, i'll have a look this weekend :)

francescopellegrini commented 4 years ago

I've just bumped into this as well.. :(

blast-hardcheese commented 4 years ago

@francescopellegrini Sorry to hear it 😞 Is this something you'd be able to spend some time investigating? I'm nearly done converting the codebase from Free to Tagless Final, I've got a branch that shouldn't change too much over the next week or so if so, blast-hardcheese/tagless-final-pt4