Fix negative zeroes in OTP27+ (#370)

According to the official documentation in
https://protobuf.dev/programming-guides/proto3/#default as of 09/05/2024,
if a float or double value is set to +0 it will not be serialized, but
if it is set to -0 it should be serialized.

Currently there are no conformance tests for this behaviour.

Co-authored-by: Andrea Leopardi <[email protected]>

Author: v0idpwn

.github/workflows/main.yml

@@ -15,6 +15,8 @@ jobs:
       fail-fast: false
       matrix:
         include:
+          - otp: 25.0
+            elixir: 1.16.2
           - otp: 25.0
             elixir: 1.14.0
             lint: true

lib/protobuf/encoder.ex

@@ -83,7 +83,7 @@ defmodule Protobuf.Encoder do
 
   defp skip_field?(:proto3, nil, _prop), do: true
   defp skip_field?(:proto3, 0, %FieldProps{oneof: nil}), do: true
-  defp skip_field?(:proto3, 0.0, %FieldProps{oneof: nil}), do: true
+  defp skip_field?(:proto3, +0.0, %FieldProps{oneof: nil}), do: true
   defp skip_field?(:proto3, "", %FieldProps{oneof: nil}), do: true
   defp skip_field?(:proto3, false, %FieldProps{oneof: nil}), do: true
   defp skip_field?(_syntax, _val, _prop), do: false

lib/protobuf/json/encode.ex

@@ -309,7 +309,13 @@ defmodule Protobuf.JSON.Encode do
   defp maybe_repeat(%{repeated?: false}, val, fun), do: fun.(val)
   defp maybe_repeat(%{repeated?: true}, val, fun), do: Enum.map(val, fun)
 
-  defp default?(_prop, value) when value in [nil, 0, false, [], "", 0.0, %{}], do: true
+  defp default?(_prop, +0.0), do: true
+  defp default?(_prop, nil), do: true
+  defp default?(_prop, 0), do: true
+  defp default?(_prop, false), do: true
+  defp default?(_prop, []), do: true
+  defp default?(_prop, ""), do: true
+  defp default?(_prop, %{} = map) when map_size(map) == 0, do: true
   defp default?(%{type: {:enum, enum}}, key) when is_atom(key), do: enum.value(key) == 0
   defp default?(_prop, _value), do: false
 

mix.lock

@@ -2,7 +2,7 @@
   "bunt": {:hex, :bunt, "0.2.0", "951c6e801e8b1d2cbe58ebbd3e616a869061ddadcc4863d0a2182541acae9a38", [:mix], [], "hexpm", "7af5c7e09fe1d40f76c8e4f9dd2be7cebd83909f31fee7cd0e9eadc567da8353"},
   "certifi": {:hex, :certifi, "2.8.0", "d4fb0a6bb20b7c9c3643e22507e42f356ac090a1dcea9ab99e27e0376d695eba", [:rebar3], [], "hexpm", "6ac7efc1c6f8600b08d625292d4bbf584e14847ce1b6b5c44d983d273e1097ea"},
   "credo": {:hex, :credo, "1.5.6", "e04cc0fdc236fefbb578e0c04bd01a471081616e741d386909e527ac146016c6", [:mix], [{:bunt, "~> 0.2.0", [hex: :bunt, repo: "hexpm", optional: false]}, {:file_system, "~> 0.2.8", [hex: :file_system, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: false]}], "hexpm", "4b52a3e558bd64e30de62a648518a5ea2b6e3e5d2b164ef5296244753fc7eb17"},
-  "dialyxir": {:hex, :dialyxir, "1.1.0", "c5aab0d6e71e5522e77beff7ba9e08f8e02bad90dfbeffae60eaf0cb47e29488", [:mix], [{:erlex, ">= 0.2.6", [hex: :erlex, repo: "hexpm", optional: false]}], "hexpm", "07ea8e49c45f15264ebe6d5b93799d4dd56a44036cf42d0ad9c960bc266c0b9a"},
+  "dialyxir": {:hex, :dialyxir, "1.4.3", "edd0124f358f0b9e95bfe53a9fcf806d615d8f838e2202a9f430d59566b6b53b", [:mix], [{:erlex, ">= 0.2.6", [hex: :erlex, repo: "hexpm", optional: false]}], "hexpm", "bf2cfb75cd5c5006bec30141b131663299c661a864ec7fbbc72dfa557487a986"},
   "earmark_parser": {:hex, :earmark_parser, "1.4.29", "149d50dcb3a93d9f3d6f3ecf18c918fb5a2d3c001b5d3305c926cddfbd33355b", [:mix], [], "hexpm", "4902af1b3eb139016aed210888748db8070b8125c2342ce3dcae4f38dcc63503"},
   "erlex": {:hex, :erlex, "0.2.6", "c7987d15e899c7a2f34f5420d2a2ea0d659682c06ac607572df55a43753aa12e", [:mix], [], "hexpm", "2ed2e25711feb44d52b17d2780eabf998452f6efda104877a3881c2f8c0c0c75"},
   "ex_doc": {:hex, :ex_doc, "0.29.1", "b1c652fa5f92ee9cf15c75271168027f92039b3877094290a75abcaac82a9f77", [:mix], [{:earmark_parser, "~> 1.4.19", [hex: :earmark_parser, repo: "hexpm", optional: false]}, {:makeup_elixir, "~> 0.14", [hex: :makeup_elixir, repo: "hexpm", optional: false]}, {:makeup_erlang, "~> 0.1", [hex: :makeup_erlang, repo: "hexpm", optional: false]}], "hexpm", "b7745fa6374a36daf484e2a2012274950e084815b936b1319aeebcf7809574f6"},
@@ -19,7 +19,7 @@
   "mimerl": {:hex, :mimerl, "1.2.0", "67e2d3f571088d5cfd3e550c383094b47159f3eee8ffa08e64106cdf5e981be3", [:rebar3], [], "hexpm", "f278585650aa581986264638ebf698f8bb19df297f66ad91b18910dfc6e19323"},
   "nimble_parsec": {:hex, :nimble_parsec, "1.2.3", "244836e6e3f1200c7f30cb56733fd808744eca61fd182f731eac4af635cc6d0b", [:mix], [], "hexpm", "c8d789e39b9131acf7b99291e93dae60ab48ef14a7ee9d58c6964f59efb570b0"},
   "parse_trans": {:hex, :parse_trans, "3.3.1", "16328ab840cc09919bd10dab29e431da3af9e9e7e7e6f0089dd5a2d2820011d8", [:rebar3], [], "hexpm", "07cd9577885f56362d414e8c4c4e6bdf10d43a8767abb92d24cbe8b24c54888b"},
-  "ssl_verify_fun": {:hex, :ssl_verify_fun, "1.1.6", "cf344f5692c82d2cd7554f5ec8fd961548d4fd09e7d22f5b62482e5aeaebd4b0", [:make, :mix, :rebar3], [], "hexpm", "bdb0d2471f453c88ff3908e7686f86f9be327d065cc1ec16fa4540197ea04680"},
+  "ssl_verify_fun": {:hex, :ssl_verify_fun, "1.1.7", "354c321cf377240c7b8716899e182ce4890c5938111a1296add3ec74cf1715df", [:make, :mix, :rebar3], [], "hexpm", "fe4c190e8f37401d30167c8c405eda19469f34577987c76dde613e838bbc67f8"},
   "stream_data": {:hex, :stream_data, "0.5.0", "b27641e58941685c75b353577dc602c9d2c12292dd84babf506c2033cd97893e", [:mix], [], "hexpm", "012bd2eec069ada4db3411f9115ccafa38540a3c78c4c0349f151fc761b9e271"},
   "unicode_util_compat": {:hex, :unicode_util_compat, "0.7.0", "bc84380c9ab48177092f43ac89e4dfa2c6d62b40b8bd132b1059ecc7232f9a78", [:rebar3], [], "hexpm", "25eee6d67df61960cf6a794239566599b09e17e668d3700247bc498638152521"},
 }

