Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support rich compiler error messages #2783

Merged
merged 3 commits into from
May 24, 2023

Conversation

ferd
Copy link
Collaborator

@ferd ferd commented Apr 8, 2023

This is rather straightforward as an implementation: given any error message that contains column information, read the source file and find that line. Display it, and indent a cursor underneath it that points to the column.

Then we can just show the same error message as before.

In practice, given the module:

-module(fake_mod).

-export([diagnostic/1]).

diagnostic(A) ->
    X = add(5 / 0),
    {X,X}.

add(X) -> X.

add(X, Y) -> X + Y.

Calling rebar3 compile yields:

...
===> Compiling apps/rebar/src/fake_mod.erl failed
   ┌─ apps/rebar/src/fake_mod.erl:
   |
 5 |  diagnostic(A) ->
   |             ^-- variable 'A' is unused

   ┌─ apps/rebar/src/fake_mod.erl:
   |
 6 |      X = add(5 / 0),
   |                ^-- evaluation of operator '/'/2 will fail with a 'badarith' exception

    ┌─ apps/rebar/src/fake_mod.erl:
    |
 11 |  add(X, Y) -> X + Y.
    |  ^-- function add/2 is unused

and in a terminal supporting color output:

This is friendlier than what we had before for sure. Adding color could be an extension to this if we find the format useful.

By default, this format is turned off, but can be turned on optionally by configuring values with

{compiler_error_format, rich}.

Compilers that want to support it must switch their calls from rebar_compiler:error_tuple(Source, Errors, Warnings, Opts) to rebar_compiler:error_tuple(Source, Errors, Warnings, Config, Opts), which allows the configuration to work and go for non-default values.

The intent of this PR is to put the initial scaffolding in place to gradually get nicer and richer error formats for users, without necessarily harming the ability of tools to still work with them (via config switches).

@ferd ferd force-pushed the try-fancy-compiler-error-msgs branch from 0b15d9a to 5c768be Compare April 8, 2023 23:35
@tsloughter
Copy link
Collaborator

Nice, much more straightforward than I expected.

@ferd
Copy link
Collaborator Author

ferd commented Apr 13, 2023

Do you think it's worth driving this forward as-is or we'd like to have fancier output?

I also imagine we'll need a switch for machine parsing to reuse the old context-free messages

@ferd ferd marked this pull request as ready for review May 15, 2023 20:47
@ferd ferd force-pushed the try-fancy-compiler-error-msgs branch 2 times, most recently from 07df8f2 to 73ba8c4 Compare May 20, 2023 16:37
@ferd ferd changed the title Attempt to support legible compiler error messages Support rich compiler error messages May 20, 2023
@ferd ferd force-pushed the try-fancy-compiler-error-msgs branch 3 times, most recently from 4a4e3c8 to e650b56 Compare May 21, 2023 00:50
@ferd
Copy link
Collaborator Author

ferd commented May 22, 2023

Just pushed an extra commit that attempts a format closer to the original issue and also colorizes the output

image

@ferd ferd force-pushed the try-fancy-compiler-error-msgs branch from c44d29a to 415a2ab Compare May 22, 2023 17:21
@ferd ferd requested a review from tsloughter May 23, 2023 00:17
@eproxus
Copy link
Contributor

eproxus commented May 23, 2023

Has there been any consideration using more fancy box drawing or unicode characters?

Rust uses carets for the whole error column range:

2 |     vec![(), ()].iter().sum::<i32>();
  |     ^^^^^^^^^^^^^^^^^^^ --- required by a bound introduced by this call

I think that looks slightly better and it is easier to see what exactly is marked as a problem.

@eproxus
Copy link
Contributor

eproxus commented May 23, 2023

Here's an example with box drawing characters (looks even better in a terminal):

    ┌╴ apps/rebar/src/fake_mod.erl:
    │
 25 │      A = foo,
    │      ╰── variable 'A' is unused

@ferd
Copy link
Collaborator Author

ferd commented May 23, 2023

I could use box drawing chars easily, but I wouldn't use the carets for the whole expression, only because I have no width information about the error, just the column it starts on.

The Color highlight is done with a sort of messy regex trick, and it's unlikely to carry well to other compilers we support. If the Color is a bit wrong but the arrow is good, it sort of limits the scope of error.

I'll update the PR to use the fancier characters.

@ferd
Copy link
Collaborator Author

ferd commented May 23, 2023

image

the arrow won't go bold but that's okay

ferd added 3 commits May 23, 2023 15:06
This is rather straightforward as an implementation: given any error
message that contains column information, read the source file and find
that line. Display it, and indent a cursor underneath it that points to
the column.

Then we can just show the same error message as before.

In practice, given the module:

    -module(rebar_fake).

    -export([diagnostic/1]).

    diagnostic(A) ->
        X = add(5 / 0),
        {X,X}.

    add(X) -> X.

    add(X, Y) -> X + Y.

Calling rebar3 compile yields:

    ...
    ===> Compiling rebar
    ===> Compiling apps/rebar/src/rebar_fake.erl failed
    apps/rebar/src/rebar_fake.erl:5:12:
      diagnostic(A) ->
                 ^-- variable 'A' is unused
    apps/rebar/src/rebar_fake.erl:6:15:
          X = add(5 / 0),
                    ^-- evaluation of operator '/'/2 will fail with a 'badarith' exception
    apps/rebar/src/rebar_fake.erl:11:1:
      add(X, Y) -> X + Y.
      ^-- function add/2 is unused

This is friendlier than what we had before for sure. Adding color could
be an extension to this if we find the format useful.

The rebar compiler output richness is conditional and configurable, and we
default to the minimal normal one to  make sure it won't break all sorts of tooling.

Rich errors also happen to support to xrl and yrl compilers
This will help testability and to better self-contain the implementation
This now looks like:

    ...
    ===> Compiling apps/rebar/src/fake_mod.erl failed
       ┌─ apps/rebar/src/fake_mod.erl:
       │
     5 │  diagnostic(A) ->
       │             ╰── variable 'A' is unused

       ┌─ apps/rebar/src/fake_mod.erl:
       │
     6 │      X = add(5 / 0),
       │                ╰── evaluation of operator '/'/2 will fail with a 'badarith' exception

        ┌─ apps/rebar/src/fake_mod.erl:
        │
     11 │  add(X, Y) -> X + Y.
        │  ╰── function add/2 is unused

And supports colors with a weak heuristic based on regexes that I expect
we'll need to fix and expand later to cover more compiler-specific
rules.
@ferd ferd force-pushed the try-fancy-compiler-error-msgs branch from 0039235 to dbbb73a Compare May 23, 2023 15:09
@ferd ferd merged commit a05f59b into erlang:main May 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants