Merge pull request #106 from inaka/jfacorro.23.no.records.in.specs

[Closes #23] No specs with records.

src/elvis_code.erl

@@ -715,6 +715,8 @@ to_map({typed_record_field, Field, Type}) ->
 
 %% Type
 
+to_map({type, Attrs, Subtype, Types}) when not is_list(Types) ->
+    to_map({type, Attrs, Subtype, [Types]});
 to_map({type, Attrs, Subtype, Types}) ->
     {Location, Text} =
         case Attrs of
@@ -743,11 +745,13 @@ to_map({type, Attrs, map_field_assoc, Name, Type}) ->
                  key => to_map(Name),
                  text => Text,
                  type => to_map(Type)}};
-to_map({remote_type, Attrs, ModuleName}) ->
-    #{type => record_field,
+to_map({remote_type, Attrs, [Module, Function, Args]}) ->
+    #{type => remote_type,
       attrs => #{location => get_location(Attrs),
-                 text => get_text(Attrs)},
-      content => to_map(ModuleName)};
+                 text => get_text(Attrs),
+                 module => to_map(Module),
+                 function => to_map(Function),
+                 args => to_map(Args)}};
 to_map({ann_type, Attrs, [Var, Type]}) ->
     #{type => record_field,
       attrs => #{location => get_location(Attrs),
@@ -766,6 +770,13 @@ to_map({attribute, Attrs, type, {Name, Type, Args}}) ->
                  name => Name,
                  args => to_map(Args),
                  type => to_map(Type)}};
+to_map({attribute, Attrs, spec, {{Name, Arity}, Types}}) ->
+    #{type => spec,
+      attrs => #{location => get_location(Attrs),
+                 text => get_text(Attrs),
+                 name => Name,
+                 arity => Arity},
+     content => to_map(Types)};
 to_map({attribute, Attrs, Type, Value}) ->
     #{type => Type,
       attrs => #{location => get_location(Attrs),

src/elvis_style.erl

@@ -13,7 +13,8 @@
          used_ignored_variable/3,
          no_behavior_info/3,
          module_naming_convention/3,
-         state_record_and_type/3
+         state_record_and_type/3,
+         no_spec_with_records/3
         ]).
 
 -define(LINE_LENGTH_MSG, "Line ~p is too long: ~p.").
@@ -69,6 +70,9 @@
         "This module implements an OTP behavior and has a 'state' record "
         "but is missing a 'state()' type.").
 
+-define(NO_SPEC_WITH_RECORDS,
+        "The spec in line ~p uses a record, please define a type for the "
+        "record and use that instead.").
 
 %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
 %% Rules
@@ -244,6 +248,19 @@ state_record_and_type(Config, Target, []) ->
             []
     end.
 
+-spec no_spec_with_records(elvis_config:config(),
+                          elvis_utils:file(),
+                          [list()]) ->
+    [elvis_result:item()].
+no_spec_with_records(Config, Target, []) ->
+    {Root, _} = elvis_utils:parse_tree(Config, Target),
+    case elvis_code:find(fun spec_includes_record/1, Root) of
+        [] -> [];
+        SpecNodes ->
+            ResultFun = result_node_line_fun(?NO_SPEC_WITH_RECORDS),
+            lists:map(ResultFun, SpecNodes)
+    end.
+
 %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
 %% Private
 %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
@@ -490,7 +507,14 @@ has_state_type(Root) ->
                 (type_attr == elvis_code:type(Node))
                     and (state == elvis_code:attr(name, Node))
         end,
-    case elvis_code:find(IsStateType, Root) of
-        [] -> false;
-        _ -> true
-    end.
+    elvis_code:find(IsStateType, Root) /= [].
+
+-spec spec_includes_record(elvis_code:tree_node()) -> boolean().
+spec_includes_record(Node) ->
+    IsTypeRecord = fun(Child) ->
+                           (elvis_code:type(Child) == type)
+                               and (elvis_code:attr(subtype, Child) == record)
+                   end,
+
+    (elvis_code:type(Node) == spec)
+        and (elvis_code:find(IsTypeRecord, Node) /= []).

test/examples/fail_no_spec_with_records.erl

@@ -0,0 +1,31 @@
+-module(fail_no_spec_with_records).
+
+-export([
+         function_1/1,
+         function_2/2,
+         function_3/2,
+         function_4/2,
+         function_5/1
+        ]).
+
+-record(state, {}).
+
+-spec function_1(atom()) -> atom().
+function_1(Arg) ->
+    Arg.
+
+-spec function_2(atom(), atom()) -> #state{}.
+function_2(_Arg1, _Arg2) ->
+    ok.
+
+-spec function_3(atom(), #state{}) -> atom().
+function_3(_Arg1, _Arg2) ->
+    ok.
+
+-spec function_4(atom(), integer()) -> atom().
+function_4(_Arg1, _Arg2) ->
+    ok.
+
+-spec function_5(#state{}) -> ok.
+function_5(_Arg1) ->
+    ok.

test/examples/pass_no_spec_with_records.erl

@@ -0,0 +1,21 @@
+-module(pass_no_spec_with_records).
+
+-export([
+         function_1/1,
+         function_2/2,
+         function_3/2
+        ]).
+
+-record(state, {}).
+
+-spec function_1(atom()) -> atom().
+function_1(Arg) ->
+    Arg.
+
+-spec function_2(atom(), atom()) -> #{}.
+function_2(_Arg1, _Arg2) ->
+    #state{}.
+
+-spec function_3(atom(), ok | error) -> atom().
+function_3(_Arg1, _Arg2) ->
+    ok.

test/style_SUITE.erl

@@ -19,7 +19,8 @@
          verify_used_ignored_variable/1,
          verify_no_behavior_info/1,
          verify_module_naming_convention/1,
-         verify_state_record_and_type/1
+         verify_state_record_and_type/1,
+         verify_no_spec_with_records/1
         ]).
 
 -define(EXCLUDED_FUNS,
@@ -230,3 +231,16 @@ verify_state_record_and_type(_Config) ->
     PathFail1 = "fail_state_type.erl",
     {ok, FileFail1} = elvis_test_utils:find_file(SrcDirs, PathFail1),
     [_] = elvis_style:state_record_and_type(ElvisConfig, FileFail1, []).
+
+-spec verify_no_spec_with_records(config()) -> any().
+verify_no_spec_with_records(_Config) ->
+    ElvisConfig = elvis_config:default(),
+    SrcDirs = elvis_config:dirs(ElvisConfig),
+
+    PathFail = "fail_no_spec_with_records.erl",
+    {ok, FileFail} = elvis_test_utils:find_file(SrcDirs, PathFail),
+    [_, _, _] = elvis_style:no_spec_with_records(ElvisConfig, FileFail, []),
+
+    PathPass = "pass_no_spec_with_records.erl",
+    {ok, FilePass} = elvis_test_utils:find_file(SrcDirs, PathPass),
+    [] = elvis_style:no_spec_with_records(ElvisConfig, FilePass, []).