Figure out how best to integrate with GHC's new error messages

[goldfirere]

Please close this ticket and redirect me if this is the wrong place to post.

@adinapoli and I are hard at work making GHC's error messages easier to work with. Specifically, instead of just rendering these into strings, we're building ADTs to describe the error messages semantically. The idea is that an IDE or other tool would be able to grab these error messages and act on them without parsing GHC's string output.

After an unreasonable amount of clearing away old cruft, we're finally at the point of actually building these ADTs. An accessible sampling of the idea is at https://gitlab.haskell.org/ghc/ghc/-/merge_requests/5533/diffs#820a2607941cd6fe77ba25c9a59b3f1ee106bb01_108_114.

We'd like feedback about what we're doing here, with two specific questions:

1. How does the type linked above look? Is this the right general approach? It's much easier to learn this now, before converting lots and lots of other error messages!
2. How should we communicate these messages to clients? See also a recent post to ghc-devs (the archives appear down at the moment, which I've reported; will add link when I can) on this point.

Ideally, we'd love to have a point person from the HLS community to collaborate with us in making sure that we're building in a direction that will be maximally useful downstream. No code contributions expected, just some design thoughts occasionally.

Thanks!

[pepeiborra]

Awesome! Thank you @adinapoli and @goldfirere for pushing this work forward.

What follows is only my personal view as a long-term ghcide contributor.

1

I think this approach is fine for HLS, since we have an active group of contributors willing to invest the time in handling every error type individually. In their current textual form, that involves grepping the textual message to both find out the type and to extract the suggestion - which usually needs to be augmented with AST or module graph information in order to construct a text edit for the code action. Can't wait to see them rewrite the CodeAction module with the new typed diagnostics when they arrive.

Reaching for a bit more, ideally GHC would provide the code action(s) itself - that is, not only a description of the error, but also a quick fix suggestion including a Diff. This would avoid duplication of efforts downstream. Now, is InstantiationSuggestion an example of that? If so, what might be useful is maybe a mapping DriverMessage -> Maybe (Suggestion, Diff) to be able to deal with all types of messages in the same manner. This can be built later or even externally as a library, but doing it now might help ensure a certain consistency is observed. Hardly required though.

2

We link against GHC itself, so we can consume the typed errors directly - there's always a contributor willing to upgrade to the next version of GHC and deal with the API changes.

That said, a useful view would be the one provided by https://hackage.haskell.org/package/apply-refact - tools like hlint already implement this encoding, it's stable, and there's even a tool/library that can mechanise the suggestions. This would also pretty much implement the DriverMessage -> Maybe (Suggestion, Diff) mapping mentioned above.

[goldfirere]

Thanks @pepeiborra!

Just this week, @adinapoli and I have been brainstorming about the very issues you raise. My conclusion, though, is that GHC is the wrong place to generate concrete refactorings. For a given error, even once we know a fix, generating the right code diff is just too arbitrary. Here is my argument against this idea, copied verbatim from a conversation with @adinapoli:

I don't think GHC should be in the business of creating Refactorings or sending them out to clients. Coming up with a concrete change to source code is hard, and has many arbitrary qualities. As a simple example, suppose we identify the need to add an extension. We now need to answer many questions:

Is there a LANGUAGE pragma already? If not, where should we add one? Before top-of-file comments or afterwards?
Assuming there is a pragma, how does the new extension fit in with existing ones? Go alphabetically? Make a separate pragma? Before or after or among existing ones?

There are many styles out there for LANGUAGE pragmas, and we should not be in the business of choosing among them or, say, analyzing what style the source code already has and trying to continue it.

Bottom line for me: I don't think we can do a better (that is, more concrete) job in describing refactorings than the original Hint type. It will be up to IDEs to figure out how to turn that Hint type into a refactoring, using whatever means it wants.

The Hint type is a high-level description of a code repair. It has constructors like AddExtension Extension and AddDo SrcSpan. Its design is very much in flux, but I don't think we should really try to make it more actionable, leaving that instead to downstream. Ideally, someone would write a library that would take GHC's Hint type and source code and figure out a concrete code repair; this library could be shared among several IDEs. But I think wiring this into GHC is not the right approach.

[adinapoli]

Now, is InstantiationSuggestion an example of that? If so, what might be useful is maybe a mapping DriverMessage -> Maybe (Suggestion, Diff) to be able to deal with all types of messages in the same manner.

To complement what @goldfirere said already, but that it might not be obvious without looking at the design in depth: we will eventually have a typeclass like this, in GHC:

class Diagnostic a where
  diagnosticMessage :: a -> DecoratedSDoc
  diagnosticReason  :: a -> DiagnosticReason
  diagnosticHints   :: a -> [GhcHint]

Then, types like the DriverMessage linked in that MR are instances of this class, so that IDEs can simply grab the list of
GhcHint in a consistent, uniform way. HEAD already contains the Diagnostic typeclass with the first two methods. We are currently working to add the 3rd method for the hints.

The bit Richard hinted at (no pun intended, I swear!) was that at some point we debated to have a Hint typeclass which would give you a list of refactorings given a single GhcHint, but we backtracked on the idea for the reasons that Richard explained above 😉 .

[wz1000]

I agree that it would be best to leave computing specific edits to the IDE or other consumer, since these can be very opinionated.

I also think it is best to first expose this information directly to consumers of the GHC API, since this is already how HLS works.
It also allows consumers to define their own serialization formats on top if they so desire.

The work in the linked MR looks like a very promising start. My suggestion is to expose as much information as possible in terms of the rich data structures that GHC itself uses internally. Just the other day, @isovector wanted to get some information about incomplete pattern matches for code completion purposes, but we realized that although GHC.HsToCore.PmCheck builds up structured data representing exactly the information we needed, it doesn't expose this information except as a pretty printed SDoc. If we wanted to recover any of this information, the only option was to parse the resultant SDoc :(

Another somewhat tangential idea I have is to add constructors to the SDoc type for certain simple fundamental GHC types like Name and Module. This would allow us to define pretty printers that annotate these elements with things like hyperlinks and tooltips. I suppose this would also be possible if we have access to the reified error data structure before it is converted to a SDoc, but I imagine a lot of the time we would just want to display the pretty printed error with a few additional annotations, and it would be a shame if we had to replicate the entire pretty printing logic just for this.

[goldfirere]

I also think it is best to first expose this information directly to consumers of the GHC API, since this is already how HLS works.

This is already done! It seems like the GHC API exposes functions like typecheckModule, which describes that it throws a SourceError in the event of trouble. Our definition of SourceError is now

newtype SourceError = SourceError (Messages GhcMessage)

where Messages is just a newtype wrapper around Bag. GhcMessage is the root of our error-types hierarchy: it contains constructors that distinguish among concrete error types from each subsystem (e.g. type-checker vs desugarer vs driver). So it would seem that this new information could be taken advantage of today -- excellent.

It also sounds like it would be good to create a JSON interface, but doing so would require the active support of someone who would consume the data, to help guide the design. Knowing that our work is usable as-is (due to the GHC API workflow via SourceError), I'm content enough to wait for someone to offer such help. My goal in starting this thread is to make sure that we're building a facility that is possible to use, not necessarily that its first version will satisfy all possible users.

Please do join in the battle if you'd like to expand the interface!

Having gotten enough information to proceed, I will now close this ticket. Thanks!