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

Start reporting errors from the translator #318

Merged
merged 2 commits into from
Nov 1, 2024
Merged

Conversation

st0012
Copy link
Member

@st0012 st0012 commented Nov 1, 2024

Motivation

First step to #309

I'd like to approach this task with these steps:

  1. Since node locations are important for autocorrection, we first make sure parse-trees still match in errored cases. So at this time we don't report errors at the same details & location as Sorbet does, as long as there are errors and they are captured in the fixtures, which we can used for comparison later.
  2. Once we got all the parse-trees right, we compare all fixtures under prism_regression/error_recovery/ with testdata/parser/error_recovery/ and try to enrich/calibrate our error reporting.
    • This doesn't include error: Hint: but only real syntax errors
  3. Finally, we start working on error: Hint

With that in mind, in this PR:

  1. To make it easier to compare with Sorbet's tests, I added error_recovery folder to host error related fixture files as well
  2. To make location tests work with 1, which means to accept error_recovery/assign without converting it to error_recovery_assign (can't easily be done in tasks.json), I updated test/prism_location_test.bzl too
  3. I added a very basic Translator::reportError function and updated the PM_MISSING_NODE case to start reporting basic errors when parsing invalid Ruby code, which starts with assign.rb

Test plan

See included automated tests.

By not replacing `/` with `_`, the test naming structure is more consistent
with normal regression tests. But more importantly, it allows us to run
location tests that are under a folder like `error_recovery/assign`, more
easily.
The error location doesn't match what Sorbet currently generates. But the
idea is to first make parse trees match, accumulate more recovery tests,
and then come up with a strategy to generate errors that match locations.
Copy link

@egiurleo egiurleo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! This is an awesome start. I'm excited to see how it evolves as we handle more errors, and of course curious to see what we have to do to properly handle locations 😬

@st0012 st0012 merged commit 6eb5a76 into prism Nov 1, 2024
1 check passed
@st0012 st0012 deleted the error-recovery-assign branch November 1, 2024 20:51
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.

2 participants