Merge branch 'aronisstav/dialyzer-missing-callback-info' into maint

* aronisstav/dialyzer-missing-callback-info:
  Fix inadvertent deletion of callback info
  Eliminate ugly case statements
  Remove dead code related to missing behaviour info

OTP-13287

4 files changed, 65 insertions, 76 deletions

diff --git a/lib/dialyzer/src/dialyzer_analysis_callgraph.erl b/lib/dialyzer/src/dialyzer_analysis_callgraph.erl
index 826ac51775..8537878dfc 100644
--- a/lib/dialyzer/src/dialyzer_analysis_callgraph.erl
+++ b/lib/dialyzer/src/dialyzer_analysis_callgraph.erl
@@ -93,31 +93,19 @@ loop(#server_state{parent = Parent} = State,
       send_log(Parent, LogMsg),
       loop(State, Analysis, ExtCalls);
     {AnalPid, warnings, Warnings} ->
-      case Warnings of
-	[] -> ok;
-	SendWarnings ->
-	  send_warnings(Parent, SendWarnings)
-      end,
+      send_warnings(Parent, Warnings),
       loop(State, Analysis, ExtCalls);
     {AnalPid, cserver, CServer, Plt} ->
       send_codeserver_plt(Parent, CServer, Plt),
       loop(State, Analysis, ExtCalls);
     {AnalPid, done, Plt, DocPlt} ->
-      case ExtCalls =:= none of
-	true ->
-	  send_analysis_done(Parent, Plt, DocPlt);
-	false ->
-	  send_ext_calls(Parent, ExtCalls),
-	  send_analysis_done(Parent, Plt, DocPlt)
-      end;
+      send_ext_calls(Parent, ExtCalls),
+      send_analysis_done(Parent, Plt, DocPlt);
     {AnalPid, ext_calls, NewExtCalls} ->
       loop(State, Analysis, NewExtCalls);
     {AnalPid, ext_types, ExtTypes} ->
       send_ext_types(Parent, ExtTypes),
       loop(State, Analysis, ExtCalls);
-    {AnalPid, unknown_behaviours, UnknownBehaviour} ->
-      send_unknown_behaviours(Parent, UnknownBehaviour),
-      loop(State, Analysis, ExtCalls);
     {AnalPid, mod_deps, ModDeps} ->
       send_mod_deps(Parent, ModDeps),
       loop(State, Analysis, ExtCalls);
@@ -572,6 +560,8 @@ send_analysis_done(Parent, Plt, DocPlt) ->
   Parent ! {self(), done, Plt, DocPlt},
   ok.
 
+send_ext_calls(_Parent, none) ->
+  ok;
 send_ext_calls(Parent, ExtCalls) ->
   Parent ! {self(), ext_calls, ExtCalls},
   ok.
@@ -580,10 +570,6 @@ send_ext_types(Parent, ExtTypes) ->
   Parent ! {self(), ext_types, ExtTypes},
   ok.
 
-send_unknown_behaviours(Parent, UnknownBehaviours) ->
-  Parent ! {self(), unknown_behaviours, UnknownBehaviours},
-  ok.
-
 send_codeserver_plt(Parent, CServer, Plt ) ->
   Parent ! {self(), cserver, CServer, Plt},
   ok.
diff --git a/lib/dialyzer/src/dialyzer_cl.erl b/lib/dialyzer/src/dialyzer_cl.erl
index 4116866916..9354b8007e 100644
--- a/lib/dialyzer/src/dialyzer_cl.erl
+++ b/lib/dialyzer/src/dialyzer_cl.erl
@@ -49,8 +49,7 @@
 	 plt_info        = none           :: 'none' | dialyzer_plt:plt_info(),
 	 report_mode     = normal         :: rep_mode(),
 	 return_status= ?RET_NOTHING_SUSPICIOUS	:: dial_ret(),
-	 stored_warnings = []             :: [raw_warning()],
-	 unknown_behaviours = []          :: [dialyzer_behaviours:behaviour()]
+	 stored_warnings = []             :: [raw_warning()]
 	}).
 
 %%--------------------------------------------------------------------
