erl_lint: Fix edge cases in unknown export warnings

Fixes erlang#9267
jcpetruzza · Jan 23, 2025 · 13f49f9 · 13f49f9
1 parent 9a603b4
commit 13f49f9
Show file tree

Hide file tree

Showing 2 changed files with 67 additions and 11 deletions.
diff --git a/lib/stdlib/src/erl_lint.erl b/lib/stdlib/src/erl_lint.erl
@@ -175,6 +175,7 @@ value_option(Flag, Default, On, OnVal, Off, OffVal, Opts) ->
                behaviour=[],                    %Behaviour
                exports=gb_sets:empty()	:: gb_sets:set(fa()),%Exports
                imports=[] :: orddict:orddict(fa(), module()),%Imports
+               remote_self_calls=#{} :: #{ fa() => gb_sets:set() },
                compile=[],                      %Compile flags
                records=maps:new()               %Record definitions
                    :: #{atom() => {anno(),Fields :: term()}},
@@ -1249,7 +1250,8 @@ post_traversal_check(Forms, St0) ->
     StG = check_dialyzer_attribute(Forms, StF),
     StH = check_callback_information(StG),
     StI = check_nifs(Forms, StH),
-    check_removed(Forms, StI).
+    StJ = check_unexported_functions(StI),
+    check_removed(Forms, StJ).
 
 %% check_behaviour(State0) -> State
 %% Check that the behaviour attribute is valid.
@@ -1652,6 +1654,30 @@ check_nifs(Forms, St0) ->
     DefFunctions1 = gb_sets:to_list(DefFunctions),
     func_location_error(undefined_nif, Bad, St1, DefFunctions1).
 
+check_unexported_functions(#lint{callbacks=Cs,
+                                 optional_callbacks=OCs,
+                                 exports=Es0}=St) ->
+    Es = case Cs =/= #{} orelse OCs =/= #{} of
+            true -> gb_sets:add({behaviour_info, 1}, Es0);
+            false -> Es0
+         end,
+    maps:fold(fun check_unexported_functions_1/3,
+              St#lint{exports=Es},
+              St#lint.remote_self_calls).
+
+check_unexported_functions_1({F, A}=Key, Annos, Acc0) ->
+    #lint{module=M,exports=Es} = Acc0,
+    case not gb_sets:is_element(Key, Es) of
+        true ->
+            gb_sets:fold(fun(Anno, Acc) ->
+                                 add_warning(Anno,
+                                             {unexported_function, {M, F, A}},
+                                             Acc)
+                         end, Acc0, Annos);
+        false ->
+            Acc0
+    end.
+
 nowarn_function(Tag, Opts) ->
     ordsets:from_list([FA || {Tag1,FAs} <- Opts,
                              Tag1 =:= Tag,
@@ -2760,7 +2786,7 @@ expr({'fun',Anno,Body}, Vt, St) ->
                 false -> {[],call_function(Anno, F, A, St)}
             end;
         {function, {atom, _, M}, {atom, _, F}, {integer, _, A}} ->
-            {[], check_unexported_function(Anno, M, F, A, St)};
+            {[], check_remote_self_call(Anno, M, F, A, St)};
         {function,M,F,A} ->
             expr_list([M,F,A], Vt, St)
     end;
@@ -2785,7 +2811,7 @@ expr({call,Anno,{remote,_Ar,{atom,_Am,M},{atom,Af,F}},As}, Vt, St0) ->
     St1 = keyword_warning(Af, F, St0),
     St2 = check_remote_function(Anno, M, F, As, St1),
     St3 = check_module_name(M, Anno, St2),
-    St4 = check_unexported_function(Anno, M, F, length(As), St3),
+    St4 = check_remote_self_call(Anno, M, F, length(As), St3),
     expr_list(As, Vt, St4);
 expr({call,Anno,{remote,_Ar,M,F},As}, Vt, St0) ->
     St1 = keyword_warning(Anno, M, St0),
@@ -3021,17 +3047,23 @@ is_valid_call(Call) ->
 
 %% Raises a warning if we're remote-calling an unexported function (or
 %% referencing it with `fun M:F/A`), as this is likely to be unintentional.
-check_unexported_function(Anno, M, F, A,
-                          #lint{module=M,
-                                compile=Opts,
-                                exports=Es} = St) ->
+check_remote_self_call(Anno, M, F, A,
+                       #lint{module=M,
+                             compile=Opts,
+                             exports=Es,
+                             remote_self_calls=Rsc0} = St) ->
     case (is_warn_enabled(unexported_function, St)
           andalso (not lists:member(export_all, Opts))
           andalso (not gb_sets:is_element({F, A}, Es))) of
-        true -> add_warning(Anno, {unexported_function, {M, F, A}}, St);
-        false -> St
+        true ->
+            Locs0 = maps:get({F, A}, Rsc0, gb_sets:empty()),
+            Locs = gb_sets:add_element(Anno, Locs0),
+            Rsc = Rsc0#{ {F, A} => Locs },
+            St#lint{remote_self_calls=Rsc};
+        false ->
+            St
     end;
-check_unexported_function(_Anno, _M, _F, _A, St) ->
+check_remote_self_call(_Anno, _M, _F, _A, St) ->
     St.
 
 %% record_def(Anno, RecordName, [RecField], State) -> State.

diff --git a/lib/stdlib/test/erl_lint_SUITE.erl b/lib/stdlib/test/erl_lint_SUITE.erl
@@ -1222,7 +1222,31 @@ unused_function(Config) when is_list(Config) ->
                 32*X.
            ">>,
            {[]}, %% Tuple indicates no 'export_all'.
-           {warnings,[{{9,15},erl_lint,{unused_function,{flurb,1}}}]}}],
+           {warnings,[{{9,15},erl_lint,{unused_function,{flurb,1}}}]}},
+
+          %% GH-9267: references prior to the -export directive caused false
+          %% positives.
+          {func6,
+           <<"-record(blurf, {a = ?MODULE:t() }).
+              -export([t/0]).
+
+              t() ->
+                 #blurf{a=hello}.
+           ">>,
+           {[]}, %% Tuple indicates no 'export_all'.
+           []},
+
+          %% GH-9267: references prior to the -export directive caused false
+          %% positives.
+          {func7,
+           <<"-callback foo() -> ok.
+              -export([t/0]).
+
+              t() ->
+                 ?MODULE:behaviour_info(callbacks).
+           ">>,
+           {[]}, %% Tuple indicates no 'export_all'.
+           []}],
 
     [] = run(Config, Ts),
     ok.