Fix #290: New Rule: no_single_clause_case

RULES.md

@@ -42,6 +42,7 @@ identified with `(since ...)` for convenience purposes.
 - [No Import](doc_rules/elvis_style/no_import.md)
 - [No Macros](doc_rules/elvis_style/no_macros.md)
 - [No Nested try...catch Blocks](doc_rules/elvis_style/no_nested_try_catch.md)
+- [No Single-Clause Case Statements](doc_rules/elvis_style/no_single_clause_case.md)
 - [No Space after #](doc_rules/elvis_style/no_space_after_pount.md)
 - [No Space](doc_rules/elvis_style/no_space.md)
 - [No Spec With Records](doc_rules/elvis_style/no_spec_with_records.md)

doc_rules/elvis_style/no_single_clause_case.md

@@ -0,0 +1,30 @@
+# No Single-Clause Case Statements
+
+(since [3.0.0](https://github.com/inaka/elvis_core/releases/tag/3.0.0))
+
+Don't write code like this:
+
+```erlang
+case do:something() of
+    {ok, Result} -> do:something("else")
+end
+```
+
+Use pattern-matching instead:
+
+```erlang
+{ok, Result} = do:something(),
+do:something("else")
+```
+
+> Works on `.beam` file? Yes!
+
+## Options
+
+- None.
+
+## Example
+
+```erlang
+{elvis_style, no_single_clause_case, #{}}
+```

src/elvis_rulesets.erl

@@ -37,6 +37,7 @@ rules(hrl_files) ->
                {elvis_style, no_author},
                {elvis_style, no_import},
                {elvis_style, no_catch_expressions},
+               {elvis_style, no_single_clause_case},
                {elvis_style, numeric_format},
                {elvis_style, no_specs},
                {elvis_style, no_types}]);
@@ -72,6 +73,7 @@ rules(erl_files) ->
                {elvis_style, no_author},
                {elvis_style, no_import},
                {elvis_style, no_catch_expressions},
+               {elvis_style, no_single_clause_case},
                {elvis_style, numeric_format},
                {elvis_style, behaviour_spelling},
                {elvis_style, export_used_types},
@@ -98,6 +100,7 @@ rules(beam_files) ->
                no_author,
                no_import,
                no_catch_expressions,
+               no_single_clause_case,
                behaviour_spelling,
                export_used_types,
                max_function_arity,

src/elvis_style.erl

@@ -10,8 +10,8 @@
          max_function_arity/3, max_function_length/3, no_call/3, 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_import/3,
-         no_catch_expressions/3, numeric_format/3, behaviour_spelling/3, always_shortcircuit/3,
-         consistent_generic_type/3, export_used_types/3, option/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, option/3]).
 
 -export_type([empty_rule_config/0]).
 -export_type([ignorable/0]).
@@ -25,7 +25,7 @@
               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_import_config/0, no_catch_expressions_config/0, numeric_format_config/0,
-              consistent_variable_casing_config/0]).
+              no_single_clause_case_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 "
@@ -112,6 +112,8 @@
 -define(NO_IMPORT_MSG, "Usage of the import attribute, on line ~p, is discouraged").
 -define(NO_CATCH_EXPRESSIONS_MSG,
         "Usage of catch expression on line ~p is not recommended").
+-define(NO_SINGLE_CLAUSE_CASE_MSG,
+        "Case statement with a single clause found on line ~p.").
 -define(NUMERIC_FORMAT_MSG,
         "Number ~p on line ~p does not respect the format "
         "defined by the regular expression '~p'.").
@@ -211,6 +213,7 @@ default(RuleWithEmptyDefault)
          RuleWithEmptyDefault == no_author;
          RuleWithEmptyDefault == no_import;
          RuleWithEmptyDefault == no_catch_expressions;
+         RuleWithEmptyDefault == no_single_clause_case;
          RuleWithEmptyDefault == always_shortcircuit;
          RuleWithEmptyDefault == no_space_after_pound;
          RuleWithEmptyDefault == export_used_types;
@@ -1057,6 +1060,30 @@ no_catch_expressions(Config, Target, RuleConfig) ->
 is_catch_node(Node) ->
     ktn_code:type(Node) =:= 'catch'.
 
+-type no_single_clause_case_config() :: #{ignore => [ignorable()]}.
+
+-spec no_single_clause_case(elvis_config:config(),
+                            elvis_file:file(),
+                            no_single_clause_case_config()) ->
+                               [elvis_result:item()].
+no_single_clause_case(Config, Target, RuleConfig) ->
+    Root = get_root(Config, Target, RuleConfig),
+    Opts = #{mode => node, traverse => content},
+    CaseNodes = elvis_code:find(fun is_single_clause_case_statement/1, Root, Opts),
+    lists:map(fun(CaseNode) ->
+                 {Line, _Col} = ktn_code:attr(location, CaseNode),
+                 elvis_result:new(item, ?NO_SINGLE_CLAUSE_CASE_MSG, [Line], Line)
+              end,
+              CaseNodes).
+
+is_single_clause_case_statement(Node) ->
+    ktn_code:type(Node) == 'case'
+    andalso length([Clause
+                    || SubNode <- ktn_code:content(Node),
+                       ktn_code:type(SubNode) == case_clauses,
+                       Clause <- ktn_code:content(SubNode)])
+            == 1.
+
 -type numeric_format_config() ::
     #{ignore => [ignorable()],
       regex => string(),

