From 89e49fba60e701a8dec3504c3e89a5a9cc2e9fbf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Britto?= Date: Fri, 15 Oct 2021 23:19:49 -0300 Subject: [PATCH 1/4] Fix typespecs generation for repeated and optional fields Optional fields types were not including `nil` as a possible value, now they do. Repeated scalar fields were not getting wrapped in brackets, now they are. --- lib/elixirpb.pb.ex | 2 +- lib/google/compiler/plugin.pb.ex | 22 +-- lib/google/descriptor.pb.ex | 172 +++++++++--------- lib/protobuf/protoc/generator/message.ex | 35 ++-- lib/protobuf/type_util.ex | 7 +- .../protoc/generator/message_test.exs | 6 +- .../protobuf/protoc/proto_gen/extension.pb.ex | 2 +- test/protobuf/protoc/proto_gen/test.pb.ex | 58 +++--- 8 files changed, 150 insertions(+), 154 deletions(-) diff --git a/lib/elixirpb.pb.ex b/lib/elixirpb.pb.ex index 62e05409..484847aa 100644 --- a/lib/elixirpb.pb.ex +++ b/lib/elixirpb.pb.ex @@ -3,7 +3,7 @@ defmodule Elixirpb.FileOptions do use Protobuf, syntax: :proto2 @type t :: %__MODULE__{ - module_prefix: String.t() + module_prefix: String.t() | nil } defstruct [:module_prefix] diff --git a/lib/google/compiler/plugin.pb.ex b/lib/google/compiler/plugin.pb.ex index 1a410092..89803f4d 100644 --- a/lib/google/compiler/plugin.pb.ex +++ b/lib/google/compiler/plugin.pb.ex @@ -12,10 +12,10 @@ defmodule Google.Protobuf.Compiler.Version do use Protobuf, syntax: :proto2 @type t :: %__MODULE__{ - major: integer, - minor: integer, - patch: integer, - suffix: String.t() + major: integer | nil, + minor: integer | nil, + patch: integer | nil, + suffix: String.t() | nil } defstruct [:major, :minor, :patch, :suffix] @@ -33,8 +33,8 @@ defmodule Google.Protobuf.Compiler.CodeGeneratorRequest do use Protobuf, syntax: :proto2 @type t :: %__MODULE__{ - file_to_generate: String.t(), - parameter: String.t(), + file_to_generate: [String.t()], + parameter: String.t() | nil, proto_file: [Google.Protobuf.FileDescriptorProto.t()], compiler_version: Google.Protobuf.Compiler.Version.t() | nil } @@ -54,9 +54,9 @@ defmodule Google.Protobuf.Compiler.CodeGeneratorResponse.File do use Protobuf, syntax: :proto2 @type t :: %__MODULE__{ - name: String.t(), - insertion_point: String.t(), - content: String.t(), + name: String.t() | nil, + insertion_point: String.t() | nil, + content: String.t() | nil, generated_code_info: Google.Protobuf.GeneratedCodeInfo.t() | nil } @@ -75,8 +75,8 @@ defmodule Google.Protobuf.Compiler.CodeGeneratorResponse do use Protobuf, syntax: :proto2 @type t :: %__MODULE__{ - error: String.t(), - supported_features: non_neg_integer, + error: String.t() | nil, + supported_features: non_neg_integer | nil, file: [Google.Protobuf.Compiler.CodeGeneratorResponse.File.t()] } diff --git a/lib/google/descriptor.pb.ex b/lib/google/descriptor.pb.ex index 81f82da1..111bbab3 100644 --- a/lib/google/descriptor.pb.ex +++ b/lib/google/descriptor.pb.ex @@ -113,18 +113,18 @@ defmodule Google.Protobuf.FileDescriptorProto do use Protobuf, syntax: :proto2 @type t :: %__MODULE__{ - name: String.t(), - package: String.t(), - dependency: String.t(), - public_dependency: integer, - weak_dependency: integer, + name: String.t() | nil, + package: String.t() | nil, + dependency: [String.t()], + public_dependency: [integer], + weak_dependency: [integer], message_type: [Google.Protobuf.DescriptorProto.t()], enum_type: [Google.Protobuf.EnumDescriptorProto.t()], service: [Google.Protobuf.ServiceDescriptorProto.t()], extension: [Google.Protobuf.FieldDescriptorProto.t()], options: Google.Protobuf.FileOptions.t() | nil, source_code_info: Google.Protobuf.SourceCodeInfo.t() | nil, - syntax: String.t() + syntax: String.t() | nil } defstruct [ @@ -163,8 +163,8 @@ defmodule Google.Protobuf.DescriptorProto.ExtensionRange do use Protobuf, syntax: :proto2 @type t :: %__MODULE__{ - start: integer, - end: integer, + start: integer | nil, + end: integer | nil, options: Google.Protobuf.ExtensionRangeOptions.t() | nil } @@ -182,8 +182,8 @@ defmodule Google.Protobuf.DescriptorProto.ReservedRange do use Protobuf, syntax: :proto2 @type t :: %__MODULE__{ - start: integer, - end: integer + start: integer | nil, + end: integer | nil } defstruct [:start, :end] @@ -199,7 +199,7 @@ defmodule Google.Protobuf.DescriptorProto do use Protobuf, syntax: :proto2 @type t :: %__MODULE__{ - name: String.t(), + name: String.t() | nil, field: [Google.Protobuf.FieldDescriptorProto.t()], extension: [Google.Protobuf.FieldDescriptorProto.t()], nested_type: [Google.Protobuf.DescriptorProto.t()], @@ -208,7 +208,7 @@ defmodule Google.Protobuf.DescriptorProto do oneof_decl: [Google.Protobuf.OneofDescriptorProto.t()], options: Google.Protobuf.MessageOptions.t() | nil, reserved_range: [Google.Protobuf.DescriptorProto.ReservedRange.t()], - reserved_name: String.t() + reserved_name: [String.t()] } defstruct [ @@ -261,17 +261,17 @@ defmodule Google.Protobuf.FieldDescriptorProto do use Protobuf, syntax: :proto2 @type t :: %__MODULE__{ - name: String.t(), - number: integer, - label: Google.Protobuf.FieldDescriptorProto.Label.t(), - type: Google.Protobuf.FieldDescriptorProto.Type.t(), - type_name: String.t(), - extendee: String.t(), - default_value: String.t(), - oneof_index: integer, - json_name: String.t(), + name: String.t() | nil, + number: integer | nil, + label: Google.Protobuf.FieldDescriptorProto.Label.t() | nil, + type: Google.Protobuf.FieldDescriptorProto.Type.t() | nil, + type_name: String.t() | nil, + extendee: String.t() | nil, + default_value: String.t() | nil, + oneof_index: integer | nil, + json_name: String.t() | nil, options: Google.Protobuf.FieldOptions.t() | nil, - proto3_optional: boolean + proto3_optional: boolean | nil } defstruct [ @@ -308,7 +308,7 @@ defmodule Google.Protobuf.OneofDescriptorProto do use Protobuf, syntax: :proto2 @type t :: %__MODULE__{ - name: String.t(), + name: String.t() | nil, options: Google.Protobuf.OneofOptions.t() | nil } @@ -325,8 +325,8 @@ defmodule Google.Protobuf.EnumDescriptorProto.EnumReservedRange do use Protobuf, syntax: :proto2 @type t :: %__MODULE__{ - start: integer, - end: integer + start: integer | nil, + end: integer | nil } defstruct [:start, :end] @@ -342,11 +342,11 @@ defmodule Google.Protobuf.EnumDescriptorProto do use Protobuf, syntax: :proto2 @type t :: %__MODULE__{ - name: String.t(), + name: String.t() | nil, value: [Google.Protobuf.EnumValueDescriptorProto.t()], options: Google.Protobuf.EnumOptions.t() | nil, reserved_range: [Google.Protobuf.EnumDescriptorProto.EnumReservedRange.t()], - reserved_name: String.t() + reserved_name: [String.t()] } defstruct [:name, :value, :options, :reserved_range, :reserved_name] @@ -369,8 +369,8 @@ defmodule Google.Protobuf.EnumValueDescriptorProto do use Protobuf, syntax: :proto2 @type t :: %__MODULE__{ - name: String.t(), - number: integer, + name: String.t() | nil, + number: integer | nil, options: Google.Protobuf.EnumValueOptions.t() | nil } @@ -388,7 +388,7 @@ defmodule Google.Protobuf.ServiceDescriptorProto do use Protobuf, syntax: :proto2 @type t :: %__MODULE__{ - name: String.t(), + name: String.t() | nil, method: [Google.Protobuf.MethodDescriptorProto.t()], options: Google.Protobuf.ServiceOptions.t() | nil } @@ -407,12 +407,12 @@ defmodule Google.Protobuf.MethodDescriptorProto do use Protobuf, syntax: :proto2 @type t :: %__MODULE__{ - name: String.t(), - input_type: String.t(), - output_type: String.t(), + name: String.t() | nil, + input_type: String.t() | nil, + output_type: String.t() | nil, options: Google.Protobuf.MethodOptions.t() | nil, - client_streaming: boolean, - server_streaming: boolean + client_streaming: boolean | nil, + server_streaming: boolean | nil } defstruct [:name, :input_type, :output_type, :options, :client_streaming, :server_streaming] @@ -432,26 +432,26 @@ defmodule Google.Protobuf.FileOptions do use Protobuf, syntax: :proto2 @type t :: %__MODULE__{ - java_package: String.t(), - java_outer_classname: String.t(), - java_multiple_files: boolean, - java_generate_equals_and_hash: boolean, - java_string_check_utf8: boolean, - optimize_for: Google.Protobuf.FileOptions.OptimizeMode.t(), - go_package: String.t(), - cc_generic_services: boolean, - java_generic_services: boolean, - py_generic_services: boolean, - php_generic_services: boolean, - deprecated: boolean, - cc_enable_arenas: boolean, - objc_class_prefix: String.t(), - csharp_namespace: String.t(), - swift_prefix: String.t(), - php_class_prefix: String.t(), - php_namespace: String.t(), - php_metadata_namespace: String.t(), - ruby_package: String.t(), + java_package: String.t() | nil, + java_outer_classname: String.t() | nil, + java_multiple_files: boolean | nil, + java_generate_equals_and_hash: boolean | nil, + java_string_check_utf8: boolean | nil, + optimize_for: Google.Protobuf.FileOptions.OptimizeMode.t() | nil, + go_package: String.t() | nil, + cc_generic_services: boolean | nil, + java_generic_services: boolean | nil, + py_generic_services: boolean | nil, + php_generic_services: boolean | nil, + deprecated: boolean | nil, + cc_enable_arenas: boolean | nil, + objc_class_prefix: String.t() | nil, + csharp_namespace: String.t() | nil, + swift_prefix: String.t() | nil, + php_class_prefix: String.t() | nil, + php_namespace: String.t() | nil, + php_metadata_namespace: String.t() | nil, + ruby_package: String.t() | nil, uninterpreted_option: [Google.Protobuf.UninterpretedOption.t()], __pb_extensions__: map } @@ -519,10 +519,10 @@ defmodule Google.Protobuf.MessageOptions do use Protobuf, syntax: :proto2 @type t :: %__MODULE__{ - message_set_wire_format: boolean, - no_standard_descriptor_accessor: boolean, - deprecated: boolean, - map_entry: boolean, + message_set_wire_format: boolean | nil, + no_standard_descriptor_accessor: boolean | nil, + deprecated: boolean | nil, + map_entry: boolean | nil, uninterpreted_option: [Google.Protobuf.UninterpretedOption.t()], __pb_extensions__: map } @@ -552,12 +552,12 @@ defmodule Google.Protobuf.FieldOptions do use Protobuf, syntax: :proto2 @type t :: %__MODULE__{ - ctype: Google.Protobuf.FieldOptions.CType.t(), - packed: boolean, - jstype: Google.Protobuf.FieldOptions.JSType.t(), - lazy: boolean, - deprecated: boolean, - weak: boolean, + ctype: Google.Protobuf.FieldOptions.CType.t() | nil, + packed: boolean | nil, + jstype: Google.Protobuf.FieldOptions.JSType.t() | nil, + lazy: boolean | nil, + deprecated: boolean | nil, + weak: boolean | nil, uninterpreted_option: [Google.Protobuf.UninterpretedOption.t()], __pb_extensions__: map } @@ -620,8 +620,8 @@ defmodule Google.Protobuf.EnumOptions do use Protobuf, syntax: :proto2 @type t :: %__MODULE__{ - allow_alias: boolean, - deprecated: boolean, + allow_alias: boolean | nil, + deprecated: boolean | nil, uninterpreted_option: [Google.Protobuf.UninterpretedOption.t()], __pb_extensions__: map } @@ -642,7 +642,7 @@ defmodule Google.Protobuf.EnumValueOptions do use Protobuf, syntax: :proto2 @type t :: %__MODULE__{ - deprecated: boolean, + deprecated: boolean | nil, uninterpreted_option: [Google.Protobuf.UninterpretedOption.t()], __pb_extensions__: map } @@ -662,7 +662,7 @@ defmodule Google.Protobuf.ServiceOptions do use Protobuf, syntax: :proto2 @type t :: %__MODULE__{ - deprecated: boolean, + deprecated: boolean | nil, uninterpreted_option: [Google.Protobuf.UninterpretedOption.t()], __pb_extensions__: map } @@ -682,8 +682,8 @@ defmodule Google.Protobuf.MethodOptions do use Protobuf, syntax: :proto2 @type t :: %__MODULE__{ - deprecated: boolean, - idempotency_level: Google.Protobuf.MethodOptions.IdempotencyLevel.t(), + deprecated: boolean | nil, + idempotency_level: Google.Protobuf.MethodOptions.IdempotencyLevel.t() | nil, uninterpreted_option: [Google.Protobuf.UninterpretedOption.t()], __pb_extensions__: map } @@ -728,12 +728,12 @@ defmodule Google.Protobuf.UninterpretedOption do @type t :: %__MODULE__{ name: [Google.Protobuf.UninterpretedOption.NamePart.t()], - identifier_value: String.t(), - positive_int_value: non_neg_integer, - negative_int_value: integer, - double_value: float | :infinity | :negative_infinity | :nan, - string_value: binary, - aggregate_value: String.t() + identifier_value: String.t() | nil, + positive_int_value: non_neg_integer | nil, + negative_int_value: integer | nil, + double_value: float | :infinity | :negative_infinity | :nan | nil, + string_value: binary | nil, + aggregate_value: String.t() | nil } defstruct [ @@ -762,11 +762,11 @@ defmodule Google.Protobuf.SourceCodeInfo.Location do use Protobuf, syntax: :proto2 @type t :: %__MODULE__{ - path: integer, - span: integer, - leading_comments: String.t(), - trailing_comments: String.t(), - leading_detached_comments: String.t() + path: [integer], + span: [integer], + leading_comments: String.t() | nil, + trailing_comments: String.t() | nil, + leading_detached_comments: [String.t()] } defstruct [:path, :span, :leading_comments, :trailing_comments, :leading_detached_comments] @@ -800,10 +800,10 @@ defmodule Google.Protobuf.GeneratedCodeInfo.Annotation do use Protobuf, syntax: :proto2 @type t :: %__MODULE__{ - path: integer, - source_file: String.t(), - begin: integer, - end: integer + path: [integer], + source_file: String.t() | nil, + begin: integer | nil, + end: integer | nil } defstruct [:path, :source_file, :begin, :end] diff --git a/lib/protobuf/protoc/generator/message.ex b/lib/protobuf/protoc/generator/message.ex index 13b936fe..3093affe 100644 --- a/lib/protobuf/protoc/generator/message.ex +++ b/lib/protobuf/protoc/generator/message.ex @@ -159,29 +159,28 @@ defmodule Protobuf.Protoc.Generator.Message do String.pad_trailing("#{name}:", len + 1) end - defp fmt_type(%{opts: %{map: true}, map: {{k_type, k_name}, {v_type, v_name}}}) do - k_type = type_to_spec(k_type, k_name) - v_type = type_to_spec(v_type, v_name) - "%{#{k_type} => #{v_type}}" - end - - defp fmt_type(%{label: "repeated", type_enum: type_enum, type: type}) do - type_to_spec(type_enum, type, true) - end + defp fmt_type(%{opts: %{map: true}, map: {{k_type_enum, _k_type}, {v_type_enum, v_type}}}) do + # Keys are guaranteed to be scalars. + # Values can be anything except another map and are considered optional. + k_spec = TypeUtil.enum_to_spec(k_type_enum) + v_spec = type_to_spec(v_type_enum, v_type) - defp fmt_type(%{type_enum: type_enum, type: type}) do - "#{type_to_spec(type_enum, type)}" + "%{#{k_spec} => #{v_spec} | nil}" end - defp type_to_spec(enum, type, repeated \\ false) + defp fmt_type(%{label: label, type_enum: type_enum, type: type}) do + spec = type_to_spec(type_enum, type) - defp type_to_spec(:TYPE_MESSAGE, type, repeated), - do: TypeUtil.enum_to_spec(:TYPE_MESSAGE, type, repeated) - - defp type_to_spec(:TYPE_ENUM, type, repeated), - do: TypeUtil.enum_to_spec(:TYPE_ENUM, type, repeated) + case label do + "optional" -> "#{spec} | nil" + "required" -> spec + "repeated" -> "[#{spec}]" + end + end - defp type_to_spec(enum, _, _), do: TypeUtil.enum_to_spec(enum) + defp type_to_spec(:TYPE_MESSAGE, type), do: "#{type}.t" + defp type_to_spec(:TYPE_ENUM, type), do: "#{type}.t" + defp type_to_spec(type_scalar, _type), do: TypeUtil.enum_to_spec(type_scalar) def get_fields(ctx, desc) do oneofs = Enum.map(desc.oneof_decl, & &1.name) diff --git a/lib/protobuf/type_util.ex b/lib/protobuf/type_util.ex index 8057623b..094ff6ce 100644 --- a/lib/protobuf/type_util.ex +++ b/lib/protobuf/type_util.ex @@ -28,6 +28,8 @@ defmodule Protobuf.TypeUtil do def enum_to_spec(:TYPE_FIXED32), do: "non_neg_integer" def enum_to_spec(:TYPE_BOOL), do: "boolean" def enum_to_spec(:TYPE_STRING), do: "String.t" + def enum_to_spec(:TYPE_GROUP), do: "any" + def enum_to_spec(:TYPE_MESSAGE), do: "binary" def enum_to_spec(:TYPE_BYTES), do: "binary" def enum_to_spec(:TYPE_UINT32), do: "non_neg_integer" def enum_to_spec(:TYPE_ENUM), do: "integer" @@ -35,9 +37,4 @@ defmodule Protobuf.TypeUtil do def enum_to_spec(:TYPE_SFIXED64), do: "integer" def enum_to_spec(:TYPE_SINT32), do: "integer" def enum_to_spec(:TYPE_SINT64), do: "integer" - def enum_to_spec(_), do: "any" - def enum_to_spec(:TYPE_MESSAGE, type, true = _repeated), do: "[#{type}.t]" - def enum_to_spec(:TYPE_MESSAGE, type, false = _repeated), do: "#{type}.t | nil" - def enum_to_spec(:TYPE_ENUM, type, true = _repeated), do: "[#{type}.t]" - def enum_to_spec(:TYPE_ENUM, type, false = _repeated), do: "#{type}.t" end diff --git a/test/protobuf/protoc/generator/message_test.exs b/test/protobuf/protoc/generator/message_test.exs index 7f154045..a104e1d8 100644 --- a/test/protobuf/protoc/generator/message_test.exs +++ b/test/protobuf/protoc/generator/message_test.exs @@ -460,9 +460,9 @@ defmodule Protobuf.Protoc.Generator.MessageTest do ) {[], [msg]} = Generator.generate(ctx, desc) - assert msg =~ "first: {:a, integer} | {:b, integer},\n" - assert msg =~ "second: {:c, integer} | {:d, integer},\n" - assert msg =~ "other: integer\n" + assert msg =~ "first: {:a, integer | nil} | {:b, integer | nil},\n" + assert msg =~ "second: {:c, integer | nil} | {:d, integer | nil},\n" + assert msg =~ "other: integer | nil\n" refute msg =~ "a: integer,\n" assert msg =~ "defstruct [:first, :second, :other]\n" assert msg =~ "oneof :first, 0\n" diff --git a/test/protobuf/protoc/proto_gen/extension.pb.ex b/test/protobuf/protoc/proto_gen/extension.pb.ex index f1ab2816..7ce6ac51 100644 --- a/test/protobuf/protoc/proto_gen/extension.pb.ex +++ b/test/protobuf/protoc/proto_gen/extension.pb.ex @@ -3,7 +3,7 @@ defmodule Protobuf.Protoc.ExtTest.Foo do use Protobuf, syntax: :proto2 @type t :: %__MODULE__{ - a: String.t() + a: String.t() | nil } defstruct [:a] diff --git a/test/protobuf/protoc/proto_gen/test.pb.ex b/test/protobuf/protoc/proto_gen/test.pb.ex index 9be7590e..4cc15d01 100644 --- a/test/protobuf/protoc/proto_gen/test.pb.ex +++ b/test/protobuf/protoc/proto_gen/test.pb.ex @@ -41,7 +41,7 @@ defmodule My.Test.Request.SomeGroup do use Protobuf, syntax: :proto2 @type t :: %__MODULE__{ - group_field: integer + group_field: integer | nil } defstruct [:group_field] @@ -56,8 +56,8 @@ defmodule My.Test.Request.NameMappingEntry do use Protobuf, map: true, syntax: :proto2 @type t :: %__MODULE__{ - key: integer, - value: String.t() + key: integer | nil, + value: String.t() | nil } defstruct [:key, :value] @@ -73,7 +73,7 @@ defmodule My.Test.Request.MsgMappingEntry do use Protobuf, map: true, syntax: :proto2 @type t :: %__MODULE__{ - key: integer, + key: integer | nil, value: My.Test.Reply.t() | nil } @@ -90,15 +90,15 @@ defmodule My.Test.Request do use Protobuf, syntax: :proto2 @type t :: %__MODULE__{ - key: integer, - hue: My.Test.Request.Color.t(), - hat: My.Test.HatType.t(), - deadline: float | :infinity | :negative_infinity | :nan, - somegroup: any, - name_mapping: %{integer => String.t()}, + key: [integer], + hue: My.Test.Request.Color.t() | nil, + hat: My.Test.HatType.t() | nil, + deadline: float | :infinity | :negative_infinity | :nan | nil, + somegroup: any | nil, + name_mapping: %{integer => String.t() | nil}, msg_mapping: %{integer => My.Test.Reply.t() | nil}, - reset: integer, - get_key: String.t() + reset: integer | nil, + get_key: String.t() | nil } defstruct [ @@ -132,8 +132,8 @@ defmodule My.Test.Reply.Entry do @type t :: %__MODULE__{ key_that_needs_1234camel_CasIng: integer, - value: integer, - _my_field_name_2: integer + value: integer | nil, + _my_field_name_2: integer | nil } defstruct [:key_that_needs_1234camel_CasIng, :value, :_my_field_name_2] @@ -151,7 +151,7 @@ defmodule My.Test.Reply do @type t :: %__MODULE__{ found: [My.Test.Reply.Entry.t()], - compact_keys: integer, + compact_keys: [integer], __pb_extensions__: map } @@ -170,7 +170,7 @@ defmodule My.Test.OtherBase do use Protobuf, syntax: :proto2 @type t :: %__MODULE__{ - name: String.t(), + name: String.t() | nil, __pb_extensions__: map } @@ -198,7 +198,7 @@ defmodule My.Test.OtherReplyExtensions do use Protobuf, syntax: :proto2 @type t :: %__MODULE__{ - key: integer + key: integer | nil } defstruct [:key] @@ -225,7 +225,7 @@ defmodule My.Test.Communique.SomeGroup do use Protobuf, syntax: :proto2 @type t :: %__MODULE__{ - member: String.t() + member: String.t() | nil } defstruct [:member] @@ -251,17 +251,17 @@ defmodule My.Test.Communique do @type t :: %__MODULE__{ union: - {:number, integer} - | {:name, String.t()} - | {:data, binary} - | {:temp_c, float | :infinity | :negative_infinity | :nan} - | {:height, float | :infinity | :negative_infinity | :nan} - | {:today, My.Test.Days.t()} - | {:maybe, boolean} - | {:delta, integer} + {:number, integer | nil} + | {:name, String.t() | nil} + | {:data, binary | nil} + | {:temp_c, float | :infinity | :negative_infinity | :nan | nil} + | {:height, float | :infinity | :negative_infinity | :nan | nil} + | {:today, My.Test.Days.t() | nil} + | {:maybe, boolean | nil} + | {:delta, integer | nil} | {:msg, My.Test.Reply.t() | nil} - | {:somegroup, any}, - make_me_cry: boolean + | {:somegroup, any | nil}, + make_me_cry: boolean | nil } defstruct [:union, :make_me_cry] @@ -288,7 +288,7 @@ defmodule My.Test.Options do use Protobuf, syntax: :proto2 @type t :: %__MODULE__{ - opt1: String.t() + opt1: String.t() | nil } defstruct [:opt1] From a730a0022ca438d3a2601e8af02da4aff501f0ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Britto?= Date: Wed, 20 Oct 2021 17:20:49 -0300 Subject: [PATCH 2/4] Add parentheses to remote calls with no arguments --- README.md | 4 ++-- lib/protobuf/protoc/generator/message.ex | 4 ++-- lib/protobuf/type_util.ex | 2 +- test/protobuf/protoc/generator/message_test.exs | 16 ++++++++-------- 4 files changed, 13 insertions(+), 13 deletions(-) diff --git a/README.md b/README.md index c693ea70..5a5d836c 100644 --- a/README.md +++ b/README.md @@ -73,7 +73,7 @@ end use Protobuf, syntax: :proto3 @type t :: %__MODULE__{ - name: String.t + name: String.t() } defstruct [:name] @@ -84,7 +84,7 @@ end use Protobuf, syntax: :proto3 @type t :: %__MODULE__{ - message: String.t + message: String.t() } defstruct [:message] diff --git a/lib/protobuf/protoc/generator/message.ex b/lib/protobuf/protoc/generator/message.ex index 3093affe..ddae42ae 100644 --- a/lib/protobuf/protoc/generator/message.ex +++ b/lib/protobuf/protoc/generator/message.ex @@ -178,8 +178,8 @@ defmodule Protobuf.Protoc.Generator.Message do end end - defp type_to_spec(:TYPE_MESSAGE, type), do: "#{type}.t" - defp type_to_spec(:TYPE_ENUM, type), do: "#{type}.t" + defp type_to_spec(:TYPE_MESSAGE, type), do: "#{type}.t()" + defp type_to_spec(:TYPE_ENUM, type), do: "#{type}.t()" defp type_to_spec(type_scalar, _type), do: TypeUtil.enum_to_spec(type_scalar) def get_fields(ctx, desc) do diff --git a/lib/protobuf/type_util.ex b/lib/protobuf/type_util.ex index 094ff6ce..e21ac2a6 100644 --- a/lib/protobuf/type_util.ex +++ b/lib/protobuf/type_util.ex @@ -27,7 +27,7 @@ defmodule Protobuf.TypeUtil do def enum_to_spec(:TYPE_FIXED64), do: "non_neg_integer" def enum_to_spec(:TYPE_FIXED32), do: "non_neg_integer" def enum_to_spec(:TYPE_BOOL), do: "boolean" - def enum_to_spec(:TYPE_STRING), do: "String.t" + def enum_to_spec(:TYPE_STRING), do: "String.t()" def enum_to_spec(:TYPE_GROUP), do: "any" def enum_to_spec(:TYPE_MESSAGE), do: "binary" def enum_to_spec(:TYPE_BYTES), do: "binary" diff --git a/test/protobuf/protoc/generator/message_test.exs b/test/protobuf/protoc/generator/message_test.exs index a104e1d8..dbe6c4a7 100644 --- a/test/protobuf/protoc/generator/message_test.exs +++ b/test/protobuf/protoc/generator/message_test.exs @@ -81,7 +81,7 @@ defmodule Protobuf.Protoc.Generator.MessageTest do {[], [msg]} = Generator.generate(ctx, desc) assert msg =~ "defstruct [:a, :b]\n" assert msg =~ "a: integer" - assert msg =~ "b: String.t" + assert msg =~ "b: String.t()" assert msg =~ "field :a, 1, optional: true, type: :int32\n" assert msg =~ "field :b, 2, required: true, type: :string\n" end @@ -120,7 +120,7 @@ defmodule Protobuf.Protoc.Generator.MessageTest do {[], [msg]} = Generator.generate(ctx, desc) assert msg =~ "defstruct [:a, :b, :c]\n" assert msg =~ "a: integer" - assert msg =~ "b: String.t" + assert msg =~ "b: String.t()" assert msg =~ "field :a, 1, type: :int32\n" assert msg =~ "field :b, 2, type: :string\n" assert msg =~ "field :c, 3, repeated: true, type: :int32\n" @@ -226,8 +226,8 @@ defmodule Protobuf.Protoc.Generator.MessageTest do ) {[], [msg]} = Generator.generate(ctx, desc) - assert msg =~ "bar: Bar.t | nil" - assert msg =~ "baz: [Baz.t]" + assert msg =~ "bar: Bar.t() | nil" + assert msg =~ "baz: [Baz.t()]" end test "generate/2 supports map field" do @@ -279,7 +279,7 @@ defmodule Protobuf.Protoc.Generator.MessageTest do ) {[[]], [_, msg]} = Generator.generate(ctx, desc) - assert msg =~ "a: %{integer => FooBar.AbCd.Bar.t | nil}" + assert msg =~ "a: %{integer => FooBar.AbCd.Bar.t() | nil}" assert msg =~ "field :a, 1, repeated: true, type: FooBar.AbCd.Foo.ProjectsEntry, map: true\n" end @@ -307,7 +307,7 @@ defmodule Protobuf.Protoc.Generator.MessageTest do ) {[], [msg]} = Generator.generate(ctx, desc) - assert msg =~ "a: FooBar.AbCd.EnumFoo.t" + assert msg =~ "a: FooBar.AbCd.EnumFoo.t()" assert msg =~ "field :a, 1, optional: true, type: FooBar.AbCd.EnumFoo, enum: true\n" end @@ -358,7 +358,7 @@ defmodule Protobuf.Protoc.Generator.MessageTest do ) {[], [msg]} = Generator.generate(ctx, desc) - assert msg =~ "a: OtherPkg.MsgFoo.t" + assert msg =~ "a: OtherPkg.MsgFoo.t()" assert msg =~ "field :a, 1, optional: true, type: OtherPkg.MsgFoo\n" end @@ -552,6 +552,6 @@ defmodule Protobuf.Protoc.Generator.MessageTest do ) {[], [msg]} = Generator.generate(ctx, desc) - assert msg =~ "a: [FooBar.AbCd.EnumFoo.t]" + assert msg =~ "a: [FooBar.AbCd.EnumFoo.t()]" end end From 0072487347a67f774d7f09d1bc32a0772416e074 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Britto?= Date: Thu, 21 Oct 2021 10:57:34 -0300 Subject: [PATCH 3/4] Revert optional fields to previous behavior Skip the ` | nil` on scalar fields, even when optional. --- lib/elixirpb.pb.ex | 2 +- lib/google/compiler/plugin.pb.ex | 20 +-- lib/google/descriptor.pb.ex | 154 +++++++++--------- lib/protobuf/protoc/generator/message.ex | 8 +- .../protoc/generator/message_test.exs | 6 +- .../protobuf/protoc/proto_gen/extension.pb.ex | 2 +- test/protobuf/protoc/proto_gen/test.pb.ex | 52 +++--- 7 files changed, 122 insertions(+), 122 deletions(-) diff --git a/lib/elixirpb.pb.ex b/lib/elixirpb.pb.ex index 484847aa..62e05409 100644 --- a/lib/elixirpb.pb.ex +++ b/lib/elixirpb.pb.ex @@ -3,7 +3,7 @@ defmodule Elixirpb.FileOptions do use Protobuf, syntax: :proto2 @type t :: %__MODULE__{ - module_prefix: String.t() | nil + module_prefix: String.t() } defstruct [:module_prefix] diff --git a/lib/google/compiler/plugin.pb.ex b/lib/google/compiler/plugin.pb.ex index 89803f4d..be3f4aa1 100644 --- a/lib/google/compiler/plugin.pb.ex +++ b/lib/google/compiler/plugin.pb.ex @@ -12,10 +12,10 @@ defmodule Google.Protobuf.Compiler.Version do use Protobuf, syntax: :proto2 @type t :: %__MODULE__{ - major: integer | nil, - minor: integer | nil, - patch: integer | nil, - suffix: String.t() | nil + major: integer, + minor: integer, + patch: integer, + suffix: String.t() } defstruct [:major, :minor, :patch, :suffix] @@ -34,7 +34,7 @@ defmodule Google.Protobuf.Compiler.CodeGeneratorRequest do @type t :: %__MODULE__{ file_to_generate: [String.t()], - parameter: String.t() | nil, + parameter: String.t(), proto_file: [Google.Protobuf.FileDescriptorProto.t()], compiler_version: Google.Protobuf.Compiler.Version.t() | nil } @@ -54,9 +54,9 @@ defmodule Google.Protobuf.Compiler.CodeGeneratorResponse.File do use Protobuf, syntax: :proto2 @type t :: %__MODULE__{ - name: String.t() | nil, - insertion_point: String.t() | nil, - content: String.t() | nil, + name: String.t(), + insertion_point: String.t(), + content: String.t(), generated_code_info: Google.Protobuf.GeneratedCodeInfo.t() | nil } @@ -75,8 +75,8 @@ defmodule Google.Protobuf.Compiler.CodeGeneratorResponse do use Protobuf, syntax: :proto2 @type t :: %__MODULE__{ - error: String.t() | nil, - supported_features: non_neg_integer | nil, + error: String.t(), + supported_features: non_neg_integer, file: [Google.Protobuf.Compiler.CodeGeneratorResponse.File.t()] } diff --git a/lib/google/descriptor.pb.ex b/lib/google/descriptor.pb.ex index 111bbab3..a9fa8e19 100644 --- a/lib/google/descriptor.pb.ex +++ b/lib/google/descriptor.pb.ex @@ -113,8 +113,8 @@ defmodule Google.Protobuf.FileDescriptorProto do use Protobuf, syntax: :proto2 @type t :: %__MODULE__{ - name: String.t() | nil, - package: String.t() | nil, + name: String.t(), + package: String.t(), dependency: [String.t()], public_dependency: [integer], weak_dependency: [integer], @@ -124,7 +124,7 @@ defmodule Google.Protobuf.FileDescriptorProto do extension: [Google.Protobuf.FieldDescriptorProto.t()], options: Google.Protobuf.FileOptions.t() | nil, source_code_info: Google.Protobuf.SourceCodeInfo.t() | nil, - syntax: String.t() | nil + syntax: String.t() } defstruct [ @@ -163,8 +163,8 @@ defmodule Google.Protobuf.DescriptorProto.ExtensionRange do use Protobuf, syntax: :proto2 @type t :: %__MODULE__{ - start: integer | nil, - end: integer | nil, + start: integer, + end: integer, options: Google.Protobuf.ExtensionRangeOptions.t() | nil } @@ -182,8 +182,8 @@ defmodule Google.Protobuf.DescriptorProto.ReservedRange do use Protobuf, syntax: :proto2 @type t :: %__MODULE__{ - start: integer | nil, - end: integer | nil + start: integer, + end: integer } defstruct [:start, :end] @@ -199,7 +199,7 @@ defmodule Google.Protobuf.DescriptorProto do use Protobuf, syntax: :proto2 @type t :: %__MODULE__{ - name: String.t() | nil, + name: String.t(), field: [Google.Protobuf.FieldDescriptorProto.t()], extension: [Google.Protobuf.FieldDescriptorProto.t()], nested_type: [Google.Protobuf.DescriptorProto.t()], @@ -261,17 +261,17 @@ defmodule Google.Protobuf.FieldDescriptorProto do use Protobuf, syntax: :proto2 @type t :: %__MODULE__{ - name: String.t() | nil, - number: integer | nil, - label: Google.Protobuf.FieldDescriptorProto.Label.t() | nil, - type: Google.Protobuf.FieldDescriptorProto.Type.t() | nil, - type_name: String.t() | nil, - extendee: String.t() | nil, - default_value: String.t() | nil, - oneof_index: integer | nil, - json_name: String.t() | nil, + name: String.t(), + number: integer, + label: Google.Protobuf.FieldDescriptorProto.Label.t(), + type: Google.Protobuf.FieldDescriptorProto.Type.t(), + type_name: String.t(), + extendee: String.t(), + default_value: String.t(), + oneof_index: integer, + json_name: String.t(), options: Google.Protobuf.FieldOptions.t() | nil, - proto3_optional: boolean | nil + proto3_optional: boolean } defstruct [ @@ -308,7 +308,7 @@ defmodule Google.Protobuf.OneofDescriptorProto do use Protobuf, syntax: :proto2 @type t :: %__MODULE__{ - name: String.t() | nil, + name: String.t(), options: Google.Protobuf.OneofOptions.t() | nil } @@ -325,8 +325,8 @@ defmodule Google.Protobuf.EnumDescriptorProto.EnumReservedRange do use Protobuf, syntax: :proto2 @type t :: %__MODULE__{ - start: integer | nil, - end: integer | nil + start: integer, + end: integer } defstruct [:start, :end] @@ -342,7 +342,7 @@ defmodule Google.Protobuf.EnumDescriptorProto do use Protobuf, syntax: :proto2 @type t :: %__MODULE__{ - name: String.t() | nil, + name: String.t(), value: [Google.Protobuf.EnumValueDescriptorProto.t()], options: Google.Protobuf.EnumOptions.t() | nil, reserved_range: [Google.Protobuf.EnumDescriptorProto.EnumReservedRange.t()], @@ -369,8 +369,8 @@ defmodule Google.Protobuf.EnumValueDescriptorProto do use Protobuf, syntax: :proto2 @type t :: %__MODULE__{ - name: String.t() | nil, - number: integer | nil, + name: String.t(), + number: integer, options: Google.Protobuf.EnumValueOptions.t() | nil } @@ -388,7 +388,7 @@ defmodule Google.Protobuf.ServiceDescriptorProto do use Protobuf, syntax: :proto2 @type t :: %__MODULE__{ - name: String.t() | nil, + name: String.t(), method: [Google.Protobuf.MethodDescriptorProto.t()], options: Google.Protobuf.ServiceOptions.t() | nil } @@ -407,12 +407,12 @@ defmodule Google.Protobuf.MethodDescriptorProto do use Protobuf, syntax: :proto2 @type t :: %__MODULE__{ - name: String.t() | nil, - input_type: String.t() | nil, - output_type: String.t() | nil, + name: String.t(), + input_type: String.t(), + output_type: String.t(), options: Google.Protobuf.MethodOptions.t() | nil, - client_streaming: boolean | nil, - server_streaming: boolean | nil + client_streaming: boolean, + server_streaming: boolean } defstruct [:name, :input_type, :output_type, :options, :client_streaming, :server_streaming] @@ -432,26 +432,26 @@ defmodule Google.Protobuf.FileOptions do use Protobuf, syntax: :proto2 @type t :: %__MODULE__{ - java_package: String.t() | nil, - java_outer_classname: String.t() | nil, - java_multiple_files: boolean | nil, - java_generate_equals_and_hash: boolean | nil, - java_string_check_utf8: boolean | nil, - optimize_for: Google.Protobuf.FileOptions.OptimizeMode.t() | nil, - go_package: String.t() | nil, - cc_generic_services: boolean | nil, - java_generic_services: boolean | nil, - py_generic_services: boolean | nil, - php_generic_services: boolean | nil, - deprecated: boolean | nil, - cc_enable_arenas: boolean | nil, - objc_class_prefix: String.t() | nil, - csharp_namespace: String.t() | nil, - swift_prefix: String.t() | nil, - php_class_prefix: String.t() | nil, - php_namespace: String.t() | nil, - php_metadata_namespace: String.t() | nil, - ruby_package: String.t() | nil, + java_package: String.t(), + java_outer_classname: String.t(), + java_multiple_files: boolean, + java_generate_equals_and_hash: boolean, + java_string_check_utf8: boolean, + optimize_for: Google.Protobuf.FileOptions.OptimizeMode.t(), + go_package: String.t(), + cc_generic_services: boolean, + java_generic_services: boolean, + py_generic_services: boolean, + php_generic_services: boolean, + deprecated: boolean, + cc_enable_arenas: boolean, + objc_class_prefix: String.t(), + csharp_namespace: String.t(), + swift_prefix: String.t(), + php_class_prefix: String.t(), + php_namespace: String.t(), + php_metadata_namespace: String.t(), + ruby_package: String.t(), uninterpreted_option: [Google.Protobuf.UninterpretedOption.t()], __pb_extensions__: map } @@ -519,10 +519,10 @@ defmodule Google.Protobuf.MessageOptions do use Protobuf, syntax: :proto2 @type t :: %__MODULE__{ - message_set_wire_format: boolean | nil, - no_standard_descriptor_accessor: boolean | nil, - deprecated: boolean | nil, - map_entry: boolean | nil, + message_set_wire_format: boolean, + no_standard_descriptor_accessor: boolean, + deprecated: boolean, + map_entry: boolean, uninterpreted_option: [Google.Protobuf.UninterpretedOption.t()], __pb_extensions__: map } @@ -552,12 +552,12 @@ defmodule Google.Protobuf.FieldOptions do use Protobuf, syntax: :proto2 @type t :: %__MODULE__{ - ctype: Google.Protobuf.FieldOptions.CType.t() | nil, - packed: boolean | nil, - jstype: Google.Protobuf.FieldOptions.JSType.t() | nil, - lazy: boolean | nil, - deprecated: boolean | nil, - weak: boolean | nil, + ctype: Google.Protobuf.FieldOptions.CType.t(), + packed: boolean, + jstype: Google.Protobuf.FieldOptions.JSType.t(), + lazy: boolean, + deprecated: boolean, + weak: boolean, uninterpreted_option: [Google.Protobuf.UninterpretedOption.t()], __pb_extensions__: map } @@ -620,8 +620,8 @@ defmodule Google.Protobuf.EnumOptions do use Protobuf, syntax: :proto2 @type t :: %__MODULE__{ - allow_alias: boolean | nil, - deprecated: boolean | nil, + allow_alias: boolean, + deprecated: boolean, uninterpreted_option: [Google.Protobuf.UninterpretedOption.t()], __pb_extensions__: map } @@ -642,7 +642,7 @@ defmodule Google.Protobuf.EnumValueOptions do use Protobuf, syntax: :proto2 @type t :: %__MODULE__{ - deprecated: boolean | nil, + deprecated: boolean, uninterpreted_option: [Google.Protobuf.UninterpretedOption.t()], __pb_extensions__: map } @@ -662,7 +662,7 @@ defmodule Google.Protobuf.ServiceOptions do use Protobuf, syntax: :proto2 @type t :: %__MODULE__{ - deprecated: boolean | nil, + deprecated: boolean, uninterpreted_option: [Google.Protobuf.UninterpretedOption.t()], __pb_extensions__: map } @@ -682,8 +682,8 @@ defmodule Google.Protobuf.MethodOptions do use Protobuf, syntax: :proto2 @type t :: %__MODULE__{ - deprecated: boolean | nil, - idempotency_level: Google.Protobuf.MethodOptions.IdempotencyLevel.t() | nil, + deprecated: boolean, + idempotency_level: Google.Protobuf.MethodOptions.IdempotencyLevel.t(), uninterpreted_option: [Google.Protobuf.UninterpretedOption.t()], __pb_extensions__: map } @@ -728,12 +728,12 @@ defmodule Google.Protobuf.UninterpretedOption do @type t :: %__MODULE__{ name: [Google.Protobuf.UninterpretedOption.NamePart.t()], - identifier_value: String.t() | nil, - positive_int_value: non_neg_integer | nil, - negative_int_value: integer | nil, - double_value: float | :infinity | :negative_infinity | :nan | nil, - string_value: binary | nil, - aggregate_value: String.t() | nil + identifier_value: String.t(), + positive_int_value: non_neg_integer, + negative_int_value: integer, + double_value: float | :infinity | :negative_infinity | :nan, + string_value: binary, + aggregate_value: String.t() } defstruct [ @@ -764,8 +764,8 @@ defmodule Google.Protobuf.SourceCodeInfo.Location do @type t :: %__MODULE__{ path: [integer], span: [integer], - leading_comments: String.t() | nil, - trailing_comments: String.t() | nil, + leading_comments: String.t(), + trailing_comments: String.t(), leading_detached_comments: [String.t()] } @@ -801,9 +801,9 @@ defmodule Google.Protobuf.GeneratedCodeInfo.Annotation do @type t :: %__MODULE__{ path: [integer], - source_file: String.t() | nil, - begin: integer | nil, - end: integer | nil + source_file: String.t(), + begin: integer, + end: integer } defstruct [:path, :source_file, :begin, :end] diff --git a/lib/protobuf/protoc/generator/message.ex b/lib/protobuf/protoc/generator/message.ex index ddae42ae..5e284207 100644 --- a/lib/protobuf/protoc/generator/message.ex +++ b/lib/protobuf/protoc/generator/message.ex @@ -171,10 +171,10 @@ defmodule Protobuf.Protoc.Generator.Message do defp fmt_type(%{label: label, type_enum: type_enum, type: type}) do spec = type_to_spec(type_enum, type) - case label do - "optional" -> "#{spec} | nil" - "required" -> spec - "repeated" -> "[#{spec}]" + cond do + label == "repeated" -> "[#{spec}]" + type_enum == :TYPE_MESSAGE -> "#{spec} | nil" + true -> spec end end diff --git a/test/protobuf/protoc/generator/message_test.exs b/test/protobuf/protoc/generator/message_test.exs index dbe6c4a7..d7b673db 100644 --- a/test/protobuf/protoc/generator/message_test.exs +++ b/test/protobuf/protoc/generator/message_test.exs @@ -460,9 +460,9 @@ defmodule Protobuf.Protoc.Generator.MessageTest do ) {[], [msg]} = Generator.generate(ctx, desc) - assert msg =~ "first: {:a, integer | nil} | {:b, integer | nil},\n" - assert msg =~ "second: {:c, integer | nil} | {:d, integer | nil},\n" - assert msg =~ "other: integer | nil\n" + assert msg =~ "first: {:a, integer} | {:b, integer},\n" + assert msg =~ "second: {:c, integer} | {:d, integer},\n" + assert msg =~ "other: integer\n" refute msg =~ "a: integer,\n" assert msg =~ "defstruct [:first, :second, :other]\n" assert msg =~ "oneof :first, 0\n" diff --git a/test/protobuf/protoc/proto_gen/extension.pb.ex b/test/protobuf/protoc/proto_gen/extension.pb.ex index 7ce6ac51..f1ab2816 100644 --- a/test/protobuf/protoc/proto_gen/extension.pb.ex +++ b/test/protobuf/protoc/proto_gen/extension.pb.ex @@ -3,7 +3,7 @@ defmodule Protobuf.Protoc.ExtTest.Foo do use Protobuf, syntax: :proto2 @type t :: %__MODULE__{ - a: String.t() | nil + a: String.t() } defstruct [:a] diff --git a/test/protobuf/protoc/proto_gen/test.pb.ex b/test/protobuf/protoc/proto_gen/test.pb.ex index 4cc15d01..5b3949cb 100644 --- a/test/protobuf/protoc/proto_gen/test.pb.ex +++ b/test/protobuf/protoc/proto_gen/test.pb.ex @@ -41,7 +41,7 @@ defmodule My.Test.Request.SomeGroup do use Protobuf, syntax: :proto2 @type t :: %__MODULE__{ - group_field: integer | nil + group_field: integer } defstruct [:group_field] @@ -56,8 +56,8 @@ defmodule My.Test.Request.NameMappingEntry do use Protobuf, map: true, syntax: :proto2 @type t :: %__MODULE__{ - key: integer | nil, - value: String.t() | nil + key: integer, + value: String.t() } defstruct [:key, :value] @@ -73,7 +73,7 @@ defmodule My.Test.Request.MsgMappingEntry do use Protobuf, map: true, syntax: :proto2 @type t :: %__MODULE__{ - key: integer | nil, + key: integer, value: My.Test.Reply.t() | nil } @@ -91,14 +91,14 @@ defmodule My.Test.Request do @type t :: %__MODULE__{ key: [integer], - hue: My.Test.Request.Color.t() | nil, - hat: My.Test.HatType.t() | nil, - deadline: float | :infinity | :negative_infinity | :nan | nil, - somegroup: any | nil, + hue: My.Test.Request.Color.t(), + hat: My.Test.HatType.t(), + deadline: float | :infinity | :negative_infinity | :nan, + somegroup: any, name_mapping: %{integer => String.t() | nil}, msg_mapping: %{integer => My.Test.Reply.t() | nil}, - reset: integer | nil, - get_key: String.t() | nil + reset: integer, + get_key: String.t() } defstruct [ @@ -132,8 +132,8 @@ defmodule My.Test.Reply.Entry do @type t :: %__MODULE__{ key_that_needs_1234camel_CasIng: integer, - value: integer | nil, - _my_field_name_2: integer | nil + value: integer, + _my_field_name_2: integer } defstruct [:key_that_needs_1234camel_CasIng, :value, :_my_field_name_2] @@ -170,7 +170,7 @@ defmodule My.Test.OtherBase do use Protobuf, syntax: :proto2 @type t :: %__MODULE__{ - name: String.t() | nil, + name: String.t(), __pb_extensions__: map } @@ -198,7 +198,7 @@ defmodule My.Test.OtherReplyExtensions do use Protobuf, syntax: :proto2 @type t :: %__MODULE__{ - key: integer | nil + key: integer } defstruct [:key] @@ -225,7 +225,7 @@ defmodule My.Test.Communique.SomeGroup do use Protobuf, syntax: :proto2 @type t :: %__MODULE__{ - member: String.t() | nil + member: String.t() } defstruct [:member] @@ -251,17 +251,17 @@ defmodule My.Test.Communique do @type t :: %__MODULE__{ union: - {:number, integer | nil} - | {:name, String.t() | nil} - | {:data, binary | nil} - | {:temp_c, float | :infinity | :negative_infinity | :nan | nil} - | {:height, float | :infinity | :negative_infinity | :nan | nil} - | {:today, My.Test.Days.t() | nil} - | {:maybe, boolean | nil} - | {:delta, integer | nil} + {:number, integer} + | {:name, String.t()} + | {:data, binary} + | {:temp_c, float | :infinity | :negative_infinity | :nan} + | {:height, float | :infinity | :negative_infinity | :nan} + | {:today, My.Test.Days.t()} + | {:maybe, boolean} + | {:delta, integer} | {:msg, My.Test.Reply.t() | nil} - | {:somegroup, any | nil}, - make_me_cry: boolean | nil + | {:somegroup, any}, + make_me_cry: boolean } defstruct [:union, :make_me_cry] @@ -288,7 +288,7 @@ defmodule My.Test.Options do use Protobuf, syntax: :proto2 @type t :: %__MODULE__{ - opt1: String.t() | nil + opt1: String.t() } defstruct [:opt1] From 94f86d76db4f61bba29382d9c3fe921f5c2bb8ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Britto?= Date: Thu, 21 Oct 2021 13:17:52 -0300 Subject: [PATCH 4/4] Revert map value specs to previous behavior Optional when type is message and required otherwise. --- lib/protobuf/protoc/generator/message.ex | 18 +++++++++++------- test/protobuf/protoc/proto_gen/test.pb.ex | 2 +- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/lib/protobuf/protoc/generator/message.ex b/lib/protobuf/protoc/generator/message.ex index 5e284207..7207531c 100644 --- a/lib/protobuf/protoc/generator/message.ex +++ b/lib/protobuf/protoc/generator/message.ex @@ -160,21 +160,22 @@ defmodule Protobuf.Protoc.Generator.Message do end defp fmt_type(%{opts: %{map: true}, map: {{k_type_enum, _k_type}, {v_type_enum, v_type}}}) do - # Keys are guaranteed to be scalars. - # Values can be anything except another map and are considered optional. + # Keys are guaranteed to be scalars. Values can be anything except another map. Map fields + # cannot be `repeated`. k_spec = TypeUtil.enum_to_spec(k_type_enum) v_spec = type_to_spec(v_type_enum, v_type) + v_spec = optional_if_message(v_type_enum, v_spec) - "%{#{k_spec} => #{v_spec} | nil}" + "%{#{k_spec} => #{v_spec}}" end defp fmt_type(%{label: label, type_enum: type_enum, type: type}) do spec = type_to_spec(type_enum, type) - cond do - label == "repeated" -> "[#{spec}]" - type_enum == :TYPE_MESSAGE -> "#{spec} | nil" - true -> spec + if label == "repeated" do + "[#{spec}]" + else + optional_if_message(type_enum, spec) end end @@ -182,6 +183,9 @@ defmodule Protobuf.Protoc.Generator.Message do defp type_to_spec(:TYPE_ENUM, type), do: "#{type}.t()" defp type_to_spec(type_scalar, _type), do: TypeUtil.enum_to_spec(type_scalar) + defp optional_if_message(:TYPE_MESSAGE, spec), do: "#{spec} | nil" + defp optional_if_message(_type_others, spec), do: spec + def get_fields(ctx, desc) do oneofs = Enum.map(desc.oneof_decl, & &1.name) nested_maps = nested_maps(ctx, desc) diff --git a/test/protobuf/protoc/proto_gen/test.pb.ex b/test/protobuf/protoc/proto_gen/test.pb.ex index 5b3949cb..965f49f2 100644 --- a/test/protobuf/protoc/proto_gen/test.pb.ex +++ b/test/protobuf/protoc/proto_gen/test.pb.ex @@ -95,7 +95,7 @@ defmodule My.Test.Request do hat: My.Test.HatType.t(), deadline: float | :infinity | :negative_infinity | :nan, somegroup: any, - name_mapping: %{integer => String.t() | nil}, + name_mapping: %{integer => String.t()}, msg_mapping: %{integer => My.Test.Reply.t() | nil}, reset: integer, get_key: String.t()