Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix warnings by conditionally compiling Decimal support #191

Merged
merged 3 commits into from
Jul 15, 2024
Merged

Fix warnings by conditionally compiling Decimal support #191

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
acf6e97
Fix warnings by conditionally compiling Decimal support
fhunleth Jul 12, 2024
a74a0e6
Remove unneeded workarounds for xref warnings
fhunleth Jul 13, 2024
c76db8b
Raise if trying to decode decimals without decimal
fhunleth Jul 13, 2024
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
21 changes: 12 additions & 9 deletions lib/decoder.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,15 +86,18 @@ defmodule Jason.Decoder do
end
end

defp float_decode_function(%{floats: :decimals}) do
fn string, token, skip ->
# silence xref warning
decimal = Decimal
try do
decimal.new(string)
rescue
Decimal.Error ->
token_error(token, skip)
if Code.ensure_loaded?(Decimal) do
defp float_decode_function(%{floats: :decimals}) do
fn string, token, skip ->
# silence xref warning
decimal = Decimal

try do
decimal.new(string)
rescue
Decimal.Error ->
token_error(token, skip)
end
Copy link
Owner

Choose a reason for hiding this comment

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

I think at this point, we should just remove the old work-around for silencing the warnings

Suggested change
if Code.ensure_loaded?(Decimal) do
defp float_decode_function(%{floats: :decimals}) do
fn string, token, skip ->
# silence xref warning
decimal = Decimal
try do
decimal.new(string)
rescue
Decimal.Error ->
token_error(token, skip)
end
if Code.ensure_loaded?(Decimal) do
defp float_decode_function(%{floats: :decimals}) do
fn string, token, skip ->
try do
Decimal.new(string)
rescue
Decimal.Error ->
token_error(token, skip)
end

Copy link
Owner

Choose a reason for hiding this comment

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

Additionally, do you think it would make sense to have a function that would emit a nice error if this option is passed but decimal was not included? I think this would be a good usability upgrade.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think at this point, we should just remove the old work-around for silencing the warnings

Oh good. I almost did that and then wasn't 100% sure there wasn't some weird edge case. I added a commit to the PR that removes them.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Additionally, do you think it would make sense to have a function that would emit a nice error if this option is passed but decimal was not included? I think this would be a good usability upgrade.

Yes, that seems nice. I raised in the decoder with a message I think is consistent with the library. The encoder calls seemed fine since they required Decimal structs to be passed.

end
end
end
Expand Down
10 changes: 6 additions & 4 deletions lib/encode.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -256,10 +256,12 @@ defmodule Jason.Encode do
end
end

defp struct(value, _escape, _encode_map, Decimal) do
# silence the xref warning
decimal = Decimal
[?", decimal.to_string(value, :normal), ?"]
if Code.ensure_loaded?(Decimal) do
defp struct(value, _escape, _encode_map, Decimal) do
# silence the xref warning
decimal = Decimal
[?", decimal.to_string(value, :normal), ?"]
end
end

defp struct(value, escape, encode_map, Fragment) do
Expand Down
12 changes: 7 additions & 5 deletions lib/encoder.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,11 +224,13 @@ defimpl Jason.Encoder, for: [Date, Time, NaiveDateTime, DateTime] do
end
end

defimpl Jason.Encoder, for: Decimal do
def encode(value, _opts) do
# silence the xref warning
decimal = Decimal
[?", decimal.to_string(value), ?"]
if Code.ensure_loaded?(Decimal) do
defimpl Jason.Encoder, for: Decimal do
def encode(value, _opts) do
# silence the xref warning
decimal = Decimal
[?", decimal.to_string(value), ?"]
end
end
end

Expand Down