bug#67463: 30.0.50; Eglot may manage js-json-mode buffers with wrong server

bug#67463: 30.0.50; Eglot may manage js-json-mode buffers with wrong server

[Pengji Zhang]

It seems that buffers in `js-json-mode', the default mode for JSON
files, could be wrongly managed by JavaScript language servers of Eglot,
perhaps because:

  - `js-json-mode' is derived from `js-mode';
  - Eglot uses `provided-mode-derived-p' to determine whether a buffer
    should be managed by a server.

Here is a recipe to reproduce the problem I encountered:

  1. Install Node.js[0] and NPM[1]. (Sorry, but I could not find an
     easier way to set up the environment. They may be installable with
     your system's package manager.)
  2. mkdir /tmp/repro && cd /tmp/repro
  3. npm install typescript typescript-language-server (This will install
     the language server locally in the directory.)
  4. emacs -Q
  5. C-x C-f main.js RET
  6. C-u M-x eglot; enter "npx typescript-language-server --stdio"
  7. C-x C-f package.json RET (This file is automatically created by
     `npm install'.)

I expect that the 'package.json' buffer is not managed by Eglot, but it
is actually managed by typescript-language-server, which does not really
understand JSON and is reporting some invalid errors.

Thanks!

[0] https://nodejs.org/en
[1] https://docs.npmjs.com/downloading-and-installing-node-js-and-npm

-----
Auxiliary information:

