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

Handle parse error for bad head exposes list #7408

Merged
merged 5 commits into from
Jan 3, 2025

Conversation

jaredramirez
Copy link
Contributor

@jaredramirez jaredramirez commented Dec 23, 2024

This PR fixes #7246

It improves the parse error message if you have an invalid exposes list in the module header. The new error is:

── WEIRD MODULE NAME in /code/proj/Main.roc ────────────────────────────────────

 I am partway through parsing a header, but I got stuck here:

    1│  module foobar []
               ^

I was expecting an `exposes` list like

    [Animal, default, tame]

To do this, I had to add a branch to handle EExposes::ListStart in the function to_exposes_report.

I originally wanted to add more context to this error message based on whether the header in question was an app or a module. However, that would've required adding additional metadata to EHeader, which seemed out of scope.

While there, I noticed the branches for EExposes::Identified and EExposes::ListEnd are the same with a TODO to split them up, so I looked into that as well. As it turns out, that would require tweaking parsing which seems out of scope, so I updated the TODO with more context and left it as is.

@jaredramirez jaredramirez force-pushed the jared/improve-module-error branch from 82680e3 to 99377b0 Compare December 24, 2024 15:47
@jaredramirez
Copy link
Contributor Author

jaredramirez commented Dec 26, 2024

Looks like in the first CI run, a test timed out:

test cli_tests::test_benchmarks::base64 has been running for over 60 seconds

I went ahead and pulled in the latest commits from main, but not sure is that would fix this timeout

@jaredramirez jaredramirez changed the title Improve module parse error Handle parse error for bad head exposes list Dec 29, 2024
@jaredramirez jaredramirez marked this pull request as draft December 30, 2024 17:43
@jaredramirez jaredramirez force-pushed the jared/improve-module-error branch from 5bbc26a to c28d6b9 Compare December 31, 2024 00:02
@jaredramirez
Copy link
Contributor Author

Okay @smores56, this is now ready. I ended up undoing the change to handle ListEnd problems into its own branch, because it would've required tweaking some parsing code, and that felt out of scope. I added a comment in the code and updated the PR description.

I've also run all tests, formatting, etc locally and all looks good, so this should pass CI now.

@jaredramirez jaredramirez marked this pull request as ready for review December 31, 2024 00:08
@smores56
Copy link
Collaborator

Thanks for the change!

@smores56 smores56 enabled auto-merge December 31, 2024 01:43
@smores56 smores56 merged commit 786488f into roc-lang:main Jan 3, 2025
19 checks passed
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.

Unhandled parse error when module is followed by anything but a list
2 participants