diff --git a/.vscode-test.mjs b/.vscode-test.mjs index 3a336df..bda513a 100644 --- a/.vscode-test.mjs +++ b/.vscode-test.mjs @@ -15,6 +15,14 @@ writeFileSync(multiRootWorkspaceFile, JSON.stringify(multiRootWorkspaceConfig, n // Use binaries from node_modules const oxlintBin = path.resolve(import.meta.dirname, "node_modules/.bin/oxlint"); const oxfmtBin = path.resolve(import.meta.dirname, "node_modules/.bin/oxfmt"); +const fakeOxlintBin = path.resolve( + import.meta.dirname, + "tests/fixtures/lsp_servers/fake_oxlint_server.js", +); +const fakeOxfmtBin = path.resolve( + import.meta.dirname, + "tests/fixtures/lsp_servers/fake_oxfmt_server.js", +); const baseTest = { files: "out_test/integration/**/*.spec.js", @@ -47,7 +55,7 @@ const allTestSuites = new Map([ ...baseTest, env: { SINGLE_FOLDER_WORKSPACE: "true", - SERVER_PATH_DEV: oxlintBin, + SERVER_PATH_DEV_OXLINT: oxlintBin, SKIP_FORMATTER_TEST: "true", }, }, @@ -59,7 +67,7 @@ const allTestSuites = new Map([ workspaceFolder: multiRootWorkspaceFile, env: { MULTI_FOLDER_WORKSPACE: "true", - SERVER_PATH_DEV: oxlintBin, + SERVER_PATH_DEV_OXLINT: oxlintBin, SKIP_FORMATTER_TEST: "true", }, }, @@ -73,7 +81,7 @@ const allTestSuites = new Map([ env: { SINGLE_FOLDER_WORKSPACE: "true", OXLINT_JS_PLUGIN: "true", - SERVER_PATH_DEV: oxlintBin, + SERVER_PATH_DEV_OXLINT: oxlintBin, SKIP_FORMATTER_TEST: "true", }, }, @@ -84,11 +92,25 @@ const allTestSuites = new Map([ ...baseTest, env: { SINGLE_FOLDER_WORKSPACE: "true", - SERVER_PATH_DEV: oxfmtBin, + SERVER_PATH_DEV_OXFMT: oxfmtBin, SKIP_LINTER_TEST: "true", }, }, ], + [ + "format-save-path", + { + ...baseTest, + files: "out_test/integration/format_on_save_no_lint.spec.js", + env: { + SINGLE_FOLDER_WORKSPACE: "true", + SERVER_PATH_DEV_OXLINT: fakeOxlintBin, + SERVER_PATH_DEV_OXFMT: fakeOxfmtBin, + FAKE_OXLINT_CODE_ACTION_MS: "5000", + FAKE_OXLINT_DIAGNOSTIC_MS: "5000", + }, + }, + ], ]); export default defineConfig({ diff --git a/client/tools/formatter.ts b/client/tools/formatter.ts index 2b8dfb3..454b42a 100644 --- a/client/tools/formatter.ts +++ b/client/tools/formatter.ts @@ -10,6 +10,7 @@ import { Uri, workspace, } from "vscode"; +import type { CodeActionContext } from "vscode"; import { DocumentFilter, @@ -42,6 +43,20 @@ formatCodeAction.command = { tooltip: "Format the document using the default formatter", }; +export function shouldProvideOxfmtCodeAction(context: CodeActionContext): boolean { + const requestedKind = context.only; + if (requestedKind === undefined) { + return true; + } + + // Avoid participating in broad source-action scans. Oxfmt only owns the + // explicit format source action; format-on-save uses the formatter provider. + return ( + requestedKind.value !== CodeActionKind.Source.value && + formatCodeActionKind.intersects(requestedKind) + ); +} + // This list is not used as-is for implementation to determine whether formatting processing is possible. const supportedExtensions = [ "cjs", @@ -276,8 +291,9 @@ export default class FormatterTool implements ToolInterface { outputChannel: LogOutputChannel, configService: ConfigService, ): Promise { - if (process.env.SERVER_PATH_DEV) { - return { path: process.env.SERVER_PATH_DEV, loader: "native" }; + if (process.env.SERVER_PATH_DEV_OXFMT) { + const path = process.env.SERVER_PATH_DEV_OXFMT; + return { path, loader: path.endsWith(".js") ? "node" : "native" }; } const bin = await configService.getOxfmtServerBinPath(); if (bin) { @@ -315,9 +331,10 @@ export default class FormatterTool implements ToolInterface { // @ts-expect-error DocumentFilter/DocumentSelector is not correctly typed, here it expects a readonly array, which we provide. this.documentSelectors, { - provideCodeActions: (doc) => { + provideCodeActions: (doc, _range, context) => { if ( configService.vsCodeConfig.enableOxfmt === false || + !shouldProvideOxfmtCodeAction(context) || workspace.getConfiguration("editor", doc).get("defaultFormatter") !== "oxc.oxc-vscode" ) { return []; diff --git a/client/tools/linter.ts b/client/tools/linter.ts index a26104b..152e091 100644 --- a/client/tools/linter.ts +++ b/client/tools/linter.ts @@ -1,6 +1,8 @@ import { promises as fsPromises } from "node:fs"; import { + CodeActionKind, + CodeActionTriggerKind, commands, ConfigurationChangeEvent, LogOutputChannel, @@ -8,6 +10,7 @@ import { window, workspace, } from "vscode"; +import type { CodeActionContext, TextDocument } from "vscode"; import { ConfigurationParams, @@ -37,6 +40,99 @@ const enum LspCommands { } const oxlintConfigDefaultFilePattern = `**/{.oxlintrc.json,.oxlintrc.jsonc,oxlint.config.ts}`; +const oxlintFixAllCodeActionKind = CodeActionKind.SourceFixAll.append("oxc"); +type CodeActionsOnSaveSetting = boolean | "always" | "explicit" | "never"; +type CodeActionsOnSave = Record; +type CodeActionsOnSaveConfiguration = CodeActionsOnSave | string[]; + +function isEnabledCodeActionsOnSaveSetting(setting: CodeActionsOnSaveSetting | undefined): boolean { + // VS Code accepts legacy boolean values and current string values here. + return setting === true || setting === "always" || setting === "explicit"; +} + +export function shouldCodeActionsOnSaveRequestOxlint( + codeActionsOnSave: CodeActionsOnSaveConfiguration | undefined, +): boolean { + if (Array.isArray(codeActionsOnSave)) { + return ( + codeActionsOnSave.includes(CodeActionKind.Source.value) || + codeActionsOnSave.includes(CodeActionKind.SourceFixAll.value) || + codeActionsOnSave.includes(oxlintFixAllCodeActionKind.value) + ); + } + + if (codeActionsOnSave === undefined) { + return false; + } + + const oxlintFixAllSetting = codeActionsOnSave[oxlintFixAllCodeActionKind.value]; + if (oxlintFixAllSetting !== undefined) { + // A provider-specific setting is the user's most precise opt-in or opt-out. + return isEnabledCodeActionsOnSaveSetting(oxlintFixAllSetting); + } + + const fixAllSetting = codeActionsOnSave[CodeActionKind.SourceFixAll.value]; + if (fixAllSetting !== undefined) { + // Generic fix-all is more specific than broad `source`, so it controls + // whether source.fixAll.oxc participates unless the oxc key overrides it. + return isEnabledCodeActionsOnSaveSetting(fixAllSetting); + } + + return isEnabledCodeActionsOnSaveSetting(codeActionsOnSave[CodeActionKind.Source.value]); +} + +function shouldRunOxlintCodeActionsOnSave(document: TextDocument): boolean { + const codeActionsOnSave = workspace + .getConfiguration("editor", document) + .get("codeActionsOnSave"); + + return shouldCodeActionsOnSaveRequestOxlint(codeActionsOnSave); +} + +export function shouldRequestOxlintCodeActions( + context: CodeActionContext, + codeActionsOnSaveRequestsOxlint: boolean = false, +): boolean { + const requestedKind = context.only; + if (requestedKind === undefined) { + // Empty automatic probes are used for editor UI discovery, not an explicit + // user command or configured fix-all action. Keep diagnostic-bearing + // automatic requests so VS Code can discover oxlint quick fixes. + return ( + context.triggerKind !== CodeActionTriggerKind.Automatic || context.diagnostics.length > 0 + ); + } + + const requestedKindValue = requestedKind.value; + // `CodeActionKind.intersects` treats `source.fixAll` as intersecting with + // provider-owned subkinds such as `source.fixAll.biome`. Save participants + // wait for those requests, so only route exact oxlint-owned source actions. + const requestsOxlintKind = + requestedKindValue === CodeActionKind.Source.value || + requestedKindValue === CodeActionKind.QuickFix.value || + requestedKindValue.startsWith(`${CodeActionKind.QuickFix.value}.`) || + requestedKindValue === CodeActionKind.SourceFixAll.value || + requestedKindValue === oxlintFixAllCodeActionKind.value; + + if (!requestsOxlintKind) { + return false; + } + + const isAutomaticSaveRunnableSourceAction = + context.triggerKind === CodeActionTriggerKind.Automatic && + (requestedKindValue === CodeActionKind.Source.value || + requestedKindValue === CodeActionKind.SourceFixAll.value || + requestedKindValue === oxlintFixAllCodeActionKind.value); + + // Automatic source-action requests are save-runnable. Oxlint should only + // participate in those when save settings enable its fix-all action; + // otherwise the extension would ignore explicit provider opt-outs. + if (isAutomaticSaveRunnableSourceAction && !codeActionsOnSaveRequestsOxlint) { + return false; + } + + return true; +} export default class LinterTool implements ToolInterface { // Global flag to check if the user allows us to start the server. @@ -56,8 +152,9 @@ export default class LinterTool implements ToolInterface { outputChannel: LogOutputChannel, configService: ConfigService, ): Promise { - if (process.env.SERVER_PATH_DEV) { - return { path: process.env.SERVER_PATH_DEV, loader: "native" }; + if (process.env.SERVER_PATH_DEV_OXLINT) { + const path = process.env.SERVER_PATH_DEV_OXLINT; + return { path, loader: path.endsWith(".js") ? "node" : "native" }; } const bin = await configService.getOxlintServerBinPath(); if (bin) { @@ -166,9 +263,33 @@ export default class LinterTool implements ToolInterface { onChange: true, onSave: true, onTabs: false, - filter: (document, mode) => !configService.shouldRequestDiagnostics(document.uri, mode), + filter: (document, mode) => { + if (!configService.shouldRequestDiagnostics(document.uri, mode)) { + return true; + } + + return false; + }, }, middleware: { + provideCodeActions: (document, range, context, token, next) => { + const needsCodeActionsOnSaveConfig = + context.triggerKind === CodeActionTriggerKind.Automatic && + (context.only?.value === CodeActionKind.Source.value || + context.only?.value === CodeActionKind.SourceFixAll.value || + context.only?.value === oxlintFixAllCodeActionKind.value); + + if ( + !shouldRequestOxlintCodeActions( + context, + needsCodeActionsOnSaveConfig && shouldRunOxlintCodeActionsOnSave(document), + ) + ) { + return []; + } + + return next(document, range, context, token); + }, handleDiagnostics: (uri, diagnostics, next) => { for (const diag of diagnostics) { // https://github.com/oxc-project/oxc/issues/12404 diff --git a/package.json b/package.json index d3489d8..2e27c8c 100644 --- a/package.json +++ b/package.json @@ -35,12 +35,13 @@ "package": "vsce package --no-dependencies -o oxc_language_server.vsix", "package:pre-release": "vsce package --no-dependencies --pre-release -o oxc_language_server.vsix", "install-extension": "code --install-extension oxc_language_server.vsix --force", - "test": "cross-env TEST=true pnpm run compile && vscode-test", - "test:unit": "cross-env TEST=true pnpm run compile && cross-env TEST_SUITE=unit vscode-test", - "test:oxlint": "cross-env TEST=true pnpm run compile && cross-env TEST_SUITE=oxlint-lsp vscode-test", - "test:oxlint-js": "cross-env TEST=true pnpm run compile && cross-env TEST_SUITE=oxlint-js vscode-test", - "test:oxlint-multi-root": "cross-env TEST=true pnpm run compile && cross-env TEST_SUITE=oxlint-lsp-multi-root vscode-test", - "test:oxfmt": "cross-env TEST=true pnpm run compile && cross-env TEST_SUITE=oxfmt-lsp vscode-test", + "test": "pnpm run compile && cross-env TEST=true pnpm run compile && vscode-test", + "test:unit": "pnpm run compile && cross-env TEST=true pnpm run compile && cross-env TEST_SUITE=unit vscode-test", + "test:oxlint": "pnpm run compile && cross-env TEST=true pnpm run compile && cross-env TEST_SUITE=oxlint-lsp vscode-test", + "test:oxlint-js": "pnpm run compile && cross-env TEST=true pnpm run compile && cross-env TEST_SUITE=oxlint-js vscode-test", + "test:oxlint-multi-root": "pnpm run compile && cross-env TEST=true pnpm run compile && cross-env TEST_SUITE=oxlint-lsp-multi-root vscode-test", + "test:oxfmt": "pnpm run compile && cross-env TEST=true pnpm run compile && cross-env TEST_SUITE=oxfmt-lsp vscode-test", + "test:format-save": "pnpm run compile && cross-env TEST=true pnpm run compile && cross-env TEST_SUITE=format-save-path vscode-test", "lint": "oxlint -c oxlint.config.json", "fmt": "oxfmt --write -c oxfmt.config.json", "fmt:check": "oxfmt --check -c oxfmt.config.json", diff --git a/tests/fixtures/lsp_servers/fake_lsp_common.js b/tests/fixtures/lsp_servers/fake_lsp_common.js new file mode 100644 index 0000000..22aa7b8 --- /dev/null +++ b/tests/fixtures/lsp_servers/fake_lsp_common.js @@ -0,0 +1,100 @@ +const fs = require("node:fs"); +const path = require("node:path"); + +function appendLog(logPath, message) { + if (!logPath) { + return; + } + + fs.mkdirSync(path.dirname(logPath), { recursive: true }); + fs.appendFileSync(logPath, `${message}\n`); +} + +function createLspServer({ logPath, capabilities, onRequest, onNotification }) { + let buffer = Buffer.alloc(0); + + function send(message) { + const json = JSON.stringify(message); + const contentLength = Buffer.byteLength(json, "utf8"); + process.stdout.write(`Content-Length: ${contentLength}\r\n\r\n${json}`); + } + + async function handleMessage(message) { + if (message.method === "initialize") { + send({ + id: message.id, + jsonrpc: "2.0", + result: { + capabilities, + serverInfo: { + name: "oxc-vscode-fake-lsp", + version: "0.0.0", + }, + }, + }); + return; + } + + if (message.method === "shutdown") { + send({ id: message.id, jsonrpc: "2.0", result: null }); + return; + } + + if (message.method === "exit") { + process.exit(0); + } + + if (Object.hasOwn(message, "id")) { + const handler = onRequest[message.method]; + const result = handler ? await handler(message.params) : null; + send({ id: message.id, jsonrpc: "2.0", result }); + return; + } + + const notificationHandler = onNotification[message.method]; + if (notificationHandler) { + notificationHandler(message.params); + } + } + + function parseMessages() { + while (true) { + const headerEnd = buffer.indexOf("\r\n\r\n"); + if (headerEnd === -1) { + return; + } + + const header = buffer.subarray(0, headerEnd).toString("ascii"); + const match = header.match(/Content-Length: (\d+)/i); + if (!match) { + throw new Error(`Invalid LSP header: ${header}`); + } + + const contentLength = Number(match[1]); + const messageStart = headerEnd + 4; + const messageEnd = messageStart + contentLength; + if (buffer.length < messageEnd) { + return; + } + + const rawMessage = buffer.subarray(messageStart, messageEnd).toString("utf8"); + buffer = buffer.subarray(messageEnd); + void handleMessage(JSON.parse(rawMessage)); + } + } + + process.stdin.on("data", (chunk) => { + buffer = Buffer.concat([buffer, chunk]); + parseMessages(); + }); + + process.on("uncaughtException", (error) => { + appendLog(logPath, `error:${error instanceof Error ? error.message : String(error)}`); + process.exit(1); + }); +} + +module.exports = { + appendLog, + createLspServer, +}; diff --git a/tests/fixtures/lsp_servers/fake_oxfmt_server.js b/tests/fixtures/lsp_servers/fake_oxfmt_server.js new file mode 100644 index 0000000..64c3e74 --- /dev/null +++ b/tests/fixtures/lsp_servers/fake_oxfmt_server.js @@ -0,0 +1,48 @@ +const path = require("node:path"); + +const { appendLog, createLspServer } = require("./fake_lsp_common"); + +const logPath = path.resolve(__dirname, "../../test_workspace/.fake-lsp-logs/oxfmt.log"); +const documents = new Map(); + +function fullRange(text) { + const lines = text.split(/\r\n|\r|\n/); + const lastLine = lines[lines.length - 1] ?? ""; + + return { + start: { line: 0, character: 0 }, + end: { line: lines.length - 1, character: lastLine.length }, + }; +} + +createLspServer({ + logPath, + capabilities: { + documentFormattingProvider: true, + textDocumentSync: 1, + }, + onRequest: { + "textDocument/formatting": (params) => { + appendLog(logPath, `textDocument/formatting uri=${params.textDocument.uri}`); + const text = documents.get(params.textDocument.uri) ?? ""; + + return [ + { + range: fullRange(text), + newText: "class X {\n foo() {\n return 42;\n }\n}\n", + }, + ]; + }, + }, + onNotification: { + "textDocument/didOpen": (params) => { + documents.set(params.textDocument.uri, params.textDocument.text); + }, + "textDocument/didChange": (params) => { + const text = params.contentChanges[0]?.text; + if (text !== undefined) { + documents.set(params.textDocument.uri, text); + } + }, + }, +}); diff --git a/tests/fixtures/lsp_servers/fake_oxlint_server.js b/tests/fixtures/lsp_servers/fake_oxlint_server.js new file mode 100644 index 0000000..b65c556 --- /dev/null +++ b/tests/fixtures/lsp_servers/fake_oxlint_server.js @@ -0,0 +1,54 @@ +const path = require("node:path"); +const { setTimeout: delay } = require("node:timers/promises"); + +const { appendLog, createLspServer } = require("./fake_lsp_common"); + +const logPath = path.resolve(__dirname, "../../test_workspace/.fake-lsp-logs/oxlint.log"); +const defaultResponseMs = Number(process.env.FAKE_OXLINT_RESPONSE_MS ?? 5_000); +const codeActionResponseMs = Number(process.env.FAKE_OXLINT_CODE_ACTION_MS ?? defaultResponseMs); +const diagnosticResponseMs = Number(process.env.FAKE_OXLINT_DIAGNOSTIC_MS ?? defaultResponseMs); + +createLspServer({ + logPath, + capabilities: { + codeActionProvider: { + codeActionKinds: ["quickfix", "source.fixAll", "source.fixAll.oxc"], + }, + diagnosticProvider: { + interFileDependencies: false, + workspaceDiagnostics: false, + }, + textDocumentSync: 1, + }, + onRequest: { + "textDocument/codeAction": async (params) => { + appendLog( + logPath, + `textDocument/codeAction only=${JSON.stringify(params.context?.only ?? null)} trigger=${String(params.context?.triggerKind ?? null)}`, + ); + await delay(codeActionResponseMs); + appendLog(logPath, "codeAction:end"); + return []; + }, + "textDocument/diagnostic": async () => { + appendLog(logPath, "textDocument/diagnostic"); + await delay(diagnosticResponseMs); + appendLog(logPath, "diagnostic:end"); + return { + kind: "full", + items: [], + }; + }, + }, + onNotification: { + "textDocument/didOpen": () => { + appendLog(logPath, "textDocument/didOpen"); + }, + "textDocument/didChange": () => { + appendLog(logPath, "textDocument/didChange"); + }, + "textDocument/didSave": () => { + appendLog(logPath, "textDocument/didSave"); + }, + }, +}); diff --git a/tests/integration/commands.spec.ts b/tests/integration/commands.spec.ts index 036c479..8a76467 100644 --- a/tests/integration/commands.spec.ts +++ b/tests/integration/commands.spec.ts @@ -45,7 +45,7 @@ suite("commands", () => { if ( process.env.SKIP_FORMATTER_TEST !== "true" && - !process.env.SERVER_PATH_DEV?.includes("oxc_language_server") + !process.env.SERVER_PATH_DEV_OXFMT?.includes("oxc_language_server") ) { expectedCommands.push("oxc.restartServerFormatter", "oxc.toggleEnableFormatter"); } diff --git a/tests/integration/format_on_save_no_lint.spec.ts b/tests/integration/format_on_save_no_lint.spec.ts new file mode 100644 index 0000000..65be43c --- /dev/null +++ b/tests/integration/format_on_save_no_lint.spec.ts @@ -0,0 +1,304 @@ +import { strictEqual } from "assert"; +import { dirname, join } from "node:path"; +import { mkdir, readFile, writeFile } from "node:fs/promises"; +import { + CodeActionKind, + languages, + Position, + Range, + Uri, + window, + workspace, + WorkspaceEdit, +} from "vscode"; +import { + activateExtension, + deleteFixtures, + fixturesWorkspaceUri, + sleep, + WORKSPACE_DIR, +} from "../test-helpers"; + +const LE = process.platform === "win32" ? "\r\n" : "\n"; +const slowLintResponseMs = 5_000; +const settingsApplyTimeoutMs = 3_000; + +const oxlintLogPath = join(WORKSPACE_DIR.fsPath, ".fake-lsp-logs", "oxlint.log"); +const oxfmtLogPath = join(WORKSPACE_DIR.fsPath, ".fake-lsp-logs", "oxfmt.log"); +const workspaceSettingsPath = join(WORKSPACE_DIR.fsPath, ".vscode", "settings.json"); +const isFormatSavePathSuite = + process.env.SERVER_PATH_DEV_OXLINT?.includes("fake_oxlint_server.js") === true && + process.env.SERVER_PATH_DEV_OXFMT?.includes("fake_oxfmt_server.js") === true; + +async function resetLog(path: string): Promise { + await mkdir(dirname(path), { recursive: true }); + await writeFile(path, ""); +} + +async function readLog(path: string): Promise { + return readFile(path, "utf8").catch(() => ""); +} + +function isExpectedSetting(key: string, expectedValue: unknown): boolean { + const actualValue = workspace.getConfiguration().get(key); + return JSON.stringify(actualValue) === JSON.stringify(expectedValue); +} + +async function writeWorkspaceSettings(settings: Record): Promise { + await mkdir(dirname(workspaceSettingsPath), { recursive: true }); + await writeFile(workspaceSettingsPath, `${JSON.stringify(settings, null, 2)}\n`); + + // VS Code observes settings-file edits asynchronously; wait for the effective + // config so save assertions do not race the file watcher. + const startedAt = Date.now(); + while ( + !Object.entries(settings).every(([key, value]) => isExpectedSetting(key, value)) && + Date.now() - startedAt < settingsApplyTimeoutMs + ) { + // oxlint-disable-next-line eslint/no-await-in-loop -- Polling VS Code config is intentional. + await sleep(50); + } + + for (const [key, value] of Object.entries(settings)) { + strictEqual(isExpectedSetting(key, value), true, `expected workspace setting ${key} to apply`); + } +} + +async function writeFormatOnlySettings(): Promise { + await writeWorkspaceSettings({ + "editor.codeActionsOnSave": {}, + "editor.defaultFormatter": "oxc.oxc-vscode", + "editor.formatOnSave": true, + "editor.formatOnSaveMode": "file", + }); +} + +async function createFormattingFile(fileName: string): Promise { + const fixturesDir = Uri.joinPath(fixturesWorkspaceUri(), "fixtures"); + await mkdir(fixturesDir.fsPath, { recursive: true }); + const fileUri = Uri.joinPath(fixturesDir, fileName); + await writeFile(fileUri.fsPath, "class X { foo() { return 42; } }\n"); + return fileUri; +} + +suite("format on save without lint code actions", () => { + if (!isFormatSavePathSuite) { + return; + } + + suiteSetup(async () => { + await writeFormatOnlySettings(); + await activateExtension(false); + }); + + setup(async () => { + await resetLog(oxlintLogPath); + await resetLog(oxfmtLogPath); + await writeFormatOnlySettings(); + }); + + teardown(async () => { + await deleteFixtures(); + }); + + suiteTeardown(async () => { + await writeWorkspaceSettings({}); + }); + + test("format-only save does not request oxlint code actions or diagnostics", async () => { + const fileUri = await createFormattingFile("format_only.ts"); + const document = await workspace.openTextDocument(fileUri); + await window.showTextDocument(document); + + const edit = new WorkspaceEdit(); + const fullRange = new Range( + new Position(0, 0), + document.lineAt(document.lineCount - 1).range.end, + ); + edit.replace(fileUri, fullRange, "class X{foo(){return 42;}}\n"); + await workspace.applyEdit(edit); + + await resetLog(oxlintLogPath); + await resetLog(oxfmtLogPath); + + const startedAt = Date.now(); + const saved = await document.save(); + const elapsedMs = Date.now() - startedAt; + strictEqual(saved, true, "expected target document to be saved"); + + const content = await workspace.fs.readFile(fileUri); + strictEqual( + content.toString(), + `class X {${LE} foo() {${LE} return 42;${LE} }${LE}}${LE}`, + `unexpected saved content:\n${content.toString()}`, + ); + + const oxfmtLog = await readLog(oxfmtLogPath); + strictEqual( + oxfmtLog.includes("textDocument/formatting"), + true, + `expected oxfmt formatting request, log:\n${oxfmtLog}`, + ); + + const oxlintLog = await readLog(oxlintLogPath); + strictEqual( + oxlintLog.includes("textDocument/codeAction"), + false, + `unexpected oxlint code action request, log:\n${oxlintLog}`, + ); + strictEqual( + oxlintLog.includes("textDocument/diagnostic"), + false, + `unexpected oxlint diagnostic request, log:\n${oxlintLog}`, + ); + strictEqual(elapsedMs < slowLintResponseMs, true, `save took ${elapsedMs}ms`); + }); + + test("biome fix-all on save does not request oxlint code actions", async () => { + const fakeBiomeKind = CodeActionKind.SourceFixAll.append("biome"); + let fakeBiomeRequests = 0; + const disposable = languages.registerCodeActionsProvider( + { language: "typescript", scheme: "file" }, + { + provideCodeActions: (_document, _range, context) => { + if (context.only?.value !== fakeBiomeKind.value) { + return []; + } + + fakeBiomeRequests += 1; + return []; + }, + }, + { + providedCodeActionKinds: [fakeBiomeKind], + }, + ); + + try { + await writeWorkspaceSettings({ + "editor.codeActionsOnSave": { + "source.fixAll.biome": "explicit", + }, + "editor.defaultFormatter": "oxc.oxc-vscode", + "editor.formatOnSave": true, + "editor.formatOnSaveMode": "file", + }); + + const fileUri = await createFormattingFile("format_biome.ts"); + const document = await workspace.openTextDocument(fileUri); + await window.showTextDocument(document); + + const edit = new WorkspaceEdit(); + const fullRange = new Range( + new Position(0, 0), + document.lineAt(document.lineCount - 1).range.end, + ); + edit.replace(fileUri, fullRange, "class X{foo(){return 42;}}\n"); + await workspace.applyEdit(edit); + + await resetLog(oxlintLogPath); + await resetLog(oxfmtLogPath); + + const startedAt = Date.now(); + const saved = await document.save(); + const elapsedMs = Date.now() - startedAt; + strictEqual(saved, true, "expected target document to be saved"); + + const oxfmtLog = await readLog(oxfmtLogPath); + strictEqual( + oxfmtLog.includes("textDocument/formatting"), + true, + `expected oxfmt formatting request, log:\n${oxfmtLog}`, + ); + strictEqual(fakeBiomeRequests, 1); + + const oxlintLog = await readLog(oxlintLogPath); + strictEqual( + oxlintLog.includes("textDocument/codeAction"), + false, + `unexpected oxlint code action request, log:\n${oxlintLog}`, + ); + strictEqual( + elapsedMs < slowLintResponseMs, + true, + `save waited for slow oxlint code actions, elapsed ${elapsedMs}ms, log:\n${oxlintLog}`, + ); + } finally { + disposable.dispose(); + } + }); + + test("generic fix-all on save respects oxlint opt-out", async () => { + let fakeFixAllRequests = 0; + const disposable = languages.registerCodeActionsProvider( + { language: "typescript", scheme: "file" }, + { + provideCodeActions: (_document, _range, context) => { + if (context.only?.value !== CodeActionKind.SourceFixAll.value) { + return []; + } + + fakeFixAllRequests += 1; + return []; + }, + }, + { + providedCodeActionKinds: [CodeActionKind.SourceFixAll], + }, + ); + + try { + await writeWorkspaceSettings({ + "editor.codeActionsOnSave": { + "source.fixAll": "explicit", + "source.fixAll.oxc": "never", + }, + "editor.defaultFormatter": "oxc.oxc-vscode", + "editor.formatOnSave": true, + "editor.formatOnSaveMode": "file", + }); + + const fileUri = await createFormattingFile("format_generic_fix_all.ts"); + const document = await workspace.openTextDocument(fileUri); + await window.showTextDocument(document); + + const edit = new WorkspaceEdit(); + const fullRange = new Range( + new Position(0, 0), + document.lineAt(document.lineCount - 1).range.end, + ); + edit.replace(fileUri, fullRange, "class X{foo(){return 42;}}\n"); + await workspace.applyEdit(edit); + + await resetLog(oxlintLogPath); + await resetLog(oxfmtLogPath); + + const startedAt = Date.now(); + const saved = await document.save(); + const elapsedMs = Date.now() - startedAt; + strictEqual(saved, true, "expected target document to be saved"); + + const oxfmtLog = await readLog(oxfmtLogPath); + strictEqual( + oxfmtLog.includes("textDocument/formatting"), + true, + `expected oxfmt formatting request, log:\n${oxfmtLog}`, + ); + strictEqual(fakeFixAllRequests, 1); + + const oxlintLog = await readLog(oxlintLogPath); + strictEqual( + oxlintLog.includes("textDocument/codeAction"), + false, + `unexpected oxlint code action request, log:\n${oxlintLog}`, + ); + strictEqual( + elapsedMs < slowLintResponseMs, + true, + `save waited for slow oxlint code actions, elapsed ${elapsedMs}ms, log:\n${oxlintLog}`, + ); + } finally { + disposable.dispose(); + } + }); +}); diff --git a/tests/unit/formatter.spec.ts b/tests/unit/formatter.spec.ts new file mode 100644 index 0000000..99fe15b --- /dev/null +++ b/tests/unit/formatter.spec.ts @@ -0,0 +1,42 @@ +import { strictEqual } from "assert"; +import { CodeActionKind, CodeActionTriggerKind } from "vscode"; +import type { CodeActionContext } from "vscode"; +import { shouldProvideOxfmtCodeAction } from "../../client/tools/formatter.js"; + +function codeActionContext(only: CodeActionKind | undefined): CodeActionContext { + return { + diagnostics: [], + only, + triggerKind: CodeActionTriggerKind.Invoke, + }; +} + +suite("formatter code action routing", () => { + test("allows unscoped code action requests", () => { + strictEqual(shouldProvideOxfmtCodeAction(codeActionContext(undefined)), true); + }); + + test("allows format source action requests", () => { + strictEqual( + shouldProvideOxfmtCodeAction(codeActionContext(CodeActionKind.Source.append("format"))), + true, + ); + strictEqual( + shouldProvideOxfmtCodeAction(codeActionContext(CodeActionKind.Source.append("format.oxc"))), + true, + ); + }); + + test("skips broad or unrelated code action requests", () => { + strictEqual(shouldProvideOxfmtCodeAction(codeActionContext(CodeActionKind.Source)), false); + strictEqual( + shouldProvideOxfmtCodeAction(codeActionContext(CodeActionKind.SourceFixAll)), + false, + ); + strictEqual( + shouldProvideOxfmtCodeAction(codeActionContext(CodeActionKind.SourceOrganizeImports)), + false, + ); + strictEqual(shouldProvideOxfmtCodeAction(codeActionContext(CodeActionKind.QuickFix)), false); + }); +}); diff --git a/tests/unit/linter.spec.ts b/tests/unit/linter.spec.ts new file mode 100644 index 0000000..995dba4 --- /dev/null +++ b/tests/unit/linter.spec.ts @@ -0,0 +1,183 @@ +import { strictEqual } from "assert"; +import { CodeActionKind, CodeActionTriggerKind, Diagnostic, Position, Range } from "vscode"; +import type { CodeActionContext } from "vscode"; +import { + shouldCodeActionsOnSaveRequestOxlint, + shouldRequestOxlintCodeActions, +} from "../../client/tools/linter.js"; + +const oxlintFixAllCodeActionKind = CodeActionKind.SourceFixAll.append("oxc"); + +function codeActionContext( + only: CodeActionKind | undefined, + triggerKind: CodeActionTriggerKind = CodeActionTriggerKind.Invoke, + diagnostics: Diagnostic[] = [], +): CodeActionContext { + return { + diagnostics, + only, + triggerKind, + }; +} + +function diagnostic(): Diagnostic { + return new Diagnostic(new Range(new Position(0, 0), new Position(0, 1)), "test"); +} + +suite("linter code actions on save settings", () => { + test("allows broad source actions on save", () => { + strictEqual( + shouldCodeActionsOnSaveRequestOxlint({ + [CodeActionKind.Source.value]: "always", + }), + true, + ); + }); + + test("allows generic and oxlint fix-all actions on save", () => { + strictEqual( + shouldCodeActionsOnSaveRequestOxlint({ + [CodeActionKind.SourceFixAll.value]: "explicit", + }), + true, + ); + strictEqual( + shouldCodeActionsOnSaveRequestOxlint({ + [oxlintFixAllCodeActionKind.value]: true, + }), + true, + ); + }); + + test("honors source.fixAll.oxc opt-out over broader source settings", () => { + strictEqual( + shouldCodeActionsOnSaveRequestOxlint({ + [CodeActionKind.Source.value]: "always", + [oxlintFixAllCodeActionKind.value]: "never", + }), + false, + ); + strictEqual( + shouldCodeActionsOnSaveRequestOxlint({ + [CodeActionKind.SourceFixAll.value]: "always", + [oxlintFixAllCodeActionKind.value]: false, + }), + false, + ); + }); + + test("honors source.fixAll opt-out over broad source settings", () => { + strictEqual( + shouldCodeActionsOnSaveRequestOxlint({ + [CodeActionKind.Source.value]: "always", + [CodeActionKind.SourceFixAll.value]: "never", + }), + false, + ); + }); + + test("allows legacy array source action settings", () => { + strictEqual(shouldCodeActionsOnSaveRequestOxlint([CodeActionKind.Source.value]), true); + strictEqual(shouldCodeActionsOnSaveRequestOxlint([CodeActionKind.SourceFixAll.value]), true); + strictEqual(shouldCodeActionsOnSaveRequestOxlint([oxlintFixAllCodeActionKind.value]), true); + }); +}); + +suite("linter code action routing", () => { + test("allows unscoped code action requests", () => { + strictEqual(shouldRequestOxlintCodeActions(codeActionContext(undefined)), true); + }); + + test("skips unscoped automatic code action requests", () => { + strictEqual( + shouldRequestOxlintCodeActions(codeActionContext(undefined, CodeActionTriggerKind.Automatic)), + false, + ); + }); + + test("allows unscoped automatic code action requests with diagnostics", () => { + strictEqual( + shouldRequestOxlintCodeActions( + codeActionContext(undefined, CodeActionTriggerKind.Automatic, [diagnostic()]), + ), + true, + ); + }); + + test("allows quick fixes and fix-all requests", () => { + strictEqual(shouldRequestOxlintCodeActions(codeActionContext(CodeActionKind.QuickFix)), true); + strictEqual( + shouldRequestOxlintCodeActions(codeActionContext(CodeActionKind.SourceFixAll)), + true, + ); + strictEqual( + shouldRequestOxlintCodeActions(codeActionContext(CodeActionKind.SourceFixAll.append("oxc"))), + true, + ); + }); + + test("skips unrelated source action requests", () => { + strictEqual( + shouldRequestOxlintCodeActions(codeActionContext(CodeActionKind.SourceOrganizeImports)), + false, + ); + strictEqual( + shouldRequestOxlintCodeActions(codeActionContext(CodeActionKind.Source.append("format.oxc"))), + false, + ); + strictEqual( + shouldRequestOxlintCodeActions( + codeActionContext(CodeActionKind.SourceFixAll.append("biome")), + ), + false, + ); + }); + + test("skips broad automatic source requests", () => { + strictEqual( + shouldRequestOxlintCodeActions( + codeActionContext(CodeActionKind.Source, CodeActionTriggerKind.Automatic), + ), + false, + ); + }); + + test("skips automatic fix-all requests when oxlint is opted out on save", () => { + strictEqual( + shouldRequestOxlintCodeActions( + codeActionContext(CodeActionKind.SourceFixAll, CodeActionTriggerKind.Automatic), + ), + false, + ); + strictEqual( + shouldRequestOxlintCodeActions( + codeActionContext(oxlintFixAllCodeActionKind, CodeActionTriggerKind.Automatic), + ), + false, + ); + }); + + test("allows automatic source requests when fix-all runs on save", () => { + strictEqual( + shouldRequestOxlintCodeActions( + codeActionContext(CodeActionKind.Source, CodeActionTriggerKind.Automatic), + true, + ), + true, + ); + strictEqual( + shouldRequestOxlintCodeActions( + codeActionContext(CodeActionKind.SourceFixAll, CodeActionTriggerKind.Automatic), + true, + ), + true, + ); + strictEqual( + shouldRequestOxlintCodeActions( + codeActionContext(oxlintFixAllCodeActionKind, CodeActionTriggerKind.Automatic), + true, + ), + true, + ); + }); +});