Fix positionMapping in stale data

[soulomoon]

Fix #3919 (comment)

[michaelpj]

I have absolutely no idea how this stuff works. Maybe @wz1000 does?

[soulomoon]

the problem lays in updatePositionMapping.
mapAccumRWithKey accumulate (key,val) decending keys, the delta should be the old ones, instead of new

[soulomoon]

Since it contains version mapping between old versions and current version
then we need to share the new deltas and the old deltas get composed one by one

[soulomoon]

In order to add tests on this, I have refactored the involved code into updatePositionMappingHelper from updatePositionMapping.
Just run cd ghcide; cabal test --test-option="-p /enumMapMappingTest/"

1. Not many understand how is the bug a problem, the test case can serve as explaination.
2. The behaviour of the bug is subtle, the test case can prevent re-introduction. I can see why no one have spotted this before.

I will explain how this is going on,
Suppose you have a text starting from (0,0) let x = 1 in 1(version 0, with x in posision (0, 4)).
For i in [1..5], you are inserting a space at (0, i) consecutively.
After the 5th insertion, the text is now l et x = 1 in 1(version 5, with x in position (0, 9)).
The bug would case the mapping of (0, 4)(version0) to (0, 8)(version5) which should be (0, 9)(version5)

The reason is that application of the mappings is reversed. The last insertion at (0,5), won't sent char at (0,4) to (0,5).

It could be easily reproduced by swtich addOldDelta back to addDelta in updatePositionMappingHelper

enumMapMappingTest :: TestTree
enumMapMappingTest = testCase "enumMapMappingTest" $ do
  let mkChangeEvent :: Range -> Text -> TextDocumentContentChangeEvent
      mkChangeEvent r t = TextDocumentContentChangeEvent $ InL $ #range .== r .+ #rangeLength .== Nothing .+ #text .== t
      mkCE :: UInt -> UInt -> UInt -> UInt -> Text -> TextDocumentContentChangeEvent
      mkCE l1 c1 l2 c2 = mkChangeEvent (Range (Position l1 c1) (Position l2 c2))
      events :: [(Int32, [TextDocumentContentChangeEvent])]
      events = map (second return) [(0, mkCE 0 0 0 0 ""), (1, mkCE 0 1 0 1 " "), (2, mkCE 0 2 0 2 " "), (3, mkCE 0 3 0 3 " "), (4, mkCE 0 4 0 4 " "), (5, mkCE 0 5 0 5 " ")]
      finalMap = Prelude.foldl (\m (i, e) -> updatePositionMappingHelper i e m) mempty events
  let updatePose fromPos = do
        mapping <- snd <$> EM.lookup 0 finalMap
        toCurrentPosition mapping fromPos
  updatePose (Position 0 4) @?= Just (Position 0 9)
  updatePose (Position 0 5) @?= Just (Position 0 10)

[soulomoon]

hi @michaelpj , I have added a test case, it should be easy to spot the bug now.

[wz1000]

Thanks, great catch. All this is quite subtle!

[michaelpj]

Thanks so much for doing the work to add a test!