janestreet / janestreet/ppx_fields_conv

Embed errors in the AST instead of raising

Open
#10 3 comments 0 reactions 0 assignees View on GitHub
forwarded-to-js-devs
Dominant language
OCaml
Stars
63
Forks
12
PR merge metrics
No merged PRs in 30d

Description

Currently, when `ppx_fields_conv` 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_fields_conv` 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:
```ocaml
type invalid1 = int [@@deriving fields]

type invalid2 = int [@@deriving fields]

type valid = { i : int } [@@deriving fields]
let _ = i
```
would report several errors:
- `Unsupported use of fields (you can only use it on records` for `invalid1`: the right error
- 'Unbound value i`

The error for `invalid2` is not shown, since the rewriting is cancelled when the exception is raised.

You can find more information about error reporting in PPXs in [this section](https://ocaml.org/p/ppxlib/latest/doc/good-practices.html#handling_errors) of the ppxlib manual.

:question: Would you be willing to accept contributions to this issue? I'm considering assigning its resolution as part of an [outreachy](https://www.outreachy.org/) internship: see more information [here](https://github.com/ocaml-ppx/ppxlib/issues/389).

Contributor guide

Open the contributing guide

Research direction

Start by reading the ppxlib manual's error-handling section and inspect how ppx_fields_conv currently uses raise_errorf. Use the invalid1/invalid2/valid example as the behavioral check: errors from both invalid declarations should be preserved, later derivers should still run, and unrelated noise should be avoided.

Written by the indexing model from the issue text.

Assessment

Tech stack
ocaml
Domain
compilers
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.