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

C# completion - candidates don't match the context #1108

Closed
sebasmonia opened this issue Oct 16, 2019 · 15 comments
Closed

C# completion - candidates don't match the context #1108

sebasmonia opened this issue Oct 16, 2019 · 15 comments

Comments

@sebasmonia
Copy link

Describe the bug

In C#, completion suggests items that are outside the current context. Example, if I type this. I would expect to get a list of items for the current class, instead I get...basically everything (namespaces, functions, etc.)

image

Also in some circumstance I get...almost nothing:

image

Which Language Server did you use
Omsnisharp-roslyn

OS
Windows 10

Error callstack
No error

Can't include logs from my current project, but can try to generate a minimal solution soon.

@yyoncho
Copy link
Member

yyoncho commented Oct 17, 2019

lsp-mode is responsible only for showing the completions, not for calculating them. The server is doing that. So we should gather enough information and report it upstream.

@sebasmonia
Copy link
Author

Hi, sorry it took me this long to get back to you. I tried a minimal project and got the same result.
I set (setq lsp-print-io t) but couldn't see any log info in a buffer or Messages? Couldn't gather from the lsp-mode code where it logs :)

I tested with a different LSP package the completion results are different:

image

Compared to lsp-mode, which for that context is picking up some global keywords:

image

Since the test project is not part of a repository I also get a warning that the file is not in the project or it is blacklisted, don't know if that's related?
Anyway this comparison is why I think the issue might be some configuration between lsp-mode and the omnisharp server, rather than server error.

I know and appreciate how much the lsp-mode team cares about a good out of the box experience so if we can adjust something to make it better, let's give it a try!

@yyoncho
Copy link
Member

yyoncho commented Oct 23, 2019

Hi, sorry it took me this long to get back to you. I tried a minimal project and got the same result.
I set (setq lsp-print-io t) but couldn't see any log info in a buffer or Messages?

Use lsp-workspace-show-logs.

Since the test project is not part of a repository I also get a warning that the file is not in the project or it is blacklisted, don't know if that's related?

This message means that lsp-mode is not started in the buffer. Can you verify that by lsp-describe-session? In case it is not part of a project, please make sure that you have lsp-auto-guess-root set no nil and reopen the project? If you still get that message you have probably blacklisted the project - use lsp-workspace-blacklist-remove to fix that.

@sebasmonia
Copy link
Author

lsp-auto-guess-root is set to t, after reading that variable I created a .projectile file and now LSP starts when I open the buffer.

Completion is back to the very first screenshot, where after the dot I get everything under the sun.

The log buffer shows the data below, which seems like an Omnisharp problem, but I see similar exceptions for eglot and still get completion, so maybe there's a workaround.

One thing that I recalled now looking at my config, is that sometimes the server sends a nil response for markup, and I adviced a function in eglot to replace nil with empty string to avoid breakage.

I see no errors for lsp though.

