Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

inconsistent textDocument/completion results #28

Closed
vytautassurvila opened this issue Apr 1, 2022 · 7 comments
Closed

inconsistent textDocument/completion results #28

vytautassurvila opened this issue Apr 1, 2022 · 7 comments

Comments

@vytautassurvila
Copy link
Contributor

On vscode I get inconsistent completion results. I depends on how auto-completion was invoked - by trigger character or invoked via shortcut. Wild guess would be that autocompletion is done based on code that was before typing trigger character (trigger character is not accounted).

Sample file I tested on

namespace tests.proj._2
{
    public class Tests
    {
        public void Setup()
        {
        }

        public void Proj2_Test1()
        {
            this.
        }
    }
}

When I type this. I get autocompletion with items as, is, switch, with. Note that these options are valid for this (with space after it). Messages that are being sent to csharp-ls:

[LSP   - 8:12:37 PM] {"isLSPMessage":true,"type":"send-notification","message":{"jsonrpc":"2.0","method":"textDocument/didChange","params":{"textDocument":{"uri":"file:///home/vytas/code/vscode/dotnettests/tests.proj.2/UnitTest1.cs","version":127},"contentChanges":[{"range":{"start":{"line":10,"character":16},"end":{"line":10,"character":16}},"rangeLength":0,"text":"."}]}},"timestamp":1648833157228}
[LSP   - 8:12:37 PM] {"isLSPMessage":true,"type":"send-request","message":{"jsonrpc":"2.0","id":603,"method":"textDocument/completion","params":{"textDocument":{"uri":"file:///home/vytas/code/vscode/dotnettests/tests.proj.2/UnitTest1.cs"},"position":{"line":10,"character":17},"context":{"triggerKind":2,"triggerCharacter":"."}}},"timestamp":1648833157229}
[LSP   - 8:12:37 PM] {"isLSPMessage":true,"type":"receive-notification","message":{"jsonrpc":"2.0","method":"textDocument/publishDiagnostics","params":{"uri":"file:///home/vytas/code/vscode/dotnettests/tests.proj.2/UnitTest1.cs","diagnostics":[{"range":{"start":{"line":10,"character":17,"debuggerDisplay":"(10,17)"},"end":{"line":10,"character":17,"debuggerDisplay":"(10,17)"},"debuggerDisplay":"(10,17)-(10,17)"},"severity":1,"source":"lsp","message":"Identifier expected","debuggerDisplay":"[Error] ((10,17)-(10,17)) Identifier expected ()"},{"range":{"start":{"line":10,"character":17,"debuggerDisplay":"(10,17)"},"end":{"line":10,"character":17,"debuggerDisplay":"(10,17)"},"debuggerDisplay":"(10,17)-(10,17)"},"severity":1,"source":"lsp","message":"; expected","debuggerDisplay":"[Error] ((10,17)-(10,17)) ; expected ()"}]}},"timestamp":1648833157235}
[LSP   - 8:12:37 PM] {"isLSPMessage":true,"type":"receive-response","message":{"jsonrpc":"2.0","id":603,"result":{"isIncomplete":false,"items":[{"label":"as","kind":14,"insertTextFormat":1},{"label":"is","kind":14,"insertTextFormat":1},{"label":"switch","kind":14,"insertTextFormat":1},{"label":"with","kind":14,"insertTextFormat":1}]}},"timestamp":1648833157236}

But if I close autocompletion and trigger it via shortcut then I get correct items Equals, Setup, ToString and others. csharp-ls messages:

[LSP   - 8:15:39 PM] {"isLSPMessage":true,"type":"send-request","message":{"jsonrpc":"2.0","id":620,"method":"textDocument/completion","params":{"textDocument":{"uri":"file:///home/vytas/code/vscode/dotnettests/tests.proj.2/UnitTest1.cs"},"position":{"line":10,"character":17},"context":{"triggerKind":1}}},"timestamp":1648833339323}
[LSP   - 8:15:39 PM] {"isLSPMessage":true,"type":"receive-response","message":{"jsonrpc":"2.0","id":620,"result":{"isIncomplete":false,"items":[{"label":"Equals","kind":2,"insertTextFormat":1},{"label":"GetHashCode","kind":2,"insertTextFormat":1},{"label":"GetType","kind":2,"insertTextFormat":1},{"label":"MemberwiseClone","kind":2,"insertTextFormat":1},{"label":"Proj2_Test1","kind":2,"insertTextFormat":1},{"label":"Setup","kind":2,"insertTextFormat":1},{"label":"ToString","kind":2,"insertTextFormat":1}]}},"timestamp":1648833339332}

Note that it's not always reproducible. I guess it somehow depends on file size too. As when working with big files it happens more often than with this sample program.

The question is can you reproduce that on other clients? If needed I probably could implement some hack/fix on vscode language client to wait for textDocument/didChange response (maybe event add some delay) and only then emit textDocument/completion

@razzmatazz
Copy link
Owner

The question is can you reproduce that on other clients? If needed I probably could implement some hack/fix on vscode language client to wait for textDocument/didChange response (maybe event add some delay) and only then emit textDocument/completion

I suspect this is because of something not working with incremental document sync. I.e. the editor and LSP server could be out of sync if incremental sync doesn't work right and diffs are not integrated properly or in wrong order from th editor

Do you get spurious validation errors too on large buffers? I do sometimes (on emacs at least) -- if confirmed that would indicate that we need to work on document syncing.

In the meantime I could provide an option to disable incremental sync and we could check if that helps

@vytautassurvila
Copy link
Contributor Author

Based on logs I added earlier I guess that client does not wait till textDocument/didChange is fully processed and emits textDocument/completion that works with old file version. Its just 1 millisecond away:

"method":"textDocument/didChange" ...  "timestamp":1648833157228
"method":"textDocument/completion" ... "timestamp":1648833157229

My naive approach would be to try to delay processing of handleTextDocumentCompletion till handleTextDocumentDidChange did not fully complete. Just maybe this delay could be implemented via withReadOnlyScope and withReadWriteScope - do not allow to enter readonly scope while at least one write scope is in progress?

@razzmatazz
Copy link
Owner

razzmatazz commented Apr 2, 2022

I think I will have to pimp the request scheduler a bit so that parallel r/w and r/o requests are not run. This was a bit of an optimization which bite me in the butt. I expected for the editor to serialize r/o requests to be run AFTER editor receives response from pending r/w request(s) but now it is obvious this is a responsibility of the server.

can you try changing this line in Server.fs:

        "textDocument/completion"        , handleTextDocumentCompletion        |> withReadOnlyScope |> requestHandling

to

        "textDocument/completion"        , handleTextDocumentCompletion        |> withReadWriteScope |> requestHandling

?

^^^ this should enforce proper serialization for until I fix the locking/serialization mechanism

@vytautassurvila
Copy link
Contributor Author

Yey, "textDocument/completion" with withReadWriteScope solves the issue for vscode client.

@razzmatazz razzmatazz reopened this Apr 3, 2022
@razzmatazz
Copy link
Owner

razzmatazz commented Apr 3, 2022

@vytautassurvila could you check if your problems are fixed with 21601a8?

I have forced for r/o requests that are subsequent to a r/w one to be queued and run only after the r/w finished

@vytautassurvila
Copy link
Contributor Author

@vytautassurvila could you check if your problems are fixed with 21601a8?

Thanks, it solves issue with test project! Tomorrow I will be able to test more extensively with real projects.

@razzmatazz
Copy link
Owner

ok, 0.4.3 has been released, closing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants