Improve error messages for dot syntax (#12003)

Author: sabiwara

lib/elixir/lib/exception.ex

@@ -824,8 +824,8 @@ defmodule ArgumentError do
         not is_atom(module) and is_atom(function) and args == [] ->
           "you attempted to apply a function named #{inspect(function)} on #{inspect(module)}. " <>
             "If you are using Kernel.apply/3, make sure the module is an atom. " <>
-            "If you are using the dot syntax, such as map.field or module.function(), " <>
-            "make sure the left side of the dot is an atom or a map"
+            "If you are using the dot syntax, such as module.function(), " <>
+            "make sure the left-hand side of the dot is a module atom"
 
         not is_atom(module) ->
           "you attempted to apply a function on #{inspect(module)}. " <>
@@ -1114,8 +1114,8 @@ defmodule UndefinedFunctionError do
   end
 
   defp hint(nil, _function, 0, _loaded?) do
-    ". If you are using the dot syntax, such as map.field or module.function(), " <>
-      "make sure the left side of the dot is an atom or a map"
+    ". If you are using the dot syntax, such as module.function(), " <>
+      "make sure the left-hand side of the dot is a module atom"
   end
 
   defp hint(module, function, arity, true) do
@@ -1635,10 +1635,19 @@ defmodule ErlangError do
     %KeyError{key: key, term: term}
   end
 
-  def normalize({:badkey, key, map}, _stacktrace) do
+  def normalize({:badkey, key, map}, _stacktrace) when is_map(map) do
     %KeyError{key: key, term: map}
   end
 
+  def normalize({:badkey, key, term}, _stacktrace) do
+    message =
+      "key #{inspect(key)} not found in: #{inspect(term)}. " <>
+        "If you are using the dot syntax, such as map.field, " <>
+        "make sure the left-hand side of the dot is a map"
+
+    %KeyError{key: key, term: term, message: message}
+  end
+
   def normalize({:case_clause, term}, _stacktrace) do
     %CaseClauseError{term: term}
   end

lib/elixir/src/elixir_erl_pass.erl

@@ -224,30 +224,50 @@ translate({{'.', _, [Left, Right]}, Meta, []}, _Ann, #elixir_erl{context=guard}
   TRight = {atom, Ann, Right},
   {?remote(Ann, erlang, map_get, [TRight, TLeft]), SL};
 
-translate({{'.', _, [Left, Right]}, Meta, []}, _Ann, S) when is_tuple(Left), is_atom(Right), is_list(Meta) ->
+translate({{'.', _, [Left, Right]}, Meta, []}, _Ann, S)
+  when is_tuple(Left) orelse Left =:= nil orelse is_boolean(Left), is_atom(Right), is_list(Meta) ->
   Ann = ?ann(Meta),
   {TLeft, SL} = translate(Left, Ann, S),
   TRight = {atom, Ann, Right},
 
   Generated = erl_anno:set_generated(true, Ann),
   {Var, SV} = elixir_erl_var:build('_', SL),
   TVar = {var, Generated, Var},
-  TError = {tuple, Ann, [{atom, Ann, badkey}, TRight, TVar]},
-
-  {{'case', Generated, TLeft, [
-    {clause, Generated,
-      [{map, Ann, [{map_field_exact, Ann, TRight, TVar}]}],
-      [],
-      [TVar]},
-    {clause, Generated,
-      [TVar],
-      [[?remote(Generated, erlang, is_map, [TVar])]],
-      [?remote(Ann, erlang, error, [TError])]},
-    {clause, Generated,
-      [TVar],
-      [],
-      [{call, Generated, {remote, Generated, TVar, TRight}, []}]}
-  ]}, SV};
+
+  case proplists:get_value(no_parens, Meta, false) of
+    true ->
+      TError = {tuple, Ann, [{atom, Ann, badkey}, TRight, TVar]},
+      {{'case', Generated, TLeft, [
+        {clause, Generated,
+          [{map, Ann, [{map_field_exact, Ann, TRight, TVar}]}],
+          [],
+          [TVar]},
+        {clause, Generated,
+          [TVar],
+          [[
+            ?remote(Generated, erlang, is_atom, [TVar]),
+            {op, Generated, '=/=', TVar, {atom, Generated, nil}},
+            {op, Generated, '=/=', TVar, {atom, Generated, true}},
+            {op, Generated, '=/=', TVar, {atom, Generated, false}}
+          ]],
+          [{call, Generated, {remote, Generated, TVar, TRight}, []}]},
+        {clause, Generated,
+          [TVar],
+          [],
+          [?remote(Ann, erlang, error, [TError])]}
+      ]}, SV};
+    false ->
+      {{'case', Generated, TLeft, [
+        {clause, Generated,
+          [{map, Ann, [{map_field_exact, Ann, TRight, TVar}]}],
+          [],
+          [TVar]},
+        {clause, Generated,
+          [TVar],
+          [],
+          [{call, Generated, {remote, Generated, TVar, TRight}, []}]}
+      ]}, SV}
+    end;
 
 translate({{'.', _, [Left, Right]}, Meta, Args}, _Ann, S)
     when (is_tuple(Left) orelse is_atom(Left)), is_atom(Right), is_list(Meta), is_list(Args) ->

lib/elixir/test/elixir/exception_test.exs

@@ -478,15 +478,15 @@ defmodule ExceptionTest do
     end
 
     test "annotates badarg on apply" do
-      assert blame_message([], & &1.foo) ==
+      assert blame_message([], & &1.foo()) ==
                "you attempted to apply a function named :foo on []. If you are using Kernel.apply/3, make sure " <>
                  "the module is an atom. If you are using the dot syntax, such as " <>
-                 "map.field or module.function(), make sure the left side of the dot is an atom or a map"
+                 "module.function(), make sure the left-hand side of the dot is a module atom"
 
       assert blame_message([], &apply(&1, :foo, [])) ==
                "you attempted to apply a function named :foo on []. If you are using Kernel.apply/3, make sure " <>
                  "the module is an atom. If you are using the dot syntax, such as " <>
-                 "map.field or module.function(), make sure the left side of the dot is an atom or a map"
+                 "module.function(), make sure the left-hand side of the dot is a module atom"
 
       assert blame_message([], &apply(Kernel, &1, [1, 2])) ==
                "you attempted to apply a function named [] on module Kernel. However [] is not a valid function name. " <>
@@ -588,10 +588,27 @@ defmodule ExceptionTest do
                "function :erlang.hash/2 is undefined or private, use erlang:phash2/2 instead"
     end
 
-    test "annotates undefined function clause error with nil hints" do
+    test "annotates undefined key error with nil hints" do
       assert blame_message(nil, & &1.foo) ==
+               "key :foo not found in: nil. If you are using the dot syntax, " <>
+                 "such as map.field, make sure the left-hand side of the dot is a map"
+
+      # we use `Code.eval_string/1` to escape the formatter and warnings
+      assert blame_message("nil.foo", &Code.eval_string/1) ==
+               "key :foo not found in: nil. If you are using the dot syntax, " <>
+                 "such as map.field, make sure the left-hand side of the dot is a map"
+    end
+
+    test "annotates undefined function clause error with nil hints" do
+      assert blame_message(nil, & &1.foo()) ==
+               "function nil.foo/0 is undefined. If you are using the dot syntax, " <>
+                 "such as module.function(), make sure the left-hand side of " <>
+                 "the dot is a module atom"
+
+      assert blame_message("nil.foo()", &Code.eval_string/1) ==
                "function nil.foo/0 is undefined. If you are using the dot syntax, " <>
-                 "such as map.field or module.function(), make sure the left side of the dot is an atom or a map"
+                 "such as module.function(), make sure the left-hand side of " <>
+                 "the dot is a module atom"
     end
 
     test "annotates key error with suggestions if keys are atoms" do

lib/elixir/test/elixir/kernel/errors_test.exs

@@ -862,7 +862,7 @@ defmodule Kernel.ErrorsTest do
                       "defmodule MisplacedOperator, do: (def bar(1 | 2), do: :ok)"
   end
 
-  defp bad_remote_call(x), do: x.foo
+  defp bad_remote_call(x), do: x.foo()
 
   defmacro sample(0), do: 0