Unable to process code using `ImportQualifiedPost` extension

[ncaq]

I've encountered an issue with sandwich when it attempts to process code that contains the ImportQualifiedPost syntax (e.g., import Data.List qualified as L). This causes parse errors because the underlying haskell-src-exts library doesn't support this GHC extension.

Workarounds

I've tried two approaches:

1. Replacing with ghc-lib-parser: I attempted to swap out haskell-src-exts with ghc-lib-parser, but this proved to be quite challenging due to significant API differences and integration complexities.

2. Patching haskell-src-exts: I've submitted a PR to add ImportQualifiedPost support to haskell-src-exts. The fix was relatively simple to implement (added the extension and modified the parser to support the syntax). While there's no guarantee it will be accepted given the maintenance status, it might provide a temporary solution.

feat: add support for ImportQualifiedPost extension by ncaq · Pull Request #477 · haskell-suite/haskell-src-exts

Discussion

I'd like to discuss options for moving forward:

1. Should sandwich consider migrating away from haskell-src-exts to ghc-lib-parser in the long term?
2. If my PR is accepted, would you consider updating the dependency in sandwich?
3. Are there any other workarounds you'd recommend in the meantime?

For context, the reason I'm using ImportQualifiedPost syntax is that it's part of the formatting standards enforced by fourmolu in my project.

[thomasjm]

Oh no, sorry to hear that haskell-src-exts is causing so much trouble. I've long wanted to get rid of it, it's a heavy dependency and it's literally only used for this function.

The function in question is only used to support the --list-tests argument, and running individual tests that export their own main function. It's not a feature that I use frequently myself, so maybe it could just be removed.

I think moving to ghc-lib-parser could be a reasonable solution, although that might be considered a heavy dependency too?

[thomasjm]

A part of me wants to just come up with some regexes for detecting if a Haskell file has a top-level main declaration and/or module export and call it a day.

[ncaq]

I agree with your thoughts on removing haskell-src-exts. Given that it's no longer being maintained, this seems like a necessary step.

However, I share your concerns about ghc-lib-parser being an even "heavier" dependency than haskell-src-exts. The build process is resource-intensive, and API changes between GHC versions often introduce breaking changes. Documentation from slightly different versions frequently becomes incompatible with other versions.

It feels odd for me to say this since I was trying to replace it myself, but using ghc-lib-parser would likely force sandwich to only work with specific GHC versions unless you implement extensive CPP macro branching. And from my personal experience, it's quite difficult to use - haskell-src-exts was much more user-friendly.

I agree that introducing these dependencies just to check for declarations seems excessive and a lighter approach makes sense. However, I'd advise against using regular expressions, which would almost certainly lead to incorrect detection. At minimum, a parser combinator like megaparsec would be better, though covering all Haskell corner cases would still be challenging.

Perhaps we should look for alternative solutions. Maybe ghc-exactprint could be considered as a successor to haskell-src-exts? I haven't tried it yet, and it seems more focused on code generation. It also depends on the heavy ghc package, which is concerning.

What would be ideal is a wrapper around ghc-lib-parser with a limited API for lightweight use cases like this, designed to work across various versions. I haven't found such a package yet - perhaps I should develop it myself.

[thomasjm]

If ghc-lib-parser is resource intensive to build, that makes it pretty unattractive -- that's some of the feedback I got when I initially published Sandwich, that it's important for a test package like this to build relatively quickly in CI even when its dependencies aren't cached.

I was thinking about it a little more and thinking the manual parsing approach could make a lot of sense, mainly because the stakes are so low. (Sorry, I didn't mean to say regexes, I meant using some kind of parser combinator library, although hopefully a lighter one than megaparsec.) Here's what happens if we get it wrong:

• If we fail to detect a main function that is present, the user won't be able to run that test individually with a flag. Not such a big deal, and we could also surface a compilation warning if the parsing process finds something ambiguous.
• If we falsely detect a main function when one is not present, the user will get a compilation error (because the TH will try to import this main function) and this should also point the user in the right direction.

Concretely, I was thinking the following:

• Write some simple parsing code to strip comments out of a file.
• Come up with a parser that looks for top-level declarations, in order to see if one called main is present.
• Come up with a parser that looks for module SomeName where or module SomeName (...) where. In the latter case, we'd do some simple parsing on the export list to see if main is exported.

The goal wouldn't be to get 100% correctness, but doing a pretty good job on it and getting rid of these dependencies would be worth it IMO. What do you think?

[ncaq]

I believe manual analysis is a viable approach here.

We should be careful as unexpected test failures can lead to builds passing that shouldn't, which is problematic. Conversely, if false positives occur and cause compilation errors, Haskell users are quite familiar with build errors, so users could report back to this issue and temporarily downgrade sandwich as a workaround.

I don't think megaparsec has particularly heavy dependencies. In fact, in this specific case, it's relatively lightweight since sandwich already depends on megaparsec indirectly through this chain:
brick -> config-ini -> megaparsec

So adding megaparsec directly wouldn't introduce any additional dependencies.

There are various implementation techniques we could choose. Whichever approach we take, we'll likely need to make adjustments during implementation anyway.

[thomasjm]

I've had good luck with asking LLMs to generate parsers. I tried it with this and got the following (not working yet): https://github.com/codedownio/sandwich/tree/attoparsec-main-detection. This is the shape of the solution I was thinking, if you'd like to play with it.