Fix multiple conflicting PbExtension modules (#332)

Closes #88.

Author: whatyouhide

lib/elixirpb.pb.ex

@@ -5,10 +5,3 @@ defmodule Elixirpb.FileOptions do
 
   field :module_prefix, 1, optional: true, type: :string
 end
-
-defmodule Elixirpb.PbExtension do
-  @moduledoc false
-  use Protobuf, protoc_gen_elixir_version: "0.11.0", syntax: :proto2
-
-  extend Google.Protobuf.FileOptions, :file, 1047, optional: true, type: Elixirpb.FileOptions
-end

lib/elixirpb/pb_extension.pb.ex

@@ -0,0 +1,6 @@
+defmodule Elixirpb.PbExtension do
+  @moduledoc false
+  use Protobuf, protoc_gen_elixir_version: "0.11.0"
+
+  extend Google.Protobuf.FileOptions, :file, 1047, optional: true, type: Elixirpb.FileOptions
+end

lib/google/protobuf/descriptor.pb.ex

@@ -154,7 +154,7 @@ defmodule Google.Protobuf.ExtensionRangeOptions do
 
   field :uninterpreted_option, 999, repeated: true, type: Google.Protobuf.UninterpretedOption
 
-  extensions [{1000, 536_870_912}]
+  extensions [{1000, Protobuf.Extension.max()}]
 end
 
 defmodule Google.Protobuf.FieldDescriptorProto do
@@ -275,7 +275,7 @@ defmodule Google.Protobuf.FileOptions do
   field :ruby_package, 45, optional: true, type: :string
   field :uninterpreted_option, 999, repeated: true, type: Google.Protobuf.UninterpretedOption
 
-  extensions [{1000, 536_870_912}]
+  extensions [{1000, Protobuf.Extension.max()}]
 end
 
 defmodule Google.Protobuf.MessageOptions do
@@ -289,7 +289,7 @@ defmodule Google.Protobuf.MessageOptions do
   field :map_entry, 7, optional: true, type: :bool
   field :uninterpreted_option, 999, repeated: true, type: Google.Protobuf.UninterpretedOption
 
-  extensions [{1000, 536_870_912}]
+  extensions [{1000, Protobuf.Extension.max()}]
 end
 
 defmodule Google.Protobuf.FieldOptions do
@@ -317,7 +317,7 @@ defmodule Google.Protobuf.FieldOptions do
   field :weak, 10, optional: true, type: :bool, default: false
   field :uninterpreted_option, 999, repeated: true, type: Google.Protobuf.UninterpretedOption
 
-  extensions [{1000, 536_870_912}]
+  extensions [{1000, Protobuf.Extension.max()}]
 end
 
 defmodule Google.Protobuf.OneofOptions do
@@ -327,7 +327,7 @@ defmodule Google.Protobuf.OneofOptions do
 
   field :uninterpreted_option, 999, repeated: true, type: Google.Protobuf.UninterpretedOption
 
-  extensions [{1000, 536_870_912}]
+  extensions [{1000, Protobuf.Extension.max()}]
 end
 
 defmodule Google.Protobuf.EnumOptions do
@@ -339,7 +339,7 @@ defmodule Google.Protobuf.EnumOptions do
   field :deprecated, 3, optional: true, type: :bool, default: false
   field :uninterpreted_option, 999, repeated: true, type: Google.Protobuf.UninterpretedOption
 
-  extensions [{1000, 536_870_912}]
+  extensions [{1000, Protobuf.Extension.max()}]
 end
 
 defmodule Google.Protobuf.EnumValueOptions do
@@ -350,7 +350,7 @@ defmodule Google.Protobuf.EnumValueOptions do
   field :deprecated, 1, optional: true, type: :bool, default: false
   field :uninterpreted_option, 999, repeated: true, type: Google.Protobuf.UninterpretedOption
 
-  extensions [{1000, 536_870_912}]
+  extensions [{1000, Protobuf.Extension.max()}]
 end
 
 defmodule Google.Protobuf.ServiceOptions do
@@ -361,7 +361,7 @@ defmodule Google.Protobuf.ServiceOptions do
   field :deprecated, 33, optional: true, type: :bool, default: false
   field :uninterpreted_option, 999, repeated: true, type: Google.Protobuf.UninterpretedOption
 
-  extensions [{1000, 536_870_912}]
+  extensions [{1000, Protobuf.Extension.max()}]
 end
 
 defmodule Google.Protobuf.MethodOptions do
@@ -379,7 +379,7 @@ defmodule Google.Protobuf.MethodOptions do
 
   field :uninterpreted_option, 999, repeated: true, type: Google.Protobuf.UninterpretedOption
 
-  extensions [{1000, 536_870_912}]
+  extensions [{1000, Protobuf.Extension.max()}]
 end
 
 defmodule Google.Protobuf.UninterpretedOption.NamePart do

lib/protobuf.ex

@@ -282,7 +282,25 @@ defmodule Protobuf do
   """
   @spec load_extensions() :: :ok
   def load_extensions() do
-    Protobuf.Extension.__cal_extensions__()
+    for mod <- get_all_modules(),
+        String.ends_with?(Atom.to_string(mod), ".PbExtension"),
+        Code.ensure_loaded?(mod),
+        function_exported?(mod, :__protobuf_info__, 1),
+        %{extensions: extensions} = mod.__protobuf_info__(:extension_props) do
+      Enum.each(extensions, fn {_, ext} ->
+        fnum = ext.field_props.fnum
+        fnum_key = {Protobuf.Extension, ext.extendee, fnum}
+        :persistent_term.put(fnum_key, mod)
+      end)
+    end
+
     :ok
   end
+
+  defp get_all_modules do
+    Enum.flat_map(Application.loaded_applications(), fn {app, _desc, _vsn} ->
+      {:ok, modules} = :application.get_key(app, :modules)
+      modules
+    end)
+  end
 end

lib/protobuf/dsl.ex

@@ -28,9 +28,50 @@ defmodule Protobuf.DSL do
 
   @doc """
   Define extensions range in the message module to allow extensions for this module.
+
+  Extension ranges are defined as a list of tuples `{start, end}`, where each tuple is
+  an **exclusive** range starting and `start` and ending at `end` (the equivalent
+  of `start..end-1` in Elixir).
+
+  To simulate the Protobuf `max` keyword, you can use `Protobuf.Extension.max/0`.
+
+  ## Examples
+
+  These Protobuf definition:
+
+  ```protobuf
+  message Foo {
+    extensions 1, 10 to 20, 100 to max;
+  }
+  ```
+
+  Would be translated in Elixir to:
+
+      extensions [{1, 2}, {10, 21}, {100, Protobuf.Extension.max()}]
+
   """
   defmacro extensions(ranges) do
     quote do
+      ranges = unquote(ranges)
+
+      if not is_list(ranges) do
+        raise ArgumentError, "expected a list of ranges, got: #{inspect(ranges)}"
+      end
+
+      Enum.each(ranges, fn
+        value when not is_tuple(value) or tuple_size(value) != 2 ->
+          raise ArgumentError, "expected a range, got: #{inspect(value)}"
+
+        {left, right} when not is_integer(left) or not is_integer(right) ->
+          raise ArgumentError, "expected an integer range, got: #{inspect({left, right})}"
+
+        {left, right} when left >= right ->
+          raise ArgumentError, "expected an ordered range, got: #{inspect({left, right})}"
+
+        other ->
+          :ok
+      end)
+
       @extensions unquote(ranges)
     end
   end

lib/protobuf/extension.ex

@@ -35,6 +35,25 @@ defmodule Protobuf.Extension do
 
   """
 
+  import Bitwise, only: [<<<: 2]
+
+  # TODO: replace bitshift with Integer.pow/2 when we depend on Elixir 1.12+.
+  # 2^29, see https://developers.google.com/protocol-buffers/docs/proto#extensions
+  @max 1 <<< 29
+
+  @doc """
+  Returns the maximum extension number.
+
+  ## Examples
+
+      iex> Protobuf.Extension.max()
+      #{inspect(@max)}
+
+  """
+  @doc since: "0.12.0"
+  @spec max() :: pos_integer()
+  def max, do: @max
+
   @doc "The actual function for `put_extension`"
   @spec put(module, map, module, atom, any) :: map
   def put(mod, struct, extension_mod, field, value) do
@@ -108,26 +127,4 @@ defmodule Protobuf.Extension do
         end
     end
   end
-
-  @doc false
-  def __cal_extensions__() do
-    for mod <- get_all_modules(),
-        String.ends_with?(Atom.to_string(mod), ".PbExtension"),
-        Code.ensure_loaded?(mod),
-        function_exported?(mod, :__protobuf_info__, 1),
-        %{extensions: extensions} = mod.__protobuf_info__(:extension_props) do
-      Enum.each(extensions, fn {_, ext} ->
-        fnum = ext.field_props.fnum
-        fnum_key = {Protobuf.Extension, ext.extendee, fnum}
-        :persistent_term.put(fnum_key, mod)
-      end)
-    end
-  end
-
-  defp get_all_modules do
-    Enum.flat_map(Application.loaded_applications(), fn {app, _desc, _vsn} ->
-      {:ok, modules} = :application.get_key(app, :modules)
-      modules
-    end)
-  end
 end

lib/protobuf/message_props.ex

@@ -22,6 +22,8 @@ defmodule Protobuf.MessageProps do
           enum?: boolean(),
           extendable?: boolean(),
           map?: boolean(),
+
+          # See Protobuf.DSL.extensions/1.
           extension_range: [{non_neg_integer(), non_neg_integer()}] | nil
         }
 

