diff --git a/RULES.md b/RULES.md index e4c0153..979908b 100644 --- a/RULES.md +++ b/RULES.md @@ -26,6 +26,7 @@ identified with `(since ...)` for convenience purposes. - [Max Anonymous Function Arity](doc_rules/elvis_style/max_anonymous_function_arity.md) - [Max Function Arity](doc_rules/elvis_style/max_function_arity.md) - [Max Function Length](doc_rules/elvis_style/max_function_length.md) +- [Max Function Clause Length](doc_rules/elvis_style/max_function_clause_length.md) - [Max Module Length](doc_rules/elvis_style/max_module_length.md) - [Module Naming Convention](doc_rules/elvis_style/module_naming_convention.md) - [Nesting Level](doc_rules/elvis_style/nesting_level.md) diff --git a/Untitled b/Untitled new file mode 100644 index 0000000..f74ccf4 --- /dev/null +++ b/Untitled @@ -0,0 +1 @@ +tyl diff --git a/doc_rules/elvis_style/max_function_clause_length.md b/doc_rules/elvis_style/max_function_clause_length.md new file mode 100644 index 0000000..7be725b --- /dev/null +++ b/doc_rules/elvis_style/max_function_clause_length.md @@ -0,0 +1,27 @@ +# Max Function Clause Length + +This specifies an upper bound on function clause **line** length. Lines that are comments and/or whitespace +can be either included or excluded from the line count. + +> Works on `.beam` file? Not really! (it consumes results Ok, but these might be unexpected, since +the files are pre-processed) + +## Options + +- `max_length :: non_neg_integer()`. + - default: `30`. +- `count_comments :: boolean()` + - default: `false` +- `count_whitespace :: boolean()` + - default: `false` + +## Example + +```erlang +{elvis_style, max_function_clause_length} +%% or +{elvis_style, max_function_clause_length, #{ max_length => 30 + , count_comments => false + , count_whitespace => false + }} +``` diff --git a/src/elvis_rulesets.erl b/src/elvis_rulesets.erl index ba1bc4b..f5b0fd6 100644 --- a/src/elvis_rulesets.erl +++ b/src/elvis_rulesets.erl @@ -93,6 +93,7 @@ rules(erl_files_strict) -> || Rule <- [always_shortcircuit, consistent_generic_type, + max_function_clause_length, max_function_length, max_module_length, no_call, diff --git a/src/elvis_style.erl b/src/elvis_style.erl index e59c59b..16bb730 100644 --- a/src/elvis_style.erl +++ b/src/elvis_style.erl @@ -7,8 +7,8 @@ invalid_dynamic_call/3, used_ignored_variable/3, no_behavior_info/3, module_naming_convention/3, state_record_and_type/3, no_spec_with_records/3, dont_repeat_yourself/3, max_module_length/3, max_anonymous_function_arity/3, - max_function_arity/3, max_function_length/3, no_call/3, no_debug_call/3, - no_common_caveats_call/3, no_nested_try_catch/3, no_successive_maps/3, + max_function_arity/3, max_function_length/3, max_function_clause_length/3, no_call/3, + no_debug_call/3, no_common_caveats_call/3, no_nested_try_catch/3, no_successive_maps/3, atom_naming_convention/3, no_throw/3, no_dollar_space/3, no_author/3, no_import/3, no_catch_expressions/3, no_single_clause_case/3, numeric_format/3, behaviour_spelling/3, always_shortcircuit/3, consistent_generic_type/3, export_used_types/3, @@ -96,6 +96,9 @@ -define(MAX_FUNCTION_LENGTH, "The code for function ~p/~w has ~p lines which exceeds the " "maximum of ~p."). +-define(MAX_FUNCTION_CLAUSE_LENGTH, + "The code for the ~ts clause of 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, @@ -190,6 +193,10 @@ default(max_function_length) -> #{max_length => 30, count_comments => false, count_whitespace => false}; +default(max_function_clause_length) -> + #{max_length => 30, + count_comments => false, + count_whitespace => false}; default(no_call) -> #{no_call_functions => []}; default(no_debug_call) -> @@ -891,6 +898,82 @@ max_function_arity(Config, Target, RuleConfig) -> end, Functions). +-spec max_function_clause_length(elvis_config:config(), + elvis_file:file(), + max_function_length_config()) -> + [elvis_result:item()]. +max_function_clause_length(Config, Target, RuleConfig) -> + MaxLength = option(max_length, RuleConfig, max_function_length), + CountComments = option(count_comments, RuleConfig, max_function_length), + CountWhitespace = option(count_whitespace, RuleConfig, max_function_length), + + Root = get_root(Config, Target, RuleConfig), + {Src, _} = elvis_file:src(Target), + Lines = elvis_utils:split_all_lines(Src, [trim]), + + IsFunction = fun(Node) -> ktn_code:type(Node) == function end, + Functions0 = elvis_code:find(IsFunction, Root), + + % clause + FilterClause = + fun(Line) -> + (CountComments orelse not line_is_comment(Line)) + andalso (CountWhitespace orelse not line_is_whitespace(Line)) + end, + + PairClause = + fun(ClauseNode, {Result, PrevAccNum}) -> + {Min, Max} = node_line_limits(ClauseNode), + FunLines = lists:sublist(Lines, Min, Max - Min + 1), + FilteredLines = lists:filter(FilterClause, FunLines), + L = length(FilteredLines), + AccNum = PrevAccNum + 1, + ClauseNumber = parse_clause_num(AccNum), + {[{Min, ClauseNumber, L} | Result], AccNum} + end, + + % fun + PairFun = + fun(FunctionNode) -> + Name = ktn_code:attr(name, FunctionNode), + Arity = ktn_code:attr(arity, FunctionNode), + + IsClause = fun(Node) -> ktn_code:type(Node) == clause end, + Clauses = elvis_code:find(IsClause, FunctionNode), + + {ClauseLenInfos, _} = lists:foldl(PairClause, {[], 0}, Clauses), + + [{Name, Arity, Min, StringClauseNumber, L} + || {Min, StringClauseNumber, L} <- ClauseLenInfos] + end, + + ClauseLenInfos = + lists:reverse( + lists:append( + lists:map(PairFun, Functions0))), + + MaxLengthPred = fun({_, _, _, _, L}) -> L > MaxLength end, + ClauseLenMaxPairs = lists:filter(MaxLengthPred, ClauseLenInfos), + + ResultFun = + fun({Name, Arity, StartPos, ClauseNumber, L}) -> + Info = [ClauseNumber, Name, Arity, L, MaxLength], + Msg = ?MAX_FUNCTION_CLAUSE_LENGTH, + elvis_result:new(item, Msg, Info, StartPos) + end, + lists:map(ResultFun, ClauseLenMaxPairs). + +parse_clause_num(Num) when Num rem 100 >= 11, Num rem 100 =< 13 -> + integer_to_list(Num) ++ "th"; +parse_clause_num(Num) when Num rem 10 == 1 -> + integer_to_list(Num) ++ "st"; +parse_clause_num(Num) when Num rem 10 == 2 -> + integer_to_list(Num) ++ "nd"; +parse_clause_num(Num) when Num rem 10 == 3 -> + integer_to_list(Num) ++ "rd"; +parse_clause_num(Num) -> + integer_to_list(Num) ++ "th". + -spec max_function_length(elvis_config:config(), elvis_file:file(), max_function_length_config()) -> diff --git a/test/examples/fail_max_function_clause_length.erl b/test/examples/fail_max_function_clause_length.erl new file mode 100644 index 0000000..a7ad6c3 --- /dev/null +++ b/test/examples/fail_max_function_clause_length.erl @@ -0,0 +1,90 @@ +-module(fail_max_function_clause_length). + +-export([f5/1, f6/1, f7/1]). + +f5(a) -> %% 1 + %% 2 + %% 3 + + ok; %% 5 + +f5(b) -> %% 1 + %% 2 + %% 3 + %% 4 + + %% 6 + %% 7 + + %% 9 + + + + ok; %% 13 + +f5(c) -> %% 1 + %% 2 + %% 3 + %% 4 + + %% 6 + %% 7 + %% 8 + %% 9 + + %% 11 + %% 12 + + %% 14 + ok. %% 15 + +f6(d) -> %% 1 + %% 2 + %% 3 + %% 4 + + %% 6 + %% 7 + %% 8 + %% 9 + + %% 11 + %% 12 + + %% 14 + ok; %% 15 + +f6(e) -> %% 1 + ok; +f6(f) -> %% 1 + ok. + +f7(a) -> 1; +f7(b) -> 2; +f7(c) -> 3; +f7(d) -> 4; +f7(e) -> 5; +f7(f) -> 6; +f7(aa) -> 7; +f7(bb) -> + 8; +f7(cc) -> 9; +f7(dd) -> + 10; +f7(ee) -> 11; +f7(ff) -> 12; +f7(aaa) -> 13; +f7(bbb) -> 14; +f7(ccc) -> 15; +f7(ddd) -> 16; +f7(eee) -> 17; +f7(fff) -> 18; +f7(aaaa) -> 19; +f7(bbbb) -> + 20; +f7(cccc) -> 21; +f7(dddd) -> 22; +f7(eeee) -> 23; +f7(ffff) -> + 24; +f7(aaaaa) -> 25. diff --git a/test/examples/function_clause_numbers.erl b/test/examples/function_clause_numbers.erl new file mode 100644 index 0000000..58b528f --- /dev/null +++ b/test/examples/function_clause_numbers.erl @@ -0,0 +1,50 @@ +-module(function_clause_numbers). + +-export([clause/1]). + +clause(1) -> + "ok"; +clause(2) -> "ok"; +clause(3) -> "ok"; +clause(4) -> "ok"; +clause(5) -> + "ok"; +clause(6) -> + "ok"; +clause(7) -> + "ok"; +clause(8) -> "ok"; +clause(9) -> "ok"; +clause(10) -> "ok"; +clause(11) -> + "ok"; +clause(12) -> "ok"; +clause(13) -> + "ok"; +clause(14) -> "ok"; +clause(15) -> "ok"; +clause(16) -> "ok"; +clause(17) -> "ok"; +clause(18) -> "ok"; +clause(19) -> + "ok"; +clause(20) -> + "ok"; +clause(21) -> + "ok"; +clause(22) -> "ok"; +clause(23) -> "ok"; +clause(24) -> "ok"; +clause(25) -> "ok"; +clause(26) -> "ok"; +clause(27) -> "ok"; +clause(28) -> "ok"; +clause(29) -> + "ok"; +clause(30) -> + "ok"; +clause(31) -> + "ok"; +clause(32) -> "ok"; +clause(33) -> + "ok". diff --git a/test/examples/pass_max_function_clause_length.erl b/test/examples/pass_max_function_clause_length.erl new file mode 100644 index 0000000..1f86aa1 --- /dev/null +++ b/test/examples/pass_max_function_clause_length.erl @@ -0,0 +1,52 @@ +-module(pass_max_function_clause_length). + +-export([f5/1]). +f5(a) -> %% 1 + %% 2 + %% 3 + + ok; %% 5 + +f5(b) -> %% 1 + %% 2 + %% 3 + %% 4 + + %% 6 + %% 7 + + %% 9 + ok; %% 10 + +f5(c) -> %% 1 + %% 2 + %% 3 + %% 4 + + %% 6 + %% 7 + %% 8 + %% 9 + ok; %% 10 + +f5(d) -> %% 1 + %% 2 + %% 3 + %% 4 + + %% 6 + %% 7 + %% 8 + %% 9 + ok; %% 10 + +f5(e) -> %% 1 + %% 2 + %% 3 + %% 4 + + %% 6 + %% 7 + %% 8 + %% 9 + ok. %% 10 diff --git a/test/style_SUITE.erl b/test/style_SUITE.erl index 5be09c9..e741307 100644 --- a/test/style_SUITE.erl +++ b/test/style_SUITE.erl @@ -17,13 +17,14 @@ verify_state_record_and_type_plus_export_used_types/1, verify_no_spec_with_records/1, verify_dont_repeat_yourself/1, verify_max_module_length/1, verify_max_anonymous_function_arity/1, verify_max_function_arity/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_successive_maps/1, - verify_atom_naming_convention/1, verify_no_throw/1, verify_no_dollar_space/1, - verify_no_author/1, verify_no_import/1, verify_no_catch_expressions/1, - verify_no_single_clause_case/1, verify_numeric_format/1, verify_behaviour_spelling/1, - verify_always_shortcircuit/1, verify_consistent_generic_type/1, verify_no_types/1, - verify_no_specs/1, verify_export_used_types/1, verify_consistent_variable_casing/1, + verify_max_function_length/1, verify_max_function_clause_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_successive_maps/1, verify_atom_naming_convention/1, verify_no_throw/1, + verify_no_dollar_space/1, verify_no_author/1, verify_no_import/1, + verify_no_catch_expressions/1, verify_no_single_clause_case/1, verify_numeric_format/1, + verify_behaviour_spelling/1, verify_always_shortcircuit/1, + verify_consistent_generic_type/1, verify_no_types/1, verify_no_specs/1, + verify_export_used_types/1, verify_consistent_variable_casing/1, verify_no_match_in_condition/1, verify_param_pattern_matching/1, verify_private_data_types/1, verify_unquoted_atoms/1]). %% -elvis attribute @@ -1142,6 +1143,78 @@ verify_max_function_length(Config) -> {comment, ""}. +-spec verify_max_function_clause_length(config()) -> any(). +verify_max_function_clause_length(Config) -> + Ext = proplists:get_value(test_file_ext, Config, "erl"), + + PathFail = "fail_max_function_clause_length." ++ Ext, + + CountAllRuleConfig = #{count_comments => true, count_whitespace => true}, + RuleConfig = CountAllRuleConfig#{max_length => 10}, + + ct:comment("Count whitespace and comment lines"), + [_, _, _] = + elvis_core_apply_rule(Config, + elvis_style, + max_function_clause_length, + RuleConfig, + PathFail), + + PathSuccess = "pass_max_function_clause_length." ++ Ext, + + [] = + elvis_core_apply_rule(Config, + elvis_style, + max_function_clause_length, + RuleConfig, + PathSuccess), + + RuleConfig2 = CountAllRuleConfig#{max_length => 15}, + PathExtraSuccess = "fail_max_function_length." ++ Ext, + + [] = + elvis_core_apply_rule(Config, + elvis_style, + max_function_clause_length, + RuleConfig2, + PathExtraSuccess), + + RuleConfig3 = CountAllRuleConfig#{max_length => 1}, + + [_, _, _, _, _, _, _, _, _, _] = + elvis_core_apply_rule(Config, + elvis_style, + max_function_clause_length, + RuleConfig3, + PathFail), + + RuleConfig4 = CountAllRuleConfig#{max_length => 1}, + PathClauseNumbers = "function_clause_numbers." ++ Ext, + + Result = + elvis_core_apply_rule(Config, + elvis_style, + max_function_clause_length, + RuleConfig4, + PathClauseNumbers), + + Numbers = [Number || #{info := [Number, _, _, _, _]} <- Result], + + _ = Numbers + =:= ["1st", + "5th", + "6th", + "7th", + "11th", + "13th", + "19th", + "20th", + "21st", + "29th", + "30th", + "31st", + "33rd"]. + -spec verify_no_debug_call(config()) -> any(). verify_no_debug_call(Config) -> Group = proplists:get_value(group, Config, erl_files),