- Emacs version: 30.0.50 (commit ea4c5fcd77257d7a7c050714b809b1507d93a6ef)
- Eglot version: 1.15 (comes with the Emacs repository of the commit above)
- Contents of `eglot-events-buffer':

[internal] Sun Nov 26 15:24:00 2023:
(:message
 "Running language server: npx typescript-language-server --stdio")
[client-request] (id:1) Sun Nov 26 15:24:00 2023:
(:jsonrpc "2.0" :id 1 :method "initialize" :params
 (:processId 26705 :clientInfo
     (:name "Eglot" :version "1.15") :rootPath
     "/tmp/eglot-bug/" :rootUri
     "file:///tmp/eglot-bug" :initializationOptions
     #s(hash-table size 1 test eql rehash-size 1.5
   rehash-threshold 0.8125 data ())
     :capabilities
     (:workspace
      (:applyEdit t :executeCommand
  (:dynamicRegistration :json-false)
  :workspaceEdit (:documentChanges t)
  :didChangeWatchedFiles
  (:dynamicRegistration t) :symbol
  (:dynamicRegistration :json-false)
  :configuration t :workspaceFolders
  t)
      :textDocument
      (:synchronization
(:dynamicRegistration :json-false :willSave t
     :willSaveWaitUntil t
     :didSave t)
:completion
(:dynamicRegistration :json-false
     :completionItem
     (:snippetSupport
      :json-false
      :deprecatedSupport t
      :resolveSupport
      (:properties
["documentation"
"details"
"additionalTextEdits"])
      :tagSupport
      (:valueSet [1]))
     :contextSupport t)
:hover
(:dynamicRegistration :json-false
     :contentFormat
     ["plaintext"])
:signatureHelp
(:dynamicRegistration :json-false
     :signatureInformation
     (:parameterInformation
      (:labelOffsetSupport t)
      :documentationFormat
      ["plaintext"]
      :activeParameterSupport
      t))
:references (:dynamicRegistration :json-false)
:definition
(:dynamicRegistration :json-false :linkSupport
     t)
:declaration
(:dynamicRegistration :json-false :linkSupport
     t)
:implementation
(:dynamicRegistration :json-false :linkSupport
     t)
:typeDefinition
(:dynamicRegistration :json-false :linkSupport
     t)
:documentSymbol
(:dynamicRegistration :json-false
     :hierarchicalDocumentSymbolSupport
     t :symbolKind
     (:valueSet
      [1 2 3 4 5 6 7 8 9 10
 11 12 13 14 15 16 17
 18 19 20 21 22 23 24
 25 26]))
:documentHighlight
(:dynamicRegistration :json-false) :codeAction
(:dynamicRegistration :json-false
     :resolveSupport
     (:properties
      ["edit" "command"])
     :dataSupport t
     :codeActionLiteralSupport
     (:codeActionKind
      (:valueSet
["quickfix" "refactor"
"refactor.extract"
"refactor.inline"
"refactor.rewrite"
"source"
"source.organizeImports"]))
     :isPreferredSupport t)
:formatting (:dynamicRegistration :json-false)
:rangeFormatting
(:dynamicRegistration :json-false) :rename
(:dynamicRegistration :json-false) :inlayHint
(:dynamicRegistration :json-false)
:publishDiagnostics
(:relatedInformation :json-false
    :codeDescriptionSupport
    :json-false :tagSupport
    (:valueSet [1 2])))
      :window
      (:showDocument (:support t) :workDoneProgress t)
      :general
      (:positionEncodings ["utf-32" "utf-8" "utf-16"])
      :experimental
      #s(hash-table size 1 test eql rehash-size 1.5
    rehash-threshold 0.8125 data ()))
     :workspaceFolders
     [(:uri "file:///tmp/eglot-bug" :name
    "/tmp/eglot-bug/")]))
[server-notification] Sun Nov 26 15:24:01 2023:
(:jsonrpc "2.0" :method "window/logMessage" :params
 (:type 3 :message
"Using Typescript version (workspace) 5.3.2 from path
\"/tmp/eglot-bug/node_modules/typescript/lib/tsserver.js\""))
[server-request] (id:0) Sun Nov 26 15:24:01 2023:
(:jsonrpc "2.0" :id 0 :method "window/workDoneProgress/create" :params
 (:token "3a0c42cc-0696-45e3-b763-564b150099bb"))
[client-reply] (id:0) Sun Nov 26 15:24:01 2023:
(:jsonrpc "2.0" :id 0 :result nil)
[server-reply] (id:1) Sun Nov 26 15:24:01 2023:
(:jsonrpc "2.0" :id 1 :result
 (:capabilities
  (:textDocumentSync 2 :completionProvider
     (:triggerCharacters
      ["." "\"" "'" "/" "@" "<"]
      :resolveProvider t)
     :codeActionProvider
     (:codeActionKinds
      ["source.fixAll.ts"
"source.removeUnused.ts"
"source.addMissingImports.ts"
"source.organizeImports.ts"
"source.removeUnusedImports.ts"
"source.sortImports.ts" "quickfix"
"refactor"])
     :codeLensProvider (:resolveProvider t)
     :definitionProvider t
     :documentFormattingProvider t
     :documentRangeFormattingProvider t
     :documentHighlightProvider t
     :documentSymbolProvider t
     :executeCommandProvider
     (:commands
      ["_typescript.applyWorkspaceEdit"
"_typescript.applyCodeAction"
"_typescript.applyRefactoring"
"_typescript.configurePlugin"
"_typescript.organizeImports"
"_typescript.applyRenameFile"
"_typescript.goToSourceDefinition"])
     :hoverProvider t :inlayHintProvider t
     :linkedEditingRangeProvider :json-false
     :renameProvider t :referencesProvider t
     :selectionRangeProvider t
     :signatureHelpProvider
     (:triggerCharacters ["(" "," "<"]
 :retriggerCharacters
 [")"])
     :workspaceSymbolProvider t
     :implementationProvider t
     :typeDefinitionProvider t
     :foldingRangeProvider t
     :semanticTokensProvider
     (:documentSelector nil :legend
(:tokenTypes
 ["class" "enum"
  "interface"
  "namespace"
  "typeParameter"
  "type" "parameter"
  "variable"
  "enumMember"
  "property"
  "function" "member"]
 :tokenModifiers
 ["declaration"
  "static" "async"
  "readonly"
  "defaultLibrary"
  "local"])
:full t :range t)
     :workspace
     (:fileOperations
      (:willRename
(:filters
[(:scheme "file" :pattern
  (:glob
   "**/*.{ts,js,jsx,tsx,mjs,mts,cjs,cts}"
   :matches "file"))]))))))
[client-notification] Sun Nov 26 15:24:01 2023:
(:jsonrpc "2.0" :method "initialized" :params
 #s(hash-table size 1 test eql rehash-size 1.5
rehash-threshold 0.8125 data ()))
[client-notification] Sun Nov 26 15:24:01 2023:
(:jsonrpc "2.0" :method "textDocument/didOpen" :params
 (:textDocument
  (:uri "file:///tmp/eglot-bug/main.js" :version 0
:languageId "javascript" :text "")))
[client-notification] Sun Nov 26 15:24:01 2023:
(:jsonrpc "2.0" :method "workspace/didChangeConfiguration" :params
 (:settings
  #s(hash-table size 1 test eql rehash-size 1.5
rehash-threshold 0.8125 data ())))
[server-notification] Sun Nov 26 15:24:01 2023:
(:jsonrpc "2.0" :method "$/progress" :params
 (:token "3a0c42cc-0696-45e3-b763-564b150099bb" :value
 (:kind "begin" :title
"Initializing JS/TS language features…")))
[server-notification] Sun Nov 26 15:24:01 2023:
(:jsonrpc "2.0" :method "$/typescriptVersion" :params
 (:version "5.3.2" :source "workspace"))
[client-request] (id:2) Sun Nov 26 15:24:01 2023:
(:jsonrpc "2.0" :id 2 :method "textDocument/hover" :params
 (:textDocument (:uri "file:///tmp/eglot-bug/main.js")
:position (:line 0 :character 0)))
[client-request] (id:3) Sun Nov 26 15:24:01 2023:
(:jsonrpc "2.0" :id 3 :method "textDocument/documentHighlight" :params
 (:textDocument (:uri "file:///tmp/eglot-bug/main.js")
:position (:line 0 :character 0)))
[client-request] (id:4) Sun Nov 26 15:24:01 2023:
(:jsonrpc "2.0" :id 4 :method "textDocument/signatureHelp" :params
 (:textDocument (:uri "file:///tmp/eglot-bug/main.js")
:position (:line 0 :character 0)))
[server-reply] (id:3) Sun Nov 26 15:24:02 2023:
(:jsonrpc "2.0" :id 3 :result [])
[server-reply] (id:4) Sun Nov 26 15:24:02 2023:
(:jsonrpc "2.0" :id 4 :result nil)
[server-reply] (id:2) Sun Nov 26 15:24:02 2023:
(:jsonrpc "2.0" :id 2 :result nil)
[server-notification] Sun Nov 26 15:24:02 2023:
(:jsonrpc "2.0" :method "$/progress" :params
 (:token "3a0c42cc-0696-45e3-b763-564b150099bb" :value
 (:kind "end")))
[server-notification] Sun Nov 26 15:24:02 2023:
(:jsonrpc "2.0" :method "textDocument/publishDiagnostics" :params
 (:uri "file:///tmp/eglot-bug/main.js" :diagnostics []))
[server-notification] Sun Nov 26 15:24:05 2023:
(:jsonrpc "2.0" :method "textDocument/publishDiagnostics" :params
 (:uri "file:///tmp/eglot-bug/main.js" :diagnostics []))
[server-notification] Sun Nov 26 15:24:05 2023:
(:jsonrpc "2.0" :method "textDocument/publishDiagnostics" :params
 (:uri "file:///tmp/eglot-bug/main.js" :diagnostics []))
[client-notification] Sun Nov 26 15:24:19 2023:
(:jsonrpc "2.0" :method "textDocument/didOpen" :params
 (:textDocument
  (:uri "file:///tmp/eglot-bug/package.json" :version 0
:languageId "javascript" :text
"{\n  \"dependencies\": {\n    \"typescript\": \"^5.3.2\",\n
\"typescript-language-server\": \"^4.1.2\"\n  }\n}\n")))
[client-request] (id:5) Sun Nov 26 15:24:19 2023:
(:jsonrpc "2.0" :id 5 :method "textDocument/inlayHint" :params
 (:textDocument (:uri "file:///tmp/eglot-bug/package.json")
:range
(:start (:line 0 :character 0) :end
(:line 6 :character 0))))
[server-reply] (id:5) Sun Nov 26 15:24:19 2023:
(:jsonrpc "2.0" :id 5 :result [])
[server-notification] Sun Nov 26 15:24:19 2023:
(:jsonrpc "2.0" :method "textDocument/publishDiagnostics" :params
 (:uri "file:///tmp/eglot-bug/package.json" :diagnostics
[(:range
 (:start (:line 1 :character 16) :end
 (:line 1 :character 17))
 :message "';' expected." :severity 1 :code 1005
 :source "typescript" :tags [])
(:range
 (:start (:line 2 :character 16) :end
 (:line 2 :character 17))
 :message "';' expected." :severity 1 :code 1005
 :source "typescript" :tags [])
(:range
 (:start (:line 3 :character 32) :end
 (:line 3 :character 33))
 :message "';' expected." :severity 1 :code 1005
 :source "typescript" :tags [])]))


In GNU Emacs 30.0.50 (build 11, x86_64-pc-linux-gnu)
Repository revision: ea4c5fcd77257d7a7c050714b809b1507d93a6ef
Repository branch: master

Configured using:
 'configure --prefix=/home/pengji/.local --without-x
 --program-transform-name=s/^ctags/ctags.emacs/ --disable-build-details
 PKG_CONFIG_PATH=/home/pengji/.local/share/pkgconfig:/home/pengji/.local/lib/pkgconfig:/home/pengji/.local/share/pkgconfig:/home/pengji/.local/lib/pkgconfig:'

Configured features:
GNUTLS LIBXML2 MODULES NOTIFY INOTIFY PDUMPER SOUND SQLITE3 THREADS
TREE_SITTER ZLIB

Important settings:
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Info

Minor modes in effect:
  mouse-wheel-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  show-paren-mode: t
  electric-indent-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  isearch-fold-quotes-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  minibuffer-regexp-mode: t
  buffer-read-only: t
  line-number-mode: t
  indent-tabs-mode: t
  transient-mark-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message mailcap yank-media puny dired
dnd dired-loaddefs rfc822 mml mml-sec epa derived epg rfc6068 epg-config
gnus-util mm-decode mm-bodies mm-encode mail-parse rfc2231 mailabbrev
gmm-utils mailheader sendmail rfc2047 rfc2045 ietf-drums mm-util
mail-prsvr mail-utils mule-util info time-date cl-extra eglot
external-completion jsonrpc xref flymake mwheel thingatpt project diff
diff-mode easy-mmode ert pp ewoc debug backtrace help-mode find-func
filenotify warnings icons compile text-property-search tool-bar pcase
url-util url-parse auth-source cl-seq eieio eieio-core cl-macs
password-cache url-vars sh-script rx smie executable files-x shell
pcomplete comint ansi-osc ansi-color ring js c-ts-common treesit json
subr-x map imenu cc-mode cc-fonts cc-guess cc-menus cc-cmds cc-styles
cc-align cc-engine cc-vars cc-defs regexp-opt cl-loaddefs cl-lib
term/tmux term/xterm xterm byte-opt gv bytecomp byte-compile rmc
iso-transl tooltip cconv eldoc paren electric uniquify ediff-hook
vc-hooks lisp-float-type elisp-mode touch-screen tabulated-list replace
newcomment text-mode lisp-mode prog-mode register page tab-bar menu-bar
rfn-eshadow isearch easymenu timer select mouse jit-lock font-lock
syntax font-core term/tty-colors frame minibuffer nadvice seq simple
cl-generic indonesian philippine cham georgian utf-8-lang misc-lang
vietnamese tibetan thai tai-viet lao korean japanese eucjp-ms cp51932
hebrew greek romanian slovak czech european ethiopic indian cyrillic
chinese composite emoji-zwj charscript charprop case-table epa-hook
jka-cmpr-hook help abbrev obarray oclosure cl-preloaded button loaddefs
theme-loaddefs faces cus-face macroexp files window text-properties
overlay sha1 md5 base64 format env code-pages mule custom widget keymap
hashtable-print-readable backquote threads inotify multi-tty
make-network-process emacs)

Memory information:
((conses 16 134366 14563) (symbols 48 13684 0) (strings 32 42470 2356)
 (string-bytes 1 1456015) (vectors 16 24965)
 (vector-slots 8 306626 12688) (floats 8 58 9041)
 (intervals 56 548 1368) (buffers 992 20) (heap 1024 7552 769))

bug#67463: 30.0.50; Eglot may manage js-json-mode buffers with wrong server

[Eli Zaretskii]

> From: Pengji Zhang <kunhtkun@gmail.com>
> Date: Sun, 26 Nov 2023 16:15:54 -0500
> 
> It seems that buffers in `js-json-mode', the default mode for JSON
> files, could be wrongly managed by JavaScript language servers of Eglot,
> perhaps because:
> 
>   - `js-json-mode' is derived from `js-mode';
>   - Eglot uses `provided-mode-derived-p' to determine whether a buffer
>     should be managed by a server.
> 
> Here is a recipe to reproduce the problem I encountered:
> 
>   1. Install Node.js[0] and NPM[1]. (Sorry, but I could not find an
>      easier way to set up the environment. They may be installable with
>      your system's package manager.)
>   2. mkdir /tmp/repro && cd /tmp/repro
>   3. npm install typescript typescript-language-server (This will install
>      the language server locally in the directory.)
>   4. emacs -Q
>   5. C-x C-f main.js RET
>   6. C-u M-x eglot; enter "npx typescript-language-server --stdio"
>   7. C-x C-f package.json RET (This file is automatically created by
>      `npm install'.)
> 
> I expect that the 'package.json' buffer is not managed by Eglot, but it
> is actually managed by typescript-language-server, which does not really
> understand JSON and is reporting some invalid errors.
> 
> Thanks!
> 
> [0] https://nodejs.org/en
> [1] https://docs.npmjs.com/downloading-and-installing-node-js-and-npm

