[Bug] enum and message enum can potentially generator same type names. v1.2.0

[jsmankoo]

Sample Proto File

message Payment {
  enum State {
    /** The payment was created locally. We still need to send it to the processor. */
    PENDING = 1;

    /** The payment was sent to the processor, we are now waiting for the result. */
    WAITING_ON_PROCESSOR = 2;

    /** The payment was successfully completed. */
    COMPLETE = 3;

    /** The payment failed. */
    FAILED = 4;
  }
}

enum PaymentState {
  PAYMENT_NOT_APPLICABLE = 1;

  /** The payment is ready to be processed. */
  PAYMENT_PENDING = 2;

  /** The payment was sent to the processor and we are waiting on the result. */
  PAYMENT_PROCESSING = 4;

  /** The payment was successfully completed. */
  PAYMENT_COMPLETE = 5;

  /** The payment failed. */
  PAYMENT_FAILED = 6;

  /** The payment was refunded. */
  PAYMENT_REFUNDED = 7;
}

Generates following types.ml

[@@@ocaml.warning "-27-30-39"]


type payment_state =
  | Pending 
  | Waiting_on_processor 
  | Complete 
  | Failed 

type payment_state =
  | Payment_not_applicable 
  | Payment_pending 
  | Payment_processing 
  | Payment_complete 
  | Payment_failed 
  | Payment_refunded 

let rec default_payment_state () = (Pending:payment_state)

let rec default_payment_state () = (Payment_not_applicable:payment_state)

causing type error

[jsmankoo]

This could be potentially resolved if all messages were not generated directly as records, but as modules with record named message.
E.g.
Messages will be Payment.message and attached enums become Payment.state

[mransan]

Yes I can see the potential for conflict and how modules would prevent it. Other solution could include a clever detection of conflict and generating a different name... the problem with the module is that the generated code become heavier to read and not sure how to handle recursive data types

[jsmankoo]

the problem with the module is that the generated code become heavier to read and not sure how to handle recursive data types

I've started to realize that as well. Your code becomes much more verbose cause you have to import every type that you are trying to use, instead of being able to just import protobuf types.

[jsmankoo]

I think if we use a name splitter that goes against the convention of protobuf naming style, maybe we can get away with it.

Other solution could include a clever detection of conflict and generating a different name

The only thing I see with this is how do you then tell which name corresponds to which type without having to read the source file over and over again.