diff --git a/src/rebar_xref.erl b/src/rebar_xref.erl index a55d71d..6120ada 100644 --- a/src/rebar_xref.erl +++ b/src/rebar_xref.erl @@ -57,27 +57,19 @@ xref(Config, _) -> true = code:add_path(rebar_utils:ebin_dir()), %% Get list of xref checks we want to run - XrefChecks = rebar_config:get(Config, xref_checks, + ConfXrefChecks = rebar_config:get(Config, xref_checks, [exports_not_used, undefined_function_calls]), - %% Look for exports that are unused by anything - ExportsNoWarn = - case lists:member(exports_not_used, XrefChecks) of - true -> - check_exports_not_used(); - false -> - true - end, + SupportedXrefs = [undefined_function_calls, undefined_functions, + locals_not_used, exports_not_used, + deprecated_function_calls, deprecated_functions], - %% Look for calls to undefined functions - UndefNoWarn = - case lists:member(undefined_function_calls, XrefChecks) of - true -> - check_undefined_function_calls(); - false -> - true - end, + XrefChecks = sets:to_list(sets:intersection(sets:from_list(SupportedXrefs), + sets:from_list(ConfXrefChecks))), + + %% Run xref checks + XrefNoWarn = xref_checks(XrefChecks), %% Run custom queries QueryChecks = rebar_config:get(Config, xref_queries, []), @@ -89,7 +81,7 @@ xref(Config, _) -> %% Stop xref stopped = xref:stop(xref), - case lists:member(false, [ExportsNoWarn, UndefNoWarn, QueryNoWarn]) of + case lists:member(false, [XrefNoWarn, QueryNoWarn]) of true -> ?FAIL; false -> @@ -100,26 +92,16 @@ xref(Config, _) -> %% Internal functions %% =================================================================== -check_exports_not_used() -> - {ok, UnusedExports0} = xref:analyze(xref, exports_not_used), - UnusedExports = filter_away_ignored(UnusedExports0), - - %% Report all the unused functions - display_mfas(UnusedExports, "is unused export (Xref)"), - UnusedExports =:= []. - -check_undefined_function_calls() -> - {ok, UndefinedCalls0} = xref:analyze(xref, undefined_function_calls), - UndefinedCalls = - [{find_mfa_source(Caller), format_fa(Caller), format_mfa(Target)} - || {Caller, Target} <- UndefinedCalls0], - - lists:foreach( - fun({{Source, Line}, FunStr, Target}) -> - ?CONSOLE("~s:~w: Warning ~s calls undefined function ~s\n", - [Source, Line, FunStr, Target]) - end, UndefinedCalls), - UndefinedCalls =:= []. +xref_checks(XrefChecks) -> + XrefWarnCount = lists:foldl( + fun(XrefCheck, Acc) -> + {ok, Results} = xref:analyze(xref, XrefCheck), + FilteredResults =filter_xref_results(XrefCheck, Results), + lists:foreach(fun(Res) -> display_xrefresult(XrefCheck, Res) end, FilteredResults), + Acc + length(FilteredResults) + end, + 0, XrefChecks), + XrefWarnCount =:= 0. check_query({Query, Value}) -> {ok, Answer} = xref:q(xref, Query), @@ -147,41 +129,101 @@ code_path(Config) -> %% %% Ignore behaviour functions, and explicitly marked functions %% -filter_away_ignored(UnusedExports) -> - %% Functions can be ignored by using - %% -ignore_xref([{F, A}, ...]). +%% Functions can be ignored by using +%% -ignore_xref([{F, A}, {M, F, A}...]). - %% Setup a filter function that builds a list of behaviour callbacks and/or - %% any functions marked to ignore. We then use this list to mask any - %% functions marked as unused exports by xref - F = fun(Mod) -> - Attrs = Mod:module_info(attributes), - Ignore = keyall(ignore_xref, Attrs), - Callbacks = [B:behaviour_info(callbacks) - || B <- keyall(behaviour, Attrs)], - [{Mod, F, A} || {F, A} <- Ignore ++ lists:flatten(Callbacks)] +get_xref_ignorelist(Mod, XrefCheck) -> + %% Get ignore_xref attribute and combine them in one list + Attributes = + try + Mod:module_info(attributes) + catch + _Class:_Error -> [] end, - AttrIgnore = - lists:flatten( - lists:map(F, lists:usort([M || {M, _, _} <- UnusedExports]))), - [X || X <- UnusedExports, not lists:member(X, AttrIgnore)]. + + Ignore_xref = keyall(ignore_xref, Attributes), + + Behaviour_callbacks = case XrefCheck of + exports_not_used -> [B:behaviour_info(callbacks) || B <- keyall(behaviour, Attributes)]; + _ -> [] + end, + + % And create a flat {M,F,A} list + lists:foldl( + fun(El,Acc) -> + case El of + {F, A} -> [{Mod,F,A} | Acc]; + {M, F, A} -> [{M,F,A} | Acc] + end + end, [],lists:flatten([Ignore_xref, Behaviour_callbacks])). keyall(Key, List) -> lists:flatmap(fun({K, L}) when Key =:= K -> L; (_) -> [] end, List). -display_mfas([], _Message) -> - ok; -display_mfas([{_Mod, Fun, Args} = MFA | Rest], Message) -> - {Source, Line} = find_mfa_source(MFA), - ?CONSOLE("~s:~w: Warning: function ~s/~w ~s\n", - [Source, Line, Fun, Args, Message]), - display_mfas(Rest, Message). +parse_xref_result(XrefResult) -> + case XrefResult of + {_, MFAt} -> MFAt; + MFAt -> MFAt + end. + +filter_xref_results(XrefCheck, XrefResults) -> + SearchModules = lists:usort(lists:map( + fun(Res) -> + case Res of + {Mt,_Ft,_At} -> Mt; + {{Ms,_Fs,_As},{_Mt,_Ft,_At}} -> Ms; + _ -> undefined + end + end, XrefResults)), + + Ignores = lists:flatten([ + get_xref_ignorelist(Module,XrefCheck) || Module <- SearchModules]), + + [Result || Result <- XrefResults, + not lists:member(parse_xref_result(Result),Ignores)]. + +display_xrefresult(Type, XrefResult) -> + { Source, SMFA, TMFA } = case XrefResult of + {MFASource, MFATarget} -> + {format_mfa_source(MFASource), format_mfa(MFASource), + format_mfa(MFATarget)}; + MFATarget -> + {format_mfa_source(MFATarget), format_mfa(MFATarget), + undefined} + end, + case Type of + undefined_function_calls -> + ?CONSOLE("~sWarning: ~s calls undefined function ~s (Xref)\n", + [Source, SMFA, TMFA]); + undefined_functions -> + ?CONSOLE("~sWarning: ~s is undefined function (Xref)\n", + [Source, SMFA]); + locals_not_used -> + ?CONSOLE("~sWarning: ~s is unused local function (Xref)\n", + [Source, SMFA]); + exports_not_used -> + ?CONSOLE("~sWarning: ~s is unused export (Xref)\n", + [Source, SMFA]); + deprecated_function_calls -> + ?CONSOLE("~sWarning: ~s calls deprecated function ~s (Xref)\n", + [Source, SMFA, TMFA]); + deprecated_functions -> + ?CONSOLE("~sWarning: ~s is deprecated function (Xref)\n", + [Source, SMFA]); + Other -> + ?CONSOLE("~sWarning: ~s - ~s xref check: ~s (Xref)\n", + [Source, SMFA, TMFA, Other]) + end. format_mfa({M, F, A}) -> ?FMT("~s:~s/~w", [M, F, A]). -format_fa({_M, F, A}) -> - ?FMT("~s/~w", [F, A]). +format_mfa_source(MFA) -> + case find_mfa_source(MFA) of + {module_not_found, function_not_found} -> ""; + {Source, function_not_found} -> ?FMT("~s: ", [Source]); + {Source, Line} -> ?FMT("~s:~w: ", [Source, Line]) + end. %% %% Extract an element from a tuple, or undefined if N > tuple size @@ -201,7 +243,12 @@ safe_element(N, Tuple) -> %% being too paranoid here. %% find_mfa_source({M, F, A}) -> - {M, Bin, _} = code:get_object_code(M), + case code:get_object_code(M) of + error -> {module_not_found, function_not_found}; + {M, Bin, _} -> find_function_source(M,F,A,Bin) + end. + +find_function_source(M, F, A, Bin) -> AbstractCode = beam_lib:chunks(Bin, [abstract_code]), {ok, {M, [{abstract_code, {raw_abstract_v1, Code}}]}} = AbstractCode, %% Extract the original source filename from the abstract code diff --git a/test/rebar_xref_eunit.erl b/test/rebar_xref_eunit.erl new file mode 100644 index 0000000..45ec283 --- /dev/null +++ b/test/rebar_xref_eunit.erl @@ -0,0 +1,203 @@ +-module(rebar_xref_eunit). + +-compile(export_all). + +-include_lib("eunit/include/eunit.hrl"). + +-define(REBAR_SCRIPT, "../rebar"). + +-define(TMP_DIR, "tmp_xref_eunit/"). + +xref_test_() -> + {"Test the various xref warnings", + setup, fun() -> setup_project(false), rebar("compile"), rebar("skip_deps=true xref") end, + fun teardown/1, + fun(RebarOut) -> + [ + {"Undefined function", ?_assert(string:str(RebarOut, + "myapp_somemod:notavailable/1 is undefined function") =/= 0)}, + {"Undefined function call", ?_assert(string:str(RebarOut, + "myapp_othermod:somefunc/0 calls undefined function myapp_somemod:notavailable/1") =/= 0)}, + {"Deprecated function", ?_assert(string:str(RebarOut, + "myapp_mymod:fdeprecated/0 is deprecated function") =/= 0)}, + {"Deprecated function call", ?_assert(string:str(RebarOut, + "myapp_othermod:somefunc/0 calls deprecated function myapp_mymod:fdeprecated/0") =/= 0)}, + {"Unused local", ?_assert(string:str(RebarOut, + "myapp_mymod:localfunc2/0 is unused local function") =/= 0)}, + {"Unused export 1", ?_assert(string:str(RebarOut, + "myapp_behaviour1:behaviour_info/1 is unused export") =/= 0)}, + {"Unused export 2", ?_assert(string:str(RebarOut, + "myapp_behaviour2:behaviour_info/1 is unused export") =/= 0)}, + {"Unused export 3", ?_assert(string:str(RebarOut, + "myapp_mymod:other2/1 is unused export") =/= 0)}, + {"Unused export 4", ?_assert(string:str(RebarOut, + "myapp_othermod:somefunc/0 is unused export") =/= 0)}, + {"Suppressed behaviour export 1", ?_assert(string:str(RebarOut, + "myapp_mymod:bh1_a/1 is unused export") =:= 0)}, + {"Suppressed behaviour export 2", ?_assert(string:str(RebarOut, + "myapp_mymod:bh1_b/1 is unused export") =:= 0)}, + {"Suppressed behaviour export 3", ?_assert(string:str(RebarOut, + "myapp_mymod:bh2_a/1 is unused export") =:= 0)}, + {"Suppressed behaviour export 4", ?_assert(string:str(RebarOut, + "myapp_mymod:bh2_b/1 is unused export") =:= 0)} + ] + end}. + +xref_ignore_test_() -> + {"Test the suppression of xref warnings", + setup, fun() -> setup_project(ignore_xref), rebar("compile"), rebar("skip_deps=true xref") end, + fun teardown/1, + fun(RebarOut) -> + [ + {"Undefined function can not be suppressed.", ?_assert(string:str(RebarOut, + "myapp_somemod:notavailable/1 is undefined function") =/= 0)}, + {"Supppressed undefined function call", ?_assert(string:str(RebarOut, + "myapp_othermod:somefunc/0 calls undefined function myapp_somemod:notavailable/1") =:= 0)}, + {"Supppressed deprecated function", ?_assert(string:str(RebarOut, + "myapp_mymod:fdeprecated/0 is deprecated function") =:= 0)}, + {"Supppressed deprecated function call", ?_assert(string:str(RebarOut, + "myapp_othermod:somefunc/0 calls deprecated function myapp_mymod:fdeprecated/0") =:= 0)}, + {"Supppressed unused local", ?_assert(string:str(RebarOut, + "myapp_mymod:localfunc2/0 is unused local function") =:= 0)}, + {"Supppressed unused export 1", ?_assert(string:str(RebarOut, + "myapp_behaviour1:behaviour_info/1 is unused export") =:= 0)}, + {"Supppressed unused export 2", ?_assert(string:str(RebarOut, + "myapp_behaviour2:behaviour_info/1 is unused export") =:= 0)}, + {"Supppressed unused export 3", ?_assert(string:str(RebarOut, + "myapp_mymod:other2/1 is unused export") =:= 0)}, + {"Supppressed unused export 4", ?_assert(string:str(RebarOut, + "myapp_othermod:somefunc/0 is unused export") =:= 0)}, + {"Suppressed behaviour export 1", ?_assert(string:str(RebarOut, + "myapp_mymod:bh1_a/1 is unused export") =:= 0)}, + {"Suppressed behaviour export 2", ?_assert(string:str(RebarOut, + "myapp_mymod:bh1_b/1 is unused export") =:= 0)}, + {"Suppressed behaviour export 3", ?_assert(string:str(RebarOut, + "myapp_mymod:bh2_a/1 is unused export") =:= 0)}, + {"Suppressed behaviour export 4", ?_assert(string:str(RebarOut, + "myapp_mymod:bh2_b/1 is unused export") =:= 0)} + ] + + end}. + + +%% ==================================================================== +%% Setup and Teardown +%% ==================================================================== + +-define(myapp_behaviour1, + ["-module(myapp_behaviour1).\n", + "-export([behaviour_info/1]).\n"]). +-define(myapp_behaviour1_body, + ["behaviour_info(callbacks) -> [{bh1_a,1},{bh1_b,1}];\n", + "behaviour_info(_Other) -> undefined.\n"]). +-define(myapp_behaviour1_ignorexref, + ["-ignore_xref({behaviour_info,1}).\n"]). + +-define(myapp_behaviour2, + ["-module(myapp_behaviour2).\n", + "-export([behaviour_info/1]).\n"]). +-define(myapp_behaviour2_body, + ["behaviour_info(callbacks) -> [{bh2_a,1},{bh2_b,1}];\n", + "behaviour_info(_Other) -> undefined.\n"]). +-define(myapp_behaviour2_ignorexref, + ["-ignore_xref({behaviour_info,1}).\n"]). + +-define(myapp_mymod, + ["-module(myapp_mymod).\n", + "-export([bh1_a/1,bh1_b/1,bh2_a/1,bh2_b/1,other1/1,other2/1,fdeprecated/0]).\n", + "-behaviour(myapp_behaviour1).\n", % 2 behaviours + "-behaviour(myapp_behaviour2).\n", + "-deprecated({fdeprecated,0}).\n"]). % deprecated function +-define(myapp_mymod_body, + ["bh1_a(A) -> localfunc1(bh1_a, A).\n", % behaviour functions + "bh1_b(A) -> localfunc1(bh1_b, A).\n", + "bh2_a(A) -> localfunc1(bh2_a, A).\n", + "bh2_b(A) -> localfunc1(bh2_b, A).\n", + "other1(A) -> localfunc1(other1, A).\n", % regular exported functions + "other2(A) -> localfunc1(other2, A).\n", + "localfunc1(A, B) -> {A, B}.\n", % used local + "localfunc2() -> ok.\n", % unused local + "fdeprecated() -> ok.\n" % deprecated function + ]). +-define(myapp_mymod_ignorexref, + ["-ignore_xref([{other2,1},{localfunc2,0},{fdeprecated,0}]).\n"]). + + + +-define(myapp_othermod, + ["-module(myapp_othermod).\n", + "-export([somefunc/0]).\n"]). +-define(myapp_othermod_body, + ["somefunc() ->\n", + " myapp_mymod:other1(arg),\n", + " myapp_somemod:notavailable(arg),\n", + " myapp_mymod:fdeprecated().\n" + ]). +-define(myapp_othermod_ignorexref, + ["-ignore_xref([{myapp_somemod,notavailable,1},{somefunc,0}]).\n", + "-ignore_xref({myapp_mymod,fdeprecated,0}).\n"]). + + +-define(myapp_rebarconfig, + ["{erl_opts, [debug_info]}.\n", + "{xref_checks, [deprecated_function_calls,deprecated_functions,\n", + " undefined_function_calls,undefined_functions,\n", + " exports_not_used,locals_not_used]}.\n" + ]). + +setup_environment() -> + ok = file:make_dir(?TMP_DIR), + prepare_rebar_script(), + ok = file:set_cwd(?TMP_DIR). + +prepare_project() -> + setup_environment(), + rebar("create-app appid=myapp"), + ok = file:make_dir("ebin"). + +setup_project(ignore_xref) -> + prepare_project(), + ok = file:write_file("src/myapp_behaviour1.erl", ?myapp_behaviour1 ++ ?myapp_behaviour1_ignorexref ++ ?myapp_behaviour1_body), + ok = file:write_file("src/myapp_behaviour2.erl", ?myapp_behaviour2 ++ ?myapp_behaviour2_ignorexref++ ?myapp_behaviour2_body), + ok = file:write_file("src/myapp_mymod.erl", ?myapp_mymod ++ ?myapp_mymod_ignorexref ++ ?myapp_mymod_body), + ok = file:write_file("src/myapp_othermod.erl", ?myapp_othermod ++ ?myapp_othermod_ignorexref ++ ?myapp_othermod_body), + ok = file:write_file("rebar.config", ?myapp_rebarconfig); + +setup_project(_) -> + prepare_project(), + ok = file:write_file("src/myapp_behaviour1.erl", ?myapp_behaviour1 ++ ?myapp_behaviour1_body), + ok = file:write_file("src/myapp_behaviour2.erl", ?myapp_behaviour2 ++ ?myapp_behaviour2_body), + ok = file:write_file("src/myapp_mymod.erl", ?myapp_mymod ++ ?myapp_mymod_body), + ok = file:write_file("src/myapp_othermod.erl", ?myapp_othermod ++ ?myapp_othermod_body), + ok = file:write_file("rebar.config", ?myapp_rebarconfig). + + +teardown(_) -> + ok = file:set_cwd(".."), + ok = remove_tmp_dir(). + +remove_tmp_dir() -> + ok = rebar_file_utils:rm_rf(?TMP_DIR). + +%% ==================================================================== +%% Helper Functions +%% ==================================================================== + +prepare_rebar_script() -> + Rebar = ?TMP_DIR ++ "rebar", + {ok, _} = file:copy(?REBAR_SCRIPT, Rebar), + case os:type() of + {unix, _} -> + [] = os:cmd("chmod u+x " ++ Rebar); + {win32, _} -> + {ok, _} = file:copy(?REBAR_SCRIPT ++ ".bat", + ?TMP_DIR ++ "rebar.bat") + end. + +rebar() -> + rebar([]). + +rebar(Args) when is_list(Args) -> + Out = os:cmd(filename:nativename("./rebar") ++ " " ++ Args), + %% ?debugMsg("**** Begin"), ?debugMsg(Out), ?debugMsg("**** End"), + Out.