fix(lsp-mode): tolerate buffers whose major mode has no language spec (#2193)

* fix(lsp-mode): tolerate buffers whose major mode has no language spec

get-language-spec asserted the per-symbol property was a spec
instance, but its callers (buffer-language-spec, etc.) already use
when-let against the result -- they expect nil for modes that should
not activate LSP.  The assertion crashed the editor command loop
whenever a buffer's major mode lacked a registered spec.

This is reachable in normal use.  clojure-repl-mode (and similar
listener-style modes) inherits from clojure-mode, which registers a
spec.  The parent activation fires *clojure-mode-hook* ->
enable-lsp-mode, which starts an async workspace connect.  By the
time the workspace's :then callback fires text-document/did-open,
the buffer's major mode has switched to clojure-repl-mode (no spec)
and get-language-spec crashed.  Even if the open was skipped, the
buffer kept lsp-mode's before-change hook, so the next keystroke
hit text-document/did-change -> find-workspace nil -> error.

Push the guard down to where it belongs -- the LSP request
functions themselves -- so every callsite stays a single,
unconditional invocation:

  - get-language-spec returns nil for unregistered modes, matching
    the when-let contract its callers already assume.
  - find-workspace returns nil silently when language-id is nil,
    regardless of :errorp.  A nil language-id means the buffer is
    not an LSP buffer -- not a runtime failure.
  - text-document/did-open, did-change, did-save, and did-close all
    wrap their request body in (when-let (workspace
    (buffer-workspace buffer nil)) ...), so a buffer that has no
    workspace (no spec, or workspace not yet connected) becomes a
    silent no-op instead of crashing on workspace-client nil.

LSP remains fully functional for any buffer whose major mode is
registered; non-LSP buffers (REPL transcripts, etc.) become no-ops
on the relevant hooks.

* test(lsp-mode): cover spec/workspace nil-tolerant lookups

Two deftests guard the regressions the previous commit fixed:

  - get-language-spec returns nil for an unregistered mode (the path
    that previously asserted and crashed the editor command loop the
    first time a REPL-style buffer interacted with lsp-mode).

  - find-workspace returns nil for a nil language-id even with
    :errorp t (the default path that previously signaled
    "The NIL workspace is not found." on the next keystroke).

Verified both fail when the source fix is reverted.

Author: theangelperalta

extensions/lsp-mode/lsp-mode.lisp

@@ -231,10 +231,15 @@ Use this when lsp-mode has side effects that you want to avoid."
     (setf (workspace-list-current-workspace workspace-list) workspace)))
 
 (defun find-workspace (language-id &key (errorp t))
-  (if-let (workspace-list (gethash language-id *workspace-list-per-language-id*))
-    (workspace-list-current-workspace workspace-list)
-    (when errorp
-      (error "The ~A workspace is not found." language-id))))
+  ;; A nil LANGUAGE-ID means the buffer's current major mode has no
+  ;; registered language spec (e.g. a REPL mode whose parent had one).
+  ;; That is not an error condition -- the buffer simply does not
+  ;; participate in LSP.  Return nil silently regardless of ERRORP.
+  (when language-id
+    (if-let (workspace-list (gethash language-id *workspace-list-per-language-id*))
+      (workspace-list-current-workspace workspace-list)
+      (when errorp
+        (error "The ~A workspace is not found." language-id)))))
 
 (defun buffer-workspace (buffer &optional (errorp t))
   (find-workspace (buffer-language-id buffer) :errorp errorp))
