File 4491-Dialyzer-Fix-checking-of-behaviours.patch of Package erlang
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
