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 2 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 @@

# No Init Lists
bormilan marked this conversation as resolved.
Show resolved Hide resolved

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
59 changes: 57 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,8 @@

-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 at position ~p.").
bormilan marked this conversation as resolved.
Show resolved Hide resolved
-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 +262,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 +1195,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),
IsIncludeMsTransform =
fun(Node) ->
ktn_code:type(Node) == include_lib
andalso ktn_code:attr(value, Node) == "stdlib/include/ms_transform.hrl"
end,

ResFunctions =
case elvis_code:find(IsIncludeMsTransform, Root) of
[] ->
get_fun_2_ms_calls(Root);
[_] ->
[]
end,

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

lists:map(ResultFun, ResFunctions).

get_fun_2_ms_calls(Root) ->
IsFun2MsFunction =
fun(Node) ->
case ktn_code:type(Node) == call of
true ->
{ets, fun2ms} == get_fun_and_mod_from_call(Node);
false ->
false
end
end,

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

lists:map(ProcessResult, Functions).

get_fun_and_mod_from_call(Node) ->
Fun = ktn_code:node_attr(function, Node),
Fun2 = ktn_code:node_attr(function, Fun),
Module = ktn_code:node_attr(module, Fun),

{ktn_code:attr(value, Module), ktn_code:attr(value, Fun2)}.

-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/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/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).
14 changes: 13 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,17 @@ 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),

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