Skip to content

OTP27 support; Fix handling of negative zero in OTP27+ #370

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
May 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion lib/protobuf/encoder.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 7 additions & 1 deletion lib/protobuf/json/encode.ex
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,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

Expand Down
4 changes: 2 additions & 2 deletions mix.lock
Original file line number Diff line number Diff line change
Expand Up @@ -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"},
Expand All @@ -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"},
}
8 changes: 5 additions & 3 deletions test/protobuf/dsl_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Comment on lines +180 to +181
Copy link
Collaborator Author

@v0idpwn v0idpwn May 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This enum is aliased, hence it's correct to decode to any of the aliases, according to documentation:

Though all alias values are valid during deserialization, the first value is always used when serializing.

(https://protobuf.dev/programming-guides/proto3/)

The result is deterministic, just varies by erlang version.


assert %FieldProps{fnum: 11, type: {:enum, TestMsg.EnumFoo}, wire_type: 0} =
Foo.__message_props__().field_props[11]
Expand Down
12 changes: 12 additions & 0 deletions test/protobuf/encoder_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions test/protobuf/json/decode_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Comment on lines +344 to +345
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here I just avoided using an aliased enum

assert decode(data, Foo) == {:ok, decoded}
end

Expand Down
9 changes: 9 additions & 0 deletions test/protobuf/json/encode_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,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="}
Expand Down
5 changes: 3 additions & 2 deletions test/protobuf/protoc/generator/message_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
2 changes: 1 addition & 1 deletion test/test_helper.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down