File 5871-erl_parse-Provide-friendlier-head-mismatch... of Package erlang

Overview Repositories Revisions Requests Users Attributes Meta

File 5871-erl_parse-Provide-friendlier-head-mismatch-errors.patch of Package erlang (Revision 371fc947ce89baa340e0441f200e4667)

Currently displaying revision 371fc947ce89baa340e0441f200e4667 , Show latest

From d74e9c575075d49a749a30079b96be8d453dca55 Mon Sep 17 00:00:00 2001
From: Johannes Christ <jc@jchri.st>
Date: Thu, 8 Jun 2023 00:34:39 +0200
Subject: [PATCH] erl_parse: Provide friendlier "head mismatch" errors

Given the following module:

%% mismatch.erl
    -module(mismatch).
    -compile([export_all, nowarn_export_all]).

test_it(ok) -> ok;

verify_it(true) -> ok;
    verify_it(false) -> {error, was_false}.
    verify_it(false, Reason) -> {error, Reason}.

On master, running `erlc mismatch.erl`  currently reports:

mismatch.erl:6:1: head mismatch
    %    6| verify_it(true) -> ok;
    %     | ^

The previous clause that the parser is comparing to may not be obvious:
The current compiler warning points to `verify_it/1`, whose two clauses
are terminated properly. When more code is inbetween these two functions
(think documentation, specs, comments) the time needed to find the
mismatched clause can be a lot higher. I also believe that this will
make the error easier to understand for people new to Erlang.

With this commit, the compiler will output the following:

mismatch.erl:6:1: head mismatch: previous function test_it/1 is distinct from verify_it/1. Is the semicolon in test_it/1 unwanted?
    %    6| verify_it(true) -> ok;
    %     | ^

For functions that are defined with the same name but distinct arities,
a separate error message is used, since the possibility of missed or
superfluous arguments also arises here. The following module illustrates
it:

%% mismatch2.erl
    -module(mismatch2).
    -compile([export_all, nowarn_export_all]).

verify_it(false) -> {error, was_false};
    verify_it(false, Reason) -> {error, Reason}.

The following error message is provided in this case:

mismatch2.erl:6:1: head mismatch: function verify_it with arities 1 and 2 is regarded as two distinct functions. Is the number of arguments incorrect or is the semicolon in verify_it/1 unwanted?
    %    6| verify_it(false, Reason) -> {error, Reason}.
    %     | ^

For tools matching on the output of the compiler warnings, this is a
breaking change. I have so far only found Erlang LS which matches on
this [1]. Adding support for the extended information should be trivial.

[1]: https://github.com/erlang-ls/erlang_ls/blob/e315f9801d60e1ccbbfd444f32a33d00b0f6795d/apps/els_lsp/src/els_compiler_diagnostics.erl#L614
---
 lib/compiler/test/error_SUITE.erl             | 20 +++++++++++---
 .../head_mismatch_same_function_name.erl      | 26 +++++++++++++++++++
 lib/stdlib/src/erl_parse.yrl                  | 17 ++++++++++--
 3 files changed, 58 insertions(+), 5 deletions(-)
 create mode 100644 lib/compiler/test/error_SUITE_data/head_mismatch_same_function_name.erl

diff --git a/lib/compiler/test/error_SUITE.erl b/lib/compiler/test/error_SUITE.erl
index f82fde9e8b..3250ebb3d6 100644
--- a/lib/compiler/test/error_SUITE.erl
+++ b/lib/compiler/test/error_SUITE.erl
@@ -22,8 +22,8 @@
 
 -export([all/0, suite/0,groups/0,init_per_suite/1, end_per_suite/1, 
 	 init_per_group/2,end_per_group/2,
-	 head_mismatch_line/1,warnings_as_errors/1, bif_clashes/1,
-	 transforms/1,maps_warnings/1,bad_utf8/1,bad_decls/1]).
+	 head_mismatch_line/1, head_mismatch_same_function_name/1, warnings_as_errors/1,
+	 bif_clashes/1, transforms/1,maps_warnings/1,bad_utf8/1,bad_decls/1]).
 
 %% Used by transforms/1 test case.
 -export([parse_transform/2]).
