Support for related locations in diagnostics?

[cpitclaudel]

Hi there,

LSPdiagnostics have a relatedInformation field that LSP-mode does not seem to be using yet. How hard would it be to display these related locations?

This was briefly discussed in https://github.com/emacs-lsp/lsp-ui/issues/165, but since LSP discards the information it's not something that LSP-ui can do on it's own, I think.

I'm guessing there are two ways to go about this:

1. Use flycheck's group feature (this is how it was intended to be used, but LSP-mode uses it to store source information). The code looks something like this:

(defun lsp-diagnostics--flycheck-start (checker callback)
  "Start an LSP syntax check with CHECKER.

CALLBACK is the status callback passed by Flycheck."

  (remove-hook 'lsp-on-idle-hook #'lsp-diagnostics--flycheck-buffer t)

  (->> (lsp--get-buffer-diagnostics)
       (-mapcat
        (-lambda ((&Diagnostic :message :severity? :tags? :code? :source? :related-information?
                               :range (&Range :start (&Position :line      start-line
                                                                :character start-character)
                                              :end   (&Position :line      end-line
                                                                :character end-character))))
          (let ((group (gensym)))
            (cons (flycheck-error-new
                   :buffer (current-buffer)
                   :checker checker
                   :filename buffer-file-name
                   :message message
                   :level (lsp-diagnostics--flycheck-calculate-level severity? tags?)
                   :id code?
                   :group group
                   :line (lsp-translate-line (1+ start-line))
                   :column (1+ (lsp-translate-column start-character))
                   :end-line (lsp-translate-line (1+ end-line))
                   :end-column (1+ (lsp-translate-column end-character)))
                  (-mapcat
                   (-lambda ((&DiagnosticRelatedInformation
                              :message
                              :location
                              (&Location :range (&Range :start (&Position :line      start-line
                                                                          :character start-character)
                                                        :end   (&Position :line      end-line
                                                                          :character end-character))
                                         :uri)))
                     (and (equal (-> uri lsp--uri-to-path lsp--fix-path-casing)
                                 (-> buffer-file-name lsp--fix-path-casing))
                          `(,(flycheck-error-new
                              :buffer (current-buffer)
                              :checker checker
                              :filename buffer-file-name
                              :message message
                              :level (lsp-diagnostics--flycheck-calculate-level (1+ severity?) tags?)
                              :id code?
                              :group group
                              :line (lsp-translate-line (1+ start-line))
                              :column (1+ (lsp-translate-column start-character))
                              :end-line (lsp-translate-line (1+ end-line))
                              :end-column (1+ (lsp-translate-column end-character))))))
                   related-information?)))))
       (funcall callback 'finished)))

2. Keep track of related locations in a separate table and query it in the error display function. Hashtables can be keyed by equal objects, so it actually works to use flycheck errors as keys; each error can be made to point to a list of other flycheck errors, and they can be displayed selectively when the cursor is on an error.

Here's a demo of the first option with flycheck-inline:

[yyoncho]

To me, 1. looks good enough. Is there anything against this approach?

[cpitclaudel]

It's pretty good but it pollutes the buffer a bit (related locations are shown at all times).

We could refine it by defining a new error level for "related locations" (since LSP related locations don't have a severity) and giving it an empty face, so that it doesn't appear in the buffer. Then LSP-ui could be made to display related locations when the point is on the main error.

[yyoncho]

It's pretty good but it pollutes the buffer a bit (related locations are shown at all times).

Isn't that what we want? Or you mean that it is visible all the time, not only when you hover the "master" error? Don't we want that feature to be part of flycheck itself?

[cpitclaudel]

Or you mean that it is visible all the time, not only when you hover the "master" error?

That's right.

Don't we want that feature to be part of flycheck itself?

We do; I think flycheck-inline does something about that already.