João, any suggestions?

bug#67463: 30.0.50; Eglot may manage js-json-mode buffers with wrong server

[João Távora]

On Sat, Dec 2, 2023 at 12:51 PM Eli Zaretskii <eliz@gnu.org> wrote:
>
> > From: Pengji Zhang <kunhtkun@gmail.com>
> > Date: Sun, 26 Nov 2023 16:15:54 -0500
> >
> > It seems that buffers in `js-json-mode', the default mode for JSON
> > files, could be wrongly managed by JavaScript language servers of Eglot,
> > perhaps because:
> >
> >   - `js-json-mode' is derived from `js-mode';
> >   - Eglot uses `provided-mode-derived-p' to determine whether a buffer
> >     should be managed by a server.
> >
> > Here is a recipe to reproduce the problem I encountered:
> >
> >   1. Install Node.js[0] and NPM[1]. (Sorry, but I could not find an
> >      easier way to set up the environment. They may be installable with
> >      your system's package manager.)
> >   2. mkdir /tmp/repro && cd /tmp/repro
> >   3. npm install typescript typescript-language-server (This will install
> >      the language server locally in the directory.)
> >   4. emacs -Q
> >   5. C-x C-f main.js RET
> >   6. C-u M-x eglot; enter "npx typescript-language-server --stdio"
> >   7. C-x C-f package.json RET (This file is automatically created by
> >      `npm install'.)
> >
> > I expect that the 'package.json' buffer is not managed by Eglot, but it
> > is actually managed by typescript-language-server, which does not really
> > understand JSON and is reporting some invalid errors.
> >
> > Thanks!
> >
> > [0] https://nodejs.org/en
> > [1] https://docs.npmjs.com/downloading-and-installing-node-js-and-npm
>
> João, any suggestions?


