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

Make type error location "clickable" #2161

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Conversation

mzagozen
Copy link
Collaborator

@mzagozen mzagozen commented Feb 7, 2025

Use the real filesystem path to the module where a type error was raised. This means replacing "." with path separator and appending the ".act" suffix. So foo.bar@123 becomes foo/bar.act@123

This makes it possible for VS Code to create a clickable link to the file. Well, almost, because VS Code does not recognize "@" as a file - location separator. Not sure if the diagnostic package can be tweaked to use ":" instead of "@"?

Use the real filesystem path to the module where a type error was
raised.  This means replacing "." with path separator and appending the
".act" suffix. So foo.bar@123 becomes foo/bar.act@123

This makes it possible for VS Code to create a clickable link to the
file. Well, almost, because VS Code does not recognize "@" as a file -
location separator. Not sure if the diagnostic package can be tweaked to
use ":" instead of "@"?
@plajjan
Copy link
Contributor

plajjan commented Feb 9, 2025

Sounds like an issue to be reported upstream https://hackage.haskell.org/package/diagnose

Diagnose is inspired by Ariadne, a rust library to print pretty error messages. Ariadne seems to be using : to separate file name from location. Not sure why diagnose pick @. Seems like colon would be better. Perhaps a patch could be accepted upstream to just change or offer customizability.

Comment on lines +879 to +880
modNameToErrorString :: A.ModName -> String
modNameToErrorString mn = joinPath (map nameToString names) ++ ".act"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
modNameToErrorString :: A.ModName -> String
modNameToErrorString mn = joinPath (map nameToString names) ++ ".act"
modNameToFilename :: A.ModName -> String
modNameToFilename mn = joinPath (map nameToString names) ++ ".act"

handleTypeError opts errKind f src paths mn ex = do
printDiag opts $ mkErrorDiagnostic (modNameToString mn) src (typeReport ex (modNameToString mn) src)
printDiag opts $ mkErrorDiagnostic (modNameToErrorString mn) src (typeReport ex (modNameToErrorString mn) src)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
printDiag opts $ mkErrorDiagnostic (modNameToErrorString mn) src (typeReport ex (modNameToErrorString mn) src)
printDiag opts $ mkErrorDiagnostic (modNameToFilename mn) src (typeReport ex (modNameToFilename mn) src)

@plajjan
Copy link
Contributor

plajjan commented Feb 11, 2025

I think the change from the internal module path to filenames is valid regardless if it makes clickable links or not, so we can proceed with this particular PR. I made a suggestion to your code.

We have golden tests for typeerrors and syntax errors that are changed because of this. You need to run make test-typeerrors-accept to accept the new output as golden, commit and push anew.

The other piece is of course work in upstream. We could vendor the Diagnose library and make local changes if we want to move fast and/or test this out to see how it feels.

A challenge might be that our error format includes multiple locations. One is picked as the primary point where we think the error is, but sometimes it is just hard to pin point the exact error, like it is often the non-alignment between location A and B. Is A or B then the error? I think we tend to point to the use / call site and consider the definition the correct one, but it's probably not 100%... so just saying, that's a challenge. In gcc style errors (from which the clickable link format comes from), stuff just gets rendered quite differently, so clickable links are easier there.

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