emacs-lsp / lsp-mode

Emacs client/library for the Language Server Protocol
https://emacs-lsp.github.io/lsp-mode
GNU General Public License v3.0
4.79k stars 888 forks source link

Workspace edits are reversed for no apparent reason #3753

Open slotThe opened 2 years ago

slotThe commented 2 years ago

Thank you for the bug report

Bug description

@akshaymankar and I discovered this while trying to debug why a new Haskell LSP plugin wasn't working correctly. Basically, https://github.com/emacs-lsp/lsp-mode/commit/ec441a1761c9a071b8648f2fd2673043df968d4c added a reverse of the documentChange requests, which we don't quite understand.

Steps to reproduce

A (probably too big) reproducer is as follows:

(let* ((edits "{
    \"documentChanges\": [
      {
        \"edits\": [
          {
            \"newText\": \"  f :: X -> X\\n  f = _\\n\",
            \"range\": {
              \"end\": {
                \"character\": 0,
                \"line\": 13
              },
              \"start\": {
                \"character\": 0,
                \"line\": 13
              }
            }
          }
        ],
        \"textDocument\": {
          \"uri\": \"file:///home/axeman/workspace/haskell-language-server/plugins/hls-class-plugin/test/testdata/DefaultImplementation.hs\",
          \"version\": 0
        }
      },
      {
        \"edits\": [
          {
            \"newText\": \"{-# LANGUAGE InstanceSigs #-}\\n\",
            \"range\": {
              \"end\": {
                \"character\": 0,
                \"line\": 1
              },
              \"start\": {
                \"character\": 0,
                \"line\": 1
              }
            }
          }
        ],
        \"textDocument\": {
          \"uri\": \"file:///home/axeman/workspace/haskell-language-server/plugins/hls-class-plugin/test/testdata/DefaultImplementation.hs\",
          \"version\": 0
        }
      }
    ]
}")
       (input "{-# LANGUAGE DefaultSignatures #-}
module DefaultImplementation where

class Test1 a where

class Test a where
  f :: a -> a
  default f :: Test1 a => a -> a
  f = id

data X = X | Y

instance Test X where

foo = 123"))
  (with-temp-buffer
    (insert input)
    (lsp--apply-workspace-edit (lsp--read-json edits))))

Expected behavior

Given the above input, the expected output is

{-# LANGUAGE DefaultSignatures #-}
{-# LANGUAGE InstanceSigs #-}
module DefaultImplementation where

class Test1 a where

class Test a where
  f :: a -> a
  default f :: Test1 a => a -> a
  f = id

data X = X | Y

instance Test X where
  f :: X -> X
  f = _

foo = 123

but since we apply the change on line one first, the output that we actually get is

{-# LANGUAGE DefaultSignatures #-}
{-# LANGUAGE InstanceSigs #-}
module DefaultImplementation where

class Test1 a where

class Test a where
  f :: a -> a
  default f :: Test1 a => a -> a
  f = id

data X = X | Y

  f :: X -> X
  f = _
instance Test X where

foo = 123

Which Language Server did you use?

haskell-language-server

OS

Linux

Error callstack

No response

Anything else?

The seq-reverse is explicitly mentioned in the commit message, so it seems very intentional. The question now is why that commit was added; is there something in the specification that we missed that specifies the order of different entries in documentChange?

I believe @akshaymankar tried this with neovim's coc (or VSCodium?) and couldn't reproduce it there, so this doesn't seem like standard behaviour across lsp implementations

kiennq commented 2 years ago

The spec of documentChanges said that each edit is for a different document version

/**

  • Depending on the client capability
  • workspace.workspaceEdit.resourceOperations document changes are either
  • an array of TextDocumentEdits to express changes to __n different text
  • documents where each text document edit addresses a specific version of
  • a text document__. Or it can contain above TextDocumentEdits mixed with
  • create, rename and delete file / folder operations.,,, */

Can you try to combine your two TextDocumentEdit into one that contains multiple edits instead to see if it works for you?