Handler Selected: OmniSharp.LanguageServerProtocol.Handlers.OmniSharpSignatureHelpHandler via [**/*.cs], [**/*.csx] (targeting OmniSharp.Extensions.JsonRpc.IJsonRpcRequestHandler`2[[OmniSharp.Extensions.LanguageServer.Protocol.Models.SignatureHelpParams, OmniSharp.Extensions.LanguageProtocol, Version=0.13.0.0, Culture=neutral, PublicKeyToken=6d868dff454e6022],[OmniSharp.Extensions.LanguageServer.Protocol.Models.SignatureHelp, OmniSharp.Extensions.LanguageProtocol, Version=0.13.0.0, Culture=neutral, PublicKeyToken=6d868dff454e6022]])
Starting: Routing Request (36) textDocument/signatureHelp
Converting params for Request (36) textDocument/signatureHelp to OmniSharp.Extensions.LanguageServer.Protocol.Models.SignatureHelpParams
Failed to handle notification textDocument/signatureHelp - System.NullReferenceException: Object reference not set to an instance of an object.

   at OmniSharp.LanguageServerProtocol.Handlers.OmniSharpSignatureHelpHandler.<Handle>d__3.MoveNext() in D:\a\1\s\src\OmniSharp.LanguageServerProtocol\Handlers\OmniSharpSignatureHelpHandler.cs:line 45

--- End of stack trace from previous location where exception was thrown ---

   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()

   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)

   at OmniSharp.Extensions.LanguageServer.Server.Pipelines.ResolveCommandPipeline`2.<Handle>d__5.MoveNext()

--- End of stack trace from previous location where exception was thrown ---

   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()

   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)

   at OmniSharp.Extensions.JsonRpc.RequestRouterBase`1.<RouteRequest>d__6.MoveNext()
Finished: Routing Request (36) textDocument/signatureHelp in 0ms

@yyoncho
Copy link
Member

yyoncho commented Oct 28, 2019

I guess there is slight differences in the initialization request which breaks lsp-mode. Can you include the client/server log from both?

@sebasmonia
Copy link
Author

lsp-log.txt
eglot-events.txt

Attached the two logs. Hope you find them useful! The program code is very simple, line 15 char 20 is where completion is triggered.

using System;

namespace testapp
{
    class Program
    {
        private static int a_number;

        static void Main(string[] args)
        {

            System.Diagnostics.Debugger.Launch();
            System.Diagnostics.Debugger.Break();
            Console.WriteLine("Hello World!");
            Program.
            System.Diagnostics.Debugger.Break();
            Console.WriteLine("Bye  World!");
        }
    }
}

@razzmatazz
Copy link
Collaborator

The issue here was that omnisharp-roslyn didn't sync the file properly. I have a PR for omnisharp-roslyn that fixes this (at least for me):

@sebasmonia
Copy link
Author

Thinking we should close this since #1125 is tracking it.

@yyoncho or @razzmatazz do you know why I can work fine with Omnisharp in Eglot, but needs the additional fix for lsp-mode?

And thank you for the follow up in #1125 and PRs submitted to Omnisharp-roslyn. Can't wait to test lsp-mode w/ Roslyn :)

@yyoncho
Copy link
Member

yyoncho commented May 14, 2020

@sebasmonia IIRC it is side effect from lsp-mode using hash-tables and eglot using plists. If the server returns nil (incorrectly) the hashtable operation will fail while plist operation will be successful. Generally, we could add guards against this invalid input. But AFAIK the server is in bad state either way, right, @razzmatazz ?

@sebasmonia
Copy link
Author

Thank you!
Like I said, I'm ok to close this issue.

@razzmatazz
Copy link
Collaborator

razzmatazz commented May 15, 2020

@sebasmonia IIRC it is side effect from lsp-mode using hash-tables and eglot using plists. If the server returns nil (incorrectly) the hashtable operation will fail while plist operation will be successful. Generally, we could add guards against this invalid input. But AFAIK the server is in bad state either way, right, @razzmatazz ?

I am not the original developer of omnisharp server nor someone who is involved a lot, but it appears LSP was/is a second-class operation mode (in contrast to the native custom protocol) of omnisharp-roslyn server thus there are still rough edges on LSP implementation. It returns null/invalid responses because internally LSP is a layer on top of a native omnisharp protocol bits where the concepts do not always map 1:1 to LSP thus the impedance mismatch, and frankly, the LSP layer was not getting tested that much so far.

Personally, I would not be bothering with adding workarounds in emacs-lsp mode just for omnisharp-roslyn bugs in the current state of development. IMHO, the server needs fixing, not the client, in this case. @yyoncho

On a positive note, the library that omnisharp-roslyn uses to expose LSP functionality is seemingly getting a lot of development time and in extension omnisharp-roslyn integration with LSP will definitely get better with time:

Hopefully, with the basics working, we will get more C# users of LSP mode of omnisharp-roslyn server and thus more fixes in the form of PRs to omnisharp-roslyn :)

@sebasmonia
Copy link
Author

Created OmniSharp/omnisharp-roslyn#1806, posting here since the other ticket is closed now.

@yyoncho
Copy link
Member

yyoncho commented Jun 19, 2020

@razzmatazz do you think there is anything to do or this issue on lsp-mode side?

@razzmatazz
Copy link
Collaborator

@yyoncho I am pretty sure the original issue (related to documents-being-out-of-sync) has been fixed with the latest versions of the server.

@yyoncho
Copy link
Member

yyoncho commented Jun 19, 2020

Thank you, I am closing the issue.

@yyoncho yyoncho closed this as completed Jun 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants