mransan / ocaml-protoc

A Protobuf Compiler for OCaml
https://mransan.github.io/ocaml-protoc/
MIT License
178 stars 33 forks source link

yojson gen_rft_variant extraneous variable "v" in pattern #239

Open yxie2023 opened 8 months ago

yxie2023 commented 8 months ago

Great work and thank you!

In pb_codegen_encode_yojson.ml, gen_rft_variant adds "v" regardless of whether vc_field_type is Vct_nullary or not, which results in errors in generated code:

          F.linep sc "| %s v ->" vc_constructor;

Corrected version should be something like:

let gen_rft_variant sc rf_label { Ot.v_constructors; _ } =
  F.linep sc "let assoc = match v.%s with" rf_label;

  F.sub_scope sc (fun sc ->
      List.iter
        (fun { Ot.vc_constructor; vc_field_type; vc_payload_kind; _ } ->
          let var_name = "v" in
          let json_label =
            Pb_codegen_util.camel_case_of_constructor vc_constructor
          in
          match vc_field_type with
            | Ot.Vct_nullary ->
               F.linep sc "| %s -> (\"%s\", `Null) :: assoc" vc_constructor json_label
            | Ot.Vct_non_nullary_constructor field_type ->
               F.linep sc "| %s v -> " vc_constructor;
               (match
                   gen_field var_name json_label field_type vc_payload_kind
                with
                  | None -> F.linep sc "(\"%s\", `Null) :: assoc" json_label
                  | Some exp -> F.linep sc "%s :: assoc " exp))
        v_constructors);

  F.linep sc "in (* match v.%s *)" rf_label
c-cube commented 8 months ago

Thank you for the report! If you're willing to open a PR with the fix in it I'll be happy to merge it :slightly_smiling_face: . A test case would be even better!