lsp-completion-at-point has incorrect try-completion and test-completion actions

[minad]

Thank you for the bug report

• [X] I am using the latest version of lsp-mode related packages.
• [X] I checked FAQ and Troubleshooting sections
• [X] You may also try reproduce the issue using clean environment using the following command: M-x lsp-start-plain

Bug description

According to the Emacs manual, section Programmed completion, completion tables should implement the try-completion and test-completion actions as follows:

• try-completion: The function should return nil if there are no matches; it should return t if the specified string is a unique and exact match; and it should return the longest common prefix substring of all matches otherwise.

• test-completion: The function should return t if the specified string is an exact match for some completion alternative; nil otherwise.

The current lsp-mode implementation looks like this:

https://github.com/emacs-lsp/lsp-mode/blob/3fa645c0397b8f438f2db2dd288b899ba330d410/lsp-completion.el#L523-L530

For comparison, the Eglot implementation looks like this:

https://github.com/joaotavora/eglot/blob/master/eglot.el#L2590-L2594

Unfortunately lsp-completion-at-point breaks packages which rely on completion tables following the specification precisely. See for example https://github.com/minad/corfu/issues/199 and https://github.com/minad/corfu/issues/202.

Steps to reproduce

See https://github.com/minad/corfu/issues/199 for a reproduction of an issue caused by the implementation.

Expected behavior

The completion table should be implemented according to the specification, such that the completion frontend can rely on the values returned by the completion table.

Which Language Server did you use?

lsp-java

OS

Linux

Error callstack

No response

Anything else?

No response

[minad]

For more background - the problem is that Corfu has some logic to check if the :exit-function should be called or if completion is supposed to continue (e.g., file path completion in Shell/Eshell which happens in multiple completion steps).

Since lsp-completion-at-point returns invalid values, the frontend cannot make the correct decision and may not call the :exit-function. For backends which provide a Lsp label, this can then lead to completion results like

someFuncti<C-M-i> -> someFunction(boolean argument, String otherArgument) : Boolean

where the signature is not stripped after completion.

Note that there has been some previous discussion a year ago, where I already pointed out that not following the specification may lead to problems: https://github.com/emacs-lsp/lsp-mode/issues/2970, https://github.com/emacs-lsp/lsp-mode/pull/2975#issuecomment-872003603. I know you worry about backward compatibility, but I suggest to test a fixed implementation for a while. If new bug reports come in due to the change one can still revisit again. The simplest possible implementation of the completion table is this:

(lambda (probe pred action)
  (if (eq action 'metadata)
      '(metadata (category . lsp-capf)
                 (display-sort-function . identity)
                 (cycle-sort-function . identity))
    (complete-with-action action (funcall candidates) probe pred)))

[minad]

if you are affected by this issue you can use the following patched lsp-completion-at-point function for now:

