Fix #265: New Rule: export used types

RULES.md

@@ -53,6 +53,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)
+- [Export Used Types](doc_rules/elvis_style/export_used_types.md)
 
 ## Project rules
 

doc_rules/elvis_style/export_used_types.md

@@ -0,0 +1,19 @@
+# Export Used Types
+
+(since [3.0.0](https://github.com/inaka/elvis_core/releases/tag/3.0.0))
+
+Exporting a function without exporting the types it depends on can result in
+reimplementing the types in every module that uses those functions. To avoid
+this, when a function is exported, its types should be too.
+
+> "Works on `.beam` file? Yes!"
+
+## Options
+
+- None.
+
+## Example
+
+```erlang
+{elvis_style, export_used_types}
+```

src/elvis_code.erl

@@ -3,8 +3,10 @@
 %% General
 -export([find/2, find/3, find_by_location/2, find_token/2, code_zipper/1, code_zipper/2]).
 %% Specific
--export([past_nesting_limit/2, exported_functions/1, function_names/1, module_name/1,
-         print_node/1, print_node/2]).
+-export([past_nesting_limit/2, exported_functions/1, exported_types/1, function_names/1,
+         module_name/1, print_node/1, print_node/2]).
+
+-export_type([find_options/0]).
 
 %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
 %%% Public API
@@ -193,6 +195,11 @@ exported_functions(#{type := root, content := Content}) ->
     Fun = make_extractor_fun(exported_functions),
     lists:flatmap(Fun, Content).
 
+-spec exported_types(ktn_code:tree_node()) -> [{atom(), integer()}].
+exported_types(#{type := root, content := Content}) ->
+    Fun = make_extractor_fun(exported_types),
+    lists:flatmap(Fun, Content).
+
 %% @doc Takes the root node of a parse_tree and returns the name
 %%      of each function, whether exported or not.
 -spec function_names(ktn_code:tree_node()) -> [atom()].
@@ -228,6 +235,12 @@ make_extractor_fun(exported_functions) ->
         (_) ->
             []
     end;
+make_extractor_fun(exported_types) ->
+    fun (Node = #{type := export_type}) ->
+            ktn_code:attr(value, Node);
+        (_) ->
+            []
+    end;
 make_extractor_fun(function_names) ->
     fun (Node = #{type := function}) ->
             [ktn_code:attr(name, Node)];

src/elvis_core.erl

@@ -9,6 +9,8 @@
 %% for eating our own dogfood
 -export([main/1]).
 
+-export_type([target/0]).
+
 -ifdef(TEST).
 
 -export([apply_rule/2]).

src/elvis_project.erl

@@ -2,6 +2,8 @@
 
 -export([default/1, no_branch_deps/3, protocol_for_deps/3, old_configuration_format/3]).
 
+-export_type([protocol_for_deps_config/0, empty_rule_config/0]).
+
 -define(DEP_BRANCH,
         "Dependency '~s' uses a branch. "
         "Please change this to a tag or specific commit.").

src/elvis_result.erl

@@ -8,7 +8,7 @@
          get_line_num/1]).
 
 %% Types
--export_type([item/0, rule/0, file/0]).
+-export_type([item/0, rule/0, file/0, elvis_error/0, elvis_warn/0]).
 
 %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
 %% Records

src/elvis_rulesets.erl

@@ -71,7 +71,8 @@ rules(erl_files) ->
                {elvis_style, no_author},
                {elvis_style, no_catch_expressions},
                {elvis_style, numeric_format},
-               {elvis_style, behaviour_spelling}]);
+               {elvis_style, behaviour_spelling},
+               {elvis_style, export_used_types}]);
 rules(beam_files) ->
     lists:map(fun(Rule) -> {elvis_style, Rule, elvis_style:default(Rule)} end,
               [nesting_level,
@@ -92,7 +93,8 @@ rules(beam_files) ->
                no_throw,
                no_author,
                no_catch_expressions,
-               behaviour_spelling]);
+               behaviour_spelling,
+               export_used_types]);
 rules(rebar_config) ->
     lists:map(fun(Rule) -> {elvis_project, Rule, elvis_project:default(Rule)} end,
               [no_branch_deps, protocol_for_deps]);

