Haskell-OpenAPI-Code-Generator / Haskell-OpenAPI-Client-Code-Generator

Generate Haskell client code from an OpenAPI 3 specification
46 stars 19 forks source link

Generation not case sensitive, which results in compilation error for generated files #85

Open maridonkers opened 1 year ago

maridonkers commented 1 year ago

For binance-api-swagger/spot_api.yaml the section

 aggTrade:
      type: object
      properties:
        a:
          type: integer
          format: int64
          description: Aggregate tradeId
          example: 26129
        p:
          type: string
          description: Price
          example: "0.01633102"
        q:
          type: string
          description: Quantity
          example: "4.70443515"
        f:
          type: integer
          format: int64
          description: First tradeId
          example: 27781
        l:
          type: integer
          format: int64
          description: Last tradeId
          example: 27781
        T:
          type: boolean
          description: Timestamp
          example: 1498793709153
        m:
          type: boolean
          description: Was the buyer the maker?
        M:
          type: boolean
          description: Was the trade the best price match?

results in compilation error for generated files, as follows:

src/OpenAPI/Types/AggTrade.hs:51:5: error:
    Multiple declarations of ‘aggTradeM’
    Declared at: src/OpenAPI/Types/AggTrade.hs:41:3
                 src/OpenAPI/Types/AggTrade.hs:51:5
   |                   
51 |   , aggTradeM :: GHC.Types.Bool
   |     ^^^^^^^^^  

(the problem apparently originates in the "m" and "M", for which case is not handled)

Generated Haskell code section:

-- | Defines the object schema located at @components.schemas.aggTrade@ in the specification.
-- 
-- 
data AggTrade = AggTrade {
  -- | M: Was the trade the best price match?
  aggTradeM :: GHC.Types.Bool
  -- | T: Timestamp
  , aggTradeT :: GHC.Types.Bool
  -- | a: Aggregate tradeId
  , aggTradeA :: GHC.Int.Int64
  -- | f: First tradeId
  , aggTradeF :: GHC.Int.Int64
  -- | l: Last tradeId
  , aggTradeL :: GHC.Int.Int64
  -- | m: Was the buyer the maker?
  , aggTradeM :: GHC.Types.Bool
  -- | p: Price
  , aggTradeP :: Data.Text.Internal.Text
  -- | q: Quantity
  , aggTradeQ :: Data.Text.Internal.Text
  } deriving (GHC.Show.Show
  , GHC.Classes.Eq)
NorfairKing commented 1 year ago

This is a fair point, but how do you suggest this code to be generated instead?

maridonkers commented 1 year ago

This is a fair point, but how do you suggest this code to be generated instead?

Leading should be — if possible and feasible — that the generated code compiles? BTW: the OpenAPI generator produces Haskell client code with the exact same problem in the generated code...

The (manual) change to the generated for the above compilation error is trivial, so it's not a big problem anyway.

joel-bach commented 1 year ago

I do not know if it is still relevant but if you do not need the AggTrade type you can use --opaque-schema="aggTrade" to exclude it from generation (or rather let it generate as Aeson.Value).

As far as I can see there are two solutions to this problem:

Any thought?

maridonkers commented 1 year ago

I do not know if it is still relevant but if you do not need the AggTrade type you can use --opaque-schema="aggTrade" to exclude it from generation (or rather let it generate as Aeson.Value).

Thanks for the followup; with different options there was only one trivial change required to the generated code. I'm not yet sure if the AggTrade is useful, but it may be. If not then the --opaque-schema is indeed useful.

As far as I can see there are two solutions to this problem:

  • Do less "smart" naming, especially do not uppercase different parts of the property name. The main disadvantage here is that is leads to quite unreadable names all throughout the generated code.
  • Do some deduplication of (property) names. Ideally this should happen on global level but maybe doing it on a per-record-basis would be a first step as well.

Any thought?

My 2 cents would be the deduplication but is it worth the effort? (not sure how common the problem is irl)