lib/protobuf/protoc/cli.ex

@@ -51,14 +51,33 @@ defmodule Protobuf.Protoc.CLI do
       |> parse_params(request.parameter || "")
       |> find_types(request.proto_file, request.file_to_generate)
 
-    files =
-      Enum.flat_map(request.file_to_generate, fn file ->
+    {files, package_level_extensions} =
+      Enum.flat_map_reduce(request.file_to_generate, %{}, fn file, acc ->
         desc = Enum.find(request.proto_file, &(&1.name == file))
-        Protobuf.Protoc.Generator.generate(ctx, desc)
+        {package_level_extensions, files} = Protobuf.Protoc.Generator.generate(ctx, desc)
+
+        acc =
+          case package_level_extensions do
+            {mod_name, extensions} -> Map.update(acc, mod_name, extensions, &(&1 ++ extensions))
+            nil -> acc
+          end
+
+        {files, acc}
       end)
 
+    ext_files =
+      for {mod_name, extensions} <- package_level_extensions do
+        {mod_name, contents} =
+          Protobuf.Protoc.Generator.Extension.generate_package_level(ctx, mod_name, extensions)
+
+        %Google.Protobuf.Compiler.CodeGeneratorResponse.File{
+          name: Macro.underscore(mod_name) <> ".pb.ex",
+          content: contents
+        }
+      end
+
     %Google.Protobuf.Compiler.CodeGeneratorResponse{
-      file: files,
+      file: files ++ ext_files,
       supported_features: supported_features()
     }
     |> Protobuf.encode_to_iodata()

lib/protobuf/protoc/context.ex

@@ -60,7 +60,7 @@ defmodule Protobuf.Protoc.Context do
       ) do
     custom_file_opts =
       Google.Protobuf.FileOptions.get_extension(options, Elixirpb.PbExtension, :file) ||