src/elvis_style.erl

@@ -10,10 +10,20 @@
          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,
-         consistent_generic_type/3, option/3]).
+         consistent_generic_type/3, export_used_types/3, option/3]).
 
 -export_type([empty_rule_config/0]).
 -export_type([ignorable/0]).
+-export_type([max_function_length_config/0, max_module_length_config/0,
+              function_naming_convention_config/0, variable_naming_convention_config/0,
+              macro_names_config/0, no_macros_config/0, no_types_config/0, no_specs_config/0,
+              no_block_expressions_config/0, no_space_after_pound_config/0,
+              operator_spaces_config/0, no_space_config/0, nesting_level_config/0,
+              god_modules_config/0, module_naming_convention_config/0,
+              dont_repeat_yourself_config/0, no_call_config/0, no_debug_call_config/0,
+              no_common_caveats_call_config/0, atom_naming_convention_config/0, no_author_config/0,
+              no_catch_expressions_config/0, numeric_format_config/0,
+              consistent_variable_casing_config/0]).
 
 -define(INVALID_MACRO_NAME_REGEX_MSG,
         "The macro named ~p on line ~p does not respect the format "
@@ -106,6 +116,8 @@
         "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.").
+-define(EXPORT_USED_TYPES_MSG,
+        "Type ~p/~p, defined on line ~p, is used by an exported function but not exported itself").
 
 %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
 %% Default values
@@ -189,6 +201,7 @@ default(RuleWithEmptyDefault)
          RuleWithEmptyDefault == no_catch_expressions;
          RuleWithEmptyDefault == always_shortcircuit;
          RuleWithEmptyDefault == no_space_after_pound;
+         RuleWithEmptyDefault == export_used_types;
          RuleWithEmptyDefault == consistent_variable_casing ->
     #{}.
 
@@ -1051,6 +1064,53 @@ always_shortcircuit(Config, Target, RuleConfig) ->
             lists:map(ResultFun, BadOperators)
     end.
 
