fix: include nested enums in parent message file instead of separate files

lib/grpc_reflection/service/builder/util.ex

@@ -9,11 +9,24 @@ defmodule GrpcReflection.Service.Builder.Util do
 
   def get_nested_types(symbol, descriptor), do: get_nested_types(symbol, descriptor, [])
 
-  def get_nested_types(_symbol, %Google.Protobuf.DescriptorProto{nested_type: []}, acc) do
+  def get_nested_types(
+        _symbol,
+        %Google.Protobuf.DescriptorProto{nested_type: [], enum_type: []},
+        acc
+      ) do
     acc
   end
 
-  def get_nested_types(symbol, %Google.Protobuf.DescriptorProto{nested_type: nested_types}, acc) do
+  def get_nested_types(
+        symbol,
+        %Google.Protobuf.DescriptorProto{nested_type: nested_types, enum_type: enum_types},
+        acc
+      ) do
+    acc =
+      Enum.reduce(enum_types, acc, fn enum_type, acc ->
+        [symbol <> "." <> enum_type.name | acc]
+      end)
+
     Enum.reduce(nested_types, acc, fn nested_type, acc ->
       new_symbol = symbol <> "." <> nested_type.name
       new_acc = [new_symbol | acc]

lib/proto/grpc/reflection/v1/reflection.pb.ex

@@ -1,7 +1,10 @@
 defmodule Grpc.Reflection.V1.ServerReflectionRequest do
   @moduledoc false
 
-  use Protobuf, protoc_gen_elixir_version: "0.14.1", syntax: :proto3
+  use Protobuf,
+    full_name: "grpc.reflection.v1.ServerReflectionRequest",
+    protoc_gen_elixir_version: "0.16.0",
+    syntax: :proto3
 
   def descriptor do
     # credo:disable-for-next-line
@@ -133,7 +136,10 @@ end
 defmodule Grpc.Reflection.V1.ExtensionRequest do
   @moduledoc false
 
-  use Protobuf, protoc_gen_elixir_version: "0.14.1", syntax: :proto3
+  use Protobuf,
+    full_name: "grpc.reflection.v1.ExtensionRequest",
+    protoc_gen_elixir_version: "0.16.0",
+    syntax: :proto3
 
   def descriptor do
     # credo:disable-for-next-line
@@ -188,7 +194,10 @@ end
 defmodule Grpc.Reflection.V1.ServerReflectionResponse do
   @moduledoc false
 
-  use Protobuf, protoc_gen_elixir_version: "0.14.1", syntax: :proto3
+  use Protobuf,
+    full_name: "grpc.reflection.v1.ServerReflectionResponse",
+    protoc_gen_elixir_version: "0.16.0",
+    syntax: :proto3
 
   def descriptor do
     # credo:disable-for-next-line
@@ -330,7 +339,10 @@ end
 defmodule Grpc.Reflection.V1.FileDescriptorResponse do
   @moduledoc false
 
-  use Protobuf, protoc_gen_elixir_version: "0.14.1", syntax: :proto3
+  use Protobuf,
+    full_name: "grpc.reflection.v1.FileDescriptorResponse",
+    protoc_gen_elixir_version: "0.16.0",
+    syntax: :proto3
 
   def descriptor do
     # credo:disable-for-next-line
@@ -370,7 +382,10 @@ end
 defmodule Grpc.Reflection.V1.ExtensionNumberResponse do
   @moduledoc false
 
-  use Protobuf, protoc_gen_elixir_version: "0.14.1", syntax: :proto3
+  use Protobuf,
+    full_name: "grpc.reflection.v1.ExtensionNumberResponse",
+    protoc_gen_elixir_version: "0.16.0",
+    syntax: :proto3
 
   def descriptor do
     # credo:disable-for-next-line
@@ -425,7 +440,10 @@ end
 defmodule Grpc.Reflection.V1.ListServiceResponse do
   @moduledoc false
 
-  use Protobuf, protoc_gen_elixir_version: "0.14.1", syntax: :proto3
+  use Protobuf,
+    full_name: "grpc.reflection.v1.ListServiceResponse",
+    protoc_gen_elixir_version: "0.16.0",
+    syntax: :proto3
 
   def descriptor do
     # credo:disable-for-next-line
@@ -465,7 +483,10 @@ end
 defmodule Grpc.Reflection.V1.ServiceResponse do
   @moduledoc false
 
-  use Protobuf, protoc_gen_elixir_version: "0.14.1", syntax: :proto3
+  use Protobuf,
+    full_name: "grpc.reflection.v1.ServiceResponse",
+    protoc_gen_elixir_version: "0.16.0",
+    syntax: :proto3
 
   def descriptor do
     # credo:disable-for-next-line
@@ -505,7 +526,10 @@ end
 defmodule Grpc.Reflection.V1.ErrorResponse do
   @moduledoc false
 
-  use Protobuf, protoc_gen_elixir_version: "0.14.1", syntax: :proto3
+  use Protobuf,
+    full_name: "grpc.reflection.v1.ErrorResponse",
+    protoc_gen_elixir_version: "0.16.0",
+    syntax: :proto3
 
   def descriptor do
     # credo:disable-for-next-line
@@ -562,7 +586,7 @@ defmodule Grpc.Reflection.V1.ServerReflection.Service do
 
   use GRPC.Service,
     name: "grpc.reflection.v1.ServerReflection",
-    protoc_gen_elixir_version: "0.14.1"
+    protoc_gen_elixir_version: "0.16.0"
 
   def descriptor do
     # credo:disable-for-next-line

lib/proto/grpc/reflection/v1alpha/reflection.pb.ex

@@ -1,7 +1,10 @@
 defmodule Grpc.Reflection.V1alpha.ServerReflectionRequest do
   @moduledoc false
 
-  use Protobuf, protoc_gen_elixir_version: "0.14.1", syntax: :proto3
+  use Protobuf,
+    full_name: "grpc.reflection.v1alpha.ServerReflectionRequest",
+    protoc_gen_elixir_version: "0.16.0",
+    syntax: :proto3
 
   def descriptor do
     # credo:disable-for-next-line
@@ -133,7 +136,10 @@ end
 defmodule Grpc.Reflection.V1alpha.ExtensionRequest do
   @moduledoc false
 
-  use Protobuf, protoc_gen_elixir_version: "0.14.1", syntax: :proto3
+  use Protobuf,
+    full_name: "grpc.reflection.v1alpha.ExtensionRequest",
+    protoc_gen_elixir_version: "0.16.0",
+    syntax: :proto3
 
   def descriptor do
     # credo:disable-for-next-line
@@ -188,7 +194,10 @@ end
 defmodule Grpc.Reflection.V1alpha.ServerReflectionResponse do
   @moduledoc false
 
-  use Protobuf, protoc_gen_elixir_version: "0.14.1", syntax: :proto3
+  use Protobuf,
+    full_name: "grpc.reflection.v1alpha.ServerReflectionResponse",
+    protoc_gen_elixir_version: "0.16.0",
+    syntax: :proto3
 
   def descriptor do
     # credo:disable-for-next-line
@@ -330,7 +339,10 @@ end
 defmodule Grpc.Reflection.V1alpha.FileDescriptorResponse do
   @moduledoc false
 
-  use Protobuf, protoc_gen_elixir_version: "0.14.1", syntax: :proto3
+  use Protobuf,
+    full_name: "grpc.reflection.v1alpha.FileDescriptorResponse",
+    protoc_gen_elixir_version: "0.16.0",
+    syntax: :proto3
 
   def descriptor do
     # credo:disable-for-next-line
@@ -370,7 +382,10 @@ end
 defmodule Grpc.Reflection.V1alpha.ExtensionNumberResponse do
   @moduledoc false
 
-  use Protobuf, protoc_gen_elixir_version: "0.14.1", syntax: :proto3
+  use Protobuf,
+    full_name: "grpc.reflection.v1alpha.ExtensionNumberResponse",
+    protoc_gen_elixir_version: "0.16.0",
+    syntax: :proto3
 
   def descriptor do
     # credo:disable-for-next-line
@@ -425,7 +440,10 @@ end
 defmodule Grpc.Reflection.V1alpha.ListServiceResponse do
   @moduledoc false
 
-  use Protobuf, protoc_gen_elixir_version: "0.14.1", syntax: :proto3
+  use Protobuf,
+    full_name: "grpc.reflection.v1alpha.ListServiceResponse",
+    protoc_gen_elixir_version: "0.16.0",
+    syntax: :proto3
 
   def descriptor do
     # credo:disable-for-next-line
@@ -465,7 +483,10 @@ end
 defmodule Grpc.Reflection.V1alpha.ServiceResponse do
   @moduledoc false
 
-  use Protobuf, protoc_gen_elixir_version: "0.14.1", syntax: :proto3
+  use Protobuf,
+    full_name: "grpc.reflection.v1alpha.ServiceResponse",
+    protoc_gen_elixir_version: "0.16.0",
+    syntax: :proto3
 
   def descriptor do
     # credo:disable-for-next-line
@@ -505,7 +526,10 @@ end
 defmodule Grpc.Reflection.V1alpha.ErrorResponse do
   @moduledoc false
 
-  use Protobuf, protoc_gen_elixir_version: "0.14.1", syntax: :proto3
+  use Protobuf,
+    full_name: "grpc.reflection.v1alpha.ErrorResponse",
+    protoc_gen_elixir_version: "0.16.0",
+    syntax: :proto3
 
   def descriptor do
     # credo:disable-for-next-line
@@ -562,7 +586,7 @@ defmodule Grpc.Reflection.V1alpha.ServerReflection.Service do
 
   use GRPC.Service,
     name: "grpc.reflection.v1alpha.ServerReflection",
-    protoc_gen_elixir_version: "0.14.1"
+    protoc_gen_elixir_version: "0.16.0"
 
   def descriptor do
     # credo:disable-for-next-line

priv/protos/nested_enum_conflict.proto

@@ -0,0 +1,40 @@
+syntax = "proto3";
+
+package nestedEnumConflict;
+
+// Two messages in the same package each define a nested enum with the same
+// local name (SortOrder). This triggers a name conflict when a gRPC reflection
+// client (e.g. k6) passes the resulting FileDescriptorSet to
+// protodesc.NewFiles(), because the builder incorrectly promotes nested enums
+// to top-level symbols in separate synthetic files.
+//
+// See: https://github.com/grafana/k6/issues/5712
+
+service ConflictService {
+  rpc ListFoos (ListFoosRequest) returns (ListFoosResponse) {}
+  rpc ListBars (ListBarsRequest) returns (ListBarsResponse) {}
+}
+
+message ListFoosRequest {
+  enum SortOrder {
+    SORT_ORDER_UNSPECIFIED = 0;
+    SORT_ORDER_NEWEST_FIRST = 1;
+    SORT_ORDER_OLDEST_FIRST = 2;
+  }
+
+  SortOrder sort_order = 1;
+}
+
+message ListFoosResponse {}
+
+message ListBarsRequest {
+  enum SortOrder {
+    SORT_ORDER_UNSPECIFIED = 0;
+    SORT_ORDER_NEWEST_FIRST = 1;
+    SORT_ORDER_OLDEST_FIRST = 2;
+  }
+
+  SortOrder sort_order = 1;
+}
+
+message ListBarsResponse {}

test/service/builder_test.exs

@@ -119,6 +119,29 @@ defmodule GrpcReflection.Service.BuilderTest do
            ]
   end
 
