Handle inspect on Erlang atoms (that can be created from strings in Gleam), that are invalid in Gleam. (#479)

3 files changed, 132 insertions, 10 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 77ce18b..9a3f2bb 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,5 +1,12 @@
 # Changelog
 
+## Unreleased
+
+- Fixed a bug on target Erlang where `string.inspect` would show atoms created
+  from strings, which are valid in Erlang but invalid in Gleam, as if they were
+  similar equivalent Gleam atoms. Now for atoms created from strings, which are
+  invalid in Gleam, the Erlang representation is shown, instead.
+
 ## v0.30.0 - 2023-07-16
 
 - The `list` module gains the `list.map2` function.
diff --git a/src/gleam_stdlib.erl b/src/gleam_stdlib.erl
index 6967cf4..959d14d 100644
--- a/src/gleam_stdlib.erl
+++ b/src/gleam_stdlib.erl
@@ -8,10 +8,10 @@
          bit_string_int_to_u32/1, bit_string_int_from_u32/1, decode_result/1,
          bit_string_slice/3, decode_bit_string/1, compile_regex/2, regex_scan/2,
          percent_encode/1, percent_decode/1, regex_check/2, regex_split/2,
-         base_decode64/1, parse_query/1, bit_string_concat/1, size_of_tuple/1, 
+         base_decode64/1, parse_query/1, bit_string_concat/1, size_of_tuple/1,
          decode_tuple/1, decode_tuple2/1, decode_tuple3/1, decode_tuple4/1,
-         decode_tuple5/1, decode_tuple6/1, tuple_get/2, classify_dynamic/1, 
-         print/1, println/1, print_error/1, println_error/1, inspect/1, 
+         decode_tuple5/1, decode_tuple6/1, tuple_get/2, classify_dynamic/1,
+         print/1, println/1, print_error/1, println_error/1, inspect/1,
          float_to_string/1, int_from_base_string/2]).
 
 %% Taken from OTP's uri_string module
@@ -27,6 +27,12 @@
            ((X) >= $a) andalso ((X) =< $f) -> (X) - $a + 10
         end).
 
+-define(is_lowercase_char(X), (X > 96 andalso X < 123)).
+
+-define(is_underscore_char(X), (X == 95)).
+
+-define(is_digit_char(X), (X > 47 andalso X < 58)).
+
 map_get(Map, Key) ->
     case maps:find(Key, Map) of
         error -> {error, nil};
@@ -363,14 +369,14 @@ inspect(true) ->
     "True";
 inspect(false) ->
     "False";
+inspect(nil) ->
+    "Nil";
 inspect(Any) when is_atom(Any) ->
-    lists:map(
-        fun(Part) ->
-            [Head | Tail] = string:next_grapheme(unicode:characters_to_binary(Part)),
-            [string:uppercase([Head]), Tail]
-        end,
-        re:split(erlang:atom_to_list(Any), "_+", [{return, iodata}])
-    );
+    AtomAsList = erlang:atom_to_list(Any),
+    case inspect_maybe_gleam_atom(AtomAsList, none, []) of
+        {ok, GleamCompatibleAtomString} -> erlang:list_to_binary(GleamCompatibleAtomString);
+        {error, no_gleam_atom} -> ["//erl('", erlang:atom_to_binary(Any), "')"]
+	end;
 inspect(Any) when is_integer(Any) ->
     erlang:integer_to_list(Any);
 inspect(Any) when is_float(Any) ->
@@ -411,6 +417,32 @@ inspect(Any) when is_function(Any) ->
 inspect(Any) ->
     ["//erl(", io_lib:format("~p", [Any]), ")"].
 
+inspect_maybe_gleam_atom([], none, []) ->
+    {error, no_gleam_atom};
+inspect_maybe_gleam_atom([Head | _Rest], none, []) when ?is_digit_char(Head) ->
+    {error, no_gleam_atom};
+inspect_maybe_gleam_atom([$_ | _Rest], none, []) ->
+    {error, no_gleam_atom};
+inspect_maybe_gleam_atom([$_ | []], _PrevChar, _Acc) ->
+    {error, no_gleam_atom};
+inspect_maybe_gleam_atom([$_ | _Rest], $_, _Acc) ->
+    {error, no_gleam_atom};
+inspect_maybe_gleam_atom([Head | _Rest], _PrevChar, _Acc)
+    when not (?is_lowercase_char(Head) orelse ?is_underscore_char(Head) orelse ?is_digit_char(Head)) ->
+    {error, no_gleam_atom};
+inspect_maybe_gleam_atom([Head | Rest], none, Acc) ->
+    inspect_maybe_gleam_atom(Rest, Head, [string:uppercase([Head]) | Acc]);
+inspect_maybe_gleam_atom([$_ | Rest], _PrevChar, Acc) ->
+    inspect_maybe_gleam_atom(Rest, $_, Acc);
+inspect_maybe_gleam_atom([Head | Rest], $_, Acc) ->
+    inspect_maybe_gleam_atom(Rest, Head, [string:uppercase([Head]) | Acc]);
+inspect_maybe_gleam_atom([Head | Rest], PrevChar, Acc) when ?is_digit_char(PrevChar) ->
+    inspect_maybe_gleam_atom(Rest, Head, [string:uppercase([Head]) | Acc]);
+inspect_maybe_gleam_atom([Head | Rest], _PrevChar, Acc) ->
+    inspect_maybe_gleam_atom(Rest, Head, [Head | Acc]);
+inspect_maybe_gleam_atom([], _PrevChar, Acc) ->
+    {ok, lists:reverse(Acc)}.
+
 inspect_list([]) ->
     {proper, []};
 inspect_list([Head]) ->
