diff --git a/RULES.md b/RULES.md index b759a0c3..ca4318bc 100644 --- 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 new file mode 100644 index 00000000..bd9a2b60 --- /dev/null +++ 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 index dd883370..1f2272df 100644 --- 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 new file mode 100644 index 00000000..b359dda0 --- /dev/null +++ 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 new file mode 100644 index 00000000..b5f32817 --- /dev/null +++ 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 new file mode 100644 index 00000000..32f773d2 --- /dev/null +++ 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 new file mode 100644 index 00000000..4c41c79a --- /dev/null +++ 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 index ad087bfe..8524412d 100644 --- 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"),