Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HLS fails to find modules with .hs-boot files in the root project directory #3794

Closed
moll opened this issue Sep 6, 2023 · 9 comments · Fixed by #3893
Closed

HLS fails to find modules with .hs-boot files in the root project directory #3794

moll opened this issue Sep 6, 2023 · 9 comments · Fixed by #3893
Labels
component: ghcide type: bug Something isn't right: doesn't work as intended, documentation is missing/outdated, etc..

Comments

@moll
Copy link

moll commented Sep 6, 2023

Hey,

This is a continuation of a discussion we had with @fendor on IRC in #haskell-language-server.

Long story short, HLS seems to fail to find two self-recursive modules with .hs-boot files if they happen to live in the project root and you have a minimal hie.yaml. That is hs-source-dirs: .. This with GHC v9.2.8, HLS v2.2.0.0.

Here's the gist/Gist reproducing this and the error message it results in: https://gist.github.com/moll/591c08e1c6fa75b237214c298e3a97e5. There's also what gen-hie generates and what hie.yaml looked like.

I also confirmed that moving the X and Y modules to subdirectory (like Lib), while leaving hs-source-dirs: ., works.

Cheers

@moll moll added status: needs triage type: bug Something isn't right: doesn't work as intended, documentation is missing/outdated, etc.. labels Sep 6, 2023
@moll
Copy link
Author

moll commented Sep 6, 2023

Another funny thing is that when using the hie.yaml that gen-hie generates, it also fails to find X and Y. So whatever HLS does without hie.yaml is smarter than what gen-hie as of today generates.

@moll
Copy link
Author

moll commented Nov 23, 2023

Hey, thanks for the triage, @fendor. Have you by any chance identified what specifically is at fault in GHCide? Perhaps it's something easy to fix that I could take a crack at.

@fendor
Copy link
Collaborator

fendor commented Nov 23, 2023

Hi!

I did not find time to properly investigate this issue. I think the issue is located in https://github.com/haskell/haskell-language-server/blob/master/ghcide/src/Development/IDE/Import/FindImports.hs#L106. I don't think it is too tricky to fix it, but writing the test is the annoying thing.

I am currently briefly looking into the issue and it seems like stuff starts working in the editor if you also open the modules X.hs-boot and Y.hs-boot. That could indicate that the issue is in the session loader.

@moll
Copy link
Author

moll commented Nov 23, 2023

Gotcha. Thanks!

@fendor
Copy link
Collaborator

fendor commented Nov 24, 2023

So, I did some digging, and it didn't seem like a simple thing to fix. Apparently, we only guess the location of hs-boot files if GHC parses X and Y (from your example) as a TargetModule Target. This works fine if we have -isrc/ but if the import path is -i. then GHC itself (called by hie-bios) infers the targets X and Y as TargetFile's.
IIRC, GHC discovers the location of hs-boot files during the downsweep in the driver, that's why it works correctly with GHC and incorrectly for us because we don't discover new source files in our downsweep phase.
The solution is simple but quite tricky to do correctly: Instead of trying to discover a hs-boot file only if the target is a TargetModule, we also need to look for hs-boot files in TargetFile Targets.
The fix could be enough in Session.hs, perhaps in the function extendKnownTargets or in the newComponentCache function.

This was mostly a brain dump on my end and not a suggestion for you to work on it.

@moll
Copy link
Author

moll commented Nov 24, 2023

Thanks for the behind-the-scenes tech details nonetheless!

@wz1000
Copy link
Collaborator

wz1000 commented Nov 24, 2023

If the issue is as you described @fendor I'm pretty sure I fixed it in #3462:

let other
| "-boot" `isSuffixOf` f = toNormalizedFilePath' (L.dropEnd 5 $ fromNormalizedFilePath nf)
| otherwise = toNormalizedFilePath' (fromNormalizedFilePath nf ++ "-boot")

@fendor
Copy link
Collaborator

fendor commented Nov 24, 2023

That seems reasonable, and I think I tested master, let me recheck, maybe I am missing something.

@fendor
Copy link
Collaborator

fendor commented Nov 24, 2023

The issue persists on current master, I will debug a little bit further.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: ghcide type: bug Something isn't right: doesn't work as intended, documentation is missing/outdated, etc..
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants