support add-argument action

[santiweight]

This action supports an action "Add argument xxx to function" whenever there is an undefined variable error:

foo = unknown
-- becomes
foo unknown = unknown

The only minor wrinkle is that

foo = True
   where
      bar = unknown
-- becomes
foo unknown = True
   where
      bar = unknown

But this is good enough for now as a matter of pragmatism.

As part of this MR, I fixed some behavior. Previously an unknown-type undefined variable did not offer a "define x" option, but now does:

foo = unknown
-- previously no code actions
-- now becomes
foo = unknown

unknown :: _
unknown = _

[kokobd]

Thanks for your contribution!

For reference, below are the behavior of some other language servers/IDEs on this:

• rust-analyzer in VSCode: no quickfix
• TypeScript in VSCode: no quickfix
• Java in Intellij IDEA Community: has this quickfix, but provides a dialog to modify function signature

screenshots

I'm not quite sure about the design, maybe someone else will provide better insight.

[santiweight]

Thanks for the screenshots! Very helpful context :) You note that Rust/TS have no quick fix.

I am quite familiar with the IntelliJ interface from my previous job. I would love to support this workflow, but after I looked around the HLS and LSP issues I got the impression that such a pretty workflow would require too much work to justify. Is that you guys' experience? I really think our UX could be best in class given Haskell's type system, but I'm concerned about maintenance hell.

On a side note, it would be really great to have some sort of function:

alterAllUsages :: {- function name -} RdrName -> (LHsExpr -> TransformT m LHsExpr) -> IDEState -> TransformT m IDEState

The idea being that when I add a function parameter to a definition, I can insert appropriate holes in all usages project wide. Does anyone have any thoughts on the feasibility of this? @OliverMadine seems like a candidate for such knowledge!

module One where

import Two

bar = foo True
----
module Two where

foo True = unknown False

----- BECOMES -----
module One where

import Two

bar = foo True _unknown
----
module Two where

foo True unknown = unknown False

[OliverMadine]

The idea being that when I add a function parameter to a definition, I can insert appropriate holes in all usages project wide. Does anyone have any thoughts on the feasibility of this? @OliverMadine seems like a candidate for such knowledge!

Yes, it seems very feasible to me! Most of the logic could be extracted from the rename plugin. It would, however, be subject to the same multi-component limitation of the rename plugin (#2193)

[drsooch]

There is a plug-in that attempts to update signatures when it can. change-type-signature It's pretty rudimentary and only works in a handful of cases where GHC provides enough information. When the upstream lsp package updates to the lsp spec 3.17. We should be able to get direct access to GHCs Error Message API and potentially be worthwhile to this case.

[santiweight]

@drsooch I've thought about the change-type-sig plugin in this context. I think they're fairly different situations. In change-type-sig, I can only see occurrences where the new type signature is given.

However, in this case, we do not have the full type signature: we only have the type of the argument we're adding. That means we do have to manually alter the HsSigType with the exact print api, which is more complicated.

I implemented a working first attempt (it's surely very buggy for non-trivial layouts): https://github.com/santiweight/haskell-language-server/tree/add-arg-type-sig

[michaelpj]

Can you add yourself to CODEOWNERS? You might also want to mention this in features.md.

[santiweight]

It seems that the tests didn't run as part of CI? Besides that I've added myself to codeowners but I don't have write access, which is causing an error in the code owners file!

I'm happy to merge whenever now :) Thanks for the help getting this one through!

[michaelpj]

It seems that the tests didn't run as part of CI?

They should have, we're testing hls-refactor-plugin which should include the new stuff, right?

Besides that I've added myself to codeowners but I don't have write access, which is causing an error in the code owners file!

I've invited you to the repo :)

[santiweight]

Thanks for the review @michaelpj

A heads up on the status of this PR... I'm pretty happy with where it is right now, but I'm a little worried about my approach to using the Exactprint API, and so I'm going to attempt reimplementing the code in a better style (by following the examples in ghc-exactprint).

This MR isn't forgotten, just hopefully being replaced by something better...

[santiweight]

I was hoping to upstream some of the changes here in ghc-exactprint before merging, but I think that process will take some time.

I've answered the comments from reviewers, and I now think this is ready to merge!

[michaelpj]

I'm happy for you to continue improving things, especially since it sounds like you're definitely continuing to work in this area :)