Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ets fun2ms without ms transform #374

Merged
merged 10 commits into from
Nov 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions RULES.md
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ identified with `(since ...)` for convenience purposes.
- [Variable Naming Convention](doc_rules/elvis_style/variable_naming_convention.md)
- [No Init Lists](doc_rules/elvis_style/no_init_lists.md)
- [Prefer Unquoted Atoms](doc_rules/elvis_text_style/prefer_unquoted_atoms.md)
- [ms_transform included](doc_rules/elvis_style/ms_transform_included.md)

## `.gitignore` rules

Expand Down
16 changes: 16 additions & 0 deletions doc_rules/elvis_style/ms_transform_included.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@

# Include `ms_transform` for `ets:fun2ms/1`

The library `ms_transform` needs to be included if the function`ets:fun2ms` is used in the module.

> Works on `.beam` files? Yes!

## Options

- None.

## Example

```erlang
{elvis_style, ms_transform_included, #{}}
```
2 changes: 2 additions & 0 deletions src/elvis_rulesets.erl
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ rules(erl_files_strict) ->
max_function_clause_length,
max_function_length,
max_module_length,
ms_transform_included,
no_call,
no_init_lists,
no_common_caveats_call,
Expand All @@ -113,6 +114,7 @@ rules(beam_files) ->
invalid_dynamic_call,
max_anonymous_function_arity,
max_function_arity,
ms_transform_included,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@elbrujohalcon, I wonder if, in the future, to ease maintenance, this list should be build like a sub-set of _strict, for example...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good idea, yeah.

module_naming_convention,
nesting_level,
no_author,
Expand Down
60 changes: 58 additions & 2 deletions src/elvis_style.erl
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
no_catch_expressions/3, no_single_clause_case/3, numeric_format/3, behaviour_spelling/3,
always_shortcircuit/3, consistent_generic_type/3, export_used_types/3,
no_match_in_condition/3, param_pattern_matching/3, private_data_types/3, option/3,
no_init_lists/3]).
no_init_lists/3, ms_transform_included/3]).

-export_type([empty_rule_config/0]).
-export_type([ignorable/0]).
Expand All @@ -33,6 +33,9 @@

