Skip to content

Commit

Permalink
Better check for funs referring to imports
Browse files Browse the repository at this point in the history
The linting of 'fun f/n' relied on marking f/n as used and later
getting a report if function f/n did not exist, except if f/n was not
in the list of local functions (could still be local but auto
generated) and was listed in auto-imports (erlang:f/n) and the import
was not suppressed. This allowed explicit imports of names such as
'max/2' to slip through with a warning, assuming that they were
auto-imports, and the expander would then expand them like an explicit
import.

This change avoids marking f/n as a used local if it is listed as an
import, makes it an error to have a 'fun f/n' that refers to an
explicit import, and furthermore checks the reference f/n just like a
call f(X1, ... Xn) so that we get the same warnings for deprecated
functions etc., for example 'fun now/0'.
  • Loading branch information
richcarl committed Dec 25, 2024
1 parent a34b2e7 commit 65f0e48
Showing 1 changed file with 19 additions and 7 deletions.
26 changes: 19 additions & 7 deletions lib/stdlib/src/erl_lint.erl
Original file line number Diff line number Diff line change
Expand Up @@ -346,6 +346,8 @@ format_error_1({redefine_function,{F,A}}) ->
{~"function ~tw/~w already defined", [F,A]};
format_error_1({define_import,{F,A}}) ->
{~"defining imported function ~tw/~w", [F,A]};
format_error_1({fun_import,{F,A}}) ->
{~"creating a fun from imported name ~tw/~w is not allowed", [F,A]};
format_error_1({unused_function,{F,A}}) ->
{~"function ~tw/~w is unused", [F,A]};
format_error_1({unexported_function, MFA}) ->
Expand Down Expand Up @@ -2751,13 +2753,23 @@ expr({'fun',Anno,Body}, Vt, St) ->
%% It is illegal to call record_info/2 with unknown arguments.
{[],add_error(Anno, illegal_record_info, St)};
{function,F,A} ->
%% BifClash - Fun expression
%% N.B. Only allows BIFs here as well, NO IMPORTS!!
case ((not is_local_function(St#lint.locals,{F,A})) andalso
(erl_internal:bif(F, A) andalso
(not is_autoimport_suppressed(St#lint.no_auto,{F,A})))) of
true -> {[],St};
false -> {[],call_function(Anno, F, A, St)}
St1 = case is_imported_function(St#lint.imports,{F,A}) of
true ->
add_error(Anno, {fun_import,{F,A}}, St);
false ->
%% check function use like for a call
As = lists:duplicate(A, undefined), % dummy args
check_call(Anno, F, As, Anno, St)
end,
%% do not mark as used as a local function if listed as
%% imported (either auto-imported or explicitly)
case not is_local_function(St1#lint.locals,{F,A}) andalso
(is_imported_function(St1#lint.imports,{F,A})
orelse
(erl_internal:bif(F, A) andalso
not is_autoimport_suppressed(St1#lint.no_auto,{F,A}))) of
true -> {[],St1};
false -> {[],call_function(Anno, F, A, St1)}
end;
{function, {atom, _, M}, {atom, _, F}, {integer, _, A}} ->
{[], check_unexported_function(Anno, M, F, A, St)};
Expand Down

0 comments on commit 65f0e48

Please sign in to comment.