Embed errors in the AST instead of raising

[panglesd]

Currently, when ppx_inline_test encounters an error, it uses the raise_errorf function to raise a located error.

The exception is caught by ppxlib, which in this case:

• Catch the error,
• stops the rewriting process
• add the error (as a [%%%ocaml.error ...] extension node) to the last valid ast
• Use the resulting AST

The interruption of the rewriting is quite bad for the user experience! The implication for the users are:

• Since ppx_inline_test runs at the "context-free" phase, the "last valid AST" is before the context-free phase. So, no other derivers/extenders get run, which generates a lot of noise in the errors (such as "uninterpreted extensions" or "unbound identifiers")
• Only one (meaningful) error from your PPX is reported at a time.

Example

For instance:

let%test ("invalid1" [@tags "invalid"]) = false

let%test ("invalid2" [@tags "invalid"]) = false

let%test ("valid" [@tags "no-js"]) = false

would report several errors:

• invalid is not a valid tag for inline tests for invalid1: the right error
• uninterpreted extension test for invalid2: the wrong error, it should be as for invalid1
• uninterpreted extension test for valid: an error when there should not be one

You can find more information about error reporting in PPXs in this section of the ppxlib manual.

❓ Would you be willing to accept contributions to this issue? I'm considering assigning its resolution as part of an outreachy internship: see more information here.

[panglesd]

Note that we decided that we will change the ppxlib behaviour regarding the handling of exceptions, to match the current use of raise_errorf in PPXs.
Catching an exception will no longer stop the rewriting process. So, the example I gave in the original issue is not relevant any more.

However, embedding errors can still have advantages: It allows reporting multiple errors, while still outputting valid AST for the part that were successful. In the case of this PPX, the example could be rewritten as:

let%test ("invalid" [@tags "invalid1", "invalid2"]) = false

which, when raising instead of embedding errors, would only report a single tag error, instead of all.

[fikayo-sanni]

Hello @panglesd. I am an outreachy applicant.

This issue looks like a good first bite into the ppxlib error reporting improvement project.

At the moment, I am in the process of understanding how the system works enough to ask competent questions as the need arise.

I will be reaching out to you both, here and on discord. Looking forward to a great collaboration

Best Regards