Merge pull request #265 from inaka/jfacorro.263.max_function_length_opts

[Closes #263] Options for max function length
inaka · Aug 31, 2015 · d23289a · d23289a
2 parents 61a0798 + 3a4ce04
commit d23289a
Show file tree

Hide file tree

Showing 4 changed files with 71 additions and 22 deletions.
diff --git a/elvis.config b/elvis.config
@@ -28,7 +28,7 @@
                   },
                   {elvis_style, state_record_and_type},
                   {elvis_style, no_spec_with_records},
-                  {elvis_style, dont_repeat_yourself, #{min_complexity => 10}}
+                  {elvis_style, dont_repeat_yourself, #{min_complexity => 20}}
                  ]
        },
       #{dirs => ["."],

diff --git a/src/elvis_style.erl b/src/elvis_style.erl
@@ -431,27 +431,42 @@ max_module_length(Config, Target, RuleConfig) ->
     [elvis_result:item()].
 max_function_length(Config, Target, RuleConfig) ->
     MaxLength = maps:get(max_length, RuleConfig, 30),
+    CountComments = maps:get(count_comments, RuleConfig, false),
+    CountWhitespace = maps:get(count_whitespace, RuleConfig, false),
 
     {Root, _} = elvis_file:parse_tree(Config, Target),
+    {Src, _} = elvis_file:src(Target),
+    Lines = binary:split(Src, <<"\n">>, [global, trim]),
 
     IsFunction = fun(Node) -> ktn_code:type(Node) == function end,
     Functions = elvis_code:find(IsFunction, Root),
+
+    FilterFun =
+        fun(Line) ->
+                (CountComments orelse (not line_is_comment(Line)))
+                    andalso (CountWhitespace
+                             orelse (not line_is_whitespace(Line)))
+        end,
+
     PairFun =
         fun(FunctionNode) ->
                 Name = ktn_code:attr(name, FunctionNode),
                 {Min, Max} = node_line_limits(FunctionNode),
-                L = (Max - Min) + 1,
-                {Name, L}
+                FunLines = lists:sublist(Lines, Min, Max - Min + 1),
+                FilteredLines = lists:filter(FilterFun, FunLines),
+                L = length(FilteredLines),
+                {Name, Min, L}
         end,
-    FunLenPairs = lists:map(PairFun, Functions),
-    MaxLengthPred = fun({_, L}) -> L > MaxLength end,
-    FunLenMaxPairs = lists:filter(MaxLengthPred, FunLenPairs),
+
+    FunLenInfos = lists:map(PairFun, Functions),
+    MaxLengthPred = fun({_, _, L}) -> L > MaxLength end,
+    FunLenMaxPairs = lists:filter(MaxLengthPred, FunLenInfos),
 
     ResultFun =
-        fun({Name, L}) ->
+        fun({Name, StartPos, L}) ->
                 Info = [Name, L, MaxLength],
                 Msg = ?MAX_FUNCTION_LENGTH,
-                elvis_result:new(item, Msg, Info, 0)
+                elvis_result:new(item, Msg, Info, StartPos)
         end,
     lists:map(ResultFun, FunLenMaxPairs).
 

diff --git a/test/examples/fail_max_function_length.erl b/test/examples/fail_max_function_length.erl
@@ -5,32 +5,32 @@
 f5(_) -> %% 1
     %% 2
     %% 3
-    %% 4
+
     ok. %% 5
 
 f10(_) -> %% 1
     %% 2
     %% 3
     %% 4
-    %% 5
+
     %% 6
     %% 7
-    %% 8
+
     %% 9
-    ok.
+    ok. %% 10
 
 f15(_) -> %% 1
     %% 2
     %% 3
     %% 4
-    %% 5
+
     %% 6
     %% 7
     %% 8
     %% 9
-    %% 10
+
     %% 11
     %% 12
-    %% 13
+
     %% 14
-    ok.
+    ok. %% 15
diff --git a/test/style_SUITE.erl b/test/style_SUITE.erl
@@ -387,19 +387,53 @@ verify_max_function_length(_Config) ->
 
     PathFail = "fail_max_function_length.erl",
     {ok, FileFail} = elvis_test_utils:find_file(SrcDirs, PathFail),
-    RuleConfig = #{max_length => 4},
+
+    CountAllRuleConfig = #{count_comments => true, count_whitespace => true},
+
+    ct:comment("Count whitespace and comment lines"),
+    RuleConfig = CountAllRuleConfig#{max_length => 4},
     [_, _, _] =
-         elvis_style:max_function_length(ElvisConfig, FileFail, RuleConfig),
+        elvis_style:max_function_length(ElvisConfig, FileFail, RuleConfig),
 
-    RuleConfig1 = #{max_length => 9},
+    RuleConfig1 = CountAllRuleConfig#{max_length => 9},
     [_, _] =
         elvis_style:max_function_length(ElvisConfig, FileFail, RuleConfig1),
 
-    RuleConfig2 = #{max_length => 14},
+    RuleConfig2 = CountAllRuleConfig#{max_length => 14},
     [_] = elvis_style:max_function_length(ElvisConfig, FileFail, RuleConfig2),
 
-    RuleConfig3 = #{max_length => 15},
-    [] = elvis_style:max_function_length(ElvisConfig, FileFail, RuleConfig3).
+    RuleConfig3 = CountAllRuleConfig#{max_length => 15},
+    [] = elvis_style:max_function_length(ElvisConfig, FileFail, RuleConfig3),
+
+    ct:comment("Don't count whitespace lines"),
+    WhitespaceRuleConfig = CountAllRuleConfig#{count_whitespace => false},
+
+    RuleConfig4 = WhitespaceRuleConfig#{max_length => 3},
+    [_, _, _] =
+        elvis_style:max_function_length(ElvisConfig, FileFail, RuleConfig4),
+
+    RuleConfig5 = WhitespaceRuleConfig#{max_length => 7},
+    [_, _] =
+        elvis_style:max_function_length(ElvisConfig, FileFail, RuleConfig5),
+
+    RuleConfig6 = WhitespaceRuleConfig#{max_length => 8},
+    [_] = elvis_style:max_function_length(ElvisConfig, FileFail, RuleConfig6),
+
+    RuleConfig7 = WhitespaceRuleConfig#{max_length => 11},
+    [_] = elvis_style:max_function_length(ElvisConfig, FileFail, RuleConfig7),
+
+    RuleConfig8 = WhitespaceRuleConfig#{max_length => 12},
+    [] = elvis_style:max_function_length(ElvisConfig, FileFail, RuleConfig8),
+
+    ct:comment("Don't count comment or whitespace lines"),
+    NoCountRuleConfig = WhitespaceRuleConfig#{count_comments => false},
+
+    RuleConfig9 = NoCountRuleConfig#{max_length => 1},
+    [_, _, _] =
+        elvis_style:max_function_length(ElvisConfig, FileFail, RuleConfig9),
+
+    RuleConfig10 = NoCountRuleConfig#{max_length => 2},
+    [] = elvis_style:max_function_length(ElvisConfig, FileFail, RuleConfig10).
 
 -spec results_are_ordered_by_line(config()) -> any().
 results_are_ordered_by_line(_Config) ->