File 4071-Dialyzer-Fix-checking-of-behaviours.patch of Package erlang

Overview Repositories Revisions Requests Users Attributes Meta

File 4071-Dialyzer-Fix-checking-of-behaviours.patch of Package erlang (Revision e821d375830f7564cfd918f62d383b35)

Currently displaying revision e821d375830f7564cfd918f62d383b35 , Show latest

From 5502f87db8cbe4936e1f255035d3f47f8d963d66 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bj=C3=B6rn=20Gustavsson?= <bjorn@erlang.org>
Date: Mon, 4 Jul 2022 12:31:17 +0200
Subject: [PATCH] Dialyzer: Fix checking of behaviours

Fix two bugs in Dialyzer's behaviour checking:

* When a mandatory callback function is present but not exported,
Dialyzer would not complain about a missing callback. However, it
would complain if the arguments and/or return values were
incompatible.

* When an optional callback function was not exported and had
incompatible arguments and/or return values were incompatible,
Dialyzer would complain. This has been changed to suppress the
warning, because the function might not be intended to be a callback
function, for instance if a release added a new optional callback
function (such as `format_status/1` for the `gen_server` behaviour
added in OTP 25).
---
 lib/dialyzer/src/dialyzer.erl                 |   3 +
 lib/dialyzer/src/dialyzer_behaviours.erl      | 139 ++++++++++--------
 .../results/gen_server_not_exported           |   3 +
 .../src/gen_server_not_exported.erl           |  30 ++++
 4 files changed, 110 insertions(+), 65 deletions(-)
 create mode 100644 lib/dialyzer/test/behaviour_SUITE_data/results/gen_server_not_exported
 create mode 100644 lib/dialyzer/test/behaviour_SUITE_data/src/gen_server_not_exported.erl

diff --git a/lib/dialyzer/src/dialyzer.erl b/lib/dialyzer/src/dialyzer.erl
index 9a5ba44592..83d3c03e7e 100644
--- a/lib/dialyzer/src/dialyzer.erl
+++ b/lib/dialyzer/src/dialyzer.erl
@@ -528,6 +528,9 @@ message_to_string({callback_spec_arg_type_mismatch, [B, F, A, N, ST, CT]},
 message_to_string({callback_missing, [B, F, A]}, _I, _E) ->
   io_lib:format("Undefined callback function ~tw/~w (behaviour ~w)\n",
 		[F, A, B]);
+message_to_string({callback_not_exported, [B, F, A]}, _I, _E) ->
+  io_lib:format("Callback function ~tw/~w exists but is not exported (behaviour ~w)\n",
+		[F, A, B]);
 message_to_string({callback_info_missing, [B]}, _I, _E) ->
   io_lib:format("Callback info about the ~w behaviour is not available\n", [B]);
 %%----- Warnings for unknown functions, types, and behaviours -------------
diff --git a/lib/dialyzer/src/dialyzer_behaviours.erl b/lib/dialyzer/src/dialyzer_behaviours.erl
index 2462c2173f..d5c8ac0886 100644
--- a/lib/dialyzer/src/dialyzer_behaviours.erl
+++ b/lib/dialyzer/src/dialyzer_behaviours.erl
@@ -95,76 +95,85 @@ check_behaviour(Module, Behaviour, #state{plt = Plt} = State, Acc) ->
 check_all_callbacks(_Module, _Behaviour, [], _State, Acc) ->
   Acc;
 check_all_callbacks(Module, Behaviour, [Cb|Rest],
-		    #state{plt = Plt, codeserver = Codeserver,
-			   records = Records} = State, Acc) ->
+		    #state{plt = Plt, codeserver = Codeserver} = State,
+                    Acc0) ->
   {{Behaviour, Function, Arity},
    {{_BehFile, _BehLocation}, Callback, Xtra}} = Cb,
   CbMFA = {Module, Function, Arity},
