Add standard xref checks and improved suppression

* Allow the following predefined analyses :
   undefined_function_calls, undefined_functions, locals_not_used,
   exports_not_used, deprecated_function_calls, deprecated_functions

* Trap some possible errors in case module information is not
    available

* ignore_xref works on all checks and can take {M,F,A} tuples.
  (Automatic behaviour export suppression still only works on
  exports_not_used)

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
+    SupportedXrefs = [undefined_function_calls, undefined_functions,
-    ExportsNoWarn =
+                        locals_not_used, exports_not_used,
-        case lists:member(exports_not_used, XrefChecks) of
+                        deprecated_function_calls, deprecated_functions],
-            true ->
-                check_exports_not_used();
-            false ->
-                true
-        end,
 
-    %% Look for calls to undefined functions
+    XrefChecks = sets:to_list(sets:intersection(sets:from_list(SupportedXrefs),
-    UndefNoWarn =
+                                sets:from_list(ConfXrefChecks))),
-        case lists:member(undefined_function_calls, XrefChecks) of
+
-            true ->
+    %% Run xref checks
-                check_undefined_function_calls();
+    XrefNoWarn = xref_checks(XrefChecks),
-            false ->
-                true
-        end,
 
     %% 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() ->
+xref_checks(XrefChecks) ->
-    {ok, UnusedExports0} = xref:analyze(xref, exports_not_used),
+    XrefWarnCount = lists:foldl(
-    UnusedExports = filter_away_ignored(UnusedExports0),
+        fun(XrefCheck, Acc) ->
-
+            {ok, Results} = xref:analyze(xref, XrefCheck),
-    %% Report all the unused functions
+            FilteredResults =filter_xref_results(XrefCheck, Results),
-    display_mfas(UnusedExports, "is unused export (Xref)"),
+            lists:foreach(fun(Res) -> display_xrefresult(XrefCheck, Res) end, FilteredResults),
-    UnusedExports =:= [].
+            Acc + length(FilteredResults)
-
+        end,
-check_undefined_function_calls() ->
+        0, XrefChecks),
-    {ok, UndefinedCalls0} = xref:analyze(xref, undefined_function_calls),
+    XrefWarnCount =:= 0.
-    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 =:= [].
 
 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
-    %% Functions can be ignored by using
+%% -ignore_xref([{F, A}, {M, F, A}...]).
-    %% -ignore_xref([{F, A}, ...]).
 
-    %% Setup a filter function that builds a list of behaviour callbacks and/or
+get_xref_ignorelist(Mod, XrefCheck) ->
-    %% any functions marked to ignore. We then use this list to mask any
+    %% Get ignore_xref attribute and combine them in one list
-    %% functions marked as unused exports by xref
+    Attributes =
-    F = fun(Mod) ->
+        try
-                Attrs  = Mod:module_info(attributes),
+            Mod:module_info(attributes)
-                Ignore = keyall(ignore_xref, Attrs),
+        catch
-                Callbacks = [B:behaviour_info(callbacks)
+            _Class:_Error -> []
-                             || B <- keyall(behaviour, Attrs)],
-                [{Mod, F, A} || {F, A} <- Ignore ++ lists:flatten(Callbacks)]
         end,
-    AttrIgnore =
+
-        lists:flatten(
+    Ignore_xref = keyall(ignore_xref, Attributes),
-          lists:map(F, lists:usort([M || {M, _, _} <- UnusedExports]))),
+
-    [X || X <- UnusedExports, not lists:member(X, AttrIgnore)].
+    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) ->
+parse_xref_result(XrefResult) ->
-    ok;
+    case XrefResult of
-display_mfas([{_Mod, Fun, Args} = MFA | Rest], Message) ->
+        {_, MFAt} -> MFAt;
-    {Source, Line} = find_mfa_source(MFA),
+        MFAt -> MFAt
-    ?CONSOLE("~s:~w: Warning: function ~s/~w ~s\n",
+    end.
-             [Source, Line, Fun, Args, Message]),
+
-    display_mfas(Rest, Message).
+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}) ->
+format_mfa_source(MFA) ->
-    ?FMT("~s/~w", [F, A]).
+    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

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.