@@ -35,7 +35,8 @@ all() ->
 
 groups() -> 
     [{p,test_lib:parallel(),
-      [head_mismatch_line,warnings_as_errors,bif_clashes,
+      [head_mismatch_line,head_mismatch_same_function_name,
+       warnings_as_errors,bif_clashes,
        transforms,maps_warnings,bad_utf8,bad_decls]}].
 
 init_per_suite(Config) ->
@@ -177,6 +178,19 @@ bif_clashes(Config) when is_list(Config) ->
 head_mismatch_line(Config) when is_list(Config) ->
     [E|_] = get_compilation_errors(Config, "head_mismatch_line"),
     {{26,1}, Mod, Reason} = E,
+    ("head mismatch: previous function foo/1 is distinct from bar/1. "
+     "Is the semicolon in foo/1 unwanted?") = lists:flatten(Reason),
+    Mod:format_error(Reason),
+    ok.
+
+%% Tests that a head mismatch with the same function name reports a different error from above.
+%% https://github.com/erlang/otp/pull/7383#issuecomment-1586564294
+head_mismatch_same_function_name(Config) when is_list(Config) ->
+    [E|_] = get_compilation_errors(Config, "head_mismatch_same_function_name"),
+    {{25,1}, Mod, Reason} = E,
+    ("head mismatch: function foo with arities 1 and 2 is regarded as "
+     "two distinct functions. Is the number of arguments incorrect "
+     "or is the semicolon in foo/1 unwanted?") = lists:flatten(Reason),
     Mod:format_error(Reason),
     ok.
 
diff --git a/lib/compiler/test/error_SUITE_data/head_mismatch_same_function_name.erl b/lib/compiler/test/error_SUITE_data/head_mismatch_same_function_name.erl
new file mode 100644
index 0000000000..2e02ca42c9
--- /dev/null
+++ b/lib/compiler/test/error_SUITE_data/head_mismatch_same_function_name.erl
@@ -0,0 +1,26 @@
+%%
+%% %CopyrightBegin%
+%% 
+%% Copyright Ericsson AB 1998-2016. 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.
+%% You may obtain a copy of the License at
+%%
+%%     http://www.apache.org/licenses/LICENSE-2.0
+%%
+%% Unless required by applicable law or agreed to in writing, software
+%% distributed under the License is distributed on an "AS IS" BASIS,
+%% WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+%% See the License for the specific language governing permissions and
+%% limitations under the License.
+%% 
+%% %CopyrightEnd%
+-module(head_mismatch_same_function_name).
+
+-export([foo/1, foo/2]).
+
+foo({A, B}) ->
+    A + B;
+foo(Other, State) ->
+    {Other, State}.
diff --git a/lib/stdlib/src/erl_parse.yrl b/lib/stdlib/src/erl_parse.yrl
index bef858cdc5..a1e41958a4 100644
--- a/lib/stdlib/src/erl_parse.yrl
+++ b/lib/stdlib/src/erl_parse.yrl
@@ -1499,8 +1499,21 @@ check_clauses(Cs, Name, Arity) ->
     [case C of
          {clause,A,N,As,G,B} when N =:= Name, length(As) =:= Arity ->
              {clause,A,As,G,B};
-         {clause,A,_N,_As,_G,_B} ->
-             ret_err(A, "head mismatch")
+         {clause,A,N,As,_G,_B} when N =:= Name ->
+             Detail = io_lib:format(
+                 "head mismatch: function ~s with arities ~w and ~w is "
+                 "regarded as two distinct functions. Is the number of "
+                 "arguments incorrect or is the semicolon in ~s/~w unwanted?",
+                 [Name, Arity, length(As), Name, Arity]
+             ),
+             ret_err(A, Detail);
+         {clause,A,N,As,_G,_B} ->
+             Detail = io_lib:format(
+                 "head mismatch: previous function ~s/~w is distinct from ~s/~w. "
+                 "Is the semicolon in ~s/~w unwanted?",
+                 [Name, Arity, N, length(As), Name, Arity]
+             ),
+             ret_err(A, Detail)
      end || C <- Cs].
 
 build_try(A,Es,Scs,{Ccs,As}) ->
-- 
2.35.3

Places

File 5871-erl_parse-Provide-friendlier-head-mismatch-errors.patch of Package erlang (Revision 371fc947ce89baa340e0441f200e4667)

Places