From 8c64519cae351c5257e09eb821d2ff5ae8ad4791 Mon Sep 17 00:00:00 2001 From: karitham Date: Sat, 8 Aug 2026 01:19:23 +0200 Subject: [PATCH] vscode: extract pure binary-resolution core, add unit tests --- .github/workflows/release.yml | 1 + vscode/.vscodeignore | 1 + vscode/package.json | 1 + vscode/src/extension.ts | 208 ++++++++++++++++++---------------- vscode/src/platform.test.ts | 79 +++++++++++++ vscode/src/platform.ts | 82 ++++++++++++++ 6 files changed, 272 insertions(+), 100 deletions(-) create mode 100644 vscode/src/platform.test.ts create mode 100644 vscode/src/platform.ts diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index ef5136e..dd4f49a 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -61,6 +61,7 @@ jobs: working-directory: vscode run: | npm ci + npm run test npm run package - uses: actions/upload-artifact@v4 with: diff --git a/vscode/.vscodeignore b/vscode/.vscodeignore index f3a7575..044d923 100644 --- a/vscode/.vscodeignore +++ b/vscode/.vscodeignore @@ -1,5 +1,6 @@ .vscode/** src/** tsconfig.json +out/*.test.js **/node_modules/typescript/** node_modules/@types/** diff --git a/vscode/package.json b/vscode/package.json index adc8585..fbc09fb 100644 --- a/vscode/package.json +++ b/vscode/package.json @@ -64,6 +64,7 @@ "vscode:prepublish": "npm run compile", "compile": "tsc -p ./", "watch": "tsc -watch -p ./", + "test": "tsc -p ./ && node --test 'out/*.test.js'", "package": "vsce package" }, "dependencies": { diff --git a/vscode/src/extension.ts b/vscode/src/extension.ts index 2edc64a..a572beb 100644 --- a/vscode/src/extension.ts +++ b/vscode/src/extension.ts @@ -1,5 +1,4 @@ import { spawnSync } from 'child_process'; -import * as crypto from 'crypto'; import * as fs from 'fs'; import * as path from 'path'; import { @@ -16,11 +15,20 @@ import { LanguageClientOptions, ServerOptions, } from 'vscode-languageclient/node'; +import { assetName, findOnPath, parseChecksums, sha256Hex } from './platform'; const REPO = 'karitham/thrift-ls'; const RELEASES_URL = `https://github.com/${REPO}/releases`; const DOWNLOAD_URL = `${RELEASES_URL}/latest/download`; +// The release asset and its checksum file for the running platform, or +// undefined when no prebuilt binary exists for it. +interface ReleaseTarget { + name: string; + url: string; + checksumUrl: string; +} + let client: LanguageClient | undefined; export async function activate(context: ExtensionContext) { @@ -55,6 +63,12 @@ function deactivate(): Thenable | undefined { return client?.stop(); } +/** + * resolveBinary returns the path of a usable thrift-ls binary: the + * configured path, then PATH, then a cached download, then a prompt that + * offers to download the release binary. Undefined means no server this + * session. + */ async function resolveBinary(context: ExtensionContext): Promise { const configured = workspace.getConfiguration('thrift-ls').get('path'); if (configured) { @@ -67,12 +81,27 @@ async function resolveBinary(context: ExtensionContext): Promise { @@ -119,7 +132,16 @@ async function reinstall(context: ExtensionContext): Promise { ); return; } - const bin = await downloadBinary(context, true); + + const target = releaseTarget(); + if (!target) { + window.showErrorMessage( + `thrift-ls publishes no binary for ${process.platform}/${process.arch}.` + ); + return; + } + + const bin = await downloadBinary(context, target, true); if (!bin) { return; } @@ -132,11 +154,17 @@ async function reinstall(context: ExtensionContext): Promise { } } +/** + * downloadBinary fetches, verifies, and installs the release binary, + * replacing a previously downloaded copy. Returns the installed path, or + * undefined on failure (an error message is shown). + */ async function downloadBinary( context: ExtensionContext, + target: ReleaseTarget, force = false ): Promise { - const dest = cachedBinaryPath(context); + const dest = cachedBinaryPath(context, target); if (!force && fs.existsSync(dest)) { return dest; } @@ -148,59 +176,58 @@ async function downloadBinary( cancellable: false, }, async () => { - let name: string; try { - name = binaryName(); + // Gather (impure): fetch the checksum and the binary. + const bytes = await fetchVerified(target); + // Commit (impure): write and atomically replace. + await installBinary(bytes, target.name, dest); } catch (err) { window.showErrorMessage( - `Cannot download thrift-ls: ${err instanceof Error ? err.message : String(err)}. See ${RELEASES_URL}` + `Failed to download thrift-ls: ${errMessage(err)}. See ${RELEASES_URL}` ); return undefined; } - const url = `${DOWNLOAD_URL}/${name}`; - - try { - const expected = await fetchChecksum(`${DOWNLOAD_URL}/checksums.txt`, name); - const bytes = await fetchBytes(url); - if (expected) { - const actual = crypto.createHash('sha256').update(bytes).digest('hex'); - if (actual !== expected) { - throw new Error( - `checksum mismatch for ${name} (got ${actual.slice(0, 12)}…, want ${expected.slice(0, 12)}…)` - ); - } - } + const version = versionOf(dest); + window.showInformationMessage( + `thrift-ls${version ? ` ${version}` : ''} installed to ${dest}` + ); + return dest; + } + ); +} - await fs.promises.mkdir(path.dirname(dest), { recursive: true }); - const tmp = `${dest}.tmp-${process.pid}`; - await fs.promises.writeFile(tmp, bytes); - makeExecutable(tmp); - try { - await fs.promises.rename(tmp, dest); - } catch (err) { - await fs.promises.rm(tmp, { force: true }).catch(() => undefined); - if (process.platform === 'win32') { - throw new Error( - `could not replace ${name}: the previous copy may still be in use by a running server. Reload the window and retry (${err instanceof Error ? err.message : String(err)})` - ); - } - throw err; - } +/** fetchVerified downloads the binary and aborts on checksum mismatch. */ +async function fetchVerified(target: ReleaseTarget): Promise { + const expected = await fetchChecksum(target.checksumUrl, target.name); + const bytes = await fetchBytes(target.url); + if (expected && sha256Hex(bytes) !== expected) { + throw new Error( + `checksum mismatch for ${target.name} (want ${expected.slice(0, 12)}…)` + ); + } + return bytes; +} - const version = versionOf(dest); - window.showInformationMessage( - `thrift-ls${version ? ` ${version}` : ''} installed to ${dest}` - ); - return dest; - } catch (err) { - window.showErrorMessage( - `Failed to download thrift-ls: ${err instanceof Error ? err.message : String(err)}. See ${RELEASES_URL}` - ); - return undefined; - } +/** installBinary writes the binary to dest, replacing any previous copy. */ +async function installBinary(bytes: Buffer, name: string, dest: string): Promise { + await fs.promises.mkdir(path.dirname(dest), { recursive: true }); + const tmp = `${dest}.tmp-${process.pid}`; + await fs.promises.writeFile(tmp, bytes); + makeExecutable(tmp); + try { + await fs.promises.rename(tmp, dest); + } catch (err) { + await fs.promises.rm(tmp, { force: true }).catch(() => undefined); + if (process.platform === 'win32') { + // Windows cannot replace a file that is in use; the previous copy is + // the running server process. + throw new Error( + `could not replace ${name}: the previous copy may still be in use by a running server. Reload the window and retry (${errMessage(err)})` + ); } - ); + throw err; + } } async function fetchChecksum(checksumUrl: string, name: string): Promise { @@ -211,14 +238,7 @@ async function fetchChecksum(checksumUrl: string, name: string): Promise { @@ -235,36 +255,20 @@ function versionOf(bin: string): string | undefined { return text || undefined; } -function cachedBinaryPath(context: ExtensionContext): string { - return path.join(context.globalStorageUri.fsPath, 'bin', binaryName()); -} - -function binaryName(): string { - return `thrift-ls-${osName()}-${archName()}${process.platform === 'win32' ? '.exe' : ''}`; -} - -function osName(): string { - switch (process.platform) { - case 'win32': - return 'windows'; - case 'darwin': - return 'darwin'; - case 'linux': - return 'linux'; - default: - throw new Error(`unsupported platform: ${process.platform}`); +function releaseTarget(): ReleaseTarget | undefined { + const name = assetName(process.platform, process.arch); + if (!name) { + return undefined; } + return { + name, + url: `${DOWNLOAD_URL}/${name}`, + checksumUrl: `${DOWNLOAD_URL}/checksums.txt`, + }; } -function archName(): string { - switch (process.arch) { - case 'x64': - return 'amd64'; - case 'arm64': - return 'arm64'; - default: - throw new Error(`unsupported architecture: ${process.arch}`); - } +function cachedBinaryPath(context: ExtensionContext, target: ReleaseTarget): string { + return path.join(context.globalStorageUri.fsPath, 'bin', target.name); } function makeExecutable(file: string): void { @@ -273,4 +277,8 @@ function makeExecutable(file: string): void { } } +function errMessage(err: unknown): string { + return err instanceof Error ? err.message : String(err); +} + export { deactivate }; diff --git a/vscode/src/platform.test.ts b/vscode/src/platform.test.ts new file mode 100644 index 0000000..c9b4568 --- /dev/null +++ b/vscode/src/platform.test.ts @@ -0,0 +1,79 @@ +import assert from 'node:assert'; +import { test } from 'node:test'; +import { assetName, findOnPath, parseChecksums, sha256Hex } from './platform'; + +test('assetName maps every released platform/arch', () => { + assert.equal(assetName('linux', 'x64'), 'thrift-ls-linux-amd64'); + assert.equal(assetName('linux', 'arm64'), 'thrift-ls-linux-arm64'); + assert.equal(assetName('darwin', 'x64'), 'thrift-ls-darwin-amd64'); + assert.equal(assetName('darwin', 'arm64'), 'thrift-ls-darwin-arm64'); + assert.equal(assetName('win32', 'x64'), 'thrift-ls-windows-amd64.exe'); + assert.equal(assetName('win32', 'arm64'), 'thrift-ls-windows-arm64.exe'); +}); + +test('assetName is undefined for unsupported platforms', () => { + assert.equal(assetName('freebsd', 'x64'), undefined); + assert.equal(assetName('linux', 'ia32'), undefined); +}); + +test('findOnPath searches PATH in order', () => { + const existing = new Set(['/usr/bin/thrift-ls', '/usr/local/bin/thrift-ls']); + const found = findOnPath({ + platform: 'linux', + pathEnv: '/usr/bin:/usr/local/bin:/opt/bin', + sep: '/', + delimiter: ':', + exists: (p) => existing.has(p), + }); + assert.equal(found, '/usr/bin/thrift-ls'); +}); + +test('findOnPath tries PATHEXT extensions on windows', () => { + const existing = new Set(['C:\\tools\\thrift-ls.exe']); + const found = findOnPath({ + platform: 'win32', + pathEnv: 'C:\\tools;C:\\other', + pathext: '.EXE;.CMD', + sep: '\\', + delimiter: ';', + exists: (p) => existing.has(p), + }); + assert.equal(found, 'C:\\tools\\thrift-ls.exe'); +}); + +test('findOnPath returns undefined when nothing matches', () => { + const found = findOnPath({ + platform: 'linux', + pathEnv: '/usr/bin:/opt/bin', + sep: '/', + delimiter: ':', + exists: () => false, + }); + assert.equal(found, undefined); +}); + +test('parseChecksums maps filenames to hashes', () => { + const checksums = parseChecksums( + 'aa11 thrift-ls-linux-amd64\nbb22 thrift-ls-linux-arm64\n' + ); + assert.equal(checksums.get('thrift-ls-linux-amd64'), 'aa11'); + assert.equal(checksums.get('thrift-ls-linux-arm64'), 'bb22'); + assert.equal(checksums.get('missing'), undefined); +}); + +test('parseChecksums ignores malformed lines', () => { + const checksums = parseChecksums('not-a-checksum-line\n\ncc33 thrift-ls-darwin-amd64\n'); + assert.equal(checksums.size, 1); + assert.equal(checksums.get('thrift-ls-darwin-amd64'), 'cc33'); +}); + +test('sha256Hex matches known digests', () => { + assert.equal( + sha256Hex(Buffer.from('')), + 'e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855' + ); + assert.equal( + sha256Hex(Buffer.from('thrift-ls')), + '4c43bf971bd110e5ff6122e6f10ab8730559f8459f7a13757df0960c0b9ccc56' + ); +}); diff --git a/vscode/src/platform.ts b/vscode/src/platform.ts new file mode 100644 index 0000000..96ee12a --- /dev/null +++ b/vscode/src/platform.ts @@ -0,0 +1,82 @@ +import { createHash } from 'crypto'; + +/** + * Pure decision logic for locating the thrift-ls binary. No I/O: filesystem + * existence and environment are injected by the caller, so every function is + * unit-testable without mocks. + */ + +// Release asset names are `thrift-ls--[.exe]`, matching the names +// the release workflow attaches. `process.platform`/`process.arch` map onto +// them; anything else has no prebuilt binary. +const ASSET_OS: Readonly> = { + linux: 'linux', + darwin: 'darwin', + win32: 'windows', +}; +const GO_ARCH: Readonly> = { + x64: 'amd64', + arm64: 'arm64', +}; + +/** + * assetName returns the release asset file name for the running platform, + * or undefined when no prebuilt binary exists (no throw: an unsupported + * platform is a value, not an exceptional condition). + */ +export function assetName(platform: string, arch: string): string | undefined { + const os = ASSET_OS[platform]; + const goarch = GO_ARCH[arch]; + if (!os || !goarch) { + return undefined; + } + return `thrift-ls-${os}-${goarch}${platform === 'win32' ? '.exe' : ''}`; +} + +/** + * findOnPath searches PATH (in order) for a `thrift-ls` executable. On + * Windows each PATHEXT extension is tried per directory. sep/delimiter are + * injected so the search is testable on any host. + */ +export function findOnPath(opts: { + platform: string; + pathEnv?: string; + pathext?: string; + sep: string; + delimiter: string; + exists: (candidate: string) => boolean; +}): string | undefined { + const { platform, pathEnv, pathext, sep, delimiter, exists } = opts; + const exts = platform === 'win32' + ? (pathext ?? '.COM;.EXE;.BAT;.CMD').split(';').filter(Boolean) + : ['']; + for (const dir of (pathEnv ?? '').split(delimiter).filter(Boolean)) { + for (const ext of exts) { + const candidate = `${dir}${sep}thrift-ls${ext.toLowerCase()}`; + if (exists(candidate)) { + return candidate; + } + } + } + return undefined; +} + +/** + * parseChecksums parses `sha256sum` output (` ` per line) into + * a filename → hash map. + */ +export function parseChecksums(text: string): ReadonlyMap { + const checksums = new Map(); + for (const line of text.split('\n')) { + const parts = line.trim().split(/\s+/); + if (parts.length >= 2) { + checksums.set(parts[1], parts[0]); + } + } + return checksums; +} + +/** sha256Hex returns the lowercase hex sha256 of bytes. */ +export function sha256Hex(bytes: Buffer): string { + return createHash('sha256').update(bytes).digest('hex'); +} -- 2.51.2