fix(lsp): address review and quality gate findings

Author: amondnet

packages/lsp/src/client.ts

@@ -102,21 +102,22 @@ export async function createLSPClient(input: {
   // out-of-order reply can't overwrite fresher diagnostics with stale ones.
   const pullDiagnostics = async (filePath: string): Promise<void> => {
     if (!supportsPullDiagnostics) { return }
-    const pullID = (diagnosticPulls[filePath] ?? 0) + 1
-    diagnosticPulls[filePath] = pullID
+    const normalizedPath = normalizePath(filePath)
+    const pullID = (diagnosticPulls[normalizedPath] ?? 0) + 1
+    diagnosticPulls[normalizedPath] = pullID
     try {
       const report = (await withTimeout(
         connection.sendRequest('textDocument/diagnostic', {
-          textDocument: { uri: pathToFileURL(filePath).href },
+          textDocument: { uri: pathToFileURL(normalizedPath).href },
         }),
         DIAGNOSTICS_WAIT_TIMEOUT_MS,
       )) as { kind?: string, items?: Diagnostic[] } | null
       // Drop a response superseded by a newer pull or close.
-      if (diagnosticPulls[filePath] !== pullID) { return }
+      if (diagnosticPulls[normalizedPath] !== pullID) { return }
       // A "full" report carries items; an "unchanged" report means keep the
       // previously reported set, so leave the map as-is.
       if (report?.kind === 'full' && Array.isArray(report.items)) {
-        applyDiagnostics(normalizePath(filePath), report.items)
+        applyDiagnostics(normalizedPath, report.items)
       }
     }
     catch {
@@ -250,8 +251,9 @@ export async function createLSPClient(input: {
           },
         })
         delete files[filePath]
-        diagnosticPulls[filePath] = (diagnosticPulls[filePath] ?? 0) + 1
-        diagnostics.delete(normalizePath(filePath))
+        const normalizedPath = normalizePath(filePath)
+        diagnosticPulls[normalizedPath] = (diagnosticPulls[normalizedPath] ?? 0) + 1
+        diagnostics.delete(normalizedPath)
       },
     },
 

packages/lsp/test/fixture/fake-lsp-server.js

@@ -1,55 +1,13 @@
 // Simple JSON-RPC 2.0 LSP-like fake server over stdio
 // Implements a minimal LSP handshake and publishes diagnostics
 
-import { Buffer } from 'node:buffer'
-import process from 'node:process'
+import { send, start } from './fake-lsp-transport.js'
 
 let nextId = 1
 // Tracks how many didOpen notifications were received per uri, so tests can
 // assert that concurrent opens do not emit a duplicate didOpen.
 const openCounts = {}
 
-function encode(message) {
-  const json = JSON.stringify(message)
-  const header = `Content-Length: ${Buffer.byteLength(json, 'utf8')}\r\n\r\n`
-  return Buffer.concat([
-    Buffer.from(header, 'utf8'),
-    Buffer.from(json, 'utf8'),
-  ])
-}
-
-function decodeFrames(buffer) {
-  const results = []
-  let idx = buffer.indexOf('\r\n\r\n')
-  while (idx !== -1) {
-    const header = buffer.slice(0, idx).toString('utf8')
-    const m = /Content-Length:\s*(\d+)/i.exec(header)
-    const len = m ? Number.parseInt(m[1], 10) : 0
-    const bodyStart = idx + 4
-    const bodyEnd = bodyStart + len
-    if (buffer.length < bodyEnd)
-      break
-    const body = buffer.slice(bodyStart, bodyEnd).toString('utf8')
-    results.push(body)
-    buffer = buffer.slice(bodyEnd)
-    idx = buffer.indexOf('\r\n\r\n')
-  }
-  return { messages: results, rest: buffer }
-}
-
-let readBuffer = Buffer.alloc(0)
-
-process.stdin.on('data', (chunk) => {
-  readBuffer = Buffer.concat([readBuffer, chunk])
-  const { messages, rest } = decodeFrames(readBuffer)
-  readBuffer = rest
-  for (const m of messages) handle(m)
-})
-
-function send(msg) {
-  process.stdout.write(encode(msg))
-}
-
 function sendNotification(method, params) {
   send({ jsonrpc: '2.0', method, params })
 }
@@ -60,14 +18,7 @@ function sendRequest(method, params) {
   return id
 }
 
-function handle(raw) {
-  let data
-  try {
-    data = JSON.parse(raw)
-  }
-  catch {
-    return
-  }
+start((data) => {
 
   // Initialize request
   if (data.method === 'initialize') {
@@ -211,4 +162,4 @@ function handle(raw) {
   if (typeof data.id !== 'undefined') {
     send({ jsonrpc: '2.0', id: data.id, result: null })
   }
-}
+})

packages/lsp/test/fixture/fake-lsp-transport.js

@@ -0,0 +1,49 @@
+import { Buffer } from 'node:buffer'
+import process from 'node:process'
+
+function encode(message) {
+  const json = JSON.stringify(message)
+  const header = `Content-Length: ${Buffer.byteLength(json, 'utf8')}\r\n\r\n`
+  return Buffer.concat([
+    Buffer.from(header, 'utf8'),
+    Buffer.from(json, 'utf8'),
+  ])
+}
+
+function decodeFrames(buffer) {
+  const messages = []
+  let headerEnd = buffer.indexOf('\r\n\r\n')
+  while (headerEnd !== -1) {
+    const header = buffer.slice(0, headerEnd).toString('utf8')
+    const match = /Content-Length:\s*(\d+)/i.exec(header)
+    const length = match ? Number.parseInt(match[1], 10) : 0
+    const bodyStart = headerEnd + 4
+    const bodyEnd = bodyStart + length
+    if (buffer.length < bodyEnd) { break }
+    messages.push(buffer.slice(bodyStart, bodyEnd).toString('utf8'))
+    buffer = buffer.slice(bodyEnd)
+    headerEnd = buffer.indexOf('\r\n\r\n')
+  }
+  return { messages, rest: buffer }
+}
+
+export function send(message) {
+  process.stdout.write(encode(message))
+}
+
+export function start(handler) {
+  let readBuffer = Buffer.alloc(0)
+  process.stdin.on('data', (chunk) => {
+    readBuffer = Buffer.concat([readBuffer, chunk])
+    const { messages, rest } = decodeFrames(readBuffer)
+    readBuffer = rest
+    for (const raw of messages) {
+      try {
+        handler(JSON.parse(raw))
+      }
+      catch {
+        // Ignore malformed test input.
+      }
+    }
+  })
+}

packages/lsp/test/fixture/fake-pull-lsp-server.js

@@ -3,58 +3,9 @@
 // Mirrors how typescript-go (tsgo) behaves, so the client's pull path can be
 // tested without the real binary.
 
-import { Buffer } from 'node:buffer'
-import process from 'node:process'
+import { send, start } from './fake-lsp-transport.js'
 
-function encode(message) {
-  const json = JSON.stringify(message)
-  const header = `Content-Length: ${Buffer.byteLength(json, 'utf8')}\r\n\r\n`
-  return Buffer.concat([
-    Buffer.from(header, 'utf8'),
-    Buffer.from(json, 'utf8'),
-  ])
-}
-
-function decodeFrames(buffer) {
-  const results = []
-  let idx = buffer.indexOf('\r\n\r\n')
-  while (idx !== -1) {
-    const header = buffer.slice(0, idx).toString('utf8')
-    const m = /Content-Length:\s*(\d+)/i.exec(header)
-    const len = m ? Number.parseInt(m[1], 10) : 0
-    const bodyStart = idx + 4
-    const bodyEnd = bodyStart + len
-    if (buffer.length < bodyEnd)
-      break
-    const body = buffer.slice(bodyStart, bodyEnd).toString('utf8')
-    results.push(body)
-    buffer = buffer.slice(bodyEnd)
-    idx = buffer.indexOf('\r\n\r\n')
-  }
-  return { messages: results, rest: buffer }
-}
-
-let readBuffer = Buffer.alloc(0)
-
-process.stdin.on('data', (chunk) => {
-  readBuffer = Buffer.concat([readBuffer, chunk])
-  const { messages, rest } = decodeFrames(readBuffer)
-  readBuffer = rest
-  for (const m of messages) handle(m)
-})
-
-function send(msg) {
-  process.stdout.write(encode(msg))
-}
-
-function handle(raw) {
-  let data
-  try {
-    data = JSON.parse(raw)
-  }
-  catch {
-    return
-  }
+start((data) => {
 
   // Initialize request - advertise a diagnostic provider (pull model).
   if (data.method === 'initialize') {
@@ -115,4 +66,4 @@ function handle(raw) {
   if (typeof data.id !== 'undefined') {
     send({ jsonrpc: '2.0', id: data.id, result: null })
   }
-}
+})