From 3b15f82fa4eaefde16b741d5178e4fe90de7047e Mon Sep 17 00:00:00 2001
From: Anders Fugmann <anders@fugmann.net>
Date: Sat, 27 Jan 2024 00:10:46 +0100
Subject: [PATCH] Fix bug with unordered oneof fields

---
 src/plugin/types.ml | 49 +++++++++++++++++++--------------------------
 test/oneof.proto    | 10 +++++++++
 2 files changed, 31 insertions(+), 28 deletions(-)

diff --git a/src/plugin/types.ml b/src/plugin/types.ml
index 203c023..3d2729e 100644
--- a/src/plugin/types.ml
+++ b/src/plugin/types.ml
@@ -528,35 +528,27 @@ let c_of_oneof ~params ~syntax:_ ~scope OneofDescriptorProto.{ name; _ } fields
 (** Return a list of plain fields + a list of fields per oneof_decl *)
 let split_oneof_decl fields oneof_decls =
   let open FieldDescriptorProto in
-  let rec inner oneofs oneof_decls = function
-    | { oneof_index = Some i; _ } as o1 :: fs -> begin
-        match oneofs with
-        | [] -> inner [o1] oneof_decls fs
-        | { oneof_index = Some j; _ } :: _ when i = j ->
-          inner (o1 :: oneofs) oneof_decls fs
-        | oneofs ->
-          `Oneof (List.hd oneof_decls, List.rev oneofs) :: inner [o1] (List.tl oneof_decls) fs
-      end
-    | f :: fs -> begin
-        match oneofs with
-        | [] -> `Field f :: inner [] oneof_decls fs
-        | oneofs ->
-          `Oneof (List.hd oneof_decls, List.rev oneofs) :: `Field f :: inner [] (List.tl oneof_decls) fs
-      end
-    | [] -> begin
-        match oneofs, oneof_decls with
-        | [], [] -> []
-        | oneofs, [oneof_decl] ->
-          [ `Oneof (oneof_decl, List.rev oneofs) ]
-        | _ -> failwith "No field or no oneof"
-      end
+  let rec filter_oneofs acc rest index = function
+    | { oneof_index = Some i; _ } as f :: fs when i = index ->
+      filter_oneofs (f :: acc) rest index fs
+    | f :: fs -> filter_oneofs acc (f :: rest) index fs
+    | [] -> List.rev acc, List.rev rest
   in
-  inner [] oneof_decls fields
+  let rec inner = function
+    | { oneof_index = Some i; _ } as f :: fs ->
+      let oneofs, fs = filter_oneofs [f] [] i fs in
+      let decl = List.nth oneof_decls i in
+      `Oneof (decl, oneofs) :: inner fs
+    | f :: fs ->
+      `Field f :: inner fs
+    | [] -> []
+  in
+  inner fields
 
 let sort_fields fields =
   let number = function
     | FieldDescriptorProto.{ number = Some number; _ } -> number
-    | _ -> failwith "XAll Fields must have a number"
+    | _ -> failwith "All Fields must have a number"
   in
   List.sort ~cmp:(fun v v' -> Int.compare (number v) (number v')) fields
 
@@ -565,10 +557,11 @@ let make ~params ~syntax ~is_cyclic ~is_map_entry ~has_extensions ~scope ~fields
   let ts =
     split_oneof_decl fields oneof_decls
     |> List.map ~f:(function
-        | `Oneof (_, [ FieldDescriptorProto.{ proto3_optional = Some true; _ } as field] )
-        | `Field field -> c_of_field ~params ~syntax ~scope field
-        | `Oneof (decl, fields) -> c_of_oneof ~params ~syntax ~scope decl fields
-      )
+      (* proto3 Oneof fields with only one field is mapped as regular field *)
+      | `Oneof (_, [ FieldDescriptorProto.{ proto3_optional = Some true; _ } as field] )
+      | `Field field -> c_of_field ~params ~syntax ~scope field
+      | `Oneof (decl, fields) -> c_of_oneof ~params ~syntax ~scope decl fields
+    )
   in
 
   let constructor_sig_arg = function
diff --git a/test/oneof.proto b/test/oneof.proto
index c70df6e..022fe58 100644
--- a/test/oneof.proto
+++ b/test/oneof.proto
@@ -50,3 +50,13 @@ message Test5 {
   message Empty { };
   oneof a { Empty e  = 1; };
 }
+
+message Test6 {
+  int64 i = 1;
+  oneof a {
+    int64 a1 = 10;
+    int64 a2 = 21;
+  };
+  int64 j = 20;
+  oneof b { int64 f = 30; };
+}