compiler: Improve error messages for using a function with wrong arity

[lucioleKi]

When a function is used with wrong arity, the compiler will try to suggest a defined function with the same name but a different arity. For example, given the following module:

-module(typos).
-export([t/0]).
bar(A) -> A.
bar(A,A,A) -> A.
bar(A,A,A,A) -> A.
t() -> bar(0, 0).

The compiler will emit the following message:

typo.erl:6:12: function bar/2 undefined, did you mean bar/1,3,4?
%   6|     t() -> bar(0, 0).
%    |            ^

Error types that are extended by this change: bad_inline, undefined_nif, bad_nowarn_unused_function, bad_nowarn_bif_clash, undefined_function.

Using a function with wrong arity has higher precedence than having a typo in the function name. If the compiler can find a defined function with the same name but a different arity, it will not suggest a defined function with a close-enough name, regardless of arity.

[github-actions]

CT Test Results

    2 files     96 suites   1h 8m 53s ⏱️
2 173 tests 2 124 ✅ 48 💤 1 ❌
2 534 runs  2 483 ✅ 50 💤 1 ❌

For more details on these failures, see this check.

Results for commit ab85bb7.

♻️ This comment has been updated with latest results.

To speed up review, make sure that you have read Contributing to Erlang/OTP and that all checks pass.

See the TESTING and DEVELOPMENT HowTo guides for details about how to run test locally.

Artifacts

• Complete CT logs (Download Logs)
• HTML Documentation (Download HTML Docs)
• Windows Installer

// Erlang/OTP Github Action Bot

[jhogberg]

Nitpick:

Suggested change

	PossibleAs = [A || {FName, A} <- FAList, FName =:= Name],
	PossibleAs = [A || {FName, A} <:- FAList, FName =:= Name],

Perhaps we should also try to pick the closest arity rather than the first one that pops up, e.g. PossibleAs = sort(fun(A, B) -> abs(A - Arity) =< abs(B - Arity) end, PossibleAs0), or by making a most_possible_function variant of most_possible_string.

[frej]

Why just suggest one alternative, if there are multiple functions with the same name, isn't it equally likely that that the the intended function was arity+1 as arity-1? For example, both g++ and clang++, give you a list of candidates when there are multiple "sane" choices.

[jhogberg]

"did you mean foo/3,4,5?" sounds good.

[frej]

Yes.

If we start guessing and proposing things, we might do as the C-compilers and have the compiler produce errors, warnings and notes. The undefined function would be the error, and each suggested function would be a note. That way you can jump to the proposed function by just a click/keystroke in your editor/IDE. But this is outside the scope of this MR.

[bjorng]

Overall looks good to me. I have a few suggestions for improvements.

[bjorng]

The following change will eliminate duplicated code and overlong lines:

Suggested change

	PossibleAs = [A || {F, A} <- DefFunctions, F =:= Name],
	case PossibleAs of
	[] ->
	case most_possible_string(Name, PossibleFs) of
	[] ->
	add_error(Anno, {undefined_function,FA}, St);
	GuessF ->
	add_error(Anno, {undefined_function,FA,GuessF,Arity}, St)
	end;
	[GuessA|_] ->
	add_error(Anno, {undefined_function,FA,Name,GuessA}, St)
	func_location_error(undefined_function,
	[{FA,Anno}],
	St,
	DefFunctions)

[bjorng]

For symmetry, I suggest the following change:

Suggested change

	add_error(Anno, {Type,F,GuessF,Arity}, St0)
	add_error(Anno, {Type,F,{GuessF,Arity}}, St0)

You must also do a similar change in line 1699, in format_error/1, and in the test suite.

[bjorng]

There does not seem to be any tests of a bad dialyzer option with a suggestion.

[bjorng]

The new messages look good. However, there is an inconsistency in the internal representation of the message:

[{{1,22},erl_lint,{undefined_function,{bar,1},{"baz",1}}},
 {{1,22},erl_lint,{undefined_function,{foo,2},{foo,[1,3]}}}]

That is, function names are lists if they originated from a misspelling, but atoms if they originated from wrong arity.

I think they should always be represented as atoms. That is also consistent with how functions are represented in other places.

The easiest way to achieve this is to change the end of most_possible_string/2 like so:

            case MaxSim > SufficientlySimilar of
                true -> list_to_atom(GuessName);
                false -> []
            end

Also, I think that for consistency the arity should always be in a list:

[{{1,22},erl_lint,{undefined_function,{bar,1},{baz,[1]}}},
 {{1,22},erl_lint,{undefined_function,{foo,2},{foo,[1,3]}}}]

[bjorng]

This clause is not covered, and I don't think it is possible to cover it.