+  Acc1 = case dialyzer_plt:lookup(Plt, CbMFA) of
+           none ->
+             case lists:member(optional_callback, Xtra) of
+               true -> Acc0;
+               false -> [{callback_missing, [Behaviour, Function, Arity]}|Acc0]
+             end;
+           {value, RetArgTypes} ->
+             case dialyzer_codeserver:is_exported(CbMFA, Codeserver) of
+               true ->
+                 check_callback(RetArgTypes, CbMFA, Behaviour, Callback, State, Acc0);
+               false ->
+                 case lists:member(optional_callback, Xtra) of
+                   true -> Acc0;
+                   false -> [{callback_not_exported, [Behaviour, Function, Arity]}|Acc0]
+                 end
+             end
+         end,
+  check_all_callbacks(Module, Behaviour, Rest, State, Acc1).
+
+check_callback(RetArgTypes, CbMFA, Behaviour, Callback,
+               #state{plt = _Plt, codeserver = Codeserver,
+                      records = Records}, Acc0) ->
+  {_Module, Function, Arity} = CbMFA,
   CbReturnType = dialyzer_contracts:get_contract_return(Callback),
   CbArgTypes = dialyzer_contracts:get_contract_args(Callback),
-  Acc0 = Acc,
-  Acc1 =
-    case dialyzer_plt:lookup(Plt, CbMFA) of
-      'none' ->
-        case lists:member(optional_callback, Xtra) of
-          true -> Acc0;
-          false -> [{callback_missing, [Behaviour, Function, Arity]}|Acc0]
-        end;
-      {'value', RetArgTypes} ->
-	Acc00 = Acc0,
-	{ReturnType, ArgTypes} = RetArgTypes,
-	Acc01 =
-	  case erl_types:t_is_subtype(ReturnType, CbReturnType) of
-	    true -> Acc00;
-	    false ->
-	      case erl_types:t_is_none(
-		     erl_types:t_inf(ReturnType, CbReturnType)) of
-		false -> Acc00;
-		true ->
-		  [{callback_type_mismatch,
-		    [Behaviour, Function, Arity,
-		     erl_types:t_to_string(ReturnType, Records),
-		     erl_types:t_to_string(CbReturnType, Records)]}|Acc00]
-	      end
-	  end,
-	case erl_types:any_none(erl_types:t_inf_lists(ArgTypes, CbArgTypes)) of
-	  false -> Acc01;
-	  true ->
-	    find_mismatching_args(type, ArgTypes, CbArgTypes, Behaviour,
-				  Function, Arity, Records, 1, Acc01)
-	end
-    end,
-  Acc2 =
-    case dialyzer_codeserver:lookup_mfa_contract(CbMFA, Codeserver) of
-      'error' -> Acc1;
-      {ok, {{File, Location}, Contract, _Xtra}} ->
-	Acc10 = Acc1,
-	SpecReturnType0 = dialyzer_contracts:get_contract_return(Contract),
-	SpecArgTypes0 = dialyzer_contracts:get_contract_args(Contract),
-	SpecReturnType = erl_types:subst_all_vars_to_any(SpecReturnType0),
-	SpecArgTypes =
-	  [erl_types:subst_all_vars_to_any(ArgT0) || ArgT0 <- SpecArgTypes0],
-	Acc11 =
-	  case erl_types:t_is_subtype(SpecReturnType, CbReturnType) of
-	    true -> Acc10;
-	    false ->
-	      ExtraType = erl_types:t_subtract(SpecReturnType, CbReturnType),
-	      [{callback_spec_type_mismatch,
-		[File, Location, Behaviour, Function, Arity,
-		 erl_types:t_to_string(ExtraType, Records),
-		 erl_types:t_to_string(CbReturnType, Records)]}|Acc10]
-	  end,
-	case erl_types:any_none(
-	       erl_types:t_inf_lists(SpecArgTypes, CbArgTypes)) of
-	  false -> Acc11;
-	  true ->
-	    find_mismatching_args({spec, File, Location}, SpecArgTypes,
-				  CbArgTypes, Behaviour, Function,
-				  Arity, Records, 1, Acc11)
-	end
-    end,
-  NewAcc = Acc2,
-  check_all_callbacks(Module, Behaviour, Rest, State, NewAcc).
+  {ReturnType, ArgTypes} = RetArgTypes,
+  Acc1 = case erl_types:t_is_subtype(ReturnType, CbReturnType) of
+           true ->
+             Acc0;
+           false ->
+             case erl_types:t_is_none(erl_types:t_inf(ReturnType, CbReturnType)) of
+               false ->
+                 Acc0;
+               true ->
+                 [{callback_type_mismatch,
+                   [Behaviour, Function, Arity,
+                    erl_types:t_to_string(ReturnType, Records),
+                    erl_types:t_to_string(CbReturnType, Records)]}|Acc0]
+             end
+         end,
+  Acc2 = case erl_types:any_none(erl_types:t_inf_lists(ArgTypes, CbArgTypes)) of
+           false -> Acc1;
+           true ->
+             find_mismatching_args(type, ArgTypes, CbArgTypes, Behaviour,
+                                   Function, Arity, Records, 1, Acc1)
+         end,
+  case dialyzer_codeserver:lookup_mfa_contract(CbMFA, Codeserver) of
+    error ->
+      Acc2;
+    {ok, {{File, Location}, Contract, _Xtra}} ->
+      SpecReturnType0 = dialyzer_contracts:get_contract_return(Contract),
+      SpecArgTypes0 = dialyzer_contracts:get_contract_args(Contract),
+      SpecReturnType = erl_types:subst_all_vars_to_any(SpecReturnType0),
+      SpecArgTypes =
+        [erl_types:subst_all_vars_to_any(ArgT0) || ArgT0 <- SpecArgTypes0],
+      Acc3 =
+        case erl_types:t_is_subtype(SpecReturnType, CbReturnType) of
+          true ->
+            Acc2;
+          false ->
+            ExtraType = erl_types:t_subtract(SpecReturnType, CbReturnType),
+            [{callback_spec_type_mismatch,
+              [File, Location, Behaviour, Function, Arity,
+               erl_types:t_to_string(ExtraType, Records),
+               erl_types:t_to_string(CbReturnType, Records)]}|Acc2]
+        end,
+      case erl_types:any_none(erl_types:t_inf_lists(SpecArgTypes, CbArgTypes)) of
+        false -> Acc3;
+        true ->
+          find_mismatching_args({spec, File, Location}, SpecArgTypes,
+                                CbArgTypes, Behaviour, Function,
+                                Arity, Records, 1, Acc3)
+      end
+  end.
 
 find_mismatching_args(_, [], [], _Beh, _Function, _Arity, _Records, _N, Acc) ->
   Acc;
