Merge pull request #104 from inaka/jfacorro.21.state.records

[#21] State records in OTP modules.

config/elvis-test.config

@@ -3,7 +3,8 @@
    elvis,
    [
     {config,
-     [#{src_dirs => ["../../src"],
+     [#{dirs => ["../../src"],
+        filter => "*.erl",
         rules => [{elvis_style, line_length, [80]},
                   {elvis_style, no_tabs, []},
                   {elvis_style, macro_names, []},
@@ -21,7 +22,8 @@
                     elvis_style,
                     module_naming_convention,
                     ["^([a-z][a-z0-9]*_?)*(_SUITE)?$", []]
-                  }
+                  },
+                  {elvis_style, state_record_and_type, []}
                  ]
        },
       #{dirs => ["."],

config/elvis.config

@@ -22,7 +22,8 @@
                     elvis_style,
                     module_naming_convention,
                     ["^([a-z][a-z0-9]*_?)*(_SUITE)?$", []]
-                  }
+                  },
+                  {elvis_style, state_record_and_type, []}
                  ]
        },
       #{dirs => ["."],

config/old/elvis-test.config

@@ -21,7 +21,8 @@
                    elvis_style,
                    module_naming_convention,
                    ["^([a-z][a-z0-9]*_?)*(_SUITE)?$", []]
-                 }
+                 },
+                 {elvis_style, state_record_and_type, []}
                 ]
       }
     }

config/old/elvis.config

@@ -21,7 +21,8 @@
                        elvis_style,
                        module_naming_convention,
                        ["^([a-z][a-z0-9]*_?)*(_SUITE)?$", []]
-                     }
+                     },
+                     {elvis_style, state_record_and_type, []}
                     ]
        }
     }

config/test.config

@@ -22,7 +22,8 @@
                     elvis_style,
                     module_naming_convention,
                     ["^([a-z][a-z0-9]*_?)*(_SUITE)?$", []]
-                  }
+                  },
+                  {elvis_style, state_record_and_type, []}
                  ]
        },
       #{dirs => ["."],

src/elvis_code.erl

@@ -729,6 +729,20 @@ to_map({type, Attrs, Subtype, Types}) ->
                  text => Text,
                  subtype => Subtype},
       content => to_map(Types)};
+to_map({type, Attrs, map_field_assoc, Name, Type}) ->
+    {Location, Text} =
+        case Attrs of
+            Line when is_integer(Attrs) ->
+                {{Line, Line}, undefined};
+            Attrs ->
+                {get_location(Attrs),
+                 get_text(Attrs)}
+        end,
+    #{type => type_map_field,
+      attrs => #{location => Location,
+                 key => to_map(Name),
+                 text => Text,
+                 type => to_map(Type)}};
 to_map({remote_type, Attrs, ModuleName}) ->
     #{type => record_field,
       attrs => #{location => get_location(Attrs),
@@ -745,6 +759,13 @@ to_map(any) -> %% any()
 
 %% Other Attributes
 
+to_map({attribute, Attrs, type, {Name, Type, Args}}) ->
+    #{type => type_attr,
+      attrs => #{location => get_location(Attrs),
+                 text => get_text(Attrs),
+                 name => Name,
+                 args => to_map(Args),
+                 type => to_map(Type)}};
 to_map({attribute, Attrs, Type, Value}) ->
     #{type => Type,
       attrs => #{location => get_location(Attrs),

src/elvis_style.erl

@@ -12,7 +12,8 @@
          invalid_dynamic_call/3,
          used_ignored_variable/3,
          no_behavior_info/3,
-         module_naming_convention/3
+         module_naming_convention/3,
+         state_record_and_type/3
         ]).
 
 -define(LINE_LENGTH_MSG, "Line ~p is too long: ~p.").
@@ -60,6 +61,15 @@
         "The module ~p does not respect the format defined by the "
         "regular expression '~p'.").
 
+-define(STATE_RECORD_MISSING_MSG,
+        "This module implements an OTP behavior but is missing "
+        "a 'state' record.").
+
+-define(STATE_TYPE_MISSING_MSG,
+        "This module implements an OTP behavior and has a 'state' record "
+        "but is missing a 'state()' type.").
+
+
 %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
 %% Rules
 %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
