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

New elvis_style rule: max_function_clause_length #365

Merged
merged 1 commit into from
Oct 16, 2024
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
1 change: 1 addition & 0 deletions RULES.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
27 changes: 27 additions & 0 deletions doc_rules/elvis_style/max_function_clause_length.md
Original file line number Diff line number Diff line change
@@ -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
}}
```
1 change: 1 addition & 0 deletions src/elvis_rulesets.erl
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
87 changes: 85 additions & 2 deletions src/elvis_style.erl
Original file line number Diff line number Diff line change
Expand Up @@ -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,
paulo-ferraz-oliveira marked this conversation as resolved.
Show resolved Hide resolved
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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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) ->
Expand Down Expand Up @@ -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".
elbrujohalcon marked this conversation as resolved.
Show resolved Hide resolved

-spec max_function_length(elvis_config:config(),
elvis_file:file(),
max_function_length_config()) ->
Expand Down
90 changes: 90 additions & 0 deletions test/examples/fail_max_function_clause_length.erl
Original file line number Diff line number Diff line change
@@ -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.
50 changes: 50 additions & 0 deletions test/examples/function_clause_numbers.erl
Original file line number Diff line number Diff line change
@@ -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".
52 changes: 52 additions & 0 deletions test/examples/pass_max_function_clause_length.erl
Original file line number Diff line number Diff line change
@@ -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
Loading