Skip to content

Commit

Permalink
[Closes #17] Implemented invalid dynamic calls rule.
Browse files Browse the repository at this point in the history
  • Loading branch information
jfacorro committed Jul 21, 2014
1 parent b87fb32 commit b30cee0
Show file tree
Hide file tree
Showing 5 changed files with 156 additions and 8 deletions.
19 changes: 13 additions & 6 deletions src/elvis_code.erl
Original file line number Diff line number Diff line change
Expand Up @@ -114,10 +114,17 @@ print_node(Node) ->

-spec print_node(tree_node(), integer()) -> ok.
print_node(Node = #{type := Type}, CurrentLevel) ->
Indentation = lists:duplicate(CurrentLevel * 4, 32),
{Line, _} = elvis_code:attr(location, Node),
lager:info("~s - [~p] ~p : ~p~n",
[Indentation, CurrentLevel, Type, Line]).
Type = type(Node),
Indentation = lists:duplicate(CurrentLevel * 4, $ ),
Content = content(Node),
% {Line, _} = elvis_code:attr(location, Node),
lager:info(
"~s - [~p] ~p~n",
[Indentation, CurrentLevel, Type]
),
lists:map(fun(Child) -> print_node(Child, CurrentLevel + 1) end, Content),
ok.


%% @private
%% @doc Takes a node type and determines its nesting level increment.
Expand Down Expand Up @@ -245,13 +252,13 @@ to_map({var, Location, Name}) ->
%% Function call

to_map({call, Location, Function, Arguments}) ->
#{type => var,
#{type => call,
attrs => #{location => Location,
function => to_map(Function),
arguments => to_map(Arguments)}};

to_map({remote, Location, Module, Function}) ->
#{type => var,
#{type => remote,
attrs => #{location => Location,
module => to_map(Module),
function => to_map(Function)}};
Expand Down
49 changes: 48 additions & 1 deletion src/elvis_style.erl
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@
operator_spaces/3,
nesting_level/3,
god_modules/3,
no_if_expression/3
no_if_expression/3,
invalid_dynamic_call/3
]).

-define(LINE_LENGTH_MSG, "Line ~p is too long: ~p.").
Expand All @@ -29,6 +30,9 @@
-define(NO_IF_EXPRESSION_MSG,
"Replace the 'if' expression on line ~p with a 'case' "
"expression or function clauses.").
-define (INVALID_DYNAMIC_CALL_MSG,
"Remove the dynamic function call on line ~p. "
"Only modules that define callbacks should make dynamic calls.").

%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
%% Rules
Expand Down Expand Up @@ -109,6 +113,19 @@ no_if_expression(Config, Target, []) ->
lists:map(ResultFun, IfExprs)
end.

-spec invalid_dynamic_call(elvis_config:config(), elvis_utils:file(), []) ->
[elvis_result:item()].
invalid_dynamic_call(Config, Target, []) ->
{ok, Src} = elvis_utils:src(Config, Target),
Root = elvis_code:parse_tree(Src),
Predicate = fun(Node) -> elvis_code:type(Node) == 'callback' end,
case elvis_code:find(Predicate, Root) of
[] ->
check_invalid_dynamic_calls(Root);
_Callbacks ->
[]
end.

%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
%% Private
%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
Expand Down Expand Up @@ -231,3 +248,33 @@ check_nesting_level(ParentNode, [MaxLevel]) ->

lists:map(Fun, NestedNodes)
end.

-spec check_invalid_dynamic_calls(elvis_code:tree_node()) ->
[elvis_result:item_result()].
check_invalid_dynamic_calls(Root) ->
case elvis_code:find(fun is_dynamic_call/1, Root) of
[] -> [];
InvalidCalls ->
ResultFun =
fun (Node) ->
{LineNum, _} = elvis_code:attr(location, Node),
Msg = ?INVALID_DYNAMIC_CALL_MSG,
Info = [LineNum],
elvis_result:new(item, Msg, Info, LineNum)
end,
lists:map(ResultFun, InvalidCalls)
end.

-spec is_dynamic_call(elvis_code:tree_node()) ->
boolean().
is_dynamic_call(Node = #{type := call}) ->
FunctionSpec = elvis_code:attr(function, Node),
case elvis_code:type(FunctionSpec) of
remote ->
ModuleName = elvis_code:attr(module, FunctionSpec),
var == elvis_code:type(ModuleName);
_Other ->
false
end;
is_dynamic_call(_Node) ->
false.
36 changes: 36 additions & 0 deletions test/examples/fail_invalid_dynamic_call.erl
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
-module(fail_invalid_dynamic_call).

-export([
dynamic_module_name_call/0,
dynamic_function_name_call/0,
another_dynamic_module_name_call/0,
dynamic_module_name_call_in_case/0
]).

dynamic_module_name_call() ->
normal:call(),
Module = a_module,
Module:call().

dynamic_function_name_call() ->
normal:call(),
Function = a_function,
a_module:Function(),
normal:call().

another_dynamic_module_name_call() ->
normal:call(),
another_normal:call(),
Module = another_module,
Module:call_to_function(),
Module:call_to__another_function().

dynamic_module_name_call_in_case() ->
normal:call(),
another_normal:call(),
case 1 of
1 ->
Module = another_module,
Module:call_to_function();
2 -> ok
end.
39 changes: 39 additions & 0 deletions test/examples/pass_invalid_dynamic_call.erl
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
-module(pass_invalid_dynamic_call).

-export([
dynamic_module_name_call/0,
dynamic_function_name_call/0,
another_dynamic_module_name_call/0,
dynamic_module_name_call_in_case/0
]).

-callback init(Arg :: any()) ->
any().

dynamic_module_name_call() ->
normal:call(),
Module = a_module,
Module:call().

dynamic_function_name_call() ->
normal:call(),
Function = a_function,
a_module:Function(),
normal:call().

another_dynamic_module_name_call() ->
normal:call(),
another_normal:call(),
Module = another_module,
Module:call_to_function(),
Module:call_to__another_function().

dynamic_module_name_call_in_case() ->
normal:call(),
another_normal:call(),
case 1 of
1 ->
Module = another_module,
Module:call_to_function();
2 -> ok
end.
21 changes: 20 additions & 1 deletion test/rules_SUITE.erl
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@
verify_operator_spaces/1,
verify_nesting_level/1,
verify_god_modules/1,
verify_no_if_expression/1
verify_no_if_expression/1,
verify_invalid_dynamic_call/1
]).

-define(EXCLUDED_FUNS,
Expand Down Expand Up @@ -145,3 +146,21 @@ verify_no_if_expression(_Config) ->
[#{line_num := 9},
#{line_num := 20},
#{line_num := 29}] = elvis_style:no_if_expression(ElvisConfig, File, []).

-spec verify_invalid_dynamic_call(config()) -> any().
verify_invalid_dynamic_call(_Config) ->
ElvisConfig = elvis_config:default(),
#{src_dirs := SrcDirs} = ElvisConfig,

PathFail = "fail_invalid_dynamic_call.erl",
{ok, FileFail} = elvis_test_utils:find_file(SrcDirs, PathFail),
[
#{line_num := 13},
#{line_num := 25},
#{line_num := 26},
#{line_num := 34}
] = elvis_style:invalid_dynamic_call(ElvisConfig, FileFail, []),

PathPass = "pass_invalid_dynamic_call.erl",
{ok, FilePass} = elvis_test_utils:find_file(SrcDirs, PathPass),
[] = elvis_style:invalid_dynamic_call(ElvisConfig, FilePass, []).

0 comments on commit b30cee0

Please sign in to comment.