Fix errors reported by Dialyzer

Author: dumbbell

src/rabbit_amqqueue.erl

@@ -453,6 +453,8 @@ not_found_or_absent(Name) ->
         [Q] -> {absent, Q, nodedown} %% Q exists on stopped node
     end.
 
+-spec not_found_or_absent_dirty(name()) -> not_found_or_absent().
+
 not_found_or_absent_dirty(Name) ->
     %% We should read from both tables inside a tx, to get a
     %% consistent view. But the chances of an inconsistency are small,
@@ -465,7 +467,7 @@ not_found_or_absent_dirty(Name) ->
 with(Name, F, E) ->
     with(Name, F, E, 2000).
 
-with(Name, F, E, RetriesLeft) ->
+with(#resource{} = Name, F, E, RetriesLeft) ->
     case lookup(Name) of
         {ok, Q = #amqqueue{state = live}} when RetriesLeft =:= 0 ->
             %% Something bad happened to that queue, we are bailing out

src/rabbit_mnesia.erl

@@ -163,20 +163,14 @@ init_from_config() ->
     {DiscoveredNodes, NodeType} =
         case rabbit_peer_discovery:discover_cluster_nodes() of
             {ok, {Nodes, Type} = Config}
-            when is_list(Nodes) andalso (Type == disc orelse Type == disk orelse Type == ram) ->
+              when is_list(Nodes) andalso
+                   (Type == disc orelse Type == disk orelse Type == ram) ->
                 case lists:foldr(FindBadNodeNames, [], Nodes) of
                     []       -> Config;
                     BadNames -> e({invalid_cluster_node_names, BadNames})
                 end;
             {ok, {_, BadType}} when BadType /= disc andalso BadType /= ram ->
                 e({invalid_cluster_node_type, BadType});
-            {ok, Nodes} when is_list(Nodes) ->
-                %% The legacy syntax (a nodes list without the node
-                %% type) is unsupported.
-                case lists:foldr(FindBadNodeNames, [], Nodes) of
-                    [] -> e(cluster_node_type_mandatory);
-                    _  -> e(invalid_cluster_nodes_conf)
-                end;
             {ok, _} ->
                 e(invalid_cluster_nodes_conf)
         end,
@@ -978,6 +972,8 @@ nodes_incl_me(Nodes) -> lists:usort([node()|Nodes]).
 
 nodes_excl_me(Nodes) -> Nodes -- [node()].
 
+-spec e(any()) -> no_return().
+
 e(Tag) -> throw({error, {Tag, error_description(Tag)}}).
 
 error_description({invalid_cluster_node_names, BadNames}) ->
@@ -987,9 +983,6 @@ error_description({invalid_cluster_node_type, BadType}) ->
     "In the 'cluster_nodes' configuration key, the node type is invalid "
         "(expected 'disc' or 'ram'): " ++
         lists:flatten(io_lib:format("~p", [BadType]));
-error_description(cluster_node_type_mandatory) ->
-    "The 'cluster_nodes' configuration key must indicate the node type: "
-        "either {[...], disc} or {[...], ram}";
 error_description(invalid_cluster_nodes_conf) ->
     "The 'cluster_nodes' configuration key is invalid, it must be of the "
         "form {[Nodes], Type}, where Nodes is a list of node names and "

src/rabbit_node_monitor.erl

@@ -104,10 +104,9 @@ cluster_status_filename() ->
 
 prepare_cluster_status_files() ->
     rabbit_mnesia:ensure_mnesia_dir(),
-    Corrupt = fun(F) -> throw({error, corrupt_cluster_status_files, F}) end,
     RunningNodes1 = case try_read_file(running_nodes_filename()) of
                         {ok, [Nodes]} when is_list(Nodes) -> Nodes;
-                        {ok, Other}                       -> Corrupt(Other);
+                        {ok, Other}                       -> corrupt_cluster_status_files(Other);
                         {error, enoent}                   -> []
                     end,
     ThisNode = [node()],
@@ -121,14 +120,19 @@ prepare_cluster_status_files() ->
             {ok, [AllNodes0]} when is_list(AllNodes0) ->
                 {legacy_cluster_nodes(AllNodes0), legacy_disc_nodes(AllNodes0)};
             {ok, Files} ->
-                Corrupt(Files);
+                corrupt_cluster_status_files(Files);
             {error, enoent} ->
                 LegacyNodes = legacy_cluster_nodes([]),
                 {LegacyNodes, LegacyNodes}
         end,
     AllNodes2 = lists:usort(AllNodes1 ++ RunningNodes2),
     ok = write_cluster_status({AllNodes2, DiscNodes, RunningNodes2}).
 
+-spec corrupt_cluster_status_files(any()) -> no_return().
+
+corrupt_cluster_status_files(F) ->
+    throw({error, corrupt_cluster_status_files, F}).
+
 write_cluster_status({All, Disc, Running}) ->
     ClusterStatusFN = cluster_status_filename(),
     Res = case rabbit_file:write_term_file(ClusterStatusFN, [{All, Disc}]) of

src/rabbit_peer_discovery.erl

@@ -105,9 +105,15 @@ maybe_init() ->
     end.
 
 
--spec discover_cluster_nodes() -> {ok, Nodes :: list()} |
-                                  {ok, {Nodes :: list(), NodeType :: rabbit_types:node_type()}} |
-                                  {error, Reason :: string()}.
+%% This module doesn't currently sanity-check the return value of
+%% `Backend:list_nodes()`. Therefore, it could return something invalid:
+%% thus the `{œk, any()} in the spec.
+%%
+%% `rabbit_mnesia:init_from_config()` does some verifications.
+
+-spec discover_cluster_nodes() ->
+    {ok, {Nodes :: [node()], NodeType :: rabbit_types:node_type()} | any()} |
+    {error, Reason :: string()}.
 
 discover_cluster_nodes() ->
     Backend = backend(),
@@ -156,10 +162,7 @@ maybe_inject_randomized_delay() ->
 -spec inject_randomized_delay() -> ok.
 
 inject_randomized_delay() ->
-    {Min, Max} = case randomized_delay_range_in_ms() of
-                     {A, B} -> {A, B};
-                     [A, B] -> {A, B}
-                 end,
+    {Min, Max} = randomized_delay_range_in_ms(),
     case {Min, Max} of
         %% When the max value is set to 0, consider the delay to be disabled.
         %% In addition, `rand:uniform/1` will fail with a "no function clause"
@@ -253,12 +256,15 @@ unlock(Data) ->
 %% Implementation
 %%
 
--spec normalize(Nodes :: list() |
-                {Nodes :: list(), NodeType :: rabbit_types:node_type()} |
-                {ok, Nodes :: list()} |
-                {ok, {Nodes :: list(), NodeType :: rabbit_types:node_type()}} |
-                {error, Reason :: string()}) -> {ok, {Nodes :: list(), NodeType :: rabbit_types:node_type()}} |
-                                                {error, Reason :: string()}.
+-spec normalize(Nodes :: [node()] |
+                {Nodes :: [node()],
+                 NodeType :: rabbit_types:node_type()} |
+                {ok, Nodes :: [node()]} |
+                {ok, {Nodes :: [node()],
+                      NodeType :: rabbit_types:node_type()}} |
+                {error, Reason :: string()}) ->
+    {ok, {Nodes :: [node()], NodeType :: rabbit_types:node_type()}} |
+    {error, Reason :: string()}.
 
 normalize(Nodes) when is_list(Nodes) ->
   {ok, {Nodes, disc}};
@@ -291,14 +297,12 @@ node_prefix() ->
 
 
 
--spec append_node_prefix(Value :: binary() | list()) -> atom().
+-spec append_node_prefix(Value :: binary() | string()) -> string().
 
-append_node_prefix(Value) ->
+append_node_prefix(Value) when is_binary(Value) orelse is_list(Value) ->
     Val = rabbit_data_coercion:to_list(Value),
     Hostname = case string:tokens(Val, ?NODENAME_PART_SEPARATOR) of
-                   [_ExistingPrefix, Val] ->
-                       Val;
-                   [Val]                  ->
-                       Val
+                   [_ExistingPrefix, HN] -> HN;
+                   [HN]                  -> HN
                end,
     string:join([node_prefix(), Hostname], ?NODENAME_PART_SEPARATOR).

src/rabbit_peer_discovery_classic_config.erl

@@ -26,13 +26,12 @@
 %% API
 %%
 
--spec list_nodes() -> {ok, Nodes :: list()} | {error, Reason :: string()}.
+-spec list_nodes() -> {ok, {Nodes :: [node()], rabbit_types:node_type()}}.
 
 list_nodes() ->
-    case application:get_env(rabbit, cluster_nodes) of
-        {_Nodes, _NodeType} = Pair -> Pair;
-        Nodes when is_list(Nodes)  -> {Nodes, disc};
-        undefined                  -> {[], disc}
+    case application:get_env(rabbit, cluster_nodes, {[], disc}) of
+        {_Nodes, _NodeType} = Pair -> {ok, Pair};
+        Nodes when is_list(Nodes)  -> {ok, {Nodes, disc}}
     end.
 
 -spec supports_registration() -> boolean().

src/rabbit_peer_discovery_dns.erl

@@ -28,24 +28,25 @@
 %% API
 %%
 
--spec list_nodes() -> {ok, Nodes :: list()} | {error, Reason :: string()}.
+-spec list_nodes() ->
+    {ok, {Nodes :: [node()], rabbit_types:node_type()}}.
 
 list_nodes() ->
     case application:get_env(rabbit, cluster_formation) of
       undefined         ->
-        {[], disc};
+            {ok, {[], disc}};
       {ok, ClusterFormation} ->
         case proplists:get_value(peer_discovery_dns, ClusterFormation) of
             undefined ->
               rabbit_log:warning("Peer discovery backend is set to ~s "
                                  "but final config does not contain rabbit.cluster_formation.peer_discovery_dns. "
                                  "Cannot discover any nodes because seed hostname is not configured!",
                                  [?MODULE]),
-              {[], disc};
+              {ok, {[], disc}};
             Proplist  ->
               Hostname = rabbit_data_coercion:to_list(proplists:get_value(hostname, Proplist)),
 
-              {discover_nodes(Hostname, net_kernel:longnames()), rabbit_peer_discovery:node_type()}
+              {ok, {discover_nodes(Hostname, net_kernel:longnames()), rabbit_peer_discovery:node_type()}}
         end
     end.
 

src/rabbit_recovery_terms.erl

@@ -115,7 +115,7 @@ upgrade_recovery_terms() ->
         [begin
              File = filename:join([QueuesDir, Dir, "clean.dot"]),
              case rabbit_file:read_term_file(File) of
-                 {ok, Terms} -> ok  = store(?MODULE, Dir, Terms);
+                 {ok, Terms} -> ok  = store_global_table(Dir, Terms);
                  {error, _}  -> ok
              end,
              file:delete(File)
@@ -132,7 +132,7 @@ dets_upgrade(Fun)->
     open_global_table(),
     try
         ok = dets:foldl(fun ({DirBaseName, Terms}, Acc) ->
-                                store(?MODULE, DirBaseName, Fun(Terms)),
+                                store_global_table(DirBaseName, Fun(Terms)),
                                 Acc
                         end, ok, ?MODULE),
         ok
@@ -158,8 +158,14 @@ close_global_table() ->
             ok
     end.
 
+store_global_table(DirBaseName, Terms) ->
+    dets:insert(?MODULE, {DirBaseName, Terms}).
+
 read_global(DirBaseName) ->
-    read(?MODULE, DirBaseName).
+    case dets:lookup(?MODULE, DirBaseName) of
+        [{_, Terms}] -> {ok, Terms};
+        _            -> {error, not_found}
+    end.
 
 delete_global_table() ->
     file:delete(filename:join(rabbit_mnesia:dir(), "recovery.dets")).

src/rabbit_table.erl

@@ -34,7 +34,7 @@
 -spec wait_for_replicated(retry()) -> 'ok'.
 -spec wait_for_replicated() -> 'ok'.
 -spec wait([atom()]) -> 'ok'.
--spec retry_timeout() -> {non_neg_integer() | infinity, non_neg_integer()}.
+-spec retry_timeout() -> non_neg_integer() | infinity.
 -spec force_load() -> 'ok'.
 -spec is_present() -> boolean().
 -spec is_empty() -> boolean().
@@ -117,8 +117,6 @@ wait(TableNames, Timeout, Retries) ->
             throw(Error);
         {_, {error, Error}} ->
             rabbit_log:warning("Error while waiting for Mnesia tables: ~p~n", [Error]),
-            wait(TableNames, Timeout, Retries - 1);
-        _ ->
             wait(TableNames, Timeout, Retries - 1)
     end.
 

src/rabbit_upgrade.erl

@@ -180,25 +180,34 @@ upgrade_mode(AllNodes) ->
             end;
         [Another|_] ->
             MyVersion = rabbit_version:desired_for_scope(mnesia),
-            ErrFun = fun (ClusterVersion) ->
-                             %% The other node(s) are running an
-                             %% unexpected version.
-                             die("Cluster upgrade needed but other nodes are "
-                                 "running ~p~nand I want ~p",
-                                 [ClusterVersion, MyVersion])
-                     end,
             case rpc:call(Another, rabbit_version, desired_for_scope,
                           [mnesia]) of
-                {badrpc, {'EXIT', {undef, _}}} -> ErrFun(unknown_old_version);
-                {badrpc, Reason}               -> ErrFun({unknown, Reason});
-                CV                             -> case rabbit_version:matches(
-                                                         MyVersion, CV) of
-                                                      true  -> secondary;
-                                                      false -> ErrFun(CV)
-                                                  end
+                {badrpc, {'EXIT', {undef, _}}} ->
+                    die_because_cluster_upgrade_needed(unknown_old_version,
+                                                       MyVersion);
+                {badrpc, Reason} ->
+                    die_because_cluster_upgrade_needed({unknown, Reason},
+                                                       MyVersion);
+                CV -> case rabbit_version:matches(
+                             MyVersion, CV) of
+                          true  -> secondary;
+                          false -> die_because_cluster_upgrade_needed(
+                                     CV, MyVersion)
+                      end
             end
     end.
 
+-spec die_because_cluster_upgrade_needed(any(), any()) -> no_return().
+
+die_because_cluster_upgrade_needed(ClusterVersion, MyVersion) ->
+    %% The other node(s) are running an
+    %% unexpected version.
+    die("Cluster upgrade needed but other nodes are "
+        "running ~p~nand I want ~p",
+        [ClusterVersion, MyVersion]).
+
+-spec die(string(), list()) -> no_return().
+
 die(Msg, Args) ->
     %% We don't throw or exit here since that gets thrown
     %% straight out into do_boot, generating an erl_crash.dump

src/rabbit_version.erl

@@ -45,8 +45,9 @@
 -spec check_version_consistency
         (string(), string(), string()) -> rabbit_types:ok_or_error(any()).
 -spec check_version_consistency
-        (string(), string(), string(), string()) ->
-                                          rabbit_types:ok_or_error(any()).
+        (string(), string(), string(),
+         fun((string(), string()) -> boolean())) ->
+    rabbit_types:ok_or_error(any()).
 -spec check_otp_consistency
         (string()) -> rabbit_types:ok_or_error(any()).
 

src/rabbit_vhost.erl

@@ -126,10 +126,6 @@ do_add(VHostPath, ActingUser) ->
             rabbit_event:notify(vhost_created, info(VHostPath)
                                 ++ [{user_who_performed_action, ActingUser}]),
             R;
-        {error, {no_such_vhost, VHostPath}} ->
-            Msg = rabbit_misc:format("failed to set up vhost '~s': it was concurrently deleted!",
-                                     [VHostPath]),
-            {error, Msg};
         {error, Reason} ->
             Msg = rabbit_misc:format("failed to set up vhost '~s': ~p",
                                      [VHostPath, Reason]),
@@ -218,10 +214,7 @@ assert_benign({error, not_found}, _) -> ok;
 assert_benign({error, {absent, Q, _}}, ActingUser) ->
     %% Removing the mnesia entries here is safe. If/when the down node
     %% restarts, it will clear out the on-disk storage of the queue.
-    case rabbit_amqqueue:internal_delete(Q#amqqueue.name, ActingUser) of
-        ok                 -> ok;
-        {error, not_found} -> ok
-    end.
+    rabbit_amqqueue:internal_delete(Q#amqqueue.name, ActingUser).
 
 internal_delete(VHostPath, ActingUser) ->
     [ok = rabbit_auth_backend_internal:clear_permissions(