From b07101c843f17ae8b2b3fcfe947e23c03f0e5ff9 Mon Sep 17 00:00:00 2001 From: Kostis Sagonas Date: Mon, 8 Feb 2010 20:30:14 +0200 Subject: Allow recursive types and check for undefined types Contains four kinds of changes: * Allows recursive types and type definitions to be in any order. * Because the checking is not performed from top to bottom, there is a separate pass which checks for undefined module-local types. * Behaviour callbacks which allow specs in them. * Clean up the code as suggested by tidier. --- lib/stdlib/src/erl_lint.erl | 104 +++++++++++++++++++++----------------------- 1 file changed, 49 insertions(+), 55 deletions(-) diff --git a/lib/stdlib/src/erl_lint.erl b/lib/stdlib/src/erl_lint.erl index 156d68554e..ed24d48235 100644 --- a/lib/stdlib/src/erl_lint.erl +++ b/lib/stdlib/src/erl_lint.erl @@ -78,7 +78,7 @@ value_option(Flag, Default, On, OnVal, Off, OffVal, Opts) -> calls = dict:new(), %Who calls who imported = [], %Actually imported functions used_records=sets:new() :: set(), %Used record definitions - used_types = sets:new() :: set() %Used type definitions + used_types = dict:new() :: dict() %Used type definitions }). %% Define the lint state record. @@ -277,6 +277,8 @@ format_error({conflicting_behaviours,{Name,Arity},B,FirstL,FirstB}) -> format_error({undefined_behaviour_func, {Func,Arity}, Behaviour}) -> io_lib:format("undefined callback function ~w/~w (behaviour '~w')", [Func,Arity,Behaviour]); +format_error({undefined_behaviour_func, {Func,Arity,_Spec}, Behaviour}) -> + format_error({undefined_behaviour_func, {Func,Arity}, Behaviour}); format_error({undefined_behaviour,Behaviour}) -> io_lib:format("behaviour ~w undefined", [Behaviour]); format_error({undefined_behaviour_callbacks,Behaviour}) -> @@ -288,7 +290,7 @@ format_error({ill_defined_behaviour_callbacks,Behaviour}) -> %% --- types and specs --- format_error({singleton_typevar, Name}) -> io_lib:format("type variable ~w is only used once (is unbound)", [Name]); -format_error({type_ref, {TypeName, Arity}}) -> +format_error({undefined_type, {TypeName, Arity}}) -> io_lib:format("type ~w~s undefined", [TypeName, gen_type_paren(Arity)]); format_error({unused_type, {TypeName, Arity}}) -> io_lib:format("type ~w~s is unused", [TypeName, gen_type_paren(Arity)]); @@ -757,10 +759,11 @@ post_traversal_check(Forms, St0) -> St7 = check_bif_clashes(Forms, St6), St8 = check_specs_without_function(St7), St9 = check_functions_without_spec(Forms, St8), - StA = check_unused_types(Forms, St9), - StB = check_untyped_records(Forms, StA), - StC = check_on_load(StB), - check_unused_records(Forms, StC). + StA = check_undefined_types(St9), + StB = check_unused_types(Forms, StA), + StC = check_untyped_records(Forms, StB), + StD = check_on_load(StC), + check_unused_records(Forms, StD). %% check_behaviour(State0) -> State %% Check that the behaviour attribute is valid. @@ -786,13 +789,20 @@ behaviour_callbacks(Line, B, St0) -> Funcs when is_list(Funcs) -> All = all(fun({FuncName, Arity}) -> is_atom(FuncName) andalso is_integer(Arity); + ({FuncName, Arity, Spec}) -> + is_atom(FuncName) andalso is_integer(Arity) + andalso is_list(Spec); (_Other) -> false end, Funcs), + MaybeRemoveSpec = fun({_F,_A}=FA) -> FA; + ({F,A,_S}) -> {F,A}; + (Other) -> Other + end, if All =:= true -> - {Funcs, St0}; + {[MaybeRemoveSpec(F) || F <- Funcs], St0}; true -> St1 = add_warning(Line, {ill_defined_behaviour_callbacks,B}, @@ -970,6 +980,16 @@ check_undefined_functions(#lint{called=Called0,defined=Def0}=St0) -> add_error(L, {undefined_function,NA}, St) end, St0, Undef). +%% check_undefined_types(State0) -> State + +check_undefined_types(#lint{usage=Usage,types=Def}=St0) -> + Used = Usage#usage.used_types, + UTAs = dict:fetch_keys(Used), + Undef = [{TA,dict:fetch(TA, Used)} || TA <- UTAs, not dict:is_key(TA, Def)], + foldl(fun ({TA,L}, St) -> + add_error(L, {undefined_type,TA}, St) + end, St0, Undef). + %% check_bif_clashes(Forms, State0) -> State check_bif_clashes(Forms, St0) -> @@ -1427,20 +1447,11 @@ is_pattern_expr_1({tuple,_Line,Es}) -> all(fun is_pattern_expr/1, Es); is_pattern_expr_1({nil,_Line}) -> true; is_pattern_expr_1({cons,_Line,H,T}) -> - case is_pattern_expr_1(H) of - true -> is_pattern_expr_1(T); - false -> false - end; + is_pattern_expr_1(H) andalso is_pattern_expr_1(T); is_pattern_expr_1({op,_Line,Op,A}) -> - case erl_internal:arith_op(Op, 1) of - true -> is_pattern_expr_1(A); - false -> false - end; + erl_internal:arith_op(Op, 1) andalso is_pattern_expr_1(A); is_pattern_expr_1({op,_Line,Op,A1,A2}) -> - case erl_internal:arith_op(Op, 2) of - true -> all(fun is_pattern_expr/1, [A1,A2]); - false -> false - end; + erl_internal:arith_op(Op, 2) andalso all(fun is_pattern_expr/1, [A1,A2]); is_pattern_expr_1(_Other) -> false. %% pattern_bin([Element], VarTable, Old, BinVarTable, State) -> @@ -1817,28 +1828,17 @@ is_gexpr({bin,_L,Fs}, RDs) -> end, Fs); is_gexpr({call,_L,{atom,_Lf,F},As}, RDs) -> A = length(As), - case erl_internal:guard_bif(F, A) of - true -> is_gexpr_list(As, RDs); - false -> false - end; + erl_internal:guard_bif(F, A) andalso is_gexpr_list(As, RDs); is_gexpr({call,_L,{remote,_Lr,{atom,_Lm,erlang},{atom,_Lf,F}},As}, RDs) -> A = length(As), - case erl_internal:guard_bif(F, A) orelse is_gexpr_op(F, A) of - true -> is_gexpr_list(As, RDs); - false -> false - end; + (erl_internal:guard_bif(F, A) orelse is_gexpr_op(F, A)) + andalso is_gexpr_list(As, RDs); is_gexpr({call,L,{tuple,Lt,[{atom,Lm,erlang},{atom,Lf,F}]},As}, RDs) -> is_gexpr({call,L,{remote,Lt,{atom,Lm,erlang},{atom,Lf,F}},As}, RDs); is_gexpr({op,_L,Op,A}, RDs) -> - case is_gexpr_op(Op, 1) of - true -> is_gexpr(A, RDs); - false -> false - end; + is_gexpr_op(Op, 1) andalso is_gexpr(A, RDs); is_gexpr({op,_L,Op,A1,A2}, RDs) -> - case is_gexpr_op(Op, 2) of - true -> is_gexpr_list([A1,A2], RDs); - false -> false - end; + is_gexpr_op(Op, 2) andalso is_gexpr_list([A1,A2], RDs); is_gexpr(_Other, _RDs) -> false. is_gexpr_op('andalso', 2) -> true; @@ -2388,7 +2388,7 @@ check_type(Types, St) -> {SeenVars, St1} = check_type(Types, dict:new(), St), dict:fold(fun(Var, {seen_once, Line}, AccSt) -> case atom_to_list(Var) of - [$_|_] -> AccSt; + "_"++_ -> AccSt; _ -> add_error(Line, {singleton_typevar, Var}, AccSt) end; (_Var, seen_multiple, AccSt) -> @@ -2400,7 +2400,7 @@ check_type({ann_type, _L, [_Var, Type]}, SeenVars, St) -> check_type({paren_type, _L, [Type]}, SeenVars, St) -> check_type(Type, SeenVars, St); check_type({remote_type, L, [{atom, _, Mod}, {atom, _, Name}, Args]}, - SeenVars, St = #lint{module=CurrentMod}) -> + SeenVars, #lint{module=CurrentMod} = St) -> St1 = case (dict:is_key({Name, length(Args)}, default_types()) orelse is_var_arity_type(Name)) of @@ -2463,21 +2463,15 @@ check_type({type, _L, product, Args}, SeenVars, St) -> lists:foldl(fun(T, {AccSeenVars, AccSt}) -> check_type(T, AccSeenVars, AccSt) end, {SeenVars, St}, Args); -check_type({type, La, TypeName, Args}, SeenVars, - St = #lint{types=Defs, usage=Usage}) -> +check_type({type, La, TypeName, Args}, SeenVars, #lint{usage=Usage} = St) -> Arity = length(Args), - St1 = - case dict:is_key({TypeName, Arity}, Defs) of - true -> - UsedTypes1 = Usage#usage.used_types, - UsedTypes2 = sets:add_element({TypeName, Arity}, UsedTypes1), - St#lint{usage=Usage#usage{used_types=UsedTypes2}}; - false -> - case is_var_arity_type(TypeName) of - true -> St; - false -> add_error(La, {type_ref, {TypeName, Arity}}, St) - end - end, + St1 = case is_var_arity_type(TypeName) of + true -> St; + false -> + OldUsed = Usage#usage.used_types, + UsedTypes = dict:store({TypeName, Arity}, La, OldUsed), + St#lint{usage=Usage#usage{used_types=UsedTypes}} + end, check_type({type, -1, product, Args}, SeenVars, St1). check_record_types(Line, Name, Fields, SeenVars, St) -> @@ -2636,7 +2630,7 @@ check_specs([FunType|Left], Arity, St0) -> check_specs([], _Arity, St) -> St. -check_specs_without_function(St = #lint{module=Mod, defined=Funcs}) -> +check_specs_without_function(#lint{module=Mod,defined=Funcs,specs=Specs}=St) -> Fun = fun({M, F, A} = MFA, Line, AccSt) when M =:= Mod -> case gb_sets:is_element({F, A}, Funcs) of true -> AccSt; @@ -2644,7 +2638,7 @@ check_specs_without_function(St = #lint{module=Mod, defined=Funcs}) -> end; ({_M, _F, _A}, _Line, AccSt) -> AccSt end, - dict:fold(Fun, St, St#lint.specs). + dict:fold(Fun, St, Specs). %% This generates warnings for functions without specs; if the user has %% specified both options, we do not generate the same warnings twice. @@ -2688,7 +2682,7 @@ check_unused_types(Forms, St = #lint{usage=Usage, types=Types}) -> (Type, FileLine, AccSt) -> case loc(FileLine) of {FirstFile, _} -> - case sets:is_element(Type, UsedTypes) of + case dict:is_key(Type, UsedTypes) of true -> AccSt; false -> add_warning(FileLine, @@ -3009,7 +3003,7 @@ check_old_unused_vars(Vt, Vt0, St0) -> unused_vars(Vt, Vt0, _St0) -> U0 = orddict:filter(fun (V, {_State,unused,_Ls}) -> case atom_to_list(V) of - [$_|_] -> false; + "_"++_ -> false; _ -> true end; (_V, _How) -> false -- cgit v1.2.3 From a037310622f85a9ae1c083b3210c63a826dda320 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Gustavsson?= Date: Tue, 9 Feb 2010 08:30:13 +0100 Subject: erl_lint_SUITE: adjust failing test case Errors are now reported slightly differently, so we'll need to adjust the test case. --- lib/stdlib/test/erl_lint_SUITE.erl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/stdlib/test/erl_lint_SUITE.erl b/lib/stdlib/test/erl_lint_SUITE.erl index bfbd7b3dc1..2c14aafb24 100644 --- a/lib/stdlib/test/erl_lint_SUITE.erl +++ b/lib/stdlib/test/erl_lint_SUITE.erl @@ -2597,7 +2597,7 @@ otp_8051(Config) when is_list(Config) -> <<"-opaque foo() :: bar(). ">>, [], - {error,[{1,erl_lint,{type_ref,{bar,0}}}], + {error,[{1,erl_lint,{undefined_type,{bar,0}}}], [{1,erl_lint,{unused_type,{foo,0}}}]}}], ?line [] = run(Config, Ts), ok. -- cgit v1.2.3