test/examples/fail_no_single_clause_case.erl

@@ -0,0 +1,25 @@
+-module(fail_no_single_clause_case).
+
+-export([simple/0, nested/0]).
+
+simple() ->
+    case do:something() of
+        {just, one, clause} -> {is, bad}
+    end.
+
+nested() ->
+    case this:statement() of
+        has -> multiple;
+        clauses ->
+            case but:this(one, inside) of
+                has ->
+                    case just:one() of
+                        {this, one} ->
+                            case also:has(one) of
+                                but -> the;
+                                one -> "in the middle of everything";
+                                has -> many
+                            end
+                    end
+            end
+    end.

test/examples/pass_nesting_level_elvis_attr.erl

@@ -13,17 +13,24 @@ exceed_with_five_levels() ->
         1 -> case 2 of
                  2 -> case 3 of
                           3 -> case 4 of
-                                   4 -> four
-                               end
-                      end
-             end
+                                   4 -> four;
+                                   _ -> other
+                               end;
+                           _ -> other
+                      end;
+                 _ -> other
+             end;
+        _ -> other
     end,
     case 1 of
         1 -> case 2 of
                  2 -> case 3 of
-                          3 -> fourth
-                      end
-             end
+                          3 -> fourth;
+                          _ -> other
+                      end;
+                 _ -> other
+             end;
+        _ -> other
     end.
 
 exceed_at_diff_branches() ->

test/examples/pass_no_single_clause_case.erl

@@ -0,0 +1,24 @@
+-module(pass_no_single_clause_case).
+
+-export([simple/0, nested/0]).
+
+simple() ->
+    case do:something() of
+        more -> than;
+        one -> clause;
+        is -> good
+    end.
+
+nested() ->
+    case this:statement() of
+        has -> multiple;
+        clauses ->
+            case this:nested(one) of
+                has -> many;
+                clauses ->
+                    case as:well() andalso this:other() of
+                        has -> two;
+                        _Clauses -> too
+                    end
+            end
+    end.

test/examples/pass_used_ignored_variable_elvis_attr.erl

@@ -13,6 +13,8 @@
 use_ignored_var(_One, Two) ->
     Three = _One + Two,
     case Three of
+        3 ->
+            three;
         _Four ->
             _Four
     end.

test/style_SUITE.erl

@@ -24,9 +24,9 @@
          verify_no_call/1, 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_import/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_export_used_types/1, verify_consistent_variable_casing/1]).
+         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]).
 %% -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,
@@ -79,8 +79,9 @@ groups() ->
        verify_no_common_caveats_call, verify_no_call, verify_no_nested_try_catch,
        verify_no_successive_maps, verify_atom_naming_convention, verify_no_throw,
        verify_no_author, verify_no_import, verify_always_shortcircuit,
-       verify_no_catch_expressions, verify_no_macros, verify_export_used_types,
-       verify_max_anonymous_function_arity, verify_max_function_arity]}].
+       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]}].
 
 -spec init_per_suite(config()) -> config().
 init_per_suite(Config) ->
@@ -1394,6 +1395,24 @@ verify_no_catch_expressions(Config) ->
                 [#{info := [24]}, #{info := [22]}, #{info := [7]}] = R
         end.
 
+-spec verify_no_single_clause_case(config()) -> any().
+verify_no_single_clause_case(Config) ->
+    Group = proplists:get_value(group, Config, erl_files),
+    Ext = proplists:get_value(test_file_ext, Config, "erl"),
+
+    PassPath = "pass_no_single_clause_case." ++ Ext,
+    [] = elvis_core_apply_rule(Config, elvis_style, no_single_clause_case, #{}, PassPath),
+
+    FailPath = "fail_no_single_clause_case." ++ Ext,
+
+    R = elvis_core_apply_rule(Config, elvis_style, no_single_clause_case, #{}, FailPath),
+    _ = case Group of
+            beam_files ->
+                [_, _, _] = R;
+            erl_files ->
+                [#{line_num := 6}, #{line_num := 14}, #{line_num := 16}] = R
+        end.
+
 -spec verify_numeric_format(config()) -> any().
 verify_numeric_format(Config) ->
     Ext = proplists:get_value(test_file_ext, Config, "erl"),