fix: add context code action with trailing comment

[guibou]

For now it is just a repro for #4648

Tested with ghc 9.12.2, it seems to fail as expected:

[guillaume@gecko:~/srcs/haskell-language-server]$ cabal run hls-refactor-plugin-tests -- -p haddock
Configuration is affected by the following files:
- cabal.project
refactor
  code actions
    add function constraint
      preexisting constraint, with haddock comment in type signature: FAIL (0.84s)
        plugins/hls-refactor-plugin/test/Main.hs:3122:
        expected: "module Testing where\ndata Pair a b = Pair a b\neq \n    :: (Eq a, Eq b)\n    =>\n   -- This is the first argument\n   Pair a b ->\n   -- And this is the second\n   Pair a b ->\n   -- And this is the result\n   Bool\neq (Pair x y) (Pair x' y') = x == x' && y == y'\n"
         but got: "module Testing where\ndata Pair a b = Pair a b\neq \n    :: (Eq a,\n\n   -- This is the first argument Eq b)\n    =>\n\n   Pair a b ->\n   -- And this is the second\n   Pair a b ->\n   -- And this is the result\n   Bool\neq (Pair x y) (Pair x' y') = x == x' && y == y'\n"

1 out of 1 tests failed (0.84s)

(see how Eq b) is introduced inside the haddock comment This is the first argument.

I'm pushing the branch for reference and to test with other GHC version (I'm too lazy to build that locally, cabal cache was hot for ghc 9.12.2).

[guibou]

The current code was tested with GHC 9.12 and does fix the broken generated code.

However it can:

• drop comments
• move all inline comments from the inside of the context block to the end of the context block.

The difficulty comes from the way comments are stored in the AST: they are not stored "inline", interleaved with the ast nodes. Instead, they are stored in a list of comment attached to an ast node and each comment is localised with it absolute position in the stream as well as the position of the latest taken (in order to introduce whitespaces).

Consider the following example:

bar :: 
  -- BEFORE
  {- yoto -} (Monad m {- yiti -}){- yutu -} => {- yete -} -- Trailing
  -- After
  m ()

• BEFORE and yoto are attached to the bar node
• yiti is attached to the Monad m node
• yutu, yete, Trailing and After are attached to the (Monad m) node (e.g. the constraint) and the pretty printer layouts everything based on the anchor and position of the items.

In order to insert a value after Monad m while keeping everything correctly linked, we need to move the anchor and span for yutu, yete, Trailing and After.

Note that I'm discovering the annotation API right now and maybe there is a solution for that. Especially, there is https://hackage-content.haskell.org/package/ghc-9.8.1/docs/GHC-Parser-Annotation.html#t:AnchorOperation AnchorOperation which seems to be done especially for this.

[guibou]

I've spent a few hours trying to understand the exact-print semantic in order to correctly handle all cases and I admit that I'm failing here. I'm not sure that exact-print is working as expected, or not or what I'm supposed to do next.

What should we do now?

@fendor, you did accept the MR, thank you. Should we merge considering that:

• This MR does not break any test
• This MR fixs newly introduces tests for GHC 9.10 and GHC 9.12
• There is a risk of moving inline commen a the end of the block without user noticing it. However this won't only happen for GHC 9.10 and 9.12, for which the code action was previously generating completely wrong code.

What is better, a code action which does not work at all, or a code action which works, but in some context, can (silently) move comments ?

Note: I think I have fixed the CI issues related to formatting, however some failures may not be related to this MR.

[fendor]

however some failures may not be related to this MR.

The bench failures seem to be caused by this PR:

plugins/hls-refactor-plugin/src/Development/IDE/Plugin/CodeAction/Util.hs:40:9: error: [GHC-88464]
Error:     Variable not in scope:
      createDirectoryIfMissing :: Bool -> t0 -> IO a0
   |
40 |         createDirectoryIfMissing True "/tmp/hls"
   |     

Should we merge considering that:

Yes, I think this PR is a net-improvement over the status quo

[fendor]

CircleCI complains about a missing warning.
Counter-intuitvely, are the stack jobs the only jobs that run with the -pedantic flag:

haskell-language-server>     Pattern match(es) are non-exhaustive
haskell-language-server>     In an equation for ‘moveCommentsToTheEnd’:
haskell-language-server>         Patterns of type ‘EpAnn ann’ not matched: EpAnnNotUsed
haskell-language-server>     |
haskell-language-server> 223 | moveCommentsToTheEnd (EpAnn entry anns (EpaComments priors)) = EpAnn entry anns (EpaCommentsBalanced { priorComments = [], followingComments = priors})

haskell-language-server>     | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^...

and it complains that moveCommentsToTheEnd is not used. So, either you need to prefix it with _ or use CPP to fix this error.

This is for GHC 9.6.6.

[guibou]

@fendor

I do really apologize for the unnecessary ping pong.

I first saw the warning about formatting (e.g. my import lines were not formatted correctly) and I fixed that super quickly and messed up my rebase / squash and hence pushed an invalid commit. However in the meantime I did not really understood the reason for the failure and thought it was unrelated to my current MR.

Anyway, thank you for the review, I pressed the merge button.