Skip to content

Fix deleting unused bindings with Haddock docs#4881

Open
agam263 wants to merge 1 commit intohaskell:masterfrom
agam263:codex/agamkundu-fix-4876-delete-unused-haddock
Open

Fix deleting unused bindings with Haddock docs#4881
agam263 wants to merge 1 commit intohaskell:masterfrom
agam263:codex/agamkundu-fix-4876-delete-unused-haddock

Conversation

@agam263
Copy link
Copy Markdown

@agam263 agam263 commented Apr 2, 2026

Closes #4876

What changed

This fixes the refactor plugin's Delete ‘…’ code action so it removes attached Haddock documentation when deleting an unused binding.

The change covers:

  • line Haddock comments like -- | and -- ^
  • block Haddock comments like {-| and {-^

It also adds regression tests for both forms.

Why

When an unused top-level binding had Haddock immediately above it, the delete-unused-binding quick fix removed the binding but left the documentation behind. That produced orphaned docs and invalid or misleading source after applying the code action.

Root cause

The delete action built text edits from declaration and signature spans, but those spans did not account for attached Haddock comments. As a result, the edit range started too late and excluded the documentation block.

There was also a follow-up issue with overlapping deletion ranges once the Haddock span was included. That is now handled by sorting and merging the related ranges before producing text edits.

Implementation details

  • Extend deletion ranges upward to include attached Haddock blocks
  • Preserve the existing behavior for ordinary whitespace/newline trimming
  • Merge overlapping ranges before constructing the final workspace edits
  • Add regression tests for:
    • line Haddock above an unused top-level binding
    • block Haddock above an unused top-level binding

Validation

Ran locally with:

cabal test hls-refactor-plugin-tests --with-compiler=$HOME/.ghcup/bin/ghc-9.10.3

@agam263 agam263 requested a review from santiweight as a code owner April 2, 2026 04:59
@vidit-od vidit-od self-requested a review April 2, 2026 08:02
Comment on lines +2552 to +2570
, testSession "delete unused top level binding with block Haddock comment" $
testFor
[ "{-# OPTIONS_GHC -Wunused-top-binds #-}"
, "module A (some) where"
, ""
, "{-| docs for f"
, "-}"
, "f :: Int"
, "f = 1"
, ""
, "some = ()"
]
(6, 0)
"Delete ‘f’"
[ "{-# OPTIONS_GHC -Wunused-top-binds #-}"
, "module A (some) where"
, ""
, "some = ()"
]
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

20260402-0820-18.0224737.mp4

It seems weird, i cannot replicate the Behavior
But the tests do Pass.

@fendor fendor added the status: needs review This PR is ready for review label Apr 13, 2026
@fendor
Copy link
Copy Markdown
Collaborator

fendor commented Apr 15, 2026

Hi, thank you for your contribution!

It looks to me like you are manually computing the haddock comment range.
Is there no way to compute the range based on the AST where we know the range of the haddock comment attached to the AST node we are removing?

If we do it stringly typed, I feel like there are corner cases we might miss.

For example:

-- | Comment
foo = ()

some = ()

If we delete some, how do we determine whether -- | Comment belongs to foo or some?


What about

{-
-- | Commented out
   -}
some = ()

Would that delete Commented out? It shouldn't!


What about

some = ()
-- ^ Comment

Do we perform arbitrary look ahead to find trailing haddock comments?


Similar and complicated:

some = ()
-- Comment
-- ^ Haddock comment

If we remove some, should Comment be deleted? It is not a haddock comment...


I imagine there are many more corner cases, and I think we need to look into the AST and figure out the correct haddock comment to compute the Range to delete.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status: needs review This PR is ready for review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Delete unused binding code action doesn't delete haddock

3 participants