Fix: Show actual parse errors for external Fourmolu/Ormolu

[ijatinydv]

Right now, if you use an external fourmolu or ormolu executable and there's a syntax error in your file, the editor just pops up a generic Internal Error: Fourmolu failed with exit code 3. The actual parse error gets buried in the HLS logs, which is a bit frustrating since the bundled library paths already show the error directly in the IDE.

This PR fixes that by grabbing the stderr output from the CLI process and tacking it onto the PluginInternalError message for both plugins.

A couple of minor UX details included:

• Added a T.null guard so we don't append empty newlines if the formatter fails silently.
• Used T.stripEnd instead of stripping both sides so that the internal indentation of the GHC parse errors stays intact when it renders in the editor popup.

Fixes #4853

[fendor]

Do we perhaps want to add a regression test for this?

[ijatinydv]

Do we perhaps want to add a regression test for this?

I agree. Just pushed an update with tests for both of them. let me know if this looks good!

[ijatinydv]

@fendor almost 15 tests are failing. Can you help me in this where is the problem occuring?

[fendor]

Your tests don't compile on older GHC versions, see

 [1 of 1] Compiling Main             ( plugins/hls-ormolu-plugin/test/Main.hs, b/build/aarch64-osx/ghc-9.8.4/hls-2.13.0.0/t/hls-ormolu-plugin-tests/build/hls-ormolu-plugin-tests/hls-ormolu-plugin-tests-tmp/Main.o )
<no location info>: warning: [GHC-42258] [-Wunused-packages]
    The following packages were specified via -package or -package-id flags,
    but were not needed for compilation:
      - ormolu-0.7.4.0 (exposed by flag -package-id rml-0.7.4.0-b3a2b833)
Configuring test suite 'hls-notes-plugin-tests' for hls-2.13.0.0...

plugins/hls-ormolu-plugin/test/Main.hs:47:37: error: [GHC-87543]
Error:     Ambiguous occurrence ‘_result’.
    It could refer to
       either the field ‘_result’ of record ‘TResponseMessage’,
              imported from ‘Test.Hls’ at plugins/hls-ormolu-plugin/test/Main.hs:16:1-25
              (and originally defined in ‘lsp-types-2.4.0.0:Language.LSP.Protocol.Message.Types’),
           or the field ‘_result’ of record ‘ResponseMessage’,
              imported from ‘Test.Hls’ at plugins/hls-ormolu-plugin/test/Main.hs:16:1-25
              (and originally defined in ‘lsp-types-2.4.0.0:Language.LSP.Protocol.Message.Types’).
   |
47 |                   TResponseMessage {_result = Left (TResponseError {_message = msg})} ->
   |                                     ^^^^^^^

plugins/hls-ormolu-plugin/test/Main.hs:47:69: error: [GHC-87543]
Error:     Ambiguous occurrence ‘_message’.
    It could refer to
       either the field ‘_message’ of record ‘TResponseError’,
              imported from ‘Test.Hls’ at plugins/hls-ormolu-plugin/test/Main.hs:16:1-25
              (and originally defined in ‘lsp-types-2.4.0.0:Language.LSP.Protocol.Message.Types’),
           or the field ‘_message’ of record ‘Diagnostic’,
              imported from ‘Test.Hls’ at plugins/hls-ormolu-plugin/test/Main.hs:16:1-25
              (and originally defined in ‘lsp-types-2.4.0.0:Language.LSP.Protocol.Internal.Types.Diagnostic’),
