Skip to content

Commit

Permalink
Remove multiple clauses warning for default values on bodiless clause (
Browse files Browse the repository at this point in the history
  • Loading branch information
ggcampinho authored and josevalim committed Dec 11, 2017
1 parent 5e79041 commit b99306b
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 31 deletions.
31 changes: 16 additions & 15 deletions lib/elixir/src/elixir_def.erl
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ fetch_definition([[Tuple] | T], File, Module, Table, All, Private) ->
fetch_definition(T, File, Module, Table, NewAll, NewPrivate)
catch
error:badarg ->
warn_bodyless_function(Check, Meta, File, Module, Kind, Tuple),
warn_bodiless_function(Check, Meta, File, Module, Kind, Tuple),
fetch_definition(T, File, Module, Table, All, Private)
end;

Expand Down Expand Up @@ -188,7 +188,7 @@ run_with_location_change(File, E, Callback) ->
end.

def_to_clauses(_Kind, Meta, Args, [], nil, E) ->
check_args_for_bodyless_clause(Meta, Args, E),
check_args_for_bodiless_clause(Meta, Args, E),
[];
def_to_clauses(Kind, Meta, _Args, _Guards, nil, E) ->
elixir_errors:form_error(Meta, ?key(E, file), elixir_expand, {missing_option, Kind, [do]});
Expand Down Expand Up @@ -223,7 +223,7 @@ store_definition(Check, Kind, Meta, Name, Arity, File, Module, Defaults, Clauses
check_valid_kind(Meta, File, Name, Arity, Kind, StoredKind),
(Check and StoredCheck) andalso
check_valid_clause(Meta, File, Name, Arity, Kind, Data, StoredMeta, StoredFile),
check_valid_defaults(Meta, File, Name, Arity, Kind, Defaults, StoredDefaults, LastDefaults, LastHasBody),
check_valid_defaults(Meta, File, Name, Arity, Kind, Defaults, StoredDefaults, LastDefaults, HasBody, LastHasBody),
max(Defaults, StoredDefaults);
[] ->
Defaults
Expand Down Expand Up @@ -292,27 +292,28 @@ check_valid_clause(Meta, File, Name, Arity, Kind, Data, StoredMeta, StoredFile)
end.

% Clause with defaults after clause with defaults
check_valid_defaults(Meta, File, Name, Arity, Kind, Defaults, StoredDefaults, _, _) when Defaults > 0, StoredDefaults > 0 ->
check_valid_defaults(Meta, File, Name, Arity, Kind, Defaults, StoredDefaults, _, true, _) when Defaults > 0, StoredDefaults > 0 ->
elixir_errors:form_error(Meta, File, ?MODULE, {clauses_with_defaults, {Kind, Name, Arity}});
% Clause with defaults after clause(s) without defaults
check_valid_defaults(Meta, File, Name, Arity, Kind, Defaults, 0, 0, _) when Defaults > 0 ->
check_valid_defaults(Meta, File, Name, Arity, Kind, Defaults, 0, 0, _, _) when Defaults > 0 ->
elixir_errors:form_warn(Meta, File, ?MODULE, {clauses_with_defaults, {Kind, Name, Arity}});
% Clause without defaults directly after clause with defaults (body less does not count)
check_valid_defaults(Meta, File, Name, Arity, Kind, 0, _, LastDefaults, true) when LastDefaults > 0 ->
% Clause without defaults directly after clause with defaults (bodiless does not count)
check_valid_defaults(Meta, File, Name, Arity, Kind, 0, _, LastDefaults, true, true) when LastDefaults > 0 ->
elixir_errors:form_warn(Meta, File, ?MODULE, {clauses_with_defaults, {Kind, Name, Arity}});
% Clause without defaults
check_valid_defaults(_Meta, _File, _Name, _Arity, _Kind, 0, _, _, _) -> ok.
check_valid_defaults(_Meta, _File, _Name, _Arity, _Kind, 0, _StoredDefaults, _LastDefaults, _HasBody, _LastHasBody) ->
ok.

warn_bodyless_function(Check, _Meta, _File, Module, _Kind, _Tuple)
warn_bodiless_function(Check, _Meta, _File, Module, _Kind, _Tuple)
when Check == false; Module == 'Elixir.Module' ->
ok;
warn_bodyless_function(_Check, Meta, File, _Module, Kind, Tuple) ->
elixir_errors:form_warn(Meta, File, ?MODULE, {bodyless_clause, Kind, Tuple}),
warn_bodiless_function(_Check, Meta, File, _Module, Kind, Tuple) ->
elixir_errors:form_warn(Meta, File, ?MODULE, {bodiless_clause, Kind, Tuple}),
ok.

check_args_for_bodyless_clause(Meta, Args, E) ->
check_args_for_bodiless_clause(Meta, Args, E) ->
[begin
elixir_errors:form_error(Meta, ?key(E, file), ?MODULE, invalid_args_for_bodyless_clause)
elixir_errors:form_error(Meta, ?key(E, file), ?MODULE, invalid_args_for_bodiless_clause)
end || Arg <- Args, invalid_arg(Arg)].

invalid_arg({Name, _, Kind}) when is_atom(Name), is_atom(Kind) -> false;
Expand Down Expand Up @@ -347,7 +348,7 @@ assert_valid_name(_Meta, _Kind, _Name, _Args, _S) ->

%% Format errors

format_error({bodyless_clause, Kind, {Name, Arity}}) ->
format_error({bodiless_clause, Kind, {Name, Arity}}) ->
io_lib:format("implementation not provided for predefined ~ts ~ts/~B", [Kind, Name, Arity]);

format_error({no_module, {Kind, Name, Arity}}) ->
Expand Down Expand Up @@ -389,7 +390,7 @@ 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_bodyless_clause) ->
format_error(invalid_args_for_bodiless_clause) ->
"only variables and \\\\ are allowed as arguments in definition header.\n"
"\n"
"If you did not intend to define a header, make sure your function "
Expand Down
26 changes: 20 additions & 6 deletions lib/elixir/test/elixir/kernel/docs_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -160,12 +160,24 @@ defmodule Kernel.DocsTest do

@doc "Function doc"
@since "1.2.3"
def foo(arg) do
arg + 1
end
def foo(arg), do: arg + 1

@doc "Multiple bodiless clause doc"
@since "1.2.3"
def bar(_arg)
def bar(_arg)
def bar(arg), do: arg + 1

@doc "Wrong doc"
@since "1.2"
def baz(_arg)
def baz(arg), do: arg + 1
@doc "Multiple bodiless clause and docs"
@since "1.2.3"
def baz(_arg)

@doc false
def bar(true), do: false
def qux(true), do: false
end
)

