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

dedicated parsing for error messages related to interface files #379

Closed
wants to merge 1 commit into from

Conversation

zth
Copy link
Collaborator

@zth zth commented Apr 3, 2022

This detects a few diagnostics related to interface files, and does the following:

One diagnostic for the implementation, one for the interface file

Previously the diagnostic would point to the implementation only. This changes it to produce one diagnostic for the implementation file, and one for the interface file. This will hopefully make it a bit easier to find where the actual issue is located.

image

Clean up error messages

The error message the compiler produces is quite dense. This produces slightly cleaned up, separate error messages for the implementation and the interface respectively.

Before (one message only):

The implementation /Users/zth/git/rescript-intro/src/Interface.res
       does not match the interface src/interface-RescriptIntro.cmi:
       ...
       In module Test:
       Values do not match:
         let bar: (
  ~some: 'a,
  ~thing: 'b,
  ~another: 'c,
  ~andSome: 'd,
  ~anotherOne: 'e,
  ~andAgain: 'f,
  ~andAgain1: 'g,
  ~andAgain2: 'h,
  ~andAgain3: 'i,
) => int
       is not included in
         let bar: (
  ~some: int,
  ~thing: int,
  ~another: string,
  ~andSome: bool,
  ~anotherOne: bool,
  ~andAgain: bool,
  ~andAgain1: bool,
  ~andAgain2: bool,
) => int
       File "/Users/zth/git/rescript-intro/src/Interface.resi", line 4, characters 3-187:
         Expected declaration
       File "/Users/zth/git/rescript-intro/src/Interface.res", line 6, characters 7-10:
         Actual declaration

...and in this PR - error message for the implementation:

This implementation does not match the type signature defined in the interface file ("Interface.resi"). The interface defines the following type signature:

let bar: (
  ~some: int,
  ~thing: int,
  ~another: string,
  ~andSome: bool,
  ~anotherOne: bool,
  ~andAgain: bool,
  ~andAgain1: bool,
  ~andAgain2: bool,
) => int

...but this implementation's type signature is:

let bar: (
  ~some: 'a,
  ~thing: 'b,
  ~another: 'c,
  ~andSome: 'd,
  ~anotherOne: 'e,
  ~andAgain: 'f,
  ~andAgain1: 'g,
  ~andAgain2: 'h,
  ~andAgain3: 'i,
) => int

And for the interface file:

This type signature does not match the implementation. The implementation in "Interface.res" currently has the following type signature:

let bar: (
  ~some: 'a,
  ~thing: 'b,
  ~another: 'c,
  ~andSome: 'd,
  ~anotherOne: 'e,
  ~andAgain: 'f,
  ~andAgain1: 'g,
  ~andAgain2: 'h,
  ~andAgain3: 'i,
) => int

The original message is still printed by the compiler in the terminal.

More

This is also a preparation for experimenting with code actions related to interface file diagnostics.

…2 diagnostics for some interface file issues - one pointing to the implementation in question, and one to the definition in the interface file. clean up some error messages for interface files.
@zth zth requested a review from cristianoc April 3, 2022 10:56
@amiralies
Copy link
Contributor

Although I think it's a great change and delivers better UX to editor users, the conflict between compiler error msg and editor error message can be confusing. I prefer this to be done somewhere in the compiler.

@zth
Copy link
Collaborator Author

zth commented May 3, 2022

Although I think it's a great change and delivers better UX to editor users, the conflict between compiler error msg and editor error message can be confusing. I prefer this to be done somewhere in the compiler.

Are you up for taking a stab at implementing it in the compiler?

@amiralies
Copy link
Contributor

I think we can use super erros implemenation to deliver such behavior iirc. i should take a look.

@zth
Copy link
Collaborator Author

zth commented May 4, 2022

I think we can use super erros implemenation to deliver such behavior iirc. i should take a look.

That would be great if you had a look at that! In fact, there's quite a lot of error messages that needs cleaning up. So if you figure out a good workflow it'd be nice to also look at a few more of them.

@zth
Copy link
Collaborator Author

zth commented Jul 13, 2022

Closing this as we're not going to move forward with something like this directly in the extension. @amiralies is doing some potentially related work on error messages in the compiler. We'll focus our efforts there instead.

@zth zth closed this Jul 13, 2022
@cristianoc cristianoc deleted the fix-interface-file-diagnostics branch July 26, 2022 23:03
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