Fix #426: New rule: no_receive_without_timeout (#472)

Author: elbrujohalcon

RULES.md

@@ -57,6 +57,7 @@ Most, if not all, of the rules will present (opinionated) documentation sections
 - [No Nested `try...catch` Blocks](doc_rules/elvis_style/no_nested_try_catch.md)
 - [No Operator With Same Values](doc_rules/elvis_style/no_operation_on_same_value.md)
 - [No Redundant Blank Lines](doc_rules/elvis_text_style/no_redundant_blank_lines.md)
+- [No `receive` Without Timeout](doc_rules/elvis_style/no_receive_without_timeout.md)
 - [No Single-Clause Case Statements](doc_rules/elvis_style/no_single_clause_case.md)
 - [No Single-Match Maybe Statements](doc_rules/elvis_style/no_single_match_maybe.md)
 - [No Space After `#`](doc_rules/elvis_style/no_space_after_pound.md)

doc_rules/elvis_style/no_receive_without_timeout.md

@@ -0,0 +1,45 @@
+# No `receive` Without Timeout [![](https://img.shields.io/badge/since-4.1.0-blue)](https://github.com/inaka/elvis_core/releases/tag/4.1.0) ![](https://img.shields.io/badge/BEAM-yes-orange)
+
+All `receive` expressions should be accompanied by an `after` expressions.
+
+## Avoid
+
+```erlang
+receive
+    something ->
+        do:something()
+end
+```
+
+## Prefer
+
+```erlang
+receive
+    something ->
+        do:something()
+after
+    60_000 ->
+        exit(nothing_received)
+end
+```
+
+## Rationale
+
+A `receive` block without a timeout will wait indefinitely if no matching message arrives; by making
+your timeout explicit you:
+
+- avoid hanging processes.
+- improve testability (deterministic behaviour under test failure conditions).
+- ease debug and recovery.
+- can implement retry and self-healing.
+- potentially avoid denial-of-service scenarios where waits are exploited.
+
+## Options
+
+- None.
+
+## Example configuration
+
+```erlang
+{elvis_style, no_receive_without_timeout, #{}}
+```

src/elvis_ruleset.erl

@@ -134,6 +134,7 @@ elvis_style_rules() ->
         {elvis_style, no_match_in_condition},
         {elvis_style, no_nested_try_catch},
         {elvis_style, no_operation_on_same_value},
+        {elvis_style, no_receive_without_timeout},
         {elvis_style, no_single_clause_case},
         {elvis_style, no_single_match_maybe},
         {elvis_style, no_space_after_pound},

src/elvis_style.erl

@@ -55,7 +55,8 @@
     no_init_lists/3,
     ms_transform_included/3,
     no_boolean_in_comparison/3,
-    no_operation_on_same_value/3
+    no_operation_on_same_value/3,
+    no_receive_without_timeout/3
 ]).
 
 -export_type([empty_rule_config/0]).
@@ -272,6 +273,9 @@
     "Operation ~p on line ~p is has the same value on both sides."
     " Since the result is known, it is redundant."
 ).
+-define(NO_RECEIVE_WITHOUT_TIMEOUT,
+    "Receive block on line ~p doesn't have an after clause"
+).
 
 %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
 %% Default values
@@ -494,7 +498,8 @@ default(RuleWithEmptyDefault) when
     RuleWithEmptyDefault =:= export_used_types;
     RuleWithEmptyDefault =:= consistent_variable_casing;
     RuleWithEmptyDefault =:= ms_transform_included;
-    RuleWithEmptyDefault =:= no_boolean_in_comparison
+    RuleWithEmptyDefault =:= no_boolean_in_comparison;
+    RuleWithEmptyDefault =:= no_receive_without_timeout
 ->
     #{}.
 
@@ -1636,6 +1641,32 @@ no_boolean_in_comparison(Config, Target, RuleConfig) ->
 
     lists:map(ResultFun, ComparisonsWithBoolean).
 
