`Credo.Check.Warning.UnsafeToAtom` warning on compile time created atom
Environment
- Credo version (
mix credo -v): 1.7.7-ref.dependabot-hex-gettext-0.26.1.58e9c1b08 - Erlang/Elixir version (
elixir -v): Elixir 1.17.2 (compiled with Erlang/OTP 26) - Operating system: Ubuntu 22.04.3 LTS
What were you trying to do?
Running the Credo.Check.Warning.UnsafeToAtom check (mix credo -c Credo.Check.Warning.UnsafeToAtom) is reporting right now issues for dynamically created atoms passed to unquote().
Expected outcome
AFAIK dynamically created atoms passed to unquote() are generated in compile time and thus should not be reported by this check, given they cannot be exploited in runtime. Thus the following code should not emit the check warning:
defmodule Test do
for n <- 1..4 do
def unquote(:"fun_#{n}")(), do: unquote(n)
end
end
Actual outcome
Running the check on the code above returns
Prefer :erlang.binary_to_existing_atom/2 over :erlang.binary_to_atom/2 to avoid creating atoms at runtime.
Rewriting the code to the following avoids the warning, but I'm unsure if that is the right solution.
defmodule Test do
for n <- 1..4 do
def unquote(:erlang.binary_to_atom("fun_#{n}"))(), do: unquote(n)
end
end
@Wigny Thanks for reporting this 😀 Sorry for the delay. It is now fixed on master.
You can try this by setting the Credo dep to
{:credo, github: "rrrene/credo"}
Please report back if your issue is solved! 👍
Yes, it worked great, thanks!
The only case the change didn't cover in our codebase was
unquote(context).unquote(:"get_#{type}_by")(id: id)
which I believe could have been written as this instead
apply(unquote(context), unquote(:"get_#{type}_by"), [[id: id]])
So it is all good IMO. Many thanks for solving it!
@Wigny Patched the check to cover that case as well. Thx for the input 👍
@Wigny This is part of Credo 1.7.8. Thx! 🎉