Thanks Eli.  Yes, some notes:

1. JSON is technically Javascript (but only really technically, in actual
   programming practice  it's not much use editing JSON as Javascript)

2. For the above reason the derivation of `json-json-mode` from `js-mode`
   is technically correct.  When looking at a JSON file you are technically
   looking at some minimalistic Javascript code.

3. It wouldn't be a bad idea for Emacs to do whatever behaviour-reuse it
   is doing for 'js-json-mode' with inheritance from 'js-mode' with some
   other mechanism like composition.  This would immediately solve Pengji's
   problem.  But it is not trivial to do so.

4. It follows from 1 and 2 that this problem can be viewed as a bug in
   typescript-language-server.  It's complaining we're not giving it
   a Javascript file, but it has no reason to complain: we are!  Not sure
   if it's worth arguing with the server devs like so.  Maybe.

5. A more promising change that can be tried today is to make use
   of Eglot's :language-id feature, which is already in place for this
   family of modes.  The current one looks like this:

  (((js-mode :language-id "javascript")
    (js-ts-mode :language-id "javascript")
    (tsx-ts-mode :language-id "typescriptreact")
    (typescript-ts-mode :language-id "typescript")
    (typescript-mode :language-id "typescript"))
  . ("typescript-language-server" "--stdio"))

Which means that any Javascript file is reported as javascript.
Maybe just adding

     (js-json-mode :language-id "json")

there would work.  In other words, this patch:

--- ./lisp/progmodes/eglot.el
+++ #<buffer eglot.el>
@@ -222,6 +222,7 @@

("vscode-json-languageserver" "--stdio")

("json-languageserver" "--stdio"))))
                                 (((js-mode :language-id "javascript")
+                                  (js-json-mode :language-id "json")
                                   (js-ts-mode :language-id "javascript")
                                   (tsx-ts-mode :language-id "typescriptreact")
                                   (typescript-ts-mode :language-id
"typescript")

bug#67463: 30.0.50; Eglot may manage js-json-mode buffers with wrong server

[João Távora]

On Sun, Dec 3, 2023 at 6:14 PM João Távora <joaotavora@gmail.com> wrote:
>
> On Sat, Dec 2, 2023 at 12:51 PM Eli Zaretskii <eliz@gnu.org> wrote:
> >
> > > From: Pengji Zhang <kunhtkun@gmail.com>
> > > Date: Sun, 26 Nov 2023 16:15:54 -0500
> > >
> > > It seems that buffers in `js-json-mode', the default mode for JSON
> > > files, could be wrongly managed by JavaScript language servers of Eglot,
> > > perhaps because:
> > >
> > >   - `js-json-mode' is derived from `js-mode';
> > >   - Eglot uses `provided-mode-derived-p' to determine whether a buffer
> > >     should be managed by a server.
> > >
> > > Here is a recipe to reproduce the problem I encountered:
> > >
> > >   1. Install Node.js[0] and NPM[1]. (Sorry, but I could not find an
> > >      easier way to set up the environment. They may be installable with
> > >      your system's package manager.)
> > >   2. mkdir /tmp/repro && cd /tmp/repro
> > >   3. npm install typescript typescript-language-server (This will install
> > >      the language server locally in the directory.)
> > >   4. emacs -Q
> > >   5. C-x C-f main.js RET
> > >   6. C-u M-x eglot; enter "npx typescript-language-server --stdio"
> > >   7. C-x C-f package.json RET (This file is automatically created by
> > >      `npm install'.)
> > >
> > > I expect that the 'package.json' buffer is not managed by Eglot, but it
> > > is actually managed by typescript-language-server, which does not really
> > > understand JSON and is reporting some invalid errors.
> > >
> > > Thanks!
> > >
> > > [0] https://nodejs.org/en
> > > [1] https://docs.npmjs.com/downloading-and-installing-node-js-and-npm
> >
> > João, any suggestions?
>
>
> Thanks Eli.  Yes, some notes:
>
> 1. JSON is technically Javascript (but only really technically, in actual
>    programming practice  it's not much use editing JSON as Javascript)
>
> 2. For the above reason the derivation of `json-json-mode` from `js-mode`
>    is technically correct.  When looking at a JSON file you are technically
>    looking at some minimalistic Javascript code.
>
> 3. It wouldn't be a bad idea for Emacs to do whatever behaviour-reuse it
>    is doing for 'js-json-mode' with inheritance from 'js-mode' with some
>    other mechanism like composition.  This would immediately solve Pengji's
>    problem.  But it is not trivial to do so.
>
> 4. It follows from 1 and 2 that this problem can be viewed as a bug in
>    typescript-language-server.  It's complaining we're not giving it
>    a Javascript file, but it has no reason to complain: we are!  Not sure
>    if it's worth arguing with the server devs like so.  Maybe.
>
> 5. A more promising change that can be tried today is to make use
>    of Eglot's :language-id feature, which is already in place for this
>    family of modes.  The current one looks like this:
>
>   (((js-mode :language-id "javascript")
>     (js-ts-mode :language-id "javascript")
>     (tsx-ts-mode :language-id "typescriptreact")
>     (typescript-ts-mode :language-id "typescript")
>     (typescript-mode :language-id "typescript"))
>   . ("typescript-language-server" "--stdio"))
>
> Which means that any Javascript file is reported as javascript.
> Maybe just adding
>
>      (js-json-mode :language-id "json")
>
> there would work.

Nope this doesn't work.  Even the slightly more involved patch after
my sig  doesn't work, and it correctly reports the :languageId as "json"

So I think the best courses of action are 3 and 4, in that order.

Another workaround is to first start M-x eglot in some json file
in your project.  If you have one of:

. ,(eglot-alternatives '(("vscode-json-language-server" "--stdio")
                         ("vscode-json-languageserver" "--stdio")
                         ("json-languageserver" "--stdio"))))

installed, then these json-specific servers should start to manage
js-json-mode files in your project.  If afterwards you start M-x
eglot in a plain js file, that server won't be (erroneously) used
to manage JSON files.

João

diff --git a/lisp/progmodes/eglot.el b/lisp/progmodes/eglot.el
index d410367f902..5cfd5baa225 100644
--- a/lisp/progmodes/eglot.el
+++ b/lisp/progmodes/eglot.el
@@ -222,6 +222,7 @@ eglot-server-programs

("vscode-json-languageserver" "--stdio")

("json-languageserver" "--stdio"))))
                                 (((js-mode :language-id "javascript")
+                                  (js-json-mode :language-id "json")
;; bug#67463
                                   (js-ts-mode :language-id "javascript")
                                   (tsx-ts-mode :language-id "typescriptreact")
                                   (typescript-ts-mode :language-id
"typescript")
@@ -2493,10 +2494,12 @@ eglot--languageId
   "Compute LSP \\='languageId\\=' string for current buffer.
 Doubles as an predicate telling if SERVER can manage current
 buffer."
-  (cl-loop for (mode . languageid) in
-           (eglot--languages server)
-           when (provided-mode-derived-p major-mode mode)
-           return languageid))
+  ;; bug#67463
+  (cl-loop for pred in (list #'equal #'provided-mode-derived-p)
+           thereis (cl-loop for (mode . languageid) in
+                            (eglot--languages server)
+                            when (funcall pred major-mode mode)
+                            return languageid)))

 (defun eglot--TextDocumentItem ()
   "Compute TextDocumentItem object for current buffer."

bug#67463: 30.0.50; Eglot may manage js-json-mode buffers with wrong server

[Pengji Zhang]

Hi João,

On Sun, Dec 3, 2023 at 3:51 PM João Távora <joaotavora@gmail.com> wrote:
>
> Nope this doesn't work.  Even the slightly more involved patch after
> my sig  doesn't work, and it correctly reports the :languageId as "json"
>
> So I think the best courses of action are 3 and 4, in that order.
>

I also think option 3 could be the best. JSON is indeed a subset of
JavaScript but that does not mean valid JSON files are also valid
JavaScript files. For instance, a bare object literal '{"a": 10}' is
not accepted by perhaps all JavaScript runtimes because it is parsed
as a block of statements. That is also why we get those invalid errors
in the example.

> Another workaround is to first start M-x eglot in some json file
> in your project.  If you have one of:
>
> . ,(eglot-alternatives '(("vscode-json-language-server" "--stdio")
>                          ("vscode-json-languageserver" "--stdio")
>                          ("json-languageserver" "--stdio"))))
>
> installed, then these json-specific servers should start to manage
> js-json-mode files in your project.  If afterwards you start M-x
> eglot in a plain js file, that server won't be (erroneously) used
> to manage JSON files.
>

