From 28447288340354ebb6087af22af7cc29156a9c8e Mon Sep 17 00:00:00 2001 From: Dmitry Lebed <209805+d-lebed@users.noreply.github.com> Date: Wed, 27 Nov 2024 00:10:55 +0100 Subject: [PATCH 1/4] Add docker compose as a version manager --- vscode/package.json | 9 + vscode/src/client.ts | 165 ++++++++++++++++- vscode/src/common.ts | 11 +- vscode/src/docker.ts | 171 ++++++++++++++++++ vscode/src/ruby.ts | 38 +++- vscode/src/ruby/compose.ts | 180 +++++++++++++++++++ vscode/src/ruby/rubyInstaller.ts | 25 +-- vscode/src/ruby/versionManager.ts | 153 ++++++++++++++-- vscode/src/rubyLsp.ts | 2 +- vscode/src/test/suite/ruby.test.ts | 7 +- vscode/src/test/suite/ruby/asdf.test.ts | 50 ++++-- vscode/src/test/suite/ruby/compose.test.ts | 126 +++++++++++++ vscode/src/test/suite/ruby/custom.test.ts | 21 ++- vscode/src/test/suite/ruby/mise.test.ts | 35 ++-- vscode/src/test/suite/ruby/none.test.ts | 32 ++-- vscode/src/test/suite/ruby/rbenv.test.ts | 47 +++-- vscode/src/test/suite/ruby/rvm.test.ts | 21 ++- vscode/src/test/suite/ruby/shadowenv.test.ts | 14 +- vscode/src/test/suite/testHelpers.ts | 59 ++++++ vscode/src/workspace.ts | 18 +- 20 files changed, 1058 insertions(+), 126 deletions(-) create mode 100644 vscode/src/docker.ts create mode 100644 vscode/src/ruby/compose.ts create mode 100644 vscode/src/test/suite/ruby/compose.test.ts create mode 100644 vscode/src/test/suite/testHelpers.ts diff --git a/vscode/package.json b/vscode/package.json index 2df1524f6b..3a18ea028f 100644 --- a/vscode/package.json +++ b/vscode/package.json @@ -338,6 +338,7 @@ "rvm", "shadowenv", "mise", + "compose", "custom" ], "default": "auto" @@ -357,6 +358,14 @@ "chrubyRubies": { "description": "An array of extra directories to search for Ruby installations when using chruby. Equivalent to the RUBIES environment variable", "type": "array" + }, + "composeService": { + "description": "The name of the service in the compose file to use to start the Ruby LSP server", + "type": "string" + }, + "composeCustomCommand": { + "description": "A shell command to start the ruby LSP server using compose", + "type": "string" } }, "default": { diff --git a/vscode/src/client.ts b/vscode/src/client.ts index 186370c74d..812a37f78e 100644 --- a/vscode/src/client.ts +++ b/vscode/src/client.ts @@ -37,6 +37,7 @@ import { SUPPORTED_LANGUAGE_IDS, FEATURE_FLAGS, featureEnabled, + PathConverterInterface, } from "./common"; import { Ruby } from "./ruby"; import { WorkspaceChannel } from "./workspaceChannel"; @@ -60,7 +61,7 @@ function enabledFeatureFlags(): Record { // Get the executables to start the server based on the user's configuration function getLspExecutables( workspaceFolder: vscode.WorkspaceFolder, - env: NodeJS.ProcessEnv, + ruby: Ruby, ): ServerOptions { let run: Executable; let debug: Executable; @@ -73,8 +74,8 @@ function getLspExecutables( const executableOptions: ExecutableOptions = { cwd: workspaceFolder.uri.fsPath, env: bypassTypechecker - ? { ...env, RUBY_LSP_BYPASS_TYPECHECKER: "true" } - : env, + ? { ...ruby.env, RUBY_LSP_BYPASS_TYPECHECKER: "true" } + : ruby.env, shell: true, }; @@ -128,6 +129,9 @@ function getLspExecutables( }; } + run = ruby.activateExecutable(run); + debug = ruby.activateExecutable(debug); + return { run, debug }; } @@ -165,6 +169,32 @@ function collectClientOptions( }, ); + const pathConverter = ruby.pathConverter; + + const pushAlternativePaths = ( + path: string, + schemes: string[] = supportedSchemes, + ) => { + schemes.forEach((scheme) => { + [ + pathConverter.toLocalPath(path), + pathConverter.toRemotePath(path), + ].forEach((convertedPath) => { + if (convertedPath !== path) { + SUPPORTED_LANGUAGE_IDS.forEach((language) => { + documentSelector.push({ + scheme, + language, + pattern: `${convertedPath}/**/*`, + }); + }); + } + }); + }); + }; + + pushAlternativePaths(fsPath); + // Only the first language server we spawn should handle unsaved files, otherwise requests will be duplicated across // all workspaces if (isMainWorkspace) { @@ -184,6 +214,8 @@ function collectClientOptions( pattern: `${gemPath}/**/*`, }); + pushAlternativePaths(gemPath, [scheme]); + // Because of how default gems are installed, the gemPath location is actually not exactly where the files are // located. With the regex, we are correcting the default gem path from this (where the files are not located) // /opt/rubies/3.3.1/lib/ruby/gems/3.3.0 @@ -194,15 +226,50 @@ function collectClientOptions( // Notice that we still need to add the regular path to the selector because some version managers will install // gems under the non-corrected path if (/lib\/ruby\/gems\/(?=\d)/.test(gemPath)) { + const correctedPath = gemPath.replace( + /lib\/ruby\/gems\/(?=\d)/, + "lib/ruby/", + ); + documentSelector.push({ scheme, language: "ruby", - pattern: `${gemPath.replace(/lib\/ruby\/gems\/(?=\d)/, "lib/ruby/")}/**/*`, + pattern: `${correctedPath}/**/*`, }); + + pushAlternativePaths(correctedPath, [scheme]); } }); }); + // Add other mapped paths to the document selector + pathConverter.pathMapping.forEach(([local, remote]) => { + if ( + (documentSelector as { pattern: string }[]).some( + (selector) => + selector.pattern?.startsWith(local) || + selector.pattern?.startsWith(remote), + ) + ) { + return; + } + + supportedSchemes.forEach((scheme) => { + SUPPORTED_LANGUAGE_IDS.forEach((language) => { + documentSelector.push({ + language, + pattern: `${local}/**/*`, + }); + + documentSelector.push({ + scheme, + language, + pattern: `${remote}/**/*`, + }); + }); + }); + }); + // This is a temporary solution as an escape hatch for users who cannot upgrade the `ruby-lsp` gem to a version that // supports ERB if (!configuration.get("erbSupport")) { @@ -211,9 +278,29 @@ function collectClientOptions( }); } + outputChannel.info( + `Document Selector Paths: ${JSON.stringify(documentSelector)}`, + ); + + // Map using pathMapping + const code2Protocol = (uri: vscode.Uri) => { + const remotePath = pathConverter.toRemotePath(uri.fsPath); + return vscode.Uri.file(remotePath).toString(); + }; + + const protocol2Code = (uri: string) => { + const remoteUri = vscode.Uri.parse(uri); + const localPath = pathConverter.toLocalPath(remoteUri.fsPath); + return vscode.Uri.file(localPath); + }; + return { documentSelector, workspaceFolder, + uriConverters: { + code2Protocol, + protocol2Code, + }, diagnosticCollectionName: LSP_NAME, outputChannel, revealOutputChannelOn: RevealOutputChannelOn.Never, @@ -316,6 +403,7 @@ export default class Client extends LanguageClient implements ClientInterface { private readonly baseFolder; private readonly workspaceOutputChannel: WorkspaceChannel; private readonly virtualDocuments = new Map(); + private readonly pathConverter: PathConverterInterface; #context: vscode.ExtensionContext; #formatter: string; @@ -333,7 +421,7 @@ export default class Client extends LanguageClient implements ClientInterface { ) { super( LSP_NAME, - getLspExecutables(workspaceFolder, ruby.env), + getLspExecutables(workspaceFolder, ruby), collectClientOptions( vscode.workspace.getConfiguration("rubyLsp"), workspaceFolder, @@ -348,6 +436,7 @@ export default class Client extends LanguageClient implements ClientInterface { this.registerFeature(new ExperimentalCapabilities()); this.workspaceOutputChannel = outputChannel; this.virtualDocuments = virtualDocuments; + this.pathConverter = ruby.pathConverter; // Middleware are part of client options, but because they must reference `this`, we cannot make it a part of the // `super` call (TypeScript does not allow accessing `this` before invoking `super`) @@ -428,7 +517,9 @@ export default class Client extends LanguageClient implements ClientInterface { range?: Range, ): Promise<{ ast: string } | null> { return this.sendRequest("rubyLsp/textDocument/showSyntaxTree", { - textDocument: { uri: uri.toString() }, + textDocument: { + uri: this.pathConverter.toRemoteUri(uri).toString(), + }, range, }); } @@ -624,10 +715,12 @@ export default class Client extends LanguageClient implements ClientInterface { token, _next, ) => { + const remoteUri = this.pathConverter.toRemoteUri(document.uri); + const response: vscode.TextEdit[] | null = await this.sendRequest( "textDocument/onTypeFormatting", { - textDocument: { uri: document.uri.toString() }, + textDocument: { uri: remoteUri.toString() }, position, ch, options, @@ -695,9 +788,65 @@ export default class Client extends LanguageClient implements ClientInterface { token?: vscode.CancellationToken, ) => Promise, ) => { - return this.benchmarkMiddleware(type, param, () => + this.workspaceOutputChannel.trace( + `Sending request: ${JSON.stringify(type)} with params: ${JSON.stringify(param)}`, + ); + + const result = (await this.benchmarkMiddleware(type, param, () => next(type, param, token), + )) as any; + + this.workspaceOutputChannel.trace( + `Received response for ${JSON.stringify(type)}: ${JSON.stringify(result)}`, ); + + const request = typeof type === "string" ? type : type.method; + + try { + switch (request) { + case "rubyLsp/workspace/dependencies": + return result.map((dep: { path: string }) => { + return { + ...dep, + path: this.pathConverter.toLocalPath(dep.path), + }; + }); + + case "textDocument/codeAction": + return result.map((action: { uri: string }) => { + const remotePath = vscode.Uri.parse(action.uri).fsPath; + const localPath = this.pathConverter.toLocalPath(remotePath); + + return { + ...action, + uri: vscode.Uri.file(localPath).toString(), + }; + }); + + case "textDocument/hover": + if ( + result?.contents?.kind === "markdown" && + result.contents.value + ) { + result.contents.value = result.contents.value.replace( + /\((file:\/\/.+?)#/gim, + (_match: string, path: string) => { + const remotePath = vscode.Uri.parse(path).fsPath; + const localPath = + this.pathConverter.toLocalPath(remotePath); + return `(${vscode.Uri.file(localPath).toString()}#`; + }, + ); + } + break; + } + } catch (error) { + this.workspaceOutputChannel.error( + `Error while processing response for ${request}: ${error}`, + ); + } + + return result; }, sendNotification: async ( type: string | MessageSignature, diff --git a/vscode/src/common.ts b/vscode/src/common.ts index 327ca2baef..4e9056da41 100644 --- a/vscode/src/common.ts +++ b/vscode/src/common.ts @@ -1,4 +1,4 @@ -import { exec } from "child_process"; +import { exec, spawn as originalSpawn } from "child_process"; import { createHash } from "crypto"; import { promisify } from "util"; @@ -65,11 +65,20 @@ export interface WorkspaceInterface { error: boolean; } +export interface PathConverterInterface { + pathMapping: [string, string][]; + toRemotePath: (localPath: string) => string; + toLocalPath: (remotePath: string) => string; + toRemoteUri: (localUri: vscode.Uri) => vscode.Uri; +} + // Event emitter used to signal that the language status items need to be refreshed export const STATUS_EMITTER = new vscode.EventEmitter< WorkspaceInterface | undefined >(); +export const spawn = originalSpawn; + export const asyncExec = promisify(exec); export const LSP_NAME = "Ruby LSP"; export const LOG_CHANNEL = vscode.window.createOutputChannel(LSP_NAME, { diff --git a/vscode/src/docker.ts b/vscode/src/docker.ts new file mode 100644 index 0000000000..3b959ac8d5 --- /dev/null +++ b/vscode/src/docker.ts @@ -0,0 +1,171 @@ +import path from "path"; + +import * as vscode from "vscode"; + +import { PathConverterInterface } from "./common"; +import { WorkspaceChannel } from "./workspaceChannel"; + +export interface ComposeConfig { + services: Record; + ["x-mutagen"]?: { sync: Record } | undefined; +} + +interface ComposeService { + volumes: ComposeVolume[]; +} + +interface ComposeVolume { + type: string; + source: string; + target: string; +} + +interface MutagenShare { + alpha: string; + beta: string; +} + +interface MutagenMount { + volume: string; + source: string; + target: string; +} + +type MutagenMountMapping = Record< + string, + { + source: string; + target: string; + } +>; + +export function fetchPathMapping( + config: ComposeConfig, + service: string, +): Record { + const mutagenMounts = fetchMutagenMounts(config["x-mutagen"]?.sync || {}); + + const bindings = fetchComposeBindings( + config.services[service]?.volumes || [], + mutagenMounts, + ); + + return bindings; +} + +export class ContainerPathConverter implements PathConverterInterface { + readonly pathMapping: [string, string][]; + private readonly outputChannel: WorkspaceChannel; + + constructor( + pathMapping: Record, + outputChannel: WorkspaceChannel, + ) { + this.pathMapping = Object.entries(pathMapping); + this.outputChannel = outputChannel; + } + + toRemotePath(path: string) { + for (const [local, remote] of this.pathMapping) { + if (path.startsWith(local)) { + const remotePath = path.replace(local, remote); + + this.outputChannel.debug( + `Converted toRemotePath ${path} to ${remotePath}`, + ); + + return path.replace(local, remote); + } + } + + return path; + } + + toLocalPath(path: string) { + for (const [local, remote] of this.pathMapping) { + if (path.startsWith(remote)) { + const localPath = path.replace(remote, local); + + this.outputChannel.debug( + `Converted toLocalPath ${path} to ${localPath}`, + ); + + return localPath; + } + } + + return path; + } + + toRemoteUri(localUri: vscode.Uri) { + const remotePath = this.toRemotePath(localUri.fsPath); + return vscode.Uri.file(remotePath); + } +} + +function fetchComposeBindings( + volumes: ComposeVolume[], + mutagenMounts: MutagenMountMapping, +): Record { + return volumes.reduce( + (acc: Record, volume: ComposeVolume) => { + if (volume.type === "bind") { + acc[volume.source] = volume.target; + } else if (volume.type === "volume") { + Object.entries(mutagenMounts).forEach( + ([ + mutagenVolume, + { source: mutagenSource, target: mutagenTarget }, + ]) => { + if (mutagenVolume.startsWith(`volume://${volume.source}/`)) { + const remotePath = path.resolve(volume.target, mutagenSource); + acc[mutagenTarget] = remotePath; + } + }, + ); + } + + return acc; + }, + {}, + ); +} + +function transformMutagenMount(alpha: string, beta: string): MutagenMount { + const [, ...path] = alpha.replace("volume://", "").split("/"); + const [volume, source] = + path.length > 0 ? [alpha, `./${path.join("/")}`] : [`${alpha}/`, "."]; + + return { volume, source, target: beta }; +} + +function fetchMutagenMounts( + sync: Record = {}, +): MutagenMountMapping { + return Object.entries(sync).reduce( + ( + acc: Record, + [name, { alpha, beta }]: [string, MutagenShare], + ) => { + if (name === "defaults") return acc; + + let mount: MutagenMount | null = null; + + if (alpha.startsWith("volume://")) { + mount = transformMutagenMount(alpha, beta); + } else if (beta.startsWith("volume://")) { + mount = transformMutagenMount(beta, alpha); + } + + if (mount) { + acc[mount.volume] = { + source: mount.source, + target: mount.target, + }; + } + + return acc; + }, + {}, + ); +} diff --git a/vscode/src/ruby.ts b/vscode/src/ruby.ts index 5270bc9a36..767b7bf46a 100644 --- a/vscode/src/ruby.ts +++ b/vscode/src/ruby.ts @@ -1,19 +1,22 @@ /* eslint-disable no-process-env */ import path from "path"; import os from "os"; +import { ExecOptions } from "child_process"; import * as vscode from "vscode"; +import { Executable } from "vscode-languageclient/node"; -import { asyncExec, RubyInterface } from "./common"; +import { asyncExec, PathConverterInterface, RubyInterface } from "./common"; import { WorkspaceChannel } from "./workspaceChannel"; import { Shadowenv } from "./ruby/shadowenv"; import { Chruby } from "./ruby/chruby"; -import { VersionManager } from "./ruby/versionManager"; +import { PathConverter, VersionManager } from "./ruby/versionManager"; import { Mise } from "./ruby/mise"; import { RubyInstaller } from "./ruby/rubyInstaller"; import { Rbenv } from "./ruby/rbenv"; import { Rvm } from "./ruby/rvm"; import { None } from "./ruby/none"; +import { Compose } from "./ruby/compose"; import { Custom } from "./ruby/custom"; import { Asdf } from "./ruby/asdf"; @@ -44,6 +47,7 @@ export enum ManagerIdentifier { Shadowenv = "shadowenv", Mise = "mise", RubyInstaller = "rubyInstaller", + Compose = "compose", None = "none", Custom = "custom", } @@ -65,6 +69,8 @@ export class Ruby implements RubyInterface { private readonly shell = process.env.SHELL?.replace(/(\s+)/g, "\\$1"); private _env: NodeJS.ProcessEnv = {}; + private _manager?: VersionManager; + private _pathConverter: PathConverterInterface = new PathConverter(); private _error = false; private readonly context: vscode.ExtensionContext; private readonly customBundleGemfile?: string; @@ -109,6 +115,14 @@ export class Ruby implements RubyInterface { } } + get pathConverter() { + return this._pathConverter; + } + + set pathConverter(pathConverter: PathConverterInterface) { + this._pathConverter = pathConverter; + } + get env() { return this._env; } @@ -189,6 +203,14 @@ export class Ruby implements RubyInterface { } } + runActivatedScript(command: string, options: ExecOptions = {}) { + return this._manager!.runActivatedScript(command, options); + } + + activateExecutable(executable: Executable) { + return this._manager!.activateExecutable(executable); + } + async manuallySelectRuby() { const manualSelection = await vscode.window.showInformationMessage( "Configure global or workspace specific fallback for the Ruby LSP?", @@ -246,9 +268,12 @@ export class Ruby implements RubyInterface { this.sanitizeEnvironment(env); + this.pathConverter = await manager.buildPathConverter(this.workspaceFolder); + // We need to set the process environment too to make other extensions such as Sorbet find the right Ruby paths process.env = env; this._env = env; + this._manager = manager; this.rubyVersion = version; this.yjitEnabled = (yjit && major > 3) || (major === 3 && minor >= 2); this.gemPath.push(...gemPath); @@ -350,6 +375,15 @@ export class Ruby implements RubyInterface { ), ); break; + case ManagerIdentifier.Compose: + await this.runActivation( + new Compose( + this.workspaceFolder, + this.outputChannel, + this.manuallySelectRuby.bind(this), + ), + ); + break; case ManagerIdentifier.Custom: await this.runActivation( new Custom( diff --git a/vscode/src/ruby/compose.ts b/vscode/src/ruby/compose.ts new file mode 100644 index 0000000000..6996942d79 --- /dev/null +++ b/vscode/src/ruby/compose.ts @@ -0,0 +1,180 @@ +/* eslint-disable no-process-env */ +import { ExecOptions } from "child_process"; +import path from "path"; + +import * as vscode from "vscode"; +import { Executable } from "vscode-languageclient/node"; + +import { + ComposeConfig, + ContainerPathConverter, + fetchPathMapping, +} from "../docker"; + +import { VersionManager, ActivationResult } from "./versionManager"; + +// Compose +// +// Docker Compose is a tool for defining and running multi-container Docker applications. If your project uses Docker +// Compose, you can run Ruby LSP in one of the services defined in your `docker-compose.yml` file. It also supports +// mutagen file synchronization and can be customized to use a different Docker Compose wrapper command. +export class Compose extends VersionManager { + protected composeConfig: ComposeConfig = { services: {} } as ComposeConfig; + + async activate(): Promise { + await this.ensureConfigured(); + + const parsedResult = await this.runEnvActivationScript( + `${this.composeRunCommand()} ${this.composeServiceName()} ruby`, + ); + + return { + env: { ...process.env }, + yjit: parsedResult.yjit, + version: parsedResult.version, + gemPath: parsedResult.gemPath, + }; + } + + runActivatedScript(command: string, options: ExecOptions = {}) { + return this.runScript( + `${this.composeRunCommand()} ${this.composeServiceName()} ${command}`, + options, + ); + } + + activateExecutable(executable: Executable) { + const composeCommand = this.parseCommand( + `${this.composeRunCommand()} ${this.composeServiceName()}`, + ); + + return { + command: composeCommand.command, + args: [ + ...composeCommand.args, + executable.command, + ...(executable.args || []), + ], + options: { + ...executable.options, + env: { ...(executable.options?.env || {}), ...composeCommand.env }, + }, + }; + } + + async buildPathConverter(workspaceFolder: vscode.WorkspaceFolder) { + const pathMapping = fetchPathMapping( + this.composeConfig, + this.composeServiceName(), + ); + + const stats = Object.entries(pathMapping).map(([local, remote]) => { + const absolute = path.resolve(workspaceFolder.uri.fsPath, local); + return vscode.workspace.fs.stat(vscode.Uri.file(absolute)).then( + (stat) => ({ stat, local, remote, absolute }), + () => ({ stat: undefined, local, remote, absolute }), + ); + }); + + const filteredMapping = (await Promise.all(stats)).reduce( + (acc, { stat, local, remote, absolute }) => { + if (stat?.type === vscode.FileType.Directory) { + this.outputChannel.info(`Path ${absolute} mapped to ${remote}`); + acc[absolute] = remote; + } else { + this.outputChannel.debug( + `Skipping path ${local} because it does not exist`, + ); + } + + return acc; + }, + {} as Record, + ); + + return new ContainerPathConverter(filteredMapping, this.outputChannel); + } + + protected composeRunCommand(): string { + return `${this.composeCommand()} run --rm -i`; + } + + protected composeServiceName(): string { + const service: string | undefined = vscode.workspace + .getConfiguration("rubyLsp.rubyVersionManager") + .get("composeService"); + + if (service === undefined) { + throw new Error( + "The composeService configuration must be set when 'compose' is selected as the version manager. \ + See the [README](https://shopify.github.io/ruby-lsp/version-managers.html) for instructions.", + ); + } + + return service; + } + + protected composeCommand(): string { + const composeCustomCommand: string | undefined = vscode.workspace + .getConfiguration("rubyLsp.rubyVersionManager") + .get("composeCustomCommand"); + + return ( + composeCustomCommand || + "docker --log-level=error compose --progress=quiet" + ); + } + + protected async ensureConfigured() { + this.composeConfig = await this.getComposeConfig(); + const services: vscode.QuickPickItem[] = []; + + const config = vscode.workspace.getConfiguration("rubyLsp"); + const currentService = config.get("rubyVersionManager.composeService") as + | string + | undefined; + + if (currentService && this.composeConfig.services[currentService]) { + return; + } + + for (const [name, _service] of Object.entries( + this.composeConfig.services, + )) { + services.push({ label: name }); + } + + const answer = await vscode.window.showQuickPick(services, { + title: "Select Docker Compose service where to run ruby-lsp", + ignoreFocusOut: true, + }); + + if (!answer) { + throw new Error("No compose service selected"); + } + + const managerConfig = config.inspect("rubyVersionManager"); + const workspaceConfig = managerConfig?.workspaceValue || {}; + + await config.update("rubyVersionManager", { + ...workspaceConfig, + ...{ composeService: answer.label }, + }); + } + + private async getComposeConfig(): Promise { + try { + const { stdout, stderr: _stderr } = await this.runScript( + `${this.composeCommand()} config --format=json`, + ); + + const config = JSON.parse(stdout) as ComposeConfig; + + return config; + } catch (error: any) { + throw new Error( + `Failed to read docker-compose configuration: ${error.message}`, + ); + } + } +} diff --git a/vscode/src/ruby/rubyInstaller.ts b/vscode/src/ruby/rubyInstaller.ts index 33f4912ab7..7bab203e0b 100644 --- a/vscode/src/ruby/rubyInstaller.ts +++ b/vscode/src/ruby/rubyInstaller.ts @@ -1,10 +1,9 @@ /* eslint-disable no-process-env */ import os from "os"; +import { ExecOptions } from "child_process"; import * as vscode from "vscode"; -import { asyncExec } from "../common"; - import { Chruby } from "./chruby"; interface RubyVersion { @@ -55,20 +54,14 @@ export class RubyInstaller extends Chruby { ); } - // Override the `runScript` method to ensure that we do not pass any `shell` to `asyncExec`. The activation script is - // only compatible with `cmd.exe`, and not Powershell, due to escaping of quotes. We need to ensure to always run the - // script on `cmd.exe`. - protected runScript(command: string) { - this.outputChannel.info( - `Running command: \`${command}\` in ${this.bundleUri.fsPath}`, - ); - this.outputChannel.debug( - `Environment used for command: ${JSON.stringify(process.env)}`, - ); - - return asyncExec(command, { + // Override the `execOptions` method to ensure that we do not pass any `shell` to `asyncExec`. The activation script + // is only compatible with `cmd.exe`, and not Powershell, due to escaping of quotes. We need to ensure to always run + // the script on `cmd.exe`. + protected execOptions(options: ExecOptions = {}): ExecOptions { + return { cwd: this.bundleUri.fsPath, - env: process.env, - }); + ...options, + env: { ...process.env, ...options.env }, + }; } } diff --git a/vscode/src/ruby/versionManager.ts b/vscode/src/ruby/versionManager.ts index f24837490d..85f6b3e8b5 100644 --- a/vscode/src/ruby/versionManager.ts +++ b/vscode/src/ruby/versionManager.ts @@ -1,11 +1,13 @@ /* eslint-disable no-process-env */ import path from "path"; import os from "os"; +import { ExecOptions } from "child_process"; import * as vscode from "vscode"; +import { Executable } from "vscode-languageclient/node"; import { WorkspaceChannel } from "../workspaceChannel"; -import { asyncExec } from "../common"; +import { asyncExec, PathConverterInterface, spawn } from "../common"; export interface ActivationResult { env: NodeJS.ProcessEnv; @@ -16,6 +18,22 @@ export interface ActivationResult { export const ACTIVATION_SEPARATOR = "RUBY_LSP_ACTIVATION_SEPARATOR"; +export class PathConverter implements PathConverterInterface { + readonly pathMapping: [string, string][] = []; + + toRemotePath(path: string) { + return path; + } + + toLocalPath(path: string) { + return path; + } + + toRemoteUri(localUri: vscode.Uri) { + return localUri; + } +} + export abstract class VersionManager { public activationScript = [ `STDERR.print("${ACTIVATION_SEPARATOR}" + `, @@ -59,9 +77,26 @@ export abstract class VersionManager { // language server abstract activate(): Promise; + runActivatedScript(command: string, options: ExecOptions = {}) { + return this.runScript(command, options); + } + + activateExecutable(executable: Executable) { + return executable; + } + + async buildPathConverter(_workspaceFolder: vscode.WorkspaceFolder) { + return new PathConverter(); + } + protected async runEnvActivationScript(activatedRuby: string) { - const result = await this.runScript( - `${activatedRuby} -W0 -rjson -e '${this.activationScript}'`, + const result = await this.runRubyCode( + `${activatedRuby} -W0 -rjson`, + this.activationScript, + ); + + this.outputChannel.debug( + `Activation script output: ${JSON.stringify(result, null, 2)}`, ); const activationContent = new RegExp( @@ -85,7 +120,72 @@ export abstract class VersionManager { // Runs the given command in the directory for the Bundle, using the user's preferred shell and inheriting the current // process environment - protected runScript(command: string) { + protected runScript(command: string, options: ExecOptions = {}) { + const execOptions = this.execOptions(options); + + this.outputChannel.info( + `Running command: \`${command}\` in ${execOptions.cwd} using shell: ${execOptions.shell}`, + ); + this.outputChannel.debug( + `Environment used for command: ${JSON.stringify(execOptions.env)}`, + ); + + return asyncExec(command, execOptions); + } + + protected runRubyCode( + rubyCommand: string, + code: string, + ): Promise<{ stdout: string; stderr: string }> { + return new Promise((resolve, reject) => { + this.outputChannel.info( + `Ruby \`${rubyCommand}\` running Ruby code: \`${code}\``, + ); + + const { command, args, env } = this.parseCommand(rubyCommand); + const ruby = spawn(command, args, this.execOptions({ env })); + + let stdout = ""; + let stderr = ""; + + ruby.stdout.on("data", (data) => { + this.outputChannel.debug(`stdout: '${data.toString()}'`); + if (data.toString().includes("END_OF_RUBY_CODE_OUTPUT")) { + stdout += data.toString().replace(/END_OF_RUBY_CODE_OUTPUT.*/s, ""); + resolve({ stdout, stderr }); + } else { + stdout += data.toString(); + } + }); + ruby.stderr.on("data", (data) => { + this.outputChannel.debug(`stderr: '${data.toString()}'`); + stderr += data.toString(); + }); + ruby.on("error", (error) => { + reject(error); + }); + ruby.on("close", (status) => { + if (status) { + reject(new Error(`Process exited with status ${status}: ${stderr}`)); + } else { + resolve({ stdout, stderr }); + } + }); + + const script = [ + "begin", + ...code.split("\n").map((line) => ` ${line}`), + "ensure", + ' puts "END_OF_RUBY_CODE_OUTPUT"', + "end", + ].join("\n"); + + ruby.stdin.write(script); + ruby.stdin.end(); + }); + } + + protected execOptions(options: ExecOptions = {}): ExecOptions { let shell: string | undefined; // If the user has configured a default shell, we use that one since they are probably sourcing their version @@ -95,18 +195,43 @@ export abstract class VersionManager { shell = vscode.env.shell; } - this.outputChannel.info( - `Running command: \`${command}\` in ${this.bundleUri.fsPath} using shell: ${shell}`, - ); - this.outputChannel.debug( - `Environment used for command: ${JSON.stringify(process.env)}`, - ); - - return asyncExec(command, { + return { cwd: this.bundleUri.fsPath, shell, - env: process.env, - }); + ...options, + env: { ...process.env, ...options.env }, + }; + } + + // Parses a command string into its command, arguments, and environment variables + protected parseCommand(commandString: string): { + command: string; + args: string[]; + env: Record; + } { + // Regular expression to split arguments while respecting quotes + const regex = /(?:[^\s"']+|"[^"]*"|'[^']*')+/g; + + const parts = + commandString.match(regex)?.map((arg) => { + // Remove surrounding quotes, if any + return arg.replace(/^['"]|['"]$/g, ""); + }) ?? []; + + // Extract environment variables + const env: Record = {}; + while (parts[0] && parts[0].includes("=")) { + const [key, value] = parts.shift()?.split("=") ?? []; + if (key) { + env[key] = value || ""; + } + } + + // The first part is the command, the rest are arguments + const command = parts.shift() || ""; + const args = parts; + + return { command, args, env }; } // Tries to find `execName` within the given directories. Prefers the executables found in the given directories over diff --git a/vscode/src/rubyLsp.ts b/vscode/src/rubyLsp.ts index 54f1b2f6c2..a439797aa8 100644 --- a/vscode/src/rubyLsp.ts +++ b/vscode/src/rubyLsp.ts @@ -110,7 +110,7 @@ export class RubyLsp { } const decodedUri = decodeURIComponent(originalUri); - return this.virtualDocuments.get(decodedUri); + return this.virtualDocuments.get(decodedUri) || ""; }, }), LOG_CHANNEL, diff --git a/vscode/src/test/suite/ruby.test.ts b/vscode/src/test/suite/ruby.test.ts index e1c4abb348..5fb74b32cc 100644 --- a/vscode/src/test/suite/ruby.test.ts +++ b/vscode/src/test/suite/ruby.test.ts @@ -8,10 +8,10 @@ import sinon from "sinon"; import { Ruby, ManagerIdentifier } from "../../ruby"; import { WorkspaceChannel } from "../../workspaceChannel"; import { LOG_CHANNEL } from "../../common"; -import * as common from "../../common"; import { ACTIVATION_SEPARATOR } from "../../ruby/versionManager"; import { FAKE_TELEMETRY } from "./fakeTelemetry"; +import { createSpawnStub } from "./testHelpers"; suite("Ruby environment activation", () => { const workspacePath = path.dirname( @@ -130,8 +130,7 @@ suite("Ruby environment activation", () => { gemPath: ["~/.gem/ruby/3.3.5", "/opt/rubies/3.3.5/lib/ruby/gems/3.3.0"], }; - const execStub = sinon.stub(common, "asyncExec").resolves({ - stdout: "", + const { spawnStub } = createSpawnStub({ stderr: `${ACTIVATION_SEPARATOR}${JSON.stringify(envStub)}${ACTIVATION_SEPARATOR}`, }); @@ -142,7 +141,7 @@ suite("Ruby environment activation", () => { FAKE_TELEMETRY, ); await ruby.activateRuby(); - execStub.restore(); + spawnStub.restore(); configStub.restore(); assert.deepStrictEqual(ruby.gemPath, [ diff --git a/vscode/src/test/suite/ruby/asdf.test.ts b/vscode/src/test/suite/ruby/asdf.test.ts index 46a4449ad3..8eb1781e06 100644 --- a/vscode/src/test/suite/ruby/asdf.test.ts +++ b/vscode/src/test/suite/ruby/asdf.test.ts @@ -9,6 +9,7 @@ import { Asdf } from "../../../ruby/asdf"; import { WorkspaceChannel } from "../../../workspaceChannel"; import * as common from "../../../common"; import { ACTIVATION_SEPARATOR } from "../../../ruby/versionManager"; +import { createSpawnStub } from "../testHelpers"; suite("Asdf", () => { if (os.platform() === "win32") { @@ -17,6 +18,13 @@ suite("Asdf", () => { return; } + let spawnStub: sinon.SinonStub; + let stdinData: string[]; + + teardown(() => { + spawnStub?.restore(); + }); + test("Finds Ruby based on .tool-versions", async () => { // eslint-disable-next-line no-process-env const workspacePath = process.env.PWD!; @@ -33,10 +41,9 @@ suite("Asdf", () => { version: "3.0.0", }; - const execStub = sinon.stub(common, "asyncExec").resolves({ - stdout: "", + ({ spawnStub, stdinData } = createSpawnStub({ stderr: `${ACTIVATION_SEPARATOR}${JSON.stringify(envStub)}${ACTIVATION_SEPARATOR}`, - }); + })); const findInstallationStub = sinon .stub(asdf, "findAsdfInstallation") @@ -46,8 +53,17 @@ suite("Asdf", () => { const { env, version, yjit } = await asdf.activate(); assert.ok( - execStub.calledOnceWithExactly( - `. ${os.homedir()}/.asdf/asdf.sh && asdf exec ruby -W0 -rjson -e '${asdf.activationScript}'`, + spawnStub.calledOnceWithExactly( + ".", + [ + `${os.homedir()}/.asdf/asdf.sh`, + "&&", + "asdf", + "exec", + "ruby", + "-W0", + "-rjson", + ], { cwd: workspacePath, shell: "/bin/bash", @@ -57,11 +73,12 @@ suite("Asdf", () => { ), ); + assert.ok(stdinData.join("\n").includes(asdf.activationScript)); + assert.strictEqual(version, "3.0.0"); assert.strictEqual(yjit, true); assert.strictEqual(env.ANY, "true"); - execStub.restore(); findInstallationStub.restore(); shellStub.restore(); }); @@ -82,10 +99,9 @@ suite("Asdf", () => { version: "3.0.0", }; - const execStub = sinon.stub(common, "asyncExec").resolves({ - stdout: "", + ({ spawnStub, stdinData } = createSpawnStub({ stderr: `${ACTIVATION_SEPARATOR}${JSON.stringify(envStub)}${ACTIVATION_SEPARATOR}`, - }); + })); const findInstallationStub = sinon .stub(asdf, "findAsdfInstallation") @@ -97,8 +113,17 @@ suite("Asdf", () => { const { env, version, yjit } = await asdf.activate(); assert.ok( - execStub.calledOnceWithExactly( - `. ${os.homedir()}/.asdf/asdf.fish && asdf exec ruby -W0 -rjson -e '${asdf.activationScript}'`, + spawnStub.calledOnceWithExactly( + ".", + [ + `${os.homedir()}/.asdf/asdf.fish`, + "&&", + "asdf", + "exec", + "ruby", + "-W0", + "-rjson", + ], { cwd: workspacePath, shell: "/opt/homebrew/bin/fish", @@ -108,11 +133,12 @@ suite("Asdf", () => { ), ); + assert.ok(stdinData.join("\n").includes(asdf.activationScript)); + assert.strictEqual(version, "3.0.0"); assert.strictEqual(yjit, true); assert.strictEqual(env.ANY, "true"); - execStub.restore(); findInstallationStub.restore(); shellStub.restore(); }); diff --git a/vscode/src/test/suite/ruby/compose.test.ts b/vscode/src/test/suite/ruby/compose.test.ts new file mode 100644 index 0000000000..2aff78c8cd --- /dev/null +++ b/vscode/src/test/suite/ruby/compose.test.ts @@ -0,0 +1,126 @@ +import assert from "assert"; +import path from "path"; +import fs from "fs"; +import os from "os"; + +import * as vscode from "vscode"; +import sinon from "sinon"; + +import { WorkspaceChannel } from "../../../workspaceChannel"; +import { Compose } from "../../../ruby/compose"; +import * as common from "../../../common"; +import { ACTIVATION_SEPARATOR } from "../../../ruby/versionManager"; +import { createSpawnStub } from "../testHelpers"; +import { ComposeConfig } from "../../../docker"; + +suite("Compose", () => { + let spawnStub: sinon.SinonStub; + let execStub: sinon.SinonStub; + let configStub: sinon.SinonStub; + let stdinData: string[]; + + let workspacePath: string; + let workspaceFolder: vscode.WorkspaceFolder; + let outputChannel: WorkspaceChannel; + + const composeService = "develop"; + const composeConfig: ComposeConfig = { + services: { [composeService]: { volumes: [] } }, + }; + + setup(() => { + workspacePath = fs.mkdtempSync(path.join(os.tmpdir(), "ruby-lsp-test-")); + workspaceFolder = { + uri: vscode.Uri.file(workspacePath), + name: path.basename(workspacePath), + index: 0, + }; + outputChannel = new WorkspaceChannel("fake", common.LOG_CHANNEL); + }); + + teardown(() => { + spawnStub?.restore(); + execStub?.restore(); + configStub?.restore(); + + fs.rmSync(workspacePath, { recursive: true, force: true }); + }); + + test("Activates Ruby environment using Docker Compose", async () => { + const compose = new Compose(workspaceFolder, outputChannel, async () => {}); + + const envStub = { + env: { ANY: "true" }, + yjit: true, + version: "3.0.0", + }; + + ({ spawnStub, stdinData } = createSpawnStub({ + stderr: `${ACTIVATION_SEPARATOR}${JSON.stringify(envStub)}${ACTIVATION_SEPARATOR}`, + })); + + execStub = sinon + .stub(common, "asyncExec") + .resolves({ stdout: JSON.stringify(composeConfig), stderr: "" }); + + configStub = sinon.stub(vscode.workspace, "getConfiguration").returns({ + get: (name: string) => { + if ( + name === "composeService" || + name === "rubyVersionManager.composeService" + ) { + return composeService; + } else if (name === "rubyVersionManager") { + return { composeService }; + } + return undefined; + }, + } as any); + + const { version, yjit } = await compose.activate(); + + // We must not set the shell on Windows + const shell = os.platform() === "win32" ? undefined : vscode.env.shell; + + assert.ok( + spawnStub.calledOnceWithExactly( + "docker", + [ + "--log-level=error", + "compose", + "--progress=quiet", + "run", + "--rm", + "-i", + composeService, + "ruby", + "-W0", + "-rjson", + ], + { + cwd: workspaceFolder.uri.fsPath, + shell, + // eslint-disable-next-line no-process-env + env: process.env, + }, + ), + ); + + assert.ok( + execStub.calledOnceWithExactly( + "docker --log-level=error compose --progress=quiet config --format=json", + { + cwd: workspaceFolder.uri.fsPath, + shell, + // eslint-disable-next-line no-process-env + env: process.env, + }, + ), + ); + + assert.ok(stdinData.join("\n").includes(compose.activationScript)); + + assert.strictEqual(version, "3.0.0"); + assert.strictEqual(yjit, true); + }); +}); diff --git a/vscode/src/test/suite/ruby/custom.test.ts b/vscode/src/test/suite/ruby/custom.test.ts index 5e1482d1cc..925a4a9087 100644 --- a/vscode/src/test/suite/ruby/custom.test.ts +++ b/vscode/src/test/suite/ruby/custom.test.ts @@ -10,8 +10,16 @@ import { Custom } from "../../../ruby/custom"; import { WorkspaceChannel } from "../../../workspaceChannel"; import * as common from "../../../common"; import { ACTIVATION_SEPARATOR } from "../../../ruby/versionManager"; +import { createSpawnStub } from "../testHelpers"; suite("Custom", () => { + let spawnStub: sinon.SinonStub; + let stdinData: string[]; + + teardown(() => { + spawnStub?.restore(); + }); + test("Invokes custom script and then Ruby", async () => { const workspacePath = fs.mkdtempSync( path.join(os.tmpdir(), "ruby-lsp-test-"), @@ -31,10 +39,9 @@ suite("Custom", () => { version: "3.0.0", }; - const execStub = sinon.stub(common, "asyncExec").resolves({ - stdout: "", + ({ spawnStub, stdinData } = createSpawnStub({ stderr: `${ACTIVATION_SEPARATOR}${JSON.stringify(envStub)}${ACTIVATION_SEPARATOR}`, - }); + })); const commandStub = sinon .stub(custom, "customCommand") @@ -45,8 +52,9 @@ suite("Custom", () => { const shell = os.platform() === "win32" ? undefined : vscode.env.shell; assert.ok( - execStub.calledOnceWithExactly( - `my_version_manager activate_env && ruby -W0 -rjson -e '${custom.activationScript}'`, + spawnStub.calledOnceWithExactly( + "my_version_manager", + ["activate_env", "&&", "ruby", "-W0", "-rjson"], { cwd: uri.fsPath, shell, @@ -56,11 +64,12 @@ suite("Custom", () => { ), ); + assert.ok(stdinData.join("\n").includes(custom.activationScript)); + assert.strictEqual(version, "3.0.0"); assert.strictEqual(yjit, true); assert.deepStrictEqual(env.ANY, "true"); - execStub.restore(); commandStub.restore(); fs.rmSync(workspacePath, { recursive: true, force: true }); }); diff --git a/vscode/src/test/suite/ruby/mise.test.ts b/vscode/src/test/suite/ruby/mise.test.ts index 5d31fa2442..c790d85f24 100644 --- a/vscode/src/test/suite/ruby/mise.test.ts +++ b/vscode/src/test/suite/ruby/mise.test.ts @@ -10,6 +10,7 @@ import { Mise } from "../../../ruby/mise"; import { WorkspaceChannel } from "../../../workspaceChannel"; import * as common from "../../../common"; import { ACTIVATION_SEPARATOR } from "../../../ruby/versionManager"; +import { createSpawnStub } from "../testHelpers"; suite("Mise", () => { if (os.platform() === "win32") { @@ -18,6 +19,13 @@ suite("Mise", () => { return; } + let spawnStub: sinon.SinonStub; + let stdinData: string[]; + + teardown(() => { + spawnStub?.restore(); + }); + test("Finds Ruby only binary path is appended to PATH", async () => { // eslint-disable-next-line no-process-env const workspacePath = process.env.PWD!; @@ -35,10 +43,10 @@ suite("Mise", () => { version: "3.0.0", }; - const execStub = sinon.stub(common, "asyncExec").resolves({ - stdout: "", + ({ spawnStub, stdinData } = createSpawnStub({ stderr: `${ACTIVATION_SEPARATOR}${JSON.stringify(envStub)}${ACTIVATION_SEPARATOR}`, - }); + })); + const findStub = sinon .stub(mise, "findMiseUri") .resolves( @@ -53,8 +61,9 @@ suite("Mise", () => { const { env, version, yjit } = await mise.activate(); assert.ok( - execStub.calledOnceWithExactly( - `${os.homedir()}/.local/bin/mise x -- ruby -W0 -rjson -e '${mise.activationScript}'`, + spawnStub.calledOnceWithExactly( + `${os.homedir()}/.local/bin/mise`, + ["x", "--", "ruby", "-W0", "-rjson"], { cwd: workspacePath, shell: vscode.env.shell, @@ -64,11 +73,12 @@ suite("Mise", () => { ), ); + assert.ok(stdinData.join("\n").includes(mise.activationScript)); + assert.strictEqual(version, "3.0.0"); assert.strictEqual(yjit, true); assert.deepStrictEqual(env.ANY, "true"); - execStub.restore(); findStub.restore(); }); @@ -90,10 +100,9 @@ suite("Mise", () => { version: "3.0.0", }; - const execStub = sinon.stub(common, "asyncExec").resolves({ - stdout: "", + ({ spawnStub, stdinData } = createSpawnStub({ stderr: `${ACTIVATION_SEPARATOR}${JSON.stringify(envStub)}${ACTIVATION_SEPARATOR}`, - }); + })); const misePath = path.join(workspacePath, "mise"); fs.writeFileSync(misePath, "fakeMiseBinary"); @@ -112,8 +121,9 @@ suite("Mise", () => { const { env, version, yjit } = await mise.activate(); assert.ok( - execStub.calledOnceWithExactly( - `${misePath} x -- ruby -W0 -rjson -e '${mise.activationScript}'`, + spawnStub.calledOnceWithExactly( + misePath, + ["x", "--", "ruby", "-W0", "-rjson"], { cwd: workspacePath, shell: vscode.env.shell, @@ -123,11 +133,12 @@ suite("Mise", () => { ), ); + assert.ok(stdinData.join("\n").includes(mise.activationScript)); + assert.strictEqual(version, "3.0.0"); assert.strictEqual(yjit, true); assert.deepStrictEqual(env.ANY, "true"); - execStub.restore(); configStub.restore(); fs.rmSync(workspacePath, { recursive: true, force: true }); }); diff --git a/vscode/src/test/suite/ruby/none.test.ts b/vscode/src/test/suite/ruby/none.test.ts index f42fbaf648..c7edf4e3a5 100644 --- a/vscode/src/test/suite/ruby/none.test.ts +++ b/vscode/src/test/suite/ruby/none.test.ts @@ -4,14 +4,21 @@ import fs from "fs"; import os from "os"; import * as vscode from "vscode"; -import sinon from "sinon"; import { None } from "../../../ruby/none"; import { WorkspaceChannel } from "../../../workspaceChannel"; import * as common from "../../../common"; import { ACTIVATION_SEPARATOR } from "../../../ruby/versionManager"; +import { createSpawnStub } from "../testHelpers"; suite("None", () => { + let spawnStub: sinon.SinonStub; + let stdinData: string[]; + + teardown(() => { + spawnStub?.restore(); + }); + test("Invokes Ruby directly", async () => { const workspacePath = fs.mkdtempSync( path.join(os.tmpdir(), "ruby-lsp-test-"), @@ -31,10 +38,9 @@ suite("None", () => { version: "3.0.0", }; - const execStub = sinon.stub(common, "asyncExec").resolves({ - stdout: "", + ({ spawnStub, stdinData } = createSpawnStub({ stderr: `${ACTIVATION_SEPARATOR}${JSON.stringify(envStub)}${ACTIVATION_SEPARATOR}`, - }); + })); const { env, version, yjit } = await none.activate(); @@ -42,22 +48,20 @@ suite("None", () => { const shell = os.platform() === "win32" ? undefined : vscode.env.shell; assert.ok( - execStub.calledOnceWithExactly( - `ruby -W0 -rjson -e '${none.activationScript}'`, - { - cwd: uri.fsPath, - shell, - // eslint-disable-next-line no-process-env - env: process.env, - }, - ), + spawnStub.calledOnceWithExactly("ruby", ["-W0", "-rjson"], { + cwd: uri.fsPath, + shell, + // eslint-disable-next-line no-process-env + env: process.env, + }), ); + assert.ok(stdinData.join("\n").includes(none.activationScript)); + assert.strictEqual(version, "3.0.0"); assert.strictEqual(yjit, true); assert.deepStrictEqual(env.ANY, "true"); - execStub.restore(); fs.rmSync(workspacePath, { recursive: true, force: true }); }); }); diff --git a/vscode/src/test/suite/ruby/rbenv.test.ts b/vscode/src/test/suite/ruby/rbenv.test.ts index 1463c2f3ec..816b629788 100644 --- a/vscode/src/test/suite/ruby/rbenv.test.ts +++ b/vscode/src/test/suite/ruby/rbenv.test.ts @@ -10,6 +10,7 @@ import { Rbenv } from "../../../ruby/rbenv"; import { WorkspaceChannel } from "../../../workspaceChannel"; import * as common from "../../../common"; import { ACTIVATION_SEPARATOR } from "../../../ruby/versionManager"; +import { createSpawnStub } from "../testHelpers"; suite("Rbenv", () => { if (os.platform() === "win32") { @@ -18,6 +19,13 @@ suite("Rbenv", () => { return; } + let spawnStub: sinon.SinonStub; + let stdinData: string[]; + + teardown(() => { + spawnStub?.restore(); + }); + test("Finds Ruby based on .ruby-version", async () => { // eslint-disable-next-line no-process-env const workspacePath = process.env.PWD!; @@ -35,16 +43,16 @@ suite("Rbenv", () => { version: "3.0.0", }; - const execStub = sinon.stub(common, "asyncExec").resolves({ - stdout: "", + ({ spawnStub, stdinData } = createSpawnStub({ stderr: `${ACTIVATION_SEPARATOR}${JSON.stringify(envStub)}${ACTIVATION_SEPARATOR}`, - }); + })); const { env, version, yjit } = await rbenv.activate(); assert.ok( - execStub.calledOnceWithExactly( - `rbenv exec ruby -W0 -rjson -e '${rbenv.activationScript}'`, + spawnStub.calledOnceWithExactly( + "rbenv", + ["exec", "ruby", "-W0", "-rjson"], { cwd: workspacePath, shell: vscode.env.shell, @@ -54,10 +62,11 @@ suite("Rbenv", () => { ), ); + assert.ok(stdinData.join("\n").includes(rbenv.activationScript)); + assert.strictEqual(version, "3.0.0"); assert.strictEqual(yjit, true); assert.strictEqual(env.ANY, "true"); - execStub.restore(); }); test("Allows configuring where rbenv is installed", async () => { @@ -78,10 +87,9 @@ suite("Rbenv", () => { version: "3.0.0", }; - const execStub = sinon.stub(common, "asyncExec").resolves({ - stdout: "", + ({ spawnStub, stdinData } = createSpawnStub({ stderr: `${ACTIVATION_SEPARATOR}${JSON.stringify(envStub)}${ACTIVATION_SEPARATOR}`, - }); + })); const rbenvPath = path.join(workspacePath, "rbenv"); fs.writeFileSync(rbenvPath, "fakeRbenvBinary"); @@ -100,8 +108,9 @@ suite("Rbenv", () => { const { env, version, yjit } = await rbenv.activate(); assert.ok( - execStub.calledOnceWithExactly( - `${rbenvPath} exec ruby -W0 -rjson -e '${rbenv.activationScript}'`, + spawnStub.calledOnceWithExactly( + rbenvPath, + ["exec", "ruby", "-W0", "-rjson"], { cwd: workspacePath, shell: vscode.env.shell, @@ -111,11 +120,12 @@ suite("Rbenv", () => { ), ); + assert.ok(stdinData.join("\n").includes(rbenv.activationScript)); + assert.strictEqual(version, "3.0.0"); assert.strictEqual(yjit, true); assert.deepStrictEqual(env.ANY, "true"); - execStub.restore(); configStub.restore(); fs.rmSync(workspacePath, { recursive: true, force: true }); }); @@ -131,10 +141,9 @@ suite("Rbenv", () => { const outputChannel = new WorkspaceChannel("fake", common.LOG_CHANNEL); const rbenv = new Rbenv(workspaceFolder, outputChannel, async () => {}); - const execStub = sinon.stub(common, "asyncExec").resolves({ - stdout: "", + ({ spawnStub, stdinData } = createSpawnStub({ stderr: `${ACTIVATION_SEPARATOR}not a json${ACTIVATION_SEPARATOR}`, - }); + })); const errorStub = sinon.stub(outputChannel, "error"); @@ -144,8 +153,9 @@ suite("Rbenv", () => { ); assert.ok( - execStub.calledOnceWithExactly( - `rbenv exec ruby -W0 -rjson -e '${rbenv.activationScript}'`, + spawnStub.calledOnceWithExactly( + "rbenv", + ["exec", "ruby", "-W0", "-rjson"], { cwd: workspacePath, shell: vscode.env.shell, @@ -155,13 +165,14 @@ suite("Rbenv", () => { ), ); + assert.ok(stdinData.join("\n").includes(rbenv.activationScript)); + assert.ok( errorStub.calledOnceWithExactly( "Tried parsing invalid JSON environment: not a json", ), ); - execStub.restore(); errorStub.restore(); }); }); diff --git a/vscode/src/test/suite/ruby/rvm.test.ts b/vscode/src/test/suite/ruby/rvm.test.ts index a1ff3c5b4a..475860d60a 100644 --- a/vscode/src/test/suite/ruby/rvm.test.ts +++ b/vscode/src/test/suite/ruby/rvm.test.ts @@ -10,6 +10,7 @@ import { Rvm } from "../../../ruby/rvm"; import { WorkspaceChannel } from "../../../workspaceChannel"; import * as common from "../../../common"; import { ACTIVATION_SEPARATOR } from "../../../ruby/versionManager"; +import { createSpawnStub } from "../testHelpers"; suite("RVM", () => { if (os.platform() === "win32") { @@ -18,6 +19,13 @@ suite("RVM", () => { return; } + let spawnStub: sinon.SinonStub; + let stdinData: string[]; + + teardown(() => { + spawnStub?.restore(); + }); + test("Populates the gem env and path", async () => { const workspacePath = process.env.PWD!; const workspaceFolder = { @@ -47,16 +55,16 @@ suite("RVM", () => { version: "3.0.0", }; - const execStub = sinon.stub(common, "asyncExec").resolves({ - stdout: "", + ({ spawnStub, stdinData } = createSpawnStub({ stderr: `${ACTIVATION_SEPARATOR}${JSON.stringify(envStub)}${ACTIVATION_SEPARATOR}`, - }); + })); const { env, version, yjit } = await rvm.activate(); assert.ok( - execStub.calledOnceWithExactly( - `${path.join(os.homedir(), ".rvm", "bin", "rvm-auto-ruby")} -W0 -rjson -e '${rvm.activationScript}'`, + spawnStub.calledOnceWithExactly( + path.join(os.homedir(), ".rvm", "bin", "rvm-auto-ruby"), + ["-W0", "-rjson"], { cwd: workspacePath, shell: vscode.env.shell, @@ -65,11 +73,12 @@ suite("RVM", () => { ), ); + assert.ok(stdinData.join("\n").includes(rvm.activationScript)); + assert.strictEqual(version, "3.0.0"); assert.strictEqual(yjit, true); assert.deepStrictEqual(env.ANY, "true"); - execStub.restore(); installationPathStub.restore(); }); }); diff --git a/vscode/src/test/suite/ruby/shadowenv.test.ts b/vscode/src/test/suite/ruby/shadowenv.test.ts index 1bd22d260a..e5c7b64caa 100644 --- a/vscode/src/test/suite/ruby/shadowenv.test.ts +++ b/vscode/src/test/suite/ruby/shadowenv.test.ts @@ -14,6 +14,7 @@ import { WorkspaceChannel } from "../../../workspaceChannel"; import { LOG_CHANNEL, asyncExec } from "../../../common"; import { RUBY_VERSION } from "../../rubyVersion"; import * as common from "../../../common"; +import { createSpawnStub } from "../testHelpers"; suite("Shadowenv", () => { if (os.platform() === "win32") { @@ -35,6 +36,8 @@ suite("Shadowenv", () => { let workspaceFolder: vscode.WorkspaceFolder; let outputChannel: WorkspaceChannel; let rubyBinPath: string; + let spawnStub: sinon.SinonStub; + const [major, minor, patch] = RUBY_VERSION.split("."); if (process.env.CI && os.platform() === "linux") { @@ -120,6 +123,8 @@ suite("Shadowenv", () => { afterEach(() => { fs.rmSync(rootPath, { recursive: true, force: true }); + + spawnStub?.restore(); }); test("Finds Ruby only binary path is appended to PATH", async () => { @@ -235,12 +240,13 @@ suite("Shadowenv", () => { async () => {}, ); - // First, reject the call to `shadowenv exec`. Then resolve the call to `which shadowenv` to return nothing + // First, reject the call to `shadowenv exec`. + ({ spawnStub } = createSpawnStub()); + spawnStub.rejects(new Error("shadowenv: command not found")); + + // Then reject the call to `shadowenv --version` const execStub = sinon .stub(common, "asyncExec") - .onFirstCall() - .rejects(new Error("shadowenv: command not found")) - .onSecondCall() .rejects(new Error("shadowenv: command not found")); await assert.rejects(async () => { diff --git a/vscode/src/test/suite/testHelpers.ts b/vscode/src/test/suite/testHelpers.ts new file mode 100644 index 0000000000..65d7cf4f13 --- /dev/null +++ b/vscode/src/test/suite/testHelpers.ts @@ -0,0 +1,59 @@ +import { EventEmitter, Readable, Writable } from "stream"; +import { SpawnOptions } from "child_process"; + +import sinon from "sinon"; + +import * as common from "../../common"; + +interface SpawnStubOptions { + stdout?: string; + stderr?: string; + exitCode?: number; +} + +export function createSpawnStub({ + stdout = "", + stderr = "", + exitCode = 0, +}: SpawnStubOptions = {}): { spawnStub: sinon.SinonStub; stdinData: string[] } { + const stdinData: string[] = []; + + const spawnStub = sinon + .stub(common, "spawn") + .callsFake( + ( + _command: string, + _args: ReadonlyArray, + _options: SpawnOptions, + ) => { + const childProcess = new EventEmitter() as any; + + childProcess.stdout = new Readable({ + read() {}, + }); + childProcess.stderr = new Readable({ + read() {}, + }); + childProcess.stdin = new Writable({ + write(chunk, _encoding, callback) { + stdinData.push(chunk.toString()); + callback(); + }, + final(callback) { + process.nextTick(() => { + childProcess.stdout.emit("data", stdout); + childProcess.stderr.emit("data", stderr); + + childProcess.emit("close", exitCode); + }); + + callback(); + }, + }); + + return childProcess; + }, + ); + + return { spawnStub, stdinData }; +} diff --git a/vscode/src/workspace.ts b/vscode/src/workspace.ts index 5fcf552201..e6efdbaa5b 100644 --- a/vscode/src/workspace.ts +++ b/vscode/src/workspace.ts @@ -6,7 +6,6 @@ import { CodeLens, State } from "vscode-languageclient/node"; import { Ruby } from "./ruby"; import Client from "./client"; import { - asyncExec, LOG_CHANNEL, WorkspaceInterface, STATUS_EMITTER, @@ -268,16 +267,19 @@ export class Workspace implements WorkspaceInterface { "sorbet-runtime", ]; - const { stdout } = await asyncExec(`gem list ${dependencies.join(" ")}`, { - cwd: this.workspaceFolder.uri.fsPath, - env: this.ruby.env, - }); + const { stdout } = await this.ruby.runActivatedScript( + `gem list ${dependencies.join(" ")}`, + { + cwd: this.workspaceFolder.uri.fsPath, + env: this.ruby.env, + }, + ); // If any of the Ruby LSP's dependencies are missing, we need to install them. For example, if the user runs `gem // uninstall prism`, then we must ensure it's installed or else rubygems will fail when trying to launch the // executable if (!dependencies.every((dep) => new RegExp(`${dep}\\s`).exec(stdout))) { - await asyncExec("gem install ruby-lsp", { + await this.ruby.runActivatedScript("gem install ruby-lsp", { cwd: this.workspaceFolder.uri.fsPath, env: this.ruby.env, }); @@ -311,7 +313,7 @@ export class Workspace implements WorkspaceInterface { Date.now() - lastUpdatedAt > oneDayInMs ) { try { - await asyncExec("gem update ruby-lsp", { + await this.ruby.runActivatedScript("gem update ruby-lsp", { cwd: this.workspaceFolder.uri.fsPath, env: this.ruby.env, }); @@ -347,7 +349,7 @@ export class Workspace implements WorkspaceInterface { this.outputChannel.info(`Running "${command}"`); } - const result = await asyncExec(command, { + const result = await this.ruby.runActivatedScript(command, { env: this.ruby.env, cwd: this.workspaceFolder.uri.fsPath, }); From a3a59304cd28d31c483f2da4c3082eaacef74369 Mon Sep 17 00:00:00 2001 From: Dmitry Lebed <209805+d-lebed@users.noreply.github.com> Date: Thu, 28 Nov 2024 18:01:32 +0100 Subject: [PATCH 2/4] Made changes more conservative --- vscode/src/client.ts | 2 +- vscode/src/common.ts | 43 +++++ vscode/src/ruby.ts | 54 ++++--- vscode/src/ruby/compose.ts | 159 +++++++++++++++---- vscode/src/ruby/rubyInstaller.ts | 25 +-- vscode/src/ruby/versionManager.ts | 154 ++---------------- vscode/src/rubyLsp.ts | 2 +- vscode/src/test/suite/ruby.test.ts | 7 +- vscode/src/test/suite/ruby/asdf.test.ts | 50 ++---- vscode/src/test/suite/ruby/custom.test.ts | 21 +-- vscode/src/test/suite/ruby/mise.test.ts | 35 ++-- vscode/src/test/suite/ruby/none.test.ts | 32 ++-- vscode/src/test/suite/ruby/rbenv.test.ts | 47 +++--- vscode/src/test/suite/ruby/rvm.test.ts | 21 +-- vscode/src/test/suite/ruby/shadowenv.test.ts | 14 +- 15 files changed, 316 insertions(+), 350 deletions(-) diff --git a/vscode/src/client.ts b/vscode/src/client.ts index 812a37f78e..9a4b84db57 100644 --- a/vscode/src/client.ts +++ b/vscode/src/client.ts @@ -278,7 +278,7 @@ function collectClientOptions( }); } - outputChannel.info( + outputChannel.debug( `Document Selector Paths: ${JSON.stringify(documentSelector)}`, ); diff --git a/vscode/src/common.ts b/vscode/src/common.ts index 4e9056da41..ac0d61d6c4 100644 --- a/vscode/src/common.ts +++ b/vscode/src/common.ts @@ -4,6 +4,7 @@ import { promisify } from "util"; import * as vscode from "vscode"; import { State } from "vscode-languageclient"; +import { Executable } from "vscode-languageclient/node"; export enum Command { Start = "rubyLsp.start", @@ -72,6 +73,22 @@ export interface PathConverterInterface { toRemoteUri: (localUri: vscode.Uri) => vscode.Uri; } +export class PathConverter implements PathConverterInterface { + readonly pathMapping: [string, string][] = []; + + toRemotePath(path: string) { + return path; + } + + toLocalPath(path: string) { + return path; + } + + toRemoteUri(localUri: vscode.Uri) { + return localUri; + } +} + // Event emitter used to signal that the language status items need to be refreshed export const STATUS_EMITTER = new vscode.EventEmitter< WorkspaceInterface | undefined @@ -155,3 +172,29 @@ export function featureEnabled(feature: keyof typeof FEATURE_FLAGS): boolean { // If that number is below the percentage, then the feature is enabled for this user return hashNum < percentage; } + +export function parseCommand(commandString: string): Executable { + // Regular expression to split arguments while respecting quotes + const regex = /(?:[^\s"']+|"[^"]*"|'[^']*')+/g; + + const parts = + commandString.match(regex)?.map((arg) => { + // Remove surrounding quotes, if any + return arg.replace(/^['"]|['"]$/g, ""); + }) ?? []; + + // Extract environment variables + const env: Record = {}; + while (parts[0] && parts[0].includes("=")) { + const [key, value] = parts.shift()?.split("=") ?? []; + if (key) { + env[key] = value || ""; + } + } + + // The first part is the command, the rest are arguments + const command = parts.shift() || ""; + const args = parts; + + return { command, args, options: { env } }; +} diff --git a/vscode/src/ruby.ts b/vscode/src/ruby.ts index 767b7bf46a..0f6e6f4600 100644 --- a/vscode/src/ruby.ts +++ b/vscode/src/ruby.ts @@ -1,16 +1,21 @@ /* eslint-disable no-process-env */ import path from "path"; import os from "os"; -import { ExecOptions } from "child_process"; import * as vscode from "vscode"; -import { Executable } from "vscode-languageclient/node"; - -import { asyncExec, PathConverterInterface, RubyInterface } from "./common"; +import { Executable, ExecutableOptions } from "vscode-languageclient/node"; + +import { + asyncExec, + parseCommand, + PathConverter, + PathConverterInterface, + RubyInterface, +} from "./common"; import { WorkspaceChannel } from "./workspaceChannel"; import { Shadowenv } from "./ruby/shadowenv"; import { Chruby } from "./ruby/chruby"; -import { PathConverter, VersionManager } from "./ruby/versionManager"; +import { VersionManager } from "./ruby/versionManager"; import { Mise } from "./ruby/mise"; import { RubyInstaller } from "./ruby/rubyInstaller"; import { Rbenv } from "./ruby/rbenv"; @@ -69,9 +74,9 @@ export class Ruby implements RubyInterface { private readonly shell = process.env.SHELL?.replace(/(\s+)/g, "\\$1"); private _env: NodeJS.ProcessEnv = {}; - private _manager?: VersionManager; - private _pathConverter: PathConverterInterface = new PathConverter(); private _error = false; + private _pathConverter: PathConverterInterface; + private _wrapCommand: (executable: Executable) => Executable; private readonly context: vscode.ExtensionContext; private readonly customBundleGemfile?: string; private readonly outputChannel: WorkspaceChannel; @@ -87,6 +92,8 @@ export class Ruby implements RubyInterface { this.workspaceFolder = workspaceFolder; this.outputChannel = outputChannel; this.telemetry = telemetry; + this._pathConverter = new PathConverter(); + this._wrapCommand = (executable: Executable) => executable; const customBundleGemfile: string = vscode.workspace .getConfiguration("rubyLsp") @@ -119,10 +126,6 @@ export class Ruby implements RubyInterface { return this._pathConverter; } - set pathConverter(pathConverter: PathConverterInterface) { - this._pathConverter = pathConverter; - } - get env() { return this._env; } @@ -203,12 +206,22 @@ export class Ruby implements RubyInterface { } } - runActivatedScript(command: string, options: ExecOptions = {}) { - return this._manager!.runActivatedScript(command, options); + runActivatedScript(script: string, options: ExecutableOptions = {}) { + const parsedExecutable = parseCommand(script); + const executable = this.activateExecutable({ + ...parsedExecutable, + options, + }); + const command = [executable.command, ...(executable.args || [])].join(" "); + + return asyncExec(command, { + cwd: this.workspaceFolder.uri.fsPath || executable.options?.cwd, + env: { ...process.env, ...executable.options?.env }, + }); } activateExecutable(executable: Executable) { - return this._manager!.activateExecutable(executable); + return this._wrapCommand(executable); } async manuallySelectRuby() { @@ -263,20 +276,25 @@ export class Ruby implements RubyInterface { } private async runActivation(manager: VersionManager) { - const { env, version, yjit, gemPath } = await manager.activate(); + const { env, version, yjit, gemPath, pathConverter, wrapCommand } = + await manager.activate(); const [major, minor, _patch] = version.split(".").map(Number); this.sanitizeEnvironment(env); - this.pathConverter = await manager.buildPathConverter(this.workspaceFolder); - // We need to set the process environment too to make other extensions such as Sorbet find the right Ruby paths process.env = env; this._env = env; - this._manager = manager; this.rubyVersion = version; this.yjitEnabled = (yjit && major > 3) || (major === 3 && minor >= 2); this.gemPath.push(...gemPath); + + if (pathConverter) { + this._pathConverter = pathConverter; + } + if (wrapCommand) { + this._wrapCommand = wrapCommand; + } } // Fetch information related to the Ruby version. This can only be invoked after activation, so that `rubyVersion` is diff --git a/vscode/src/ruby/compose.ts b/vscode/src/ruby/compose.ts index 6996942d79..53a5b81200 100644 --- a/vscode/src/ruby/compose.ts +++ b/vscode/src/ruby/compose.ts @@ -1,6 +1,8 @@ /* eslint-disable no-process-env */ -import { ExecOptions } from "child_process"; import path from "path"; +import os from "os"; +import { StringDecoder } from "string_decoder"; +import { ExecOptions } from "child_process"; import * as vscode from "vscode"; import { Executable } from "vscode-languageclient/node"; @@ -10,8 +12,13 @@ import { ContainerPathConverter, fetchPathMapping, } from "../docker"; +import { parseCommand, spawn } from "../common"; -import { VersionManager, ActivationResult } from "./versionManager"; +import { + VersionManager, + ActivationResult, + ACTIVATION_SEPARATOR, +} from "./versionManager"; // Compose // @@ -24,52 +31,63 @@ export class Compose extends VersionManager { async activate(): Promise { await this.ensureConfigured(); - const parsedResult = await this.runEnvActivationScript( - `${this.composeRunCommand()} ${this.composeServiceName()} ruby`, + const rubyCommand = `${this.composeRunCommand()} ${this.composeServiceName()} ruby -W0 -rjson`; + const { stderr: output } = await this.runRubyCode( + rubyCommand, + this.activationScript, ); + this.outputChannel.debug(`Activation output: ${output}`); + + const activationContent = new RegExp( + `${ACTIVATION_SEPARATOR}(.*)${ACTIVATION_SEPARATOR}`, + ).exec(output); + + const parsedResult = this.parseWithErrorHandling(activationContent![1]); + const pathConverter = await this.buildPathConverter(); + + const wrapCommand = (executable: Executable) => { + const composeCommad = parseCommand( + `${this.composeRunCommand()} ${this.composeServiceName()}`, + ); + + const command = { + command: composeCommad.command, + args: [ + ...(composeCommad.args ?? []), + executable.command, + ...(executable.args ?? []), + ], + options: { + ...executable.options, + env: { + ...executable.options?.env, + ...composeCommad.options?.env, + }, + }, + }; + + return command; + }; + return { env: { ...process.env }, yjit: parsedResult.yjit, version: parsedResult.version, gemPath: parsedResult.gemPath, + pathConverter, + wrapCommand, }; } - runActivatedScript(command: string, options: ExecOptions = {}) { - return this.runScript( - `${this.composeRunCommand()} ${this.composeServiceName()} ${command}`, - options, - ); - } - - activateExecutable(executable: Executable) { - const composeCommand = this.parseCommand( - `${this.composeRunCommand()} ${this.composeServiceName()}`, - ); - - return { - command: composeCommand.command, - args: [ - ...composeCommand.args, - executable.command, - ...(executable.args || []), - ], - options: { - ...executable.options, - env: { ...(executable.options?.env || {}), ...composeCommand.env }, - }, - }; - } - - async buildPathConverter(workspaceFolder: vscode.WorkspaceFolder) { + protected async buildPathConverter() { const pathMapping = fetchPathMapping( this.composeConfig, this.composeServiceName(), ); const stats = Object.entries(pathMapping).map(([local, remote]) => { - const absolute = path.resolve(workspaceFolder.uri.fsPath, local); + const absolute = path.resolve(this.workspaceFolder.uri.fsPath, local); return vscode.workspace.fs.stat(vscode.Uri.file(absolute)).then( (stat) => ({ stat, local, remote, absolute }), () => ({ stat: undefined, local, remote, absolute }), @@ -162,6 +180,83 @@ export class Compose extends VersionManager { }); } + protected runRubyCode( + rubyCommand: string, + code: string, + ): Promise<{ stdout: string; stderr: string }> { + return new Promise((resolve, reject) => { + this.outputChannel.info( + `Ruby \`${rubyCommand}\` running Ruby code: \`${code}\``, + ); + + const { + command, + args, + options: { env } = { env: {} }, + } = parseCommand(rubyCommand); + const ruby = spawn(command, args, this.execOptions({ env })); + + let stdout = ""; + let stderr = ""; + + const stdoutDecoder = new StringDecoder("utf-8"); + const stderrDecoder = new StringDecoder("utf-8"); + + ruby.stdout.on("data", (data) => { + stdout += stdoutDecoder.write(data); + + if (stdout.includes("END_OF_RUBY_CODE_OUTPUT")) { + stdout = stdout.replace(/END_OF_RUBY_CODE_OUTPUT.*/s, ""); + resolve({ stdout, stderr }); + } + }); + ruby.stderr.on("data", (data) => { + stderr += stderrDecoder.write(data); + }); + ruby.on("error", (error) => { + reject(error); + }); + ruby.on("close", (status) => { + if (status) { + reject(new Error(`Process exited with status ${status}: ${stderr}`)); + } else { + resolve({ stdout, stderr }); + } + }); + + const script = [ + "begin", + ...code.split("\n").map((line) => ` ${line}`), + "ensure", + ' puts "END_OF_RUBY_CODE_OUTPUT"', + "end", + ].join("\n"); + + this.outputChannel.info(`Running Ruby code:\n${script}`); + + ruby.stdin.write(script); + ruby.stdin.end(); + }); + } + + protected execOptions(options: ExecOptions = {}): ExecOptions { + let shell: string | undefined; + + // If the user has configured a default shell, we use that one since they are probably sourcing their version + // manager scripts in that shell's configuration files. On Windows, we never set the shell no matter what to ensure + // that activation runs on `cmd.exe` and not PowerShell, which avoids complex quoting and escaping issues. + if (vscode.env.shell.length > 0 && os.platform() !== "win32") { + shell = vscode.env.shell; + } + + return { + cwd: this.bundleUri.fsPath, + shell, + ...options, + env: { ...process.env, ...options.env }, + }; + } + private async getComposeConfig(): Promise { try { const { stdout, stderr: _stderr } = await this.runScript( diff --git a/vscode/src/ruby/rubyInstaller.ts b/vscode/src/ruby/rubyInstaller.ts index 7bab203e0b..33f4912ab7 100644 --- a/vscode/src/ruby/rubyInstaller.ts +++ b/vscode/src/ruby/rubyInstaller.ts @@ -1,9 +1,10 @@ /* eslint-disable no-process-env */ import os from "os"; -import { ExecOptions } from "child_process"; import * as vscode from "vscode"; +import { asyncExec } from "../common"; + import { Chruby } from "./chruby"; interface RubyVersion { @@ -54,14 +55,20 @@ export class RubyInstaller extends Chruby { ); } - // Override the `execOptions` method to ensure that we do not pass any `shell` to `asyncExec`. The activation script - // is only compatible with `cmd.exe`, and not Powershell, due to escaping of quotes. We need to ensure to always run - // the script on `cmd.exe`. - protected execOptions(options: ExecOptions = {}): ExecOptions { - return { + // Override the `runScript` method to ensure that we do not pass any `shell` to `asyncExec`. The activation script is + // only compatible with `cmd.exe`, and not Powershell, due to escaping of quotes. We need to ensure to always run the + // script on `cmd.exe`. + protected runScript(command: string) { + this.outputChannel.info( + `Running command: \`${command}\` in ${this.bundleUri.fsPath}`, + ); + this.outputChannel.debug( + `Environment used for command: ${JSON.stringify(process.env)}`, + ); + + return asyncExec(command, { cwd: this.bundleUri.fsPath, - ...options, - env: { ...process.env, ...options.env }, - }; + env: process.env, + }); } } diff --git a/vscode/src/ruby/versionManager.ts b/vscode/src/ruby/versionManager.ts index 85f6b3e8b5..e4c4372d35 100644 --- a/vscode/src/ruby/versionManager.ts +++ b/vscode/src/ruby/versionManager.ts @@ -1,39 +1,24 @@ /* eslint-disable no-process-env */ import path from "path"; import os from "os"; -import { ExecOptions } from "child_process"; import * as vscode from "vscode"; import { Executable } from "vscode-languageclient/node"; import { WorkspaceChannel } from "../workspaceChannel"; -import { asyncExec, PathConverterInterface, spawn } from "../common"; +import { asyncExec, PathConverterInterface } from "../common"; export interface ActivationResult { env: NodeJS.ProcessEnv; yjit: boolean; version: string; gemPath: string[]; + pathConverter?: PathConverterInterface; + wrapCommand?: (executable: Executable) => Executable; } export const ACTIVATION_SEPARATOR = "RUBY_LSP_ACTIVATION_SEPARATOR"; -export class PathConverter implements PathConverterInterface { - readonly pathMapping: [string, string][] = []; - - toRemotePath(path: string) { - return path; - } - - toLocalPath(path: string) { - return path; - } - - toRemoteUri(localUri: vscode.Uri) { - return localUri; - } -} - export abstract class VersionManager { public activationScript = [ `STDERR.print("${ACTIVATION_SEPARATOR}" + `, @@ -77,26 +62,9 @@ export abstract class VersionManager { // language server abstract activate(): Promise; - runActivatedScript(command: string, options: ExecOptions = {}) { - return this.runScript(command, options); - } - - activateExecutable(executable: Executable) { - return executable; - } - - async buildPathConverter(_workspaceFolder: vscode.WorkspaceFolder) { - return new PathConverter(); - } - protected async runEnvActivationScript(activatedRuby: string) { - const result = await this.runRubyCode( - `${activatedRuby} -W0 -rjson`, - this.activationScript, - ); - - this.outputChannel.debug( - `Activation script output: ${JSON.stringify(result, null, 2)}`, + const result = await this.runScript( + `${activatedRuby} -W0 -rjson -e '${this.activationScript}'`, ); const activationContent = new RegExp( @@ -120,72 +88,7 @@ export abstract class VersionManager { // Runs the given command in the directory for the Bundle, using the user's preferred shell and inheriting the current // process environment - protected runScript(command: string, options: ExecOptions = {}) { - const execOptions = this.execOptions(options); - - this.outputChannel.info( - `Running command: \`${command}\` in ${execOptions.cwd} using shell: ${execOptions.shell}`, - ); - this.outputChannel.debug( - `Environment used for command: ${JSON.stringify(execOptions.env)}`, - ); - - return asyncExec(command, execOptions); - } - - protected runRubyCode( - rubyCommand: string, - code: string, - ): Promise<{ stdout: string; stderr: string }> { - return new Promise((resolve, reject) => { - this.outputChannel.info( - `Ruby \`${rubyCommand}\` running Ruby code: \`${code}\``, - ); - - const { command, args, env } = this.parseCommand(rubyCommand); - const ruby = spawn(command, args, this.execOptions({ env })); - - let stdout = ""; - let stderr = ""; - - ruby.stdout.on("data", (data) => { - this.outputChannel.debug(`stdout: '${data.toString()}'`); - if (data.toString().includes("END_OF_RUBY_CODE_OUTPUT")) { - stdout += data.toString().replace(/END_OF_RUBY_CODE_OUTPUT.*/s, ""); - resolve({ stdout, stderr }); - } else { - stdout += data.toString(); - } - }); - ruby.stderr.on("data", (data) => { - this.outputChannel.debug(`stderr: '${data.toString()}'`); - stderr += data.toString(); - }); - ruby.on("error", (error) => { - reject(error); - }); - ruby.on("close", (status) => { - if (status) { - reject(new Error(`Process exited with status ${status}: ${stderr}`)); - } else { - resolve({ stdout, stderr }); - } - }); - - const script = [ - "begin", - ...code.split("\n").map((line) => ` ${line}`), - "ensure", - ' puts "END_OF_RUBY_CODE_OUTPUT"', - "end", - ].join("\n"); - - ruby.stdin.write(script); - ruby.stdin.end(); - }); - } - - protected execOptions(options: ExecOptions = {}): ExecOptions { + protected runScript(command: string) { let shell: string | undefined; // If the user has configured a default shell, we use that one since they are probably sourcing their version @@ -195,43 +98,18 @@ export abstract class VersionManager { shell = vscode.env.shell; } - return { + this.outputChannel.info( + `Running command: \`${command}\` in ${this.bundleUri.fsPath} using shell: ${shell}`, + ); + this.outputChannel.debug( + `Environment used for command: ${JSON.stringify(process.env)}`, + ); + + return asyncExec(command, { cwd: this.bundleUri.fsPath, shell, - ...options, - env: { ...process.env, ...options.env }, - }; - } - - // Parses a command string into its command, arguments, and environment variables - protected parseCommand(commandString: string): { - command: string; - args: string[]; - env: Record; - } { - // Regular expression to split arguments while respecting quotes - const regex = /(?:[^\s"']+|"[^"]*"|'[^']*')+/g; - - const parts = - commandString.match(regex)?.map((arg) => { - // Remove surrounding quotes, if any - return arg.replace(/^['"]|['"]$/g, ""); - }) ?? []; - - // Extract environment variables - const env: Record = {}; - while (parts[0] && parts[0].includes("=")) { - const [key, value] = parts.shift()?.split("=") ?? []; - if (key) { - env[key] = value || ""; - } - } - - // The first part is the command, the rest are arguments - const command = parts.shift() || ""; - const args = parts; - - return { command, args, env }; + env: process.env, + }); } // Tries to find `execName` within the given directories. Prefers the executables found in the given directories over diff --git a/vscode/src/rubyLsp.ts b/vscode/src/rubyLsp.ts index a439797aa8..54f1b2f6c2 100644 --- a/vscode/src/rubyLsp.ts +++ b/vscode/src/rubyLsp.ts @@ -110,7 +110,7 @@ export class RubyLsp { } const decodedUri = decodeURIComponent(originalUri); - return this.virtualDocuments.get(decodedUri) || ""; + return this.virtualDocuments.get(decodedUri); }, }), LOG_CHANNEL, diff --git a/vscode/src/test/suite/ruby.test.ts b/vscode/src/test/suite/ruby.test.ts index 5fb74b32cc..e1c4abb348 100644 --- a/vscode/src/test/suite/ruby.test.ts +++ b/vscode/src/test/suite/ruby.test.ts @@ -8,10 +8,10 @@ import sinon from "sinon"; import { Ruby, ManagerIdentifier } from "../../ruby"; import { WorkspaceChannel } from "../../workspaceChannel"; import { LOG_CHANNEL } from "../../common"; +import * as common from "../../common"; import { ACTIVATION_SEPARATOR } from "../../ruby/versionManager"; import { FAKE_TELEMETRY } from "./fakeTelemetry"; -import { createSpawnStub } from "./testHelpers"; suite("Ruby environment activation", () => { const workspacePath = path.dirname( @@ -130,7 +130,8 @@ suite("Ruby environment activation", () => { gemPath: ["~/.gem/ruby/3.3.5", "/opt/rubies/3.3.5/lib/ruby/gems/3.3.0"], }; - const { spawnStub } = createSpawnStub({ + const execStub = sinon.stub(common, "asyncExec").resolves({ + stdout: "", stderr: `${ACTIVATION_SEPARATOR}${JSON.stringify(envStub)}${ACTIVATION_SEPARATOR}`, }); @@ -141,7 +142,7 @@ suite("Ruby environment activation", () => { FAKE_TELEMETRY, ); await ruby.activateRuby(); - spawnStub.restore(); + execStub.restore(); configStub.restore(); assert.deepStrictEqual(ruby.gemPath, [ diff --git a/vscode/src/test/suite/ruby/asdf.test.ts b/vscode/src/test/suite/ruby/asdf.test.ts index 8eb1781e06..46a4449ad3 100644 --- a/vscode/src/test/suite/ruby/asdf.test.ts +++ b/vscode/src/test/suite/ruby/asdf.test.ts @@ -9,7 +9,6 @@ import { Asdf } from "../../../ruby/asdf"; import { WorkspaceChannel } from "../../../workspaceChannel"; import * as common from "../../../common"; import { ACTIVATION_SEPARATOR } from "../../../ruby/versionManager"; -import { createSpawnStub } from "../testHelpers"; suite("Asdf", () => { if (os.platform() === "win32") { @@ -18,13 +17,6 @@ suite("Asdf", () => { return; } - let spawnStub: sinon.SinonStub; - let stdinData: string[]; - - teardown(() => { - spawnStub?.restore(); - }); - test("Finds Ruby based on .tool-versions", async () => { // eslint-disable-next-line no-process-env const workspacePath = process.env.PWD!; @@ -41,9 +33,10 @@ suite("Asdf", () => { version: "3.0.0", }; - ({ spawnStub, stdinData } = createSpawnStub({ + const execStub = sinon.stub(common, "asyncExec").resolves({ + stdout: "", stderr: `${ACTIVATION_SEPARATOR}${JSON.stringify(envStub)}${ACTIVATION_SEPARATOR}`, - })); + }); const findInstallationStub = sinon .stub(asdf, "findAsdfInstallation") @@ -53,17 +46,8 @@ suite("Asdf", () => { const { env, version, yjit } = await asdf.activate(); assert.ok( - spawnStub.calledOnceWithExactly( - ".", - [ - `${os.homedir()}/.asdf/asdf.sh`, - "&&", - "asdf", - "exec", - "ruby", - "-W0", - "-rjson", - ], + execStub.calledOnceWithExactly( + `. ${os.homedir()}/.asdf/asdf.sh && asdf exec ruby -W0 -rjson -e '${asdf.activationScript}'`, { cwd: workspacePath, shell: "/bin/bash", @@ -73,12 +57,11 @@ suite("Asdf", () => { ), ); - assert.ok(stdinData.join("\n").includes(asdf.activationScript)); - assert.strictEqual(version, "3.0.0"); assert.strictEqual(yjit, true); assert.strictEqual(env.ANY, "true"); + execStub.restore(); findInstallationStub.restore(); shellStub.restore(); }); @@ -99,9 +82,10 @@ suite("Asdf", () => { version: "3.0.0", }; - ({ spawnStub, stdinData } = createSpawnStub({ + const execStub = sinon.stub(common, "asyncExec").resolves({ + stdout: "", stderr: `${ACTIVATION_SEPARATOR}${JSON.stringify(envStub)}${ACTIVATION_SEPARATOR}`, - })); + }); const findInstallationStub = sinon .stub(asdf, "findAsdfInstallation") @@ -113,17 +97,8 @@ suite("Asdf", () => { const { env, version, yjit } = await asdf.activate(); assert.ok( - spawnStub.calledOnceWithExactly( - ".", - [ - `${os.homedir()}/.asdf/asdf.fish`, - "&&", - "asdf", - "exec", - "ruby", - "-W0", - "-rjson", - ], + execStub.calledOnceWithExactly( + `. ${os.homedir()}/.asdf/asdf.fish && asdf exec ruby -W0 -rjson -e '${asdf.activationScript}'`, { cwd: workspacePath, shell: "/opt/homebrew/bin/fish", @@ -133,12 +108,11 @@ suite("Asdf", () => { ), ); - assert.ok(stdinData.join("\n").includes(asdf.activationScript)); - assert.strictEqual(version, "3.0.0"); assert.strictEqual(yjit, true); assert.strictEqual(env.ANY, "true"); + execStub.restore(); findInstallationStub.restore(); shellStub.restore(); }); diff --git a/vscode/src/test/suite/ruby/custom.test.ts b/vscode/src/test/suite/ruby/custom.test.ts index 925a4a9087..5e1482d1cc 100644 --- a/vscode/src/test/suite/ruby/custom.test.ts +++ b/vscode/src/test/suite/ruby/custom.test.ts @@ -10,16 +10,8 @@ import { Custom } from "../../../ruby/custom"; import { WorkspaceChannel } from "../../../workspaceChannel"; import * as common from "../../../common"; import { ACTIVATION_SEPARATOR } from "../../../ruby/versionManager"; -import { createSpawnStub } from "../testHelpers"; suite("Custom", () => { - let spawnStub: sinon.SinonStub; - let stdinData: string[]; - - teardown(() => { - spawnStub?.restore(); - }); - test("Invokes custom script and then Ruby", async () => { const workspacePath = fs.mkdtempSync( path.join(os.tmpdir(), "ruby-lsp-test-"), @@ -39,9 +31,10 @@ suite("Custom", () => { version: "3.0.0", }; - ({ spawnStub, stdinData } = createSpawnStub({ + const execStub = sinon.stub(common, "asyncExec").resolves({ + stdout: "", stderr: `${ACTIVATION_SEPARATOR}${JSON.stringify(envStub)}${ACTIVATION_SEPARATOR}`, - })); + }); const commandStub = sinon .stub(custom, "customCommand") @@ -52,9 +45,8 @@ suite("Custom", () => { const shell = os.platform() === "win32" ? undefined : vscode.env.shell; assert.ok( - spawnStub.calledOnceWithExactly( - "my_version_manager", - ["activate_env", "&&", "ruby", "-W0", "-rjson"], + execStub.calledOnceWithExactly( + `my_version_manager activate_env && ruby -W0 -rjson -e '${custom.activationScript}'`, { cwd: uri.fsPath, shell, @@ -64,12 +56,11 @@ suite("Custom", () => { ), ); - assert.ok(stdinData.join("\n").includes(custom.activationScript)); - assert.strictEqual(version, "3.0.0"); assert.strictEqual(yjit, true); assert.deepStrictEqual(env.ANY, "true"); + execStub.restore(); commandStub.restore(); fs.rmSync(workspacePath, { recursive: true, force: true }); }); diff --git a/vscode/src/test/suite/ruby/mise.test.ts b/vscode/src/test/suite/ruby/mise.test.ts index c790d85f24..5d31fa2442 100644 --- a/vscode/src/test/suite/ruby/mise.test.ts +++ b/vscode/src/test/suite/ruby/mise.test.ts @@ -10,7 +10,6 @@ import { Mise } from "../../../ruby/mise"; import { WorkspaceChannel } from "../../../workspaceChannel"; import * as common from "../../../common"; import { ACTIVATION_SEPARATOR } from "../../../ruby/versionManager"; -import { createSpawnStub } from "../testHelpers"; suite("Mise", () => { if (os.platform() === "win32") { @@ -19,13 +18,6 @@ suite("Mise", () => { return; } - let spawnStub: sinon.SinonStub; - let stdinData: string[]; - - teardown(() => { - spawnStub?.restore(); - }); - test("Finds Ruby only binary path is appended to PATH", async () => { // eslint-disable-next-line no-process-env const workspacePath = process.env.PWD!; @@ -43,10 +35,10 @@ suite("Mise", () => { version: "3.0.0", }; - ({ spawnStub, stdinData } = createSpawnStub({ + const execStub = sinon.stub(common, "asyncExec").resolves({ + stdout: "", stderr: `${ACTIVATION_SEPARATOR}${JSON.stringify(envStub)}${ACTIVATION_SEPARATOR}`, - })); - + }); const findStub = sinon .stub(mise, "findMiseUri") .resolves( @@ -61,9 +53,8 @@ suite("Mise", () => { const { env, version, yjit } = await mise.activate(); assert.ok( - spawnStub.calledOnceWithExactly( - `${os.homedir()}/.local/bin/mise`, - ["x", "--", "ruby", "-W0", "-rjson"], + execStub.calledOnceWithExactly( + `${os.homedir()}/.local/bin/mise x -- ruby -W0 -rjson -e '${mise.activationScript}'`, { cwd: workspacePath, shell: vscode.env.shell, @@ -73,12 +64,11 @@ suite("Mise", () => { ), ); - assert.ok(stdinData.join("\n").includes(mise.activationScript)); - assert.strictEqual(version, "3.0.0"); assert.strictEqual(yjit, true); assert.deepStrictEqual(env.ANY, "true"); + execStub.restore(); findStub.restore(); }); @@ -100,9 +90,10 @@ suite("Mise", () => { version: "3.0.0", }; - ({ spawnStub, stdinData } = createSpawnStub({ + const execStub = sinon.stub(common, "asyncExec").resolves({ + stdout: "", stderr: `${ACTIVATION_SEPARATOR}${JSON.stringify(envStub)}${ACTIVATION_SEPARATOR}`, - })); + }); const misePath = path.join(workspacePath, "mise"); fs.writeFileSync(misePath, "fakeMiseBinary"); @@ -121,9 +112,8 @@ suite("Mise", () => { const { env, version, yjit } = await mise.activate(); assert.ok( - spawnStub.calledOnceWithExactly( - misePath, - ["x", "--", "ruby", "-W0", "-rjson"], + execStub.calledOnceWithExactly( + `${misePath} x -- ruby -W0 -rjson -e '${mise.activationScript}'`, { cwd: workspacePath, shell: vscode.env.shell, @@ -133,12 +123,11 @@ suite("Mise", () => { ), ); - assert.ok(stdinData.join("\n").includes(mise.activationScript)); - assert.strictEqual(version, "3.0.0"); assert.strictEqual(yjit, true); assert.deepStrictEqual(env.ANY, "true"); + execStub.restore(); configStub.restore(); fs.rmSync(workspacePath, { recursive: true, force: true }); }); diff --git a/vscode/src/test/suite/ruby/none.test.ts b/vscode/src/test/suite/ruby/none.test.ts index c7edf4e3a5..f42fbaf648 100644 --- a/vscode/src/test/suite/ruby/none.test.ts +++ b/vscode/src/test/suite/ruby/none.test.ts @@ -4,21 +4,14 @@ import fs from "fs"; import os from "os"; import * as vscode from "vscode"; +import sinon from "sinon"; import { None } from "../../../ruby/none"; import { WorkspaceChannel } from "../../../workspaceChannel"; import * as common from "../../../common"; import { ACTIVATION_SEPARATOR } from "../../../ruby/versionManager"; -import { createSpawnStub } from "../testHelpers"; suite("None", () => { - let spawnStub: sinon.SinonStub; - let stdinData: string[]; - - teardown(() => { - spawnStub?.restore(); - }); - test("Invokes Ruby directly", async () => { const workspacePath = fs.mkdtempSync( path.join(os.tmpdir(), "ruby-lsp-test-"), @@ -38,9 +31,10 @@ suite("None", () => { version: "3.0.0", }; - ({ spawnStub, stdinData } = createSpawnStub({ + const execStub = sinon.stub(common, "asyncExec").resolves({ + stdout: "", stderr: `${ACTIVATION_SEPARATOR}${JSON.stringify(envStub)}${ACTIVATION_SEPARATOR}`, - })); + }); const { env, version, yjit } = await none.activate(); @@ -48,20 +42,22 @@ suite("None", () => { const shell = os.platform() === "win32" ? undefined : vscode.env.shell; assert.ok( - spawnStub.calledOnceWithExactly("ruby", ["-W0", "-rjson"], { - cwd: uri.fsPath, - shell, - // eslint-disable-next-line no-process-env - env: process.env, - }), + execStub.calledOnceWithExactly( + `ruby -W0 -rjson -e '${none.activationScript}'`, + { + cwd: uri.fsPath, + shell, + // eslint-disable-next-line no-process-env + env: process.env, + }, + ), ); - assert.ok(stdinData.join("\n").includes(none.activationScript)); - assert.strictEqual(version, "3.0.0"); assert.strictEqual(yjit, true); assert.deepStrictEqual(env.ANY, "true"); + execStub.restore(); fs.rmSync(workspacePath, { recursive: true, force: true }); }); }); diff --git a/vscode/src/test/suite/ruby/rbenv.test.ts b/vscode/src/test/suite/ruby/rbenv.test.ts index 816b629788..1463c2f3ec 100644 --- a/vscode/src/test/suite/ruby/rbenv.test.ts +++ b/vscode/src/test/suite/ruby/rbenv.test.ts @@ -10,7 +10,6 @@ import { Rbenv } from "../../../ruby/rbenv"; import { WorkspaceChannel } from "../../../workspaceChannel"; import * as common from "../../../common"; import { ACTIVATION_SEPARATOR } from "../../../ruby/versionManager"; -import { createSpawnStub } from "../testHelpers"; suite("Rbenv", () => { if (os.platform() === "win32") { @@ -19,13 +18,6 @@ suite("Rbenv", () => { return; } - let spawnStub: sinon.SinonStub; - let stdinData: string[]; - - teardown(() => { - spawnStub?.restore(); - }); - test("Finds Ruby based on .ruby-version", async () => { // eslint-disable-next-line no-process-env const workspacePath = process.env.PWD!; @@ -43,16 +35,16 @@ suite("Rbenv", () => { version: "3.0.0", }; - ({ spawnStub, stdinData } = createSpawnStub({ + const execStub = sinon.stub(common, "asyncExec").resolves({ + stdout: "", stderr: `${ACTIVATION_SEPARATOR}${JSON.stringify(envStub)}${ACTIVATION_SEPARATOR}`, - })); + }); const { env, version, yjit } = await rbenv.activate(); assert.ok( - spawnStub.calledOnceWithExactly( - "rbenv", - ["exec", "ruby", "-W0", "-rjson"], + execStub.calledOnceWithExactly( + `rbenv exec ruby -W0 -rjson -e '${rbenv.activationScript}'`, { cwd: workspacePath, shell: vscode.env.shell, @@ -62,11 +54,10 @@ suite("Rbenv", () => { ), ); - assert.ok(stdinData.join("\n").includes(rbenv.activationScript)); - assert.strictEqual(version, "3.0.0"); assert.strictEqual(yjit, true); assert.strictEqual(env.ANY, "true"); + execStub.restore(); }); test("Allows configuring where rbenv is installed", async () => { @@ -87,9 +78,10 @@ suite("Rbenv", () => { version: "3.0.0", }; - ({ spawnStub, stdinData } = createSpawnStub({ + const execStub = sinon.stub(common, "asyncExec").resolves({ + stdout: "", stderr: `${ACTIVATION_SEPARATOR}${JSON.stringify(envStub)}${ACTIVATION_SEPARATOR}`, - })); + }); const rbenvPath = path.join(workspacePath, "rbenv"); fs.writeFileSync(rbenvPath, "fakeRbenvBinary"); @@ -108,9 +100,8 @@ suite("Rbenv", () => { const { env, version, yjit } = await rbenv.activate(); assert.ok( - spawnStub.calledOnceWithExactly( - rbenvPath, - ["exec", "ruby", "-W0", "-rjson"], + execStub.calledOnceWithExactly( + `${rbenvPath} exec ruby -W0 -rjson -e '${rbenv.activationScript}'`, { cwd: workspacePath, shell: vscode.env.shell, @@ -120,12 +111,11 @@ suite("Rbenv", () => { ), ); - assert.ok(stdinData.join("\n").includes(rbenv.activationScript)); - assert.strictEqual(version, "3.0.0"); assert.strictEqual(yjit, true); assert.deepStrictEqual(env.ANY, "true"); + execStub.restore(); configStub.restore(); fs.rmSync(workspacePath, { recursive: true, force: true }); }); @@ -141,9 +131,10 @@ suite("Rbenv", () => { const outputChannel = new WorkspaceChannel("fake", common.LOG_CHANNEL); const rbenv = new Rbenv(workspaceFolder, outputChannel, async () => {}); - ({ spawnStub, stdinData } = createSpawnStub({ + const execStub = sinon.stub(common, "asyncExec").resolves({ + stdout: "", stderr: `${ACTIVATION_SEPARATOR}not a json${ACTIVATION_SEPARATOR}`, - })); + }); const errorStub = sinon.stub(outputChannel, "error"); @@ -153,9 +144,8 @@ suite("Rbenv", () => { ); assert.ok( - spawnStub.calledOnceWithExactly( - "rbenv", - ["exec", "ruby", "-W0", "-rjson"], + execStub.calledOnceWithExactly( + `rbenv exec ruby -W0 -rjson -e '${rbenv.activationScript}'`, { cwd: workspacePath, shell: vscode.env.shell, @@ -165,14 +155,13 @@ suite("Rbenv", () => { ), ); - assert.ok(stdinData.join("\n").includes(rbenv.activationScript)); - assert.ok( errorStub.calledOnceWithExactly( "Tried parsing invalid JSON environment: not a json", ), ); + execStub.restore(); errorStub.restore(); }); }); diff --git a/vscode/src/test/suite/ruby/rvm.test.ts b/vscode/src/test/suite/ruby/rvm.test.ts index 475860d60a..a1ff3c5b4a 100644 --- a/vscode/src/test/suite/ruby/rvm.test.ts +++ b/vscode/src/test/suite/ruby/rvm.test.ts @@ -10,7 +10,6 @@ import { Rvm } from "../../../ruby/rvm"; import { WorkspaceChannel } from "../../../workspaceChannel"; import * as common from "../../../common"; import { ACTIVATION_SEPARATOR } from "../../../ruby/versionManager"; -import { createSpawnStub } from "../testHelpers"; suite("RVM", () => { if (os.platform() === "win32") { @@ -19,13 +18,6 @@ suite("RVM", () => { return; } - let spawnStub: sinon.SinonStub; - let stdinData: string[]; - - teardown(() => { - spawnStub?.restore(); - }); - test("Populates the gem env and path", async () => { const workspacePath = process.env.PWD!; const workspaceFolder = { @@ -55,16 +47,16 @@ suite("RVM", () => { version: "3.0.0", }; - ({ spawnStub, stdinData } = createSpawnStub({ + const execStub = sinon.stub(common, "asyncExec").resolves({ + stdout: "", stderr: `${ACTIVATION_SEPARATOR}${JSON.stringify(envStub)}${ACTIVATION_SEPARATOR}`, - })); + }); const { env, version, yjit } = await rvm.activate(); assert.ok( - spawnStub.calledOnceWithExactly( - path.join(os.homedir(), ".rvm", "bin", "rvm-auto-ruby"), - ["-W0", "-rjson"], + execStub.calledOnceWithExactly( + `${path.join(os.homedir(), ".rvm", "bin", "rvm-auto-ruby")} -W0 -rjson -e '${rvm.activationScript}'`, { cwd: workspacePath, shell: vscode.env.shell, @@ -73,12 +65,11 @@ suite("RVM", () => { ), ); - assert.ok(stdinData.join("\n").includes(rvm.activationScript)); - assert.strictEqual(version, "3.0.0"); assert.strictEqual(yjit, true); assert.deepStrictEqual(env.ANY, "true"); + execStub.restore(); installationPathStub.restore(); }); }); diff --git a/vscode/src/test/suite/ruby/shadowenv.test.ts b/vscode/src/test/suite/ruby/shadowenv.test.ts index e5c7b64caa..1bd22d260a 100644 --- a/vscode/src/test/suite/ruby/shadowenv.test.ts +++ b/vscode/src/test/suite/ruby/shadowenv.test.ts @@ -14,7 +14,6 @@ import { WorkspaceChannel } from "../../../workspaceChannel"; import { LOG_CHANNEL, asyncExec } from "../../../common"; import { RUBY_VERSION } from "../../rubyVersion"; import * as common from "../../../common"; -import { createSpawnStub } from "../testHelpers"; suite("Shadowenv", () => { if (os.platform() === "win32") { @@ -36,8 +35,6 @@ suite("Shadowenv", () => { let workspaceFolder: vscode.WorkspaceFolder; let outputChannel: WorkspaceChannel; let rubyBinPath: string; - let spawnStub: sinon.SinonStub; - const [major, minor, patch] = RUBY_VERSION.split("."); if (process.env.CI && os.platform() === "linux") { @@ -123,8 +120,6 @@ suite("Shadowenv", () => { afterEach(() => { fs.rmSync(rootPath, { recursive: true, force: true }); - - spawnStub?.restore(); }); test("Finds Ruby only binary path is appended to PATH", async () => { @@ -240,13 +235,12 @@ suite("Shadowenv", () => { async () => {}, ); - // First, reject the call to `shadowenv exec`. - ({ spawnStub } = createSpawnStub()); - spawnStub.rejects(new Error("shadowenv: command not found")); - - // Then reject the call to `shadowenv --version` + // First, reject the call to `shadowenv exec`. Then resolve the call to `which shadowenv` to return nothing const execStub = sinon .stub(common, "asyncExec") + .onFirstCall() + .rejects(new Error("shadowenv: command not found")) + .onSecondCall() .rejects(new Error("shadowenv: command not found")); await assert.rejects(async () => { From f6276dd53a7b2914e0bb9376ba59ffb9aac8944b Mon Sep 17 00:00:00 2001 From: Dmitry Lebed <209805+d-lebed@users.noreply.github.com> Date: Thu, 28 Nov 2024 18:44:33 +0100 Subject: [PATCH 3/4] Refactor collecting documentSelector paths --- vscode/src/client.ts | 83 ++++++++++++++++---------------------------- vscode/src/common.ts | 5 +++ vscode/src/docker.ts | 10 ++++++ 3 files changed, 44 insertions(+), 54 deletions(-) diff --git a/vscode/src/client.ts b/vscode/src/client.ts index 9a4b84db57..37bee4fba9 100644 --- a/vscode/src/client.ts +++ b/vscode/src/client.ts @@ -156,6 +156,7 @@ function collectClientOptions( const supportedSchemes = ["file", "git"]; const fsPath = workspaceFolder.uri.fsPath.replace(/\/$/, ""); + const pathConverter = ruby.pathConverter; // For each workspace, the language client is responsible for handling requests for: // 1. Files inside of the workspace itself @@ -163,38 +164,14 @@ function collectClientOptions( // 3. Default gems let documentSelector: DocumentSelector = SUPPORTED_LANGUAGE_IDS.flatMap( (language) => { - return supportedSchemes.map((scheme) => { - return { scheme, language, pattern: `${fsPath}/**/*` }; + return pathConverter.alternativePaths(fsPath).flatMap((pathVariant) => { + return supportedSchemes.map((scheme) => { + return { scheme, language, pattern: `${pathVariant}/**/*` }; + }); }); }, ); - const pathConverter = ruby.pathConverter; - - const pushAlternativePaths = ( - path: string, - schemes: string[] = supportedSchemes, - ) => { - schemes.forEach((scheme) => { - [ - pathConverter.toLocalPath(path), - pathConverter.toRemotePath(path), - ].forEach((convertedPath) => { - if (convertedPath !== path) { - SUPPORTED_LANGUAGE_IDS.forEach((language) => { - documentSelector.push({ - scheme, - language, - pattern: `${convertedPath}/**/*`, - }); - }); - } - }); - }); - }; - - pushAlternativePaths(fsPath); - // Only the first language server we spawn should handle unsaved files, otherwise requests will be duplicated across // all workspaces if (isMainWorkspace) { @@ -208,37 +185,35 @@ function collectClientOptions( ruby.gemPath.forEach((gemPath) => { supportedSchemes.forEach((scheme) => { - documentSelector.push({ - scheme, - language: "ruby", - pattern: `${gemPath}/**/*`, - }); - - pushAlternativePaths(gemPath, [scheme]); - - // Because of how default gems are installed, the gemPath location is actually not exactly where the files are - // located. With the regex, we are correcting the default gem path from this (where the files are not located) - // /opt/rubies/3.3.1/lib/ruby/gems/3.3.0 - // - // to this (where the files are actually stored) - // /opt/rubies/3.3.1/lib/ruby/3.3.0 - // - // Notice that we still need to add the regular path to the selector because some version managers will install - // gems under the non-corrected path - if (/lib\/ruby\/gems\/(?=\d)/.test(gemPath)) { - const correctedPath = gemPath.replace( - /lib\/ruby\/gems\/(?=\d)/, - "lib/ruby/", - ); - + pathConverter.alternativePaths(gemPath).forEach((pathVariant) => { documentSelector.push({ scheme, language: "ruby", - pattern: `${correctedPath}/**/*`, + pattern: `${pathVariant}/**/*`, }); - pushAlternativePaths(correctedPath, [scheme]); - } + // Because of how default gems are installed, the gemPath location is actually not exactly where the files are + // located. With the regex, we are correcting the default gem path from this (where the files are not located) + // /opt/rubies/3.3.1/lib/ruby/gems/3.3.0 + // + // to this (where the files are actually stored) + // /opt/rubies/3.3.1/lib/ruby/3.3.0 + // + // Notice that we still need to add the regular path to the selector because some version managers will install + // gems under the non-corrected path + if (/lib\/ruby\/gems\/(?=\d)/.test(pathVariant)) { + const correctedPath = pathVariant.replace( + /lib\/ruby\/gems\/(?=\d)/, + "lib/ruby/", + ); + + documentSelector.push({ + scheme, + language: "ruby", + pattern: `${correctedPath}/**/*`, + }); + } + }); }); }); diff --git a/vscode/src/common.ts b/vscode/src/common.ts index ac0d61d6c4..f26f9f62b7 100644 --- a/vscode/src/common.ts +++ b/vscode/src/common.ts @@ -71,6 +71,7 @@ export interface PathConverterInterface { toRemotePath: (localPath: string) => string; toLocalPath: (remotePath: string) => string; toRemoteUri: (localUri: vscode.Uri) => vscode.Uri; + alternativePaths: (path: string) => string[]; } export class PathConverter implements PathConverterInterface { @@ -87,6 +88,10 @@ export class PathConverter implements PathConverterInterface { toRemoteUri(localUri: vscode.Uri) { return localUri; } + + alternativePaths(path: string) { + return [path]; + } } // Event emitter used to signal that the language status items need to be refreshed diff --git a/vscode/src/docker.ts b/vscode/src/docker.ts index 3b959ac8d5..cbbe619ffe 100644 --- a/vscode/src/docker.ts +++ b/vscode/src/docker.ts @@ -101,6 +101,16 @@ export class ContainerPathConverter implements PathConverterInterface { const remotePath = this.toRemotePath(localUri.fsPath); return vscode.Uri.file(remotePath); } + + alternativePaths(path: string) { + const alternatives = [ + this.toRemotePath(path), + this.toLocalPath(path), + path, + ]; + + return Array.from(new Set(alternatives)); + } } function fetchComposeBindings( From 9dd8fca1e922fdf08f864640a66b22fac8c53a25 Mon Sep 17 00:00:00 2001 From: Dmitry Lebed <209805+d-lebed@users.noreply.github.com> Date: Fri, 29 Nov 2024 14:36:25 +0100 Subject: [PATCH 4/4] Moved path conversion logic to ruby-lsp --- exe/ruby-lsp | 11 ++ lib/ruby_lsp/base_server.rb | 1 + lib/ruby_lsp/global_state.rb | 54 ++++++- lib/ruby_lsp/listeners/definition.rb | 26 +++- lib/ruby_lsp/requests/references.rb | 4 +- lib/ruby_lsp/requests/support/common.rb | 5 +- lib/ruby_lsp/requests/workspace_symbol.rb | 4 +- lib/ruby_lsp/server.rb | 7 +- vscode/src/client.ts | 167 ++++------------------ vscode/src/common.ts | 28 ---- vscode/src/docker.ts | 65 --------- vscode/src/ruby.ts | 21 +-- vscode/src/ruby/compose.ts | 28 ++-- vscode/src/ruby/versionManager.ts | 4 +- 14 files changed, 153 insertions(+), 272 deletions(-) diff --git a/exe/ruby-lsp b/exe/ruby-lsp index 7cd7d64bc6..67222c729e 100755 --- a/exe/ruby-lsp +++ b/exe/ruby-lsp @@ -37,6 +37,10 @@ parser = OptionParser.new do |opts| options[:launcher] = true end + opts.on("--local-fs-map [FS_MAP]", "Local fs map in format 'local:remote,local2:remote2,...'") do |map| + options[:local_fs_map] = map + end + opts.on("-h", "--help", "Print this help") do puts opts.help puts @@ -146,6 +150,13 @@ if options[:doctor] return end +if options[:local_fs_map] + ENV["RUBY_LSP_LOCAL_FS_MAP"] = [ + ENV["RUBY_LSP_LOCAL_FS_MAP"], + options[:local_fs_map], + ].reject(&:nil?).join(",") +end + # Ensure all output goes out stderr by default to allow puts/p/pp to work # without specifying output device. $> = $stderr diff --git a/lib/ruby_lsp/base_server.rb b/lib/ruby_lsp/base_server.rb index 8fac3592c4..443fca78fa 100644 --- a/lib/ruby_lsp/base_server.rb +++ b/lib/ruby_lsp/base_server.rb @@ -57,6 +57,7 @@ def start if uri begin parsed_uri = URI(uri) + parsed_uri = @global_state.to_internal_uri(parsed_uri) message[:params][:textDocument][:uri] = parsed_uri # We don't want to try to parse documents on text synchronization notifications diff --git a/lib/ruby_lsp/global_state.rb b/lib/ruby_lsp/global_state.rb index 004df40d9a..9d480cacfb 100644 --- a/lib/ruby_lsp/global_state.rb +++ b/lib/ruby_lsp/global_state.rb @@ -29,6 +29,9 @@ class GlobalState sig { returns(ClientCapabilities) } attr_reader :client_capabilities + sig { returns(T::Hash[String, String]) } + attr_accessor :local_fs_map + sig { void } def initialize @workspace_uri = T.let(URI::Generic.from_path(path: Dir.pwd), URI::Generic) @@ -53,6 +56,7 @@ def initialize ) @client_capabilities = T.let(ClientCapabilities.new, ClientCapabilities) @enabled_feature_flags = T.let({}, T::Hash[Symbol, T::Boolean]) + @local_fs_map = T.let(build_local_fs_map_from_env, T::Hash[String, String]) end sig { params(addon_name: String).returns(T.nilable(T::Hash[Symbol, T.untyped])) } @@ -81,8 +85,15 @@ def apply_options(options) notifications = [] direct_dependencies = gather_direct_dependencies all_dependencies = gather_direct_and_indirect_dependencies + + options.dig(:initializationOptions, :localFsMap)&.each do |local, remote| + local_fs_map[local.to_s] = remote + end + workspace_uri = options.dig(:workspaceFolders, 0, :uri) - @workspace_uri = URI(workspace_uri) if workspace_uri + if workspace_uri + @workspace_uri = to_internal_uri(URI(workspace_uri)) + end specified_formatter = options.dig(:initializationOptions, :formatter) @@ -171,6 +182,36 @@ def supports_watching_files @client_capabilities.supports_watching_files end + sig { params(uri: URI::Generic).returns(URI::Generic) } + def to_internal_uri(uri) + path = uri.path + return uri unless path + + local_fs_map.each do |external, internal| + next unless path.start_with?(external) + + uri.path = path.sub(external, internal) + return uri + end + + uri + end + + sig { params(uri: URI::Generic).returns(URI::Generic) } + def to_external_uri(uri) + path = uri.path + return uri unless path + + local_fs_map.each do |external, internal| + next unless path.start_with?(internal) + + uri.path = path.sub(internal, external) + return uri + end + + uri + end + private sig { params(direct_dependencies: T::Array[String], all_dependencies: T::Array[String]).returns(String) } @@ -263,5 +304,16 @@ def gather_direct_and_indirect_dependencies rescue Bundler::GemfileNotFound [] end + + sig { returns(T::Hash[String, String]) } + def build_local_fs_map_from_env + env = ENV["RUBY_LSP_LOCAL_FS_MAP"] + return {} unless env + + env.split(",").each_with_object({}) do |pair, map| + local, remote = pair.split(":", 2) + map[local] = remote + end + end end end diff --git a/lib/ruby_lsp/listeners/definition.rb b/lib/ruby_lsp/listeners/definition.rb index 9c37b6cf94..ba2c9edb20 100644 --- a/lib/ruby_lsp/listeners/definition.rb +++ b/lib/ruby_lsp/listeners/definition.rb @@ -222,8 +222,10 @@ def handle_global_variable_definition(name) entries.each do |entry| location = entry.location + external_uri = @global_state.to_external_uri(entry.uri) + @response_builder << Interface::Location.new( - uri: entry.uri.to_s, + uri: external_uri.to_s, range: Interface::Range.new( start: Interface::Position.new(line: location.start_line - 1, character: location.start_column), end: Interface::Position.new(line: location.end_line - 1, character: location.end_column), @@ -247,8 +249,10 @@ def handle_instance_variable_definition(name) entries.each do |entry| location = entry.location + external_uri = @global_state.to_external_uri(entry.uri) + @response_builder << Interface::Location.new( - uri: entry.uri.to_s, + uri: external_uri.to_s, range: Interface::Range.new( start: Interface::Position.new(line: location.start_line - 1, character: location.start_column), end: Interface::Position.new(line: location.end_line - 1, character: location.end_column), @@ -278,8 +282,10 @@ def handle_method_definition(message, receiver_type, inherited_only: false) uri = target_method.uri next if sorbet_level_true_or_higher?(@sorbet_level) && not_in_dependencies?(T.must(uri.full_path)) + external_uri = @global_state.to_external_uri(uri) + @response_builder << Interface::LocationLink.new( - target_uri: uri.to_s, + target_uri: external_uri.to_s, target_range: range_from_location(target_method.location), target_selection_range: range_from_location(target_method.name_location), ) @@ -298,8 +304,11 @@ def handle_require_definition(node, message) candidate = entry.full_path if candidate + uri = URI::Generic.from_path(path: candidate) + external_uri = @global_state.to_external_uri(uri) + @response_builder << Interface::Location.new( - uri: URI::Generic.from_path(path: candidate).to_s, + uri: external_uri.to_s, range: Interface::Range.new( start: Interface::Position.new(line: 0, character: 0), end: Interface::Position.new(line: 0, character: 0), @@ -313,8 +322,11 @@ def handle_require_definition(node, message) current_folder = path ? Pathname.new(CGI.unescape(path)).dirname : @global_state.workspace_path candidate = File.expand_path(File.join(current_folder, required_file)) + uri = URI::Generic.from_path(path: candidate) + external_uri = @global_state.to_external_uri(uri) + @response_builder << Interface::Location.new( - uri: URI::Generic.from_path(path: candidate).to_s, + uri: external_uri.to_s, range: Interface::Range.new( start: Interface::Position.new(line: 0, character: 0), end: Interface::Position.new(line: 0, character: 0), @@ -351,8 +363,10 @@ def find_in_index(value) uri = entry.uri next if @sorbet_level != RubyDocument::SorbetLevel::Ignore && not_in_dependencies?(T.must(uri.full_path)) + external_uri = @global_state.to_external_uri(uri) + @response_builder << Interface::LocationLink.new( - target_uri: uri.to_s, + target_uri: external_uri.to_s, target_range: range_from_location(entry.location), target_selection_range: range_from_location(entry.name_location), ) diff --git a/lib/ruby_lsp/requests/references.rb b/lib/ruby_lsp/requests/references.rb index bd7694f37a..caca36a44f 100644 --- a/lib/ruby_lsp/requests/references.rb +++ b/lib/ruby_lsp/requests/references.rb @@ -137,8 +137,10 @@ def collect_references(target, parse_result, uri) dispatcher.visit(parse_result.value) finder.references.each do |reference| + external_uri = @global_state.to_external_uri(uri) + @locations << Interface::Location.new( - uri: uri.to_s, + uri: external_uri.to_s, range: range_from_location(reference.location), ) end diff --git a/lib/ruby_lsp/requests/support/common.rb b/lib/ruby_lsp/requests/support/common.rb index de1821d49d..812cb0fd8b 100644 --- a/lib/ruby_lsp/requests/support/common.rb +++ b/lib/ruby_lsp/requests/support/common.rb @@ -89,11 +89,14 @@ def categorized_markdown_from_index_entries(title, entries, max_entries = nil) entries_to_format.each do |entry| loc = entry.location + @global_state ||= T.let(GlobalState.new, T.nilable(RubyLsp::GlobalState)) + external_uri = @global_state.to_external_uri(entry.uri) + # We always handle locations as zero based. However, for file links in Markdown we need them to be one # based, which is why instead of the usual subtraction of 1 to line numbers, we are actually adding 1 to # columns. The format for VS Code file URIs is # `file:///path/to/file.rb#Lstart_line,start_column-end_line,end_column` - uri = "#{entry.uri}#L#{loc.start_line},#{loc.start_column + 1}-#{loc.end_line},#{loc.end_column + 1}" + uri = "#{external_uri}#L#{loc.start_line},#{loc.start_column + 1}-#{loc.end_line},#{loc.end_column + 1}" definitions << "[#{entry.file_name}](#{uri})" content << "\n\n#{entry.comments}" unless entry.comments.empty? end diff --git a/lib/ruby_lsp/requests/workspace_symbol.rb b/lib/ruby_lsp/requests/workspace_symbol.rb index 9b6c238ee3..384abbff9c 100644 --- a/lib/ruby_lsp/requests/workspace_symbol.rb +++ b/lib/ruby_lsp/requests/workspace_symbol.rb @@ -39,12 +39,14 @@ def perform # short name `Bar`, then searching for `Foo::Bar` would not return any results *container, _short_name = entry.name.split("::") + external_uri = @global_state.to_external_uri(uri) + Interface::WorkspaceSymbol.new( name: entry.name, container_name: container.join("::"), kind: kind, location: Interface::Location.new( - uri: uri.to_s, + uri: external_uri.to_s, range: Interface::Range.new( start: Interface::Position.new(line: loc.start_line - 1, character: loc.start_column), end: Interface::Position.new(line: loc.end_line - 1, character: loc.end_column), diff --git a/lib/ruby_lsp/server.rb b/lib/ruby_lsp/server.rb index f975e065e2..d2c4931c74 100644 --- a/lib/ruby_lsp/server.rb +++ b/lib/ruby_lsp/server.rb @@ -421,6 +421,8 @@ def text_document_did_close(message) uri = message.dig(:params, :textDocument, :uri) @store.delete(uri) + uri = global_state.to_external_uri(uri) + # Clear diagnostics for the closed file, so that they no longer appear in the problems tab send_message( Notification.new( @@ -1106,10 +1108,13 @@ def workspace_dependencies(message) dep_keys = definition.locked_deps.keys.to_set definition.specs.map do |spec| + uri = URI("file://#{spec.full_gem_path}") + uri = global_state.to_external_uri(uri) + { name: spec.name, version: spec.version, - path: spec.full_gem_path, + path: uri.path, dependency: dep_keys.include?(spec.name), } end diff --git a/vscode/src/client.ts b/vscode/src/client.ts index 37bee4fba9..802a5ee75e 100644 --- a/vscode/src/client.ts +++ b/vscode/src/client.ts @@ -37,7 +37,6 @@ import { SUPPORTED_LANGUAGE_IDS, FEATURE_FLAGS, featureEnabled, - PathConverterInterface, } from "./common"; import { Ruby } from "./ruby"; import { WorkspaceChannel } from "./workspaceChannel"; @@ -156,7 +155,7 @@ function collectClientOptions( const supportedSchemes = ["file", "git"]; const fsPath = workspaceFolder.uri.fsPath.replace(/\/$/, ""); - const pathConverter = ruby.pathConverter; + const pathMapping = ruby.pathMapping; // For each workspace, the language client is responsible for handling requests for: // 1. Files inside of the workspace itself @@ -164,10 +163,8 @@ function collectClientOptions( // 3. Default gems let documentSelector: DocumentSelector = SUPPORTED_LANGUAGE_IDS.flatMap( (language) => { - return pathConverter.alternativePaths(fsPath).flatMap((pathVariant) => { - return supportedSchemes.map((scheme) => { - return { scheme, language, pattern: `${pathVariant}/**/*` }; - }); + return supportedSchemes.map((scheme) => { + return { scheme, language, pattern: `${fsPath}/**/*` }; }); }, ); @@ -185,63 +182,28 @@ function collectClientOptions( ruby.gemPath.forEach((gemPath) => { supportedSchemes.forEach((scheme) => { - pathConverter.alternativePaths(gemPath).forEach((pathVariant) => { - documentSelector.push({ - scheme, - language: "ruby", - pattern: `${pathVariant}/**/*`, - }); - - // Because of how default gems are installed, the gemPath location is actually not exactly where the files are - // located. With the regex, we are correcting the default gem path from this (where the files are not located) - // /opt/rubies/3.3.1/lib/ruby/gems/3.3.0 - // - // to this (where the files are actually stored) - // /opt/rubies/3.3.1/lib/ruby/3.3.0 - // - // Notice that we still need to add the regular path to the selector because some version managers will install - // gems under the non-corrected path - if (/lib\/ruby\/gems\/(?=\d)/.test(pathVariant)) { - const correctedPath = pathVariant.replace( - /lib\/ruby\/gems\/(?=\d)/, - "lib/ruby/", - ); - - documentSelector.push({ - scheme, - language: "ruby", - pattern: `${correctedPath}/**/*`, - }); - } + documentSelector.push({ + scheme, + language: "ruby", + pattern: `${gemPath}/**/*`, }); - }); - }); - - // Add other mapped paths to the document selector - pathConverter.pathMapping.forEach(([local, remote]) => { - if ( - (documentSelector as { pattern: string }[]).some( - (selector) => - selector.pattern?.startsWith(local) || - selector.pattern?.startsWith(remote), - ) - ) { - return; - } - - supportedSchemes.forEach((scheme) => { - SUPPORTED_LANGUAGE_IDS.forEach((language) => { - documentSelector.push({ - language, - pattern: `${local}/**/*`, - }); + // Because of how default gems are installed, the gemPath location is actually not exactly where the files are + // located. With the regex, we are correcting the default gem path from this (where the files are not located) + // /opt/rubies/3.3.1/lib/ruby/gems/3.3.0 + // + // to this (where the files are actually stored) + // /opt/rubies/3.3.1/lib/ruby/3.3.0 + // + // Notice that we still need to add the regular path to the selector because some version managers will install + // gems under the non-corrected path + if (/lib\/ruby\/gems\/(?=\d)/.test(gemPath)) { documentSelector.push({ scheme, - language, - pattern: `${remote}/**/*`, + language: "ruby", + pattern: `${gemPath.replace(/lib\/ruby\/gems\/(?=\d)/, "lib/ruby/")}/**/*`, }); - }); + } }); }); @@ -253,29 +215,9 @@ function collectClientOptions( }); } - outputChannel.debug( - `Document Selector Paths: ${JSON.stringify(documentSelector)}`, - ); - - // Map using pathMapping - const code2Protocol = (uri: vscode.Uri) => { - const remotePath = pathConverter.toRemotePath(uri.fsPath); - return vscode.Uri.file(remotePath).toString(); - }; - - const protocol2Code = (uri: string) => { - const remoteUri = vscode.Uri.parse(uri); - const localPath = pathConverter.toLocalPath(remoteUri.fsPath); - return vscode.Uri.file(localPath); - }; - return { documentSelector, workspaceFolder, - uriConverters: { - code2Protocol, - protocol2Code, - }, diagnosticCollectionName: LSP_NAME, outputChannel, revealOutputChannelOn: RevealOutputChannelOn.Never, @@ -289,6 +231,7 @@ function collectClientOptions( indexing: configuration.get("indexing"), addonSettings: configuration.get("addonSettings"), enabledFeatureFlags: enabledFeatureFlags(), + localFsMap: pathMapping, }, }; } @@ -378,7 +321,6 @@ export default class Client extends LanguageClient implements ClientInterface { private readonly baseFolder; private readonly workspaceOutputChannel: WorkspaceChannel; private readonly virtualDocuments = new Map(); - private readonly pathConverter: PathConverterInterface; #context: vscode.ExtensionContext; #formatter: string; @@ -411,7 +353,6 @@ export default class Client extends LanguageClient implements ClientInterface { this.registerFeature(new ExperimentalCapabilities()); this.workspaceOutputChannel = outputChannel; this.virtualDocuments = virtualDocuments; - this.pathConverter = ruby.pathConverter; // Middleware are part of client options, but because they must reference `this`, we cannot make it a part of the // `super` call (TypeScript does not allow accessing `this` before invoking `super`) @@ -492,9 +433,7 @@ export default class Client extends LanguageClient implements ClientInterface { range?: Range, ): Promise<{ ast: string } | null> { return this.sendRequest("rubyLsp/textDocument/showSyntaxTree", { - textDocument: { - uri: this.pathConverter.toRemoteUri(uri).toString(), - }, + textDocument: { uri: uri.toString() }, range, }); } @@ -690,12 +629,10 @@ export default class Client extends LanguageClient implements ClientInterface { token, _next, ) => { - const remoteUri = this.pathConverter.toRemoteUri(document.uri); - const response: vscode.TextEdit[] | null = await this.sendRequest( "textDocument/onTypeFormatting", { - textDocument: { uri: remoteUri.toString() }, + textDocument: { uri: document.uri.toString() }, position, ch, options, @@ -763,65 +700,9 @@ export default class Client extends LanguageClient implements ClientInterface { token?: vscode.CancellationToken, ) => Promise, ) => { - this.workspaceOutputChannel.trace( - `Sending request: ${JSON.stringify(type)} with params: ${JSON.stringify(param)}`, - ); - - const result = (await this.benchmarkMiddleware(type, param, () => + return this.benchmarkMiddleware(type, param, () => next(type, param, token), - )) as any; - - this.workspaceOutputChannel.trace( - `Received response for ${JSON.stringify(type)}: ${JSON.stringify(result)}`, ); - - const request = typeof type === "string" ? type : type.method; - - try { - switch (request) { - case "rubyLsp/workspace/dependencies": - return result.map((dep: { path: string }) => { - return { - ...dep, - path: this.pathConverter.toLocalPath(dep.path), - }; - }); - - case "textDocument/codeAction": - return result.map((action: { uri: string }) => { - const remotePath = vscode.Uri.parse(action.uri).fsPath; - const localPath = this.pathConverter.toLocalPath(remotePath); - - return { - ...action, - uri: vscode.Uri.file(localPath).toString(), - }; - }); - - case "textDocument/hover": - if ( - result?.contents?.kind === "markdown" && - result.contents.value - ) { - result.contents.value = result.contents.value.replace( - /\((file:\/\/.+?)#/gim, - (_match: string, path: string) => { - const remotePath = vscode.Uri.parse(path).fsPath; - const localPath = - this.pathConverter.toLocalPath(remotePath); - return `(${vscode.Uri.file(localPath).toString()}#`; - }, - ); - } - break; - } - } catch (error) { - this.workspaceOutputChannel.error( - `Error while processing response for ${request}: ${error}`, - ); - } - - return result; }, sendNotification: async ( type: string | MessageSignature, diff --git a/vscode/src/common.ts b/vscode/src/common.ts index f26f9f62b7..012be26559 100644 --- a/vscode/src/common.ts +++ b/vscode/src/common.ts @@ -66,34 +66,6 @@ export interface WorkspaceInterface { error: boolean; } -export interface PathConverterInterface { - pathMapping: [string, string][]; - toRemotePath: (localPath: string) => string; - toLocalPath: (remotePath: string) => string; - toRemoteUri: (localUri: vscode.Uri) => vscode.Uri; - alternativePaths: (path: string) => string[]; -} - -export class PathConverter implements PathConverterInterface { - readonly pathMapping: [string, string][] = []; - - toRemotePath(path: string) { - return path; - } - - toLocalPath(path: string) { - return path; - } - - toRemoteUri(localUri: vscode.Uri) { - return localUri; - } - - alternativePaths(path: string) { - return [path]; - } -} - // Event emitter used to signal that the language status items need to be refreshed export const STATUS_EMITTER = new vscode.EventEmitter< WorkspaceInterface | undefined diff --git a/vscode/src/docker.ts b/vscode/src/docker.ts index cbbe619ffe..e1e9010217 100644 --- a/vscode/src/docker.ts +++ b/vscode/src/docker.ts @@ -1,10 +1,5 @@ import path from "path"; -import * as vscode from "vscode"; - -import { PathConverterInterface } from "./common"; -import { WorkspaceChannel } from "./workspaceChannel"; - export interface ComposeConfig { services: Record; ["x-mutagen"]?: { sync: Record } | undefined; @@ -53,66 +48,6 @@ export function fetchPathMapping( return bindings; } -export class ContainerPathConverter implements PathConverterInterface { - readonly pathMapping: [string, string][]; - private readonly outputChannel: WorkspaceChannel; - - constructor( - pathMapping: Record, - outputChannel: WorkspaceChannel, - ) { - this.pathMapping = Object.entries(pathMapping); - this.outputChannel = outputChannel; - } - - toRemotePath(path: string) { - for (const [local, remote] of this.pathMapping) { - if (path.startsWith(local)) { - const remotePath = path.replace(local, remote); - - this.outputChannel.debug( - `Converted toRemotePath ${path} to ${remotePath}`, - ); - - return path.replace(local, remote); - } - } - - return path; - } - - toLocalPath(path: string) { - for (const [local, remote] of this.pathMapping) { - if (path.startsWith(remote)) { - const localPath = path.replace(remote, local); - - this.outputChannel.debug( - `Converted toLocalPath ${path} to ${localPath}`, - ); - - return localPath; - } - } - - return path; - } - - toRemoteUri(localUri: vscode.Uri) { - const remotePath = this.toRemotePath(localUri.fsPath); - return vscode.Uri.file(remotePath); - } - - alternativePaths(path: string) { - const alternatives = [ - this.toRemotePath(path), - this.toLocalPath(path), - path, - ]; - - return Array.from(new Set(alternatives)); - } -} - function fetchComposeBindings( volumes: ComposeVolume[], mutagenMounts: MutagenMountMapping, diff --git a/vscode/src/ruby.ts b/vscode/src/ruby.ts index 0f6e6f4600..efac1ba811 100644 --- a/vscode/src/ruby.ts +++ b/vscode/src/ruby.ts @@ -5,13 +5,7 @@ import os from "os"; import * as vscode from "vscode"; import { Executable, ExecutableOptions } from "vscode-languageclient/node"; -import { - asyncExec, - parseCommand, - PathConverter, - PathConverterInterface, - RubyInterface, -} from "./common"; +import { asyncExec, parseCommand, RubyInterface } from "./common"; import { WorkspaceChannel } from "./workspaceChannel"; import { Shadowenv } from "./ruby/shadowenv"; import { Chruby } from "./ruby/chruby"; @@ -75,7 +69,7 @@ export class Ruby implements RubyInterface { private readonly shell = process.env.SHELL?.replace(/(\s+)/g, "\\$1"); private _env: NodeJS.ProcessEnv = {}; private _error = false; - private _pathConverter: PathConverterInterface; + private _pathMapping: Record = {}; private _wrapCommand: (executable: Executable) => Executable; private readonly context: vscode.ExtensionContext; private readonly customBundleGemfile?: string; @@ -92,7 +86,6 @@ export class Ruby implements RubyInterface { this.workspaceFolder = workspaceFolder; this.outputChannel = outputChannel; this.telemetry = telemetry; - this._pathConverter = new PathConverter(); this._wrapCommand = (executable: Executable) => executable; const customBundleGemfile: string = vscode.workspace @@ -122,8 +115,8 @@ export class Ruby implements RubyInterface { } } - get pathConverter() { - return this._pathConverter; + get pathMapping() { + return this._pathMapping; } get env() { @@ -276,7 +269,7 @@ export class Ruby implements RubyInterface { } private async runActivation(manager: VersionManager) { - const { env, version, yjit, gemPath, pathConverter, wrapCommand } = + const { env, version, yjit, gemPath, pathMapping, wrapCommand } = await manager.activate(); const [major, minor, _patch] = version.split(".").map(Number); @@ -289,8 +282,8 @@ export class Ruby implements RubyInterface { this.yjitEnabled = (yjit && major > 3) || (major === 3 && minor >= 2); this.gemPath.push(...gemPath); - if (pathConverter) { - this._pathConverter = pathConverter; + if (pathMapping) { + this._pathMapping = pathMapping; } if (wrapCommand) { this._wrapCommand = wrapCommand; diff --git a/vscode/src/ruby/compose.ts b/vscode/src/ruby/compose.ts index 53a5b81200..57e7ace05a 100644 --- a/vscode/src/ruby/compose.ts +++ b/vscode/src/ruby/compose.ts @@ -7,11 +7,7 @@ import { ExecOptions } from "child_process"; import * as vscode from "vscode"; import { Executable } from "vscode-languageclient/node"; -import { - ComposeConfig, - ContainerPathConverter, - fetchPathMapping, -} from "../docker"; +import { ComposeConfig, fetchPathMapping } from "../docker"; import { parseCommand, spawn } from "../common"; import { @@ -44,7 +40,21 @@ export class Compose extends VersionManager { ).exec(output); const parsedResult = this.parseWithErrorHandling(activationContent![1]); - const pathConverter = await this.buildPathConverter(); + const pathMapping = await this.buildPathMapping(); + + if (parsedResult.gemPath) { + const convertedPaths = (parsedResult.gemPath as string[]).map((path) => { + for (const [local, remote] of Object.entries(pathMapping)) { + if (path.startsWith(remote)) { + return path.replace(remote, local); + } + } + + return path; + }); + + parsedResult.gemPath = convertedPaths; + } const wrapCommand = (executable: Executable) => { const composeCommad = parseCommand( @@ -75,12 +85,12 @@ export class Compose extends VersionManager { yjit: parsedResult.yjit, version: parsedResult.version, gemPath: parsedResult.gemPath, - pathConverter, + pathMapping, wrapCommand, }; } - protected async buildPathConverter() { + protected async buildPathMapping() { const pathMapping = fetchPathMapping( this.composeConfig, this.composeServiceName(), @@ -110,7 +120,7 @@ export class Compose extends VersionManager { {} as Record, ); - return new ContainerPathConverter(filteredMapping, this.outputChannel); + return filteredMapping; } protected composeRunCommand(): string { diff --git a/vscode/src/ruby/versionManager.ts b/vscode/src/ruby/versionManager.ts index e4c4372d35..86184dd01d 100644 --- a/vscode/src/ruby/versionManager.ts +++ b/vscode/src/ruby/versionManager.ts @@ -6,14 +6,14 @@ import * as vscode from "vscode"; import { Executable } from "vscode-languageclient/node"; import { WorkspaceChannel } from "../workspaceChannel"; -import { asyncExec, PathConverterInterface } from "../common"; +import { asyncExec } from "../common"; export interface ActivationResult { env: NodeJS.ProcessEnv; yjit: boolean; version: string; gemPath: string[]; - pathConverter?: PathConverterInterface; + pathMapping?: Record; wrapCommand?: (executable: Executable) => Executable; }