Improve error message and docs for headless clauses, closes #15104

Author: josevalim

lib/elixir/pages/getting-started/modules-and-functions.md

@@ -151,6 +151,7 @@ defmodule Concat do
   # A function head declaring defaults
   def join(a, b, sep \\ " ")
 
+  # The separator is unused in this case, so we prefix it with underscore
   def join(a, b, _sep) when b == "" do
     a
   end
@@ -165,7 +166,7 @@ IO.puts(Concat.join("Hello", "world"))      #=> Hello world
 IO.puts(Concat.join("Hello", "world", "_")) #=> Hello_world
 ```
 
-When a variable is not used by a function or a clause, we add a leading underscore (`_`) to its name to signal this intent. This rule is also covered in our [Naming Conventions](../references/naming-conventions.md#underscore-_foo) document.
+Function heads cannot have patterns nor guards. They may only define the argument names and their default values.
 
 ## Understanding Aliases
 
@@ -182,8 +183,7 @@ true
 
 By using the `alias/2` directive, we are changing the atom the alias expands to.
 
-Aliases expand to atoms because in the Erlang Virtual Machine (and consequently Elixir) modules are always represented by atoms. By namespacing
-those atoms elixir modules avoid conflicting with existing erlang modules.
+Aliases expand to atoms because in the Erlang Virtual Machine (and consequently Elixir) modules are always represented by atoms. By namespacing those atoms, Elixir modules avoid conflicting with existing Erlang modules.
 
 ```elixir
 iex> List.flatten([1, [2], 3])
@@ -212,9 +212,7 @@ end
 
 The example above will define two modules: `Foo` and `Foo.Bar`. The second can be accessed as `Bar` inside `Foo` as long as they are in the same lexical scope.
 
-If, later, the `Bar` module is moved outside the `Foo` module definition, it must be referenced by its full name (`Foo.Bar`) or an alias must be set using the `alias` directive discussed above.
-
-**Note**: in Elixir, you don't have to define the `Foo` module before being able to define the `Foo.Bar` module, as they are effectively independent. The above could also be written as:
+If, later, the `Bar` module is moved outside the `Foo` module definition, it must be referenced by its full name (`Foo.Bar`) or an alias must be set using the `alias` directive discussed above:
 
 ```elixir
 defmodule Foo.Bar do
@@ -226,7 +224,11 @@ defmodule Foo do
 end
 ```
 
-Aliasing a nested module does not bring parent modules into scope. Consider the following example:
+> #### Module names are isolated {: .info}
+>
+> You don't have to define the `Foo` module before defining the `Foo.Bar` module, as they are effectively independent.
+
+Aliasing a module only alias a single module, it doesn't alias any of its parents. Consider the following example:
 
 ```elixir
 defmodule Foo do
@@ -241,4 +243,4 @@ alias Foo.Bar.Baz
 # However, the module `Foo.Bar` is *not* available as `Bar`
 ```
 
-As we will see in later chapters, aliases also play a crucial role in macros, to guarantee they are hygienic.
+As we will see in later chapters, aliases also work well with macros, to guarantee they are hygienic.

lib/elixir/src/elixir_def.erl

@@ -238,8 +238,9 @@ run_with_location_change(File, #{file := File} = E, Callback) ->
 run_with_location_change(File, E, Callback) ->
   elixir_lexical:with_file(File, E, Callback).
 
-def_to_clauses(_Kind, Meta, Args, [], nil, E) ->
+def_to_clauses(_Kind, Meta, Args, Guards, nil, E) ->
   check_args_for_function_head(Meta, Args, E),
+  (Guards /= []) andalso elixir_errors:module_error(Meta, E, ?MODULE, {invalid_function_head, guards}),
   [];
 def_to_clauses(_Kind, Meta, Args, Guards, [{do, Body}], _E) ->
   [{Meta, Args, Guards, Body}];
@@ -374,7 +375,7 @@ check_valid_defaults(_Meta, _File, _Name, _Arity, _Kind, 0, _StoredMeta, _Stored
 
 check_args_for_function_head(Meta, Args, E) ->
   [begin
-     elixir_errors:module_error(Meta, E, ?MODULE, invalid_args_for_function_head)
+     elixir_errors:module_error(Meta, E, ?MODULE, {invalid_function_head, patterns})
    end || Arg <- Args, invalid_arg(Arg)].
 
 invalid_arg({Name, _, Kind}) when is_atom(Name), is_atom(Kind) -> false;
@@ -486,16 +487,17 @@ format_error({no_alias, Atom}) ->
 format_error({invalid_def, Kind, NameAndArgs}) ->
   io_lib:format("invalid syntax in ~ts ~ts", [Kind, 'Elixir.Macro':to_string(NameAndArgs)]);
 
-format_error(invalid_args_for_function_head) ->
-  "patterns are not allowed in function head, only variables and default arguments (using \\\\)\n"
+format_error({invalid_function_head, Prefix}) ->
+  atom_to_list(Prefix) ++ (
+  " are not allowed in function head, only variables and default arguments (using \\\\)\n"
   "\n"
   "If you did not intend to define a function head, make sure your function "
   "definition has the proper syntax by wrapping the arguments in parentheses "
-  "and using the do instruction accordingly:\n\n"
+  "and using the do keyword accordingly:\n\n"
   "    def add(a, b), do: a + b\n\n"
   "    def add(a, b) do\n"
   "      a + b\n"
-  "    end\n";
+  "    end\n");
 
 format_error({'__info__', Kind}) ->
   io_lib:format("cannot define ~ts __info__/1 as it is automatically defined by Elixir", [Kind]);

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

@@ -854,33 +854,44 @@ defmodule Kernel.ErrorsTest do
     )
   end
 
-  test "function head with guard" do
-    assert_compile_error(["nofile:2:7: ", "missing :do option in \"def\""], ~c"""
-    defmodule Kernel.ErrorsTest.BodyessFunctionWithGuard do
-      def foo(n) when is_number(n)
-    end
-    """)
-
-    assert_compile_error(["nofile:2:7: ", "missing :do option in \"def\""], ~c"""
-    defmodule Kernel.ErrorsTest.BodyessFunctionWithGuard do
-      def foo(n) when is_number(n), true
-    end
-    """)
-  end
-
-  test "invalid args for function head" do
+  test "invalid function head" do
     assert_compile_error(
       [
         "nofile:2:7: ",
         "patterns are not allowed in function head, only variables and default arguments (using \\\\)"
       ],
       ~c"""
-      defmodule Kernel.ErrorsTest.InvalidArgsForBodylessClause do
+      defmodule Kernel.ErrorsTest.InvalidPatternsInFunctionHead do
         def foo(nil)
         def foo(_), do: :ok
       end
       """
     )
+
+    assert_compile_error(
+      [
+        "nofile:2:7: ",
+        "guards are not allowed in function head, only variables and default arguments (using \\\\)"
+      ],
+      ~c"""
+      defmodule Kernel.ErrorsTest.InvalidGuardsInFunctionHead do
+        def foo(x) when x == nil
+        def foo(_), do: :ok
+      end
+      """
+    )
+
+    assert_compile_error(["nofile:2:7: ", "missing :do option in \"def\""], ~c"""
+    defmodule Kernel.ErrorsTest.BodyessFunctionWithGuard do
+      def foo(n), true
+    end
+    """)
+
+    assert_compile_error(["nofile:2:7: ", "missing :do option in \"def\""], ~c"""
+    defmodule Kernel.ErrorsTest.BodyessFunctionWithGuard do
+      def foo(n) when is_number(n), true
+    end
+    """)
   end
 
   test "bad multi-call" do