Skip to content

Commit

Permalink
Fix #284: New Rule: Param Pattern Matching
Browse files Browse the repository at this point in the history
  • Loading branch information
elbrujohalcon committed Mar 2, 2023
1 parent b1cf9b8 commit 2763872
Show file tree
Hide file tree
Showing 9 changed files with 238 additions and 15 deletions.
1 change: 1 addition & 0 deletions RULES.md
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ identified with `(since ...)` for convenience purposes.
- [No Types](doc_rules/elvis_style/no_types.md)
- [Numeric Format](doc_rules/elvis_style/numeric_format.md)
- [Operator Spaces](doc_rules/elvis_style/operator_spaces.md)
- [Param Pattern Matching](doc_rules/elvis_style/param_pattern_matching.md)
- [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)
Expand Down
21 changes: 21 additions & 0 deletions doc_rules/elvis_style/param_pattern_matching.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
# Param Pattern-Matching

(since [3.0.0](https://github.com/inaka/elvis_core/releases/tag/3.0.0))

When capturing a parameter using pattern matching you can either put the parameter
name on the left (`Param = #{pattern := ToMatch}`) or right (`#{pattern := ToMatch} = Param`) side
of the pattern that you use in the function clause.
This rule will make sure you are consistent through your code and use always the same style.

> Works on `.beam` file? Yes!
## Options

- `side :: left | right`.
- default: `left`.

## Example

```erlang
{elvis_style, param_pattern_matching, #{side => right}}
```
6 changes: 4 additions & 2 deletions src/elvis_rulesets.erl
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,8 @@ rules(erl_files) ->
{elvis_style, behaviour_spelling},
{elvis_style, export_used_types},
{elvis_style, max_function_arity},
{elvis_style, max_anonymous_function_arity}]);
{elvis_style, max_anonymous_function_arity},
{elvis_style, param_pattern_matching}]);
rules(beam_files) ->
lists:map(fun(Rule) -> {elvis_style, Rule, elvis_style:default(Rule)} end,
[nesting_level,
Expand All @@ -107,7 +108,8 @@ rules(beam_files) ->
behaviour_spelling,
export_used_types,
max_function_arity,
max_anonymous_function_arity]);
max_anonymous_function_arity,
param_pattern_matching]);
rules(rebar_config) ->
lists:map(fun(Rule) -> {elvis_project, Rule, elvis_project:default(Rule)} end,
[no_branch_deps, protocol_for_deps]);
Expand Down
74 changes: 69 additions & 5 deletions src/elvis_style.erl
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
atom_naming_convention/3, no_throw/3, no_dollar_space/3, no_author/3, no_import/3,
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, option/3]).
no_match_in_condition/3, param_pattern_matching/3, option/3]).

-export_type([empty_rule_config/0]).
-export_type([ignorable/0]).
Expand All @@ -27,7 +27,8 @@
no_common_caveats_call_config/0, atom_naming_convention_config/0, no_author_config/0,
no_import_config/0, no_catch_expressions_config/0, numeric_format_config/0,
no_single_clause_case_config/0, consistent_variable_casing_config/0,
no_match_in_condition_config/0]).
no_match_in_condition_config/0, behaviour_spelling_config/0,
param_pattern_matching_config/0]).

-define(INVALID_MACRO_NAME_REGEX_MSG,
"The macro named ~p on line ~p does not respect the format "
Expand Down Expand Up @@ -121,9 +122,12 @@
-define(NUMERIC_FORMAT_MSG,
"Number ~p on line ~p does not respect the format "
"defined by the regular expression '~p'.").
-define(BEHAVIOUR_SPELLING,
-define(BEHAVIOUR_SPELLING_MSG,
"The behavior/behaviour in line ~p is misspelt, please use the "
"~p spelling.").
-define(PARAM_PATTERN_MATCHING_MSG,
"Variable ~ts, used to match a parameter in line ~p is placed on "
"the wrong side of the match. It was expected on the ~p side.").
-define(ALWAYS_SHORTCIRCUIT_MSG,
"Non-shortcircuiting operator (~p) found in line ~p. "
"It's recommended to use ~p, instead.").
Expand Down Expand Up @@ -196,6 +200,8 @@ default(numeric_format) ->
float_regex => same};
default(behaviour_spelling) ->
#{spelling => behaviour};
default(param_pattern_matching) ->
#{side => left};
default(consistent_generic_type) ->
#{preferred_type => term};
default(RuleWithEmptyDefault)
Expand Down Expand Up @@ -1125,7 +1131,12 @@ numeric_format(Config, Target, RuleConfig) ->
IntNodes,
check_numeric_format(FloatRegex, FloatNodes, [])).

-spec behaviour_spelling(elvis_config:config(), elvis_file:file(), empty_rule_config()) ->
-type behaviour_spelling_config() ::
#{ignore => [ignorable()], spelling => behaviour | behavior}.

