Skip to content

Commit 0600b2b

Browse files
committed
User a looser comparison to match Diagnostic objects
Discovered while investigating issue haskell#3857. Technically, there are only two required fields in an incoming `Diagnostic` object: `range` and `message`. However, the HLS was comparing all fields to determine equality. This caused mismatches when neovim stopped sending the (optional) `tags` field.
1 parent c3abd82 commit 0600b2b

File tree

1 file changed

+12
-2
lines changed
  • plugins/hls-refactor-plugin/src/Development/IDE/Plugin

1 file changed

+12
-2
lines changed

plugins/hls-refactor-plugin/src/Development/IDE/Plugin/CodeAction.hs

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -442,6 +442,16 @@ suggestRemoveRedundantImport ParsedModule{pm_parsed_source = L _ HsModule{hsmod
442442
| otherwise = []
443443

444444

445+
-- Using a looser equality check, determines if a given `Diagnostic` is present
446+
-- in a list. Compares using *only* required fields for this type as defined by:
447+
-- https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#diagnostic
448+
-- See https://github.com/haskell/haskell-language-server/issues/3857 for more
449+
-- context.
450+
diagIsElemOf :: Diagnostic -> [Diagnostic] -> Bool
451+
diagIsElemOf d ds = any (requiredFieldsEqual d) ds
452+
where
453+
requiredFieldsEqual Diagnostic{_range=r1, _message=m1} Diagnostic{_range=r2, _message=m2} = r1 == r2 && m1 == m2
454+
445455
-- Note [Removing imports is preferred]
446456
-- It's good to prefer the remove imports code action because an unused import
447457
-- is likely to be removed and less likely the warning will be disabled.
@@ -453,7 +463,7 @@ caRemoveRedundantImports m contents digs ctxDigs uri
453463
r <- join $ map (\d -> repeat d `zip` suggestRemoveRedundantImport pm contents d) digs,
454464
allEdits <- [ e | (_, (_, edits)) <- r, e <- edits],
455465
caRemoveAll <- removeAll allEdits,
456-
ctxEdits <- [ x | x@(d, _) <- r, d `elem` ctxDigs],
466+
ctxEdits <- [ x | x@(d, _) <- r, d `diagIsElemOf` ctxDigs],
457467
not $ null ctxEdits,
458468
caRemoveCtx <- map (\(d, (title, tedit)) -> removeSingle title tedit d) ctxEdits
459469
= caRemoveCtx ++ [caRemoveAll]
@@ -488,7 +498,7 @@ caRemoveInvalidExports m contents digs ctxDigs uri
488498
allRanges <- nubOrd $ [ range | (_,_,ranges) <- r, range <- ranges],
489499
allRanges' <- extend txt' allRanges,
490500
Just caRemoveAll <- removeAll allRanges',
491-
ctxEdits <- [ x | x@(_, d, _) <- r, d `elem` ctxDigs],
501+
ctxEdits <- [ x | x@(_, d, _) <- r, d `diagIsElemOf` ctxDigs],
492502
not $ null ctxEdits
493503
= caRemoveCtx ++ [caRemoveAll]
494504
| otherwise = []

0 commit comments

Comments
 (0)