Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Send back diags using language server #3960

Merged
merged 8 commits into from
Dec 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .prettierignore
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
**/*.mdx
**/*.schema.json
**/dist/
**/out/
**/scripts/ts-json-schema-generator.cjs
**/yarn.lock
CHANGELOG.md
Expand Down
1 change: 1 addition & 0 deletions eslint.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ export default tsEslint.config(
'**/fixtures/**',
'**/fixtures/**/*.js',
'**/node_modules/**',
'**/out/**',
'**/scripts/ts-json-schema-generator.cjs',
'**/temp/**',
'**/webpack*.js',
Expand Down
2 changes: 1 addition & 1 deletion packages/_integrationTests/src/ExtensionApi.mts
Original file line number Diff line number Diff line change
@@ -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';
11 changes: 6 additions & 5 deletions packages/_server/.gitignore
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
out
node_modules
!.vscode
temp
dist
/out
node_modules
!.vscode
temp
/dist
/lib
4 changes: 2 additions & 2 deletions packages/_server/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
"require": "./dist/api.cjs"
},
"./lib": {
"import": "./dist/lib/index.mjs"
"import": "./out/lib/index.mjs"
}
},
"devDependencies": {
Expand Down Expand Up @@ -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",
Expand Down
2 changes: 1 addition & 1 deletion packages/_server/rollup.config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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()],
},
Expand Down
6 changes: 0 additions & 6 deletions packages/_server/src/api/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import type {
OnBlockFile,
OnDocumentConfigChange,
OnSpellCheckDocumentStep,
PublishDiagnostics,
SpellingSuggestionsResult,
SplitTextIntoWordsResult,
TextDocumentInfo,
Expand Down Expand Up @@ -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.
Expand Down
19 changes: 10 additions & 9 deletions packages/_server/src/server.mts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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,
Expand All @@ -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(
Expand Down
1 change: 0 additions & 1 deletion packages/_server/src/serverApi.mts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ export function createServerApi(connection: MessageConnection, handlers: Partial
},
clientNotifications: {
onSpellCheckDocument: true,
onDiagnostics: true,
onDocumentConfigChange: true,
onBlockFile: true,
},
Expand Down
1 change: 0 additions & 1 deletion packages/_server/src/test/test.api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ export function createMockServerSideApi() {
},
clientNotification: {
onSpellCheckDocument: vi.fn(),
onDiagnostics: vi.fn(),
onDocumentConfigChange: vi.fn(),
onBlockFile: vi.fn(),
},
Expand Down
35 changes: 23 additions & 12 deletions packages/_server/src/utils/catchPromise.mts
Original file line number Diff line number Diff line change
@@ -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}`);
};
}

Expand All @@ -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);
}
}
6 changes: 3 additions & 3 deletions packages/_server/src/utils/catchPromise.test.mts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});
});
2 changes: 1 addition & 1 deletion packages/_server/tsconfig.api.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"extends": "./tsconfig.json",
"compilerOptions": {
"outDir": "./dist",
"outDir": "./out",
"rootDir": "./src",
"skipLibCheck": true,
"types": ["node"]
Expand Down
1 change: 1 addition & 0 deletions packages/_server/tsconfig.test.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
{
"extends": "./tsconfig.json",
"compilerOptions": {
"noEmit": true,
"outDir": "./temp/dist",
"types": ["node"]
}
Expand Down
30 changes: 11 additions & 19 deletions packages/client/src/client/client.mts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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>;

Expand All @@ -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.
Expand All @@ -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
Expand All @@ -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();
Expand Down Expand Up @@ -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() {
Expand Down
3 changes: 0 additions & 3 deletions packages/client/src/client/server/server.mts
Original file line number Diff line number Diff line change
Expand Up @@ -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'];
Expand Down Expand Up @@ -98,7 +97,6 @@ export function createServerApi(client: LanguageClient): ServerApi {
},
clientNotifications: {
onSpellCheckDocument: true,
onDiagnostics: true,
onDocumentConfigChange: true,
onBlockFile: true,
},
Expand Down Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions vitest.config.mts
Original file line number Diff line number Diff line change
Expand Up @@ -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.*',
Expand Down
Loading