diff --git a/test/gleam/string_test.gleam b/test/gleam/string_test.gleam
index 3306f0e..0fa860f 100644
--- a/test/gleam/string_test.gleam
+++ b/test/gleam/string_test.gleam
@@ -917,6 +917,9 @@ if javascript {
 
 if erlang {
   import gleam/regex
+  import gleam/dynamic.{Dynamic}
+
+  // Test inspect on Erlang atoms valid and invalid in Gleam
 
   external fn create_erlang_pid() -> String =
     "erlang" "self"
@@ -984,6 +987,86 @@ if erlang {
     improper_tail,
   ) -> List(anything) =
     "gleam_stdlib_test_ffi" "improper_list_append"
+
+  external fn string_to_erlang_atom(String) -> Dynamic =
+    "erlang" "binary_to_atom"
+
+  pub fn inspect_erlang_atom_is_valid_in_gleam_test() {
+    string_to_erlang_atom("a_common_erlang_atom_is_valid_in_gleam")
+    |> string.inspect
+    |> should.equal("ACommonErlangAtomIsValidInGleam")
+
+    string_to_erlang_atom(
+      "an_erlang_atom_with_1_or_many_non_leading_digits_is_valid_in_gleam",
+    )
+    |> string.inspect
+    |> should.equal("AnErlangAtomWith1OrManyNonLeadingDigitsIsValidInGleam")
+  }
+
+  pub fn inspect_erlang_atom_with_a_leading_underscore_is_invalid_in_gleam_test() {
+    string_to_erlang_atom(
+      "_an_erlang_atom_with_a_leading_underscore_is_invalid_in_gleam",
+    )
+    |> string.inspect
+    |> should.equal(
+      "//erl('_an_erlang_atom_with_a_leading_underscore_is_invalid_in_gleam')",
+    )
+  }
+
+  pub fn inspect_erlang_atom_with_a_trailing_underscore_is_invalid_in_gleam_test() {
+    string_to_erlang_atom(
+      "an_erlang_atom_with_a_trailing_underscore_is_invalid_in_gleam_",
+    )
+    |> string.inspect
+    |> should.equal(
+      "//erl('an_erlang_atom_with_a_trailing_underscore_is_invalid_in_gleam_')",
+    )
+  }
+
+  pub fn inspect_erlang_atom_with_a_double_underscore_is_invalid_in_gleam_test() {
+    string_to_erlang_atom("an_erlang_atom_with_a_double__underscore_is_invalid")
+    |> string.inspect
+    |> should.equal(
+      "//erl('an_erlang_atom_with_a_double__underscore_is_invalid')",
+    )
+  }
+
+  pub fn inspect_erlang_atom_with_white_spaces_is_invalid_in_gleam_test() {
+    string_to_erlang_atom(
+      "an erlang atom with white spaces is invalid in gleam",
+    )
+    |> string.inspect
+    |> should.equal(
+      "//erl('an erlang atom with white spaces is invalid in gleam')",
+    )
+  }
+
+  pub fn inspect_erlang_atom_that_is_an_empty_string_is_invalid_in_gleam_test() {
+    // An empty string based atom is invalid in gleam
+    string_to_erlang_atom("")
+    |> string.inspect
+    |> should.equal("//erl('')")
+  }
+
+  pub fn inspect_erlang_atom_with_uppercases_invalid_in_gleam_test() {
+    string_to_erlang_atom("AnErlangAtomWithUpperCasesIsInvalidInGleam")
+    |> string.inspect
+    |> should.equal("//erl('AnErlangAtomWithUpperCasesIsInvalidInGleam')")
+  }
+
+  pub fn inspect_erlang_atom_with_leading_digit_invalid_in_gleam_test() {
+    string_to_erlang_atom(
+      "1_erlang_atom_with_a_leading_digit_is_invalid_in_gleam",
+    )
+    |> string.inspect
+    |> should.equal(
+      "//erl('1_erlang_atom_with_a_leading_digit_is_invalid_in_gleam')",
+    )
+
+    string_to_erlang_atom("1ErlangAtomWithALeadingDigitIsInvalidInGleam")
+    |> string.inspect
+    |> should.equal("//erl('1ErlangAtomWithALeadingDigitIsInvalidInGleam')")
+  }
 }
 
 pub fn byte_size_test() {