-        %Elixirpb.PbExtension{}
+        %Elixirpb.FileOptions{}
 
     %__MODULE__{
       ctx

lib/protobuf/protoc/generator.ex

@@ -4,31 +4,32 @@ defmodule Protobuf.Protoc.Generator do
   alias Protobuf.Protoc.Context
   alias Protobuf.Protoc.Generator
 
+  # TODO: improve spec
   @spec generate(Context.t(), %Google.Protobuf.FileDescriptorProto{}) ::
-          [Google.Protobuf.Compiler.CodeGeneratorResponse.File.t()]
+          {term(), [Google.Protobuf.Compiler.CodeGeneratorResponse.File.t()]}
   def generate(%Context{} = ctx, %Google.Protobuf.FileDescriptorProto{} = desc) do
-    module_definitions =
-      ctx
-      |> generate_module_definitions(desc)
-      |> Enum.reject(&is_nil/1)
-
-    if ctx.one_file_per_module? do
-      Enum.map(module_definitions, fn {mod_name, content} ->
-        file_name = Macro.underscore(mod_name) <> ".pb.ex"
-        %Google.Protobuf.Compiler.CodeGeneratorResponse.File{name: file_name, content: content}
-      end)
-    else
-      # desc.name is the filename, ending in ".proto".
-      file_name = Path.rootname(desc.name) <> ".pb.ex"
-
-      content =
-        module_definitions
-        |> Enum.map(fn {_mod_name, contents} -> [contents, ?\n] end)
-        |> IO.iodata_to_binary()
-        |> Generator.Util.format()
-
-      [%Google.Protobuf.Compiler.CodeGeneratorResponse.File{name: file_name, content: content}]
-    end
+    {package_level_extensions, module_definitions} = generate_module_definitions(ctx, desc)
+
+    files =
+      if ctx.one_file_per_module? do
+        Enum.map(module_definitions, fn {mod_name, content} ->
+          file_name = Macro.underscore(mod_name) <> ".pb.ex"
+          %Google.Protobuf.Compiler.CodeGeneratorResponse.File{name: file_name, content: content}
+        end)
+      else
+        # desc.name is the filename, ending in ".proto".
+        file_name = Path.rootname(desc.name) <> ".pb.ex"
+
+        content =
+          module_definitions
+          |> Enum.map(fn {_mod_name, contents} -> [contents, ?\n] end)
+          |> IO.iodata_to_binary()
+          |> Generator.Util.format()
+
+        [%Google.Protobuf.Compiler.CodeGeneratorResponse.File{name: file_name, content: content}]
+      end
+
+    {package_level_extensions, files}
   end
 
   defp generate_module_definitions(ctx, %Google.Protobuf.FileDescriptorProto{} = desc) do
@@ -41,14 +42,12 @@ defmodule Protobuf.Protoc.Generator do
       }
       |> Protobuf.Protoc.Context.custom_file_options_from_file_desc(desc)
 
-    nested_extensions = Generator.Extension.get_nested_extensions(ctx, desc.message_type)
-
     enum_defmodules = Enum.map(desc.enum_type, &Generator.Enum.generate(ctx, &1))
 
     {nested_enum_defmodules, message_defmodules} =
       Generator.Message.generate_list(ctx, desc.message_type)
 
-    extension_defmodules = Generator.Extension.generate(ctx, desc, nested_extensions)
+    {package_level_extensions, extension_defmodules} = Generator.Extension.generate(ctx, desc)
 
     service_defmodules =
       if "grpc" in ctx.plugins do
@@ -57,13 +56,16 @@ defmodule Protobuf.Protoc.Generator do
         []
       end
 
-    List.flatten([
-      enum_defmodules,
-      nested_enum_defmodules,
-      message_defmodules,
-      service_defmodules,
-      extension_defmodules
-    ])
+    defmodules =
+      List.flatten([
+        enum_defmodules,
+        nested_enum_defmodules,
+        message_defmodules,
+        service_defmodules,
+        extension_defmodules
+      ])
+
+    {package_level_extensions, defmodules}
   end
 
   defp get_dep_type_mapping(%Context{global_type_mapping: global_mapping}, deps, file_name) do