Improve error messages for dot syntax

Author: sabiwara

lib/elixir/lib/exception.ex

@@ -820,15 +820,8 @@ defmodule ArgumentError do
           "you attempted to apply a function named #{inspect(function)} on module #{inspect(module)} " <>
             "with arguments #{inspect(args)}. Arguments (the third argument of apply) must always be a proper list"
 
-        # Note that args may be an empty list even if they were supplied
-        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"
-
         not is_atom(module) ->
-          "you attempted to apply a function on #{inspect(module)}. " <>
+          "you attempted to apply a function named #{inspect(function)} on #{inspect(module)}. " <>
             "Modules (the first argument of apply) must always be an atom"
 
         not is_atom(function) ->
@@ -1639,6 +1632,22 @@ defmodule ErlangError do
     %KeyError{key: key, term: map}
   end
 
+  def normalize({:baddot, true, key, map}, _stacktrace) do
+    %ArgumentError{
+      message:
+        "could not find key #{inspect(key)} on #{inspect(map)}. " <>
+          "When using the map.field syntax, make sure the left side of the dot is a map"
+    }
+  end
+
+  def normalize({:baddot, false, key, module}, _stacktrace) do
+    %ArgumentError{
+      message:
+        "could not find function #{inspect(key)} on #{inspect(module)}. " <>
+          "When using the module.function() syntax, make sure the left side of the dot is a module"
+    }
+  end
+
   def normalize({:case_clause, term}, _stacktrace) do
     %CaseClauseError{term: term}
   end

lib/elixir/src/elixir_erl_pass.erl

@@ -232,7 +232,8 @@ translate({{'.', _, [Left, Right]}, Meta, []}, _Ann, S) when is_tuple(Left), is_
   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]},
+  TBadKeyError = {tuple, Ann, [{atom, Ann, badkey}, TRight, TVar]},
+  TBadDotError = build_baddot_error(TVar, TRight, Ann, Meta),
 
   {{'case', Generated, TLeft, [
     {clause, Generated,
@@ -242,11 +243,22 @@ translate({{'.', _, [Left, Right]}, Meta, []}, _Ann, S) when is_tuple(Left), is_
     {clause, Generated,
       [TVar],
       [[?remote(Generated, erlang, is_map, [TVar])]],
-      [?remote(Ann, erlang, error, [TError])]},
+      [?remote(Ann, erlang, error, [TBadKeyError])]},
+    {clause, Generated,
+      [TVar],
+      [[{op, Ann, 'orelse',
+        {op, Ann, '=:=', TVar, {atom, Ann, nil}},
+        ?remote(Generated, erlang, is_boolean, [TVar])
+      }]],
+      [?remote(Ann, erlang, error, [TBadDotError])]},
+    {clause, Generated,
+      [TVar],
+      [[?remote(Generated, erlang, is_atom, [TVar])]],
+      [{call, Generated, {remote, Generated, TVar, TRight}, []}]},
     {clause, Generated,
       [TVar],
       [],
-      [{call, Generated, {remote, Generated, TVar, TRight}, []}]}
+      [?remote(Ann, erlang, error, [TBadDotError])]}
   ]}, SV};
 
 translate({{'.', _, [Left, Right]}, Meta, Args}, _Ann, S)
@@ -596,6 +608,12 @@ translate_remote(maps, merge, Meta, [Map1, Map2], S) ->
     {[TMap1, TMap2], TS} ->
       {{call, Ann, {remote, Ann, {atom, Ann, maps}, {atom, Ann, merge}}, [TMap1, TMap2]}, TS}
   end;
+
+translate_remote(Left, Right, Meta, _Args, S)
+    when (Left =:= nil orelse is_boolean(Left)), is_atom(Right) ->
+  Ann = ?ann(Meta),
+  TBadDotError = build_baddot_error({atom, Ann, Left}, {atom, Ann, Right}, Ann, Meta),
+  {?remote(Ann, erlang, error, [TBadDotError]), S};
 translate_remote(Left, Right, Meta, Args, S) ->
   Ann = ?ann(Meta),
   {TLeft, SL} = translate(Left, Ann, S),