-define(NO_INIT_LISTS_MSG,
"Do not use a list as the parameter for the 'init' callback at position ~p.").
-define(MS_TRANSFORM_INCLUDED_MSG,
"Missing inclide library: stdlib/include/ms_transform.hrl when ets:fun2ms/1
is used at position ~p.").
-define(INVALID_MACRO_NAME_REGEX_MSG,
"The macro named ~p on line ~p does not respect the format "
"defined by the regular expression '~p'.").
Expand Down Expand Up @@ -260,7 +263,8 @@ default(RuleWithEmptyDefault)
RuleWithEmptyDefault == always_shortcircuit;
RuleWithEmptyDefault == no_space_after_pound;
RuleWithEmptyDefault == export_used_types;
RuleWithEmptyDefault == consistent_variable_casing ->
RuleWithEmptyDefault == consistent_variable_casing;
RuleWithEmptyDefault == ms_transform_included ->
#{}.

%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
Expand Down Expand Up @@ -1192,6 +1196,58 @@ is_list_node(#{type := match, content := Content}) ->
is_list_node(_) ->
false.

-spec ms_transform_included(elvis_config:config(),
elvis_file:file(),
empty_rule_config()) ->
[elvis_result:item()].
ms_transform_included(Config, Target, RuleConfig) ->
Root = get_root(Config, Target, RuleConfig),

FunctionCalls = get_fun_2_ms_calls(Root),

IsIncluded = FunctionCalls /= [] andalso has_include_ms_transform(Root),

ResultFun =
fun(Location) ->
Info = [Location],
Msg = ?MS_TRANSFORM_INCLUDED_MSG,
elvis_result:new(item, Msg, Info, Location)
end,

case IsIncluded of
true ->
[];
false ->
lists:map(ResultFun, FunctionCalls)
end.

-spec get_fun_2_ms_calls(ktn_code:tree_node()) -> [any()].
get_fun_2_ms_calls(Root) ->
IsFun2MsFunctionCall =
fun(Node) -> ktn_code:type(Node) == call andalso is_ets_fun2ms(Node) end,

Functions = elvis_code:find(IsFun2MsFunctionCall, Root),
ProcessResult = fun(Node) -> ktn_code:attr(location, Node) end,

lists:map(ProcessResult, Functions).

-spec is_ets_fun2ms(ktn_code:tree_node()) -> boolean().
is_ets_fun2ms(Node) ->
Fun = ktn_code:node_attr(function, Node),
Fun2 = ktn_code:node_attr(function, Fun),
Module = ktn_code:node_attr(module, Fun),

ets == ktn_code:attr(value, Module) andalso fun2ms == ktn_code:attr(value, Fun2).

-spec has_include_ms_transform(ktn_code:tree_node()) -> boolean().
has_include_ms_transform(Root) ->
Fun = fun(Node) ->
ktn_code:type(Node) == include_lib
andalso ktn_code:attr(value, Node) == "stdlib/include/ms_transform.hrl"
end,

[] /= elvis_code:find(Fun, Root).

-spec no_throw(elvis_config:config(), elvis_file:file(), empty_rule_config()) ->
[elvis_result:item()].
no_throw(Config, Target, RuleConfig) ->
Expand Down
5 changes: 5 additions & 0 deletions test/examples/custom_ms_transform_included.erl
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
-module(custom_ms_transform_included).

-export([test/0]).

test() -> {this, is, "not", ets, my:fun2ms("It's my own")}.
8 changes: 8 additions & 0 deletions test/examples/double_include_ms_transform.erl
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
-module(double_include_ms_transform).

-include_lib("stdlib/include/ms_transform.hrl").
-include_lib("stdlib/include/ms_transform.hrl").

-export([test/0]).

test() -> ets:fun2ms(fun(_) -> ok end).
5 changes: 5 additions & 0 deletions test/examples/fail_ms_transform_included.erl
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
-module(fail_ms_transform_included).

-export([test/0]).

test() -> ets:fun2ms(fun(_) -> ok end).
7 changes: 7 additions & 0 deletions test/examples/included_but_not_used_ms_transform.erl
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
-module(included_but_not_used_ms_transform).

-include_lib("stdlib/include/ms_transform.hrl").

-export([test/0]).

test() -> ok.
7 changes: 7 additions & 0 deletions test/examples/pass_ms_transform_included.erl
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
-module(pass_ms_transform_included).

-include_lib("stdlib/include/ms_transform.hrl").

-export([test/0]).

test() -> ets:fun2ms(fun(_) -> ok end).
34 changes: 33 additions & 1 deletion test/style_SUITE.erl
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@
verify_consistent_generic_type/1, verify_no_types/1, verify_no_specs/1,
verify_export_used_types/1, verify_consistent_variable_casing/1,
verify_no_match_in_condition/1, verify_param_pattern_matching/1,
verify_private_data_types/1, verify_unquoted_atoms/1, verify_no_init_lists/1]).
verify_private_data_types/1, verify_unquoted_atoms/1, verify_no_init_lists/1,
verify_ms_transform_included/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,
Expand Down Expand Up @@ -1397,6 +1398,37 @@ verify_unquoted_atoms(Config) ->
[_, _] =
elvis_core_apply_rule(Config, elvis_text_style, prefer_unquoted_atoms, #{}, FailPath).

-spec verify_ms_transform_included(config()) -> any().
verify_ms_transform_included(Config) ->
Ext = proplists:get_value(test_file_ext, Config, "erl"),

PassPath = "pass_ms_transform_included." ++ Ext,
[] = elvis_core_apply_rule(Config, elvis_style, ms_transform_included, #{}, PassPath),

CustomFunctionPath = "custom_ms_transform_included." ++ Ext,
[] =
elvis_core_apply_rule(Config,
elvis_style,
ms_transform_included,
#{},
CustomFunctionPath),

IncludedButNotUsed = "included_but_not_used_ms_transform." ++ Ext,
[] =
elvis_core_apply_rule(Config,
elvis_style,
ms_transform_included,
#{},
IncludedButNotUsed),

DoubleInclude = "double_include_ms_transform." ++ Ext,
[] =
elvis_core_apply_rule(Config, elvis_style, ms_transform_included, #{}, DoubleInclude),

FailPath = "fail_ms_transform_included." ++ Ext,
[_] = elvis_core_apply_rule(Config, elvis_style, ms_transform_included, #{}, FailPath),
ok.

-spec verify_atom_naming_convention(config()) -> any().
verify_atom_naming_convention(Config) ->
Group = proplists:get_value(group, Config, erl_files),
Expand Down