+  test "does not produce top-level enum files for nested enums with the same name" do
+    assert {:ok, tree} =
+             Builder.build_reflection_tree([NestedEnumConflict.ConflictService.Service])
+
+    file_names = Map.keys(tree.files)
+
+    refute "nestedEnumConflict.ListFoosRequest.SortOrder.proto" in file_names
+    refute "nestedEnumConflict.ListBarsRequest.SortOrder.proto" in file_names
+
+    assert "nestedEnumConflict.ListFoosRequest.proto" in file_names
+    assert "nestedEnumConflict.ListBarsRequest.proto" in file_names
+
+    foos_file = tree.files["nestedEnumConflict.ListFoosRequest.proto"]
+
+    assert [%{name: "ListFoosRequest", enum_type: [%{name: "SortOrder"}]}] =
+             foos_file.message_type
+
+    bars_file = tree.files["nestedEnumConflict.ListBarsRequest.proto"]
+
+    assert [%{name: "ListBarsRequest", enum_type: [%{name: "SortOrder"}]}] =
+             bars_file.message_type
+  end
+
   test "handles a non-service module" do
     assert_raise UndefinedFunctionError, fn ->
       Builder.build_reflection_tree([Enum])

test/support/protos/custom_prefix_service.pb.ex

@@ -1,7 +1,11 @@
 defmodule HLW.Enum do
   @moduledoc false
 