@@ -207,7 +217,31 @@ module_naming_convention(Config, Target, [Regex, IgnoreModules]) ->
                     Result = elvis_result:new(item, Msg, Info, 1),
                     [Result];
                 {match, _} -> []
-            end
+            end;
+        true -> []
+    end.
+
+-spec state_record_and_type(elvis_config:config(),
+                            elvis_utils:file(),
+                            [list()]) ->
+    [elvis_result:item()].
+state_record_and_type(Config, Target, []) ->
+    {Root, _} = elvis_utils:parse_tree(Config, Target),
+    case is_otp_module(Root) of
+        true ->
+            case {has_state_record(Root), has_state_type(Root)} of
+                {true, true} -> [];
+                {false, _} ->
+                    Msg = ?STATE_RECORD_MISSING_MSG,
+                    Result = elvis_result:new(item, Msg, [], 1),
+                    [Result];
+                {true, false} ->
+                    Msg = ?STATE_TYPE_MISSING_MSG,
+                    Result = elvis_result:new(item, Msg, [], 1),
+                    [Result]
+            end;
+        false ->
+            []
     end.
 
 %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
@@ -415,3 +449,48 @@ check_parent_match(Zipper) ->
                 _ -> check_parent_match(ParentZipper)
             end
     end.
+
+-spec is_otp_module(elvis_code:tree_node()) -> boolean().
+is_otp_module(Root) ->
+    OtpSet = sets:from_list([gen_server,
+                             gen_event,
+                             gen_fsm,
+                             supervisor_bridge
+                            ]),
+    IsBehaviorAttr = fun(Node) ->  behavior == elvis_code:type(Node) end,
+    case elvis_code:find(IsBehaviorAttr, Root) of
+        [] ->
+            false;
+        Behaviors ->
+            ValueFun = fun(Node) -> elvis_code:attr(value, Node) end,
+            Names = lists:map(ValueFun, Behaviors),
+            BehaviorsSet = sets:from_list(Names),
+            case sets:to_list(sets:intersection(OtpSet, BehaviorsSet)) of
+                [] -> false;
+                _ -> true
+            end
+    end.
+
+-spec has_state_record(elvis_code:node()) -> boolean().
+has_state_record(Root) ->
+    IsStateRecord =
+        fun(Node) ->
+                (record_attr == elvis_code:type(Node))
+                    and (state == elvis_code:attr(name, Node))
+        end,
+    case elvis_code:find(IsStateRecord, Root) of
+        [] -> false;
+        _ -> true
+    end.
+
+-spec has_state_type(elvis_code:node()) -> boolean().
+has_state_type(Root) ->
+    IsStateType =
+        fun(Node) ->
+                (type_attr == elvis_code:type(Node))
+                    and (state == elvis_code:attr(name, Node))
+        end,
+    case elvis_code:find(IsStateType, Root) of
+        [] -> false;
+        _ -> true
+    end.

src/elvis_utils.erl

@@ -185,7 +185,7 @@ filter_files(Files, Filter) ->
 -spec glob_to_regex(iodata()) -> iodata().
 glob_to_regex(Glob) ->
     Regex1 = re:replace(Glob, "\\.", "\\\\."),
-    re:replace(Regex1,"\\*", ".*").
+    re:replace(Regex1, "\\*", ".*").
 
 %% @doc Takes a line, a character and a count, returning the indentation level
 %%      invalid if the number of character is not a multiple of count.

test/elvis_SUITE.erl

@@ -343,12 +343,6 @@ main_unexistent(_Config) ->
 %%% Private
 %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
 
-check_some_line_output(Fun, Expected) ->
-    IsEqual = fun(Result, Exp) ->
-                     Result == Exp
-             end,
-    check_some_line_output(Fun, Expected, IsEqual).
-
 check_some_line_output(Fun, Expected, FilterFun) ->
     ct:capture_start(),
     Fun(),

test/examples/fail_state_record_and_type.erl

