Skip to content

Commit

Permalink
fix: Reduce handshaking between extension and spell checker server (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
Jason3S authored Oct 12, 2023
1 parent f9c161d commit 73b7c03
Show file tree
Hide file tree
Showing 22 changed files with 426 additions and 287 deletions.
3 changes: 0 additions & 3 deletions packages/_server/src/api/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,6 @@ export interface ServerNotificationsAPI {
* Note: RPC requests to the client/extension is rare.
*/
export interface ClientRequestsAPI {
// addWordsToVSCodeSettingsFromServer: (words: string[], documentUri: string, target: ConfigurationTarget) => void;
// addWordsToDictionaryFileFromServer: (words: string[], documentUri: string, dict: { uri: string; name: string }) => void;
// addWordsToConfigFileFromServer: (words: string[], documentUri: string, config: { uri: string; name: string }) => void;
onWorkspaceConfigForDocumentRequest: (req: WorkspaceConfigForDocumentRequest) => WorkspaceConfigForDocumentResponse;
}

Expand Down
30 changes: 16 additions & 14 deletions packages/_server/src/codeActions.mts
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,13 @@ import { capitalize } from '@internal/common-utils/util.js';
import type { SpellingDictionary } from 'cspell-lib';
import { constructSettingsForText, getDictionary, IssueType, Text } from 'cspell-lib';
import { format } from 'util';
import type { CodeActionParams, Range as LangServerRange, TextDocuments } from 'vscode-languageserver/node.js';
import { Command as LangServerCommand } from 'vscode-languageserver/node.js';
import type { TextDocument } from 'vscode-languageserver-textdocument';
import type { Diagnostic } from 'vscode-languageserver-types';
import { CodeAction, CodeActionKind, TextEdit } from 'vscode-languageserver-types';

import type { ServerSideApi } from './api.js';
import type { UriString, WorkspaceConfigForDocument } from './api.js';
import { clientCommands as cc } from './commands.mjs';
import type { ConfigScope, ConfigTarget, ConfigTargetCSpell, ConfigTargetDictionary, ConfigTargetVSCode } from './config/configTargets.mjs';
import { ConfigKinds, ConfigScopes } from './config/configTargets.mjs';
Expand All @@ -21,8 +23,6 @@ import { SuggestionGenerator } from './SuggestionsGenerator.mjs';
import { uniqueFilter } from './utils/index.mjs';
import * as range from './utils/range.mjs';
import * as Validator from './validator.mjs';
import type { CodeActionParams, Range as LangServerRange, TextDocuments } from './vscodeLanguageServer/index.cjs';
import { Command as LangServerCommand } from './vscodeLanguageServer/index.cjs';

const createCommand = LangServerCommand.create;

Expand All @@ -38,13 +38,17 @@ function extractDiagnosticData(diag: Diagnostic): DiagnosticData {
return data as DiagnosticData;
}

export interface CodeActionHandlerDependencies {
fetchSettings: (doc: TextDocument) => Promise<CSpellUserSettings>;
getSettingsVersion: (doc: TextDocument) => number;
fetchWorkspaceConfigForDocument: (uri: UriString) => Promise<WorkspaceConfigForDocument>;
}

export function onCodeActionHandler(
documents: TextDocuments<TextDocument>,
fnSettings: (doc: TextDocument) => Promise<CSpellUserSettings>,
fnSettingsVersion: (doc: TextDocument) => number,
clientApi: ServerSideApi,
dependencies: CodeActionHandlerDependencies,
): (params: CodeActionParams) => Promise<CodeAction[]> {
const codeActionHandler = new CodeActionHandler(documents, fnSettings, fnSettingsVersion, clientApi);
const codeActionHandler = new CodeActionHandler(documents, dependencies);

return (params) => codeActionHandler.handler(params);
}
Expand All @@ -62,17 +66,15 @@ class CodeActionHandler {

constructor(
readonly documents: TextDocuments<TextDocument>,
readonly fnSettings: (doc: TextDocument) => Promise<CSpellUserSettings>,
readonly fnSettingsVersion: (doc: TextDocument) => number,
readonly clientApi: ServerSideApi,
readonly dependencies: CodeActionHandlerDependencies,
) {
this.settingsCache = new Map<string, CacheEntry>();
this.sugGen = new SuggestionGenerator((doc) => this.getSettings(doc));
}

async getSettings(doc: TextDocument): Promise<GetSettingsResult> {
const cached = this.settingsCache.get(doc.uri);
const settingsVersion = this.fnSettingsVersion(doc);
const settingsVersion = this.dependencies.getSettingsVersion(doc);
if (cached?.docVersion === doc.version && cached.settingsVersion === settingsVersion) {
return cached.settings;
}
Expand All @@ -82,7 +84,7 @@ class CodeActionHandler {
}

private async constructSettings(doc: TextDocument): Promise<SettingsDictPair> {
const settings = constructSettingsForText(await this.fnSettings(doc), doc.getText(), doc.languageId);
const settings = constructSettingsForText(await this.dependencies.fetchSettings(doc), doc.getText(), doc.languageId);
const dictionary = await getDictionary(settings);
return { settings, dictionary };
}
Expand Down Expand Up @@ -134,7 +136,7 @@ class CodeActionHandler {
log(`CodeAction Uri Not allowed: ${uri}`);
return [];
}
const pWorkspaceConfig = this.clientApi.clientRequest.onWorkspaceConfigForDocumentRequest({ uri });
const pWorkspaceConfig = this.dependencies.fetchWorkspaceConfigForDocument(uri);

function replaceText(range: LangServerRange, text?: string) {
return TextEdit.replace(range, text || '');
Expand Down Expand Up @@ -194,7 +196,7 @@ class CodeActionHandler {
if (eslintSpellCheckerDiags.length > 1) return [];

const { settings: docSetting, dictionary } = await this.getSettings(textDocument);
const pWorkspaceConfig = this.clientApi.clientRequest.onWorkspaceConfigForDocumentRequest({ uri });
const pWorkspaceConfig = this.dependencies.fetchWorkspaceConfigForDocument(uri);

async function genCodeActions(_dictionary: SpellingDictionary) {
const word = extractText(textDocument, params.range);
Expand Down
3 changes: 2 additions & 1 deletion packages/_server/src/commands.mts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { Command } from 'vscode-languageserver/node.js';

import type { CommandsToClient } from './api.js';
import { Command } from './vscodeLanguageServer/index.cjs';

const prefix = 'cSpell.';

Expand Down
2 changes: 1 addition & 1 deletion packages/_server/src/config/WorkspacePathResolver.mts
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@ import { logError } from '@internal/common-utils/log.js';
import type { BaseSetting, Glob, GlobDef } from 'cspell-lib';
import * as os from 'os';
import * as Path from 'path';
import type { WorkspaceFolder } from 'vscode-languageserver/node.js';
import { URI as Uri } from 'vscode-uri';

import type { WorkspaceFolder } from '../vscodeLanguageServer/index.cjs';
import type { CSpellUserSettings } from './cspellConfig/index.mjs';
import { extractDictionaryDefinitions, extractDictionaryList } from './customDictionaries.mjs';

Expand Down
2 changes: 1 addition & 1 deletion packages/_server/src/config/WorkspacePathResolver.test.mts
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import { logError } from '@internal/common-utils/log.js';
import * as Path from 'path';
import { describe, expect, type Mock, test, vi } from 'vitest';
import type { WorkspaceFolder } from 'vscode-languageserver/node.js';
import { URI as Uri } from 'vscode-uri';

import type { WorkspaceFolder } from '../vscodeLanguageServer/index.cjs';
import type { CSpellUserSettings, CustomDictionaries } from './cspellConfig/index.mjs';
import { createWorkspaceNamesResolver, debugExports, resolveSettings } from './WorkspacePathResolver.mjs';

Expand Down
2 changes: 1 addition & 1 deletion packages/_server/src/config/configWatcher.mts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import type { CSpellUserSettings } from '@cspell/cspell-types';
import { getSources } from 'cspell-lib';
import type { Disposable } from 'vscode-languageserver/node.js';

import { FileWatcher } from '../utils/fileWatcher.mjs';
import type { Disposable } from '../vscodeLanguageServer/index.cjs';

export class ConfigWatcher extends FileWatcher implements Disposable {
constructor() {
Expand Down
2 changes: 1 addition & 1 deletion packages/_server/src/config/dictionaryWatcher.mts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import type { CSpellUserSettings } from '@cspell/cspell-types';
import type { Disposable } from 'vscode-languageserver/node.js';

import { FileWatcher } from '../utils/fileWatcher.mjs';
import type { Disposable } from '../vscodeLanguageServer/index.cjs';

export type Listener = (eventType?: string, filename?: string) => void;

Expand Down
39 changes: 24 additions & 15 deletions packages/_server/src/config/documentSettings.mts
Original file line number Diff line number Diff line change
Expand Up @@ -24,20 +24,20 @@ import {
ExclusionHelper,
getSources,
mergeSettings,
readSettingsFiles as cspellReadSettingsFiles,
readSettings as cspellReadSettingsFile,
searchForConfig,
} from 'cspell-lib';
import * as fs from 'fs';
import type { Sequence } from 'gensequence';
import { genSequence } from 'gensequence';
import * as os from 'os';
import * as path from 'path';
import type { Connection, WorkspaceFolder } from 'vscode-languageserver/node.js';
import { URI as Uri, Utils as UriUtils } from 'vscode-uri';

import type { VSCodeSettingsCspell } from '../api.js';
import type { DocumentUri, ServerSideApi, VSCodeSettingsCspell, WorkspaceConfigForDocument } from '../api.js';
import { extensionId } from '../constants.mjs';
import { uniqueFilter } from '../utils/index.mjs';
import type { Connection, WorkspaceFolder } from '../vscodeLanguageServer/index.cjs';
import type { CSpellUserSettings } from './cspellConfig/index.mjs';
import { canAddWordsToDictionary } from './customDictionaries.mjs';
import { handleSpecialUri } from './docUriHelper.mjs';
Expand Down Expand Up @@ -100,20 +100,21 @@ const schemeBlockList = ['git', 'output', 'debug'];

const defaultRootUri = toFileUri(process.cwd()).toString();

const _defaultSettings: CSpellUserSettings = Object.freeze({});
const _defaultSettings: CSpellUserSettings = Object.freeze(Object.create(null));

const defaultCheckOnlyEnabledFileTypes = true;

interface Clearable {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
clear: () => any;
clear: () => void;
}

export class DocumentSettings {
// Cache per folder settings
private cachedValues: Clearable[] = [];
private readonly fetchSettingsForUri = this.createCache((key: string) => this._fetchSettingsForUri(key));
private readonly fetchVSCodeConfiguration = this.createCache((key: string) => this._fetchVSCodeConfiguration(key));
private readonly fetchSettingsForUri = this.createCache((docUri: string) => this._fetchSettingsForUri(docUri));
private readonly fetchVSCodeConfiguration = this.createCache((uri: string) => this._fetchVSCodeConfiguration(uri));
private readonly fetchRepoRootForDir = this.createCache((dir: FsPath) => findRepoRoot(dir));
public readonly fetchWorkspaceConfiguration = this.createCache((docUri: DocumentUri) => this._fetchWorkspaceConfiguration(docUri));
private readonly _folders = this.createLazy(() => this.fetchFolders());
readonly configsToImport = new Set<string>();
private readonly importedSettings = this.createLazy(() => this._importSettings());
Expand All @@ -122,6 +123,7 @@ export class DocumentSettings {

constructor(
readonly connection: Connection,
readonly api: ServerSideApi,
readonly defaultSettings: CSpellUserSettings = _defaultSettings,
) {}

Expand Down Expand Up @@ -197,12 +199,13 @@ export class DocumentSettings {
return calcExcludedBy(uri, extSettings);
}

resetSettings(): void {
async resetSettings(): Promise<void> {
log('resetSettings');
clearCachedFiles();
const waitFor = clearCachedFiles();
this.cachedValues.forEach((cache) => cache.clear());
this._version += 1;
this.gitIgnore = new GitIgnore();
await waitFor;
}

get folders(): Promise<WorkspaceFolder[]> {
Expand All @@ -212,18 +215,22 @@ export class DocumentSettings {
private _importSettings() {
log('importSettings');
const importPaths = [...this.configsToImport].sort();
return readSettingsFiles(importPaths);
return mergeSettings({}, ...readSettingsFiles(importPaths));
}

private async _fetchWorkspaceConfiguration(uri: DocumentUri): Promise<WorkspaceConfigForDocument> {
return this.api.clientRequest.onWorkspaceConfigForDocumentRequest({ uri });
}

get version(): number {
return this._version;
}

registerConfigurationFile(path: string): void {
async registerConfigurationFile(path: string): Promise<void> {
log('registerConfigurationFile:', path);
this.configsToImport.add(path);
this.importedSettings.clear();
this.resetSettings();
await this.resetSettings();
}

private async fetchUriSettings(uri: string): Promise<CSpellUserSettings> {
Expand Down Expand Up @@ -394,15 +401,17 @@ function resolveConfigImports(config: CSpellUserSettings, folderUri: string): CS
const importAbsPath = imports.map((file) => resolvePath(uriFsPath, file));
log(`resolvingConfigImports: [\n${imports.join('\n')}]`);
log(`resolvingConfigImports ABS: [\n${importAbsPath.join('\n')}]`);
const { import: _import, ...result } = importAbsPath.length ? mergeSettings(readSettingsFiles([...importAbsPath]), config) : config;
const { import: _import, ...result } = importAbsPath.length
? mergeSettings({}, ...readSettingsFiles([...importAbsPath]), config)
: config;
return result;
}

function readSettingsFiles(paths: string[]) {
// log('readSettingsFiles:', paths);
const existingPaths = paths.filter((filename) => exists(filename));
log('readSettingsFiles:', existingPaths);
return existingPaths.length ? cspellReadSettingsFiles(existingPaths) : {};
return existingPaths.map((file) => cspellReadSettingsFile(file));
}

function exists(file: string): boolean {
Expand Down
27 changes: 14 additions & 13 deletions packages/_server/src/config/documentSettings.test.mts
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,11 @@ import { getDefaultSettings } from 'cspell-lib';
import * as os from 'os';
import * as Path from 'path';
import { beforeEach, describe, expect, test, vi } from 'vitest';
import type { Connection, WorkspaceFolder } from 'vscode-languageserver/node.js';
import { URI as Uri } from 'vscode-uri';

import { createMockServerSideApi } from '../test/test.api.js';
import { extendExpect } from '../test/test.matchers.js';
import type { Connection, WorkspaceFolder } from '../vscodeLanguageServer/index.cjs';
import type { CSpellUserSettings } from './cspellConfig/index.mjs';
import type { ExcludedByMatch } from './documentSettings.mjs';
import {
Expand Down Expand Up @@ -82,10 +83,10 @@ describe('Validate DocumentSettings', () => {
mockGetWorkspaceFolders.mockClear();
});

test('version', () => {
test('version', async () => {
const docSettings = newDocumentSettings();
expect(docSettings.version).toEqual(0);
docSettings.resetSettings();
await docSettings.resetSettings();
expect(docSettings.version).toEqual(1);
});

Expand All @@ -111,14 +112,14 @@ describe('Validate DocumentSettings', () => {
expect(folders).toBe(mockFolders);
});

test('tests register config path', () => {
test('tests register config path', async () => {
const mockFolders: WorkspaceFolder[] = [workspaceFolderServer];
mockGetWorkspaceFolders.mockReturnValue(Promise.resolve(mockFolders));

const docSettings = newDocumentSettings();
const configFile = Path.join(pathSampleSourceFiles, 'cSpell.json');
expect(docSettings.version).toEqual(0);
docSettings.registerConfigurationFile(configFile);
await docSettings.registerConfigurationFile(configFile);
expect(docSettings.version).toEqual(1);
expect(docSettings.configsToImport).toContain(configFile);
});
Expand All @@ -129,7 +130,7 @@ describe('Validate DocumentSettings', () => {
mockGetConfiguration.mockReturnValue(Promise.resolve([cspellConfigInVsCode, {}]));
const docSettings = newDocumentSettings();
const configFile = Path.join(pathSampleSourceFiles, 'cspell-ext.json');
docSettings.registerConfigurationFile(configFile);
await docSettings.registerConfigurationFile(configFile);

const settings = await docSettings.getSettings({ uri: Uri.file(__filename).toString() });
expect(settings.enabled).toBeUndefined();
Expand All @@ -144,7 +145,7 @@ describe('Validate DocumentSettings', () => {
);
const docSettings = newDocumentSettings();
const configFile = Path.join(pathSampleSourceFiles, 'cspell-ext.json');
docSettings.registerConfigurationFile(configFile);
await docSettings.registerConfigurationFile(configFile);

const settings = await docSettings.getSettings({ uri: Uri.file(__filename).toString() });
expect(settings.workspaceRootPath?.toLowerCase()).toBe(pathWorkspaceClient.toLowerCase());
Expand All @@ -156,7 +157,7 @@ describe('Validate DocumentSettings', () => {
mockGetConfiguration.mockReturnValue(Promise.resolve([{}, {}]));
const docSettings = newDocumentSettings();
const configFile = Path.join(pathSampleSourceFiles, 'cSpell.json');
docSettings.registerConfigurationFile(configFile);
await docSettings.registerConfigurationFile(configFile);

const result = await docSettings.isExcluded(Uri.file(__filename).toString());
expect(result).toBe(false);
Expand All @@ -170,7 +171,7 @@ describe('Validate DocumentSettings', () => {
);
const docSettings = newDocumentSettings();
const configFile = Path.join(pathSampleSourceFiles, 'cSpell.json');
docSettings.registerConfigurationFile(configFile);
await docSettings.registerConfigurationFile(configFile);

const settings = await docSettings.getSettings({ uri: Uri.file(__filename).toString() });
expect(settings.enabledLanguageIds).not.toContain('typescript');
Expand Down Expand Up @@ -214,7 +215,7 @@ describe('Validate DocumentSettings', () => {
mockGetConfiguration.mockReturnValue(Promise.resolve([{}, {}]));
const docSettings = newDocumentSettings();
const configFile = Path.join(pathSampleSourceFiles, 'cSpell.json');
docSettings.registerConfigurationFile(configFile);
await docSettings.registerConfigurationFile(configFile);

const result = await docSettings.calcExcludedBy(Uri.file(__filename).toString());
expect(result).toHaveLength(0);
Expand Down Expand Up @@ -305,7 +306,7 @@ describe('Validate DocumentSettings', () => {
mockGetConfiguration.mockReturnValue(Promise.resolve([{}, {}]));
const docSettings = newDocumentSettings();
const configFile = Path.join(pathSampleSourceFiles, 'cspell-exclude-tests.json');
docSettings.registerConfigurationFile(configFile);
await docSettings.registerConfigurationFile(configFile);

const uri = Uri.file(Path.resolve(pathWorkspaceRoot, filename)).toString();
const result = await docSettings.calcExcludedBy(uri);
Expand All @@ -328,7 +329,7 @@ describe('Validate DocumentSettings', () => {
mockGetWorkspaceFolders.mockReturnValue(Promise.resolve(mockFolders));
mockGetConfiguration.mockReturnValue(Promise.resolve([{}, {}]));
const docSettings = newDocumentSettings();
docSettings.registerConfigurationFile(Path.join(pathWorkspaceRoot, 'cSpell.json'));
await docSettings.registerConfigurationFile(Path.join(pathWorkspaceRoot, 'cSpell.json'));

const uri = Uri.file(Path.resolve(pathWorkspaceRoot, filename));
const result = await docSettings.isGitIgnored(uri);
Expand Down Expand Up @@ -431,7 +432,7 @@ describe('Validate DocumentSettings', () => {
});

function newDocumentSettings(defaultSettings: CSpellUserSettings = {}) {
return new DocumentSettings({} as Connection, defaultSettings);
return new DocumentSettings({} as Connection, createMockServerSideApi(), defaultSettings);
}
});

Expand Down
2 changes: 1 addition & 1 deletion packages/_server/src/config/vscode.config.mts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { log } from '@internal/common-utils/log.js';
import type { ConfigurationItem, Connection } from 'vscode-languageserver/node.js';

import { isDefined } from '../utils/index.mjs';
import type { ConfigurationItem, Connection } from '../vscodeLanguageServer/index.cjs';

export interface TextDocumentUri {
uri: string;
Expand Down
Loading

0 comments on commit 73b7c03

Please sign in to comment.