-  use Protobuf, enum: true, protoc_gen_elixir_version: "0.14.1", syntax: :proto2
+  use Protobuf,
+    enum: true,
+    full_name: "testserviceV2.Enum",
+    protoc_gen_elixir_version: "0.16.0",
+    syntax: :proto2
 
   def descriptor do
     # credo:disable-for-next-line
@@ -35,7 +39,11 @@ end
 defmodule HLW.TestRequest.GEntry do
   @moduledoc false
 
-  use Protobuf, map: true, protoc_gen_elixir_version: "0.14.1", syntax: :proto2
+  use Protobuf,
+    full_name: "testserviceV2.TestRequest.GEntry",
+    map: true,
+    protoc_gen_elixir_version: "0.16.0",
+    syntax: :proto2
 
   def descriptor do
     # credo:disable-for-next-line
@@ -100,7 +108,10 @@ end
 defmodule HLW.TestRequest do
   @moduledoc false
 
-  use Protobuf, protoc_gen_elixir_version: "0.14.1", syntax: :proto2
+  use Protobuf,
+    full_name: "testserviceV2.TestRequest",
+    protoc_gen_elixir_version: "0.16.0",
+    syntax: :proto2
 
   def descriptor do
     # credo:disable-for-next-line
@@ -285,7 +296,10 @@ end
 defmodule HLW.Location do
   @moduledoc false
 