@@ -638,9 +637,6 @@ cl_loop(State, LogCache) ->
     {BackendPid, warnings, Warnings} ->
       NewState = store_warnings(State, Warnings),
       cl_loop(NewState, LogCache);
-    {BackendPid, unknown_behaviours, Behaviours} ->
-      NewState = store_unknown_behaviours(State, Behaviours),
-      cl_loop(NewState, LogCache);
     {BackendPid, done, NewPlt, _NewDocPlt} ->
       return_value(State, NewPlt);
     {BackendPid, ext_calls, ExtCalls} ->
@@ -683,11 +679,6 @@ format_log_cache(LogCache) ->
 store_warnings(#cl_state{stored_warnings = StoredWarnings} = St, Warnings) ->
   St#cl_state{stored_warnings = StoredWarnings ++ Warnings}.
 
--spec store_unknown_behaviours(#cl_state{}, [dialyzer_behaviours:behaviour()]) -> #cl_state{}.
-
-store_unknown_behaviours(#cl_state{unknown_behaviours = Behs} = St, Beh) ->
-  St#cl_state{unknown_behaviours = Beh ++ Behs}.
-
 -spec cl_error(string()) -> no_return().
 
 cl_error(Msg) ->
@@ -724,7 +715,6 @@ return_value(State = #cl_state{erlang_mode = ErlangMode,
       print_warnings(State),
       print_ext_calls(State),
       print_ext_types(State),
-      print_unknown_behaviours(State),
       maybe_close_output_file(State),
       {RetValue, []};
     true -> 
@@ -737,8 +727,7 @@ unknown_warnings(State = #cl_state{legal_warnings = LegalWarnings}) ->
   Unknown = case ordsets:is_element(?WARN_UNKNOWN, LegalWarnings) of
               true ->
                 unknown_functions(State) ++
-                  unknown_types(State) ++
-                  unknown_behaviours(State);
+                  unknown_types(State);
               false -> []
             end,
   WarningInfo = {_Filename = "", _Line = 0, _MorMFA = ''},
@@ -814,48 +803,6 @@ do_print_ext_types(Output, [{M,F,A}|T], Before) ->
 do_print_ext_types(_, [], _) ->
   ok.
 
-unknown_behaviours(#cl_state{unknown_behaviours = DupBehaviours,
-                             legal_warnings = LegalWarnings}) ->
-  case ordsets:is_element(?WARN_BEHAVIOUR, LegalWarnings) of
-    false -> [];
-    true ->
-      Behaviours = lists:usort(DupBehaviours),
-      [{unknown_behaviour, B} || B <- Behaviours]
-  end.
-
-%%print_unknown_behaviours(#cl_state{report_mode = quiet}) ->
-%%  ok;
-print_unknown_behaviours(#cl_state{output = Output,
-				   external_calls = Calls,
-				   external_types = Types,
-				   stored_warnings = Warnings,
-				   unknown_behaviours = DupBehaviours,
-				   legal_warnings = LegalWarnings,
-				   output_format = Format}) ->
-  case ordsets:is_element(?WARN_BEHAVIOUR, LegalWarnings)
-    andalso DupBehaviours =/= [] of
-    false -> ok;
-    true ->
-      Behaviours = lists:usort(DupBehaviours),
-      case Warnings =:= [] andalso Calls =:= [] andalso Types =:= [] of
-	true -> io:nl(Output); %% Need to do a newline first
-	false -> ok
-      end,
-      {Prompt, Prefix} =
-	case Format of
-	  formatted -> {"Unknown behaviours:\n","  "};
-	  raw -> {"%% Unknown behaviours:\n","%%  "}
-	end,
-      io:put_chars(Output, Prompt),
-      do_print_unknown_behaviours(Output, Behaviours, Prefix)
-  end.
-
-do_print_unknown_behaviours(Output, [B|T], Before) ->
-  io:format(Output, "~s~p\n", [Before,B]),
-  do_print_unknown_behaviours(Output, T, Before);
-do_print_unknown_behaviours(_, [], _) ->
-  ok.
-
 print_warnings(#cl_state{stored_warnings = []}) ->
   ok;
 print_warnings(#cl_state{output = Output,
diff --git a/lib/dialyzer/src/dialyzer_plt.erl b/lib/dialyzer/src/dialyzer_plt.erl
index 634871b2eb..769f26a3df 100644
--- a/lib/dialyzer/src/dialyzer_plt.erl
+++ b/lib/dialyzer/src/dialyzer_plt.erl
@@ -137,7 +137,7 @@ delete_list(#plt{info = Info, types = Types,
   #plt{info = table_delete_list(Info, List),
        types = Types,
        contracts = table_delete_list(Contracts, List),
-       callbacks = table_delete_list(Callbacks, List),
+       callbacks = Callbacks,
        exported_types = ExpTypes}.
 
 -spec insert_contract_list(plt(), dialyzer_contracts:plt_contracts()) -> plt().
diff --git a/lib/dialyzer/test/plt_SUITE.erl b/lib/dialyzer/test/plt_SUITE.erl
index ecbac14e5d..16180a7435 100644
--- a/lib/dialyzer/test/plt_SUITE.erl
+++ b/lib/dialyzer/test/plt_SUITE.erl
@@ -7,12 +7,15 @@
 -include("dialyzer_test_constants.hrl").
 
 -export([suite/0, all/0, build_plt/1, beam_tests/1, update_plt/1,
+         local_fun_same_as_callback/1,
          run_plt_check/1, run_succ_typings/1]).
 
 suite() ->
   [{timetrap, ?plt_timeout}].
 
-all() -> [build_plt, beam_tests, update_plt, run_plt_check, run_succ_typings].
+all() ->
+  [build_plt, beam_tests, update_plt, run_plt_check, run_succ_typings,
+   local_fun_same_as_callback].
 
 build_plt(Config) ->
   OutDir = ?config(priv_dir, Config),
@@ -158,6 +161,59 @@ update_plt(Config) ->
                       {init_plt, Plt}] ++ Opts),
     ok.
 
+%%% If a behaviour module contains an non-exported function with the same name
+%%% as one of the behaviour's callbacks, the callback info was inadvertently
+%%% deleted from the PLT as the dialyzer_plt:delete_list/2 function was cleaning
+%%% up the callback table. This bug was reported by Brujo Benavides.
+
+local_fun_same_as_callback(Config) when is_list(Config) ->
+    PrivDir = ?config(priv_dir, Config),
+    Prog1 =
+      <<"-module(bad_behaviour).
+         -callback bad() -> bad.
+         -export([publicly_bad/0]).
+
+         %% @doc This function is here just to avoid the 'unused' warning for bad/0
+         publicly_bad() -> bad().
+
+         %% @doc This function overlaps with the callback with the same name, and
+         %%      that was an issue for dialyzer since it's a private function.
+         bad() -> bad.">>,
+    {ok, Beam} = compile(Config, Prog1, bad_behaviour, []),
+
+    ErlangBeam = case code:where_is_file("erlang.beam") of
+                     non_existing ->
+                         filename:join([code:root_dir(),
+                                        "erts", "preloaded", "ebin",
+                                        "erlang.beam"]);
+                     EBeam ->
+                         EBeam
+                 end,
+    Plt = filename:join(PrivDir, "plt_bad_behaviour.plt"),
+    Opts = [{check_plt, true}, {from, byte_code}],
+    [] = dialyzer:run([{analysis_type, plt_build},
+                       {files, [Beam, ErlangBeam]},
+                       {output_plt, Plt}] ++ Opts),
+
+    Prog2 =
+        <<"-module(bad_child).
+           -behaviour(bad_behaviour).
+
+           -export([bad/0]).
+
+           %% @doc This function incorreclty implements bad_behaviour.
+           bad() -> not_bad.">>,
+    {ok, TestBeam} = compile(Config, Prog2, bad_child, []),
+
+    [{warn_behaviour, _,
+      {callback_type_mismatch,
+       [bad_behaviour,bad,0,"'not_bad'","'bad'"]}}] =
+        dialyzer:run([{analysis_type, succ_typings},
+                      {files, [TestBeam]},
+                      {init_plt, Plt}] ++ Opts),
+    ok.
+  
+
 compile(Config, Prog, Module, CompileOpts) ->
     Source = lists:concat([Module, ".erl"]),
     PrivDir = ?config(priv_dir,Config),