Configuring test suite 'hls-hlint-plugin-tests' for hls-2.13.0.0...
           or the field ‘_message’ of record ‘DiagnosticRelatedInformation’,
              imported from ‘Test.Hls’ at plugins/hls-ormolu-plugin/test/Main.hs:16:1-25
              (and originally defined in ‘lsp-types-2.4.0.0:Language.LSP.Protocol.Internal.Types.DiagnosticRelatedInformation’),
           or the field ‘_message’ of record ‘LogMessageParams’,
              imported from ‘Test.Hls’ at plugins/hls-ormolu-plugin/test/Main.hs:16:1-25
              (and originally defined in ‘lsp-types-2.4.0.0:Language.LSP.Protocol.Internal.Types.LogMessageParams’),
           or the field ‘_message’ of record ‘LogTraceParams’,
              imported from ‘Test.Hls’ at plugins/hls-ormolu-plugin/test/Main.hs:16:1-25
              (and originally defined in ‘lsp-types-2.4.0.0:Language.LSP.Protocol.Internal.Types.LogTraceParams’),
           or the field ‘_message’ of record ‘ShowMessageParams’,
              imported from ‘Test.Hls’ at plugins/hls-ormolu-plugin/test/Main.hs:16:1-25
              (and originally defined in ‘lsp-types-2.4.0.0:Language.LSP.Protocol.Internal.Types.ShowMessageParams’),
           or the field ‘_message’ of record ‘ShowMessageRequestParams’,
              imported from ‘Test.Hls’ at plugins/hls-ormolu-plugin/test/Main.hs:16:1-25
              (and originally defined in ‘lsp-types-2.4.0.0:Language.LSP.Protocol.Internal.Types.ShowMessageRequestParams’),
           or the field ‘_message’ of record ‘WorkDoneProgressBegin’,
              imported from ‘Test.Hls’ at plugins/hls-ormolu-plugin/test/Main.hs:16:1-25
              (and originally defined in ‘lsp-types-2.4.0.0:Language.LSP.Protocol.Internal.Types.WorkDoneProgressBegin’),
           or the field ‘_message’ of record ‘WorkDoneProgressEnd’,
              imported from ‘Test.Hls’ at plugins/hls-ormolu-plugin/test/Main.hs:16:1-25
              (and originally defined in ‘lsp-types-2.4.0.0:Language.LSP.Protocol.Internal.Types.WorkDoneProgressEnd’),
           or the field ‘_message’ of record ‘WorkDoneProgressReport’,
              imported from ‘Test.Hls’ at plugins/hls-ormolu-plugin/test/Main.hs:16:1-25
              (and originally defined in ‘lsp-types-2.4.0.0:Language.LSP.Protocol.Internal.Types.WorkDoneProgressReport’),
           or the field ‘_message’ of record ‘ResponseError’,
              imported from ‘Test.Hls’ at plugins/hls-ormolu-plugin/test/Main.hs:16:1-25
              (and originally defined in ‘lsp-types-2.4.0.0:Language.LSP.Protocol.Message.Types’).
   |
47 |                   TResponseMessage {_result = Left (TResponseError {_message = msg})} ->
   |                                                                     ^^^^^^^^

plugins/hls-ormolu-plugin/test/Main.hs:53:37: error: [GHC-87543]
Error:     Ambiguous occurrence ‘_result’.
    It could refer to
       either the field ‘_result’ of record ‘TResponseMessage’,
              imported from ‘Test.Hls’ at plugins/hls-ormolu-plugin/test/Main.hs:16:1-25
              (and originally defined in ‘lsp-types-2.4.0.0:Language.LSP.Protocol.Message.Types’),
           or the field ‘_result’ of record ‘ResponseMessage’,
              imported from ‘Test.Hls’ at plugins/hls-ormolu-plugin/test/Main.hs:16:1-25
              (and originally defined in ‘lsp-types-2.4.0.0:Language.LSP.Protocol.Message.Types’).
   |
53 |                   TResponseMessage {_result = Right _} ->
   |                                     ^^^^^^^

[ijatinydv]

@fendor right, I totally missed that. I tried to skip the lens dependency but totally forgot how overloaded the lsp-types fields are on older GHC versions. Just fixed it using Language.LSP.Protocol.Lens
thanks for this fendor!

[fendor]

Some comments, Ill leave the rest to @georgefst

[fendor]

Can we check for the actual message, or is parse error truly the only thing we get reliably check for?

[ijatinydv]

Can we check for the actual message, or is parse error truly the only thing we get reliably check for?

matching the full actual message gets pretty brittle since ghc changes its parser error formatting across versions (like injecting GHC codes or changing quote styles), and the <stdin> vs filepath prefix changes based on the environment.
since we're just verifying the stderr flow works, a stable substring is the safest choice to avoid flaky tests. i agree it could be more specific though, so i tightened it to "parse error on input" and dropped a comment in the code explaining the intentional flexibility. pushed the update, please have a review.

[fendor]

Very nice, LGTM!
Thank you for your contribution!