lsp-mode icon indicating copy to clipboard operation
lsp-mode copied to clipboard

Support dynamic registration for `textDocument/diagnostic`

Open sin-ack opened this issue 9 months ago • 5 comments

As far as I can tell, the new csharp-roslyn language server only supports pull diagnostics (#4280) if the client supports dynamic registration:

https://github.com/dotnet/roslyn/blob/f296737ba49ebf723d3e119284a3955a00f3bcce/src/LanguageServer/Protocol/Handler/Diagnostics/Public/PublicDocumentPullDiagnosticsHandler_IOnInitialized.cs#L22

lsp-mode currently doesn't support this (sending diagnostic: dynamicRegistration: false), and thus it's not possible to see diagnostics in Emacs at the moment. Other features of the language server work fine, but this is currently a blocker for me to switch away from OmniSharp (which has its own set of issues).

sin-ack avatar Mar 18 '25 14:03 sin-ack

~~I've been advice-add-ing functions to fiddle with enabling dynamicRegistration so that we can get diagnostics with roslyn. I got it working .. somewhat. Here are my findings~~ ~~1. Just enabling dymamicRegistration is not enough. When requesting textDocument/diagnostic, we have to send the identifier that was sent in response to the initial registration. lsp-mode sends identifier:nil by default. With roslyn, these identifiers are~~

syntax
DocumentAnalyzerSyntax
DocumentCompilerSemantic
DocumentAnalyzerSemantic
NonLocal
XamlDiagnostics
enc
HotReloadDiagnostics
WorkspaceDocumentsAndProject 

~~I tried looping over these identifiers and sending a textDocument/diagnostic for each of them. But then the server sends independent results back which clobber each other. Most often, the DocumentAnalyzerSyntax was the last one sent by the server, and that is a list of code improvements (severity: 4). To get to see the actual errors, I ended up sending textDocument/diagnostic requests with just DocumentCompilerSemantic as the identifier. With that, the server only returns errors .~~

Finally, I think I need to send a request for workspace/diagnostic to get the server to return project wide errors. I'll be experimenting with that next.

EDITED - READ NEXT COMMENT :)

gamedolphin avatar Apr 23 '25 07:04 gamedolphin

Okay. Most of the above comment is just wrong. :) I got carried away in implementing textDocument/diagnostic properly. 🤦 just adding the capability was enough for diagnostics to show up for me.

