Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for no_call and no_common_caveats #98

Merged
merged 1 commit into from
Jun 29, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
110 changes: 82 additions & 28 deletions src/elvis_style.erl
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@
dont_repeat_yourself/3,
max_module_length/3,
max_function_length/3,
no_call/3,
no_debug_call/3,
no_common_caveats_call/3,
no_nested_try_catch/3,
no_seqbind/3,
no_useless_seqbind/3
Expand Down Expand Up @@ -110,9 +112,16 @@
"The code for function ~p/~w has ~p lines which exceeds the "
"maximum of ~p.").

-define(NO_CALL_MSG,
"The call to ~p:~p/~p on line ~p is in the no_call list.").

-define(NO_DEBUG_CALL_MSG,
"Remove the debug call to ~p:~p/~p on line ~p.").

-define(NO_COMMON_CAVEATS_CALL_MSG,
"The call to ~p:~p/~p on line ~p is in the list of "
"Erlang Efficiency Guide common caveats.").

-define(NO_NESTED_TRY_CATCH,
"Nested try...catch block starting at line ~p.").

Expand Down Expand Up @@ -607,37 +616,58 @@ max_function_length(Config, Target, RuleConfig) ->
end,
lists:map(ResultFun, FunLenMaxPairs).

-type no_debug_call_config() :: #{debug_functions => [function_spec()],
ignore => [module()]
}.
-type function_spec() :: {module(), atom(), non_neg_integer()}
| {module(), atom()}.

-type no_call_config() :: #{no_call_functions => [function_spec()],
ignore => [module()]
}.
-spec no_call(elvis_config:config(),
elvis_file:file(),
no_call_config()) ->
[elvis_result:item()].
no_call(Config, Target, RuleConfig) ->
DefaultFns = [],
no_call_common(Config, Target, RuleConfig, no_call_functions, DefaultFns, ?NO_CALL_MSG).


-type no_debug_call_config() :: #{debug_functions => [function_spec()],
ignore => [module()]
}.
-spec no_debug_call(elvis_config:config(),
elvis_file:file(),
no_debug_call_config()) ->
[elvis_result:item()].
no_debug_call(Config, Target, RuleConfig) ->
IgnoreModules = maps:get(ignore, RuleConfig, []),
{Root, _} = elvis_file:parse_tree(Config, Target),
ModuleName = elvis_code:module_name(Root),
DefaultDebugFuns = [{ct, pal},
{ct, print, 1},
{ct, print, 2},
{ct, print, 3},
{ct, print, 4},
{ct, print, 5},
{io, format, 1},
{io, format, 2}],
DebugFuns = maps:get(debug_functions, RuleConfig, DefaultDebugFuns),
DefaultFns = [{ct, pal},
{ct, print, 1},
{ct, print, 2},
{ct, print, 3},
{ct, print, 4},
{ct, print, 5},
{ct, print, 5},
{io, format, 1},
{io, format, 2}
],
no_call_common(Config, Target, RuleConfig, debug_functions, DefaultFns, ?NO_DEBUG_CALL_MSG).


-type no_common_caveats_call_config() :: #{caveat_functions => [function_spec()],
ignore => [module()]
}.
-spec no_common_caveats_call(elvis_config:config(),
elvis_file:file(),
no_common_caveats_call_config()) ->
[elvis_result:item()].

case lists:member(ModuleName, IgnoreModules) of
false ->
IsCall = fun(Node) -> ktn_code:type(Node) =:= 'call' end,
Calls = elvis_code:find(IsCall, Root),
check_no_debug_call(Calls, DebugFuns);
true -> []
end.
no_common_caveats_call(Config, Target, RuleConfig) ->
DefaultFns = [{timer, send_after, 2},
{timer, send_after, 3},
{timer, send_interval, 2},
{timer, send_interval, 3},
{erlang, size, 1}
],
no_call_common(Config, Target, RuleConfig, caveat_functions, DefaultFns, ?NO_COMMON_CAVEATS_CALL_MSG).

-spec node_line_limits(ktn_code:tree_node())->
{Min :: integer(), Max :: integer()}.
Expand Down Expand Up @@ -1216,23 +1246,47 @@ is_children(Parent, Node) ->
Zipper = elvis_code:code_zipper(Parent),
[] =/= zipper:filter(fun(Child) -> Child == Node end, Zipper).

%% No debug call
%% No call
-type no_call_configs() :: no_call_config()
| no_debug_call_config()
| no_common_caveats_call_config().
-spec no_call_common(elvis_config:config(),
elvis_file:file(),
no_call_configs(),
atom(),
[function_spec()],
string()
) ->
[elvis_result:item()].
no_call_common(Config, Target, RuleConfig, ConfigKey, DefaultNoCallFns, Msg) ->
IgnoreModules = maps:get(ignore, RuleConfig, []),
{Root, _} = elvis_file:parse_tree(Config, Target),
ModuleName = elvis_code:module_name(Root),
NoCallFuns = maps:get(ConfigKey, RuleConfig, DefaultNoCallFns),

