Elaborate interface signature errors#5777
Conversation
Signature errors were not reported before.
There was a problem hiding this comment.
I think this needs to be if (localErrors) {...} because you only want to report this error if you haven't reported other errors already. If you look at the changes in all of the tests you'll see what I mean. There are a bunch of "Signature has no corresponding..." errors that are out of place because there actually was a corresponding signature for which you've reported the reason that it wasn't compatible.
There was a problem hiding this comment.
For example here. You've already reported why the signatures aren't compatible, so it doesn't make sense to say there isn't a corresponding signature.
There was a problem hiding this comment.
Done. Thanks, I thought it was over-reporting.
Accept the new baselines based on this change. Gets rid of a lot of redundant errors.
And accept baselines
|
Error baselines look much better now. 👍 |
There was a problem hiding this comment.
Can you add documentation to localErrors where it's defined? Also, give it a better name because I don't think it accurately reflects the functionality.
There was a problem hiding this comment.
Hmmm, good point. I changed the name to reportMoreErrors.
Change `localErrors` to `reportMoreErrors` to more accurately reflect what the variable does.
There was a problem hiding this comment.
Just call it shouldElaborateError.
|
@DanielRosenwasser should I go ahead and merge now? I don't really want to add a way to distinguish |
…-errors Elaborate interface signature errors
|
Fixes #5751. |
|
I'm not sold on this special casing of the apparent types of the primitives. I thought you had (correctly) concluded it wasn't worth the effort. |
|
My objections are
On the other hand, the error messages do get less noisy. When I discussed it with @DanielRosenwasser in person, he pointed out that we are already in an error case, so (2) doesn't matter. And I decided that the small number of primitives (three) was not likely to change much, so it was worth it to get the shorter error messages. If that analysis of the tradeoff isn't satisfying, I could change it back. |
Thanks @RyanCavanaugh for this -- it just needed one small change to pass
kindinto the error reporting.Can you take a look at the new-and-improved signature errors? The change adds a line to many errors, which is more correct but also more verbose.