(require 'lsp-mode)

(defun custom-capabilities-override (&optional custom-capabilities)
  "Return the client capabilities appending CUSTOM-CAPABILITIES."
  (append
   `((general . ((positionEncodings . ["utf-32", "utf-16"])))
     (workspace . ((workspaceEdit . ((documentChanges . t)
                                     (resourceOperations . ["create" "rename" "delete"])))
                   (applyEdit . t)
                   (symbol . ((symbolKind . ((valueSet . ,(apply 'vector (number-sequence 1 26)))))))
                   (executeCommand . ((dynamicRegistration . :json-false)))
                   ,@(when lsp-enable-file-watchers '((didChangeWatchedFiles . ((dynamicRegistration . t)))))
                   (workspaceFolders . t)
                   (configuration . t)
                   ,@(when lsp-semantic-tokens-enable
                       `((semanticTokens . ((refreshSupport . ,(or (and (boundp 'lsp-semantic-tokens-honor-refresh-requests)
                                                                        lsp-semantic-tokens-honor-refresh-requests)
                                                                   :json-false))))))
                   ,@(when lsp-lens-enable '((codeLens . ((refreshSupport . t)))))
                   ,@(when lsp-inlay-hint-enable '((inlayHint . ((refreshSupport . :json-false)))))
                   (diagnostics . ((refreshSupport . :json-false)))
                   (fileOperations . ((didCreate . :json-false)
                                      (willCreate . :json-false)
                                      (didRename . t)
                                      (willRename . t)
                                      (didDelete . :json-false)
                                      (willDelete . :json-false)))))
     (textDocument . ((declaration . ((dynamicRegistration . t)
                                      (linkSupport . t)))
                      (definition . ((dynamicRegistration . t)
                                     (linkSupport . t)))
                      (references . ((dynamicRegistration . t)))
                      (implementation . ((dynamicRegistration . t)
                                         (linkSupport . t)))
                      (typeDefinition . ((dynamicRegistration . t)
                                         (linkSupport . t)))
                      (synchronization . ((willSave . t) (didSave . t) (willSaveWaitUntil . t)))
                      (documentSymbol . ((symbolKind . ((valueSet . ,(apply 'vector (number-sequence 1 26)))))
                                         (hierarchicalDocumentSymbolSupport . t)))
                      (formatting . ((dynamicRegistration . t)))
                      (rangeFormatting . ((dynamicRegistration . t)))
                      (onTypeFormatting . ((dynamicRegistration . t)))
                      ,@(when (and lsp-semantic-tokens-enable
                                   (functionp 'lsp--semantic-tokens-capabilities))
                          (lsp--semantic-tokens-capabilities))
                      (rename . ((dynamicRegistration . t) (prepareSupport . t)))
                      (codeAction . ((dynamicRegistration . t)
                                     (isPreferredSupport . t)
                                     (codeActionLiteralSupport . ((codeActionKind . ((valueSet . [""
                                                                                                  "quickfix"
                                                                                                  "refactor"
                                                                                                  "refactor.extract"
                                                                                                  "refactor.inline"
                                                                                                  "refactor.rewrite"
                                                                                                  "source"
                                                                                                  "source.organizeImports"])))))
                                     (resolveSupport . ((properties . ["edit" "command"])))
                                     (dataSupport . t)))
                      (completion . ((completionItem . ((snippetSupport . ,(cond
                                                                            ((and lsp-enable-snippet (not (fboundp 'yas-minor-mode)))
                                                                             (lsp--warn (concat
                                                                                         "Yasnippet is not installed, but `lsp-enable-snippet' is set to `t'. "
                                                                                         "You must either install yasnippet, or disable snippet support."))
                                                                             :json-false)
                                                                            (lsp-enable-snippet t)
                                                                            (t :json-false)))
                                                        (documentationFormat . ["markdown" "plaintext"])
                                                        ;; Remove this after jdtls support resolveSupport
                                                        (resolveAdditionalTextEditsSupport . t)
                                                        (insertReplaceSupport . t)
                                                        (deprecatedSupport . t)
                                                        (resolveSupport
                                                         . ((properties . ["documentation"
                                                                           "detail"
                                                                           "additionalTextEdits"
                                                                           "command"])))
                                                        (insertTextModeSupport . ((valueSet . [1 2])))
                                                        (labelDetailsSupport . t)))
                                     (contextSupport . t)
                                     (dynamicRegistration . t)))
                      (signatureHelp . ((signatureInformation . ((parameterInformation . ((labelOffsetSupport . t)))
                                                                 (activeParameterSupport . t)))
                                        (dynamicRegistration . t)))
                      (documentLink . ((dynamicRegistration . t)
                                       (tooltipSupport . t)))
                      (hover . ((contentFormat . ["markdown" "plaintext"])
                                (dynamicRegistration . t)))
                      ,@(when lsp-enable-folding
                          `((foldingRange . ((dynamicRegistration . t)
                                             ,@(when lsp-folding-range-limit
                                                 `((rangeLimit . ,lsp-folding-range-limit)))
                                             ,@(when lsp-folding-line-folding-only
                                                 `((lineFoldingOnly . t)))))))
                      (selectionRange . ((dynamicRegistration . t)))
                      (callHierarchy . ((dynamicRegistration . :json-false)))
                      (typeHierarchy . ((dynamicRegistration . t)))
                      (publishDiagnostics . ((relatedInformation . t)
                                             (tagSupport . ((valueSet . [1 2])))
                                             (versionSupport . t)))
                      (diagnostic . ((dynamicRegistration . t)      ;; HERE
                                     (relatedDocumentSupport . t)))   ;; AND HERE
                      (linkedEditingRange . ((dynamicRegistration . t)))
                      (inlineCompletion . ())
                      ,@(when lsp-inlay-hint-enable '((inlayHint . ((dynamicRegistration . :json-false)))))))
     (window . ((workDoneProgress . t)
                (showDocument . ((support . t))))))
   custom-capabilities))

(advice-add 'lsp--client-capabilities :override #'custom-capabilities-override)

My first attempt was to create a custom client so that i could just pass in the custom-capabilities param. But you might notice that dynamic registration sits nested within textDocument so its impossible to reach through that param. I get document level diagnostics with just the above change.

Also had to set relatedDocumentSupport to true as well.

gamedolphin avatar Apr 23 '25 20:04 gamedolphin

LOL I undo all the changes. and it stops working again. Deep dive again, and I find that I had made another small change that actually enabled everything. :)

(defun custom-lsp-roslyn--on-initialized (workspace)
  (lsp-roslyn-open-solution-file)
  (with-lsp-workspace workspace
    (lsp--set-configuration
     #s(hash-table
         size 30
         test equal
         data (
               "csharp|background_analysis.dotnet_analyzer_diagnostics_scope" "fullSolution"
               "csharp|background_analysis.dotnet_compiler_diagnostics_scope" "fullSolution"
               )))))

(advice-add 'lsp-roslyn--on-initialized :override #'custom-lsp-roslyn--on-initialized)

So, you must update the diagnostic scope to "fullSolution" for roslyn to give you diagnostics for a file. My earlier, earlier comment then still holds. Unless you set the scope to "fullSolution", roslyn will not send diagnostics for a file unless you also specify the "identifier".

What a journey :P

PS - You might notice that I manually create the configuration object instead of using the recommended lsp-defcustom and lsp-configuration-section. This is because I couldn't get the csharp|<something> beyond the dot based configuration 🤦 advice-add was just easier at that point.

gamedolphin avatar Apr 23 '25 20:04 gamedolphin

Thanks for the deep dive! Will try to reproduce locally to see if it works.

sin-ack avatar Apr 23 '25 20:04 sin-ack

i've basically added this to my .dir-locals so that I can use roslyn. Its so much faster than omnisharp, its night and day. I will try to open a pr when i get some time next month.

gamedolphin avatar Apr 30 '25 20:04 gamedolphin