fix dialyzer warnings in prelude

Summary: Running dialyzer on prelude//erlang/... reveals a lot of issues we are fixing in this commit.

Reviewed By: Balajiganapathi

Differential Revision: D74003025

fbshipit-source-id: d785de05c60950585c1870df96059165abdb0455

Author: TheGeorge

prelude/erlang/common_test/common/include/buck_ct_records.hrl

@@ -28,7 +28,7 @@
     suite_path :: file:filename_all(),
     output_dir :: file:filename_all(),
     dependencies :: [file:filename_all()],
-    test_spec_file :: file:filename_all(),
+    test_spec_file :: file:filename_all() | undefined,
     output_format :: xml | json,
     config_files :: [file:filename_all()],
     providers :: [{module(), [term()]}],
@@ -37,7 +37,7 @@
     erl_cmd :: [binary()],
     extra_flags :: [string()],
     artifact_annotation_mfa :: artifact_annotations:annotation_function(),
-    raw_target :: binary()
+    raw_target :: binary() | undefined
 }).
 
 -record(run_specs, {

prelude/erlang/common_test/common/include/tpx_records.hrl

@@ -7,17 +7,19 @@
 
 -record(test_spec_test_info, {name :: string(), filter :: string()}).
 
--record(test_spec_test_case, {suite :: string(), testcases :: [#test_spec_test_info{}]}).
+-record(test_spec_test_case, {suite :: binary(), testcases :: [#test_spec_test_info{}]}).
+
+-type optional(Type) :: undefined | Type.
 
 -record(test, {
     name :: string(),
     suite :: string(),
     type :: string(),
-    message :: junit_interfacer:optional(string()),
-    stacktrace :: junit_interfacer:optional(string()),
-    stdout :: junit_interfacer:optional(string()),
-    stderr :: junit_interfacer:optional(string()),
-    time :: junit_interfacer:optional(integer())
+    message :: optional(string()),
+    stacktrace :: optional(string()),
+    stdout :: optional(string()),
+    stderr :: optional(string()),
+    time :: optional(integer())
 }).
 
 -record(test_case, {

prelude/erlang/common_test/common/src/ct_error_printer.erl

@@ -50,7 +50,7 @@ format_error(ErrType, Reason, FormatStackTrace) ->
             end
     end.
 
--spec format_reason(term()) -> {ok, unicode:chardata()}.
+-spec format_reason(term()) -> {ok, unicode:chardata()} | unrecognized_error.
 format_reason(Reason) ->
     blame_reason(Reason),
     lists:foldl(

prelude/erlang/common_test/common/src/io_buffer.erl

@@ -181,8 +181,8 @@ multi_request([], {LatestReply, State}) ->
     {LatestReply, State};
 multi_request([_ | _], {{error, Error}, State}) ->
     {{error, Error}, State};
-multi_request([R | _Rs], {_Reply, State}) ->
-    multi_request(R, request(R, State)).
+multi_request([R | Rs], {_Reply, State}) ->
+    multi_request(Rs, request(R, State)).
 
 terminate(#state{process = Process, group_leader = GroupLeader} = _State) ->
     group_leader(GroupLeader, Process).

prelude/erlang/common_test/cth_hooks/src/cth_tpx.erl

@@ -7,6 +7,8 @@
 
 %% % @format
 -module(cth_tpx).
+
+-export([is_running_in_sandcastle/0]).
 -eqwalizer(ignore).
 
 %% Callbacks
@@ -33,6 +35,9 @@
 
 -export([terminate/1]).
 
+%% please dialyzer
+-export([ok_group/1, fail_group/1]).
+
 %% For tests purposes
 
 -include("method_ids.hrl").
@@ -53,21 +58,21 @@
 
 -record(state, {
     io_buffer :: pid() | undefined,
-    suite :: string(),
+    suite :: string() | undefined,
     groups :: list(string()),
     starting_times :: starting_times(),
     tree_results :: tree_node(),
-    previous_group_failed :: string(),
+    previous_group_failed :: boolean() | undefined,
     output :: {file, string()} | stdout
 }).
 
--type hook_opts() :: #{role := top, result_json => string()} | #{role := bot}.
+-type hook_opts() :: #{role := cth_tpx_role:role(), result_json => string()}.
 
 -type shared_state() :: #state{}.
 -type hook_state() :: #{
     id := term(),
-    role := ct_tpx_role:role(),
-    server := shared_state()
+    role := cth_tpx_role:role(),
+    server := 'undefined' | pid() | port()
 }.
 -type starting_times() :: #{method_id() => float()}.
 
@@ -117,13 +122,13 @@ fmt_stack(_Suite, _CasePat, _CaseArgs, Reason, _Label) ->
 %% -----------------------------------------------------------------------------
 
 %% @doc Return a unique id for this CTH.
--spec id(hook_opts()) -> term().
+-spec id(hook_opts()) -> {?MODULE, cth_tpx_role:role()}.
 id(#{role := Role}) ->
     {?MODULE, Role}.
 
 %% @doc Always called before any other callback function. Use this to initiate
 %% any common state.
--spec init(_Id :: term(), Opts :: hook_opts()) -> {ok, hook_state()}.
+-spec init(_Id :: term(), Opts :: hook_opts()) -> {ok, hook_state(), integer()}.
 init(Id, Opts = #{role := Role}) ->
     ServerName = '$cth_tpx$server$',
     case Role of
@@ -138,7 +143,8 @@ init(Id, Opts = #{role := Role}) ->
             init_role_bot(Id, ServerName)
     end.
 
--spec init_role_top(Id :: term(), ServerName :: atom(), Output :: stdout | {file, string()}) -> {ok, hook_state()}.
+-spec init_role_top(Id :: term(), ServerName :: atom(), Output :: stdout | {file, string()}) ->
+    {ok, hook_state(), integer()}.
 init_role_top(Id, ServerName, Output) ->
     % IoBuffer that will catpures all the output produced by ct
     IoBuffer = whereis(cth_tpx_io_buffer),
@@ -167,7 +173,7 @@ init_role_top(Id, ServerName, Output) ->
     },
     {ok, HookState, cth_tpx_role:role_priority(top)}.
 
--spec init_role_bot(Id :: term(), ServerName :: atom()) -> {ok, hook_state()}.
+-spec init_role_bot(Id :: term(), ServerName :: atom()) -> {ok, hook_state(), integer()}.
 init_role_bot(Id, ServerName) ->
     % Put there by init_role_top
     Handle = whereis(ServerName),
@@ -180,7 +186,7 @@ init_role_bot(Id, ServerName) ->
     {ok, HookState, cth_tpx_role:role_priority(bot)}.
 
 %% @doc Called before init_per_suite is called.
--spec pre_init_per_suite(string(), any(), hook_state()) -> hook_state().
+-spec pre_init_per_suite(string(), any(), hook_state()) -> {any(), hook_state()}.
 pre_init_per_suite(Suite, Config, HookState) ->
     on_shared_state(HookState, ?FUNCTION_NAME, Config, fun(State) ->
         initialize_stdout_capture(State),
@@ -480,11 +486,6 @@ add_result(
         output = {file, OutputFile}
     }
 ) ->
-    NameMethod =
-        case Method of
-            {TestCase, Phase} -> io_lib:format("~s.~s", [TestCase, atom_to_list(Phase)]);
-            NameMethod0 -> NameMethod0
-        end,
     StdOut =
         case IoBuffer of
             undefined ->
@@ -515,7 +516,8 @@ add_result(
                         Io
                 end
         end,
-    QualifiedName = cth_tpx_test_tree:qualified_name(Groups, NameMethod),
+
+    QualifiedName = method_name(Method, Groups),
     TS = second_timestamp(),
     Result0 = #{
         name => QualifiedName,
@@ -539,6 +541,15 @@ add_result(
     NewTreeResults = cth_tpx_test_tree:register_result(TreeResults, Result, Groups, Method),
     State#state{starting_times = ST1, tree_results = NewTreeResults}.
 
+-spec method_name(method_id(), [string()]) -> string().
+method_name(Method, Groups) ->
+    MethodName =
+        case Method of
+            {TestCase, Phase} -> io_lib:format("~s.~s", [atom_to_list(TestCase), atom_to_list(Phase)]);
+            MethodName0 -> atom_to_list(MethodName0)
+        end,
+    cth_tpx_test_tree:qualified_name(Groups, MethodName).
+
 pre_end_per_testcase(_SuiteName, TC, Config, HookState) ->
     on_shared_state(HookState, ?FUNCTION_NAME, Config, fun(State) ->
         {Config, capture_starting_time(State, {TC, ?END_PER_TESTCASE})}
@@ -632,7 +643,7 @@ terminate(#{role := top, server := Handle}) ->
 terminate(#{role := bot}) ->
     ok.
 
--spec write_output({file, string()} | stdout, string()) -> ok.
+-spec write_output({file, string()} | stdout, binary()) -> ok.
 write_output({file, FN}, JSON) ->
     io:format("Writing result file ~p", [FN]),
     ok = filelib:ensure_dir(FN),

prelude/erlang/common_test/cth_hooks/src/cth_tpx_test_tree.erl

@@ -46,20 +46,20 @@
     type := node,
     init_method := option(method_result()),
     end_method := option(method_result()),
-    test_cases := #{string() => test_leaf()},
+    test_cases := #{name() => test_leaf()},
     sub_groups := #{name() => tree_node()}
 }.
 
 -type test_leaf() :: #{
-    name := string(),
+    name := name(),
     type := leaf,
     init_method := option(method_result()),
     end_method := option(method_result()),
     main_method := option(method_result())
 }.
 
 -type method_result() :: #{
-    name := string(),
+    name := name(),
     startedTime => float(),
     endedTime => float(),
     outcome := outcome(),
@@ -87,10 +87,9 @@ qualified_name(Groups, TestCase) ->
     JoinedGroups = string:join(lists:reverse(StringGroups), ":"),
     Raw = io_lib:format("~s.~s", [JoinedGroups, TestCase]),
     case unicode:characters_to_list(Raw, latin1) of
-        Res when is_list(Res) -> Res
+        Res when not is_tuple(Res) -> Res
     end.
 
-
 %% Tree creation and update
 
 %% @doc Creates a new node
@@ -106,7 +105,7 @@ new_node(Name) ->
     }.
 
 %% @doc Creates a new leaf
--spec new_leaf(Name :: string()) -> test_leaf().
+-spec new_leaf(Name :: name()) -> test_leaf().
 new_leaf(Name) ->
     #{
         name => Name,
@@ -198,15 +197,19 @@ report_end_failure(
 ) ->
     MergedOutcome = merge_outcome(EndOutcome, ResultOutcome),
     EndFailedDetails =
-        unicode_characters_to_string([io_lib:format("~p ~p because ~p failed with ~n", [TestName, MergedOutcome, EndName]), EndDetails]),
+        unicode_characters_to_string([
+            io_lib:format("~p ~p because ~p failed with ~n", [TestName, MergedOutcome, EndName]), EndDetails
+        ]),
     MergedDetails =
         case ResultOutcome of
             passed ->
                 EndFailedDetails;
             _ ->
-                unicode_characters_to_string(lists:flatten(
-                    io_lib:format("~s~n~n~s", [ResultDetails, EndFailedDetails])
-                ))
+                unicode_characters_to_string(
+                    lists:flatten(
+                        io_lib:format("~s~n~n~s", [ResultDetails, EndFailedDetails])
+                    )
+                )
         end,
     report_end_failure(Rest, ResultAcc#{outcome => MergedOutcome, details => MergedDetails}).
 

prelude/erlang/common_test/cth_hooks/src/method_ids.hrl

@@ -23,6 +23,7 @@
     ?END_PER_TESTCASE |
     ?END_PER_GROUP |
     ?END_PER_SUITE |
-    ?MAIN_TESTCASE.
+    ?MAIN_TESTCASE |
+    {atom(), ?INIT_PER_TESTCASE | ?END_PER_TESTCASE | ?MAIN_TESTCASE}.
 
 -endif. % CTH_TPX_METHOD_IDS_HRL