+-spec no_receive_without_timeout(
+    elvis_config:config(),
+    elvis_file:file(),
+    empty_rule_config()
+) ->
+    [elvis_result:item()].
+no_receive_without_timeout(Config, Target, RuleConfig) ->
+    Root = get_root(Config, Target, RuleConfig),
+
+    Receives = elvis_code:find_by_types(['receive'], Root),
+
+    ReceivesWithoutTimeout = lists:filter(fun is_receive_without_timeout/1, Receives),
+
+    ResultFun =
+        fun(Node) ->
+            {Line, _} = ktn_code:attr(location, Node),
+            Info = [Line],
+            Msg = ?NO_RECEIVE_WITHOUT_TIMEOUT,
+            elvis_result:new(item, Msg, Info, Line)
+        end,
+
+    lists:map(ResultFun, ReceivesWithoutTimeout).
+
+is_receive_without_timeout(Receive) ->
+    [] == elvis_code:find_by_types([receive_after], Receive).
+
 -type no_operation_on_same_value_config() :: #{operations := [atom()]}.
 
 -spec no_operation_on_same_value(

test/examples/fail_no_receive_without_timeout.erl

@@ -0,0 +1,23 @@
+-module(fail_no_receive_without_timeout).
+
+-export([no_after/0, try_after/0, nested/0]).
+
+no_after() ->
+    receive X -> X end.
+
+try_after() ->
+    try
+        receive X -> X end
+    catch
+        timeout -> timeout
+    after
+        this:is_not(the, 'after', "that you are looking for")
+    end.
+
+nested() ->
+    receive
+        good ->
+            receive bad -> "This one doesn't have an after clause" end
+    after 1_000 ->
+        "This one is fine"
+    end.

test/examples/pass_nesting_level_elvis_attr.erl

@@ -134,6 +134,7 @@ exceed_with_receive() ->
                          false -> false
                      end;
                  4 -> ok
+             after 1000 -> ok
              end;
         3 -> 3
     end.
@@ -181,6 +182,7 @@ exceed_with_list_compr() ->
                        end
                        || X <- [1, 2, 3]];
                  4 -> ok
+             after 1000 -> ok
              end;
         3 -> 3
     end.
@@ -193,6 +195,7 @@ exceed_with_fun() ->
                  2 -> ok;
                  3 -> fun() -> ok end;
                  4 -> ok
+             after 1000 -> ok
              end;
         3 -> 3
     end.
@@ -205,6 +208,7 @@ dont_exceed_with_fun() ->
                  2 -> ok;
                  3 -> fun erlang:display/1;
                  4 -> ok
+             after 1000 -> ok
              end;
         3 -> 3
     end.

test/examples/pass_no_receive_without_timeout.erl

@@ -0,0 +1,35 @@
+-module(pass_no_receive_without_timeout).
+
+-define(TIMEOUT, 1_000).
+
+-export([literal/0, macro/0, variable/0, function_call/0, nested/0]).
+
+literal() ->
+    receive X -> X
+    after 1_000 -> ok
+    end.
+
+macro() ->
+    receive X -> X
+    after ?TIMEOUT -> ok
+    end.
+
+variable() ->
+    Timeout = 1_000,
+    receive X -> X
+    after Timeout -> ok
+    end.
+
+function_call() ->
+    receive X -> X
+    after default:timeout() -> ok
+    end.
+
+nested() ->
+    try
+        receive X -> X
+        after 10_000 -> throw(timeout)
+        end
+    catch
+        timeout -> timeout
+    end.

test/style_SUITE.erl

@@ -71,7 +71,8 @@
     verify_ms_transform_included/1,
     verify_redundant_blank_lines/1,
     verify_no_boolean_in_comparison/1,
-    verify_no_operation_on_same_value/1
+    verify_no_operation_on_same_value/1,
+    verify_no_receive_without_timeout/1
 ]).
 %% -elvis attribute
 -export([
@@ -1987,6 +1988,21 @@ verify_no_boolean_in_comparison(Config) ->
     ] =
         elvis_core_apply_rule(Config, elvis_style, no_boolean_in_comparison, #{}, FailPath).
 
+-spec verify_no_receive_without_timeout(config()) -> any().
+verify_no_receive_without_timeout(Config) ->
+    Ext = proplists:get_value(test_file_ext, Config, "erl"),
+
+    PassPath = "pass_no_receive_without_timeout." ++ Ext,
+    [] = elvis_core_apply_rule(Config, elvis_style, no_receive_without_timeout, #{}, PassPath),
+
+    FailPath = "fail_no_receive_without_timeout." ++ Ext,
+    [
+        #{line_num := 6},
+        #{line_num := 10},
+        #{line_num := 20}
+    ] =
+        elvis_core_apply_rule(Config, elvis_style, no_receive_without_timeout, #{}, FailPath).
+
 -spec verify_atom_naming_convention(config()) -> any().
 verify_atom_naming_convention(Config) ->
     Group = proplists:get_value(group, Config, erl_files),
@@ -2736,11 +2752,13 @@ verify_elvis_attr(Config, FilenameNoExt) ->
     SrcDirs = elvis_config:dirs(ElvisConfig),
     Ext = proplists:get_value(test_file_ext, Config, "erl"),
 
-    {ok, File} = elvis_test_utils:find_file(SrcDirs, FilenameNoExt ++ "." ++ Ext),
+    FullFilename = FilenameNoExt ++ "." ++ Ext,
+    {ok, File} = elvis_test_utils:find_file(SrcDirs, FullFilename),
 
+    ct:comment("Checking ~ts", [FullFilename]),
     {ok, #{rules := RuleResults}} = elvis_core:do_rock(File, ElvisConfig),
     [[] = Items || #{items := Items} <- RuleResults],
-    true.
+    {comment, ""}.
 
 -spec is_item_line_sort([elvis_result:file()]) -> [boolean()].
 is_item_line_sort(Result) ->