-
-
Notifications
You must be signed in to change notification settings - Fork 661
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
Show helpful error message when calling a function in a constant value #3130
Show helpful error message when calling a function in a constant value #3130
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you. Could you add tests for calling functions from other modules please? For example list.map(items, to_string)
.
@lpil Turns out, there already was such a test case. I've updated the code to make the error message look the same whether or not the function is from another module. (i.e., the error highlights the whole function call from beginning of name to end of arguments) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a bit unsure about using the expression function args parsing method here as it emits errors relating to expressions, not constants. This will be inaccurate at times.
Previously we stopped when we saw a (
as this is enough to know that it's a function call. How come you decided to move away from that here?
That's a good point. I'd decided to parse the function params because that would make the error message nicer, by highlighting the whole function call, rather than just the name and opening paren. (Also hadn't realized that the parser already checks for I guess, to get the error message to highlight the whole function call, we'd have to wrap I've now changed the code for |
79d5a50
to
01fbdd2
Compare
01fbdd2
to
be6afe9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah! OK cool that sounds great.
Thank you once again! This should help people a lot 💜
This should resolve #2983.
Attempting to call a function in a constant definition will now show a more helpful error message:
(the second test is at
compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__functions_called_outside_module.snap
, which already existed)