New Rule: Consistent Generic Type (#259)

* New Rule: Types term or any * New Rule: Types term or any - additional changes post-feedback * Update src/elvis_style.erl Co-authored-by: Paulo F. Oliveira <[email protected]> * New Rule: Types term or any - additional changes post-feedback v2 * Update doc_rules/elvis_style/consistent_generic_type.md Co-authored-by: Paulo F. Oliveira <[email protected]>
inaka · Oct 27, 2022 · 0216160 · 0216160
1 parent 0ebed61
commit 0216160
Show file tree

Hide file tree

Showing 8 changed files with 221 additions and 2 deletions.
diff --git a/RULES.md b/RULES.md
@@ -46,6 +46,7 @@ identified with `(since ...)` for convenience purposes.
 - [Numeric Format](doc_rules/elvis_style/numeric_format.md)
 - [Operator Spaces](doc_rules/elvis_style/operator_spaces.md)
 - [State Record and Type](doc_rules/elvis_style/state_record_and_type.md)
+- [Consistent Generic Type](doc_rules/elvis_style/consistent_generic_type.md)
 - [Used Ignored Variable](doc_rules/elvis_style/used_ignored_variable.md)
 - [Variable Naming Convention](doc_rules/elvis_style/variable_naming_convention.md)
 

diff --git a/doc_rules/elvis_style/consistent_generic_type.md b/doc_rules/elvis_style/consistent_generic_type.md
@@ -0,0 +1,16 @@
+# Consistent Generic Type
+
+Use `term()` or `any()` consistently for types in specs.
+
+> Works on `.beam` file? Yes.
+
+## Options
+
+- `preferred_type :: term | any`.
+  - default: `term`.
+
+## Example
+
+```erlang
+{elvis_style, consistent_generic_type, #{ preferred_type => term }}
+```
diff --git a/src/elvis_style.erl b/src/elvis_style.erl
@@ -9,7 +9,7 @@
          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_catch_expressions/3, numeric_format/3, behaviour_spelling/3, always_shortcircuit/3,
-         option/3]).
+         consistent_generic_type/3, option/3]).
 
 -export_type([empty_rule_config/0]).
 -export_type([ignorable/0]).
@@ -99,6 +99,8 @@
 -define(ALWAYS_SHORTCIRCUIT_MSG,
         "Non-shortcircuiting operator (~p) found in line ~p. "
         "It's recommended to use ~p, instead.").
+-define(CONSISTENT_GENERIC_TYPE,
+        "Found usage of type ~p/0 on line ~p. Please use ~p/0, instead.").
 
 %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
 %% Default values
@@ -160,6 +162,8 @@ default(numeric_format) ->
       float_regex => same};
 default(behaviour_spelling) ->
     #{spelling => behaviour};
+default(consistent_generic_type) ->
+    #{preferred_type => term};
 default(RuleWithEmptyDefault)
     when RuleWithEmptyDefault == macro_module_names;
          RuleWithEmptyDefault == no_macros;
@@ -897,6 +901,22 @@ behaviour_spelling(Config, Target, RuleConfig) ->
             lists:map(ResultFun, InconsistentBehaviorNodes)
     end.
 
