diff --git a/packages/lsp/src/server/pyright.ts b/packages/lsp/src/server/pyright.ts index da32e07..03a0278 100644 --- a/packages/lsp/src/server/pyright.ts +++ b/packages/lsp/src/server/pyright.ts @@ -5,7 +5,7 @@ import type { LSPServerInfo } from './types' import { spawn } from 'node:child_process' import process from 'node:process' -import { attachLSPProcessHandlers, log, nearestRoot } from './utils' +import { attachLSPProcessHandlers, findPackageBin, log, nearestRoot, prepareSpawnCommand } from './utils' export const PYRIGHT_VERSION = '1.1.414' @@ -20,7 +20,8 @@ export const PyrightServer: LSPServerInfo = { ]), async spawn(root) { // Read PATH now rather than the copy Bun captured at startup - const pyright = Bun.which('pyright-langserver', { PATH: process.env.PATH ?? '' }) + const pyright = findPackageBin(root, 'pyright', 'pyright-langserver') + ?? Bun.which('pyright-langserver', { PATH: process.env.PATH ?? '' }) if (!pyright) { // pyright-langserver is a bin of the pyright package, not a package itself. // spawn reports a missing bunx only as an async 'error' event, so check for it first. @@ -36,8 +37,10 @@ export const PyrightServer: LSPServerInfo = { return { process: proc } } - const proc = spawn(pyright, ['--stdio'], { + const launch = prepareSpawnCommand(pyright) + const proc = spawn(launch.command, ['--stdio'], { cwd: root, + shell: launch.shell, }) attachLSPProcessHandlers(proc, 'pyright') return { process: proc } diff --git a/packages/lsp/src/server/typescript.ts b/packages/lsp/src/server/typescript.ts index 7d8b25e..3c016f2 100644 --- a/packages/lsp/src/server/typescript.ts +++ b/packages/lsp/src/server/typescript.ts @@ -14,7 +14,7 @@ import fs from 'node:fs' import { createRequire } from 'node:module' import path from 'node:path' import process from 'node:process' -import { attachLSPProcessHandlers, findRunnableBin, log, nearestRoot, runsVersion } from './utils' +import { attachLSPProcessHandlers, findPackageBin, findRunnableBin, log, nearestRoot, prepareSpawnCommand, resolvePackageBin, runsVersion } from './utils' const TS_EXTENSIONS = ['.ts', '.tsx', '.js', '.jsx', '.mjs', '.cjs', '.mts', '.cts'] @@ -33,52 +33,6 @@ function findTypeScriptPackageRoot(start: string): string | undefined { } } -/** Resolve a package-owned bin declared by the package and exposed by its shared shim. */ -function resolvePackageBin(packageRoot: string, name: string): string | undefined { - try { - const pkg = JSON.parse( - fs.readFileSync(path.join(packageRoot, 'package.json'), 'utf8'), - ) as { bin?: string | Record } - const relativeTarget = typeof pkg.bin === 'string' ? pkg.bin : pkg.bin?.[name] - if (!relativeTarget) { return undefined } - - const target = path.resolve(packageRoot, relativeTarget) - if (!fs.existsSync(target)) { return undefined } - const ext = process.platform === 'win32' ? '.cmd' : '' - const packageDir = path.dirname(packageRoot) - const nodeModules = path.basename(packageDir).startsWith('@') - ? path.dirname(packageDir) - : packageDir - const shim = path.join(nodeModules, '.bin', `${name}${ext}`) - if (!fs.existsSync(shim)) { return undefined } - - // npm-style symlinks and package-manager-generated wrapper scripts are both - // valid shims. For wrappers, require a reference to the package's declared - // target so a stale or unrelated shared shim is never selected. - const stat = fs.lstatSync(shim) - if (stat.isSymbolicLink()) { - const resolvedShim = path.resolve(path.dirname(shim), fs.readlinkSync(shim)) - return fs.existsSync(resolvedShim) - && fs.realpathSync(resolvedShim) === fs.realpathSync(target) - ? shim - : undefined - } - const normalizeShimPath = (value: string): string => { - const normalized = value.replaceAll('\\', '/') - return process.platform === 'win32' ? normalized.toLowerCase() : normalized - } - const wrapper = normalizeShimPath(fs.readFileSync(shim, 'utf8')) - const relativeFromBin = normalizeShimPath(path.relative(path.dirname(shim), target)) - const normalizedTarget = normalizeShimPath(target) - return wrapper.includes(relativeFromBin) || wrapper.includes(normalizedTarget) - ? shim - : undefined - } - catch { - return undefined - } -} - /** Whether the installed `typescript` package is the native (TS 7+) build. */ function isNativeTypeScriptPackage(packageRoot: string): boolean { try { @@ -140,8 +94,7 @@ export function nativeServerSpawnArgs( command: string, platform: NodeJS.Platform = process.platform, ): { command: string, args: string[], shell: boolean } { - const win32 = platform === 'win32' - return { command: win32 ? `"${command}"` : command, args: ['--lsp', '--stdio'], shell: win32 } + return { ...prepareSpawnCommand(command, platform), args: ['--lsp', '--stdio'] } } function hasTsrsBinary(packageRoot: string, root: string): boolean { @@ -199,23 +152,6 @@ function findWrapperPackageRoot(wrapper: string, bin: string): string | undefine return undefined } -function findPackageBin( - start: string, - packageName: string, - name: string, - isUsable: (packageRoot: string) => boolean = () => true, -): string | undefined { - let dir = path.resolve(start) - while (true) { - const packageRoot = path.join(dir, 'node_modules', ...packageName.split('/')) - const bin = resolvePackageBin(packageRoot, name) - if (bin && isUsable(packageRoot)) { return bin } - const parent = path.dirname(dir) - if (parent === dir) { return undefined } - dir = parent - } -} - /** * Resolve a native TypeScript 7 LSP command for `root`: a project-local one if * the workspace provides it, else a global `tsrs` on PATH. Exported for testing. @@ -331,7 +267,7 @@ export const TypescriptServer: LSPServerInfo = { } // On Windows the bin is a .cmd shim, which only runs through a shell. - const launch = nativeServerSpawnArgs(tsserver) + const launch = prepareSpawnCommand(tsserver) const proc = spawn(launch.command, ['--stdio'], { cwd: root, env: { ...process.env, BUN_BE_BUN: '1' }, diff --git a/packages/lsp/src/server/utils.ts b/packages/lsp/src/server/utils.ts index 955f493..edf8ee1 100644 --- a/packages/lsp/src/server/utils.ts +++ b/packages/lsp/src/server/utils.ts @@ -5,6 +5,7 @@ import type { ChildProcessWithoutNullStreams } from 'node:child_process' import type { RootFunction } from './types' +import { existsSync, lstatSync, readFileSync, readlinkSync, realpathSync } from 'node:fs' import fs from 'node:fs/promises' import path from 'node:path' import process from 'node:process' @@ -95,6 +96,88 @@ export async function runsVersion(bin: string, root: string): Promise { } } +/** + * On Windows an npm bin is a .cmd shim, which spawn can only run through a shell, + * so the path is quoted for it. + */ +export function prepareSpawnCommand( + command: string, + platform: NodeJS.Platform = process.platform, +): { command: string, shell: boolean } { + const win32 = platform === 'win32' + return { command: win32 ? `"${command}"` : command, shell: win32 } +} + +/** Resolve a package-owned bin declared by the package and exposed by its shared shim. */ +export function resolvePackageBin(packageRoot: string, name: string): string | undefined { + try { + const pkg = JSON.parse( + readFileSync(path.join(packageRoot, 'package.json'), 'utf8'), + ) as { bin?: string | Record } + const relativeTarget = typeof pkg.bin === 'string' ? pkg.bin : pkg.bin?.[name] + if (!relativeTarget) { return undefined } + + const target = path.resolve(packageRoot, relativeTarget) + if (!existsSync(target)) { return undefined } + const ext = process.platform === 'win32' ? '.cmd' : '' + const packageDir = path.dirname(packageRoot) + const nodeModules = path.basename(packageDir).startsWith('@') + ? path.dirname(packageDir) + : packageDir + const shim = path.join(nodeModules, '.bin', `${name}${ext}`) + if (!existsSync(shim)) { return undefined } + + // npm-style symlinks and package-manager-generated wrapper scripts are both + // valid shims. For wrappers, require a reference to the package's declared + // target so a stale or unrelated shared shim is never selected. + const stat = lstatSync(shim) + if (stat.isSymbolicLink()) { + const resolvedShim = path.resolve(path.dirname(shim), readlinkSync(shim)) + return existsSync(resolvedShim) + && realpathSync(resolvedShim) === realpathSync(target) + ? shim + : undefined + } + const normalizeShimPath = (value: string): string => { + const normalized = value.replaceAll('\\', '/') + return process.platform === 'win32' ? normalized.toLowerCase() : normalized + } + const wrapper = normalizeShimPath(readFileSync(shim, 'utf8')) + // pnpm wrappers reference the package's real path in its .pnpm store, not the node_modules link. + const binDir = path.dirname(shim) + const realTarget = realpathSync(target) + const references = [ + path.relative(binDir, target), + target, + path.relative(realpathSync(binDir), realTarget), + realTarget, + ].map(normalizeShimPath) + return references.some(reference => wrapper.includes(reference)) + ? shim + : undefined + } + catch { + return undefined + } +} + +export function findPackageBin( + start: string, + packageName: string, + name: string, + isUsable: (packageRoot: string) => boolean = () => true, +): string | undefined { + let dir = path.resolve(start) + while (true) { + const packageRoot = path.join(dir, 'node_modules', ...packageName.split('/')) + const bin = resolvePackageBin(packageRoot, name) + if (bin && isUsable(packageRoot)) { return bin } + const parent = path.dirname(dir) + if (parent === dir) { return undefined } + dir = parent + } +} + // ============================================================================= // Root Detection Utilities // ============================================================================= diff --git a/packages/lsp/test/unit/server.test.ts b/packages/lsp/test/unit/server.test.ts index 2d1f830..8514490 100644 --- a/packages/lsp/test/unit/server.test.ts +++ b/packages/lsp/test/unit/server.test.ts @@ -466,11 +466,11 @@ describe.skipIf(process.platform === 'win32')('findRunnableBin', () => { // Each fake server records its name and arguments in `invoked` under the project root. const recorder = (name: string): string => `#!/bin/sh\necho "${name} $*" > invoked\n` -/** Spawn `server` with only `pathBins` on PATH and return what the launched binary recorded. */ async function spawnServerWith( server: LSPServerInfo, pathBins: Record, setupRoot: (rootDir: string) => Promise = async () => {}, + subdir = '', ): Promise { const binDir = await fs.mkdtemp(path.join(os.tmpdir(), 'lsp-fallback-bin-')) const rootDir = await fs.mkdtemp(path.join(os.tmpdir(), 'lsp-fallback-root-')) @@ -480,15 +480,17 @@ async function spawnServerWith( await fs.writeFile(path.join(binDir, name), script, { mode: 0o755 }) } await setupRoot(rootDir) + const spawnRoot = path.join(rootDir, subdir) + await fs.mkdir(spawnRoot, { recursive: true }) process.env.PATH = binDir - const handle = await server.spawn(rootDir) + const handle = await server.spawn(spawnRoot) process.env.PATH = savedPath expect(handle).toBeDefined() const proc = handle!.process if (proc.exitCode === null && proc.signalCode === null) { await new Promise(resolve => proc.once('exit', resolve)) } - return (await fs.readFile(path.join(rootDir, 'invoked'), 'utf8')).trim() + return (await fs.readFile(path.join(spawnRoot, 'invoked'), 'utf8')).trim() } finally { process.env.PATH = savedPath @@ -606,6 +608,64 @@ describe.skipIf(process.platform === 'win32')('PyrightServer fallback', () => { expect(invoked).toBe('path --stdio') }) + async function installLocalPyright(dir: string, script: string, strayShim?: string): Promise { + const packageDir = path.join(dir, 'node_modules', 'pyright') + await fs.mkdir(packageDir, { recursive: true }) + await fs.writeFile( + path.join(packageDir, 'package.json'), + JSON.stringify({ name: 'pyright', bin: { 'pyright': 'index.js', 'pyright-langserver': 'langserver.index.js' } }), + ) + await fs.writeFile(path.join(packageDir, 'langserver.index.js'), script, { mode: 0o755 }) + const binDir = path.join(dir, 'node_modules', '.bin') + await fs.mkdir(binDir) + const shim = path.join(binDir, 'pyright-langserver') + if (strayShim) { + await fs.writeFile(shim, strayShim, { mode: 0o755 }) + } + else { + await fs.symlink(path.join('..', 'pyright', 'langserver.index.js'), shim) + } + } + + const pathAndBunx = { 'pyright-langserver': recorder('path'), 'bunx': recorder('bunx') } + + test('prefers a project-local pyright-langserver over PATH and bunx', async () => { + const invoked = await spawnServerWith(PyrightServer, pathAndBunx, rootDir => installLocalPyright(rootDir, recorder('local'))) + expect(invoked).toBe('local --stdio') + }) + + test('finds a project-local pyright-langserver in a parent of the root', async () => { + const invoked = await spawnServerWith(PyrightServer, pathAndBunx, rootDir => installLocalPyright(rootDir, recorder('local')), 'backend') + expect(invoked).toBe('local --stdio') + }) + + test('accepts a pnpm wrapper shim that points into the .pnpm store', async () => { + const invoked = await spawnServerWith(PyrightServer, pathAndBunx, async (rootDir) => { + const storeRelative = path.join('.pnpm', 'pyright@1.1.414', 'node_modules', 'pyright') + const storeDir = path.join(rootDir, 'node_modules', storeRelative) + await fs.mkdir(storeDir, { recursive: true }) + await fs.writeFile( + path.join(storeDir, 'package.json'), + JSON.stringify({ name: 'pyright', bin: { 'pyright-langserver': 'langserver.index.js' } }), + ) + await fs.writeFile(path.join(storeDir, 'langserver.index.js'), recorder('local'), { mode: 0o755 }) + await fs.symlink(storeRelative, path.join(rootDir, 'node_modules', 'pyright')) + const binDir = path.join(rootDir, 'node_modules', '.bin') + await fs.mkdir(binDir) + await fs.writeFile( + path.join(binDir, 'pyright-langserver'), + `#!/bin/sh\nbasedir=\${0%/*}\nexec "$basedir/../${storeRelative}/langserver.index.js" "$@"\n`, + { mode: 0o755 }, + ) + }) + expect(invoked).toBe('local --stdio') + }) + + test('ignores a .bin/pyright-langserver shim the pyright package does not own', async () => { + const invoked = await spawnServerWith(PyrightServer, pathAndBunx, rootDir => installLocalPyright(rootDir, recorder('local'), recorder('stray'))) + expect(invoked).toBe('path --stdio') + }) + test('returns undefined with the install hint when bunx is missing', async () => { const emptyDir = await fs.mkdtemp(path.join(os.tmpdir(), 'lsp-fallback-empty-')) const savedPath = process.env.PATH