Text_document has bugs when performing updates and getting positions

[faldor20]

The Text_document module has some weird bugs:
These expect tests show the issue:

let%expect_test "replace second line first line is \\n" =
  let range = tuple_range (1, 2) (1, 2) in
  let doc = make_document (Uri.of_path "foo.ml") ~text:"\nfoo\nbar\nbaz\n" in
  let edit = TextEdit.create ~newText:"change" ~range in
  let newDoc = Text_document.apply_text_document_edits doc [ edit ] in
  newDoc |> Text_document.text |> String.escaped |> print_endline;
  [%expect {|
    result: \nfochangeo\nbar\nbaz\n |}]

let%expect_test "get position after change" =
  let range = tuple_range (1, 2) (1, 2) in
  let doc = make_document (Uri.of_path "foo.ml") ~text:"\nfoo\nbar\nbaz\n" in
  let edit = TextDocumentContentChangeEvent.create ~text:"change" ~range () in
  let newDoc = Text_document.apply_content_changes doc [ edit ] in
  let pos = Text_document.absolute_position newDoc range.start in
  newDoc |> Text_document.text |> String.escaped |> print_endline;
  printf "pos: %d\n" pos;
  [%expect {|
    pos: 3 |}]

Test one demonstrates how making the change causes text to become duplicated
Test two demonstrates how after making a change the absolute position comes out messed up.
I noticed that in my testing the end_ position when running absolute_range seemed unaffected, maybe that helps.

I discovered these issues while trying to use the lsp as a library in another project, they were very very confusing to debug 😅

I've noticed that sometimes it seems ocmallsp kind of gets confused and has to be restarted, I suspect this is a contributor to that issue as it could cause the editor and language-service to go out of sync.

I wonder if we could set up a property based test for the text_Document that run's the same edit's on a string using a simpler method than the zipper, then just feed in a huge sample of random positions and change text and see what happens. Just a thought.

[faldor20]

Another possibility is to have the lsp check the on disk file, when it gets a file-saved notification to see if the current editor file has diverged from the file on disk, if so it could freak out and also load the on-disk file

[rgrinberg]

I don't understand the issue with the first test. As far as I see, you're inserting the string change at the 3rd position on the second line and the output demonstrates that's exactly what happens.

[faldor20]

ahh, sorry, the tests actually fail. The test shows what should happen, if you actually run them you see the output is wrong.
I made an attempt at fixing this a while ago and got some of the way there, from memory there are two issues

1. The beginning_of_line function within the string_zipper sometimes puts you on the previous line and leaves you there.
   Which was fixed by the below change adding a check to see if you've been put on the wrong line

let beginning_of_line t =
  let line=t.line in
  let t = prev_newline t in
  if is_begin t &&t.line=line  then t else advance_char t

2. drop_until leaves the abs_pos completely borked( like a position longer than the entire string). This means when you call Text_document.absolute_position which intern calls String_zipper.offset the output is all over the place. I tried a few things to try and fix that, but I ran out of time and couldn't get it quite right.

[rgrinberg]

I attempted to reproduce in #1216. The first test seems to pass. Haven't looked closely at the second one yet.

[faldor20]

Oh okay, I'll test it myself again but my results were

\nfochangeo\nfoo\nbar\nbaz\n

You can see foo gets duplicated

[rgrinberg]

Thanks for the bug report @faldor20 and @emilienlemaire for the fix