Thanks! This workaround indeed works, even though it is a bit tedious
(and I honestly do not quite need a language server for JSON).

Pengji

bug#67463: 30.0.50; Eglot may manage js-json-mode buffers with wrong server

[Eli Zaretskii]

> From: Pengji Zhang <kunhtkun@gmail.com>
> Date: Wed, 6 Dec 2023 20:32:39 -0500
> Cc: 67463@debbugs.gnu.org, Eli Zaretskii <eliz@gnu.org>
> 
> Hi João,
> 
> On Sun, Dec 3, 2023 at 3:51 PM João Távora <joaotavora@gmail.com> wrote:
> >
> > Nope this doesn't work.  Even the slightly more involved patch after
> > my sig  doesn't work, and it correctly reports the :languageId as "json"
> >
> > So I think the best courses of action are 3 and 4, in that order.
> >
> 
> I also think option 3 could be the best. JSON is indeed a subset of
> JavaScript but that does not mean valid JSON files are also valid
> JavaScript files. For instance, a bare object literal '{"a": 10}' is
> not accepted by perhaps all JavaScript runtimes because it is parsed
> as a block of statements. That is also why we get those invalid errors
> in the example.
> 
> > Another workaround is to first start M-x eglot in some json file
> > in your project.  If you have one of:
> >
> > . ,(eglot-alternatives '(("vscode-json-language-server" "--stdio")
> >                          ("vscode-json-languageserver" "--stdio")
> >                          ("json-languageserver" "--stdio"))))
> >
> > installed, then these json-specific servers should start to manage
> > js-json-mode files in your project.  If afterwards you start M-x
> > eglot in a plain js file, that server won't be (erroneously) used
> > to manage JSON files.
> >
> 
> Thanks! This workaround indeed works, even though it is a bit tedious
> (and I honestly do not quite need a language server for JSON).

