Improve the error message for unknown modules

[fendor]

Context

When users add a new module to a cabal project, they are usually greeted with an uninformative Multi Cradle: No prefixes matched or something similar.

We can identify three different cases and different behaviour:

1. Adding an other-module Test.hs to an executable component

In an example project with only a Main.hs module, adding an other-module Test.hs results in the following error message:

Multi Cradle: No prefixes matched
pwd: d:\Privat\Documents\programming\haskell\example
filepath: D:\Privat\Documents\programming\haskell\example\app\Test.hs
prefixes:
("app/Main.hs",Cabal {component = Just "example:exe:example"})

The cause is that implicit-hie generates roughly this hie.yaml file

cradle:
  cabal:
    - path: ./app/Main.hs
      component: "example:exe:example"

Which essentially means, only map ./app/Main.hs to the component example:exe:example. Clearly ./app/Test.hs is not listed here, resulting in the sub-par error message from hie-bios.

Note, implicit-hie doesn't handle overlapping source directories correctly in most cases, see #3606 for example.

2. Adding an exposed-module to a library component

This generally works ok because implicit-hie generates hie.yamls of the form:

cradle:
  cabal:
    - path: ./src
      component: "example:lib"

Essentially, maps for each source directory to the library component. So why does this work even if a hypothetical Lib.hs is not part of the library component? hie-bios doesn't care about that and just gives us the compilation options for the component example.:lib. Then haskell-language-server just takes the options, creates a unit, and adds Lib.hs to it:

haskell-language-server/ghcide/session-loader/Development/IDE/Session.hs

Line 841 in feb5965

let special_target = TargetDetails (TargetFile cfp) targetEnv targetDepends [componentFP ci]

Read the comment right above it for the justification.

That's why adding modules to a library works "okayish" most of the time.

3. Adding a new module to a component with a simple 'hie.yaml'

However, this all falls apart when we handwrite the recommended hie.yaml file:

cradle:
  cabal:

This basically runs cabal repl src/Lib.hs to find the compilation options. However, then cabal complains even in the case where it previously was working fine!

Failed to run ["cabal","v2-repl","src\\Lib.hs"] in directory "D:\Privat\Documents\programming\haskell\example". Consult the logs for full command and error.
Failed command: cabal --builddir=C:\Users\Hugin\AppData\Local\hie-bios\dist-example-897af6579b5c0528e593e285a92d28a4 v2-repl --with-compiler C:\Users\Hugin\AppData\Local\hie-bios\wrapper-340ffcbd9b6dc8c3bed91eb5c533e4e3.exe --with-hc-pkg C:\ghcup\ghc\9.2.7\bin\ghc-pkg-9.2.7.exe src\Lib.hs

Error: cabal-3.10.1.0.exe: Failed extracting script block: `{- cabal:` start
marker not found

Naturally, if Lib.hs is not an exposed- or other-module of the library component, cabal cannot find it. In this case, the error message is terrible in particular, as it tries to interpret the target as a cabal script, which is not even close to the desired result.

Solution

We can identify at least two possible ways forward.

One way, is improving implicit-hie to generate more lax hie.yaml files. I am in general against that approach. It will conceal the problem, and will just produce inconsistent views between the build-tool (cabal) and HLS. E.g. HLS reports no error or warning while cabal build straight up fails (also the package is rejected on hackage, etc..).

The better approach is to interject these terrible error messages and provide useful, actionable diagnostics to the user.
For example, for the first example, we should display a message like this:

Loading the module 'Test.hs' failed. It seems like it is not listed in your example.cabal file!

Perhaps you need to add `Test` as an other-module to your executable named 'example' in example.cabal.

If you don't know about other-module, yet, read this link <some-link>

It might not always be possible to provide precise information, but general information, perhaps with links to https://errors.haskell.org/, would be great already.

To improve the error message further, there are a couple of hacky solutions we can think of. We can try to guess the component the module likely will be part of by looking at already loaded components, and comparing the importPaths. If the loaded component options have a unit-id (supplied by -this-unit-id), we can then lookup that unit-id in dist-newstyle/cache/plan.json, find the component and produce a more accurate error message.

Fixing the issue programmatically is not something we want to do in this issue, we want to focus on purely better error messages.

Implementation Roadmap

1. Find the location where the error message/diagnostic is shown to the user

• Here hie-bios is called:

  haskell-language-server/ghcide/session-loader/Development/IDE/Session.hs

  Line 738 in feb5965

  CradleFail err -> return (Left [err])

• That result is eventually turned into a Diagnostic

2. Change the error message only if we are trying to load a 'Cabal' cradle https://github.com/haskell/hie-bios/blob/78d0cd2332e05c46b747b70ca69c0746209f440e/src/HIE/Bios/Cradle.hs#L200
3. Parse the error message, and produce a nicer diagnostics as discussed above, with mostly a high-level description of what's going wrong.

• It might be sensible, to turn the generic 'CradleError' into an ADT describing the error in more details, but parsing is also fine for the first approximation

4. Try to guess the location of the module based on its proximity to already loaded components