test/protobuf/dsl_test.exs

@@ -165,18 +165,20 @@ defmodule Protobuf.DSLTest do
     assert TestMsg.EnumFoo.key(213_123) == 213_123
     assert TestMsg.EnumFoo.mapping() == %{UNKNOWN: 0, A: 1, B: 2, C: 4, D: 4, E: 4}
 
-    assert TestMsg.EnumFoo.__reverse_mapping__() == %{
+    assert %{
              0 => :UNKNOWN,
              1 => :A,
              2 => :B,
-             4 => :E,
              "A" => :A,
              "B" => :B,
              "C" => :C,
              "UNKNOWN" => :UNKNOWN,
              "D" => :D,
              "E" => :E
-           }
+           } = TestMsg.EnumFoo.__reverse_mapping__()
+
+    # Varies with erlang version
+    assert TestMsg.EnumFoo.__reverse_mapping__()[4] in [:C, :D, :E]
 
     assert %FieldProps{fnum: 11, type: {:enum, TestMsg.EnumFoo}, wire_type: 0} =
              Foo.__message_props__().field_props[11]

test/protobuf/encoder_test.exs

@@ -171,6 +171,18 @@ defmodule Protobuf.EncoderTest do
     assert Encoder.encode(msg) == <<>>
   end
 