So should we now close this bug?  Or is there something left to do
here?

bug#67463: 30.0.50; Eglot may manage js-json-mode buffers with wrong server

[João Távora]

[-- Attachment #1: Type: text/plain, Size: 2166 bytes --]

On Sat, Dec 16, 2023, 09:30 Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Pengji Zhang <kunhtkun@gmail.com>
> > Date: Wed, 6 Dec 2023 20:32:39 -0500
> > Cc: 67463@debbugs.gnu.org, Eli Zaretskii <eliz@gnu.org>
> >
> > Hi João,
> >
> > On Sun, Dec 3, 2023 at 3:51 PM João Távora <joaotavora@gmail.com> wrote:
> > >
> > > Nope this doesn't work.  Even the slightly more involved patch after
> > > my sig  doesn't work, and it correctly reports the :languageId as
> "json"
> > >
> > > So I think the best courses of action are 3 and 4, in that order.
> > >
> >
> > I also think option 3 could be the best. JSON is indeed a subset of
> > JavaScript but that does not mean valid JSON files are also valid
> > JavaScript files. For instance, a bare object literal '{"a": 10}' is
> > not accepted by perhaps all JavaScript runtimes because it is parsed
> > as a block of statements. That is also why we get those invalid errors
> > in the example.
> >
> > > Another workaround is to first start M-x eglot in some json file
> > > in your project.  If you have one of:
> > >
> > > . ,(eglot-alternatives '(("vscode-json-language-server" "--stdio")
> > >                          ("vscode-json-languageserver" "--stdio")
> > >                          ("json-languageserver" "--stdio"))))
> > >
> > > installed, then these json-specific servers should start to manage
> > > js-json-mode files in your project.  If afterwards you start M-x
> > > eglot in a plain js file, that server won't be (erroneously) used
> > > to manage JSON files.
> > >
> >
> > Thanks! This workaround indeed works, even though it is a bit tedious
> > (and I honestly do not quite need a language server for JSON).
>
> So should we now close this bug?  Or is there something left to do
> here?
>