• Some relevant type

  haskell-language-server/ghcide/session-loader/Development/IDE/Session.hs

  Line 939 in feb5965

  data ComponentInfo = ComponentInfo

• dist-newstyle/cache/plan.json helps you map a unit-id to a component name.

This issue tracks the effort for improving the error message.

[fendor]

@seanhess This tries to explain the context and the roadmap. Happy to flesh out any details if you have questions. But as you can imagine, this is a big problem, with lots of moving parts, so I might be forgetting some things.

[seanhess]

This is great, thanks for the help getting oriented and clear roadmap. I'm on it!

[seanhess]

@fendor Tests are failing after checkout. I followed the contributing guidelines. ghc 9.6.2, cabal 3.10.1.0. Is there a specific ghc version I should be using?

format documents:
    formatting provider
      respects none:                                                         FAIL
        Exception: Language server unexpectedly terminated
        Use -p '/respects none/' to rerun this test only.
...
code actions:
    import suggestions
      import works with 3.8 code action kinds:                               FAIL
        Exception: Language server unexpectedly terminated
        Use -p '/import suggestions.import works with 3.8 code action kinds/' to rerun this test only.
  ...
  redundant import code actions
    remove solitary redundant imports: 
    ...
    expected: Nothing
    ...

[michaelpj]

We have a lot of problems with flaky tests :( I might just ignore it if I was you

@fendor rather than parsing the error, could we pass up more structured errors from hie-bios?

[seanhess]

I'm struggling to figure out how to hack HLS. Following the instructions in the contributing guide, I've built HLS with cabal install exe:haskell-language-server --overwrite-policy=always. This results in a binary at /Users/sean/.cabal/bin/haskell-language-server

Even if I manually run that binary in my test directory, it doesn't pick up any changes. I've tried hacking Development.IDE.Session.cradleToOptsAndLibDir in multiple ways: adding putStrLn in various places, changing the error, etc. When I run it, I always get the same error. I must be missing something basic. Any ideas?

Steps:

1. Alter the line 738 in Session.hs you mentioned to the following

        CradleFail err -> do
          putStrLn "HELLO?"
          return (Left [err { cradleErrorStderr = ["HELLO!"] }])

2. cabal install exe:haskell-language-server --overwrite-policy=always

3. /Users/sean/.cabal/bin/haskell-language-server-wrapper in my test directory

Results:

I get the exact same error message as I did before I changed the code, and the term "HELLO" doesn't appear in my terminal.

 new diagnostics - file diagnostics: File:     /Users/sean/Documents/code/hls-file-error/app/Test.hs
Hidden:   no
Range:    1:1-2:1
Source:   cradle
Severity: DsError
Message:
  Multi Cradle: No prefixes matched
  pwd: /Users/sean/Documents/code/hls-file-error
  filepath: /Users/sean/Documents/code/hls-file-error/app/Test.hs
  prefixes:
  ("app/Main.hs",Cabal {component = Just "hls-file-error:exe:hls-file-error"})

[fendor]

@fendor rather than parsing the error, could we pass up more structured errors from hie-bios?

Well, yesn't, that'd be moving the parsing to hie-bios. Which is fine in my opinion.

@seanhess Check in the stderr logs if the correct hls binary is invoked. For this issue in particular, it should be enough to run the binary directly, e.g. ~/.cabal/bin/haskell-language-server --debug ./app/Test.hs. I think that should print the error message already. Entrypoint to the relevant session loading:

haskell-language-server/ghcide/src/Development/IDE/Main.hs

Line 383 in 7007861

sessionLoader <- loadSessionWithOptions (cmapWithPrio LogSession recorder) argsSessionLoadingOptions dir

Also, it might be helpful to ask on IRC libera #haskell-language-server for quicker help with setup issues.

[seanhess]

Should I work in hie-bios then?

[seanhess]

@fendor See PR ^

I couldn't figure out how to test hie-bios correctly, so instead I'm parsing the error here.

[georgefst]

One way, is improving implicit-hie to generate more lax hie.yaml files. I am in general against that approach. It will conceal the problem, and will just produce inconsistent views between the build-tool (cabal) and HLS. E.g. HLS reports no error or warning while cabal build straight up fails (also the package is rejected on hackage, etc..).

The one major advantage of this approach, presumably, is that there's much less waiting around when adding a new module. Since adding a new entry to other-modules (or any other changes to the .cabal file) basically causes a full reload of HLS.

Perhaps we could do both? Unless there's work going on in Cabal to make these reloads quicker, at least for such simple cases?

[fendor]

Yeah, that's true, that's an advantage, but it may also break unexpectedly. For example, you run cabal test or something, then cabal greets you with terrible linker errors.

If we do both... what would happen in that situation? You still see an error/warning at the very top, you may just ignore it for some time. Ignorable errors/warnings are not great UX in my opinion.

Perhaps we could do both? Unless there's work going on in Cabal to make these reloads quicker, at least for such simple cases?

If we can reliably figure out that we just added a module to a cradle, I think we might do something cool in HLS itself.