@@ -616,6 +634,10 @@ translate_remote(Left, Right, Meta, Args, S) ->
       {{call, Ann, {remote, Ann, TLeft, TRight}, TArgs}, SA}
   end.
 
+build_baddot_error(Left, Right, Ann, Meta) ->
+  NoParens = proplists:get_value(no_parens, Meta, false),
+  {tuple, Ann, [{atom, Ann, baddot}, {atom, Ann, NoParens}, Right, Left]}.
+
 generate_struct_name_guard([{map_field_exact, Ann, {atom, _, '__struct__'} = Key, Var} | Rest], Acc, S0) ->
   {ModuleVarName, S1} = elixir_erl_var:build('_', S0),
   Generated = erl_anno:set_generated(true, Ann),

lib/elixir/src/elixir_erl_try.erl

@@ -203,9 +203,13 @@ erl_rescue_guard_for(Meta, Var, 'Elixir.KeyError') ->
 erl_rescue_guard_for(Meta, Var, 'Elixir.ArgumentError') ->
   erl_or(Meta,
          {erl(Meta, '=='), Meta, [Var, badarg]},
-         erl_and(Meta,
-                 erl_tuple_size(Meta, Var, 2),
-                 erl_record_compare(Meta, Var, badarg)));
+         erl_or(Meta,
+            erl_and(Meta,
+                    erl_tuple_size(Meta, Var, 2),
+                    erl_record_compare(Meta, Var, badarg)),
+            erl_and(Meta,
+                    erl_tuple_size(Meta, Var, 4),
+                    erl_record_compare(Meta, Var, baddot))));
 
 erl_rescue_guard_for(Meta, Var, 'Elixir.ErlangError') ->
   Condition =

lib/elixir/test/elixir/exception_test.exs

@@ -477,16 +477,37 @@ defmodule ExceptionTest do
              """
     end
 
-    test "annotates badarg on apply" do
+    test "annotates dot syntax on invalid argument errors" do
       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"
+               "could not find key :foo on []. When using the map.field syntax, " <>
+                 "make sure the left side of the dot is a map"
+
+      assert blame_message([], & &1.foo()) ==
+               "could not find function :foo on []. When using the module.function() " <>
+                 "syntax, make sure the left side of the dot is a module"
+
+      # atoms that are invalid module name:
+      assert blame_message(nil, & &1.foo()) ==
+               "could not find function :foo on nil. When using the module.function() " <>
+                 "syntax, make sure the left side of the dot is a module"
+
+      assert blame_message(false, & &1.foo()) ==
+               "could not find function :foo on false. When using the module.function() " <>
+                 "syntax, make sure the left side of the dot is a module"
+
+      assert blame_message(nil, fn _ -> nil.foo() end) ==
+               "could not find function :foo on nil. When using the module.function() " <>
+                 "syntax, make sure the left side of the dot is a module"
 
+      assert blame_message(false, fn _ -> false.foo() end) ==
+               "could not find function :foo on false. When using the module.function() " <>
+                 "syntax, make sure the left side of the dot is a module"
+    end
+
+    test "annotates badarg on apply" do
       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"
+               "you attempted to apply a function named :foo on []. Modules " <>
+                 "(the first argument of apply) must always be an 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,12 +609,6 @@ 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
-      assert blame_message(nil, & &1.foo) ==
-               "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"
-    end
-
     test "annotates key error with suggestions if keys are atoms" do
       message = blame_message(%{first: nil, second: nil}, fn map -> map.firts end)
 

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

@@ -826,7 +826,6 @@ defmodule Kernel.ErrorsTest do
     rescue
       ArgumentError ->
         assert [
-                 {:erlang, :apply, [1, :foo, []], _},
                  {__MODULE__, :bad_remote_call, 1, [file: _, line: _]} | _
                ] = __STACKTRACE__
     end