tools: Correct Xref's handling of behaviour_info/1

See also https://github.com/erlang/otp/issues/4192.

4 files changed, 99 insertions, 36 deletions

diff --git a/lib/tools/doc/src/xref.xml b/lib/tools/doc/src/xref.xml
index 33b8003320..72cc5c6c39 100644
--- a/lib/tools/doc/src/xref.xml
+++ b/lib/tools/doc/src/xref.xml
@@ -885,7 +885,9 @@ Evaluates a predefined analysis.
            locally used.</item>
           <tag><c>exports_not_used</c></tag>
           <item>Returns a list of exported functions that have not been
-           externally used.</item>
+           externally used. Note that in <c>modules</c> mode,
+	    <c>M:behaviour_info/1</c> is never reported as unused.
+	  </item>
           <tag><c>deprecated_function_calls</c>(*)</tag>
           <item>Returns a list of external calls to <seeerl marker="#deprecated_function">deprecated functions</seeerl>.</item>
           <tag><c>{deprecated_function_calls, DeprFlag}</c>(*)</tag>
diff --git a/lib/tools/src/xref_reader.erl b/lib/tools/src/xref_reader.erl
index 87c02db9eb..9857e8c150 100644
--- a/lib/tools/src/xref_reader.erl
+++ b/lib/tools/src/xref_reader.erl
@@ -1,7 +1,7 @@
 %%
 %% %CopyrightBegin%
 %%
-%% Copyright Ericsson AB 2000-2017. All Rights Reserved.
+%% Copyright Ericsson AB 2000-2020. All Rights Reserved.
 %%
 %% Licensed under the Apache License, Version 2.0 (the "License");
 %% you may not use this file except in compliance with the License.
@@ -53,7 +53,8 @@
 %% R8:  abstract_v2
 %% R9C: raw_abstract_v1
 
-%% -> {ok, Module, {DefAt, CallAt, LC, XC, X, Attrs}, Unresolved}} | EXIT
+%% -> {ok, Module, {DefAt, LCallAt, XCallAt, LC, XC, X, Attrs, Depr, OL},
+%%         Unresolved}} | EXIT
 %% Attrs = {ALC, AXC, Bad}
 %% ALC, AXC and Bad are extracted from the attribute 'xref'. An experiment.
 module(Module, Forms, CollectBuiltins, X, DF) ->
@@ -95,16 +96,19 @@ form({function, _, module_info, 0, _Clauses}, S) ->
     S;
 form({function, _, module_info, 1, _Clauses}, S) ->
     S;
-form({function, 0 = _Line, behaviour_info, 1, _Clauses}, S) ->
-    S;
 form({function, Anno, Name, Arity, Clauses}, S) ->
-    MFA0 = {S#xrefr.module, Name, Arity},
-    MFA = adjust_arity(S, MFA0),
-    S1 = S#xrefr{function = MFA},
-    Line = erl_anno:line(Anno),
-    S2 = S1#xrefr{def_at = [{MFA,Line} | S#xrefr.def_at]},
-    S3 = clauses(Clauses, S2),
-    S3#xrefr{function = []};
+    case {Name, Arity, erl_anno:location(Anno)} of
+        {behaviour_info, 1, 0} ->
+            S; % generated
+        _ ->
+            MFA0 = {S#xrefr.module, Name, Arity},
+            MFA = adjust_arity(S, MFA0),
+            S1 = S#xrefr{function = MFA},
+            Line = erl_anno:line(Anno),
+            S2 = S1#xrefr{def_at = [{MFA,Line} | S#xrefr.def_at]},
+            S3 = clauses(Clauses, S2),
+            S3#xrefr{function = []}
+    end;
 form(_, S) ->
     %% OTP 20. Other uninteresting forms such as {eof, _} and {warning, _}.
     %% Exposed because sys_pre_expand is no longer run.
diff --git a/lib/tools/src/xref_utils.erl b/lib/tools/src/xref_utils.erl
index eca751337b..c516f272cb 100644
--- a/lib/tools/src/xref_utils.erl
+++ b/lib/tools/src/xref_utils.erl
@@ -327,7 +327,7 @@ list_dirs([], _I, _Exts, C, E) ->
 
 %% Returns functions that are present in all modules.
 predefined_functions() ->
-    [{module_info,0}, {module_info,1}].
+    [{module_info,0}, {module_info,1}, {behaviour_info,1}].
 
 %% Returns true if an MFA takes functional arguments.
 is_funfun(erlang, apply, 2) -> true;
diff --git a/lib/tools/test/xref_SUITE.erl b/lib/tools/test/xref_SUITE.erl
index b5b3ff7796..86dcb3c94d 100644
--- a/lib/tools/test/xref_SUITE.erl
+++ b/lib/tools/test/xref_SUITE.erl
@@ -49,7 +49,7 @@
          fun_mfa_vars/1, qlc/1]).
 
 -export([analyze/1, basic/1, md/1, q/1, variables/1, unused_locals/1,
-         behaviour_info_t/1, fake_behaviour_info_t/1]).
+         behaviour/1]).
 
 -export([format_error/1, otp_7423/1, otp_7831/1, otp_10192/1, otp_13708/1,
          otp_14464/1, otp_14344/1]).
