Merge pull request #284 from ElFantasma/master

[#163] No nesting try…catch'es

.gitignore

@@ -10,4 +10,4 @@ log*/
 .erlang.mk.packages.*
 
 # Ignore elvis escript
-elvis
+elvis

elvis.config

@@ -28,7 +28,8 @@
                   },
                   {elvis_style, state_record_and_type},
                   {elvis_style, no_spec_with_records},
-                  {elvis_style, dont_repeat_yourself, #{min_complexity => 20}}
+                  {elvis_style, dont_repeat_yourself, #{min_complexity => 20}},
+                  {elvis_style, no_nested_try_catch}
                  ]
        },
       #{dirs => ["."],

rebar.config

@@ -24,12 +24,12 @@
   {lager,   "2.*", {git, "https://github.com/basho/lager.git",          "2.0.0"}},
   {getopt,  "0.*", {git, "https://github.com/jcomellas/getopt.git",     "v0.8.2"}},
   {meck,    "0.*", {git, "https://github.com/eproxus/meck.git",         "0.8.2"}},
-  {jiffy,   "0.*", {git, "https://github.com/davisp/jiffy.git",         "0.14.2"}},
+  {jiffy,   "0.*", {git, "https://github.com/davisp/jiffy.git",         "0.14.3"}},
   {ibrowse, "4.*", {git, "https://github.com/cmullaparthi/ibrowse.git", "v4.1.2"}},
   {aleppo,  "0.*", {git, "https://github.com/inaka/aleppo.git",         "0.9.0"}},
-  {zipper,  ".*",  {git, "https://github.com/inaka/zipper.git",         "0.1.0"}},
+  {zipper,  ".*",  {git, "https://github.com/inaka/zipper.git",         "0.1.2"}},
   {egithub, ".*",  {git, "https://github.com/inaka/erlang-github.git",  "0.1.1"}},
-  {katana,  ".*",  {git, "https://github.com/inaka/erlang-katana.git",  "0.2.0"}}
+  {katana,  ".*",  {git, "https://github.com/inaka/erlang-katana.git",  "0.2.13"}}
  ]
 }.
 {escript_name, "elvis"}.

src/elvis_style.erl

@@ -20,7 +20,8 @@
          no_spec_with_records/3,
          dont_repeat_yourself/3,
          max_module_length/3,
-         max_function_length/3
+         max_function_length/3,
+         no_nested_try_catch/3
         ]).
 
 -define(LINE_LENGTH_MSG, "Line ~p is too long: ~s.").
@@ -101,6 +102,9 @@
         "The code for function ~p has ~p lines which exceeds the "
         "maximum of ~p.").
 
+-define(NO_NESTED_TRY_CATCH,
+        "Nested try...catch block starting at line ~p.").
+
 %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
 %% Rules
 %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
@@ -512,6 +516,23 @@ node_line_limits(FunctionNode) ->
     Min = lists:min(LineNums),
     {Min, Max}.
 
+-spec no_nested_try_catch(elvis_config:config(),
+                          elvis_file:file(),
+                          empty_rule_config()) ->
+    [elvis_result:item()].
+no_nested_try_catch(Config, Target, _RuleConfig) ->
+    {Root, _} = elvis_file:parse_tree(Config, Target),
+    Predicate = fun(Node) -> ktn_code:type(Node) == 'try' end,
+    ResultFun = result_node_line_fun(?NO_NESTED_TRY_CATCH),
+    case elvis_code:find(Predicate, Root) of
+        [] ->
+            [];
+        TryExprs ->
+            lists:flatmap(fun (TryExp) ->
+                                   check_nested_try_catchs(ResultFun, TryExp)
+                          end,
+                          TryExprs)
+    end.
 %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
 %% Private
 %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
@@ -982,3 +1003,14 @@ filter_repeated(NodesLocs) ->
 is_children(Parent, Node) ->
     Zipper = elvis_code:code_zipper(Parent),
     [] =/= zipper:filter(fun(Child) -> Child == Node end, Zipper).
+
+%% No nested try...catch blocks
+
+check_nested_try_catchs(ResultFun, TryExp) ->
+    Predicate = fun(Node) -> ktn_code:type(Node) == 'try' end,
+    lists:filtermap(fun (Node) when Node /= TryExp ->
+                             {true, ResultFun(Node)};
+                        (_) ->
+                             false
+                    end,
+                    elvis_code:find(Predicate, TryExp)).

test/examples/fail_no_nested_try_catch.erl

@@ -0,0 +1,67 @@
+-module(fail_no_nested_try_catch).
+
+-export([
+         bad1/0,
+         bad2/0,
+         good1/0,
+         good2/0
+        ]).
+
+bad1() ->
+  try
+    maybe:throw(exception1),
+    try
+      maybe:throw(exception2),
+      "We are safe!"
+    catch
+      _:exception2 ->
+        "Oh, no! Exception #2"
+    end
+  catch
+    _:exception1 ->
+      "Bummer! Exception #1"
+  end.
+
+bad2() ->
+  try
+    maybe:throw(exception1),
+    try
+      maybe:throw(exception2),
+      "We are safe!"
+    catch
+      _:exception2 ->
+        "Oh, no! Exception #2"
+    end,
+    try
+      maybe:throw(exception3),
+      "We are safe!"
+    catch
+      _:exception3 ->
+        "Oh, no! Exception #3"
+    end
+  catch
+    _:exception1 ->
+      "Bummer! Exception #1"
+  end.
+
+good1() ->
+  try
+    maybe:throw(exception1),
+    maybe:throw(exception2),
+    "We are safe!"
+  catch
+    _:exception1 ->
+      "Bummer! Exception #1";
+    _:exception2 ->
+      "Oh, no! Exception #2"
+  end.
+
+good2() ->
+  try
+    maybe:throw(exception1),
+    a_function:that_deals(with, exception2),
+    "We are safe!"
+  catch
+    _:exception1 ->
+      "Bummer! Exception #1"
+  end.

test/style_SUITE.erl

@@ -27,6 +27,7 @@
          verify_dont_repeat_yourself/1,
          verify_max_module_length/1,
          verify_max_function_length/1,
+         verify_no_nested_try_catch/1,
          %% Non-rule
          results_are_ordered_by_line/1
         ]).
@@ -454,6 +455,19 @@ verify_max_function_length(_Config) ->
     RuleConfig10 = NoCountRuleConfig#{max_length => 2},
     [] = elvis_style:max_function_length(ElvisConfig, FileFail, RuleConfig10).
 
+-spec verify_no_nested_try_catch(config()) -> any().
+verify_no_nested_try_catch(_Config) ->
+    ElvisConfig = elvis_config:default(),
+    SrcDirs = elvis_config:dirs(ElvisConfig),
+
+    Path = "fail_no_nested_try_catch.erl",
+    {ok, File} = elvis_test_utils:find_file(SrcDirs, Path),
+    [
+     #{line_num := 13},
+     #{line_num := 28},
+     #{line_num := 35}
+    ] = elvis_style:no_nested_try_catch(ElvisConfig, File, #{}).
+
 -spec results_are_ordered_by_line(config()) -> any().
 results_are_ordered_by_line(_Config) ->
     ElvisConfig = elvis_config:default(),