-spec behaviour_spelling(elvis_config:config(),
elvis_file:file(),
behaviour_spelling_config()) ->
[elvis_result:item()].
behaviour_spelling(Config, Target, RuleConfig) ->
Spelling = option(spelling, RuleConfig, behaviour_spelling),
Expand All @@ -1143,11 +1154,64 @@ behaviour_spelling(Config, Target, RuleConfig) ->
fun(Node) ->
{Line, _} = ktn_code:attr(location, Node),
Info = [Line, Spelling],
elvis_result:new(item, ?BEHAVIOUR_SPELLING, Info, Line)
elvis_result:new(item, ?BEHAVIOUR_SPELLING_MSG, Info, Line)
end,
lists:map(ResultFun, InconsistentBehaviorNodes)
end.

-type param_pattern_matching_config() :: #{ignore => [ignorable()], side => left | right}.

-spec param_pattern_matching(elvis_config:config(),
elvis_file:file(),
param_pattern_matching_config()) ->
[elvis_result:item()].
param_pattern_matching(Config, Target, RuleConfig) ->
Side = option(side, RuleConfig, param_pattern_matching),
Root = get_root(Config, Target, RuleConfig),

FunctionClausePatterns =
lists:flatmap(fun(Clause) -> ktn_code:node_attr(pattern, Clause) end,
elvis_code:find(fun is_function_clause/1,
Root,
#{mode => zipper, traverse => all})),

MatchesInFunctionClauses =
lists:filter(fun(Pattern) -> ktn_code:type(Pattern) == match end, FunctionClausePatterns),

lists:filtermap(fun(Match) ->
case lists:map(fun ktn_code:type/1, ktn_code:content(Match)) of
[var, var] ->
false;
[var, _] when Side == right ->
{Line, _} = ktn_code:attr(location, Match),
[Var, _] = ktn_code:content(Match),
VarName = ktn_code:attr(name, Var),
Info = [VarName, Line, Side],
{true,
elvis_result:new(item, ?PARAM_PATTERN_MATCHING_MSG, Info, Line)};
[_, var] when Side == left ->
{Line, _} = ktn_code:attr(location, Match),
[_, Var] = ktn_code:content(Match),
VarName = ktn_code:attr(name, Var),
Info = [VarName, Line, Side],
{true,
elvis_result:new(item, ?PARAM_PATTERN_MATCHING_MSG, Info, Line)};
_ ->
false
end
end,
MatchesInFunctionClauses).

is_function_clause(Zipper) ->
clause
== ktn_code:type(
zipper:node(Zipper))
andalso lists:member(
ktn_code:type(
zipper:node(
zipper:up(Zipper))),
[function, 'fun']).

-spec consistent_generic_type(elvis_config:config(),
elvis_file:file(),
empty_rule_config()) ->
Expand Down
24 changes: 24 additions & 0 deletions test/examples/left_param_pattern_matching.erl
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
-module(left_param_pattern_matching).

-export([single/1, multiple/1, different_param/3, different_clause/1]).

single(Simple = {left, side, assignment}) ->
fun(SimpleToo = {left, side, assignment}) -> Simple == SimpleToo end.

multiple(Multiple = Assignments = {on, the, left, side}) ->
fun(MultipleToo = AssignmentsToo = {on, the, left, side}) ->
{Multiple, Assignments} == {MultipleToo, AssignmentsToo}
end.

different_param(happens_on, TheSecond = #{param := of_the}, function) ->
fun(happens_on, TheSecondToo = #{param := of_the}, function) -> TheSecond == TheSecondToo
end.

different_clause("it doesn't happen on the first clause") ->
not_here;
different_clause(But = "it does in the second one") ->
fun ("it doesn't happen on the first clause") ->
not_here;
(ButToo = "it does in the second one") ->
But == ButToo
end.
10 changes: 5 additions & 5 deletions test/examples/pass_operator_spaces_elvis_attr.erl
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ function7() ->
RegExp = "^.{1,20}$" ,
re:run(Name , RegExp , [unicode]).

tag_filters(DocName , #{conn := Conn} = State) ->
tag_filters(DocName , State = #{conn := Conn}) ->
TableName = atom_to_list(DocName) ,
Sql = ["SELECT "
" 'tag' AS \"type\", "
Expand All @@ -80,7 +80,7 @@ unicode_characters() ->
<<"ß"/utf8>> = <<"\\o337">> ,
ok.

windows_newlines() ->
<<_/bytes>> = <<"Foo" ,
"bar">> ,
ok.
windows_newlines() ->
<<_/bytes>> = <<"Foo" ,
"bar">> ,
ok.
19 changes: 19 additions & 0 deletions test/examples/pass_param_pattern_matching_elvis_attr.erl
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
-module(pass_param_pattern_matching_elvis_attr).

-elvis([{elvis_style, param_pattern_matching, #{side => right}}]).

-export([my_fun/1, my_fun/3]).

my_fun(#{[variable, goes] := [on, the]} = Right) ->
fun (#{[variable, goes] := [on, the]} = RightToo) ->
Right == RightToo;
({can, be, Left} = {_If, _It, "is not a single variable"}) ->
Left
end;
my_fun({can, be, Left} = {_If, _It, "is not a single variable"}) ->
Left.

my_fun(works, {as, well, on} = TheOther, parameters) ->
fun(works, {as, well, on} = TheOtherToo, parameters) ->
TheOther == TheOtherToo
end.
41 changes: 41 additions & 0 deletions test/examples/right_param_pattern_matching.erl
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
-module(right_param_pattern_matching).

-export([single/1, multiple/1, different_param/3, different_clause/1]).
-export([regular_matching_works_fine/0]).

single({right, side, assignment} = Simple) ->
fun({right, side, assignment} = SimpleToo) -> Simple == SimpleToo end.

%% This is not a match. According to Erlang precedence rules, the following code is actually
%% equivalent to: {on, the, right, side} = (Multiple = Assignments).
%% It's a tuple against a match, not a tuple against a variable against another variable
%% That's why no warnings are emitted for this line even if side is 'left'.
multiple({on, the, right, side} = Multiple = Assignments) ->
fun({on, the, right, side} = MultipleToo = AssignmentsToo) ->
{Multiple, Assignments} == {MultipleToo, AssignmentsToo}
end.

different_param(happens_on, #{the_second := param_of} = The, function) ->
fun(happens_on, #{the_second := param_of} = TheToo, function) -> The == TheToo
end.

different_clause("it doesn't happen on the first clause") ->
not_here;
different_clause("it does in the second one" = AsYoda) ->
fun ("it doesn't happen on the first clause") ->
not_here;
("it does in the second one" = AsYodaToo) ->
AsYoda == AsYodaToo
end.

regular_matching_works_fine() ->
This = works:fine(),
case This of
This = #{is := also} ->
valid;
_ ->
fun() ->
Like = This,
valid:as_well(Like)
end
end.
57 changes: 54 additions & 3 deletions test/style_SUITE.erl
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
verify_no_single_clause_case/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_export_used_types/1, verify_consistent_variable_casing/1,
verify_no_match_in_condition/1]).
verify_no_match_in_condition/1, verify_param_pattern_matching/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 All @@ -43,7 +43,7 @@
verify_elvis_attr_no_tabs/1, verify_elvis_attr_no_trailing_whitespace/1,
verify_elvis_attr_operator_spaces/1, verify_elvis_attr_state_record_and_type/1,
verify_elvis_attr_used_ignored_variable/1, verify_elvis_attr_variable_naming_convention/1,
verify_elvis_attr_behaviour_spelling/1]).
verify_elvis_attr_behaviour_spelling/1, verify_elvis_attr_param_pattern_matching/1]).
%% Non-rule
-export([results_are_ordered_by_line/1, oddities/1]).

Expand Down Expand Up @@ -82,7 +82,7 @@ groups() ->
verify_no_author, verify_no_import, verify_always_shortcircuit,
verify_no_catch_expressions, verify_no_single_clause_case, verify_no_macros,
verify_export_used_types, verify_max_anonymous_function_arity, verify_max_function_arity,
verify_no_match_in_condition]}].
verify_no_match_in_condition, verify_behaviour_spelling, verify_param_pattern_matching]}].

-spec init_per_suite(config()) -> config().
init_per_suite(Config) ->
Expand Down Expand Up @@ -756,6 +756,53 @@ verify_behaviour_spelling(Config) ->
#{spelling => behavior},
PathPass1).

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

PathRight = "right_param_pattern_matching." ++ Ext,
PathLeft = "left_param_pattern_matching." ++ Ext,

[#{info := ['Simple' | _]},
#{info := ['SimpleToo' | _]},
#{info := ['The' | _]},
#{info := ['TheToo' | _]},
#{info := ['AsYoda' | _]},
#{info := ['AsYodaToo' | _]}] =
elvis_core_apply_rule(Config,
elvis_style,
param_pattern_matching,
#{side => left},
PathRight),

[#{info := ['Simple' | _]},
#{info := ['SimpleToo' | _]},
#{info := ['Multiple' | _]},
#{info := ['MultipleToo' | _]},
#{info := ['TheSecond' | _]},
#{info := ['TheSecondToo' | _]},
#{info := ['But' | _]},
#{info := ['ButToo' | _]}] =
elvis_core_apply_rule(Config,
elvis_style,
param_pattern_matching,
#{side => right},
PathLeft),

[] =
elvis_core_apply_rule(Config,
elvis_style,
param_pattern_matching,
#{side => right},
PathRight),

[] =
elvis_core_apply_rule(Config,
elvis_style,
param_pattern_matching,
#{side => left},
PathLeft).

-spec verify_consistent_generic_type(config()) -> any().
verify_consistent_generic_type(Config) ->
Ext = proplists:get_value(test_file_ext, Config, "erl"),
Expand Down Expand Up @@ -1672,6 +1719,10 @@ verify_elvis_attr_variable_naming_convention(Config) ->
verify_elvis_attr_behaviour_spelling(Config) ->
verify_elvis_attr(Config, "pass_behaviour_spelling_elvis_attr").

-spec verify_elvis_attr_param_pattern_matching(config()) -> true.
verify_elvis_attr_param_pattern_matching(Config) ->
verify_elvis_attr(Config, "pass_param_pattern_matching_elvis_attr").

%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
%% Private
%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
Expand Down

0 comments on commit 2763872

Please sign in to comment.