Merge pull request #6913 from rabbitmq/dialyzer-warnings-rabbitmq_amqp1_0

Fix all dialyzer warnings in rabbitmq_amqp1_0

9 files changed, 38 insertions, 47 deletions

diff --git a/deps/rabbit/src/rabbit_reader.erl b/deps/rabbit/src/rabbit_reader.erl
index 767e4ecad7..a7a163b042 100644
--- a/deps/rabbit/src/rabbit_reader.erl
+++ b/deps/rabbit/src/rabbit_reader.erl
@@ -216,7 +216,7 @@ conserve_resources(Pid, Source, {_, Conserve, _}) ->
     Pid ! {conserve_resources, Source, Conserve},
     ok.
 
--spec server_properties(rabbit_types:protocol()) ->
+-spec server_properties(rabbit_types:protocol() | 'amqp_1_0') ->
           rabbit_framing:amqp_table().
 
 server_properties(Protocol) ->
diff --git a/deps/rabbitmq_amqp1_0/BUILD.bazel b/deps/rabbitmq_amqp1_0/BUILD.bazel
index 3ddac7482b..4a766e45a5 100644
--- a/deps/rabbitmq_amqp1_0/BUILD.bazel
+++ b/deps/rabbitmq_amqp1_0/BUILD.bazel
@@ -9,6 +9,7 @@ load(
     "rabbitmq_app",
     "rabbitmq_integration_suite",
     "rabbitmq_suite",
+    "without",
 )
 
 APP_ENV = """[
@@ -53,14 +54,17 @@ plt(
     name = "base_plt",
     # this ought to include BUILD_DEPS, but not sure how to
     # get dialyze of elixir code working yet
-    deps = DEPS,
+    deps = without(
+        "//deps/rabbitmq_cli:rabbitmqctl",
+        BUILD_DEPS + DEPS + RUNTIME_DEPS,
+    ),
+    apps = ["erts", "kernel", "stdlib", "ssl"]
 )
 
 dialyze(
     size = "medium",
     dialyzer_opts = RABBITMQ_DIALYZER_OPTS + ["-Wno_undefined_callbacks"],
     plt = ":base_plt",
-    warnings_as_errors = False,
 )
 
 broker_for_integration_suites()
diff --git a/deps/rabbitmq_amqp1_0/src/rabbit_amqp1_0_message.erl b/deps/rabbitmq_amqp1_0/src/rabbit_amqp1_0_message.erl
index 454f38cf25..97b893c17f 100644
--- a/deps/rabbitmq_amqp1_0/src/rabbit_amqp1_0_message.erl
+++ b/deps/rabbitmq_amqp1_0/src/rabbit_amqp1_0_message.erl
@@ -33,13 +33,15 @@ assemble(header, {R, P, C}, {H = #'v1_0.header'{}, Rest}, _Uneaten) ->
 assemble(header, {R, P, C}, Else, Uneaten) ->
     assemble(message_annotations, {R, P, C}, Else, Uneaten);
 
-assemble(delivery_annotations, RPC, {#'v1_0.delivery_annotations'{}, Rest},
-         Uneaten) ->
-    %% ignore delivery annotations for now
-    %% TODO: handle "rejected" error
-    assemble(message_annotations, RPC, Rest, Uneaten);
-assemble(delivery_annotations, RPC, Else, Uneaten) ->
-    assemble(message_annotations, RPC, Else, Uneaten);
+%% This clause doesn't get called, and is commented out as not to confuse dialyzer.
+%%
+%% assemble(delivery_annotations, RPC, {#'v1_0.delivery_annotations'{}, Rest},
+%%          Uneaten) ->
+%%     %% ignore delivery annotations for now
+%%     %% TODO: handle "rejected" error
+%%     assemble(message_annotations, RPC, Rest, Uneaten);
+%% assemble(delivery_annotations, RPC, Else, Uneaten) ->
+%%     assemble(message_annotations, RPC, Else, Uneaten);
 
 assemble(message_annotations, {R, P = #'P_basic'{headers = Headers}, C},
          {#'v1_0.message_annotations'{}, Rest}, Uneaten) ->
@@ -114,10 +116,11 @@ assemble(footer, {R, P = #'P_basic'{headers = Headers}, C},
 assemble(footer, {R, P, C}, none, _) ->
     {R, P, C};
 assemble(footer, _, Else, _) ->
-    exit({unexpected_trailing_sections, Else});
+    exit({unexpected_trailing_sections, Else}).
 
-assemble(Expected, _, Actual, _) ->
-    exit({expected_section, Expected, Actual}).
+%% Catch-all clause, not needed according to dialyzer
+%% assemble(Expected, _, Actual, _) ->
+%%     exit({expected_section, Expected, Actual}).
 
 decode_section(<<>>) ->
     none;
@@ -179,8 +182,6 @@ to_expiration(undefined) ->
 to_expiration({uint, Num}) ->
     list_to_binary(integer_to_list(Num)).
 
-from_expiration(undefined) ->
-    undefined;
 from_expiration(PBasic) ->
     case rabbit_basic:parse_expiration(PBasic) of
         {ok, undefined} -> undefined;
@@ -302,8 +303,6 @@ wrap(Type, Val) ->
 table_lookup(undefined, _)    -> undefined;
 table_lookup(Headers, Header) -> rabbit_misc:table_lookup(Headers, Header).
 
-map_add(_T, _Key, _Type, undefined, Acc) ->
-    Acc;
 map_add(KeyType, Key, Type, Value, Acc) ->
     [{wrap(KeyType, Key), wrap(Type, Value)} | Acc].
 
diff --git a/deps/rabbitmq_amqp1_0/src/rabbit_amqp1_0_reader.erl b/deps/rabbitmq_amqp1_0/src/rabbit_amqp1_0_reader.erl
index 3cf91da6c2..a780d00df9 100644
--- a/deps/rabbitmq_amqp1_0/src/rabbit_amqp1_0_reader.erl
+++ b/deps/rabbitmq_amqp1_0/src/rabbit_amqp1_0_reader.erl
@@ -74,6 +74,7 @@ shutdown(Pid, Explanation) ->
 system_continue(Parent, Deb, State) ->
     ?MODULE:mainloop(Deb, State#v1{parent = Parent}).
 
+-spec system_terminate(term(), term(), term(), term()) -> no_return().
 system_terminate(Reason, _Parent, _Deb, _State) ->
     exit(Reason).
 
@@ -147,8 +148,8 @@ handle_other({conserve_resources, Source, Conserve},
     Throttle1 = Throttle#throttle{alarmed_by = CR1},
     control_throttle(State#v1{throttle = Throttle1});
 handle_other({'EXIT', Parent, Reason}, State = #v1{parent = Parent}) ->
-    terminate(io_lib:format("broker forced connection closure "
-                            "with reason '~w'", [Reason]), State),
+    _ = terminate(io_lib:format("broker forced connection closure "
+                                "with reason '~w'", [Reason]), State),
     %% this is what we are expected to do according to
     %% http://www.erlang.org/doc/man/sys.html
     %%
@@ -203,8 +204,8 @@ switch_callback(State, Callback, Length) ->
 
 terminate(Reason, State) when ?IS_RUNNING(State) ->
     {normal, handle_exception(State, 0,
-                              {?V_1_0_AMQP_ERROR_INTERNAL_ERROR,
-                               "Connection forced: ~tp", [Reason]})};
+                              error_frame(?V_1_0_AMQP_ERROR_INTERNAL_ERROR,
+                                          "Connection forced: ~tp", [Reason]))};
 terminate(_Reason, State) ->
     {force, State}.
 
@@ -246,8 +247,7 @@ handle_dependent_exit(ChPid, Reason, State) ->
             maybe_close(control_throttle(State));
         {Channel, uncontrolled} ->
             {RealReason, Trace} = Reason,
-            R = {?V_1_0_AMQP_ERROR_INTERNAL_ERROR,
-                 "Session error: ~tp~n~tp", [RealReason, Trace]},
+            R = error_frame(?V_1_0_AMQP_ERROR_INTERNAL_ERROR, "Session error: ~tp~n~tp", [RealReason, Trace]),
             maybe_close(handle_exception(control_throttle(State), Channel, R))
     end.
 
@@ -262,8 +262,8 @@ maybe_close(State = #v1{connection_state = closing,
     % process it's internal message buffer before the supervision tree
     % shuts everything down and in flight messages such as dispositions
     % could be lost
-    [ _ = rabbit_amqp1_0_session:get_info(SessionPid)
-      || {{channel, _}, {ch_fr_pid, SessionPid}} <- get()],
+    _ = [ _ = rabbit_amqp1_0_session:get_info(SessionPid)
+          || {{channel, _}, {ch_fr_pid, SessionPid}} <- get()],
     NewState;
 maybe_close(State) ->
     State.
diff --git a/deps/rabbitmq_amqp1_0/src/rabbit_amqp1_0_session_process.erl b/deps/rabbitmq_amqp1_0/src/rabbit_amqp1_0_session_process.erl
index 5be43bd115..435da955b3 100644
--- a/deps/rabbitmq_amqp1_0/src/rabbit_amqp1_0_session_process.erl
+++ b/deps/rabbitmq_amqp1_0/src/rabbit_amqp1_0_session_process.erl
@@ -416,9 +416,7 @@ run_buffer1(WriterPid, BCh, Session, Buffer) ->
                     rabbit_amqp1_0_writer:send_command(
                       WriterPid,
                       rabbit_amqp1_0_session:flow_fields(Flow, Session1)),
-                    run_buffer1(WriterPid, BCh, Session1, Buffer);
-                {none, Session1} ->
-                    {Session1, Buffer}
+                    run_buffer1(WriterPid, BCh, Session1, Buffer)
             end;
         _ ->
             {Session, Buffer}
diff --git a/deps/rabbitmq_amqp1_0/src/rabbit_amqp1_0_session_sup.erl b/deps/rabbitmq_amqp1_0/src/rabbit_amqp1_0_session_sup.erl
index 98ad2be951..dcbca00674 100644
--- a/deps/rabbitmq_amqp1_0/src/rabbit_amqp1_0_session_sup.erl
+++ b/deps/rabbitmq_amqp1_0/src/rabbit_amqp1_0_session_sup.erl
@@ -20,11 +20,12 @@
 -export_type([start_link_args/0]).
 
 -type start_link_args() ::
-        {rabbit_types:protocol(), rabbit_net:socket(),
-         rabbit_channel:channel_number(), non_neg_integer(), pid(),
-         rabbit_access_control:username(), rabbit_types:vhost(), pid()}.
+        {'amqp10_framing', rabbit_net:socket(),
+         rabbit_channel:channel_number(), non_neg_integer() | 'unlimited', pid(),
+         #user{}, rabbit_types:vhost(), pid(),
+         {'rabbit_proxy_socket', rabbit_net:socket(), term()} | 'undefined'}.
 
--spec start_link(start_link_args()) -> {'ok', pid(), pid()}.
+-spec start_link(start_link_args()) -> {'ok', pid(), pid()} | {'error', term()}.
 
 %%----------------------------------------------------------------------------
 start_link({amqp10_framing, Sock, Channel, FrameMax, ReaderPid,
diff --git a/deps/rabbitmq_amqp1_0/src/rabbit_amqp1_0_session_sup_sup.erl b/deps/rabbitmq_amqp1_0/src/rabbit_amqp1_0_session_sup_sup.erl
index c792e8078e..05cece5b4f 100644
--- a/deps/rabbitmq_amqp1_0/src/rabbit_amqp1_0_session_sup_sup.erl
+++ b/deps/rabbitmq_amqp1_0/src/rabbit_amqp1_0_session_sup_sup.erl
@@ -20,7 +20,7 @@
 
 -spec start_link() -> rabbit_types:ok_pid_or_error().
 -spec start_session(pid(), rabbit_amqp1_0_session_sup:start_link_args()) ->
-                              {'ok', pid(), pid()}.
+          supervisor:startchild_ret().
 
 %%----------------------------------------------------------------------------
 
diff --git a/deps/rabbitmq_amqp1_0/src/rabbit_amqp1_0_util.erl b/deps/rabbitmq_amqp1_0/src/rabbit_amqp1_0_util.erl
index 4688f8f2f0..90a8230bd2 100644
--- a/deps/rabbitmq_amqp1_0/src/rabbit_amqp1_0_util.erl
+++ b/deps/rabbitmq_amqp1_0/src/rabbit_amqp1_0_util.erl
@@ -23,6 +23,7 @@
 -spec serial_diff(serial_number(), serial_number()) ->
              integer().
 
+-spec protocol_error(term(), io:format(), [term()]) -> no_return().
 protocol_error(Condition, Msg, Args) ->
     exit(#'v1_0.error'{
         condition   = Condition,
diff --git a/deps/rabbitmq_amqp1_0/src/rabbit_amqp1_0_writer.erl b/deps/rabbitmq_amqp1_0/src/rabbit_amqp1_0_writer.erl
index 9592ca971e..48f20a0b5a 100644
--- a/deps/rabbitmq_amqp1_0/src/rabbit_amqp1_0_writer.erl
+++ b/deps/rabbitmq_amqp1_0/src/rabbit_amqp1_0_writer.erl
@@ -14,7 +14,7 @@
 -export([send_command/2, send_command/3,
          send_command_sync/2, send_command_sync/3,
          send_command_and_notify/4, send_command_and_notify/5]).
--export([internal_send_command/4, internal_send_command/6]).
+-export([internal_send_command/4]).
 
 %% internal
 -export([mainloop/1, mainloop1/1]).
@@ -62,12 +62,7 @@
         -> 'ok'.
 -spec internal_send_command
         (rabbit_net:socket(), rabbit_channel:channel_number(),
-         rabbit_framing:amqp_method_record(), rabbit_types:protocol())
-        -> 'ok'.
--spec internal_send_command
-        (rabbit_net:socket(), rabbit_channel:channel_number(),
-         rabbit_framing:amqp_method_record(), rabbit_types:content(),
-         non_neg_integer(), rabbit_types:protocol())
+         rabbit_framing:amqp_method_record(), 'amqp10_framing' | 'rabbit_amqp1_0_sasl')
         -> 'ok'.
 
 %%---------------------------------------------------------------------------
@@ -226,13 +221,6 @@ tcp_send(Sock, Data) ->
 internal_send_command(Sock, Channel, MethodRecord, Protocol) ->
     ok = tcp_send(Sock, assemble_frame(Channel, MethodRecord, Protocol)).
 
-internal_send_command(Sock, Channel, MethodRecord, Content, FrameMax,
-                      Protocol) ->
-    ok = lists:foldl(fun (Frame,     ok) -> tcp_send(Sock, Frame);
-                         (_Frame, Other) -> Other
-                     end, ok, assemble_frames(Channel, MethodRecord,
-                                              Content, FrameMax, Protocol)).
-
 internal_send_command_async(MethodRecord,
                             State = #wstate{channel   = Channel,
                                             protocol  = Protocol,