Add warning when implicitly casting maps/keywords to struct encode-time (#413)

Author: v0idpwn

lib/protobuf/encoder.ex

@@ -142,8 +142,26 @@ defmodule Protobuf.Encoder do
           message:
             "struct #{inspect(other_mod)} can't be encoded as #{inspect(mod)}: #{inspect(struct)}"
 
-      _ ->
+      enumerable when is_map(enumerable) or is_list(enumerable) ->
+        IO.warn("""
+        Implicitly casting a non-struct to a #{inspect(mod)} message:
+
+        #{inspect(enumerable, pretty: true)}
+
+        This automatic coercion is deprecated in Protobuf 0.15 and will raise an error in future versions.
+
+        Instead of:
+          %Parent{child: %{name: ""}}
+
+        Build child structs explicitly:
+          %Parent{child: %Child{name: ""}}
+        """)
+
         do_encode_to_iodata(struct(mod, msg))
+
+      other ->
+        raise Protobuf.EncodeError,
+          message: "invalid value for type #{inspect(mod)}: #{inspect(other)}"
     end
   end
 

test/protobuf/encoder_test.exs

@@ -1,5 +1,11 @@
 defmodule Protobuf.EncoderTest do
-  use ExUnit.Case, async: true
+  # TODO: make async
+  #
+  # This is sync because we are using `capture_io` to get a
+  # deprecation warning when casting enumerables to structs.
+  # When the feature is removed, this module should be made
+  # sync again.
+  use ExUnit.Case, async: false
 
   import Protobuf.Wire.Types
 
@@ -184,7 +190,10 @@ defmodule Protobuf.EncoderTest do
   end
 
   test "encodes map with oneof" do
-    msg = %Google.Protobuf.Struct{fields: %{"valid" => %{kind: {:bool_value, true}}}}
+    msg = %Google.Protobuf.Struct{
+      fields: %{"valid" => %Google.Protobuf.Value{kind: {:bool_value, true}}}
+    }
+
     bin = Google.Protobuf.Struct.encode(msg)
 
     assert Google.Protobuf.Struct.decode(bin) ==
@@ -283,9 +292,7 @@ defmodule Protobuf.EncoderTest do
       Encoder.encode(%TestMsg.Foo{c: 123})
     end
 
-    # For Elixir 1.18+ it's `type Integer`, before, it was just `123`
-    # TODO: fix once we require Elixir 1.18+
-    message = ~r/protocol Enumerable not implemented for (123|type Integer)/
+    message = ~r/invalid value for type TestMsg.Foo.Bar: 123/
 
     assert_raise Protobuf.EncodeError, message, fn ->
       Encoder.encode(%TestMsg.Foo{e: 123})
@@ -309,6 +316,17 @@ defmodule Protobuf.EncoderTest do
     end
   end
 
+  # TODO: remove when implicit struct cast is removed
+  test "gives a warning for implicitly cast structs" do
+    warning =
+      ExUnit.CaptureIO.capture_io(:stderr, fn ->
+        assert <<50, 2, 8, 1>> = Encoder.encode(%TestMsg.Foo{e: %{a: 1}})
+      end)
+
+    assert warning =~ "Implicitly casting a non-struct to a TestMsg.Foo.Bar message"
+    assert warning =~ "%{a: 1}"
+  end
+
   test "encodes with transformer module" do
     msg = %TestMsg.ContainsTransformModule{field: 0}
     assert Encoder.encode(msg) == <<10, 0>>

test/protobuf/encoder_validation_test.exs

@@ -149,8 +149,8 @@ defmodule Protobuf.EncoderTest.Validation do
 
   test "build embedded field map when encode" do
     msg = %TestMsg.Foo{}
-    msg = %TestMsg.Foo{msg | e: %{a: 1}}
-    msg1 = %TestMsg.Foo{e: %{a: 1}}
+    msg = %TestMsg.Foo{msg | e: %TestMsg.Foo.Bar{a: 1}}
+    msg1 = %TestMsg.Foo{e: %TestMsg.Foo.Bar{a: 1}}
 
     assert Protobuf.Encoder.encode(msg) == Protobuf.Encoder.encode(msg1)
   end