Skip to content
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
45 changes: 13 additions & 32 deletions editors/vscode/client/ConfigService.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,5 @@
import * as path from "node:path";
import {
CancellationTokenSource,
ConfigurationChangeEvent,
Uri,
workspace,
WorkspaceFolder,
} from "vscode";
import { ConfigurationChangeEvent, Uri, workspace, WorkspaceFolder } from "vscode";
import { DiagnosticPullMode } from "vscode-languageclient";
import { validateSafeBinaryPath } from "./PathValidator";
import { IDisposable } from "./types";
Expand Down Expand Up @@ -183,32 +177,19 @@ export class ConfigService implements IDisposable {
* If multiple workspaces contain the binary, the first one found is returned.
*/
private async searchNodeModulesBin(binaryName: string): Promise<string | undefined> {
const cts = new CancellationTokenSource();
setTimeout(() => cts.cancel(), 20000); // cancel after 20 seconds

// try to resolve via require.resolve
try {
// search workspace root plus up to 3 subdirectory levels for the binary path
let patterns = [
`node_modules/.bin/${binaryName}`,
`*/node_modules/.bin/${binaryName}`,
`*/*/node_modules/.bin/${binaryName}`,
`*/*/*/node_modules/.bin/${binaryName}`,
];

if (process.platform === "win32") {
// bun package manager uses `.exe` extension on Windows
// search for both with and without `.exe` extension
patterns = patterns.flatMap((pattern) => [`${pattern}`, `${pattern}.exe`]);
}

for (const pattern of patterns) {
// maybe use `tinyglobby` later for better performance, VSCode can be slow on globbing large projects.
// oxlint-disable-next-line no-await-in-loop -- search sequentially up the directories
const files = await workspace.findFiles(pattern, null, 1, cts.token);
if (files.length > 0) {
return files[0].fsPath;
}
}
const resolvedPath = require
.resolve(binaryName, {
paths: workspace.workspaceFolders?.map((folder) => folder.uri.fsPath) ?? [],
})
// we want to target the binary instead of the main index file
// Improvement: search inside package.json "bin" and `main` field for more reliability
.replace(
`${binaryName}${path.sep}dist${path.sep}index.js`,
`${binaryName}${path.sep}bin${path.sep}${binaryName}`,
);
return resolvedPath;
} catch {}
}

Expand Down
6 changes: 4 additions & 2 deletions editors/vscode/client/tools/formatter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ export default class FormatterTool implements ToolInterface {
outputChannel: LogOutputChannel,
configService: ConfigService,
): Promise<string | undefined> {
if (process.env.SERVER_PATH_DEV) {
return process.env.SERVER_PATH_DEV;
}
const bin = await configService.getOxfmtServerBinPath();
if (bin) {
try {
Expand All @@ -43,7 +46,6 @@ export default class FormatterTool implements ToolInterface {
outputChannel.error(`Invalid bin path: ${bin}`, e);
}
}
return process.env.SERVER_PATH_DEV;
}

async activate(
Expand All @@ -67,7 +69,7 @@ export default class FormatterTool implements ToolInterface {

outputChannel.info(`Using server binary at: ${binaryPath}`);

const run: Executable = runExecutable(binaryPath, configService.vsCodeConfig.nodePath);
const run: Executable = runExecutable(binaryPath, "oxfmt", configService.vsCodeConfig.nodePath);

const serverOptions: ServerOptions = {
run,
Expand Down
5 changes: 4 additions & 1 deletion editors/vscode/client/tools/linter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,9 @@ export default class LinterTool implements ToolInterface {
outputChannel: LogOutputChannel,
configService: ConfigService,
): Promise<string | undefined> {
if (process.env.SERVER_PATH_DEV) {
return process.env.SERVER_PATH_DEV;
}
const bin = await configService.getOxlintServerBinPath();
if (bin) {
try {
Expand All @@ -57,7 +60,6 @@ export default class LinterTool implements ToolInterface {
outputChannel.error(`Invalid bin path: ${bin}`, e);
}
}
return process.env.SERVER_PATH_DEV;
}

async activate(
Expand Down Expand Up @@ -116,6 +118,7 @@ export default class LinterTool implements ToolInterface {

const run: Executable = runExecutable(
binaryPath,
"oxlint",
configService.vsCodeConfig.nodePath,
configService.vsCodeConfig.binPathTsGoLint,
);
Expand Down
21 changes: 17 additions & 4 deletions editors/vscode/client/tools/lsp_helper.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,13 @@
import * as path from "node:path";
import { LogOutputChannel, window } from "vscode";
import { Executable, MessageType, ShowMessageParams } from "vscode-languageclient/node";

export function runExecutable(path: string, nodePath?: string, tsgolintPath?: string): Executable {
export function runExecutable(
binaryPath: string,
nodeBinName: string,
nodePath?: string,
tsgolintPath?: string,
): Executable {
const serverEnv: Record<string, string> = {
...process.env,
RUST_LOG: process.env.RUST_LOG || "info", // Keep for backward compatibility for a while
Expand All @@ -13,20 +19,27 @@ export function runExecutable(path: string, nodePath?: string, tsgolintPath?: st
if (tsgolintPath) {
serverEnv.OXLINT_TSGOLINT_PATH = tsgolintPath;
}
const isNode = path.endsWith(".js") || path.endsWith(".cjs") || path.endsWith(".mjs");
// when the binary path ends with `oxlint/bin/oxlint` or a common js extension, we should run it with `node`
// the path is defined in `ConfigService.searchNodeModulesBin`
const isNode =
binaryPath.endsWith(".js") ||
binaryPath.endsWith(".cjs") ||
binaryPath.endsWith(".mjs") ||
binaryPath.endsWith(`${nodeBinName}${path.sep}bin${path.sep}${nodeBinName}`);

const isWindows = process.platform === "win32";

return isNode
? {
command: "node",
args: [path, "--lsp"],
args: [binaryPath, "--lsp"],
options: {
env: serverEnv,
},
}
: {
// On Windows with shell, quote the command path to handle spaces in usernames/paths
command: isWindows ? `"${path}"` : path,
command: isWindows ? `"${binaryPath}"` : binaryPath,
args: ["--lsp"],
options: {
// On Windows we need to run the binary in a shell to be able to execute the shell npm bin script.
Expand Down
44 changes: 15 additions & 29 deletions editors/vscode/tests/unit/ConfigService.spec.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { strictEqual } from "assert";
import { workspace } from "vscode";
import { ConfigService } from "../../client/ConfigService.js";
import { WORKSPACE_FOLDER, WORKSPACE_SECOND_FOLDER } from "../test-helpers.js";
import { WORKSPACE_FOLDER } from "../test-helpers.js";

const conf = workspace.getConfiguration("oxc");

Expand All @@ -19,14 +19,7 @@ suite("ConfigService", () => {
});

const getWorkspaceFolderPlatformSafe = (folder = WORKSPACE_FOLDER) => {
let workspace_path = folder.uri.path;
if (process.platform === "win32") {
workspace_path = workspace_path.replaceAll("/", "\\");
if (workspace_path.startsWith("\\")) {
workspace_path = workspace_path.slice(1);
}
}
return workspace_path;
return folder.uri.fsPath;
};

const createWorkspaceFolderFileUri = async (relativePath: string, folder = WORKSPACE_FOLDER) => {
Expand All @@ -50,16 +43,26 @@ suite("ConfigService", () => {
};

suite("getOxfmtServerBinPath", () => {
test("falls back to node resolving when server path is not set", async () => {
const service = new ConfigService();
const oxlintPath = (await service.getOxlintServerBinPath())!;
const cwd = process.cwd().replace("/editors/vscode", "");
// it targets the oxc project's oxlint/bin/oxlint path
strictEqual(oxlintPath.startsWith(cwd), true, "path should start with cwd");
strictEqual(
oxlintPath.endsWith("oxlint/bin/oxlint"),
true,
"path should end with oxlint/bin/oxlint",
);
});

test("resolves relative server path with workspace folder", async () => {
const service = new ConfigService();
const workspace_path = getWorkspaceFolderPlatformSafe();
const nonDefinedServerPath = await service.getOxfmtServerBinPath();

await createWorkspaceFolderFileUri("absolute/oxfmt");
await createWorkspaceFolderFileUri("relative/oxfmt");

strictEqual(nonDefinedServerPath, undefined);

await conf.update("path.oxfmt", `${workspace_path}/absolute/oxfmt`);
const absoluteServerPath = await service.getOxfmtServerBinPath();

Expand Down Expand Up @@ -105,14 +108,11 @@ suite("ConfigService", () => {
suite("getOxlintServerBinPath", () => {
test("resolves relative server path with workspace folder", async () => {
const service = new ConfigService();
const nonDefinedServerPath = await service.getOxlintServerBinPath();
const workspace_path = getWorkspaceFolderPlatformSafe();

await createWorkspaceFolderFileUri("absolute/oxlint");
await createWorkspaceFolderFileUri("relative/oxlint");

strictEqual(nonDefinedServerPath, undefined);

await conf.update("path.oxlint", `${workspace_path}/absolute/oxlint`);
const absoluteServerPath = await service.getOxlintServerBinPath();

Expand Down Expand Up @@ -153,19 +153,5 @@ suite("ConfigService", () => {
);
strictEqual(relativeServerPath, `${workspace_path}\\relative\\oxlint`);
});

test("resolves binary path in multi-folder workspace", async () => {
const service = new ConfigService();
const workspace_path = getWorkspaceFolderPlatformSafe();

await createWorkspaceFolderFileUri("node_modules/.bin/oxlint");
await createWorkspaceFolderFileUri("node_modules/.bin/oxlint", WORKSPACE_SECOND_FOLDER);
const absoluteServerPath = await service.getOxlintServerBinPath();

strictEqual(absoluteServerPath, `${workspace_path}/node_modules/.bin/oxlint`);

await deleteWorkspaceFolderFileUri("node_modules/.bin/oxlint");
await deleteWorkspaceFolderFileUri("node_modules/.bin/oxlint", WORKSPACE_SECOND_FOLDER);
});
});
});
16 changes: 9 additions & 7 deletions editors/vscode/tests/unit/lsp_helper.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,38 +4,40 @@ import { runExecutable } from '../../client/tools/lsp_helper';
suite('runExecutable', () => {
const originalPlatform = process.platform;
const originalEnv = process.env;
const tool = "oxlint";

teardown(() => {
Object.defineProperty(process, 'platform', { value: originalPlatform });
process.env = originalEnv;
});


test('should create Node.js executable for .js files', () => {
const result = runExecutable('/path/to/server.js');
const result = runExecutable('/path/to/server.js', tool);

strictEqual(result.command, 'node');
strictEqual(result.args?.[0], '/path/to/server.js');
strictEqual(result.args?.[1], '--lsp');
});

test('should create Node.js executable for .cjs files', () => {
const result = runExecutable('/path/to/server.cjs');
const result = runExecutable('/path/to/server.cjs', tool);

strictEqual(result.command, 'node');
strictEqual(result.args?.[0], '/path/to/server.cjs');
strictEqual(result.args?.[1], '--lsp');
});

test('should create Node.js executable for .mjs files', () => {
const result = runExecutable('/path/to/server.mjs');
const result = runExecutable('/path/to/server.mjs', tool);

strictEqual(result.command, 'node');
strictEqual(result.args?.[0], '/path/to/server.mjs');
strictEqual(result.args?.[1], '--lsp');
});

test('should create binary executable for non-Node files', () => {
const result = runExecutable('/path/to/oxc-language-server');
const result = runExecutable('/path/to/oxc-language-server', tool);

strictEqual(result.command, '/path/to/oxc-language-server');
strictEqual(result.args?.[0], '--lsp');
Expand All @@ -45,7 +47,7 @@ suite('runExecutable', () => {
test('should use shell on Windows for binary executables', () => {
Object.defineProperty(process, 'platform', { value: 'win32' });

const result = runExecutable('/path/to/oxc-language-server');
const result = runExecutable('/path/to/oxc-language-server', tool);

strictEqual(result.options?.shell, true);
});
Expand All @@ -54,15 +56,15 @@ suite('runExecutable', () => {
Object.defineProperty(process, 'platform', { value: 'linux' });
process.env.PATH = '/usr/bin:/bin';

const result = runExecutable('/path/to/server', '/custom/node/path');
const result = runExecutable('/path/to/server', tool, '/custom/node/path');

strictEqual(result.options?.env?.PATH, '/custom/node/path:/usr/bin:/bin');
});

test('should set path in quotes on Windows for binary executables', () => {
Object.defineProperty(process, 'platform', { value: 'win32' });

const result = runExecutable('C:\\Path With Spaces\\oxc-language-server');
const result = runExecutable('C:\\Path With Spaces\\oxc-language-server', tool);

strictEqual(result.command, '"C:\\Path With Spaces\\oxc-language-server"');
});
Expand Down
Loading