case lists:member(ModuleName, IgnoreModules) of
false ->
IsCall = fun(Node) -> ktn_code:type(Node) =:= 'call' end,
Calls = elvis_code:find(IsCall, Root),
check_no_call(Calls, Msg, NoCallFuns);
true -> []
end.

-spec check_no_debug_call([ktn_code:node()], [function_spec()]) ->
-spec check_no_call([ktn_code:node()], string(), [function_spec()]) ->
[elvis_result:item()].
check_no_debug_call(Calls, DebugFuns) ->
DebugCalls = [Call || Call <- Calls, is_debug_call(Call, DebugFuns)],
check_no_call(Calls, Msg, NoCallFuns) ->
DebugCalls = [Call || Call <- Calls, is_in_call_list(Call, NoCallFuns)],
ResultFun = fun(Call) ->
{M, F, A} = call_mfa(Call),
{Line, _} = ktn_code:attr(location, Call),
elvis_result:new(item,
?NO_DEBUG_CALL_MSG,
Msg,
[M, F, A, Line],
Line)
end,
lists:map(ResultFun, DebugCalls).

is_debug_call(Call, DebugFuns) ->
is_in_call_list(Call, DebugFuns) ->
MFA = call_mfa(Call),
MatchFun = fun(Spec) -> fun_spec_match(Spec, MFA) end,
lists:any(MatchFun, DebugFuns).
Expand Down
16 changes: 16 additions & 0 deletions test/examples/fail_no_call_classes.erl
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
-module(fail_no_call_classes).
-export([fail/0]).

-spec fail() -> any().
fail() ->
_ = timer:send_after(0, fail_after),
_ = timer:send_after(0, self(), fail_after),

_ = timer:send_interval(0, fail_interval),
_ = timer:send_interval(0, self(), fail_interval),

_ = erlang:size({1,2,3}),
_ = erlang:size(<<"fail_size">>),

_ = erlang:tuple_size({1,2,3}),
_ = erlang:byte_size(<<"ok_size">>).
47 changes: 47 additions & 0 deletions test/style_SUITE.erl
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@
verify_max_module_length/1,
verify_max_function_length/1,
verify_no_debug_call/1,
verify_no_common_caveats_call/1,
verify_no_call/1,
verify_no_nested_try_catch/1,
verify_no_seqbind/1,
verify_no_useless_seqbind/1,
Expand Down Expand Up @@ -586,6 +588,44 @@ verify_no_debug_call(_Config) ->
RuleConfig5 = #{debug_functions => [{ct, print}]},
[_, _] = elvis_style:no_debug_call(ElvisConfig, FileFail, RuleConfig5).

%% We test no_call and no_common_caveats_call by building the equivalent config and make sure that
%% other than defaults, they behave the same
-spec verify_no_common_caveats_call(config()) -> any().
verify_no_common_caveats_call(_Config) ->
verify_no_call_flavours(no_common_caveats_call, fun elvis_style:no_common_caveats_call/3, caveat_functions, 6).

-spec verify_no_call(config()) -> any().
verify_no_call(_Config) ->
verify_no_call_flavours(no_call, fun elvis_style:no_call/3, no_call_functions, 0).

-spec verify_no_call_flavours(atom(), fun(), atom(), non_neg_integer()) -> any().
verify_no_call_flavours(RuleName, RuleFun, RuleConfigMapKey, ExpectedDefaultRuleMatchCount) ->
ElvisConfig = elvis_config:default(),
SrcDirs = elvis_config:dirs(ElvisConfig),

PathFail = "fail_no_call_classes.erl",
{ok, FileFail} = elvis_test_utils:find_file(SrcDirs, PathFail),

assert_length(ExpectedDefaultRuleMatchCount, RuleFun(ElvisConfig, FileFail, #{}), RuleName),

RuleConfig = #{ignore => [fail_no_call_classes]},
assert_length(0, RuleFun(ElvisConfig, FileFail, RuleConfig), RuleName),

RuleMatchTuples = [{{timer, send_after, 2}, 1},
{{timer, send_after, 3}, 1},
{{timer, send_interval, 2}, 1},
{{timer, send_interval, 3}, 1},
{{erlang, size, 1}, 2},
{{timer, send_after}, 2}
],

lists:foreach(fun({FunSpec, ExpectedCount}) ->
ThisRuleConfig = maps:from_list([{RuleConfigMapKey, [FunSpec]}]),
Result = RuleFun(ElvisConfig, FileFail, ThisRuleConfig),
assert_length(ExpectedCount, Result, RuleName)
end,
RuleMatchTuples).

-spec verify_no_nested_try_catch(config()) -> any().
verify_no_nested_try_catch(_Config) ->
ElvisConfig = elvis_config:default(),
Expand Down Expand Up @@ -651,3 +691,10 @@ is_list_sort([#{line_num := Line1} | T1]) ->
true -> is_list_sort(T1);
false -> false
end.

-spec assert_length(non_neg_integer(), [any()], atom()) -> any().
assert_length(Expected, List, RuleName) ->
case length(List) of
Expected -> ok;
_ -> throw({unexpected_response_length, RuleName, {expected, Expected}, {got, List}})
end.