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

Add better explanation to error message #18665

Merged
merged 1 commit into from
Oct 24, 2023
Merged

Conversation

odersky
Copy link
Contributor

@odersky odersky commented Oct 7, 2023

Fixes #18657

@odersky odersky marked this pull request as ready for review October 7, 2023 17:32
@odersky
Copy link
Contributor Author

odersky commented Oct 7, 2023

How I fixed it

It was easy (I did not expect it to be hard):

  • Compile the program with the unclear error message with -Xprompt, followed by abort. This gives the location where the error was produced.
  • Find out what explanations would have clarified the error, and what the values containing this info are. To be sure, I added a test println with the suspected valuable info before the error report call.
  • Once I was happy with the kind of additional info, I changed the error message to take these additional infos as parameters. If the error had been reported by a simple string, without going through an error message in messages.scala, now would have been the time to create that message. One needs to make sure to create an ErrorMessageId as well in this case.
  • Add an explain section to the error message.
  • Add a test starting with //> using options -explain and a check file to make sure we have the right info.

That's it. It would be great if others would try to do the same thing when they see an obscure error message. Create an issue first and then fix it according to these steps. These are incremental improvements but they are really valuable in everyday work!

@SethTisue SethTisue removed their assignment Oct 24, 2023
|
| (Int, Int) => Int
|
| requires a function with 2 parameters.
Copy link
Member

Choose a reason for hiding this comment

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

Maybe the reader can infer from looking at the printed tree, but perhaps also explain that placeholder parameters always infer parameters 1 level of nesting up from their position (however best to explain that).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed, not sure how to explain that, in a situation like this, where context is missing.

@bishabosha bishabosha merged commit 9fccb7e into scala:main Oct 24, 2023
18 checks passed
@bishabosha bishabosha deleted the fix-18657 branch October 24, 2023 11:42
@bishabosha bishabosha added the backport:nominated If we agree to backport this PR, replace this tag with "backport:accepted", otherwise delete it. label Oct 24, 2023
@Kordyjan Kordyjan added this to the 3.4.0 milestone Dec 20, 2023
WojciechMazur added a commit that referenced this pull request Jun 23, 2024
Backports #18665 to the LTS branch.

PR submitted by the release tooling.
[skip ci]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:nominated If we agree to backport this PR, replace this tag with "backport:accepted", otherwise delete it. how-i-fixed-it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

unhelpful syntax error when inferred lambda parameters mismatch expected count
4 participants