Expand All @@ -176,8 +188,10 @@ defmodule Kernel.DocsTest do
assert Code.get_docs(SampleDocs, :callback_docs) == docs[:callback_docs]

assert [
{{:bar, 1}, _, :def, [{:bool, _, Elixir}], false},
{{:foo, 1}, _, :def, [{:arg, _, nil}], "Function doc"}
{{:bar, 1}, _, :def, [{:arg, _, nil}], "Multiple bodiless clause doc"},
{{:baz, 1}, _, :def, [{:arg, _, nil}], "Multiple bodiless clause and docs"},
{{:foo, 1}, _, :def, [{:arg, _, nil}], "Function doc"},
{{:qux, 1}, _, :def, [{:bool, _, Elixir}], false}
] = docs[:docs]

assert {_, "Module doc"} = docs[:moduledoc]
Expand Down
42 changes: 32 additions & 10 deletions lib/elixir/test/elixir/kernel/warning_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -594,29 +594,51 @@ defmodule Kernel.WarningTest do
end

test "clause with defaults should be first" do
message = "definitions with multiple clauses and default values require a header"

assert capture_err(fn ->
Code.eval_string(~S"""
defmodule Sample do
def hello(arg), do: nil
def hello(arg \\ 0), do: nil
defmodule Sample1 do
def hello(arg), do: arg
def hello(arg \\ 0), do: arg
end
""")
end) =~ "definitions with multiple clauses and default values require a header"
end) =~ message

assert capture_err(fn ->
Code.eval_string(~S"""
defmodule Sample2 do
def hello(_arg)
def hello(arg \\ 0), do: arg
end
""")
end) =~ message
after
purge(Sample)
purge([Sample1, Sample2])
end

test "clauses with default should use fun head" do
message = "definitions with multiple clauses and default values require a header"

assert capture_err(fn ->
Code.eval_string(~S"""
defmodule Sample do
def hello(arg \\ 0), do: nil
def hello(arg), do: nil
defmodule Sample1 do
def hello(arg \\ 0), do: arg
def hello(arg), do: arg
end
""")
end) =~ "definitions with multiple clauses and default values require a header"
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(Sample)
purge([Sample1, Sample2])
end

test "unused with local with overridable" do
Expand Down

0 comments on commit b99306b

Please sign in to comment.