fix: Send back diags using language server (#3960)

.prettierignore

@@ -10,6 +10,7 @@
 **/*.mdx
 **/*.schema.json
 **/dist/
+**/out/
 **/scripts/ts-json-schema-generator.cjs
 **/yarn.lock
 CHANGELOG.md

eslint.config.js

@@ -30,6 +30,7 @@ export default tsEslint.config(
             '**/fixtures/**',
             '**/fixtures/**/*.js',
             '**/node_modules/**',
+            '**/out/**',
             '**/scripts/ts-json-schema-generator.cjs',
             '**/temp/**',
             '**/webpack*.js',

packages/_integrationTests/src/ExtensionApi.mts

@@ -1,3 +1,3 @@
-export type { OnSpellCheckDocumentStep } from '../../_server/dist/api.js';
+export type { OnSpellCheckDocumentStep } from '../../_server/dist/api.cjs';
 export type { CSpellClient } from '../../client/dist/client/index.mjs';
 export type { ExtensionApi } from '../../client/dist/extensionApi.mjs';

packages/_server/.gitignore

@@ -1,5 +1,6 @@
-out
-node_modules
-!.vscode
-temp
-dist
+/out
+node_modules
+!.vscode
+temp
+/dist
+/lib

packages/_server/package.json

@@ -26,7 +26,7 @@
       "require": "./dist/api.cjs"
     },
     "./lib": {
-      "import": "./dist/lib/index.mjs"
+      "import": "./out/lib/index.mjs"
     }
   },
   "devDependencies": {
@@ -72,7 +72,7 @@
     "#build:ts-json-schema-generator": "esbuild --bundle ../../../../code/clones/ts-json-schema-generator/dist/ts-json-schema-generator.js --outfile=scripts/ts-json-schema-generator.cjs  --platform=node --external:typescript",
     "clean-build-production": "npm run clean && npm run build:production",
     "clean-build": "npm run clean && npm run build",
-    "clean": "shx rm -rf dist temp out coverage",
+    "clean": "shx rm -rf dist temp out coverage lib",
     "test-watch": "vitest",
     "test": "vitest run",
     "watch": "concurrently npm:watch:esbuild npm:watch:api npm:watch:tsc",

packages/_server/rollup.config.mjs

@@ -2,7 +2,7 @@ import dts from 'rollup-plugin-dts';
 
 const config = [
     {
-        input: './dist/api.d.ts',
+        input: './out/api.d.ts',
         output: [{ file: './dist/api.d.cts', format: 'es' }],
         plugins: [dts()],
     },

packages/_server/src/api/api.ts

@@ -25,7 +25,6 @@ import type {
     OnBlockFile,
     OnDocumentConfigChange,
     OnSpellCheckDocumentStep,
-    PublishDiagnostics,
     SpellingSuggestionsResult,
     SplitTextIntoWordsResult,
     TextDocumentInfo,
@@ -92,11 +91,6 @@ export interface ClientNotificationsAPI {
      * @param step - The step in the spell checking process.
      */
     onSpellCheckDocument(step: OnSpellCheckDocumentStep): void;
-    /**
-     * Send updated document diagnostics to the client.
-     * @param pub - The diagnostics to publish.
-     */
-    onDiagnostics(pub: PublishDiagnostics): void;
 
     /**
      * Notify the client that the configuration has for the listed document URIs.

packages/_server/src/server.mts

@@ -83,6 +83,14 @@ async function calcDefaultSettings(): Promise<CSpellUserAndExtensionSettings> {
     };
 }
 
+// This is to filter out the "Off" severity that is used to hide issues from the VS Code Problems panel.
+const knownDiagnosticSeverityLevels = new Set<number | undefined>([
+    DiagnosticSeverity.Error,
+    DiagnosticSeverity.Warning,
+    DiagnosticSeverity.Information,
+    DiagnosticSeverity.Hint,
+]);
+
 export function run(): void {
     // debounce buffer
     const disposables = createDisposableList();
@@ -488,14 +496,6 @@ export function run(): void {
 
         const diags: Required<PublishDiagnosticsParams> = { uri, version, diagnostics };
 
-        // This is to filter out the "Off" severity that is used to hide issues from the VS Code Problems panel.
-        const knownDiagnosticSeverityLevels = new Set<number | undefined>([
-            DiagnosticSeverity.Error,
-            DiagnosticSeverity.Warning,
-            DiagnosticSeverity.Information,
-            DiagnosticSeverity.Hint,
-        ]);
-
         function mapDiagnostic(diag: Diagnostic): Diagnostic {
             return {
                 ...diag,
@@ -504,7 +504,8 @@ export function run(): void {
         }
 
         const diagsForClient = { ...diags, diagnostics: diags.diagnostics.map(mapDiagnostic) };
-        catchPromise(clientServerApi.clientNotification.onDiagnostics(diagsForClient));
+        catchPromise(connection.sendDiagnostics(diagsForClient));
+        // catchPromise(clientServerApi.clientNotification.onDiagnostics(diagsForClient));
     }
 
     async function shouldValidateDocument(

packages/_server/src/serverApi.mts

@@ -31,7 +31,6 @@ export function createServerApi(connection: MessageConnection, handlers: Partial
         },
         clientNotifications: {
             onSpellCheckDocument: true,
-            onDiagnostics: true,
             onDocumentConfigChange: true,
             onBlockFile: true,
         },

packages/_server/src/test/test.api.ts

@@ -22,7 +22,6 @@ export function createMockServerSideApi() {
         },
         clientNotification: {
             onSpellCheckDocument: vi.fn(),
-            onDiagnostics: vi.fn(),
             onDocumentConfigChange: vi.fn(),
             onBlockFile: vi.fn(),
         },

packages/_server/src/utils/catchPromise.mts

@@ -1,16 +1,14 @@
 /* eslint-disable @typescript-eslint/unified-signatures */
-type ErrorHandler<T> = (e: unknown) => T;
+type ErrorHandler = (e: unknown) => void;
 
 function defaultHandler(e: unknown) {
     console.error(e);
-    return undefined;
 }
 
-function contextHandler(context: string | undefined): (e: unknown) => undefined {
+function contextHandler(context: string | undefined): (e: unknown) => void {
     if (!context) return defaultHandler;
     return (e) => {
-        console.error('%s: %s', context, e);
-        return undefined;
+        console.error(`${context}: ${e}`);
     };
 }
 
@@ -19,20 +17,33 @@ function contextHandler(context: string | undefined): (e: unknown) => undefined
  * @param p - the promise to catch
  * @param handler - a handler to handle the rejection.
  */
-export function catchPromise<T, U>(p: Promise<T>, handler: ErrorHandler<U>): Promise<T | U>;
+export function catchPromise<T, U>(p: Promise<T>, handler: ErrorHandler): Promise<T | U>;
 /**
  * Used for catching promises that are not returned. A fire and forget situation.
  * If the promise is rejected, it is resolved with `undefined`.
  * @param p - the promise to catch
  */
-export function catchPromise<T>(p: Promise<T>): Promise<T | undefined>;
-export function catchPromise<T>(p: Promise<T>, context: string): Promise<T | undefined>;
-export function catchPromise<T>(p: Promise<T>, handler?: ErrorHandler<T | undefined>): Promise<T | undefined>;
-export async function catchPromise<T>(p: Promise<T>, handlerOrContext?: ErrorHandler<T | undefined> | string): Promise<T | undefined> {
+export function catchPromise<T>(p: Promise<T>): Promise<void>;
+/**
+ * Used for catching promises that are not returned. A fire and forget situation.
+ * If the promise is rejected, it is resolved with `undefined`.
+ * @param p - the promise to catch
+ * @param context - A context string to help identify where the error came from.
+ */
+export function catchPromise<T>(p: Promise<T>, context: string): Promise<void>;
+/**
+ * Used for catching promises that are not returned. A fire and forget situation.
+ * If the promise is rejected, it is resolved with `undefined`.
+ * @param p - the promise to catch
+ * @param handler - a handler to handle the rejection.
+ */
+export function catchPromise<T>(p: Promise<T>, handler: ErrorHandler): Promise<void>;
+export function catchPromise<T>(p: Promise<T>, handlerOrContext?: ErrorHandler | string): Promise<void>;
+export async function catchPromise<T>(p: Promise<T>, handlerOrContext?: ErrorHandler | string): Promise<void> {
     const handler = typeof handlerOrContext === 'function' ? handlerOrContext : contextHandler(handlerOrContext);
     try {
-        return await p;
+        await p;
     } catch (e) {
-        return handler(e);
+        handler(e);
     }
 }

packages/_server/src/utils/catchPromise.test.mts

@@ -16,18 +16,18 @@ describe('catchPromise', () => {
     test('catchPromise with context', async () => {
         const err = vi.spyOn(console, 'error').mockImplementation(() => undefined);
         await expect(catchPromise(Promise.reject(Error('test')), 'Testing')).resolves.toBe(undefined);
-        expect(err).toHaveBeenCalledWith('%s: %s', 'Testing', expect.any(Error));
+        expect(err).toHaveBeenCalledWith(expect.stringContaining(`Testing: `));
     });
 
     test('catchPromise custom handler', async () => {
         const err = vi.spyOn(console, 'error').mockImplementation(() => undefined);
-        await expect(catchPromise(Promise.reject('error'), () => 23)).resolves.toBe(23);
+        await expect(catchPromise(Promise.reject('error'), () => 23)).resolves.toBe(undefined);
         expect(err).not.toHaveBeenCalled();
     });
 
     test('catchPromise resolve', async () => {
         const err = vi.spyOn(console, 'error').mockImplementation(() => undefined);
-        await expect(catchPromise(Promise.resolve('msg'))).resolves.toBe('msg');
+        await expect(catchPromise(Promise.resolve('msg'))).resolves.toBe(undefined);
         expect(err).not.toHaveBeenCalled();
     });
 });

packages/_server/tsconfig.api.json

@@ -1,7 +1,7 @@
 {
     "extends": "./tsconfig.json",
     "compilerOptions": {
-        "outDir": "./dist",
+        "outDir": "./out",
         "rootDir": "./src",
         "skipLibCheck": true,
         "types": ["node"]

packages/_server/tsconfig.test.json

@@ -1,6 +1,7 @@
 {
     "extends": "./tsconfig.json",
     "compilerOptions": {
+        "noEmit": true,
         "outDir": "./temp/dist",
         "types": ["node"]
     }

packages/client/src/client/client.mts

@@ -10,9 +10,9 @@ import type {
     WorkspaceConfigForDocument,
 } from 'code-spell-checker-server/api';
 import { extractEnabledSchemeList, extractKnownFileTypeIds } from 'code-spell-checker-server/lib';
-import { createDisposableList, type DisposableHybrid } from 'utils-disposables';
+import { createDisposableList, type DisposableHybrid, makeDisposable } from 'utils-disposables';
 import type { CodeAction, Diagnostic, DiagnosticCollection, Disposable, ExtensionContext, Range, TextDocument } from 'vscode';
-import { languages as vsCodeSupportedLanguages, Uri, workspace } from 'vscode';
+import { EventEmitter, languages as vsCodeSupportedLanguages, Uri, workspace } from 'vscode';
 
 import { diagnosticSource } from '../constants.js';
 import { isLcCodeAction, mapDiagnosticToLc, mapLcCodeAction, mapRangeToLc } from '../languageServer/clientHelpers.js';
@@ -48,7 +48,7 @@ export { GetConfigurationForDocumentResult } from './server/index.mjs';
 // The debug options for the server
 const debugExecArgv = ['--nolazy', '--inspect=60048'];
 
-const diagnosticCollectionName = diagnosticSource;
+const diagnosticCollectionName = diagnosticSource + 'Server';
 
 export type ServerResponseIsSpellCheckEnabled = Partial<IsSpellCheckEnabledResult>;
 
@@ -75,6 +75,7 @@ export class CSpellClient implements Disposable {
     private broadcasterOnDocumentConfigChange = createBroadcaster<OnDocumentConfigChange>();
     private broadcasterOnBlockFile = createBroadcaster<OnBlockFile>();
     private ready = new Resolvable<void>();
+    private diagEmitter = new EventEmitter<DiagnosticsFromServer>();
 
     /**
      * @param: {string} module -- absolute path to the server module.
@@ -98,6 +99,11 @@ export class CSpellClient implements Disposable {
 
         this.languageIds = new Set([...languageIds, ...LanguageIds.languageIds, ...extractKnownFileTypeIds(settings)]);
 
+        const handleDiagnostics = (uri: Uri, diagnostics: Diagnostic[]) => {
+            // logger.log(`${new Date().toISOString()} Client handleDiagnostics: ${uri.toString()}`);
+            this.diagEmitter.fire({ uri, diagnostics });
+        };
+
         const uniqueLangIds = [...this.languageIds];
         const documentSelector = [...this.allowedSchemas].flatMap((scheme) => uniqueLangIds.map((language) => ({ language, scheme })));
         // Options to control the language client
@@ -108,12 +114,7 @@ export class CSpellClient implements Disposable {
                 // Synchronize the setting section 'spellChecker' to the server
                 configurationSection: [sectionCSpell, 'search'],
             },
-            middleware: {
-                handleDiagnostics(uri, diagnostics, next) {
-                    logger.log(`${new Date().toISOString()} Client handleDiagnostics: ${uri.toString()}`);
-                    next(uri, diagnostics);
-                },
-            },
+            middleware: { handleDiagnostics },
         };
 
         const execArgv = this.calcServerArgs();
@@ -340,16 +341,7 @@ export class CSpellClient implements Disposable {
     }
 
     public onDiagnostics(fn: (diags: DiagnosticsFromServer) => void): DisposableHybrid {
-        return this.serverApi.onDiagnostics((pub) => {
-            const cvt = this.client.protocol2CodeConverter;
-            const uri = cvt.asUri(pub.uri);
-            const diags: DiagnosticsFromServer = {
-                uri,
-                version: pub.version,
-                diagnostics: pub.diagnostics.map((diag) => cvt.asDiagnostic(diag)),
-            };
-            return fn(diags);
-        });
+        return makeDisposable(this.diagEmitter.event(fn));
     }
 
     private async initWhenReady() {

packages/client/src/client/server/server.mts

@@ -60,7 +60,6 @@ interface ServerSide {
 }
 
 interface ExtensionSide {
-    onDiagnostics: ClientSideApi['clientNotification']['onDiagnostics']['subscribe'];
     onDocumentConfigChange: ClientSideApi['clientNotification']['onDocumentConfigChange']['subscribe'];
     onSpellCheckDocument: ClientSideApi['clientNotification']['onSpellCheckDocument']['subscribe'];
     onBlockFile: ClientSideApi['clientNotification']['onBlockFile']['subscribe'];
@@ -98,7 +97,6 @@ export function createServerApi(client: LanguageClient): ServerApi {
         },
         clientNotifications: {
             onSpellCheckDocument: true,
-            onDiagnostics: true,
             onDocumentConfigChange: true,
             onBlockFile: true,
         },
@@ -127,7 +125,6 @@ export function createServerApi(client: LanguageClient): ServerApi {
         onSpellCheckDocument: (fn) => clientNotification.onSpellCheckDocument.subscribe(log2Cfn(fn, 'onSpellCheckDocument')),
         onDocumentConfigChange: (fn) => clientNotification.onDocumentConfigChange.subscribe(log2Cfn(fn, 'onDocumentConfigChange')),
         onBlockFile: (fn) => clientNotification.onBlockFile.subscribe(log2Cfn(fn, 'onBlockFile')),
-        onDiagnostics: (fn) => clientNotification.onDiagnostics.subscribe(log2Cfn(fn, 'onDiagnostics')),
         onWorkspaceConfigForDocumentRequest: (fn) =>
             clientRequest.onWorkspaceConfigForDocumentRequest.subscribe(log2Cfn(fn, 'onWorkspaceConfigForDocumentRequest')),
         dispose: rpcApi.dispose,

vitest.config.mts

@@ -9,6 +9,7 @@ export default defineConfig({
             '**/dist/**',
             '**/cypress/**',
             '**/coverage/**',
+            '**/out/**',
             '**/temp/**',
             '**/.{idea,git,cache,output,temp}/**',
             '**/{karma,rollup,webpack,vite,vitest,jest,ava,babel,nyc,cypress,tsup,build}.config.*',