;;;###autoload
(defun lsp-completion-at-point ()
  "Get lsp completions."
  (when (or (--some (lsp--client-completion-in-comments? (lsp--workspace-client it))
                    (lsp-workspaces))
            (not (nth 4 (syntax-ppss))))
    (let* ((trigger-chars (->> (lsp--server-capabilities)
                               (lsp:server-capabilities-completion-provider?)
                               (lsp:completion-options-trigger-characters?)))
           (bounds-start (or (-some--> (cl-first (bounds-of-thing-at-point 'symbol))
                               (save-excursion
                                 (ignore-errors
                                   (goto-char (+ it 1))
                                   (while (lsp-completion--looking-back-trigger-characterp
                                           trigger-chars)
                                     (cl-incf it)
                                     (forward-char))
                                   it)))
                             (point)))
           result done?
           (candidates
            (lambda ()
              (lsp--catch 'input
                  (let ((lsp--throw-on-input lsp-completion-use-last-result)
                        (same-session? (and lsp-completion--cache
                                            ;; Special case for empty prefix and empty result
                                            (or (cl-second lsp-completion--cache)
                                                (not (string-empty-p
                                                      (plist-get (cddr lsp-completion--cache) :prefix))))
                                            (equal (cl-first lsp-completion--cache) bounds-start)
                                            (s-prefix?
                                             (plist-get (cddr lsp-completion--cache) :prefix)
                                             (buffer-substring-no-properties bounds-start (point))))))
                    (cond
                     ((or done? result) result)
                     ((and (not lsp-completion-no-cache)
                           same-session?
                           (listp (cl-second lsp-completion--cache)))
                      (setf result (apply #'lsp-completion--filter-candidates
                                          (cdr lsp-completion--cache))))
                     (t
                      (-let* ((resp (lsp-request-while-no-input
                                     "textDocument/completion"
                                     (plist-put (lsp--text-document-position-params)
                                                :context (lsp-completion--get-context trigger-chars))))
                              (completed (and resp
                                              (not (and (lsp-completion-list? resp)
                                                        (lsp:completion-list-is-incomplete resp)))))
                              (items (lsp--while-no-input
                                       (--> (cond
                                             ((lsp-completion-list? resp)
                                              (lsp:completion-list-items resp))
                                             (t resp))
                                         (if (or completed
                                                 (seq-some #'lsp:completion-item-sort-text? it))
                                             (lsp-completion--sort-completions it)
                                           it)
                                         (-map (lambda (item)
                                                 (lsp-put item
                                                          :_emacsStartPoint
                                                          (or (lsp-completion--guess-prefix item)
                                                              bounds-start)))
                                               it))))
                              (markers (list bounds-start (copy-marker (point) t)))
                              (prefix (buffer-substring-no-properties bounds-start (point)))
                              (lsp-completion--no-reordering (not lsp-completion-sort-initial-results)))
                        (lsp-completion--clear-cache same-session?)
                        (setf done? completed
                              lsp-completion--cache (list bounds-start
                                                          (cond
                                                           ((and done? (not (seq-empty-p items)))
                                                            (lsp-completion--to-internal items))
                                                           ((not done?) :incomplete))
                                                          :lsp-items nil
                                                          :markers markers
                                                          :prefix prefix)
                              result (lsp-completion--filter-candidates
                                      (cond (done?
                                             (cl-second lsp-completion--cache))
                                            (lsp-completion-filter-on-incomplete
                                             (lsp-completion--to-internal items)))
                                      :lsp-items items
                                      :markers markers
                                      :prefix prefix))))))
                (:interrupted lsp-completion--last-result)
                (`,res (setq lsp-completion--last-result res))))))
      (list
       bounds-start
       (point)
;; changed completion table
       (lambda (probe pred action)
         (if (eq action 'metadata)
             '(metadata (category . lsp-capf)
                        (display-sort-function . identity)
                        (cycle-sort-function . identity))
           (complete-with-action action (funcall candidates) probe pred)))
;; end of changed completion table
       :annotation-function #'lsp-completion--annotate
       :company-kind #'lsp-completion--candidate-kind
       :company-deprecated #'lsp-completion--candidate-deprecated
       :company-require-match 'never
       :company-prefix-length
       (save-excursion
         (goto-char bounds-start)
         (and (lsp-completion--looking-back-trigger-characterp trigger-chars) t))
       :company-match #'lsp-completion--company-match
       :company-doc-buffer (-compose #'lsp-doc-buffer
                                     #'lsp-completion--get-documentation)
       :exit-function
       (-rpartial #'lsp-completion--exit-fn candidates)))))

[anonimitoraf]

Thanks @minad for that patch! It seems to work for me for company, corfu and default completion (at least as far as my basic testing went)

[kiennq]

Can anyone has tested this extensively help to create a PR for this? @anonimitoraf

[anonimitoraf]

Sure, would be happy to! I don't know where to start though.

[minad]

Should be fixed now.