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.

Sign up for GitHub

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

[Closes #261] Options for max module line length #264

Merged
merged 1 commit into from
Aug 31, 2015
+73 −24
Merged

[Closes #261] Options for max module line length #264

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
57 changes: 39 additions & 18 deletions src/elvis_style.erl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -394,14 +394,27 @@ dont_repeat_yourself(Config, Target, RuleConfig) ->
max_module_length(Config, Target, RuleConfig) ->
MaxLength = maps:get(max_length, RuleConfig, 500),
IgnoreModules = maps:get(ignore, RuleConfig, []),
CountComments = maps:get(count_comments, RuleConfig, false),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we make it work the other way around? like ignore_comments instead of count_comments?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried that first (actually with skip_*) but it is a lot more confusing for the user when setting a value for the options. count_* is an affirmation while ignore_* is a negation. What should ignore_comments do? Ignore them in the sense of counting them so they add up to the line count or ignore them in the sense of not counting them?

CountWhitespace = maps:get(count_whitespace, RuleConfig, false),

{Root, _} = elvis_file:parse_tree(Config, Target),
{Src, _} = elvis_file:src(Target),


ModuleName = elvis_code:module_name(Root),
Lines = binary:split(Src, <<"\n">>, [global, trim]),
Ignored = lists:member(ModuleName, IgnoreModules),

FilterFun =
fun(Line) ->
(CountComments orelse (not line_is_comment(Line)))
andalso (CountWhitespace
orelse (not line_is_whitespace(Line)))
end,
Lines = case binary:split(Src, <<"\n">>, [global, trim]) of
Ls when CountComments andalso CountWhitespace -> Ls;
Ls -> lists:filter(FilterFun, Ls)
end,

case length(Lines) of
L when L > MaxLength, not Ignored ->
Info = [ModuleName, L, MaxLength],
Expand All @@ -426,7 +439,8 @@ max_function_length(Config, Target, RuleConfig) ->
PairFun =
fun(FunctionNode) ->
Name = ktn_code:attr(name, FunctionNode),
L = function_line_length(FunctionNode),
{Min, Max} = node_line_limits(FunctionNode),
L = (Max - Min) + 1,
{Name, L}
end,
FunLenPairs = lists:map(PairFun, Functions),
Expand All @@ -441,14 +455,15 @@ max_function_length(Config, Target, RuleConfig) ->
end,
lists:map(ResultFun, FunLenMaxPairs).

-spec function_line_length(ktn_code:tree_node())-> [{atom(), integer()}].
function_line_length(FunctionNode) ->
-spec node_line_limits(ktn_code:tree_node())->
{Min :: integer(), Max :: integer()}.
node_line_limits(FunctionNode) ->
Zipper = elvis_code:code_zipper(FunctionNode),
LineFun = fun(N) -> {L, _} = ktn_code:attr(location, N), L end,
LineNums = zipper:map(LineFun, Zipper),
Max = lists:max(LineNums),
Min = lists:min(LineNums),
(Max - Min) + 1.
{Min, Max}.

%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
%% Private
Expand All @@ -474,13 +489,20 @@ result_node_line_col_fun(Msg) ->

%% Line Length

-spec line_is_comment(binary()) -> true | false.
-spec line_is_comment(binary()) -> boolean().
line_is_comment(Line) ->
case re:run(Line, "^[ \t]*%") of
nomatch -> false;
{match, _} -> true
end.

-spec line_is_whitespace(binary()) -> boolean().
line_is_whitespace(Line) ->
case re:run(Line, "^[ \t]*$") of
nomatch -> false;
{match, _} -> true
end.

-spec remove_comment(binary()) -> binary().
remove_comment(Line) ->
case re:run(Line, "([^%]+)", [{capture, first, binary}]) of
Expand Down Expand Up @@ -547,11 +569,11 @@ check_no_spaces(Line, Num, _Args) ->
no_result | {ok, elvis_result:item()}.
check_no_trailing_whitespace(Line, Num, RuleConfig) ->
Regex =
case RuleConfig of
%% Lookbehind assertion: http://erlang.org/doc/man/re.html#sect17
#{ignore_empty_lines := true} -> "(?<=\\S)\\s+$";
_AnythingElse -> "\\s+$"
end,
case RuleConfig of
%% Lookbehind assertion: http://erlang.org/doc/man/re.html#sect17
#{ignore_empty_lines := true} -> "(?<=\\S)\\s+$";
_AnythingElse -> "\\s+$"
end,

case re:run(Line, Regex) of
nomatch ->
Expand Down Expand Up @@ -637,8 +659,8 @@ is_remote_call({Num, Col}, Root) ->
{ok, Node0} ->
Pred =
fun(Zipper) ->
(Node0 == zipper:node(Zipper))
andalso has_remote_call_parent(Zipper)
(Node0 == zipper:node(Zipper))
andalso has_remote_call_parent(Zipper)
end,
[] =/= elvis_code:find(Pred, Root, #{mode => zipper})
end.
Expand Down Expand Up @@ -712,9 +734,9 @@ check_nesting_level(ParentNode, [MaxLevel]) ->
Msg = ?NESTING_LEVEL_MSG,

Fun = fun(Node) ->
{Line, Col} = ktn_code:attr(location, Node),
Info = [Line, Col, MaxLevel],
elvis_result:new(item, Msg, Info, Line)
{Line, Col} = ktn_code:attr(location, Node),
Info = [Line, Col, MaxLevel],
elvis_result:new(item, Msg, Info, Line)
end,

lists:map(Fun, NestedNodes)
Expand All @@ -732,8 +754,7 @@ check_invalid_dynamic_calls(Root) ->
lists:map(ResultFun, InvalidCalls)
end.

-spec is_dynamic_call(ktn_code:tree_node()) ->
boolean().
-spec is_dynamic_call(ktn_code:tree_node()) -> boolean().
is_dynamic_call(Node) ->
case ktn_code:type(Node) of
call ->
Expand Down
4 changes: 2 additions & 2 deletions test/examples/fail_max_module_length.erl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,11 @@



%% Random comment







%% @doc A function
f(_) -> ok.
36 changes: 32 additions & 4 deletions test/style_SUITE.erl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -343,14 +343,42 @@ verify_max_module_length(_Config) ->

PathFail = "fail_max_module_length.erl",
{ok, FileFail} = elvis_test_utils:find_file(SrcDirs, PathFail),
RuleConfig = #{max_length => 10},

CountAllRuleConfig = #{count_comments => true, count_whitespace => true},

ct:comment("Count whitespace and comment lines"),
RuleConfig = CountAllRuleConfig#{max_length => 10},
[_] = elvis_style:max_module_length(ElvisConfig, FileFail, RuleConfig),

RuleConfig1 = #{max_length => 14},
RuleConfig1 = CountAllRuleConfig#{max_length => 14},
[_] = elvis_style:max_module_length(ElvisConfig, FileFail, RuleConfig1),

RuleConfig2 = #{max_length => 15},
[] = elvis_style:max_module_length(ElvisConfig, FileFail, RuleConfig2).
RuleConfig2 = CountAllRuleConfig#{max_length => 15},
[] = elvis_style:max_module_length(ElvisConfig, FileFail, RuleConfig2),

ct:comment("Don't count whitespace lines"),
WhitespaceRuleConfig = CountAllRuleConfig#{count_whitespace => false},

RuleConfig3 = WhitespaceRuleConfig#{max_length => 3},
[_] = elvis_style:max_module_length(ElvisConfig, FileFail, RuleConfig3),

RuleConfig4 = WhitespaceRuleConfig#{max_length => 4},
[_] = elvis_style:max_module_length(ElvisConfig, FileFail, RuleConfig4),

RuleConfig5 = WhitespaceRuleConfig#{max_length => 5},
[] = elvis_style:max_module_length(ElvisConfig, FileFail, RuleConfig5),

ct:comment("Don't count comment or whitespace lines"),
NoCountRuleConfig = WhitespaceRuleConfig#{count_comments => false},

RuleConfig6 = NoCountRuleConfig#{max_length => 1},
[_] = elvis_style:max_module_length(ElvisConfig, FileFail, RuleConfig6),

RuleConfig7 = NoCountRuleConfig#{max_length => 2},
[_] = elvis_style:max_module_length(ElvisConfig, FileFail, RuleConfig7),

RuleConfig8 = NoCountRuleConfig#{max_length => 3},
[] = elvis_style:max_module_length(ElvisConfig, FileFail, RuleConfig8).

-spec verify_max_function_length(config()) -> any().
verify_max_function_length(_Config) ->
Expand Down