-  use Protobuf, protoc_gen_elixir_version: "0.14.1", syntax: :proto2
+  use Protobuf,
+    full_name: "testserviceV2.Location",
+    protoc_gen_elixir_version: "0.16.0",
+    syntax: :proto2
 
   def descriptor do
     # credo:disable-for-next-line
@@ -340,7 +354,10 @@ end
 defmodule HLW.TestReply do
   @moduledoc false
 
-  use Protobuf, protoc_gen_elixir_version: "0.14.1", syntax: :proto2
+  use Protobuf,
+    full_name: "testserviceV2.TestReply",
+    protoc_gen_elixir_version: "0.16.0",
+    syntax: :proto2
 
   def descriptor do
     # credo:disable-for-next-line
@@ -380,7 +397,7 @@ end
 defmodule HLW.TestService.Service do
   @moduledoc false
 
-  use GRPC.Service, name: "testserviceV2.TestService", protoc_gen_elixir_version: "0.14.1"
+  use GRPC.Service, name: "testserviceV2.TestService", protoc_gen_elixir_version: "0.16.0"
 
   def descriptor do
     # credo:disable-for-next-line

test/support/protos/empty_service.pb.ex

@@ -1,7 +1,7 @@
 defmodule EmptyService.Service do
   @moduledoc false
 
-  use GRPC.Service, name: "EmptyService", protoc_gen_elixir_version: "0.14.1"
+  use GRPC.Service, name: "EmptyService", protoc_gen_elixir_version: "0.16.0"
 
   def descriptor do
     # credo:disable-for-next-line