@@ -613,22 +618,24 @@ Use this when lsp-mode has side effects that you want to avoid."
 ;;; Text Synchronization
 
 (defun text-document/did-open (buffer)
-  (request:request
-   (workspace-client (buffer-workspace buffer))
-   (make-instance 'lsp:text-document/did-open)
-   (make-instance 'lsp:did-open-text-document-params
-                  :text-document (buffer-to-text-document-item buffer))))
+  (when-let (workspace (buffer-workspace buffer nil))
+    (request:request
+     (workspace-client workspace)
+     (make-instance 'lsp:text-document/did-open)
+     (make-instance 'lsp:did-open-text-document-params
+                    :text-document (buffer-to-text-document-item buffer)))))
 
 (defun text-document/did-change (buffer content-changes)
-  (request:request
-   (workspace-client (buffer-workspace buffer))
-   (make-instance
-    'lsp:text-document/did-change)
-   (make-instance 'lsp:did-change-text-document-params
-                  :text-document (make-instance 'lsp:versioned-text-document-identifier
-                                                :version (buffer-version buffer)
-                                                :uri (buffer-uri buffer))
-                  :content-changes content-changes)))
+  (when-let (workspace (buffer-workspace buffer nil))
+    (request:request
+     (workspace-client workspace)
+     (make-instance
+      'lsp:text-document/did-change)
+     (make-instance 'lsp:did-change-text-document-params
+                    :text-document (make-instance 'lsp:versioned-text-document-identifier
+                                                  :version (buffer-version buffer)
+                                                  :uri (buffer-uri buffer))
+                    :content-changes content-changes))))
 
 (defun provide-did-save-text-document-p (workspace)
   (let ((sync (lsp:server-capabilities-text-document-sync
@@ -644,20 +651,22 @@ Use this when lsp-mode has side effects that you want to avoid."
            nil))))))
 
 (defun text-document/did-save (buffer)
-  (when (provide-did-save-text-document-p (buffer-workspace buffer))
-    (request:request
-     (workspace-client (buffer-workspace buffer))
-     (make-instance 'lsp:text-document/did-save)
-     (make-instance 'lsp:did-save-text-document-params
-                    :text-document (make-text-document-identifier buffer)
-                    :text (buffer-text buffer)))))
+  (when-let (workspace (buffer-workspace buffer nil))
+    (when (provide-did-save-text-document-p workspace)
+      (request:request
+       (workspace-client workspace)
+       (make-instance 'lsp:text-document/did-save)
+       (make-instance 'lsp:did-save-text-document-params
+                      :text-document (make-text-document-identifier buffer)
+                      :text (buffer-text buffer))))))
 
 (defun text-document/did-close (buffer)
-  (request:request
-   (workspace-client (buffer-workspace buffer))
-   (make-instance 'lsp:text-document/did-close)
-   (make-instance 'lsp:did-close-text-document-params
-                  :text-document (make-text-document-identifier buffer))))
+  (when-let (workspace (buffer-workspace buffer nil))
+    (request:request
+     (workspace-client workspace)
+     (make-instance 'lsp:text-document/did-close)
+     (make-instance 'lsp:did-close-text-document-params
+                    :text-document (make-text-document-identifier buffer)))))
 
 ;;; publishDiagnostics
 

extensions/lsp-mode/spec.lisp

@@ -47,9 +47,14 @@
     :reader spec-mode)))
 
 (defun get-language-spec (major-mode)
+  "Return the registered language `spec' for MAJOR-MODE, or NIL when no
+spec has been registered.  Callers like `buffer-language-spec' already
+use `when-let' against this result and rely on a NIL return for modes
+that should not activate LSP -- for example REPL buffers whose major
+mode inherits from a parent mode that did register a spec."
   (let ((spec (get major-mode 'spec)))
-    (assert (typep spec 'spec))
-    spec))
+    (when (typep spec 'spec)
+      spec)))
 
 (defun register-language-spec (major-mode spec)
   (check-type spec spec)

lem-tests.asd

@@ -38,7 +38,8 @@
                 :components ((:file "mock-client")
                              (:file "test-utils")
                              (:file "tests")
-                             (:file "integration-tests")))
+                             (:file "integration-tests")
+                             (:file "spec-test")))
                #+sbcl
                (:module "mcp-server"
                 :components ((:file "utils")

tests/lsp-mode/spec-test.lisp

@@ -0,0 +1,41 @@
+(defpackage :lem-tests/lsp-mode/spec-test
+  (:use :cl :rove)
+  (:import-from :lem-lsp-mode/spec
+                :get-language-spec
+                :register-language-spec
+                :spec)
+  (:import-from :lem-lsp-mode
+                :find-workspace))
+(in-package :lem-tests/lsp-mode/spec-test)
+
+;;;; Regression tests for the no-spec / nil-language-id path that the
+;;;; lsp-mode hardening change made tolerant.  Both functions are called
+;;;; transitively by lsp-mode hooks attached to every buffer, so any
+;;;; regression here crashes the editor command loop on the next keystroke
+;;;; in a buffer whose major mode has no language spec (e.g. REPL
+;;;; transcripts).
+
+(deftest get-language-spec-returns-nil-for-unregistered-mode
+  (testing "unknown mode -> nil (callers use when-let against this)"
+    (let ((mode (gensym "UNREGISTERED-MODE-")))
+      (ok (null (get-language-spec mode))
+          "Unregistered modes must return nil, not signal an assertion")))
+
+  (testing "registered mode -> the spec instance"
+    (let ((mode (gensym "REGISTERED-MODE-"))
+          (spec (make-instance 'spec
+                               :mode 'dummy-mode
+                               :language-id "dummy"
+                               :connection-mode :stdio)))
+      (register-language-spec mode spec)
+      (ok (eq spec (get-language-spec mode))
+          "A registered spec must round-trip through get-language-spec"))))
+
+(deftest find-workspace-tolerates-nil-language-id
+  ;; Before the fix, find-workspace defaulted to :errorp t and signaled
+  ;; "The NIL workspace is not found." on the first keystroke in a
+  ;; buffer whose major mode had no spec.  The :errorp nil path was
+  ;; already nil-safe; only the default-errorp case regressed.
+  (testing "nil language-id with :errorp t -> nil (was an error)"
+    (ok (null (find-workspace nil :errorp t))
+        "A nil language-id means the buffer is not an LSP buffer, not a failure")))