diff --git a/src/elvis_code.erl b/src/elvis_code.erl index 0bc7ac1..e85a00d 100644 --- a/src/elvis_code.erl +++ b/src/elvis_code.erl @@ -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. @@ -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)}}; diff --git a/src/elvis_style.erl b/src/elvis_style.erl index 967ac73..70054b9 100644 --- a/src/elvis_style.erl +++ b/src/elvis_style.erl @@ -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."). @@ -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 @@ -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 %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% @@ -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. diff --git a/test/examples/fail_invalid_dynamic_call.erl b/test/examples/fail_invalid_dynamic_call.erl new file mode 100644 index 0000000..4d2df78 --- /dev/null +++ b/test/examples/fail_invalid_dynamic_call.erl @@ -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. diff --git a/test/examples/pass_invalid_dynamic_call.erl b/test/examples/pass_invalid_dynamic_call.erl new file mode 100644 index 0000000..18c074c --- /dev/null +++ b/test/examples/pass_invalid_dynamic_call.erl @@ -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. diff --git a/test/rules_SUITE.erl b/test/rules_SUITE.erl index a282cd4..c4eda64 100644 --- a/test/rules_SUITE.erl +++ b/test/rules_SUITE.erl @@ -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, @@ -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, []).