Fix deleting unused bindings with Haddock docs V2#4965
Conversation
9ac310a to
784c49c
Compare
| , testSession "delete unused top level binding with Haddock comment" $ | ||
| testFor | ||
| [ "{-# OPTIONS_GHC -Wunused-top-binds #-}" | ||
| , "module A (some) where" | ||
| , "" | ||
| , "-- | line docs for f" | ||
| , "f :: Int" | ||
| -- TODO: , "-- ^ trailing docs for f" |
There was a problem hiding this comment.
@fendor Could you please help me decide what to do? There are a two problems with my current version:
- The GHC AST changed a lot in 9.10 and the types do not match. It would mean writing the code that finds comments again. Should I dive into type families, or disable (how?) this test for GHC 9.6/.8?
- Apply.Refact.Compat could help, but that does not in turn work on 9.10
- Trailing Haddocks might not be in the same place after GHC does the parsing, the test would not pass. Maybe it's connected to this note about GHC parser only adding "prior comments":
https://gitlab.haskell.org/ghc/ghc/-/blob/master/compiler/GHC/Parser/Annotation.hs?ref_type=heads#L406-413- Trailing style for function declaration seems niche to me, I only used it on record fields. Is this worth fixing, or is there maybe some better example?
There was a problem hiding this comment.
@alanz may have some insights - i'm too far away
There was a problem hiding this comment.
The parser emits an AST with the comments preceding only, but ghc-exactprint has balanceComments which spreads them as appropriate. And if you follow that link the comment talks about how exact print processes haddock comments (TL;DR: it ignores them, they show up as regular comments too).
Also, if you use the HasDecls facilities I think it might balance comments when you ask for the decls. But I may be wrong. But after balancing comments you should be able to just remove the decl from the ast, and the comments go with it.
There was a problem hiding this comment.
Thanks a lot @alanz! 👍 That does indeed put the docs on the function signature. Except in this case:
foo :: Int
-- ^ Trailing doc?
foo = 42This seems like a weird edge-case to me, where the exact print decided to leave the doc on foo value definition instead of consistently moving it to the function signature. I only found out, because it was the first test I hastily wrote. I am not sure if this is a bug in ghc-exactprint.
It would be annoying to get the comments from value definition as it would require some CPP magic. 😕
There was a problem hiding this comment.
In terms of exact print, it treats haddock comments as normal comments, and ignored the specially haddock-processed ones,
When it balances comments, it assumes a comment stays where it is unless there is a blank line to make it clear.
So
foo :: Int
-- ^ Trailing doc?
foo = 42would balance the comment as trailing to the signature.
Perhaps an enhancement to balanceComments would be to use the haddock comment prefixes to decide on the comment balancing. PR on ghc-exactprint welcome
784c49c to
ecf65d7
Compare
Co-authored-by: kunduagam23@gmail.com
* Tweak unused binding test * Refactor findRelatedSigSpan to avoid inner let * Use epaLocationRealSrcSpan * Revert unused annotated type * Fixup for GHC 9.10 * Just give up for GHC=<9.8 * Add comments
ecf65d7 to
75647a6
Compare
L EpAnn SigD, not on the definitionValD