Warn when function head comes immediately after the implementation

3 files changed, 61 insertions, 22 deletions

diff --git a/lib/elixir/src/elixir_def.erl b/lib/elixir/src/elixir_def.erl
index 46324545f..dade3bf27 100644
--- a/lib/elixir/src/elixir_def.erl
+++ b/lib/elixir/src/elixir_def.erl
@@ -247,9 +247,9 @@ store_definition(Check, Kind, Meta, Name, Arity, File, Module, Defaults, Clauses
     case ets:lookup(Set, {def, Tuple}) of
       [{_, StoredKind, StoredMeta, StoredFile, StoredCheck, {StoredDefaults, LastHasBody, LastDefaults}}] ->
         check_valid_kind(Meta, File, Name, Arity, Kind, StoredKind),
-        (Check and StoredCheck) andalso
-          check_valid_clause(Meta, File, Name, Arity, Kind, Set, StoredMeta, StoredFile),
         check_valid_defaults(Meta, File, Name, Arity, Kind, Defaults, StoredDefaults, LastDefaults, HasBody, LastHasBody),
+        (Check and StoredCheck) andalso
+          check_valid_clause(Meta, File, Name, Arity, Kind, Set, StoredMeta, StoredFile, Clauses),
 
         {max(Defaults, StoredDefaults), StoredMeta};
       [] ->
@@ -309,10 +309,15 @@ check_valid_kind(Meta, File, Name, Arity, Kind, StoredKind) ->
   elixir_errors:form_error(Meta, File, ?MODULE,
     {changed_kind, {Name, Arity, StoredKind, Kind}}).
 
-check_valid_clause(Meta, File, Name, Arity, Kind, Set, StoredMeta, StoredFile) ->
+check_valid_clause(Meta, File, Name, Arity, Kind, Set, StoredMeta, StoredFile, Clauses) ->
   case ets:lookup_element(Set, ?last_def, 2) of
-    none -> ok;
-    {Name, Arity} -> ok;
+    none ->
+      ok;
+    {Name, Arity} when Clauses == [] ->
+      elixir_errors:form_warn(Meta, File, ?MODULE,
+        {late_function_head, {Kind, Name, Arity}});
+    {Name, Arity} ->
+      ok;
     {Name, _} ->
       Relative = elixir_utils:relative_to_cwd(StoredFile),
       elixir_errors:form_warn(Meta, File, ?MODULE,
@@ -434,6 +439,18 @@ format_error({ungrouped_arity, {Kind, Name, Arity, OrigLine, OrigFile}}) ->
   io_lib:format("clauses with the same name and arity (number of arguments) should be grouped together, \"~ts ~ts/~B\" was previously defined (~ts:~B)",
     [Kind, Name, Arity, OrigFile, OrigLine]);
 
+format_error({late_function_head, {Kind, Name, Arity}}) ->
+  io_lib:format("function head for ~ts ~ts/~B must come at the top of its direct implementation. Instead of:\n"
+    "\n"
+    "    def add(a, b), do: a + b\n"
+    "    def add(a, b)\n"
+    "\n"
+    "one should write:\n"
+    "\n"
+    "    def add(a, b)\n"
+    "    def add(a, b), do: a + b\n",
+    [Kind, Name, Arity]);
+
 format_error({changed_kind, {Name, Arity, Previous, Current}}) ->
   io_lib:format("~ts ~ts/~B already defined as ~ts", [Current, Name, Arity, Previous]);
 
diff --git a/lib/elixir/test/elixir/kernel/docs_test.exs b/lib/elixir/test/elixir/kernel/docs_test.exs
index 37a3099ac..e5aea3b91 100644
--- a/lib/elixir/test/elixir/kernel/docs_test.exs
+++ b/lib/elixir/test/elixir/kernel/docs_test.exs
@@ -5,6 +5,15 @@ defmodule Kernel.DocsTest do
 
   import PathHelpers
 
+  defmacro wrong_doc_baz do
+    quote do
+      @doc "Wrong doc"
+      @doc since: "1.2"
+      def baz(_arg)
+      def baz(arg), do: arg + 1
+    end
+  end
+
   test "attributes format" do
     defmodule DocAttributes do
       @moduledoc "Module doc"
@@ -192,13 +201,11 @@ defmodule Kernel.DocsTest do
           @doc "Multiple function head doc"
           @deprecated "something else"
           def bar(_arg)
-          def bar(_arg)
           def bar(arg), do: arg + 1
 
-          @doc "Wrong doc"
-          @doc since: "1.2"
-          def baz(_arg)
-          def baz(arg), do: arg + 1
+          require Kernel.DocsTest
+          Kernel.DocsTest.wrong_doc_baz()
+
           @doc "Multiple function head and docs"
           @doc since: "1.2.3"
           def baz(_arg)
diff --git a/lib/elixir/test/elixir/kernel/warning_test.exs b/lib/elixir/test/elixir/kernel/warning_test.exs
index cefd1de82..be17b5b9d 100644
--- a/lib/elixir/test/elixir/kernel/warning_test.exs
+++ b/lib/elixir/test/elixir/kernel/warning_test.exs
@@ -679,7 +679,7 @@ defmodule Kernel.WarningTest do
     purge(Sample)
   end
 
-  test "empty clause" do
+  test "empty function head" do
     assert capture_err(fn ->
              Code.eval_string("""
              defmodule Sample1 do
@@ -691,6 +691,30 @@ defmodule Kernel.WarningTest do
     purge(Sample1)
   end
 
+  test "late function heads" do
+    assert capture_err(fn ->
+             Code.eval_string("""
+             defmodule Sample1 do
+               use Task
+               @doc "hello"
+               def child_spec(opts)
+             end
+             """)
+           end) =~ ""
+
+    assert capture_err(fn ->
+             Code.eval_string("""
+             defmodule Sample2 do
+               def child_spec(spec), do: spec
+               @doc "hello"
+               def child_spec(opts)
+             end
+             """)
+           end) =~ ""
+  after
+    purge([Sample1, Sample2])
+  end
+
   test "used import via alias" do
     assert capture_err(fn ->
              Code.eval_string("""
@@ -781,23 +805,14 @@ defmodule Kernel.WarningTest do
 
     assert capture_err(fn ->
              Code.eval_string(~S"""
-             defmodule Sample1 do
+             defmodule Sample do
                def hello(arg \\ 0), do: arg
                def hello(arg), do: arg
              end
              """)
            end) =~ message
-
-    assert capture_err(fn ->
-             Code.eval_string(~S"""
-               defmodule Sample2 do
-                 def hello(arg \\ 0), do: arg
-                 def hello(_arg)
-               end
-             """)
-           end) == ""
   after
-    purge([Sample1, Sample2])
+    purge(Sample)
   end
 
   test "unused with local with overridable" do