@@ -84,7 +84,7 @@ groups() ->
        fun_mfa_r14, fun_mfa_vars, qlc]},
      {analyses, [],
 
-      [analyze, basic, md, q, variables, unused_locals, behaviour_info_t, fake_behaviour_info_t]},
+      [analyze, basic, md, q, variables, unused_locals, behaviour]},
      {misc, [], [format_error, otp_7423, otp_7831, otp_10192, otp_13708,
                  otp_14464, otp_14344]}].
 
@@ -2472,6 +2472,84 @@ otp_14344(Conf) when is_list(Conf) ->
     ok = file:delete(File1),
     ok = file:delete(Beam1).
 
+%% PR-2752, ERL-1353, ERL-1476, GH-4192.
+behaviour(Config) ->
+    ModMode = [{xref_mode, modules}],
+    FunMode = [],
+
+    Test1 = [{a, <<"-module(a).
+                    -callback a() -> ok.
+                    ">>}],
+    {Undef1, UnusedExports1} = behaviour_test(Test1, Config, FunMode),
+    [] = Undef1,
+    [] = UnusedExports1,
+    {Undef1m, UnusedExports1m} = behaviour_test(Test1, Config, ModMode),
+    [] = Undef1m,
+    [] = UnusedExports1m,
+
+    Test2 = [{a, <<"-module(a).
+                    -export([behaviour_info/1]).
+                    behaviour_info(_) ->
+                        ok.
+                   ">>}],
+    {Undef2, UnusedExports2} = behaviour_test(Test2, Config, FunMode),
+    [] = Undef2,
+    [{a,behaviour_info,1}] = UnusedExports2,
+    {Undef2m, UnusedExports2m} = behaviour_test(Test2, Config, ModMode),
+    [] = Undef2m,
+    %% Without abstract code it is not possible to determine if
+    %% M:behaviour_info/1 is generated or not. The best we can do is
+    %% to assume it is generated since it would otherwise always be
+    %% returned as unused.
+    [] = UnusedExports2m,
+
+    Test3 = [{a, <<"-module(a).
+                    -export([behaviour_info/1]).
+                    behaviour_info(_) ->
+                        ok.
+               ">>},
+             {b, <<"-module(b).
+                    -export([bar/0]).
+                    bar() -> a:behaviour_info(callbacks).
+                   ">>}],
+    {Undef3, UnusedExports3} = behaviour_test(Test3, Config, FunMode),
+    [] = Undef3,
+    [{b,bar,0}] = UnusedExports3,
+    {Undef3m, UnusedExports3m} = behaviour_test(Test3, Config, ModMode),
+    [] = Undef3m,
+    [{b,bar,0}] = UnusedExports3m,
+    ok.
+
+behaviour_test(Tests, Conf, Opts) ->
+    {ok, _} = xref:start(s, Opts),
+    add_modules(Tests, Conf),
+    case lists:keyfind(xref_mode, 1, Opts) of
+        {xref_mode, modules} ->
+            UndefinedFunctionCalls = [];
+        _ ->
+            {ok, UndefinedFunctionCalls} =
+                xref:analyze(s, undefined_function_calls)
+    end,
+    {ok, ExportsNotUsed} = xref:analyze(s, exports_not_used),
+    xref:stop(s),
+    {UndefinedFunctionCalls, ExportsNotUsed}.
+
+add_modules([], _Conf) ->
+    ok;
+add_modules([{Mod, Test} |Tests], Conf) ->
+    Dir = ?copydir,
+    Name = atom_to_list(Mod),
+    File = fname(Dir, Name ++ ".erl"),
+    MFile = fname(Dir, Name),
+    Beam = fname(Dir, Name ++ ".beam"),
+    ok = file:write_file(File, Test),
+    {ok, Mod} = compile:file(File, [debug_info,{outdir,Dir}]),
+    {ok, Mod} = xref:add_module(s, MFile),
+    check_state(s),
+    ok = file:delete(File),
+    ok = file:delete(Beam),
+    add_modules(Tests, Conf).
+
 %%%
 %%% Utilities
 %%%
@@ -2826,24 +2904,3 @@ add_erts_code_path(KernelPath) ->
                     [KernelPath]
             end
     end.
-
-behaviour_info_t(Config) ->
-    bi_t(_Module = bi,
-         _IsExportNotUsed = false,
-         Config).
-
-fake_behaviour_info_t(Config) ->
-    bi_t(_Module = no_bi,
-         _IsExportNotUsed = true,
-         Config).
-
-bi_t(Module, IsExportNotUsed, Conf) ->
-    LibTestDir = fname(?copydir, "lib_test"),
-    XRefServer = s,
-    {ok, Module} = compile:file(fname(LibTestDir, Module),
-                                [debug_info, {outdir, LibTestDir}]),
-    {ok, _} = start(XRefServer),
-    {ok, Module} = xref:add_module(XRefServer, fname(LibTestDir, Module)),
-    {ok, MFAs} = xref:analyze(XRefServer, exports_not_used),
-    true = lists:member({Module, behaviour_info, 1}, MFAs) =:= IsExportNotUsed,
-    _ = xref:stop(XRefServer).