+  if System.otp_release() < "27" do
+    @tag :skip
+  end
+
+  test "skips float positive zero, but doesn't skip negative zero" do
+    msg = %TestMsg.Foo{d: 0.0, n: 0.0}
+    assert Encoder.encode(msg) == <<>>
+
+    msg = %TestMsg.Foo{d: -0.0, n: -0.0}
+    refute Encoder.encode(msg) == <<>>
+  end
+
   test "encodes map with oneof" do
     msg = %Google.Protobuf.Struct{fields: %{"valid" => %{kind: {:bool_value, true}}}}
     bin = Google.Protobuf.Struct.encode(msg)

test/protobuf/json/decode_test.exs

@@ -341,8 +341,8 @@ defmodule Protobuf.JSON.DecodeTest do
 
   describe "enums" do
     test "known integer value is valid" do
-      data = %{"j" => 4}
-      decoded = %Foo{j: :E}
+      data = %{"j" => 1}
+      decoded = %Foo{j: :A}
       assert decode(data, Foo) == {:ok, decoded}
     end
 

test/protobuf/json/encode_test.exs

@@ -108,6 +108,15 @@ defmodule Protobuf.JSON.EncodeTest do
     assert encode(message) == %{"float" => "-Infinity", "double" => "-Infinity"}
   end
 
+  if System.otp_release() < "27" do
+    @tag :skip
+  end
+
+  test "encodes negative zero" do
+    message = %Scalars{float: -0.0, double: -0.0}
+    assert %{"float" => -0.0, "double" => -0.0} = encode(message)
+  end
+
   test "encodes bytes in Base64 with padding" do
     message = %Scalars{bytes: "abcde"}
     assert encode(message) == %{"bytes" => "YWJjZGU="}

test/protobuf/protoc/generator/message_test.exs

@@ -37,8 +37,9 @@ defmodule Protobuf.Protoc.Generator.MessageTest do
     {[], [{_mod, msg}]} = Generator.generate(ctx, desc)
     assert msg =~ "defmodule Foo do\n"
 
-    assert msg =~
-             "use Protobuf, protoc_gen_elixir_version: \"#{Util.version()}\", syntax: :proto3\n"
+    assert msg =~ "use Protobuf"
+    assert msg =~ "protoc_gen_elixir_version: #{inspect(Util.version())}"
+    assert msg =~ "syntax: :proto3"
 
     assert [{compiled_mod, bytecode}] = Code.compile_string(msg)
 

test/test_helper.exs

@@ -51,7 +51,7 @@ defmodule Protobuf.TestHelpers do
 
   # This code is taken from Code.fetch_docs/1 in Elixir (v1.13 in particular).
   def fetch_docs_from_bytecode(bytecode) when is_binary(bytecode) do
-    docs_chunk = 'Docs'
+    docs_chunk = ~c"Docs"
     assert {:ok, {_module, [{^docs_chunk, bin}]}} = :beam_lib.chunks(bytecode, [docs_chunk])
     :erlang.binary_to_term(bin)
   end