New elvis_style rule: no_init_lists (#367)

RULES.md

@@ -62,6 +62,7 @@ identified with `(since ...)` for convenience purposes.
 - [State Record and Type](doc_rules/elvis_style/state_record_and_type.md)
 - [Used Ignored Variable](doc_rules/elvis_style/used_ignored_variable.md)
 - [Variable Naming Convention](doc_rules/elvis_style/variable_naming_convention.md)
+- [No Init Lists](doc_rules/elvis_style/no_init_lists.md)
 - [Prefer Unquoted Atoms](doc_rules/elvis_text_style/prefer_unquoted_atoms.md)
 
 ## `.gitignore` rules

doc_rules/elvis_style/no_init_lists.md

@@ -0,0 +1,17 @@
+# No Init Lists
+
+Do not use a list as the parameter for the `init/1` callback when implementing `gen_*` behaviours.
+It's semantically clearer to use a tuple or a map.
+[More info](https://erlangforums.com/t/args-in-gen-init-1/3169/5)
+
+> Works on `.beam` files? Yes!
+
+## Options
+
+- None.
+
+## Example
+
+```erlang
+{elvis_style, no_init_lists, #{}}
+```

src/elvis_rulesets.erl

@@ -97,6 +97,7 @@ rules(erl_files_strict) ->
                              max_function_length,
                              max_module_length,
                              no_call,
+                             no_init_lists,
                              no_common_caveats_call,
                              no_macros,
                              state_record_and_type]]);
@@ -119,6 +120,7 @@ rules(beam_files) ->
                no_debug_call,
                no_if_expression,
                no_import,
+               no_init_lists,
                no_match_in_condition,
                no_nested_try_catch,
                no_single_clause_case,

src/elvis_style.erl

@@ -12,7 +12,8 @@
          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,
-         no_match_in_condition/3, param_pattern_matching/3, private_data_types/3, option/3]).
+         no_match_in_condition/3, param_pattern_matching/3, private_data_types/3, option/3,
+         no_init_lists/3]).
 
 -export_type([empty_rule_config/0]).
 -export_type([ignorable/0]).
@@ -28,8 +29,10 @@
               no_import_config/0, no_catch_expressions_config/0, numeric_format_config/0,
               no_single_clause_case_config/0, consistent_variable_casing_config/0,
               no_match_in_condition_config/0, behaviour_spelling_config/0,
-              param_pattern_matching_config/0, private_data_type_config/0]).
+              param_pattern_matching_config/0, private_data_type_config/0, no_init_lists_config/0]).
 
+-define(NO_INIT_LISTS_MSG,
+        "Do not use a list as the parameter for the 'init' callback at position ~p.").
 -define(INVALID_MACRO_NAME_REGEX_MSG,
         "The macro named ~p on line ~p does not respect the format "
         "defined by the regular expression '~p'.").
@@ -147,6 +150,9 @@
 %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
 
 -spec default(Rule :: atom()) -> DefaultRuleConfig :: term().
+default(no_init_lists) ->
+    #{behaviours =>
+          [gen_server, gen_statem, gen_fsm, supervisor, supervisor_bridge, gen_event]};
 default(macro_names) ->
     #{regex => "^[A-Z](_?[A-Z0-9]+)*$"};
 default(operator_spaces) ->
