diff --git a/Changelog.md b/Changelog.md index 678d572..64a9f2a 100644 --- a/Changelog.md +++ b/Changelog.md @@ -1,5 +1,5 @@ ## 6.1.0: 2024-04-25 -- Fix name resolution leading to wrongly mapped named +- Fix name resolution leading to wrongly mapped names - Fix codegen bug causing the plugin to reject valid protobuf - Add preliminary support for melange though disabling eager evaluation of serialize and deserialize functions when not using @@ -21,7 +21,7 @@ - [x] Fix file output name if files contains a '-' - [x] Resolve bug for Request/Response module aliases leading to generating uncompilable code. (#21) -- [x] Fix codegen bug for messages with out fields and setting +- [x] Fix codegen bug for messages without fields and setting singleton_records = true (#20) - [x] In Services, the package field is now correctly set to None if the service if not defined in a package scope (#24) diff --git a/src/ocaml_protoc_plugin/ocaml_protoc_plugin.ml b/src/ocaml_protoc_plugin/ocaml_protoc_plugin.ml index a76c985..25057e9 100644 --- a/src/ocaml_protoc_plugin/ocaml_protoc_plugin.ml +++ b/src/ocaml_protoc_plugin/ocaml_protoc_plugin.ml @@ -1,14 +1,3 @@ -(**/**) - -module Serialize = Serialize -module Deserialize = Deserialize -module Serialize_json = Serialize_json -module Deserialize_json = Deserialize_json -module Spec = Spec -module Field = Field -module Merge = Merge -(**/**) - module Json = Json module Reader = Reader module Writer = Writer @@ -17,11 +6,28 @@ module Result = Result module Extensions = Extensions module Json_options = Json_options +(**/**) +module Serialize = Serialize +module Deserialize = Deserialize +module Serialize_json = Serialize_json +module Deserialize_json = Deserialize_json +module Spec = Spec +module Field = Field +module Merge = Merge -let apply_lazy f = +let[@inline] apply_lazy f = match Sys.backend_type with | Native | Bytecode -> f () | Other _ -> let f = Lazy.from_fun f in fun x -> (Lazy.force f) x +(**/**) + +type test = + { a: int; (** This is an int *) + b: [`First | `Second]; (** {e b} is [b] + - `First is ok + - `Second is also + *) + } diff --git a/src/plugin/code.ml b/src/plugin/code.ml index 80840c5..e6ea0f4 100644 --- a/src/plugin/code.ml +++ b/src/plugin/code.ml @@ -141,3 +141,34 @@ let emit_comment ~(position:[`Leading | `Trailing]) t = function let contents t = List.map ~f:(Printf.sprintf "%s") (List.rev t.code) |> String.concat ~sep:"\n" + +(** Emit function comments *) +let add_arg_doc t + ~(position:[`Leading | `Trailing]) + ?(format:('a -> 'b, unit, string, unit) format4="param %s") + ?(comment=[]) + param_comments = + + let comments = map_comments comment in + + (* Remove parameters with no comments *) + let param_comments = + List.filter ~f:(fun (_, comments) -> not (List.is_empty comments)) param_comments + in + + match comments, param_comments with + | [], [] -> () + | [comment], [] -> + emit t `None "(** %s *)" (String.trim comment) + | comments, param_comments -> + if position = `Leading then emit t `None ""; + emit t `Begin "(**"; + List.iter ~f:(emit t `None "%s") comments; + emit t `None ""; + List.iter ~f:(fun (param, comments) -> + let comments = map_comments comments in + emit t `Begin format param; + List.iter ~f:(emit t `None "%s") comments; + emit t `None ""; + ) param_comments; + emit t `End "*)" diff --git a/src/plugin/emit.ml b/src/plugin/emit.ml index 406a499..44f3f70 100644 --- a/src/plugin/emit.ml +++ b/src/plugin/emit.ml @@ -166,7 +166,7 @@ let emit_extension ~scope ~params ~comment_db ~type_db field = (* Get spec and type *) let c = let params = Parameters.{params with singleton_record = false} in - Types.spec_of_field ~params ~syntax:`Proto2 ~scope ~type_db ~map_type:None field + Types.spec_of_field ~params ~syntax:`Proto2 ~scope ~type_db ~comment_db ~map_type:None field in let signature = Code.init () in let implementation = Code.init () in @@ -227,6 +227,20 @@ let rec emit_message ~params ~syntax ~scope ~type_db ~comment_db let signature = Code.init () in let implementation = Code.init () in let deprecated = match options with Some { deprecated; _ } -> deprecated | None -> false in + let proto_path = Scope.get_proto_path scope ?name in + + (* Need extensions if specified, added to the list of fields *) + let emit_message_type code ~annot = function + | _, [] -> + Code.emit code `None "type t = unit %s" annot + | `Tuple, (types: Types.c list) -> + let types = List.map ~f:(fun (c: Types.c) -> Types.string_of_type c.type') types in + Code.emit code `None "type t = (%s) %s" (String.concat ~sep:" * " types) annot + | `Record, (types: Types.c list) -> + Code.emit code `Begin "type t = {"; + List.iter ~f:(fun (c: Types.c) -> Code.emit code `None "%s:%s;" (Type_db.get_message_field type_db ~proto_path c.name) (Types.string_of_type c.type')) types; + Code.emit code `End "} %s" annot; + in let extension_ranges = List.map ~f:(function @@ -270,7 +284,7 @@ let rec emit_message ~params ~syntax ~scope ~type_db ~comment_db default_constructor_sig; default_constructor_impl; merge_impl } = Types.make ~params ~syntax ~is_cyclic ~extension_ranges ~scope ~type_db ~comment_db ~fields oneof_decls in - Code.emit signature `None "type t = %s%s" type' params.annot; + emit_message_type signature ~annot:params.annot type'; Code.emit signature `None "val make: %s" default_constructor_sig; Code.emit signature `None "(** Helper function to generate a message using default values *)\n"; @@ -296,7 +310,7 @@ let rec emit_message ~params ~syntax ~scope ~type_db ~comment_db Code.emit signature `None "(**/**)"; Code.emit implementation `None "let name () = \"%s\"" (Scope.get_proto_path scope); - Code.emit implementation `None "type t = %s%s" type' params.annot; + emit_message_type implementation ~annot:params.annot type'; Code.emit implementation `None "type make_t = %s" default_constructor_sig; Code.emit implementation `None "let make %s" default_constructor_impl; diff --git a/src/plugin/type_db.ml b/src/plugin/type_db.ml index fe3f7a8..a398a1d 100644 --- a/src/plugin/type_db.ml +++ b/src/plugin/type_db.ml @@ -5,10 +5,7 @@ open !MoreLabels open !Utils open Spec.Descriptor.Google.Protobuf -(* TODO: - - Understand proto3_optional flags (Used to map oneof fields for some reason) - - Packages should be mapped also, so they can be named correctly -*) + let sprintf = Printf.sprintf type oneof = { name: string; constructor_name: string; type_: string option; } @@ -26,6 +23,9 @@ type element_type = | Package | Extension +(** Name of extensions field *) +let extensions_name = "extensions'" + let string_of_element_type = function | Message _ -> "Message" | Enum _ -> "Enum" @@ -48,7 +48,7 @@ let add_scope ~proto_name ~ocaml_name { proto_path; ocaml_path; module_name; oca { proto_path; ocaml_path; ocaml_name; module_name } let element_of_message ~mangle_f descriptorproto = - let DescriptorProto.{ field = fields; oneof_decl = oneof_decls; options; _ } = descriptorproto in + let DescriptorProto.{ field = fields; oneof_decl = oneof_decls; options; extension_range; _ } = descriptorproto in let map_type = match options with | Some MessageOptions.{ map_entry = Some true; _ } -> Some descriptorproto | _ -> None @@ -68,9 +68,17 @@ let element_of_message ~mangle_f descriptorproto = in let field_name_map = + (* If the message specifies extensions, reserve the name here *) + let name_map = match List.is_empty extension_range with + | true -> StringMap.empty + | false -> + StringMap.singleton extensions_name extensions_name + in + let plain_field_names = List.filter_map ~f:(fun field -> field.FieldDescriptorProto.name) plain_fields in let oneof_names = List.filter_map ~f:(fun field -> field.OneofDescriptorProto.name) oneof_decls in - Names.create_ocaml_mapping ~mangle_f ~name_f:Names.field_name (plain_field_names @ oneof_names) + (* Extend name mapping. The 'extensions_name' should already have been allocated *) + Names.create_ocaml_mapping ~name_map ~mangle_f ~name_f:Names.field_name (plain_field_names @ oneof_names) in (* Need to exclude oneof's where its a proto3 message. In reality, we should not really care. *) let oneofs = @@ -110,11 +118,15 @@ let element_of_message ~mangle_f descriptorproto = | Some FieldDescriptorProto.Type.TYPE_MESSAGE -> type_name | _ -> None in - { name; ocaml_name; }, Plain { type_; } + { name; ocaml_name; }, Plain { type_ } ) plain_fields in - let fields = plain_fields @ oneofs in - (* Interesting that this just returns the scope *) + let fields = + let fields = plain_fields @ oneofs in + match List.is_empty extension_range with + | true -> fields + | false -> ({ name = extensions_name; ocaml_name = extensions_name }, Plain { type_ = None }) :: fields + in Message { map_type; fields } let element_of_enum ~mangle_f EnumDescriptorProto.{ value; _ } = @@ -154,14 +166,12 @@ let rec traverse_message ~mangle_f ~scope map services descriptorproto = (* Scope contains this element *) let message_element = element_of_message ~mangle_f descriptorproto in - (* Extension name should not interfere with other module names, but should still be uniq *) let extension_names = List.filter_map ~f:(fun e -> e.FieldDescriptorProto.name) extensions in let name_map = Names.create_ocaml_mapping ~name_map ~mangle_f ~name_f:Names.module_name extension_names in - (* Add extensions *) let map = List.fold_left ~init:map ~f:(fun map extension -> let proto_name = Option.value_exn ~message:"Enums must have a name" extension.FieldDescriptorProto.name in @@ -278,33 +288,19 @@ let make_module_name ~prefix_module_names ?package name = in Names.module_name_of_proto ?package name -let dump { map; cyclic_set; file_map } = - ignore cyclic_set; - StringMap.iter ~f:(fun ~key ~data -> Printf.eprintf "Module %s: %s\n" key data) file_map; - StringMap.iter ~f:(fun ~key ~data -> Printf.eprintf "Type: %s: %s\n" key (string_of_element_type (snd data))) map; - (* - StringMap.iter ~f:(fun ~key ~data:{ module_name; ocaml_name; element_type } -> - let element_str = match element_type with - | Message { is_map; fields; _ } -> - List.map ~f:(function - | { name; _ }, Plain { type_ } -> sprintf "(%s, %s)" name (Option.value ~default:"" type_) - | { name; _ }, Oneof _ -> name - ) fields - |> String.concat ~sep:"; " - |> sprintf "is_map: %b, [ %s ]" is_map - | _ -> "" - in - match StringSet.mem key cyclic_set with - | true -> Printf.eprintf "Cyclic: %s -> %s.%s : %s\n" key module_name ocaml_name element_str - | false -> () - ) map; - StringMap.iter ~f:(fun ~key ~data:{ module_name; ocaml_name; element_type } -> +let dump { map; cyclic_set = _; file_map } = + let eprintf = Printf.eprintf in + + StringMap.iter ~f:(fun ~key ~data -> eprintf "Module %s: %s\n" key data) file_map; + (* Just traverse everything *) + StringMap.iter ~f:(fun ~key:proto_path ~data:({module_name; _ }, element_type) -> match element_type with - | Message { is_map = true; _ } -> Printf.eprintf "Map: %s -> %s#%s\n" key module_name ocaml_name + | Message { fields; _ } -> + List.iter ~f:(fun ({name; ocaml_name }, _field) -> + eprintf "%s%s.%s -> %s\n" module_name proto_path name ocaml_name + ) fields | _ -> () - ) map; - *) - () + ) map let init ~prefix_module_names (files : FileDescriptorProto.t list) = let map, file_map = List.fold_left ~init:(StringMap.empty, StringMap.empty) ~f:( @@ -316,8 +312,6 @@ let init ~prefix_module_names (files : FileDescriptorProto.t list) = (map, file_map) ) files in - - let cyclic_set = create_cyclic_set map in let t = { map; cyclic_set; file_map } in if false then dump t; diff --git a/src/plugin/types.ml b/src/plugin/types.ml index 470f25c..3a993c4 100644 --- a/src/plugin/types.ml +++ b/src/plugin/types.ml @@ -1,4 +1,6 @@ open StdLabels +open Utils + (* This module is a bit elaborate. The idea is to construct the actual types needed in the spec module. @@ -17,14 +19,18 @@ type type_modifier = | Oneof_type of string * (string * string) list type type' = - { name: string; modifier: type_modifier; deprecated: bool } + { name: string; (** Name of the type, i.e. float *) + modifier: type_modifier; (** Modifier: list, option etc. *) + } type c = { name : string; type' : type'; spec_str: string; + deprecated: bool; (** True if the type is marked as deprecated *) + comments: string list; (** Comments associated with the type. *) } - +(* This should be replace by c - it has the same info (and more) *) type field_spec = { typestr : string; spec_str: string; @@ -32,7 +38,7 @@ type field_spec = { } type t = { - type' : string; + type' : [`Tuple | `Record] * c list; destructor: string; args: string list; spec_str: string; @@ -314,40 +320,40 @@ let string_of_type = function | { name; modifier = List; _ } -> sprintf "%s list" name | { name; modifier = Optional; _ } -> sprintf "%s option" name -let c_of_compound: type a b. deprecated:bool -> string -> (a, b) compound -> c = fun ~deprecated name -> function +let c_of_compound: type a b. deprecated:bool -> comments:string list -> string -> (a, b) compound -> c = fun ~deprecated ~comments name -> function | Basic (index, spec, default) -> let index_string = string_of_index index in let spec_str = sprintf "basic (%s, %s, %s)" index_string (string_of_spec spec) (string_of_proto_type spec default) in let modifier = No_modifier (string_of_default spec default) in - let type' = { name = type_of_spec spec; modifier; deprecated } in - { name; type'; spec_str } + let type' = { name = type_of_spec spec; modifier } in + { name; type'; spec_str; deprecated; comments } | Basic_req (index, spec) -> let index_string = string_of_index index in let spec_str = sprintf "basic_req (%s, %s)" index_string (string_of_spec spec) in - let type' = { name = type_of_spec spec; modifier = Required; deprecated } in - { name; type'; spec_str } + let type' = { name = type_of_spec spec; modifier = Required } in + { name; type'; spec_str; deprecated; comments } | Basic_opt (index, spec) -> let index_string = string_of_index index in let spec_str = sprintf "basic_opt (%s, %s)" index_string (string_of_spec spec) in - let type' = { name = type_of_spec spec; modifier = Optional; deprecated } in - { name; type'; spec_str } + let type' = { name = type_of_spec spec; modifier = Optional } in + { name; type'; spec_str; deprecated; comments } | Repeated (index, spec, packed) -> let index_string = string_of_index index in let spec_str = sprintf "repeated (%s, %s, %s)" index_string (string_of_spec spec) (string_of_packed packed) in - let type' = { name = type_of_spec spec; modifier = List; deprecated } in - { name; type'; spec_str; } + let type' = { name = type_of_spec spec; modifier = List } in + { name; type'; spec_str; deprecated; comments } | Map (index, { key_spec; key_type; value_compound } ) -> let index_string = string_of_index index in let spec_str = sprintf "map (%s, (%s, %s))" index_string key_spec value_compound.spec_str in let type_name = sprintf "(%s * %s)" key_type (string_of_type value_compound.type') in - let type' = { name = type_name; modifier = List; deprecated } in - { name; type'; spec_str; } + let type' = { name = type_name; modifier = List } in + { name; type'; spec_str; deprecated; comments } | Oneof { type'; spec; fields; _ } -> let spec_str = sprintf "oneof (%s)" spec in - let type' = { name = type'; modifier = Oneof_type ({|`not_set|}, fields); deprecated } in - { name; type'; spec_str } + let type' = { name = type'; modifier = Oneof_type ({|`not_set|}, fields) } in + { name; type'; spec_str; deprecated; comments } -let rec c_of_field ~params ~syntax ~scope ~type_db ~map_type field = +let rec c_of_field ~params ~syntax ~scope ~type_db ~comment_db ~map_type field = let open FieldDescriptorProto in let open FieldDescriptorProto.Type in let number = Option.value_exn field.number in @@ -355,6 +361,8 @@ let rec c_of_field ~params ~syntax ~scope ~type_db ~map_type field = let json_name = Option.value_exn field.json_name in let index = (number, name, json_name) in let deprecated = is_deprecated field in + let proto_path = Scope.get_proto_path scope in + let comments = Comment_db.get_field_comments comment_db ~proto_path ~name:json_name in match syntax, field with (* This function cannot handle oneof types *) @@ -373,56 +381,56 @@ let rec c_of_field ~params ~syntax ~scope ~type_db ~map_type field = | _, { label = Some Label.LABEL_OPTIONAL; type' = Some TYPE_MESSAGE; type_name; _ } -> let spec = spec_of_message ~scope ~type_db type_name in Basic_opt (index, spec) - |> c_of_compound ~deprecated name + |> c_of_compound ~deprecated ~comments name (* Required message *) | `Proto2, { label = Some Label.LABEL_REQUIRED; type' = Some TYPE_MESSAGE; type_name; _ } -> let spec = spec_of_message ~scope ~type_db type_name in Basic_req (index, spec) - |> c_of_compound ~deprecated name + |> c_of_compound ~deprecated ~comments name (* Enum under proto2 with a default value *) | `Proto2, { label = Some Label.LABEL_OPTIONAL; type' = Some TYPE_ENUM; type_name; default_value = Some default; _ } -> let spec = spec_of_enum ~scope ~type_db type_name (Some default) in Basic (index, spec, default) - |> c_of_compound ~deprecated name + |> c_of_compound ~deprecated ~comments name (* Enum under proto2 with no default value *) | `Proto2, { label = Some Label.LABEL_OPTIONAL; type' = Some TYPE_ENUM; type_name; default_value = None; _ } -> let spec = spec_of_enum ~scope ~type_db type_name None in Basic_opt (index, spec) - |> c_of_compound ~deprecated name + |> c_of_compound ~deprecated ~comments name (* Required Enum under proto2 *) | `Proto2, { label = Some Label.LABEL_REQUIRED; type' = Some TYPE_ENUM; type_name; _ } -> let spec = spec_of_enum ~scope ~type_db type_name None in Basic_req (index, spec) - |> c_of_compound ~deprecated name + |> c_of_compound ~deprecated ~comments name (* Required fields under proto2 *) | `Proto2, { label = Some Label.LABEL_REQUIRED; type' = Some type'; type_name; _ } -> let Espec spec = spec_of_type ~params ~scope ~type_db type_name None type' in Basic_req (index, spec) - |> c_of_compound ~deprecated name + |> c_of_compound ~deprecated ~comments name (* Proto2 optional fields with a default *) | `Proto2, { label = Some Label.LABEL_OPTIONAL; type' = Some type'; type_name; default_value = Some default; _ } -> let Espec spec = spec_of_type ~params ~scope ~type_db type_name (Some default) type' in let default = make_default spec default in Basic (index, spec, default) - |> c_of_compound ~deprecated name + |> c_of_compound ~deprecated ~comments name (* Proto2 optional fields - no default *) | `Proto2, { label = Some Label.LABEL_OPTIONAL; type' = Some type'; type_name; default_value = None; _ } -> let Espec spec = spec_of_type ~params ~scope ~type_db type_name None type' in Basic_opt (index, spec) - |> c_of_compound ~deprecated name + |> c_of_compound ~deprecated ~comments name (* Proto3 explicitly optional field are mapped as proto2 optional fields *) | _, { label = Some Label.LABEL_OPTIONAL; type' = Some type'; type_name; proto3_optional = Some true; _ } -> let Espec spec = spec_of_type ~params ~scope ~type_db type_name None type' in Basic_opt (index, spec) - |> c_of_compound ~deprecated name + |> c_of_compound ~deprecated ~comments name (* Proto3 enum implicitly optional field *) | `Proto3, { label = Some Label.LABEL_OPTIONAL; type' = Some TYPE_ENUM; type_name; _} -> @@ -433,14 +441,14 @@ let rec c_of_field ~params ~syntax ~scope ~type_db ~map_type field = | _ -> failwith "Must be an enum spec" in Basic (index, spec, default) - |> c_of_compound ~deprecated name + |> c_of_compound ~deprecated ~comments name (* Proto3 implicitly optional field *) | `Proto3, { label = Some Label.LABEL_OPTIONAL; type' = Some type'; type_name; _} -> let Espec spec = spec_of_type ~params ~scope ~type_db type_name None type' in let default = default_of_spec spec in Basic (index, spec, default) - |> c_of_compound ~deprecated name + |> c_of_compound ~deprecated ~comments name (* Repeated fields cannot have a default *) | _, { label = Some Label.LABEL_REPEATED; default_value = Some _; _ } -> failwith "Repeated fields does not support default values" @@ -463,23 +471,23 @@ let rec c_of_field ~params ~syntax ~scope ~type_db ~map_type field = let value_compound = lookup "value" map_type |> Option.value_exn ~message:"Maps must contain a value field" - |> c_of_field ~params ~syntax ~scope ~type_db ~map_type:None + |> c_of_field ~params ~syntax ~scope ~type_db ~comment_db ~map_type:None in Map (index, { key_spec; key_type; value_compound }) (* The spec is not the same here *) - |> c_of_compound ~deprecated name + |> c_of_compound ~deprecated ~comments name (* Repeated message *) | _, { label = Some Label.LABEL_REPEATED; type' = Some Type.TYPE_MESSAGE; type_name; _ } -> let spec = spec_of_message ~scope ~type_db type_name in Repeated (index, spec, Not_packed) - |> c_of_compound ~deprecated name + |> c_of_compound ~deprecated ~comments name (* Repeated bytes and strings are not packed *) | _, { label = Some Label.LABEL_REPEATED; type' = Some (TYPE_STRING | TYPE_BYTES as type'); type_name; _ } -> let Espec spec = spec_of_type ~params ~scope ~type_db type_name None type' in Repeated (index, spec, Not_packed) - |> c_of_compound ~deprecated name + |> c_of_compound ~deprecated ~comments name (* Repeated enum *) | _, { label = Some Label.LABEL_REPEATED; type' = Some Type.TYPE_ENUM; type_name; options; _} -> @@ -491,7 +499,7 @@ let rec c_of_field ~params ~syntax ~scope ~type_db ~map_type field = | `Proto3, _ -> Packed in Repeated (index, spec, packed) - |> c_of_compound ~deprecated name + |> c_of_compound ~deprecated ~comments name (* Repeated basic type *) | _, { label = Some Label.LABEL_REPEATED; type' = Some type'; type_name; options; _} -> @@ -503,20 +511,20 @@ let rec c_of_field ~params ~syntax ~scope ~type_db ~map_type field = | `Proto3, _ -> Packed in Repeated (index, spec, packed) - |> c_of_compound ~deprecated name + |> c_of_compound ~deprecated ~comments name | _, { label = None; _ } -> failwith "Label not set on field struct" | _, { type' = None; _ } -> failwith "Type must be set" -and spec_of_field ~params ~syntax ~scope ~type_db ~map_type field : field_spec = - let c = c_of_field ~params ~syntax ~scope ~type_db ~map_type field in +and spec_of_field ~params ~syntax ~scope ~type_db ~comment_db ~map_type field : field_spec = + let c = c_of_field ~params ~syntax ~scope ~type_db ~comment_db ~map_type field in { typestr = string_of_type c.type'; spec_str = c.spec_str; deprecated = is_deprecated field; } -let c_of_oneof ~params ~syntax:_ ~scope ~type_db OneofDescriptorProto.{ name; _ } fields = +let c_of_oneof ~params ~syntax:_ ~scope ~type_db ~comment_db OneofDescriptorProto.{ name; _ } fields = (* Construct the type. *) let oneof_name = Option.value_exn ~message:"Oneofs must have a name" name in let proto_name = Scope.get_proto_path scope in @@ -576,7 +584,10 @@ let c_of_oneof ~params ~syntax:_ ~scope ~type_db OneofDescriptorProto.{ name; _ Oneof { type'; spec = spec; fields } in - c_of_compound ~deprecated:false (Option.value_exn name) oneof + (* TODO: Construct comments for this oneof element *) + let comments = Comment_db.get_oneof_comments comment_db ~proto_path:proto_name ~name:oneof_name in + + c_of_compound ~deprecated:false ~comments (Option.value_exn name) oneof (** Return a list of plain fields + a list of fields per oneof_decl *) let split_oneof_decl fields oneof_decls = @@ -616,23 +627,33 @@ let append ?(cond=true) elm l = match cond with let make ~params ~syntax ~is_cyclic ~extension_ranges ~scope ~type_db ~comment_db ~fields oneof_decls = let proto_path = Scope.get_proto_path scope in let fields = sort_fields fields in + + let extensions_c = + { name = Type_db.extensions_name; + type' = { name = "Runtime'.Extensions.t"; modifier = No_modifier "Runtime'.Extensions.default"; }; + spec_str = ""; deprecated = false; comments = [] } + in + let ts = split_oneof_decl fields oneof_decls |> List.map ~f:(function (* proto3 Oneof fields with only one field is mapped as regular field *) | `Oneof (_, [ (FieldDescriptorProto.{ proto3_optional = Some true; _ } as field, map_type) ] ) - | `Field (field, map_type) -> c_of_field ~params ~syntax ~scope ~map_type ~type_db field - | `Oneof (decl, fields) -> c_of_oneof ~params ~syntax ~scope ~type_db decl fields + | `Field ( field, map_type) -> + c_of_field ~params ~syntax ~scope ~map_type ~type_db ~comment_db field + | `Oneof (decl, fields) -> + c_of_oneof ~params ~syntax ~scope ~type_db ~comment_db decl fields + ) + |> (fun l -> match List.is_empty extension_ranges with + | false -> l @ [extensions_c] + | true -> l ) in - let has_extensions = match extension_ranges with [] -> false | _ -> true in let field_info = - List.rev_map ~f:(fun { name; type'; _} -> - (Type_db.get_message_field type_db ~proto_path name, (string_of_type type', type'.deprecated, name)) + List.map ~f:(fun ({ name; type'; deprecated; _ }) -> + (Type_db.get_message_field type_db ~proto_path name, (string_of_type type', deprecated, name)) ) ts - |> prepend ~cond:has_extensions ("extensions'", ("Runtime'.Extensions.t", false, "")) - |> List.rev in let t_as_tuple = @@ -643,21 +664,21 @@ let make ~params ~syntax ~is_cyclic ~extension_ranges ~scope ~type_db ~comment_d - the message does not define extensions *) let must_be_record = - List.length field_info > 1 || is_cyclic || params.singleton_record || has_extensions + List.length field_info > 1 || is_cyclic || params.singleton_record || not (List.is_empty extension_ranges) in (* Must be a tuple if there are no fields *) List.length field_info = 0 || not must_be_record in - let has_deprecated_fields = List.exists ~f:(fun ({ type' = { deprecated; _ }; _ }: c) -> deprecated) ts in + let _has_deprecated_fields = List.exists ~f:(fun ({ deprecated; _ }: c) -> deprecated) ts in let constructor_sig_arg c = let field_name = Type_db.get_message_field type_db ~proto_path c.name in match c with - | { type' = { name = type_name; modifier = Required; deprecated = _ }; _ } -> + | { type' = { name = type_name; modifier = Required; }; _ } -> sprintf "%s:%s" field_name type_name - | { type' = { name = type_name; modifier = List; deprecated = _ }; _} -> + | { type' = { name = type_name; modifier = List; }; _} -> sprintf "?%s:%s list" field_name type_name - | { type' = { name = type_name; modifier = (Optional | No_modifier _ | Oneof_type _); deprecated = _ }; _} -> + | { type' = { name = type_name; modifier = (Optional | No_modifier _ | Oneof_type _); }; _} -> sprintf "?%s:%s" field_name type_name in let constructor_arg c = @@ -682,45 +703,17 @@ let make ~params ~syntax ~is_cyclic ~extension_ranges ~scope ~type_db ~comment_d in (* Only add comments if the arity of the tuple is > 1. *) - let tuple_type = - let arity = List.length field_info in - let comments = - List.filter_map ~f:(fun (name, (_, _, proto_name)) -> - let comment_lines = - Comment_db.get_field_comments comment_db ~proto_path:proto_path ~name:proto_name - |> Code.map_comments - in - match (String.concat ~sep:"\n" comment_lines |> String.trim) with - | "" -> None - | comment when arity > 1 -> sprintf "@param %s %s" name comment |> Option.some - | comment -> comment |> Option.some - ) field_info - |> String.concat ~sep:"\n\n" - |> function "" -> "" | comment -> sprintf "\n(**\n%s\n*)\n" comment - in - + let _tuple_type = match field_info = [] with - | true -> "unit" + | true -> ["unit"] | false -> List.map field_info ~f:(fun (_, (type_, _, _proto_name)) -> type_ ) - |> String.concat ~sep:" * " - |> sprintf "(%s)" - |> Code.append_deprecaton_if ~deprecated:has_deprecated_fields `Item - |> fun s -> sprintf "%s%s" s comments in - let type' = match t_as_tuple || field_info = [] with - | true -> tuple_type - | false -> - List.map ~f:(fun (name, (type', deprecated, proto_name)) -> - sprintf "\t%s: %s" name type' - |> Code.append_deprecaton_if ~deprecated `Attribute - |> sprintf "%s;" - |> Code.append_comments - ~comments:(Comment_db.get_field_comments comment_db ~proto_path ~name:proto_name) - ) field_info - |> String.concat ~sep:"\n" - |> sprintf "{\n%s\n}" + let type' = + (match t_as_tuple || field_info = [] with + | true -> `Tuple + | false -> `Record), ts in (* a b c *) @@ -733,7 +726,6 @@ let make ~params ~syntax ~is_cyclic ~extension_ranges ~scope ~type_db ~comment_d let default_constructor_sig = List.rev_map ~f:constructor_sig_arg ts - |> prepend ~cond:has_extensions "?extensions':Runtime'.Extensions.t" |> prepend "unit" |> prepend "t" |> List.rev @@ -742,7 +734,6 @@ let make ~params ~syntax ~is_cyclic ~extension_ranges ~scope ~type_db ~comment_d let default_constructor_impl = let args = List.rev_map ~f:constructor_arg ts - |> prepend ~cond:has_extensions "?(extensions' = Runtime'.Extensions.default)" |> prepend "()" |> List.rev |> String.concat ~sep: " " @@ -751,19 +742,22 @@ let make ~params ~syntax ~is_cyclic ~extension_ranges ~scope ~type_db ~comment_d let constructor = type_destr field_info in sprintf "%s = %s" args constructor in + (* Create the deserialize spec *) let spec_str = - let nil = - match has_extensions with - | true -> - extension_ranges - |> List.map ~f:(fun (start', end') -> sprintf "(%d, %d)" start' end') - |> String.concat ~sep:"; " - |> sprintf "nil_ext [ %s ]" - | false -> "nil" - in - let spec = List.map ~f:(fun (c : c) -> c.spec_str) ts in - String.concat ~sep:" ^:: " (spec @ [nil]) + List.filter_map ~f:(function { spec_str = ""; name = _; _ } -> None | { spec_str; _ } -> Some spec_str) ts + |> (fun spec -> match List.is_empty extension_ranges with + | true -> spec @ ["nil"] + | false -> + let nil_ext = + extension_ranges + |> List.map ~f:(fun (start', end') -> sprintf "(%d, %d)" start' end') + |> String.concat ~sep:"; " + |> sprintf "nil_ext [ %s ]" + in + spec @ [nil_ext] + ) + |> String.concat ~sep:" ^:: " |> sprintf "Runtime'.Spec.( %s )" in @@ -806,7 +800,10 @@ let make ~params ~syntax ~is_cyclic ~extension_ranges ~scope ~type_db ~comment_d |> String.concat ~sep:"\n" in name, definitions, merge - + | { name; spec_str = ""; _ } -> + (* Extensions *) + let name = Type_db.get_message_field type_db ~proto_path name in + (name, [], sprintf "List.append t1%s%s t2%s%s" sep name sep name) | { name; spec_str; _ } -> let name = Type_db.get_message_field type_db ~proto_path name in let alias = sprintf "merge_%s" name in @@ -814,7 +811,6 @@ let make ~params ~syntax ~is_cyclic ~extension_ranges ~scope ~type_db ~comment_d let merge = sprintf "%s t1%s%s t2%s%s" alias sep name sep name in name, [definition], merge ) - |> append ~cond:has_extensions ("extensions'", [], sprintf "List.append t1%sextensions' t2%sextensions'" sep sep) in let constr = match t_as_tuple with diff --git a/src/plugin/types.mli b/src/plugin/types.mli deleted file mode 100644 index 5ce5aae..0000000 --- a/src/plugin/types.mli +++ /dev/null @@ -1,35 +0,0 @@ -open Spec.Descriptor.Google.Protobuf - -type t = { - type': string; - destructor: string; - args: string list; - spec_str: string; - default_constructor_sig: string; - default_constructor_impl: string; - merge_impl: string; -} - -type field_spec = { - typestr : string; - spec_str: string; - deprecated: bool; -} - -val spec_of_field: - params:Parameters.t -> - syntax:[ `Proto2 | `Proto3 ] -> - scope:Scope.t -> - type_db:Type_db.t -> - map_type:DescriptorProto.t option -> - FieldDescriptorProto.t -> field_spec - -val make: - params:Parameters.t -> - syntax:[ `Proto2 | `Proto3 ] -> - is_cyclic: bool -> - extension_ranges: (int*int) list -> - scope:Scope.t -> - type_db:Type_db.t -> - comment_db:Comment_db.t -> - fields:(FieldDescriptorProto.t * DescriptorProto.t option) list -> OneofDescriptorProto.t list -> t diff --git a/test/extensions.proto b/test/extensions.proto index 191020e..b721e31 100644 --- a/test/extensions.proto +++ b/test/extensions.proto @@ -34,3 +34,12 @@ message Bar { extend Bar { optional uint32 z = 131; } + +// Test name clashes for extensions name. +message extensions { + required uint32 extensions = 1; + required uint32 extensions_ = 2; + required uint32 Extensions = 3; + required uint32 Extensions_ = 4; + extensions 100 to 199; // This should have reserved the name. +}