diff --git a/lib/dialyzer/test/behaviour_SUITE_data/results/gen_server_not_exported b/lib/dialyzer/test/behaviour_SUITE_data/results/gen_server_not_exported
new file mode 100644
index 0000000000..d5a375bf58
--- /dev/null
+++ b/lib/dialyzer/test/behaviour_SUITE_data/results/gen_server_not_exported
@@ -0,0 +1,3 @@
+
+gen_server_not_exported.erl:14:1: Callback function init/1 exists but is not exported (behaviour gen_server)
+gen_server_not_exported.erl:23:1: Callback function handle_cast/2 exists but is not exported (behaviour gen_server)
diff --git a/lib/dialyzer/test/behaviour_SUITE_data/src/gen_server_not_exported.erl b/lib/dialyzer/test/behaviour_SUITE_data/src/gen_server_not_exported.erl
new file mode 100644
index 0000000000..5c5bf3fbff
--- /dev/null
+++ b/lib/dialyzer/test/behaviour_SUITE_data/src/gen_server_not_exported.erl
@@ -0,0 +1,30 @@
+-module(gen_server_not_exported).
+
+-behaviour(gen_server).
+
+-export([main/0, handle_call/3]).
+
+%% Not exported. Should be a warning.
+main() ->
+    _ = init(whatever),
+    _ = handle_cast(whatever, some_state),
+    _ = format_status(<<"xyz">>),
+    ok.
+
+init(_) ->
+    ok.
+
+%% OK. No warning.
+handle_call(_Request, _From, State) ->
+    {noreply, State}.
+
+%% Not exported. Should be a warning.
+-spec handle_cast(any(), any()) -> binary().
+handle_cast(_Request, _State) ->
+    <<"abc">>.
+
+%% Not exported and conflicting arguments and return value. No warning
+%% since format_status/1 is an optional callback.
+-spec format_status(binary()) -> binary().
+format_status(Bin) when is_binary(Bin) ->
+    Bin.
-- 
2.35.3

Places

File 4071-Dialyzer-Fix-checking-of-behaviours.patch of Package erlang (Revision e821d375830f7564cfd918f62d383b35)

Places