Change else clause error to warningemj/else-warn

4 files changed, 44 insertions, 32 deletions

diff --git a/lib/elixir/src/elixir_clauses.erl b/lib/elixir/src/elixir_clauses.erl
index 373d9c531..3a2800e3f 100644
--- a/lib/elixir/src/elixir_clauses.erl
+++ b/lib/elixir/src/elixir_clauses.erl
@@ -185,12 +185,17 @@ expand_with_else(Meta, Opts, E, HasMatch) ->
 'try'(Meta, [], E) ->
   form_error(Meta, ?key(E, file), elixir_expand, {missing_option, 'try', [do]});
 'try'(Meta, [{do, _}], E) ->
-  form_error(Meta, ?key(E, file), elixir_expand, {missing_option, 'try', ['catch', 'rescue', 'after', 'else']});
-'try'(Meta, [{do, _}, {else, _}], E) ->
-  form_error(Meta, ?key(E, file), ?MODULE, {try_with_only_else_clause, origin(Meta, 'try')});
+  form_error(Meta, ?key(E, file), elixir_expand, {missing_option, 'try', ['catch', 'rescue', 'after']});
 'try'(Meta, Opts, E) when not is_list(Opts) ->
   form_error(Meta, ?key(E, file), elixir_expand, {invalid_args, 'try'});
 'try'(Meta, Opts, E) ->
+  % TODO: Make this an error in Elixir 2.0
+  case Opts of
+    [{do, _}, {else, _}] ->
+      form_warn(Meta, ?key(E, file), ?MODULE, {try_with_only_else_clause, origin(Meta, 'try')});
+    _ ->
+      ok
+  end,
   RaiseError = fun(Key) ->
     form_error(Meta, ?key(E, file), ?MODULE, {duplicated_clauses, 'try', Key})
   end,
@@ -351,7 +356,7 @@ format_error({catch_before_rescue, Origin}) ->
   io_lib:format("\"catch\" should always come after \"rescue\" in ~ts", [Origin]);
 
 format_error({try_with_only_else_clause, Origin}) ->
-  io_lib:format("\"else\" can't be used as the only clause in \"~ts\" since it doesn't do anything",
+  io_lib:format("\"else\" shouldn't be used as the only clause in \"~ts\", use \"case\" instead",
                 [Origin]);
 
 format_error(unmatchable_else_in_with) ->
diff --git a/lib/elixir/test/elixir/kernel/errors_test.exs b/lib/elixir/test/elixir/kernel/errors_test.exs
index 4013c231b..ff5aa32a8 100644
--- a/lib/elixir/test/elixir/kernel/errors_test.exs
+++ b/lib/elixir/test/elixir/kernel/errors_test.exs
@@ -987,18 +987,6 @@ defmodule Kernel.ErrorsTest do
     """
   end
 
-  test "def raises if there's only the else clause" do
-    assert_eval_raise CompileError, ~r"\"else\" can't be used as the only clause in \"def\"", """
-    defmodule Example do
-      def foo do
-        bar()
-      else
-        _other -> :ok
-      end
-    end
-    """
-  end
-
   defp bad_remote_call(x), do: x.foo
 
   defmacro sample(0), do: 0
diff --git a/lib/elixir/test/elixir/kernel/expansion_test.exs b/lib/elixir/test/elixir/kernel/expansion_test.exs
index da795fb47..1517a1f3f 100644
--- a/lib/elixir/test/elixir/kernel/expansion_test.exs
+++ b/lib/elixir/test/elixir/kernel/expansion_test.exs
@@ -1637,7 +1637,7 @@ defmodule Kernel.ExpansionTest do
     end
 
     test "expects more than do" do
-      assert_raise CompileError, ~r"missing :catch/:rescue/:after/:else option in \"try\"", fn ->
+      assert_raise CompileError, ~r"missing :catch/:rescue/:after option in \"try\"", fn ->
         code =
           quote do
             try do
@@ -1657,21 +1657,6 @@ defmodule Kernel.ExpansionTest do
       end
     end
 
-    test "raises if the only clause other than do is else" do
-      assert_raise CompileError, ~r"\"else\" can't be used as the only clause", fn ->
-        code =
-          quote do
-            try do
-              :ok
-            else
-              other -> other
-            end
-          end
-
-        expand(code)
-      end
-    end
-
     test "expects at most one clause" do
       assert_raise CompileError, ~r"duplicated :do clauses given for \"try\"", fn ->
         expand(quote(do: try(do: e, do: f)))
diff --git a/lib/elixir/test/elixir/kernel/warning_test.exs b/lib/elixir/test/elixir/kernel/warning_test.exs
index 0689e1fd6..1df6c3ccd 100644
--- a/lib/elixir/test/elixir/kernel/warning_test.exs
+++ b/lib/elixir/test/elixir/kernel/warning_test.exs
@@ -1640,6 +1640,40 @@ defmodule Kernel.WarningTest do
     assert message =~ "1 < x < y < 10"
   end
 
+  test "def warns if only clause is else" do
+    message =
+      capture_err(fn ->
+        Code.compile_string("""
+        defmodule Sample do
+          def foo do
+            :bar
+          else
+            _other -> :ok
+          end
+        end
+        """)
+      end)
+
+    assert message =~ "\"else\" shouldn't be used as the only clause in \"def\""
+  after
+    purge(Sample)
+  end
+
+  test "try warns if only clause is else" do
+    message =
+      capture_err(fn ->
+        Code.compile_string("""
+          try do
+            :ok
+          else
+            other -> other
+          end
+        """)
+      end)
+
+    assert message =~ "\"else\" shouldn't be used as the only clause in \"try\""
+  end
+
   defp purge(list) when is_list(list) do
     Enum.each(list, &purge/1)
   end