diff options
author | Björn Gustavsson <bjorn@erlang.org> | 2022-08-02 07:29:17 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-08-02 07:29:17 +0200 |
commit | 40b10b72839ff0dddecffe02e6bd2102fd8cbf48 (patch) | |
tree | 1fac2450c6181705040721847f233e97cdef410c | |
parent | af88aa69cabba2cc7fe3dbea08b6faf73ae9c471 (diff) | |
parent | 5502f87db8cbe4936e1f255035d3f47f8d963d66 (diff) | |
download | erlang-40b10b72839ff0dddecffe02e6bd2102fd8cbf48.tar.gz |
Merge pull request #6126 from bjorng/bjorn/dialyzer/behaviours/OTP-18127
Dialyzer: Fix checking of behaviours
4 files changed, 110 insertions, 65 deletions
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. |