Emit holes as diagnostics

[isovector]

This PR adds new diagnostics on every hole. The use case is to support workflows where Wingman users can quickly jump to the next area they need to work on when refining holes.

The severity of these diagnostics is configurable, since many clients do not support jumping to diagnostics by code, and VSCode doesn't even allow jumping to severity=hint diagnostics.

[wz1000]

Is this really necessary? It will be executed on every single keystroke, so please be very sure about it.

[isovector]

Happy to get suggestions here; this is just cargo culted from hlint.

[wz1000]

What is the desired purpose? Do you want to refresh the code action diagnostics on every keystroke, or only when code actions are invoked?

[isovector]

I must admit I'm in over my head here. I'd like a workflow that allows for refining holes by hand and via Wingman, that makes it easy to get to the next hole.

My original thought was to just automatically move to the next hole after running a code action, but LSP doesn't support that. And so if users need to do the movement themselves, it'd be nice if it worked under all circumstances.

[wz1000]

Have you seen this: haskell/ghcide#889

The idea is that you can hook into GHC to record all the holes in the program while its being typechecked, no need to do a potentially slow SYB style traversal afterwards.

[isovector]

That's embarrassing. I had them disabled in my test project for some reason. Thanks. How can I get the existing diagnostics?

[berberman]

You might want this:

haskell-language-server/ghcide/src/Development/IDE/Core/Shake.hs

Lines 781 to 784 in 14b46e1

	getDiagnostics :: IdeState -> IO [FileDiagnostic]
	getDiagnostics IdeState{shakeExtras = ShakeExtras{diagnostics}} = do
	val <- readVar diagnostics
	return $ getAllDiagnostics val

[isovector]

I'm confused about the flow here, and worried about races.

getDiagnostics above requires an IdeState, which AFAICT isn't available inside of the shake Action where the new diagnostics need to be generated. Easy enough to work around this by mucking with the internals, but then getDiagnostics just reads a mutable variable. Without a proper rule that only fires when the diagnostics have changed, to ensure the other diagnostics get generated before I look at them, it seems like I'm just inviting race conditions.

But since diagnostics are generated (exclusively?) by rules, and that this new one will produce new diagnostics, it feels like this approach won't work. Unless I'm missing something obvious?

[pepeiborra]

No, you are right. The diagnostic store is mutable, and getDiagnostics will only read whatever is in there.

But that said, I think you could add a dependency on the Typecheck rule, and then I believe getDiagnostics will find the Typecheck diagnostics in there.

[isovector]

That makes sense, thanks!

[isovector]

I'm going to make this opt-in and continue using SYB for the time being. I have a follow up feature that is going to need to interact with this (saving intermediate hypotheses from Wingman inside of the generated holes), and it seems like reusing the diagnostics approach won't work there. Rather than do something smart that might end up being unhelpful next week, I'm just going to do the stupid thing to get unblocked and re-evaluate in the followup work.

[wz1000]

I guess I don't really understand what the point of this PR is supposed to be. We already get diagnostics for holes, why duplicate them?

[isovector]

LSP doesn't support "move cursor", but does support "go to next diagnostic". Wingman needs the ability to jump to the next hole, not just any diagnostic, so it's necessary to differentiate the hole diagnostics from others. To make matters worse, most clients only support filtering diagnostics by severity, despite all the information that can be attached to diagnostics. It's a wildly frustrating state of affairs, but this is the best I can come up with.

[wz1000]

I guess if you simply want to add more metadata to hole diagnostics, a better way would be extending ghcide to support a hook which can modify diagnostics, and making use of that.