+-spec export_used_types(elvis_config:config(), elvis_file:file(), empty_rule_config()) ->
+                           [elvis_result:item()].
+export_used_types(Config, Target, RuleConfig) ->
+    TreeRootNode = get_root(Config, Target, RuleConfig),
+    ExportedFunctions = elvis_code:exported_functions(TreeRootNode),
+    AllTypes =
+        elvis_code:find(fun is_type_attribute/1, TreeRootNode, #{traverse => all, mode => node}),
+    ExportedTypes = elvis_code:exported_types(TreeRootNode),
+    SpecNodes =
+        elvis_code:find(fun is_spec_attribute/1, TreeRootNode, #{traverse => all, mode => node}),
+    ExportedSpecs =
+        lists:filter(fun(#{attrs := #{arity := Arity, name := Name}}) ->
+                        lists:member({Name, Arity}, ExportedFunctions)
+                     end,
+                     SpecNodes),
+    UsedTypes =
+        lists:usort(
+            lists:flatmap(fun(Spec) ->
+                             Types =
+                                 elvis_code:find(fun(Node) -> ktn_code:type(Node) =:= user_type end,
+                                                 Spec,
+                                                 #{mode => node, traverse => all}),
+                             [{Name, length(Vars)}
+                              || #{attrs := #{name := Name}, content := Vars} <- Types]
+                          end,
+                          ExportedSpecs)),
+    UnexportedUsedTypes = lists:subtract(UsedTypes, ExportedTypes),
+
+    % Get line numbers for all types
+    LineNumbers =
+        lists:foldl(fun (#{attrs := #{location := {Line, _}, name := Name},
+                           node_attrs := #{args := Args}},
+                         Acc) ->
+                            maps:put({Name, length(Args)}, Line, Acc);
+                        (_, Acc) ->
+                            Acc
+                    end,
+                    #{},
+                    AllTypes),
+
+    % Report
+    lists:map(fun({Name, Arity} = Info) ->
+                 Line = maps:get(Info, LineNumbers, unknown),
+                 elvis_result:new(item, ?EXPORT_USED_TYPES_MSG, [Name, Arity, Line], Line)
+              end,
+              UnexportedUsedTypes).
+
 %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
 %% Private
 %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%

src/elvis_text_style.erl

@@ -2,6 +2,8 @@
 
 -export([default/1, line_length/3, no_tabs/3, no_trailing_whitespace/3]).
 
+-export_type([line_length_config/0, no_trailing_whitespace_config/0]).
+
 -define(LINE_LENGTH_MSG, "Line ~p is too long. It has ~p characters.").
 -define(NO_TABS_MSG, "Line ~p has a tab at column ~p.").
 -define(NO_TRAILING_WHITESPACE_MSG, "Line ~b has ~b trailing whitespace characters.").

src/elvis_utils.erl

@@ -10,7 +10,7 @@
 -export([info/1, info/2, notice/1, notice/2, error/1, error/2, error_prn/1, error_prn/2,
          warn_prn/2, parse_colors/1]).
 
--export_type([file/0]).
+-export_type([file/0, line_content/0]).
 
 -type file() :: #{path => string(), content => binary()}.
 -type line_content() :: {integer(), integer()}.

test/examples/fail_export_used_types.erl

@@ -0,0 +1,9 @@
+-module fail_export_used_types.
+
+-type my_type() :: my | type.
+
+-export [my_fun/1].
+
+-spec my_fun(my_type()) -> my_type().
+my_fun(my) -> type;
+my_fun(type) -> my.

test/examples/pass_export_used_types.erl

@@ -0,0 +1,17 @@
+-module pass_export_used_types.
+
+-type my_type() :: my | type.
+-type private_type(X) :: {X}.
+-export_type [my_type/0].
+
+-export [my_fun/1].
+
+-spec my_fun(my_type()) -> my_type().
+my_fun(my) -> type;
+my_fun(type) ->
+    private_fun(none),
+    my.
+
+% ignore types only used by private functions
+-spec private_fun(X) -> private_type(X).
+private_fun(none) -> {none}.

test/style_SUITE.erl

@@ -25,7 +25,7 @@
          verify_no_author/1, verify_no_catch_expressions/1, verify_numeric_format/1,
          verify_behaviour_spelling/1, verify_always_shortcircuit/1,
          verify_consistent_generic_type/1, verify_no_types/1, verify_no_specs/1,
-         verify_consistent_variable_casing/1]).
+         verify_export_used_types/1, verify_consistent_variable_casing/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,
@@ -92,7 +92,8 @@ groups() ->
        verify_no_author,
        verify_always_shortcircuit,
        verify_no_catch_expressions,
-       verify_no_macros]}].
+       verify_no_macros,
+       verify_export_used_types]}].
 
 -spec init_per_suite(config()) -> config().
 init_per_suite(Config) ->
@@ -1411,6 +1412,16 @@ verify_numeric_format(Config) ->
 
     true.
 
+-spec verify_export_used_types(config()) -> any().
+verify_export_used_types(Config) ->
+    Ext = proplists:get_value(test_file_ext, Config, "erl"),
+    PathPass = "pass_export_used_types." ++ Ext,
+    [] = elvis_core_apply_rule(Config, elvis_style, export_used_types, #{}, PathPass),
+
+    PathFail = "fail_export_used_types." ++ Ext,
+    [#{line_num := 3}] =
+        elvis_core_apply_rule(Config, elvis_style, export_used_types, #{}, PathFail).
+
 -spec results_are_ordered_by_line(config()) -> true.
 results_are_ordered_by_line(_Config) ->
     ElvisConfig = elvis_test_utils:config(),