Skip to content

Commit

Permalink
fix: respect enable configuration more accurately
Browse files Browse the repository at this point in the history
  • Loading branch information
spence-s committed Feb 10, 2024
1 parent b103d00 commit 88edd52
Show file tree
Hide file tree
Showing 7 changed files with 73 additions and 27 deletions.
4 changes: 3 additions & 1 deletion client/extension.ts
Expand Up @@ -87,7 +87,9 @@ export async function activate(context: ExtensionContext) {

logger.debug('[client] onDidChangeActiveTextEditor', textDocument?.uri.fsPath);

const isEnabled = workspace.getConfiguration('xo').get<boolean>('enable', true);
const isEnabled = workspace
.getConfiguration('xo', textDocument)
.get<boolean>('enable', true);

if (!isEnabled) {
logger.debug('[client] onDidChangeActiveTextEditor > XO is not enabled');
Expand Down
15 changes: 7 additions & 8 deletions server/get-document-config.ts
Expand Up @@ -11,22 +11,21 @@ async function getDocumentConfig(
this: LintServer,
document: TextDocumentIdentifier
): Promise<XoConfig> {
const folderUri = path.dirname(document.uri);

if (this.configurationCache.has(folderUri)) {
const config: XoConfig = this.configurationCache.get(folderUri)!;
if (this.configurationCache.has(document.uri)) {
const config: XoConfig = this.configurationCache.get(document.uri)!;

if (config !== undefined) return config;

return {};
}

const config: XoConfig = (await this.connection.workspace.getConfiguration({
scopeUri: folderUri,
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
const config: XoConfig = await this.connection.workspace.getConfiguration({
scopeUri: document.uri,
section: 'xo'
})) as XoConfig;
});

this.configurationCache.set(folderUri, config);
this.configurationCache.set(document.uri, config);

return config;
}
Expand Down
4 changes: 3 additions & 1 deletion server/lint-document.ts
Expand Up @@ -17,7 +17,9 @@ export async function lintDocument(this: LintServer, document: TextDocument): Pr

if (document.version !== currentDocument.version) return;

const {overrideSeverity} = await this.getDocumentConfig(document);
const {overrideSeverity, enable} = await this.getDocumentConfig(document);

if (!enable) return;

const {results, rulesMeta} = await this.getLintResults(document);

Expand Down
28 changes: 16 additions & 12 deletions server/server.ts
Expand Up @@ -57,7 +57,6 @@ class LintServer {
documentFixCache: Map<string, Map<string, XoFix>>;
hasShownResolutionError: boolean;
hasReceivedShutdownRequest?: boolean;
debounceTime = 0;

constructor({isTest}: {isTest?: boolean} = {}) {
/**
Expand Down Expand Up @@ -157,7 +156,7 @@ class LintServer {
this.xoCache = new Map();

/**
* A mapping of folderPaths to configuration options
* A mapping of document uri strings to configuration options
*/
this.configurationCache = new Map();

Expand Down Expand Up @@ -217,13 +216,6 @@ class LintServer {
* Handle connection.onDidChangeConfiguration
*/
async handleDidChangeConfiguration(params: ChangeConfigurationParams) {
if (
Number.isInteger(Number(params?.settings?.xo?.debounce)) &&
Number(params?.settings?.xo?.debounce) !== this.debounceTime
) {
this.debounceTime = params.settings.xo.debounce ?? 0;
}

// recache each folder config
this.configurationCache.clear();
return this.lintDocuments(this.documents.all());
Expand Down Expand Up @@ -286,7 +278,7 @@ class LintServer {

const config = await this.getDocumentConfig(params.textDocument);

if (config === undefined || !config?.format?.enable) {
if (config === undefined || !config?.format?.enable || !config.enable) {
resolve([]);
return;
}
Expand Down Expand Up @@ -345,7 +337,18 @@ class LintServer {
}

const document = this.documents.get(params.textDocument.uri);
if (!document) return;

if (!document) {
resolve(undefined);
return;
}

const config = await this.getDocumentConfig(document);

if (config === undefined || !config.enable) {
resolve(undefined);
return;
}

const codeActions: CodeAction[] = [];
if (context.only?.includes(CodeActionKind.SourceFixAll)) {
Expand Down Expand Up @@ -397,8 +400,9 @@ class LintServer {
return;
}

const config = await this.getDocumentConfig(event.document);
const {default: debounce} = await import('p-debounce');
await debounce(this.lintDocument, this.debounceTime)(event.document);
await debounce(this.lintDocument, config.debounce ?? 0)(event.document);
} catch (error: unknown) {
if (error instanceof Error) this.logError(error);
}
Expand Down
7 changes: 6 additions & 1 deletion test/index.ts
@@ -1,9 +1,14 @@
/* eslint-disable import/no-unassigned-import */
// since globs are not fully supported in node v18 and v20 we import the files manually here

import process from 'node:process';
// TODO: remove this file once node v21 is LTS
import './server.test.js';
import './lsp/document-sync.test.js';
import './lsp/initialization.test.js';
import './lsp/code-actions.test.js';
import './code-actions-builder.test.js';

process.on('unhandledRejection', (error) => {
console.error(error);
process.exit(1);
});
20 changes: 17 additions & 3 deletions test/lsp/code-actions.test.ts
Expand Up @@ -9,6 +9,7 @@ import {
type CodeActionParams,
type Range,
type TextDocumentIdentifier
// type Connection
} from 'vscode-languageserver';
import Server from '../../server/server.js';
import {
Expand All @@ -27,6 +28,7 @@ describe('Server code actions', async () => {
log: Mock<Server['log']>;
getDocumentFormatting: Mock<Server['getDocumentFormatting']>;
documents: Map<string, TextDocument> & {all?: typeof Map.prototype.values};
getDocumentConfig: Mock<Server['getDocumentConfig']>;
};

test.beforeEach((t) => {
Expand All @@ -42,6 +44,7 @@ describe('Server code actions', async () => {
server.documents = documents;
mock.method(server, 'log', noop);
mock.method(server, 'getDocumentFormatting');
mock.method(server, 'getDocumentConfig', async () => ({enable: true}));
});

test.afterEach(async () => {
Expand All @@ -64,6 +67,7 @@ describe('Server code actions', async () => {
context: {diagnostics: [Diagnostic.create(range, 'test message', 1, 'test', 'test')]}
};
const codeActions = await server.handleCodeActionRequest(mockCodeActionParams);
assert.equal(server.getDocumentConfig.mock.callCount(), 1);
assert.deepEqual(codeActions, []);
});

Expand All @@ -79,10 +83,11 @@ describe('Server code actions', async () => {
}
};
const codeActions = await server.handleCodeActionRequest(mockCodeActionParams);

assert.equal(server.getDocumentConfig.mock.callCount(), 1);
assert.deepEqual(codeActions, [
{title: 'Fix all XO auto-fixable problems', kind: 'source.fixAll', edit: {changes: {uri: []}}}
]);
assert.equal(server.getDocumentConfig.mock.callCount(), 1);
assert.equal(server.getDocumentFormatting.mock.callCount(), 1);
assert.deepEqual(server.getDocumentFormatting.mock.calls[0].arguments, ['uri']);
});
Expand All @@ -99,7 +104,7 @@ describe('Server code actions', async () => {
}
};
const codeActions = await server.handleCodeActionRequest(mockCodeActionParams);

assert.equal(server.getDocumentConfig.mock.callCount(), 1);
assert.deepEqual(codeActions, []);
assert.equal(server.getDocumentFormatting.mock.callCount(), 0);
});
Expand All @@ -115,7 +120,7 @@ describe('Server code actions', async () => {
}
};
const codeActions = await server.handleCodeActionRequest(mockCodeActionParams);

assert.equal(server.getDocumentConfig.mock.callCount(), 1);
assert.deepEqual(codeActions, []);
assert.equal(server.getDocumentFormatting.mock.callCount(), 0);
});
Expand All @@ -141,4 +146,13 @@ describe('Server code actions', async () => {
getIgnoreFileCodeAction()
]);
});

await test('codeAction with only quickfix produces quickfix code actions', async (t) => {
const params = getCodeActionParams();
params.context.only = [CodeActionKind.QuickFix];
mock.method(server, 'getDocumentConfig', async () => ({enable: false}));
const codeActions = await server.handleCodeActionRequest(params);

assert.deepStrictEqual(codeActions, undefined);
});
});
22 changes: 21 additions & 1 deletion test/lsp/document-sync.test.ts
Expand Up @@ -8,10 +8,14 @@ import Server from '../../server/server.js';
const noop = () => {};

describe('Server documents syncing', () => {
let server: Omit<Server, 'lintDocument' | 'documents' | 'connection' | 'log'> & {
let server: Omit<
Server,
'lintDocument' | 'documents' | 'connection' | 'log' | 'getDocumentConfig'
> & {
lintDocument: Mock<Server['lintDocument']>;
log: Mock<Server['log']>;
documents: Map<string, TextDocument> & {all?: typeof Map.prototype.values};
getDocumentConfig: Mock<Server['getDocumentConfig']>;
connection: Omit<Connection, 'sendDiagnostics'> & {
sendDiagnostics: Mock<Connection['sendDiagnostics']>;
};
Expand All @@ -30,6 +34,7 @@ describe('Server documents syncing', () => {
server.documents = documents;
mock.method(server, 'log', noop);
mock.method(server, 'lintDocument', noop);
mock.method(server, 'getDocumentConfig', async () => ({enable: true}));
mock.method(server.connection, 'sendDiagnostics', noop);
});

Expand All @@ -53,6 +58,7 @@ describe('Server documents syncing', () => {
resolve(undefined);
});
});
assert.equal(server.getDocumentConfig.mock.callCount(), 1);
assert.equal(server.lintDocument.mock.callCount(), 1);
});

Expand Down Expand Up @@ -92,4 +98,18 @@ describe('Server documents syncing', () => {
diagnostics: []
});
});

test('Server.handleDocumentOnDidChangeContent does not send diagnostics if xo is disabled', async (t) => {
mock.method(server, 'getDocumentConfig', async () => ({enable: false}));
server.handleDocumentsOnDidChangeContent({
document: TextDocument.create('uri', 'javascript', 1, 'content')
});
await new Promise((resolve) => {
server.queue.once('end', () => {
resolve(undefined);
});
});
assert.equal(server.getDocumentConfig.mock.callCount(), 1);
assert.equal(server.connection.sendDiagnostics.mock.callCount(), 0);
});
});

0 comments on commit 88edd52

Please sign in to comment.