guardrail-dev / guardrail

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

Guardrail generates invalid Scala method when operation has two parameters from different sources but with the same name #175

Open dsilvasc opened 5 years ago

dsilvasc commented 5 years ago

Hi, I have an existing server that for a certain path, receives a query parameter and a multi-part form parameter both with the same name (let's call it "foo"), plus other parameters. The relevant part in the swagger spec for that would be:

  /some/path:
    post:
      operationId: "theOperation"
      produces:
      - "application/json"
      parameters:
      - name: "foo"
        in: "formData"
        required: false
        type: "string"
      - name: "foo"
        in: "query"
        required: false
        type: "string"
        enum:
        - "FIRST"
        - "SECOND"
        - "THIRD"

I believe that's valid swagger 2.0.

Guardrail generates the following definition for it:

def theOperation(respond: Resource.theOperationResponse.type)(foo: Option[String] = None, foo: Option[String] = None, ...): scala.concurrent.Future[Resource.theOperationResponse]

This fails to compile because Scala methods can't have two parameters with the same name.

blast-hardcheese commented 5 years ago

So, this is where this validation happens: https://github.com/twilio/guardrail/blob/39072d5a8c77bcf1289199d95779fb1617b4923b/modules/codegen/src/main/scala/com/twilio/guardrail/generators/ScalaParameter.scala

The assumption that was made at the time was that there wouldn't be identically named parameters, only parameters that normalize to the same thing (foo_bar would collide with fooBar after applying the parameter name munging).

I think the cleanest thing in this edge case is to test again, after we've done this first level of name adjustment, then if there are still conflicts defer to an x-scala-name extension in the specification.

If that's not present, we can still fall back to collided names prefixed with which in they are from.