Repository: elixir-protobuf/protobuf
Pull request #412 (2025-05-01 15:46:59)

 Remove deprecated support for own `t()` and `defstruct` in Protobufs 

v0idpwn: Addresses #404 

Patch:
diff --git a/lib/protobuf.ex b/lib/protobuf.ex
index 5a3980a..345e910 100644
--- a/lib/protobuf.ex
+++ b/lib/protobuf.ex
@@ -64,24 +64,6 @@ defmodule Protobuf do
 
       @behaviour Protobuf
 
-      @deprecated "Build the struct by hand with %MyMessage{...} or use struct/1"
-      @impl unquote(__MODULE__)
-      def new() do
-        Protobuf.Builder.new(__MODULE__)
-      end
-
-      @deprecated "Build the struct by hand with %MyMessage{...} or use struct/2"
-      @impl unquote(__MODULE__)
-      def new(attrs) do
-        Protobuf.Builder.new(__MODULE__, attrs)
-      end
-
-      @deprecated "Build the struct by hand with %MyMessage{...} or use struct!/2"
-      @impl unquote(__MODULE__)
-      def new!(attrs) do
-        Protobuf.Builder.new!(__MODULE__, attrs)
-      end
-
       @impl unquote(__MODULE__)
       def transform_module() do
         nil
@@ -98,59 +80,6 @@ defmodule Protobuf do
     end
   end
 
