From 3b8144b7c930bf87ee1034955828239561e8d7b3 Mon Sep 17 00:00:00 2001 From: sabiwara Date: Tue, 16 Jun 2026 22:11:17 +0900 Subject: [PATCH 1/3] Rework anti-pattern doc about unsafe atom creation --- .../pages/anti-patterns/code-anti-patterns.md | 23 +++++++++++-------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/lib/elixir/pages/anti-patterns/code-anti-patterns.md b/lib/elixir/pages/anti-patterns/code-anti-patterns.md index 002768837e7..d2c80bc8f32 100644 --- a/lib/elixir/pages/anti-patterns/code-anti-patterns.md +++ b/lib/elixir/pages/anti-patterns/code-anti-patterns.md @@ -169,26 +169,31 @@ When we use the `String.to_unsafe_atom/1` function to dynamically create an atom #### Refactoring -To eliminate this anti-pattern, developers must either perform explicit conversions by mapping strings to atoms or replace the use of `String.to_unsafe_atom/1` with `String.to_existing_atom/1`. An explicit conversion could be done as follows: +To eliminate this anti-pattern, developers must either: +* perform explicit conversions by replacing the use of `String.to_unsafe_atom/1` with `String.to_existing_atom/2` and provide the list of expected atoms. +* manually perform the explicit conversion using pattern-matching to map strings to atoms for instance. +* use `String.to_existing_atom/1` when the former options are not possible. + +An explicit conversion could be done as follows: ```elixir defmodule MyRequestHandler do def parse(%{"status" => status, "message" => message} = _payload) do - %{status: convert_status(status), message: message} + status = String.to_existing_atom(status, [:ok, :error, :redirect]) + %{status: status, message: message} end - - defp convert_status("ok"), do: :ok - defp convert_status("error"), do: :error - defp convert_status("redirect"), do: :redirect end ``` +This will only accept valid values and convert `"ok"` as `:ok`, `"error"` as `:error`, and +`"redirect"` as `:redirect`. Any other value would result in an `ArgumentError`, either because the atom doesn't exist or because it isn't part of the list: + ```elixir iex> MyRequestHandler.parse(%{"status" => "status_not_seen_anywhere", "message" => "all good"}) -** (FunctionClauseError) no function clause matching in MyRequestHandler.convert_status/1 +** (ArgumentError) ... ``` -By explicitly listing all supported statuses, you guarantee only a limited number of conversions may happen. Passing an invalid status will lead to a function clause error. +By explicitly listing all supported statuses, you guarantee that only a limited number of conversions may happen, and that all expected atoms already exist within the system. An alternative is to use `String.to_existing_atom/1`, which will only convert a string to atom if the atom already exists in the system: @@ -207,7 +212,7 @@ iex> MyRequestHandler.parse(%{"status" => "status_not_seen_anywhere", "message" * 1st argument: not an already existing atom ``` -In such cases, passing an unknown status will raise as long as the status was not defined anywhere as an atom in the system. However, assuming `status` can be either `:ok`, `:error`, or `:redirect`, how can you guarantee those atoms exist? You must ensure those atoms exist somewhere **in the same module** where `String.to_existing_atom/1` is called. For example, if you had this code: +In such cases, unlike `String.to_existing_atom/2`, passing an unknown status will raise as long as the status was not defined anywhere as an atom in the system. However, assuming `status` can be either `:ok`, `:error`, or `:redirect`, how can you guarantee those atoms exist? You must ensure those atoms exist somewhere **in the same module** where `String.to_existing_atom/1` is called. For example, if you had this code: ```elixir defmodule MyRequestHandler do From fbcf96f0e7ae42152ac261890d3769bfba1bdb69 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Tue, 16 Jun 2026 17:28:01 +0200 Subject: [PATCH 2/3] Clarify refactoring for string to atom conversions Updated the refactoring section to clarify the use of explicit conversions and pattern matching for string to atom conversions. Added examples and emphasized the importance of defining valid statuses within the same module. --- .../pages/anti-patterns/code-anti-patterns.md | 51 +++++-------------- 1 file changed, 12 insertions(+), 39 deletions(-) diff --git a/lib/elixir/pages/anti-patterns/code-anti-patterns.md b/lib/elixir/pages/anti-patterns/code-anti-patterns.md index d2c80bc8f32..bcaca5ce163 100644 --- a/lib/elixir/pages/anti-patterns/code-anti-patterns.md +++ b/lib/elixir/pages/anti-patterns/code-anti-patterns.md @@ -170,9 +170,12 @@ When we use the `String.to_unsafe_atom/1` function to dynamically create an atom #### Refactoring To eliminate this anti-pattern, developers must either: -* perform explicit conversions by replacing the use of `String.to_unsafe_atom/1` with `String.to_existing_atom/2` and provide the list of expected atoms. -* manually perform the explicit conversion using pattern-matching to map strings to atoms for instance. -* use `String.to_existing_atom/1` when the former options are not possible. + + * replace the use of `String.to_unsafe_atom/1` with a list of expected atoms in `String.to_existing_atom/2` + + * use pattern-matching to map strings to their equivalent atoms + + * use `String.to_existing_atom/1` when the former options are not possible An explicit conversion could be done as follows: @@ -185,8 +188,7 @@ defmodule MyRequestHandler do end ``` -This will only accept valid values and convert `"ok"` as `:ok`, `"error"` as `:error`, and -`"redirect"` as `:redirect`. Any other value would result in an `ArgumentError`, either because the atom doesn't exist or because it isn't part of the list: +This will only accept valid values and convert `"ok"` as `:ok`, `"error"` as `:error`, and `"redirect"` as `:redirect`. Any other value would result in an `ArgumentError`, either because the atom doesn't exist or because it isn't part of the list: ```elixir iex> MyRequestHandler.parse(%{"status" => "status_not_seen_anywhere", "message" => "all good"}) @@ -195,50 +197,21 @@ iex> MyRequestHandler.parse(%{"status" => "status_not_seen_anywhere", "message" By explicitly listing all supported statuses, you guarantee that only a limited number of conversions may happen, and that all expected atoms already exist within the system. -An alternative is to use `String.to_existing_atom/1`, which will only convert a string to atom if the atom already exists in the system: +An alternative is to use pattern matching and explicitly convert each string to its respective atom. This is useful when you need additional logic on differnt branches: ```elixir defmodule MyRequestHandler do def parse(%{"status" => status, "message" => message} = _payload) do - %{status: String.to_existing_atom(status), message: message} - end -end -``` - -```elixir -iex> MyRequestHandler.parse(%{"status" => "status_not_seen_anywhere", "message" => "all good"}) -** (ArgumentError) errors were found at the given arguments: - - * 1st argument: not an already existing atom -``` - -In such cases, unlike `String.to_existing_atom/2`, passing an unknown status will raise as long as the status was not defined anywhere as an atom in the system. However, assuming `status` can be either `:ok`, `:error`, or `:redirect`, how can you guarantee those atoms exist? You must ensure those atoms exist somewhere **in the same module** where `String.to_existing_atom/1` is called. For example, if you had this code: - -```elixir -defmodule MyRequestHandler do - def parse(%{"status" => status, "message" => message} = _payload) do - %{status: String.to_existing_atom(status), message: message} - end - - def handle(%{status: status}) do case status do - :ok -> ... - :error -> ... - :redirect -> ... + "ok" -> %{status: :ok, message: message} + "error" -> %{status: :error, message: message} + "redirect" -> %{status: :redirect, message: message, code: 302} end end end ``` -All valid statuses are defined as atoms within the same module, and that's enough. If you want to be explicit, you could also have a function that lists them: - -```elixir -def valid_statuses do - [:ok, :error, :redirect] -end -``` - -However, keep in mind using a module attribute or defining the atoms in the module body, outside of a function, are not sufficient, as the module body is only executed during compilation and it is not necessarily part of the compiled module loaded at runtime. +You may alternatively use `String.to_existing_atom/1`, but keep in mind it does have pitfalls related to code loading. Read the documentation for more information. ## Long parameter list From 336346a2a0bd3d1735c651d6e13cfd9a1d1547ca Mon Sep 17 00:00:00 2001 From: sabiwara Date: Wed, 17 Jun 2026 06:47:09 +0900 Subject: [PATCH 3/3] Add missing @doc since --- lib/elixir/lib/list.ex | 2 ++ lib/elixir/lib/string.ex | 2 ++ 2 files changed, 4 insertions(+) diff --git a/lib/elixir/lib/list.ex b/lib/elixir/lib/list.ex index ab758ad287c..f62df412fb9 100644 --- a/lib/elixir/lib/list.ex +++ b/lib/elixir/lib/list.ex @@ -1046,6 +1046,7 @@ defmodule List do :"🌢 Elixir" """ + @doc since: "1.21.0" @spec to_unsafe_atom(charlist) :: atom def to_unsafe_atom(charlist) do :erlang.list_to_atom(charlist) @@ -1104,6 +1105,7 @@ defmodule List do ** (ArgumentError) unexpected value: ~c\"unknown\", the allowed atoms are: [:foo, :bar] """ + @doc since: "1.21.0" @spec to_existing_atom(charlist, nonempty_list(a)) :: a when a: atom() def to_existing_atom(charlist, [_ | _] = allowed_atoms) when is_list(charlist) do atom = :erlang.list_to_existing_atom(charlist) diff --git a/lib/elixir/lib/string.ex b/lib/elixir/lib/string.ex index c148577da6e..6c32ee54636 100644 --- a/lib/elixir/lib/string.ex +++ b/lib/elixir/lib/string.ex @@ -2988,6 +2988,7 @@ defmodule String do :my_atom """ + @doc since: "1.21.0" @spec to_unsafe_atom(String.t()) :: atom def to_unsafe_atom(string) when is_binary(string) do :erlang.binary_to_atom(string, :utf8) @@ -3046,6 +3047,7 @@ defmodule String do ** (ArgumentError) unexpected value: \"unknown\", the allowed atoms are: [:foo, :bar] """ + @doc since: "1.21.0" @spec to_existing_atom(String.t(), nonempty_list(a)) :: a when a: atom() def to_existing_atom(string, [_ | _] = allowed_atoms) when is_binary(string) do atom = :erlang.binary_to_existing_atom(string, :utf8)