+-spec consistent_generic_type(elvis_config:config(),
+                              elvis_file:file(),
+                              empty_rule_config()) ->
+                                 [elvis_result:item()].
+consistent_generic_type(Config, Target, RuleConfig) ->
+    TypePreference = option(preferred_type, RuleConfig, consistent_generic_type),
+    Root = get_root(Config, Target, RuleConfig),
+    Predicate = consistent_generic_type_predicate(TypePreference),
+    case elvis_code:find(Predicate, Root, #{traverse => all, mode => node}) of
+        [] ->
+            [];
+        InconsistentTypeNodes ->
+            ResultFun = consistent_generic_type_result(TypePreference),
+            lists:map(ResultFun, InconsistentTypeNodes)
+    end.
+
 -spec always_shortcircuit(elvis_config:config(),
                           elvis_file:file(),
                           empty_rule_config()) ->
@@ -1524,6 +1544,24 @@ check_successive_maps(ResultFun, MapExp) ->
             end
     end.
 
+%% Consistent Generic Type
+consistent_generic_type_predicate(TypePreference) ->
+    fun(Node) ->
+       NodeType = ktn_code:type(Node),
+       NodeName = ktn_code:attr(name, Node),
+       lists:member(NodeType, [type, callback])
+       andalso lists:member(NodeName, [term, any])
+       andalso NodeName /= TypePreference
+    end.
+
+consistent_generic_type_result(TypePreference) ->
+    fun(Node) ->
+       {Line, _} = ktn_code:attr(location, Node),
+       NodeName = ktn_code:attr(name, Node),
+       Info = [NodeName, Line, TypePreference],
+       elvis_result:new(item, ?CONSISTENT_GENERIC_TYPE, Info, Line)
+    end.
+
 %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
 %% Internal Function Definitions
 %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%

diff --git a/test/examples/consistent_generic_type_any.erl b/test/examples/consistent_generic_type_any.erl
@@ -0,0 +1,24 @@
+-module(consistent_generic_type_any).
+
+-export([simple_any/1, simple_combined/1, simple_when/1]).
+
+% Type definitions when this is alone or combined
+-type my_type() :: any().
+-type combined() :: any() | my_type().
+
+% Record definitions
+-record(my_record, {c :: combined(), a :: any()}).
+
+% Callback definitions (with or without when)
+-callback my_callback(any()) -> any().
+-callback my_callback_when(X) -> X when X :: any().
+
+
+-spec simple_any(any()) -> ok.
+simple_any(_Args) -> ok.
+
+-spec simple_combined(combined()) -> ok.
+simple_combined(_Args) -> ok.
+
+-spec simple_when(#my_record{}) -> {combined(), X} when X :: any().
+simple_when(#my_record{c = C, a = A}) -> {C, A}.
diff --git a/test/examples/consistent_generic_type_no_checks.erl b/test/examples/consistent_generic_type_no_checks.erl
@@ -0,0 +1,27 @@
+-module(consistent_generic_type_no_checks).
+
+-export([any/0, term/0, my_function/1]).
+
+% A parametric type called any
+-type any(Thing) :: Thing.
+
+% A parametric type called term
+-type term(Thing) :: Thing.
+
+% Record definitions with attributes named term/any
+-record(my_record, {term :: term, any :: any}).
+
+
+% A function called any
+-spec any() -> any.
+any() -> any.
+
+% A function called term
+-spec term() -> term.
+term() -> term.
+
+
+% A function that calls the function called any
+-spec my_function(Thing :: any | term) -> any | term.
+my_function(#my_record{any = any}) -> any();
+my_function(#my_record{term = term}) -> term().
diff --git a/test/examples/consistent_generic_type_term.erl b/test/examples/consistent_generic_type_term.erl
@@ -0,0 +1,25 @@
+-module(consistent_generic_type_term).
+
+-export([simple_term/1, simple_combined/1, simple_when/1]).
+
+% Type definitions when this is alone or combined
+-type my_type() :: term().
+-type combined() :: term() | my_type().
+
+% Record definitions
+-record(my_record, {c :: combined(), t :: term()}).
+
+% Callback definitions (with or without when),
+-callback my_callback(term()) -> term().
+-callback my_callback_when(X) -> X when X :: term().
+
+
+-spec simple_term(term()) -> ok.
+simple_term(_Args) -> ok.
+
+-spec simple_combined(combined()) -> ok.
+simple_combined(_Args) -> ok.
+
+% Specs with when
+-spec simple_when(#my_record{}) -> {combined(), X} when X :: term().
+simple_when(#my_record{c = C, t = T}) -> {C, T}.
diff --git a/test/examples/consistent_generic_type_term_and_any.erl b/test/examples/consistent_generic_type_term_and_any.erl
@@ -0,0 +1,25 @@
+-module(consistent_generic_type_term_and_any).
+
+-export([simple_term/1, simple_combined/1, simple_when/1]).
+
+% Type definitions when this is alone or combined
+-type my_type() :: term().
+-type combined() :: any() | my_type().
+
+% Record definitions
+-record(my_record, {t :: term(), a :: any()}).
+
+% Callback definitions (with or without when)
+-callback my_callback(term()) -> any().
+-callback my_callback_when(X) -> X when X :: term().
+
+
+-spec simple_term(term()) -> ok.
+simple_term(_Args) -> ok.
+
+-spec simple_combined(combined()) -> ok.
+simple_combined(_Args) -> ok.
+
+% Specs with when
+-spec simple_when(#my_record{}) -> {any(), X} when X :: term().
+simple_when(#my_record{a = A, t = T}) -> {A, T}.
diff --git a/test/style_SUITE.erl b/test/style_SUITE.erl
@@ -23,7 +23,8 @@
          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_catch_expressions/1, verify_numeric_format/1,
-         verify_behaviour_spelling/1, verify_always_shortcircuit/1]).
+         verify_behaviour_spelling/1, verify_always_shortcircuit/1,
+         verify_consistent_generic_type/1]).
 %% -elvis attribute
 -export([verify_elvis_attr_atom_naming_convention/1, verify_elvis_attr_numeric_format/1,
          verify_elvis_attr_dont_repeat_yourself/1, verify_elvis_attr_function_naming_convention/1,
@@ -702,6 +703,68 @@ verify_behaviour_spelling(Config) ->
                               #{spelling => behavior},
                               PathPass1).
 
+-spec verify_consistent_generic_type(config()) -> any().
+verify_consistent_generic_type(Config) ->
+    Ext = proplists:get_value(test_file_ext, Config, "erl"),
+
+    PathFail = "consistent_generic_type_term." ++ Ext,
+    [_, _, _, _, _] =
+        elvis_core_apply_rule(Config,
+                              elvis_style,
+                              consistent_generic_type,
+                              #{preferred_type => any},
+                              PathFail),
+    PathFail1 = "consistent_generic_type_any." ++ Ext,
+    [_, _, _, _, _] =
+        elvis_core_apply_rule(Config,
+                              elvis_style,
+                              consistent_generic_type,
+                              #{preferred_type => term},
+                              PathFail1),
+    PathFail2 = "consistent_generic_type_term_and_any." ++ Ext,
+    [_, _, _, _] =
+        elvis_core_apply_rule(Config,
+                              elvis_style,
+                              consistent_generic_type,
+                              #{preferred_type => any},
+                              PathFail2),
+    PathFail3 = "consistent_generic_type_term_and_any." ++ Ext,
+    [_, _, _] =
+        elvis_core_apply_rule(Config,
+                              elvis_style,
+                              consistent_generic_type,
+                              #{preferred_type => term},
+                              PathFail3),
+
+    PathPass = "consistent_generic_type_term." ++ Ext,
+    [] =
+        elvis_core_apply_rule(Config,
+                              elvis_style,
+                              consistent_generic_type,
+                              #{preferred_type => term},
+                              PathPass),
+    PathPass1 = "consistent_generic_type_any." ++ Ext,
+    [] =
+        elvis_core_apply_rule(Config,
+                              elvis_style,
+                              consistent_generic_type,
+                              #{preferred_type => any},
+                              PathPass1),
+    PathPass2 = "consistent_generic_type_no_checks." ++ Ext,
+    [] =
+        elvis_core_apply_rule(Config,
+                              elvis_style,
+                              consistent_generic_type,
+                              #{preferred_type => term},
+                              PathPass2),
+    PathPass2 = "consistent_generic_type_no_checks." ++ Ext,
+    [] =
+        elvis_core_apply_rule(Config,
+                              elvis_style,
+                              consistent_generic_type,
+                              #{preferred_type => any},
+                              PathPass2).
+
 -spec verify_always_shortcircuit(config()) -> any().
 verify_always_shortcircuit(Config) ->
     Ext = proplists:get_value(test_file_ext, Config, "erl"),