Skip to content

Commit 88edd52

Browse files
committed
fix: respect enable configuration more accurately
1 parent b103d00 commit 88edd52

7 files changed

+73
-27
lines changed

client/extension.ts

+3-1
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,9 @@ export async function activate(context: ExtensionContext) {
8787

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

90-
const isEnabled = workspace.getConfiguration('xo').get<boolean>('enable', true);
90+
const isEnabled = workspace
91+
.getConfiguration('xo', textDocument)
92+
.get<boolean>('enable', true);
9193

9294
if (!isEnabled) {
9395
logger.debug('[client] onDidChangeActiveTextEditor > XO is not enabled');

server/get-document-config.ts

+7-8
Original file line numberDiff line numberDiff line change
@@ -11,22 +11,21 @@ async function getDocumentConfig(
1111
this: LintServer,
1212
document: TextDocumentIdentifier
1313
): Promise<XoConfig> {
14-
const folderUri = path.dirname(document.uri);
15-
16-
if (this.configurationCache.has(folderUri)) {
17-
const config: XoConfig = this.configurationCache.get(folderUri)!;
14+
if (this.configurationCache.has(document.uri)) {
15+
const config: XoConfig = this.configurationCache.get(document.uri)!;
1816

1917
if (config !== undefined) return config;
2018

2119
return {};
2220
}
2321

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

29-
this.configurationCache.set(folderUri, config);
28+
this.configurationCache.set(document.uri, config);
3029

3130
return config;
3231
}

server/lint-document.ts

+3-1
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,9 @@ export async function lintDocument(this: LintServer, document: TextDocument): Pr
1717

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

20-
const {overrideSeverity} = await this.getDocumentConfig(document);
20+
const {overrideSeverity, enable} = await this.getDocumentConfig(document);
21+
22+
if (!enable) return;
2123

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

server/server.ts

+16-12
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,6 @@ class LintServer {
5757
documentFixCache: Map<string, Map<string, XoFix>>;
5858
hasShownResolutionError: boolean;
5959
hasReceivedShutdownRequest?: boolean;
60-
debounceTime = 0;
6160

6261
constructor({isTest}: {isTest?: boolean} = {}) {
6362
/**
@@ -157,7 +156,7 @@ class LintServer {
157156
this.xoCache = new Map();
158157

159158
/**
160-
* A mapping of folderPaths to configuration options
159+
* A mapping of document uri strings to configuration options
161160
*/
162161
this.configurationCache = new Map();
163162

@@ -217,13 +216,6 @@ class LintServer {
217216
* Handle connection.onDidChangeConfiguration
218217
*/
219218
async handleDidChangeConfiguration(params: ChangeConfigurationParams) {
220-
if (
221-
Number.isInteger(Number(params?.settings?.xo?.debounce)) &&
222-
Number(params?.settings?.xo?.debounce) !== this.debounceTime
223-
) {
224-
this.debounceTime = params.settings.xo.debounce ?? 0;
225-
}
226-
227219
// recache each folder config
228220
this.configurationCache.clear();
229221
return this.lintDocuments(this.documents.all());
@@ -286,7 +278,7 @@ class LintServer {
286278

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

289-
if (config === undefined || !config?.format?.enable) {
281+
if (config === undefined || !config?.format?.enable || !config.enable) {
290282
resolve([]);
291283
return;
292284
}
@@ -345,7 +337,18 @@ class LintServer {
345337
}
346338

347339
const document = this.documents.get(params.textDocument.uri);
348-
if (!document) return;
340+
341+
if (!document) {
342+
resolve(undefined);
343+
return;
344+
}
345+
346+
const config = await this.getDocumentConfig(document);
347+
348+
if (config === undefined || !config.enable) {
349+
resolve(undefined);
350+
return;
351+
}
349352

350353
const codeActions: CodeAction[] = [];
351354
if (context.only?.includes(CodeActionKind.SourceFixAll)) {
@@ -397,8 +400,9 @@ class LintServer {
397400
return;
398401
}
399402

403+
const config = await this.getDocumentConfig(event.document);
400404
const {default: debounce} = await import('p-debounce');
401-
await debounce(this.lintDocument, this.debounceTime)(event.document);
405+
await debounce(this.lintDocument, config.debounce ?? 0)(event.document);
402406
} catch (error: unknown) {
403407
if (error instanceof Error) this.logError(error);
404408
}

test/index.ts

+6-1
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,14 @@
11
/* eslint-disable import/no-unassigned-import */
22
// since globs are not fully supported in node v18 and v20 we import the files manually here
3-
3+
import process from 'node:process';
44
// TODO: remove this file once node v21 is LTS
55
import './server.test.js';
66
import './lsp/document-sync.test.js';
77
import './lsp/initialization.test.js';
88
import './lsp/code-actions.test.js';
99
import './code-actions-builder.test.js';
10+
11+
process.on('unhandledRejection', (error) => {
12+
console.error(error);
13+
process.exit(1);
14+
});

test/lsp/code-actions.test.ts

+17-3
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import {
99
type CodeActionParams,
1010
type Range,
1111
type TextDocumentIdentifier
12+
// type Connection
1213
} from 'vscode-languageserver';
1314
import Server from '../../server/server.js';
1415
import {
@@ -27,6 +28,7 @@ describe('Server code actions', async () => {
2728
log: Mock<Server['log']>;
2829
getDocumentFormatting: Mock<Server['getDocumentFormatting']>;
2930
documents: Map<string, TextDocument> & {all?: typeof Map.prototype.values};
31+
getDocumentConfig: Mock<Server['getDocumentConfig']>;
3032
};
3133

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

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

@@ -79,10 +83,11 @@ describe('Server code actions', async () => {
7983
}
8084
};
8185
const codeActions = await server.handleCodeActionRequest(mockCodeActionParams);
82-
86+
assert.equal(server.getDocumentConfig.mock.callCount(), 1);
8387
assert.deepEqual(codeActions, [
8488
{title: 'Fix all XO auto-fixable problems', kind: 'source.fixAll', edit: {changes: {uri: []}}}
8589
]);
90+
assert.equal(server.getDocumentConfig.mock.callCount(), 1);
8691
assert.equal(server.getDocumentFormatting.mock.callCount(), 1);
8792
assert.deepEqual(server.getDocumentFormatting.mock.calls[0].arguments, ['uri']);
8893
});
@@ -99,7 +104,7 @@ describe('Server code actions', async () => {
99104
}
100105
};
101106
const codeActions = await server.handleCodeActionRequest(mockCodeActionParams);
102-
107+
assert.equal(server.getDocumentConfig.mock.callCount(), 1);
103108
assert.deepEqual(codeActions, []);
104109
assert.equal(server.getDocumentFormatting.mock.callCount(), 0);
105110
});
@@ -115,7 +120,7 @@ describe('Server code actions', async () => {
115120
}
116121
};
117122
const codeActions = await server.handleCodeActionRequest(mockCodeActionParams);
118-
123+
assert.equal(server.getDocumentConfig.mock.callCount(), 1);
119124
assert.deepEqual(codeActions, []);
120125
assert.equal(server.getDocumentFormatting.mock.callCount(), 0);
121126
});
@@ -141,4 +146,13 @@ describe('Server code actions', async () => {
141146
getIgnoreFileCodeAction()
142147
]);
143148
});
149+
150+
await test('codeAction with only quickfix produces quickfix code actions', async (t) => {
151+
const params = getCodeActionParams();
152+
params.context.only = [CodeActionKind.QuickFix];
153+
mock.method(server, 'getDocumentConfig', async () => ({enable: false}));
154+
const codeActions = await server.handleCodeActionRequest(params);
155+
156+
assert.deepStrictEqual(codeActions, undefined);
157+
});
144158
});

test/lsp/document-sync.test.ts

+21-1
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,14 @@ import Server from '../../server/server.js';
88
const noop = () => {};
99

1010
describe('Server documents syncing', () => {
11-
let server: Omit<Server, 'lintDocument' | 'documents' | 'connection' | 'log'> & {
11+
let server: Omit<
12+
Server,
13+
'lintDocument' | 'documents' | 'connection' | 'log' | 'getDocumentConfig'
14+
> & {
1215
lintDocument: Mock<Server['lintDocument']>;
1316
log: Mock<Server['log']>;
1417
documents: Map<string, TextDocument> & {all?: typeof Map.prototype.values};
18+
getDocumentConfig: Mock<Server['getDocumentConfig']>;
1519
connection: Omit<Connection, 'sendDiagnostics'> & {
1620
sendDiagnostics: Mock<Connection['sendDiagnostics']>;
1721
};
@@ -30,6 +34,7 @@ describe('Server documents syncing', () => {
3034
server.documents = documents;
3135
mock.method(server, 'log', noop);
3236
mock.method(server, 'lintDocument', noop);
37+
mock.method(server, 'getDocumentConfig', async () => ({enable: true}));
3338
mock.method(server.connection, 'sendDiagnostics', noop);
3439
});
3540

@@ -53,6 +58,7 @@ describe('Server documents syncing', () => {
5358
resolve(undefined);
5459
});
5560
});
61+
assert.equal(server.getDocumentConfig.mock.callCount(), 1);
5662
assert.equal(server.lintDocument.mock.callCount(), 1);
5763
});
5864

@@ -92,4 +98,18 @@ describe('Server documents syncing', () => {
9298
diagnostics: []
9399
});
94100
});
101+
102+
test('Server.handleDocumentOnDidChangeContent does not send diagnostics if xo is disabled', async (t) => {
103+
mock.method(server, 'getDocumentConfig', async () => ({enable: false}));
104+
server.handleDocumentsOnDidChangeContent({
105+
document: TextDocument.create('uri', 'javascript', 1, 'content')
106+
});
107+
await new Promise((resolve) => {
108+
server.queue.once('end', () => {
109+
resolve(undefined);
110+
});
111+
});
112+
assert.equal(server.getDocumentConfig.mock.callCount(), 1);
113+
assert.equal(server.connection.sendDiagnostics.mock.callCount(), 0);
114+
});
95115
});

0 commit comments

Comments
 (0)