-  @doc """
-  Builds a blank struct with default values.
-
-  > #### Deprecated {: .warning}
-  >
-  > This is deprecated in favor of building the struct with `%MyMessage{...}` or using
-  > `struct/1`.
-
-  """
-  @callback new() :: struct()
-
-  @doc """
-  Builds and updates the struct with passed fields.
-
-  This function will:
-
-    * Recursively call `c:new/1` for embedded fields
-    * Create structs using `struct/2` for keyword lists and maps
-    * Create the correct struct if passed the wrong struct
-    * Call `c:new/1` for each element in the list for repeated fields
-
-  > #### Deprecated {: .warning}
-  > This is deprecated in favor of building the struct with `%MyMessage{...}` or using
-  > `struct/2`.
-
-  ## Examples
-
-      MyMessage.new(field1: "foo")
-      #=> %MyMessage{field1: "foo", ...}
-
-      MyMessage.new(field1: [field2: "foo"])
-      #=> %MyMessage{field1: %MySubMessage{field2: "foo"}}
-
-      MyMessage.new(field1: %WrongStruct{field2: "foo"})
-      #=> %MyMessage{field1: %MySubMessage{field2: "foo"}}
-
-      MyMessage.new(repeated: [%{field1: "foo"}, %{field1: "bar"}])
-      #=> %MyMessage{repeated: [%MyRepeated{field1: "foo"}, %MyRepeated{field1: "bar"}]}
-
-  """
-  @callback new(attributes :: Enum.t()) :: struct
-
-  @doc """
-  Similar to `c:new/1`, but use `struct!/2` to build the struct, so
-  errors will be raised if unknown keys are passed.
-
-  > #### Deprecated {: .warning}
-  > This is deprecated in favor of building the struct with `%MyMessage{...}` or using
-  > `struct!/2` directly.
-
-  """
-  @callback new!(attributes :: Enum.t()) :: struct()
-
   @doc """
   Encodes the given struct into to a Protobuf binary.
 
@@ -253,7 +182,7 @@ defmodule Protobuf do
 
   You encode this:
 
-      payload = Protobuf.encode(User.new!(email: "user@example.com"))
+      payload = Protobuf.encode(%User{email: "user@example.com"})
       #=> <<...>>
 
   Now, you try to decode this payload using this schema instead:
diff --git a/lib/protobuf/builder.ex b/lib/protobuf/builder.ex
deleted file mode 100644
index 0c71e91..0000000
--- a/lib/protobuf/builder.ex
+++ /dev/null
@@ -1,80 +0,0 @@
-defmodule Protobuf.Builder do
-  @moduledoc false
-
-  # TODO: Elixir already provides a slew of nice features to build structs from enums.
-  # The main ones are struct/1 and struct!/1. These provide the correct level of validation
-  # (raise on unknown keys, ignore unknown keys, etc). We should reduce the API surface
-  # of this library and use those instead. Protobuf defaults are, by design, compile-time
-  # values, which means that we can generate the right struct defaults.
-  # The only problem with removing new/2 and new!/2 is that we lose the ability to
-  # encode embedded messages from structs, but I (Andrea) think that's a reasonable
-  # tradeoff.
-
-  alias Protobuf.FieldProps
-
-  @spec new(module, Enum.t()) :: %{required(:__struct__) => module, optional(atom()) => any()}
-        when module: module()
-  def new(mod, attrs \\ []) when is_atom(mod) do
-    new_maybe_strict(mod, attrs, _strict? = false)
-  end
-
-  @spec new!(module, Enum.t()) :: %{required(:__struct__) => module, optional(atom()) => any()}
-        when module: module()
-  def new!(mod, attrs) when is_atom(mod) do
-    new_maybe_strict(mod, attrs, _strict? = true)
-  end
-
-  defp new_maybe_strict(mod, attrs, strict?) do
-    case attrs do
-      # If the attrs is the module, we just return it.
-      %{__struct__: ^mod} ->
-        attrs
-
-      # If "attrs" is a struct but not the same struct as "mod", then we raise if are being
-      # strict.
-      %{__struct__: _} = struct when strict? ->
-        raise ArgumentError,
-          message: "Struct %#{inspect(mod)}{} was expected, but given #{inspect(struct)}"
-
-      # If "attrs" is a struct but not the same struct as "mod", then we use it as attributes
-      # to build our new struct:
-      %{__struct__: _} ->
-        new_from_enum(mod, Map.from_struct(attrs), strict?)
-
-      not_a_struct ->
-        new_from_enum(mod, not_a_struct, strict?)
-    end
-  end
-
-  defp new_from_enum(mod, attrs, strict?) do
-    props = mod.__message_props__()
-    msg = if strict?, do: struct!(mod, attrs), else: struct(mod, attrs)
-
-    Enum.reduce(props.embedded_fields, msg, fn field_name, acc ->
-      case msg do
-        %{^field_name => value} when not is_nil(value) ->
-          case props.field_props[props.field_tags[field_name]] do
-            %FieldProps{embedded?: true, repeated?: true, type: type} ->
-              if type.transform_module() do
-                acc
-              else
-                %{acc | field_name => Enum.map(value, &type.new/1)}
-              end
-
-            %FieldProps{embedded?: true, repeated?: false, type: type} ->
-              if type.transform_module() do
-                acc
-              else
-                %{acc | field_name => type.new(value)}
-              end
-
-            _other ->
-              acc
-          end
-
-        _ ->
-          acc
-      end
-    end)
-  end
-end
diff --git a/lib/protobuf/dsl.ex b/lib/protobuf/dsl.ex
index 04b9dfd..cb153ff 100644
--- a/lib/protobuf/dsl.ex
+++ b/lib/protobuf/dsl.ex
@@ -174,58 +174,17 @@ defmodule Protobuf.DSL do
         unquote(Macro.escape(msg_props))
       end
 
-      cond do
-        # If this is an enum and "@type t()" is already called, it's fine because it's likely
-        # the old code generated by this library.
-        unquote(msg_props.enum?) and unquote(defines_t_type?) ->
-          IO.warn("""
-          Since v0.10.0 of the :protobuf library, the t/0 type in Protobuf enum modules \
-          is automatically generated by "use Protobuf". Remove your explicit definition or \
-          regenerate the files with a newer version of the protoc-gen-elixir escript. \
-          This warning will become an error in version 0.11.0+ of this library.
-          """)
-
-        # If both "defstruct" and "@type t()" are called, it's probably okay because it's the code
-        # we used to generated before from this library, but we want to get rid of it, so we warn.
-        unquote(defines_defstruct?) and unquote(defines_t_type?) ->
-          IO.warn("""
-          Since v0.10.0 of the :protobuf library, the t/0 type and the struct are automatically \
-          generated for modules that call "use Protobuf" if they are Protobuf enums or messages. \
-          Remove your explicit definition of both of these or regenerate the files with the \
-          latest version of the protoc-gen-elixir plugin. This warning will become an error \
-          in version 0.11.0+ of the :protobuf library.\
-          """)
-
-        # If users defined only "defstruct" OR "@type t()", it means either they didn't generate
-        # the code through this library or they modified the generated files. In either case,
-        # let's raise here since we could have inconsistencies between the user-defined spec/type
-        # and our type/spec, respectively.
-        unquote(not msg_props.enum?) and (unquote(defines_defstruct?) or unquote(defines_t_type?)) ->
-          what = if unquote(defines_defstruct?), do: "defstruct/1", else: "@type t() :: ..."
-
-          raise """
-          since v0.10.0 of the :protobuf library, the t/0 type and the struct are automatically \
-          generated for modules that call "use Protobuf" if they are Protobuf enums or messages. \
-          This was the case for #{inspect(unquote(__MODULE__))}, where you call #{what}. \
-          This could cause inconsistencies with the type or struct generated by the library. \
-          You can either:
-
-            * make sure that you define both the t/0 type as well as the struct, but that will
-              become an error in later versions of the Protobuf library
-
-            * remove both the t/0 type definition as well as the struct definition and let the
-              library define both
-
-            * regenerate the file from the Protobuf source definition with the latest version
-              of the protoc-gen-elixir plugin, which won't generate the struct or the t/0 type
-              definition
-
-          """
-
-        # Newest version of this library generate both the t/0 type as well as the struct.
-        true ->
-          unquote(def_t_typespec(msg_props, extension_props, transform_module_ast))
-          unquote(gen_defstruct(msg_props))
+      if unquote(defines_defstruct?) or unquote(defines_t_type?) do
+        raise Protobuf.InvalidError, """
+        since v0.10.0 of the :protobuf library, the t/0 type and the struct are automatically \
+        generated for modules that call "use Protobuf" if they are Protobuf enums or messages, \
+        and their usage was deprecated. Since v0.15.0, they are errors. Remove your explicit \
+        definition of both of these or regenerate the files with the latest version of the \
+        protoc-gen-elixir plugin.\
+        """
+      else
+        unquote(def_t_typespec(msg_props, extension_props, transform_module_ast))
+        unquote(gen_defstruct(msg_props))
       end
 
       unquote(msg_props.enum? && Protobuf.DSL.Enum.quoted_enum_functions(msg_props))
diff --git a/lib/protobuf/transform_module.ex b/lib/protobuf/transform_module.ex
index 76fe066..3a5f80e 100644
--- a/lib/protobuf/transform_module.ex
+++ b/lib/protobuf/transform_module.ex
@@ -5,9 +5,6 @@ defmodule Protobuf.TransformModule do
   By defining a `transform_module/0` function on your protobuf message module
   you can add custom encoding and decoding logic for your message.
 
-  The `c:Protobuf.new/1` function will not be called for structs that have a transform module, if
-  you still want to emulate this behavior you can use `Protobuf.TransformModule.InferFieldsFromEnum`.
-
   As an example we can use this to implement a message that will be decoded as a string value:
 
       defmodule StringMessage do
diff --git a/lib/protobuf/transform_module/infer_fields_from_enum.ex b/lib/protobuf/transform_module/infer_fields_from_enum.ex
deleted file mode 100644
index 646414c..0000000
--- a/lib/protobuf/transform_module/infer_fields_from_enum.ex
+++ /dev/null
@@ -1,15 +0,0 @@
-defmodule Protobuf.TransformModule.InferFieldsFromEnum do
-  @moduledoc """
-  For structs with a `Protobuf.TransformModule` set, the `c:Protobuf.new/1` function
-  does not get called. This transform module brings back the original behavior of calling
-  `c:Protobuf.new/1` before the value is encoded.
-  """
-
-  @behaviour Protobuf.TransformModule
-
-  @impl true
-  def encode(value, type), do: type.new(value)
-
-  @impl true
-  def decode(value, _type), do: value
-end


Test patch:
diff --git a/test/protobuf/builder_test.exs b/test/protobuf/builder_test.exs
index 7bd1d0b..897382d 100644
--- a/test/protobuf/builder_test.exs
+++ b/test/protobuf/builder_test.exs
@@ -1,7 +1,7 @@
 defmodule Protobuf.BuilderTest do
   use ExUnit.Case, async: true
 
-  alias TestMsg.{Foo, Foo2, ContainsTransformModule, Proto3Optional}
+  alias TestMsg.{Foo, Foo2, Proto3Optional}
 
   describe "default values for structs" do
     test "for proto3" do
@@ -29,60 +29,5 @@ defmodule Protobuf.BuilderTest do
       assert %Foo{e: %Foo.Bar{a: 1}}.e == %Foo.Bar{a: 1}
       assert %Foo{h: [%Foo.Bar{a: 1}]}.h == [%Foo.Bar{a: 1}]
     end
-
-    test "new/2 doesn't build embedded messages for nil and maps" do
-      assert %Foo{e: nil}.e == nil
-      assert %Foo{}.e == nil
-      assert %Foo{l: %{"k" => 1}}.l == %{"k" => 1}
-    end
-  end
-
-  # TODO: remove these tests once we remove new/2.
-  describe "new/2" do
-    test "doesn't process struct" do
-      msg = %Foo{}
-      assert msg == Foo.new(msg)
-    end
-
-    test "raises an error for non-list repeated embedded msgs" do
-      # TODO: remove conditional once we support only Elixir 1.18+
-      message =
-        if System.version() >= "1.18.0" do
-          ~r/protocol Enumerable not implemented for type TestMsg.Foo.Ba/
-        else
-          ~r/protocol Enumerable not implemented for %TestMsg.Foo.Bar/
-        end
-
-      assert_raise Protocol.UndefinedError, message, fn ->
-        Foo.new(h: Foo.Bar.new())
-      end
-    end
-
-    test "builds correct message for non matched struct" do
-      foo = Foo.new(Foo2.new(non_matched: 1))
-
-      assert_raise Protobuf.EncodeError, fn ->
-        Foo.encode(foo)
-      end
-    end
-
-    test "ignores structs with transform modules" do
-      assert ContainsTransformModule.new(field: 123) == %ContainsTransformModule{field: 123}
-    end
-  end
-
-  # TODO: remove these tests once we remove new!/2.
-  describe "new!/2" do
-    test "raises for fields that don't exist in the schema" do
-      assert_raise KeyError, fn ->
-        Foo.new!(nonexisting_field: "foo")
-      end
-    end
-
-    test "raises for non-matched struct" do
-      assert_raise ArgumentError, fn ->
-        Foo.new!(%Foo2{})
-      end
-    end
   end
 end
diff --git a/test/protobuf/dsl_test.exs b/test/protobuf/dsl_test.exs
index 548c48e..999a8ef 100644
--- a/test/protobuf/dsl_test.exs
+++ b/test/protobuf/dsl_test.exs
@@ -1,8 +1,6 @@
 defmodule Protobuf.DSLTest do
   use ExUnit.Case, async: true
 
-  import ExUnit.CaptureIO
-
   alias Protobuf.{FieldProps, MessageProps}
   alias TestMsg.{Foo, Foo2, Proto3Optional}
 
@@ -190,38 +188,6 @@ defmodule Protobuf.DSLTest do
     refute msg_props.field_props[11].embedded?
   end
 
-  # TODO: remove once we remove new/0 and new/1.
-  test "generates new/0 and new/1 functions" do
-    assert %Foo{
-             a: 0,
-             c: "",
-             d: +0.0,
-             e: nil,
-             f: 0,
-             g: [],
-             h: [],
-             i: [],
-             j: :UNKNOWN,
-             k: false,
-             l: %{}
-           } = Foo.new()
-
-    assert %Foo{
-             a: 1,
-             b: 42,
-             c: "abc",
-             d: +0.0,
-             e: %Foo.Bar{a: 2, b: "asd"},
-             f: 0,
-             g: [],
-             h: [],
-             i: [],
-             j: :UNKNOWN,
-             k: false,
-             l: %{}
-           } = Foo.new(%{a: 1, b: 42, c: "abc", e: Foo.Bar.new(a: 2, b: "asd")})
-  end
-
   test "set oneof of message props" do
     msg_props = TestMsg.Oneof.__message_props__()
     assert %{oneof: [{:first, 0}, {:second, 1}]} = msg_props
@@ -237,88 +203,92 @@ defmodule Protobuf.DSLTest do
     assert TestMsg.WithTransformModule.transform_module() == TestMsg.TransformModule
   end
 
-  test "emits a warning if there is already a definition for the t/0 type for an enum" do
-    output =
-      capture_io(:stderr, fn ->
-        Code.eval_quoted(
-          quote do
-            defmodule MessageWithWarning do
-              use Protobuf, syntax: :proto3, enum: true
-
-              @type t() :: integer() | :FOO
-
-              field :FOO, 0, type: :bool
-            end
-          end
-        )
-      end)
-
-    assert output =~ "the t/0 type in Protobuf enum modules is automatically generated"
+  test "raises a compilation error if there is already a definition for the t/0 type for an enum" do
+    assert_raise Protobuf.InvalidError,
+                 ~r{t/0 type and the struct are automatically generated},
+                 fn ->
+                   Code.eval_quoted(
+                     quote do
+                       defmodule MessageWithWarning do
+                         use Protobuf, syntax: :proto3, enum: true
+
+                         @type t() :: integer() | :FOO
+
+                         field :FOO, 0, type: :bool
+                       end
+                     end
+                   )
+                 end
   end
 
-  test "emits a warning if there is already a call to defstruct/1 and a definition for the t/0 type" do
-    output =
-      capture_io(:stderr, fn ->
-        Code.eval_quoted(
-          quote do
-            defmodule MessageWithWarning do
-              use Protobuf, syntax: :proto3
-
-              @type t() :: %__MODULE__{foo: boolean()}
+  test "raises a compilation error if there is already a call to defstruct/1 and a definition for the t/0 type" do
+    assert_raise Protobuf.InvalidError,
+                 ~r{t/0 type and the struct are automatically generated},
+                 fn ->
+                   Code.eval_quoted(
+                     quote do
+                       defmodule MessageWithWarning do
+                         use Protobuf, syntax: :proto3
 
-              defstruct [:foo]
+                         @type t() :: %__MODULE__{foo: boolean()}
 
-              field :foo, 1, type: :bool
-            end
-          end
-        )
-      end)
+                         defstruct [:foo]
 
-    assert output =~ "t/0 type and the struct are automatically generated"
+                         field :foo, 1, type: :bool
+                       end
+                     end
+                   )
+                 end
   end
 
   test "raises a compilation error if there is already a call to defstruct/1 but no definition for the t/0 type" do
-    assert_raise RuntimeError, ~r{t/0 type and the struct are automatically generated}, fn ->
-      Code.eval_quoted(
-        quote do
-          defmodule MessageWithDefstructError do
-            use Protobuf, syntax: :proto3
-
-            defstruct [:foo]
-
-            field :foo, 1, type: :bool
-          end
-        end
-      )
-    end
+    assert_raise Protobuf.InvalidError,
+                 ~r{t/0 type and the struct are automatically generated},
+                 fn ->
+                   Code.eval_quoted(
+                     quote do
+                       defmodule MessageWithDefstructError do
+                         use Protobuf, syntax: :proto3
+
+                         defstruct [:foo]
+
+                         field :foo, 1, type: :bool
+                       end
+                     end
+                   )
+                 end
   end
 
   test "raises a compilation error if there is already a definition for the t/0 type but no defstruct" do
-    assert_raise RuntimeError, ~r{the t/0 type and the struct are automatically generated}, fn ->
-      Code.eval_quoted(
-        quote do
-          defmodule MessageWithTTypeError do
-            use Protobuf, syntax: :proto3
-
-            @type t() :: %__MODULE__{foo: boolean()}
-
-            field :foo, 1, type: :bool
-          end
-        end
-      )
-    end
+    assert_raise Protobuf.InvalidError,
+                 ~r{the t/0 type and the struct are automatically generated},
+                 fn ->
+                   Code.eval_quoted(
+                     quote do
+                       defmodule MessageWithTTypeError do
+                         use Protobuf, syntax: :proto3
+
+                         @type t() :: %__MODULE__{foo: boolean()}
+
+                         field :foo, 1, type: :bool
+                       end
+                     end
+                   )
+                 end
   end
 
   test "raises a compilation error if syntax is proto3 and the first enum has tag other than 0" do
-    assert_raise RuntimeError, "the first enum value must have tag 0 in proto3, got: 1", fn ->
-      Code.eval_quoted(
-        quote do
-          defmodule MessageWithProto3BadEnumTag do
-            use Protobuf, syntax: :proto3, enum: true
-            field :NOT_ZERO, 1
-          end
-        end
-      )
-    end
+    assert_raise RuntimeError,
+                 "the first enum value must have tag 0 in proto3, got: 1",
+                 fn ->
+                   Code.eval_quoted(
+                     quote do
+                       defmodule MessageWithProto3BadEnumTag do
+                         use Protobuf, syntax: :proto3, enum: true
+                         field :NOT_ZERO, 1
+                       end
+                     end
+                   )
+                 end
   end
 end
diff --git a/test/protobuf/encoder_test.exs b/test/protobuf/encoder_test.exs
index 98873c0..7123707 100644
--- a/test/protobuf/encoder_test.exs
+++ b/test/protobuf/encoder_test.exs
@@ -335,14 +335,4 @@ defmodule Protobuf.EncoderTest do
     assert Encoder.encode(msg) == <<>>
     assert TestMsg.ContainsTransformModule.decode(Encoder.encode(msg)) == msg
   end
-
-  test "NewTransform calls new/2 before encoding" do
-    msg = %TestMsg.ContainsNewTransformModule{field: [field: 123]}
-    assert msg == %TestMsg.ContainsNewTransformModule{field: [field: 123]}
-
-    assert TestMsg.ContainsNewTransformModule.decode(Encoder.encode(msg)) ==
-             %TestMsg.ContainsNewTransformModule{
-               field: %TestMsg.WithNewTransformModule{field: 123}
-             }
-  end
 end
diff --git a/test/protobuf/protoc/cli_integration_test.exs b/test/protobuf/protoc/cli_integration_test.exs
index 081a8b7..fd5ca99 100644
--- a/test/protobuf/protoc/cli_integration_test.exs
+++ b/test/protobuf/protoc/cli_integration_test.exs
@@ -45,7 +45,7 @@ defmodule Protobuf.Protoc.CLIIntegrationTest do
       protoc!([
         "--proto_path=#{tmp_dir}",
         "--elixir_out=#{tmp_dir}",
-        "--elixir_opt=transform_module=Protobuf.TransformModule.InferFieldsFromEnum",
+        "--elixir_opt=transform_module=TestMsg.TransformModule",
         "--plugin=./protoc-gen-elixir",
         proto_path
       ])
@@ -53,7 +53,7 @@ defmodule Protobuf.Protoc.CLIIntegrationTest do
       assert [mod] = compile_file_and_clean_modules_on_exit("#{tmp_dir}/user.pb.ex")
       assert mod == Foo.User
 
-      assert mod.transform_module() == Protobuf.TransformModule.InferFieldsFromEnum
+      assert mod.transform_module() == TestMsg.TransformModule
     end
 
     test "gen_descriptors option", %{tmp_dir: tmp_dir, proto_path: proto_path} do
diff --git a/test/support/test_msg.ex b/test/support/test_msg.ex
index e90b77b..0156858 100644
--- a/test/support/test_msg.ex
+++ b/test/support/test_msg.ex
@@ -296,20 +296,6 @@ defmodule TestMsg do
     field :field, 1, type: WithTransformModule
   end
 
-  defmodule WithNewTransformModule do
-    use Protobuf, syntax: :proto3
-
-    field :field, 1, type: :int32
-
-    def transform_module(), do: Protobuf.TransformModule.InferFieldsFromEnum
-  end
-
-  defmodule ContainsNewTransformModule do
-    use Protobuf, syntax: :proto3
-
-    field :field, 1, type: WithNewTransformModule
-  end
-
   defmodule TransformIntegerStrings do
     @behaviour Protobuf.TransformModule