@@ -0,0 +1,36 @@
+-module(fail_state_record_and_type).
+
+-behavior(gen_server).
+
+-export([
+         init/1,
+         handle_call/3,
+         handle_cast/2,
+         handle_info/2,
+         code_change/3,
+         terminate/2
+        ]).
+
+-spec init(term()) -> ok.
+init(_Args) ->
+    ok.
+
+-spec handle_call(term(), term(), term()) -> ok.
+handle_call(_Request, _From, _State) ->
+    ok.
+
+-spec handle_cast(term(), term()) -> ok.
+handle_cast(_Request, _State) ->
+    ok.
+
+-spec handle_info(term(), term()) -> ok.
+handle_info(_Info, _State) ->
+    ok.
+
+-spec code_change(term(), term(), term()) -> term().
+code_change(_OldVsn, State, _Extra) ->
+    {ok, State}.
+
+-spec terminate(term(), term()) -> ok.
+terminate(_Reason, _State) ->
+    ok.

test/examples/fail_state_type.erl

@@ -0,0 +1,38 @@
+-module(fail_state_type).
+
+-behavior(gen_server).
+
+-export([
+         init/1,
+         handle_call/3,
+         handle_cast/2,
+         handle_info/2,
+         code_change/3,
+         terminate/2
+        ]).
+
+-record(state, {}).
+
+-spec init(term()) -> ok.
+init(_Args) ->
+    #state{}.
+
+-spec handle_call(term(), term(), term()) -> ok.
+handle_call(_Request, _From, _State) ->
+    ok.
+
+-spec handle_cast(term(), term()) -> ok.
+handle_cast(_Request, _State) ->
+    ok.
+
+-spec handle_info(term(), term()) -> ok.
+handle_info(_Info, _State) ->
+    ok.
+
+-spec code_change(term(), term(), term()) -> term().
+code_change(_OldVsn, State, _Extra) ->
+    {ok, State}.
+
+-spec terminate(term(), term()) -> ok.
+terminate(_Reason, _State) ->
+    ok.

test/examples/pass_state_record_and_type.erl

@@ -0,0 +1,40 @@
+-module(pass_state_record_and_type).
+
+-behavior(gen_server).
+
+-export([
+         init/1,
+         handle_call/3,
+         handle_cast/2,
+         handle_info/2,
+         code_change/3,
+         terminate/2
+        ]).
+
+-record(state, {}).
+
+-type state() :: #state{}.
+
+-spec init(term()) -> state().
+init(_Args) ->
+    #state{}.
+
+-spec handle_call(term(), term(), term()) -> ok.
+handle_call(_Request, _From, _State) ->
+    ok.
+
+-spec handle_cast(term(), term()) -> ok.
+handle_cast(_Request, _State) ->
+    ok.
+
+-spec handle_info(term(), term()) -> ok.
+handle_info(_Info, _State) ->
+    ok.
+
+-spec code_change(term(), term(), term()) -> term().
+code_change(_OldVsn, State, _Extra) ->
+    {ok, State}.
+
+-spec terminate(term(), term()) -> ok.
+terminate(_Reason, _State) ->
+    ok.

test/style_SUITE.erl

@@ -18,7 +18,8 @@
          verify_invalid_dynamic_call/1,
          verify_used_ignored_variable/1,
          verify_no_behavior_info/1,
-         verify_module_naming_convention/1
+         verify_module_naming_convention/1,
+         verify_state_record_and_type/1
         ]).
 
 -define(EXCLUDED_FUNS,
@@ -212,3 +213,20 @@ verify_module_naming_convention(_Config) ->
     {ok, FileFail} = elvis_test_utils:find_file(SrcDirs, PathFail),
     [_] =
         elvis_style:module_naming_convention(ElvisConfig, FileFail, RuleConfig).
+
+-spec verify_state_record_and_type(config()) -> any().
+verify_state_record_and_type(_Config) ->
+    ElvisConfig = elvis_config:default(),
+    SrcDirs = elvis_config:dirs(ElvisConfig),
+
+    PathPass = "pass_state_record_and_type.erl",
+    {ok, FilePass} = elvis_test_utils:find_file(SrcDirs, PathPass),
+    [] = elvis_style:state_record_and_type(ElvisConfig, FilePass, []),
+
+    PathFail = "fail_state_record_and_type.erl",
+    {ok, FileFail} = elvis_test_utils:find_file(SrcDirs, PathFail),
+    [_] = elvis_style:state_record_and_type(ElvisConfig, FileFail, []),
+
+    PathFail1 = "fail_state_type.erl",
+    {ok, FileFail1} = elvis_test_utils:find_file(SrcDirs, PathFail1),
+    [_] = elvis_style:state_record_and_type(ElvisConfig, FileFail1, []).