The problem is not solved and the workaround is tedious. There is something
to be fixed, but outside Eglot.

I think the fix should be (3) breaking the inheritance between js-json-mode
and js-mode, using instead composition to achieve the same code reuse.  As
Pengji noted, JSON is not JS in any practical (or even theoretical) sense.

João

>

[-- Attachment #2: Type: text/html, Size: 3536 bytes --]

bug#67463: 30.0.50; Eglot may manage js-json-mode buffers with wrong server

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>> So should we now close this bug?  Or is there something left to do
>> here?
> The problem is not solved and the workaround is tedious. There is something
> to be fixed, but outside Eglot.

Agreed.  I can see two solutions:

- Add a tool like `derived-mode-remove-parent`.
- Fix the definition of `js-json-mode` so it doesn't appear as a child
  of `js-mode`.

I think the second solution is best.
A quick&dirty way to do that could be the patch below.


        Stefan


diff --git a/lisp/progmodes/js.el b/lisp/progmodes/js.el
index 0115feb0e97..ac99c96166c 100644
--- a/lisp/progmodes/js.el
+++ b/lisp/progmodes/js.el
@@ -3924,7 +3924,8 @@ js-ts--syntax-propertize
         (put-text-property (1- ne) ne 'syntax-table syntax)))))
 
 ;;;###autoload
-(define-derived-mode js-json-mode js-mode "JSON"
+(define-derived-mode js-json-mode prog-mode "JSON"
+  (js-mode) ;; For expediency, reuse js-mode, but not as parent (bug#67463).
   (setq-local js-enabled-frameworks nil)
   ;; Speed up `syntax-ppss': JSON files can be big but can't hold
   ;; regexp matchers nor #! thingies (and `js-enabled-frameworks' is nil).

bug#67463: 30.0.50; Eglot may manage js-json-mode buffers with wrong server

[Dmitry Gutov]

On 06/01/2024 18:01, Stefan Monnier via Bug reports for GNU Emacs, the 
Swiss army knife of text editors wrote:
> -(define-derived-mode js-json-mode js-mode "JSON"
> +(define-derived-mode js-json-mode prog-mode "JSON"
> +  (js-mode) ;; For expediency, reuse js-mode, but not as parent (bug#67463).

Nice.

bug#67463: 30.0.50; Eglot may manage js-json-mode buffers with wrong server

[João Távora]

On Sat, Jan 6, 2024 at 6:04 PM Dmitry Gutov <dmitry@gutov.dev> wrote:
>
> On 06/01/2024 18:01, Stefan Monnier via Bug reports for GNU Emacs, the
> Swiss army knife of text editors wrote:
> > -(define-derived-mode js-json-mode js-mode "JSON"
> > +(define-derived-mode js-json-mode prog-mode "JSON"
> > +  (js-mode) ;; For expediency, reuse js-mode, but not as parent (bug#67463).
>
> Nice.

I agree.  If this works, I think it's quick, but not necessarily
dirty.

On 06/01/2024 18:01, Stefan Monnier

> Add a tool like `derived-mode-remove-parent`.

Also agree, but the things we're doing in that symbol-plist
seem to have gotten rather complicated lately, so I wouldn't
be surpsied this is buggy in some edge case.

(require 'cl-lib)
(defun derived-mode-remove-parent (mode parent)
  (cl-macrolet ((allp (m) `(plist-get (symbol-plist ,m)
'derived-mode--all-parents))
                (dmp (m) `(plist-get (symbol-plist ,m) 'derived-mode-parent)))
    (setf (allp mode) (delq parent (allp mode)))
    (when (eq (dmp mode) parent)
      (setf (dmp mode) (dmp parent)))))

but Pengji can try to use it to do the following in the meantime:

(derived-mode-remove-parent 'js-json-mode 'js-mode)
(derived-mode-remove-parent 'js-json-mode 'js-base-mode)

thought the second one shouldn't strictly be needed at the moment.

João

bug#67463: 30.0.50; Eglot may manage js-json-mode buffers with wrong server

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>> On 06/01/2024 18:01, Stefan Monnier via Bug reports for GNU Emacs, the
>> Swiss army knife of text editors wrote:
>> > -(define-derived-mode js-json-mode js-mode "JSON"
>> > +(define-derived-mode js-json-mode prog-mode "JSON"
>> > +  (js-mode) ;; For expediency, reuse js-mode, but not as parent (bug#67463).
>>
>> Nice.
>
> I agree.  If this works, I think it's quick, but not necessarily
> dirty.

It's dirty because it means `kill-all-local-variables` is called twice.
And there might be other bad side effects.

I suggest the patch below instead, which also "quick" but much less dirty.

>> Add a tool like `derived-mode-remove-parent`.
> Also agree, but the things we're doing in that symbol-plist
> seem to have gotten rather complicated lately, so I wouldn't
> be surpsied this is buggy in some edge case.

We'd first have to decide what is the behavior we want from it.
Note that `derived-mode-add-parents` doesn't really "add", it just sets
(i.e. overwrites) the list of extra-parents, so a corresponding behavior
would be for `derived-mode-remove-parent` to set a new property
`derived-mode-muted-parents`.

This said, I'm not at all sure we want/need that.  It seems that its
only use-cases would be to fix erroneous inheritance as we have here in
`js-json-mode`, which are better fixed at the source.
If users wants to work around the problem before it's fixed at its
source, they can already do something like

    (put 'js-json-mode 'derived-mode-parent nil)


-- Stefan

bug#67463: 30.0.50; Eglot may manage js-json-mode buffers with wrong server

[João Távora]

On Sun, Jan 7, 2024 at 5:20 AM Stefan Monnier <monnier@iro.umontreal.ca> wrote:
 effects.

> I suggest the patch below instead, which also "quick" but much less dirty.

I didn't see it, did you attach it?

> We'd first have to decide what is the behavior we want from it.
> Note that `derived-mode-add-parents` doesn't really "add"

I'd think for every "add" it should be reasonably easy to
envision a "remove", but that complicates things indeed.

> (i.e. overwrites) the list of extra-parents, so a corresponding behavior
> would be for `derived-mode-remove-parent` to set a new property
> `derived-mode-muted-parents`.

Extra parents, muted parents, siblings become parents,
definitely some good 90's sitcom material :-)

João

bug#67463: 30.0.50; Eglot may manage js-json-mode buffers with wrong server

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

[-- Attachment #1: Type: text/plain, Size: 294 bytes --]

>> I suggest the patch below instead, which also "quick" but much less dirty.
> I didn't see it, did you attach it?

See second try below.

> Extra parents, muted parents, siblings become parents,
> definitely some good 90's sitcom material :-)

can't wait to see your script,


        Stefan

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: js-json.patch --]
[-- Type: text/x-diff, Size: 992 bytes --]

diff --git a/lisp/progmodes/js.el b/lisp/progmodes/js.el
index 0115feb0e97..f042774431d 100644
--- a/lisp/progmodes/js.el
+++ b/lisp/progmodes/js.el
@@ -3702,6 +3702,9 @@ js-base-mode
 (define-derived-mode js-mode js-base-mode "JavaScript"
   "Major mode for editing JavaScript."
   :group 'js
+  (js--mode-setup))
+
+(defun js--mode-setup ()
   ;; Ensure all CC Mode "lang variables" are set to valid values.
   (c-init-language-vars js-mode)
   (setq-local indent-line-function #'js-indent-line)
@@ -3924,7 +3927,8 @@ js-ts--syntax-propertize
         (put-text-property (1- ne) ne 'syntax-table syntax)))))
 
 ;;;###autoload
-(define-derived-mode js-json-mode js-mode "JSON"
+(define-derived-mode js-json-mode prog-mode "JSON"
+  (js--mode-setup) ;Reuse most of `js-mode', but not as parent (bug#67463).
   (setq-local js-enabled-frameworks nil)
   ;; Speed up `syntax-ppss': JSON files can be big but can't hold
   ;; regexp matchers nor #! thingies (and `js-enabled-frameworks' is nil).

bug#67463: 30.0.50; Eglot may manage js-json-mode buffers with wrong server

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>>> I suggest the patch below instead, which also "quick" but much less dirty.
>> I didn't see it, did you attach it?
> See second try below.

Pushed to `master`.


        Stefan