@@ -1112,6 +1118,79 @@ atom_naming_convention(Config, Target, RuleConfig) ->
     AtomNodes = elvis_code:find(fun is_atom_node/1, Root, #{traverse => all, mode => node}),
     check_atom_names(Regex, RegexEnclosed, AtomNodes, []).
 
+-type no_init_lists_config() :: #{behaviours => [atom()]}.
+
+-spec no_init_lists(elvis_config:config(), elvis_file:file(), no_init_lists_config()) ->
+                       [elvis_result:item()].
+no_init_lists(Config, Target, RuleConfig) ->
+    Root = get_root(Config, Target, RuleConfig),
+
+    ListInitClauses =
+        case is_relevant_behaviour(Root, RuleConfig) of
+            true ->
+                IsInit1Function =
+                    fun(Node) ->
+                       ktn_code:type(Node) == function
+                       andalso ktn_code:attr(name, Node) == init
+                       andalso ktn_code:attr(arity, Node) == 1
+                    end,
+
+                case elvis_code:find(IsInit1Function, Root) of
+                    [] ->
+                        [];
+                    [Init1Fun] ->
+                        Content = ktn_code:content(Init1Fun),
+                        ListAttrClauses =
+                            lists:filtermap(fun(X) -> filter_list_clause_location(X) end, Content),
+                        case length(ListAttrClauses) =:= length(Content) of
+                            true ->
+                                ListAttrClauses;
+                            false ->
+                                []
+                        end
+                end;
+            false ->
+                []
+        end,
+
+    ResultFun =
+        fun(Location) ->
+           Info = [Location],
+           Msg = ?NO_INIT_LISTS_MSG,
+           elvis_result:new(item, Msg, Info, Location)
+        end,
+
+    lists:map(ResultFun, ListInitClauses).
+
+is_relevant_behaviour(Root, RuleConfig) ->
+    ConfigBehaviors = option(behaviours, RuleConfig, no_init_lists),
+    IsBehaviour = fun(Node) -> ktn_code:type(Node) == behaviour end,
+    Behaviours = elvis_code:find(IsBehaviour, Root),
+    lists:any(fun(Elem) -> Elem =:= true end,
+              lists:map(fun(BehaviourNode) ->
+                           lists:member(
+                               ktn_code:attr(value, BehaviourNode), ConfigBehaviors)
+                        end,
+                        Behaviours)).
+
+filter_list_clause_location(Clause) ->
+    [Attribute] = ktn_code:node_attr(pattern, Clause),
+    case is_list_node(Attribute) of
+        true ->
+            {true, ktn_code:attr(location, Clause)};
+        false ->
+            false
+    end.
+
+is_list_node(#{type := cons}) ->
+    true;
+is_list_node(#{type := nil}) ->
+    true;
+is_list_node(#{type := match, content := Content}) ->
+    lists:any(fun(Elem) -> is_list_node(Elem) end, Content);
+is_list_node(_) ->
+    false.
+
 -spec no_throw(elvis_config:config(), elvis_file:file(), empty_rule_config()) ->
                   [elvis_result:item()].
 no_throw(Config, Target, RuleConfig) ->

test/examples/no_init_lists_examples/example_behaviour.erl

@@ -0,0 +1,3 @@
+-module(example_behaviour).
+
+-callback example() -> atom(). 

test/examples/no_init_lists_examples/fail_no_init_lists.erl

@@ -0,0 +1,15 @@
+-module(fail_no_init_lists).
+
+-behaviour(gen_server).
+
+-export([start_link/1, init/1, handle_cast/2, handle_call/3]).
+
+start_link(AParam) ->
+  gen_server:start_link(?MODULE, AParam, []).
+
+init([_AParam]) ->
+  ok.
+
+handle_cast(_, _) -> ok.
+
+handle_call(_, _, _) -> ok.

test/examples/no_init_lists_examples/fail_no_init_lists2.erl

@@ -0,0 +1,15 @@
+-module(fail_no_init_lists2).
+
+-behaviour(gen_server).
+
+-export([start_link/1, init/1, handle_cast/2, handle_call/3]).
+
+start_link(AParam) ->
+  gen_server:start_link(?MODULE, AParam, []).
+
+init(_A = [1, 2, 3]) ->
+  ok.
+
+handle_cast(_, _) -> ok.
+
+handle_call(_, _, _) -> ok.

test/examples/no_init_lists_examples/fail_no_init_lists3.erl

@@ -0,0 +1,21 @@
+-module(fail_no_init_lists3).
+
+-behaviour(gen_server).
+
+-export([start_link/1, init/1, handle_cast/2, handle_call/3]).
+
+start_link(AParam) ->
+  gen_server:start_link(?MODULE, AParam, []).
+
+init([_]) ->
+  ok;
+
+init(_A = [1, 2, 3]) ->
+  ok;
+
+init([undefined]) ->
+  ok.
+
+handle_cast(_, _) -> ok.
+
+handle_call(_, _, _) -> ok.

test/examples/no_init_lists_examples/fail_no_init_lists4.erl

@@ -0,0 +1,15 @@
+-module(fail_no_init_lists4).
+
+-behaviour(gen_server).
+-behaviour(example_behaviour).
+
+-export([init/1, handle_cast/2, handle_cast/3, handle_call/3]).
+-export([example/0]).
+
+init([]) -> {error, "should not be a list"}.
+
+handle_cast(_, _) -> ok.
+handle_cast(_, _, _) -> ok.
+handle_call(_, _, _) -> ok.
+
+example() -> ok.

test/examples/no_init_lists_examples/fail_no_init_lists5.erl

@@ -0,0 +1,12 @@
+-module(fail_no_init_lists5).
+
+-behaviour(gen_fsm).
+
+-export([init/1, handle_sync_event/4, handle_event/3]).
+
+init([]) -> {error, "Don't use list for init/1"}.
+
+handle_sync_event(_, _, _, _) -> ok.
+
+handle_event(_, _, _) -> ok.
+

test/examples/no_init_lists_examples/fail_no_init_lists6.erl

@@ -0,0 +1,7 @@
+-module(fail_no_init_lists6).
+
+-behaviour(supervisor).
+
+-export([init/1]).
+
+init([]) -> {error, "Don't use list for init/1"}.

test/examples/no_init_lists_examples/fail_no_init_lists7.erl

@@ -0,0 +1,9 @@
+-module(fail_no_init_lists7).
+
+-behaviour(supervisor_bridge).
+
+-export([init/1, terminate/2]).
+
+init([]) -> {error, "Don't use list for init/1"}.
+
+terminate(_, _) -> ok.

test/examples/no_init_lists_examples/fail_no_init_lists8.erl

@@ -0,0 +1,11 @@
+-module(fail_no_init_lists8).
+
+-behaviour(gen_event).
+
+-export([init/1, handle_event/2, handle_call/2]).
+
+init([]) -> {error, "Don't use list for init/1"}.
+
+handle_event(_, _) -> ok.
+
+handle_call(_, _) -> ok.

test/examples/no_init_lists_examples/pass_no_init_lists.erl

@@ -0,0 +1,21 @@
+-module(pass_no_init_lists).
+
+-behaviour(gen_server).
+
+-export([start_link/0, init/1, handle_cast/2, handle_call/3]).
+
+start_link() ->
+  gen_server:start_link(?MODULE, 1, []).
+
+init(1) ->
+  ok;
+
+init([undefined]) ->
+  ok;
+
+init([_]) ->
+  ok.
+
+handle_cast(_, _) -> ok.
+
+handle_call(_, _, _) -> ok.

test/examples/no_init_lists_examples/pass_no_init_lists2.erl

@@ -0,0 +1,18 @@
+-module(pass_no_init_lists2).
+
+-behaviour(gen_server).
+
+-export([start_link/0, init/1, init/2, handle_cast/2, handle_call/3]).
+
+start_link() ->
+  gen_server:start_link(?MODULE, #{a => map}, []).
+
+init(#{}) ->
+  ok.
+
+init([_], undefined) ->
+  ok.
+
+handle_cast(_, _) -> ok.
+
+handle_call(_, _, _) -> ok.

test/examples/no_init_lists_examples/pass_no_init_lists3.erl

@@ -0,0 +1,18 @@
+-module(pass_no_init_lists3).
+
+-behaviour(gen_server).
+
+-export([start_link/0, init/0, init/1, handle_cast/2, handle_call/3]).
+
+start_link() ->
+  gen_server:start_link(?MODULE, {1, 2}, []).
+
+init({1, 2}) ->
+  ok.
+
+init() ->
+  ok.
+
+handle_cast(_, _) -> ok.
+
+handle_call(_, _, _) -> ok.

test/examples/no_init_lists_examples/pass_no_init_lists4.erl

@@ -0,0 +1,8 @@
+-module(pass_no_init_lists4).
+
+-export([start_link/1, init/1]).
+
+start_link(AParam) ->
+  gen_server:start_link(?MODULE, [AParam], []).
+
+init([_AParam]) -> ok.

test/examples/no_init_lists_examples/pass_no_init_lists5.erl

@@ -0,0 +1,8 @@
+-module(pass_no_init_lists5).
+
+-behaviour(example_behaviour).
+
+-export([init/1, example/0]).
+
+init([]) -> {error, "can be a list"}.
+example() -> ok.