From 2eb606fac8fe6ac9b60e19b18ec095ede950ebca Mon Sep 17 00:00:00 2001 From: quobix Date: Fri, 24 Jul 2026 09:06:45 -0400 Subject: [PATCH] extract Windows shim launching into tested executable helper Move the cmd.exe .cmd/.bat shim handling out of extension.ts into a new buildLanguageServerExecutable() in src/executable.ts, now spawning with windowsVerbatimArguments and caret-escaping metacharacters so shim paths with spaces and special characters launch correctly. Add unit tests (node --test) covering shim invocation, escaping, and direct execution, plus a test:unit script. Run the tests in a new reusable Test workflow on ubuntu/windows for PRs, gate release tagging on it, and run it before packaging in the Open VSX workflow. Exclude out/test/** from the VSIX. --- .github/workflows/open-vsx.yml | 6 +- .github/workflows/tag.yml | 4 ++ .github/workflows/test.yml | 34 +++++++++ .vscodeignore | 1 + package.json | 3 +- src/executable.ts | 44 ++++++++++++ src/extension.ts | 22 ++---- src/test/executable.test.ts | 121 +++++++++++++++++++++++++++++++++ 8 files changed, 216 insertions(+), 19 deletions(-) create mode 100644 .github/workflows/test.yml create mode 100644 src/executable.ts create mode 100644 src/test/executable.test.ts diff --git a/.github/workflows/open-vsx.yml b/.github/workflows/open-vsx.yml index 672bd10..2d452c4 100644 --- a/.github/workflows/open-vsx.yml +++ b/.github/workflows/open-vsx.yml @@ -39,15 +39,15 @@ jobs: npm version --no-git-tag-version "${tag_version}" + - name: Run unit tests + run: npm run test:unit + - name: Read package version id: package run: | VERSION="$(node -p "require('./package.json').version")" echo "version=${VERSION}" >> "$GITHUB_OUTPUT" - - name: Compile extension - run: npm run compile - - name: Package VSIX run: npm run package:vsix diff --git a/.github/workflows/tag.yml b/.github/workflows/tag.yml index 12a982f..c269ebc 100644 --- a/.github/workflows/tag.yml +++ b/.github/workflows/tag.yml @@ -15,7 +15,11 @@ concurrency: cancel-in-progress: false jobs: + test: + uses: ./.github/workflows/test.yml + tag: + needs: test runs-on: ubuntu-latest steps: - name: Checkout diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml new file mode 100644 index 0000000..2267e95 --- /dev/null +++ b/.github/workflows/test.yml @@ -0,0 +1,34 @@ +name: Test + +on: + pull_request: + workflow_call: + +permissions: + contents: read + +jobs: + test: + strategy: + fail-fast: false + matrix: + os: + - ubuntu-latest + - windows-latest + runs-on: ${{ matrix.os }} + + steps: + - name: Checkout + uses: actions/checkout@v4 + + - name: Set up Node.js + uses: actions/setup-node@v4 + with: + node-version: 20 + cache: npm + + - name: Install dependencies + run: npm ci + + - name: Run unit tests + run: npm run test:unit diff --git a/.vscodeignore b/.vscodeignore index 3cf6bbf..a237b91 100644 --- a/.vscodeignore +++ b/.vscodeignore @@ -5,6 +5,7 @@ .idea/** src/** dist/** +out/test/** .gitignore .yarnrc vsc-extension-quickstart.md diff --git a/package.json b/package.json index ac76f77..bc605ed 100644 --- a/package.json +++ b/package.json @@ -167,7 +167,8 @@ "publish:openvsx": "ovsx publish --skip-duplicate", "publish:marketplace": "vsce publish --skip-duplicate --packagePath", "pretest": "npm run compile", - "test": "vscode-test" + "test": "vscode-test", + "test:unit": "npm run compile && node --test out/test/executable.test.js" }, "devDependencies": { "@types/mocha": "^10.0.6", diff --git a/src/executable.ts b/src/executable.ts new file mode 100644 index 0000000..f05905b --- /dev/null +++ b/src/executable.ts @@ -0,0 +1,44 @@ +import * as path from 'path'; + +const windowsCommandMetaCharacters = /([()\][%!^"`<>&|;, *?])/g; + +export interface ResolvedExecutable { + command: string; + args: string[]; + options?: { + shell: boolean; + windowsVerbatimArguments: boolean; + }; +} + +// buildLanguageServerExecutable returns a spawn-safe command for the vacuum +// language server. Windows command shims must run through cmd.exe with arguments +// passed verbatim; otherwise Node escapes the quotes and cmd.exe treats them as +// literal characters. +export function buildLanguageServerExecutable( + command: string, + platform: NodeJS.Platform = process.platform, + commandShell: string | undefined = process.env.ComSpec +): ResolvedExecutable { + const extension = platform === 'win32' + ? path.win32.extname(command).toLowerCase() + : path.extname(command).toLowerCase(); + + if (platform === 'win32' && (extension === '.cmd' || extension === '.bat')) { + const shellCommand = `${escapeWindowsCommand(command)} language-server`; + return { + command: commandShell ?? 'cmd.exe', + args: ['/d', '/s', '/c', `"${shellCommand}"`], + options: { + shell: false, + windowsVerbatimArguments: true, + }, + }; + } + + return { command, args: ['language-server'] }; +} + +function escapeWindowsCommand(command: string): string { + return command.replace(windowsCommandMetaCharacters, '^$1'); +} diff --git a/src/extension.ts b/src/extension.ts index ee3fc68..c6facce 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -7,6 +7,10 @@ import { LanguageClientOptions, ServerOptions, } from 'vscode-languageclient/node'; +import { + buildLanguageServerExecutable, + type ResolvedExecutable, +} from './executable'; const configSection = 'vacuum'; const enabledSetting = 'languageServer.enabled'; @@ -17,11 +21,6 @@ const isWindows = os.platform() === 'win32'; let lspClient: LanguageClient | undefined; -interface ResolvedExecutable { - command: string; - args: string[]; -} - interface ConfigurationUpdateScope { target: vscode.ConfigurationTarget; overrideInLanguage: boolean; @@ -91,8 +90,8 @@ async function startLanguageServer(showReadyMessage: boolean): Promise { } const serverOptions: ServerOptions = { - run: { command: executable.command, args: executable.args }, - debug: { command: executable.command, args: executable.args }, + run: executable, + debug: executable, }; const clientOptions: LanguageClientOptions = { documentSelector: [{ scheme: 'file', language: 'yaml' }, { scheme: 'file', language: 'json' }], @@ -347,14 +346,7 @@ function findFirstExistingExecutable(candidates: string[]): ResolvedExecutable | } function toResolvedExecutable(command: string): ResolvedExecutable { - const extension = path.extname(command).toLowerCase(); - if (isWindows && (extension === '.cmd' || extension === '.bat')) { - return { - command: process.env.ComSpec ?? 'cmd.exe', - args: ['/d', '/s', '/c', `"${command}" language-server`], - }; - } - return { command, args: ['language-server'] }; + return buildLanguageServerExecutable(command); } function expandPath(value: string): string { diff --git a/src/test/executable.test.ts b/src/test/executable.test.ts new file mode 100644 index 0000000..ab3a56b --- /dev/null +++ b/src/test/executable.test.ts @@ -0,0 +1,121 @@ +import * as assert from 'node:assert/strict'; +import { spawn } from 'node:child_process'; +import { mkdtemp, rm, writeFile } from 'node:fs/promises'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import { test } from 'node:test'; +import { buildLanguageServerExecutable } from '../executable'; + +test('builds a verbatim cmd.exe invocation for an npm command shim', () => { + const executable = buildLanguageServerExecutable( + 'C:\\Users\\erwin\\AppData\\Roaming\\npm\\vacuum.cmd', + 'win32', + 'C:\\Windows\\System32\\cmd.exe' + ); + + assert.deepEqual(executable, { + command: 'C:\\Windows\\System32\\cmd.exe', + args: [ + '/d', + '/s', + '/c', + '"C:\\Users\\erwin\\AppData\\Roaming\\npm\\vacuum.cmd language-server"', + ], + options: { + shell: false, + windowsVerbatimArguments: true, + }, + }); +}); + +test('builds the same cmd.exe invocation for batch shims', () => { + const executable = buildLanguageServerExecutable( + 'C:\\Tools\\vacuum.BAT', + 'win32', + 'cmd.exe' + ); + + assert.deepEqual(executable, { + command: 'cmd.exe', + args: ['/d', '/s', '/c', '"C:\\Tools\\vacuum.BAT language-server"'], + options: { + shell: false, + windowsVerbatimArguments: true, + }, + }); +}); + +test('escapes spaces and command metacharacters in a Windows shim path', () => { + const executable = buildLanguageServerExecutable( + 'C:\\Program Files\\PB33F & Co\\vacuum.cmd', + 'win32' + ); + + assert.equal( + executable.args[3], + '"C:\\Program^ Files\\PB33F^ ^&^ Co\\vacuum.cmd language-server"' + ); +}); + +test('runs Windows executables directly', () => { + assert.deepEqual( + buildLanguageServerExecutable('C:\\Tools\\vacuum.exe', 'win32'), + { + command: 'C:\\Tools\\vacuum.exe', + args: ['language-server'], + } + ); +}); + +test('runs non-Windows executables directly', () => { + assert.deepEqual( + buildLanguageServerExecutable('/usr/local/bin/vacuum', 'darwin'), + { + command: '/usr/local/bin/vacuum', + args: ['language-server'], + } + ); +}); + +test('launches a Windows command shim from a path containing spaces', { + skip: process.platform !== 'win32', +}, async () => { + const directory = await mkdtemp(path.join(os.tmpdir(), 'vacuum launcher ')); + const shim = path.join(directory, 'vacuum.cmd'); + + try { + await writeFile( + shim, + '@echo off\r\nif not "%~1"=="language-server" exit /b 2\r\necho vacuum-lsp-started\r\nexit /b 0\r\n' + ); + + const executable = buildLanguageServerExecutable(shim); + const result = await run(executable); + + assert.equal(result.code, 0, result.stderr); + assert.match(result.stdout, /vacuum-lsp-started/); + } finally { + await rm(directory, { recursive: true, force: true }); + } +}); + +async function run(executable: ReturnType): Promise<{ + code: number | null; + stdout: string; + stderr: string; +}> { + return new Promise((resolve, reject) => { + const child = spawn(executable.command, executable.args, executable.options); + let stdout = ''; + let stderr = ''; + + child.stdout.on('data', (chunk: Buffer) => { + stdout += chunk.toString(); + }); + child.stderr.on('data', (chunk: Buffer) => { + stderr += chunk.toString(); + }); + child.on('error', reject); + child.on('close', (code) => resolve({ code, stdout, stderr })); + }); +}