switch exec -> execFile; ShellExecution -> ProcessExecution to easily…

… handle whitespace in paths

README.md

@@ -1,19 +1,18 @@
 # Rust support for Visual Studio Code
 
 [![](https://vsmarketplacebadge.apphb.com/version/rust-lang.rust.svg)](https://marketplace.visualstudio.com/items?itemName=rust-lang.rust)
-[![Build Status](https://travis-ci.com/rust-lang/rls-vscode.svg?branch=master)](https://travis-ci.com/rust-lang/rls-vscode)
+[![Build Status](https://travis-ci.com/AndriesK/rls-vscode.svg?branch=master)](https://travis-ci.com/AndriesK/rls-vscode)
 
 Adds language support for Rust to Visual Studio Code. Supports:
 
-* code completion
-* jump to definition, peek definition, find all references, symbol search
-* types and documentation on hover
-* code formatting
-* refactoring (rename, deglob)
-* error squiggles and apply suggestions from errors
-* snippets
-* build tasks
-
+- code completion
+- jump to definition, peek definition, find all references, symbol search
+- types and documentation on hover
+- code formatting
+- refactoring (rename, deglob)
+- error squiggles and apply suggestions from errors
+- snippets
+- build tasks
 
 Rust support is powered by the [Rust Language Server](https://github.com/rust-lang/rls)
 (RLS). If you don't have it installed, the extension will install it for you.
@@ -33,20 +32,18 @@ advice.
 Contributing code, tests, documentation, and bug reports is appreciated! For
 more details on building and debugging, etc., see [contributing.md](contributing.md).
 
-
 ## Quick start
 
-* Install [rustup](https://www.rustup.rs/) (Rust toolchain manager).
-* Install this extension from [the VSCode Marketplace](https://marketplace.visualstudio.com/items?itemName=rust-lang.rust)
+- Install [rustup](https://www.rustup.rs/) (Rust toolchain manager).
+- Install this extension from [the VSCode Marketplace](https://marketplace.visualstudio.com/items?itemName=rust-lang.rust)
   (or by entering `ext install rust-lang.rust` at the command palette <kbd>Ctrl</kbd>+<kbd>P</kbd>).
-* (Skip this step if you already have Rust projects that you'd like to work on.)
+- (Skip this step if you already have Rust projects that you'd like to work on.)
   Create a new Rust project by following [these instructions](https://doc.rust-lang.org/book/ch01-03-hello-cargo.html).
-* Open a Rust project (`File > Add Folder to Workspace...`). Open the folder for the whole
+- Open a Rust project (`File > Add Folder to Workspace...`). Open the folder for the whole
   project (i.e., the folder containing 'Cargo.toml'), not the 'src' folder.
-* You'll be prompted to install the RLS. Once installed, the RLS should start
+- You'll be prompted to install the RLS. Once installed, the RLS should start
   building your project.
 
-
 ## Configuration
 
 This extension provides options in VSCode's configuration settings. These
@@ -57,11 +54,11 @@ have Intellisense help.
 
 Some highlights:
 
-* `rust.show_warnings` - set to false to silence warnings in the editor.
-* `rust.all_targets` - build and index code for all targets (i.e., integration tests, examples, and benches)
-* `rust.cfg_test` - build and index test code (i.e., code with `#[cfg(test)]`/`#[test]`)
+- `rust.show_warnings` - set to false to silence warnings in the editor.
+- `rust.all_targets` - build and index code for all targets (i.e., integration tests, examples, and benches)
+- `rust.cfg_test` - build and index test code (i.e., code with `#[cfg(test)]`/`#[test]`)
 
-* `rust-client.channel` - specifies from which toolchain the RLS should be spawned
+- `rust-client.channel` - specifies from which toolchain the RLS should be spawned
 
 ## Features
 
@@ -72,16 +69,16 @@ includes snippet names as options when you type; select one by pressing 'enter'.
 You can move to the next 'hole' in the template by pressing 'tab'. We provide
 the following snippets:
 
-* `for` - a for loop
-* `unimplemented`
-* `unreachable`
-* `print(ln)`
-* `assert(_eq)`
-* `macro_rules` - declare a macro
-* `if let Option` - an `if let` statement for executing code only in the `Some`
+- `for` - a for loop
+- `unimplemented`
+- `unreachable`
+- `print(ln)`
+- `assert(_eq)`
+- `macro_rules` - declare a macro
+- `if let Option` - an `if let` statement for executing code only in the `Some`
   case.
-* `spawn` - spawn a thread
-* `extern crate` - insert an `extern crate` statement
+- `spawn` - spawn a thread
+- `extern crate` - insert an `extern crate` statement
 
 This extension is deliberately conservative about snippets and doesn't include
 too many. If you want more, check out
@@ -97,22 +94,19 @@ The plugin writes these into `tasks.json`. The plugin will not overwrite
 existing tasks, so you can customise these tasks. To refresh back to the
 defaults, delete `tasks.json` and restart VSCode.
 
-
 ## Format on save
 
 To enable formatting on save, you need to set the `editor.formatOnSave` setting
 to `true`. Find it under `File > Preferences > Settings`.
 
-
 ## Requirements
 
-* [Rustup](https://www.rustup.rs/),
-* A Rust toolchain (the extension will configure this for you, with
+- [Rustup](https://www.rustup.rs/),
+- A Rust toolchain (the extension will configure this for you, with
   permission),
-* `rls`, `rust-src`, and `rust-analysis` components (the
+- `rls`, `rust-src`, and `rust-analysis` components (the
   extension will install these for you, with permission).
 
-
 ## Implementation
 
 This extension almost exclusively uses the RLS for its feature support (syntax
@@ -121,4 +115,3 @@ the Rust compiler (`rustc`) to get data about Rust programs. It uses Cargo to
 manage building. Both Cargo and `rustc` are run in-process by the RLS. Formatting
 and code completion are provided by `rustfmt` and Racer, again both of these are
 run in-process by the RLS.
-

src/configuration.ts

@@ -79,6 +79,10 @@ export class RLSConfiguration {
     return this.configuration.get('rust-client.rustupPath', 'rustup');
   }
 
+  public get rustupPathQuotes(): string {
+    return '"' + this.rustupPath + '"';
+  }
+
   public get useWSL(): boolean {
     return this.configuration.get<boolean>('rust-client.useWSL', false);
   }
@@ -118,6 +122,10 @@ export class RLSConfiguration {
     return this.configuration.get<string>('rust-client.rlsPath');
   }
 
+  public get rlsPathQuotes(): string | undefined {
+    return this.rlsPath ? '"' + this.rlsPath + '"' : undefined;
+  }
+
   public get multiProjectEnabled(): boolean {
     return this.configuration.get<boolean>(
       'rust-client.enableMultiProjectSetup',
@@ -130,6 +138,7 @@ export class RLSConfiguration {
     return {
       channel: ignoreChannel ? '' : this.channel,
       path: this.rustupPath,
+      pathQuotes: this.rustupPathQuotes,
       useWSL: this.useWSL,
     };
   }

src/extension.ts

@@ -362,8 +362,9 @@ class ClientWorkspace {
     const rustcPrintSysroot = () =>
       this.config.rustupDisabled
         ? wslWrapper.exec('rustc --print sysroot', { env })
-        : wslWrapper.exec(
-            `${this.config.rustupPath} run ${this.config.channel} rustc --print sysroot`,
+        : wslWrapper.execFile(
+            this.config.rustupPath,
+            ['run', this.config.channel, 'rustc', '--print', 'sysroot'],
             { env },
           );
 
@@ -416,7 +417,7 @@ class ClientWorkspace {
 
   private async makeRlsProcess(): Promise<child_process.ChildProcess> {
     // Run "rls" from the PATH unless there's an override.
-    const rlsPath = this.config.rlsPath || 'rls';
+    const rlsPath = this.config.rlsPathQuotes || 'rls';
 
     // We don't need to set [DY]LD_LIBRARY_PATH if we're using rustup,
     // as rustup will set it for us when it chooses a toolchain.
@@ -451,8 +452,9 @@ class ClientWorkspace {
       }
 
       const env = await makeRlsEnv();
+
       childProcess = withWsl(config.useWSL).spawn(
-        config.path,
+        config.pathQuotes,
         ['run', config.channel, rlsPath],
         { env, cwd, shell: true },
       );

src/rustup.ts

@@ -17,16 +17,17 @@ function isInstalledRegex(componentName: string): RegExp {
 export interface RustupConfig {
   channel: string;
   path: string;
+  pathQuotes: string;
   useWSL: boolean;
 }
 
 export async function rustupUpdate(config: RustupConfig) {
   startSpinner('RLS', 'Updating…');
 
   try {
-    const { stdout } = await withWsl(config.useWSL).exec(
-      `${config.path} update`,
-    );
+    const { stdout } = await withWsl(config.useWSL).execFile(config.path, [
+      'update',
+    ]);
 
     // This test is imperfect because if the user has multiple toolchains installed, they
     // might have one updated and one unchanged. But I don't want to go too far down the
@@ -65,13 +66,38 @@ export async function ensureToolchain(config: RustupConfig) {
  * Checks for required RLS components and prompts the user to install if it's
  * not already.
  */
+
 export async function checkForRls(config: RustupConfig) {
   if (await hasRlsComponents(config)) {
     return;
   }
 
+  // Format an easier to understand component install prompt
+  const componentsNeededUserOutput = REQUIRED_COMPONENTS.map((component, i) => {
+    if (
+      REQUIRED_COMPONENTS.length > 1 &&
+      i === REQUIRED_COMPONENTS.length - 1
+    ) {
+      return ' and `' + component + '`';
+    } else if (
+      i !== 0 &&
+      i !== REQUIRED_COMPONENTS.length - 1 &&
+      REQUIRED_COMPONENTS.length > 2
+    ) {
+      return ', `' + component + '`';
+    } else {
+      return '`' + component + '`';
+    }
+  });
   const clicked = await Promise.resolve(
-    window.showInformationMessage('RLS not installed. Install?', 'Yes'),
+    window.showInformationMessage(
+      `
+      Rustup
+      ${
+        componentsNeededUserOutput.length > 1 ? 'components' : 'component'
+      }  ${componentsNeededUserOutput.join('')} not installed. Install?`,
+      'Yes',
+    ),
   );
   if (clicked) {
     await installRlsComponents(config);
@@ -83,9 +109,10 @@ export async function checkForRls(config: RustupConfig) {
 
 async function hasToolchain(config: RustupConfig): Promise<boolean> {
   try {
-    const { stdout } = await withWsl(config.useWSL).exec(
-      `${config.path} toolchain list`,
-    );
+    const { stdout } = await withWsl(config.useWSL).execFile(config.path, [
+      'toolchain',
+      'list',
+    ]);
     return stdout.includes(config.channel);
   } catch (e) {
     console.log(e);
@@ -129,7 +156,7 @@ async function tryToInstallToolchain(config: RustupConfig) {
  */
 async function listComponents(config: RustupConfig): Promise<string[]> {
   return withWsl(config.useWSL)
-    .exec(`${config.path} component list --toolchain ${config.channel}`)
+    .execFile(config.path, ['component', 'list', '--toolchain', config.channel])
     .then(({ stdout }) =>
       stdout
         .toString()
@@ -142,9 +169,20 @@ async function hasRlsComponents(config: RustupConfig): Promise<boolean> {
   try {
     const components = await listComponents(config);
 
-    return REQUIRED_COMPONENTS.map(isInstalledRegex).every(isInstalledRegex =>
-      components.some(c => isInstalledRegex.test(c)),
-    );
+    // Splice the components that are installed from the REQUIRED_COMPONENTS array
+    for (let i = REQUIRED_COMPONENTS.length; i >= 0; --i) {
+      const installedRegex = isInstalledRegex(REQUIRED_COMPONENTS[i]);
+      components.forEach(component => {
+        if (installedRegex.test(component)) {
+          REQUIRED_COMPONENTS.splice(i, 1);
+        }
+      });
+    }
+    if (REQUIRED_COMPONENTS.length === 0) {
+      return true;
+    } else {
+      return false;
+    }
   } catch (e) {
     console.log(e);
     window.showErrorMessage(`Can't detect RLS components: ${e.message}`);
@@ -153,7 +191,7 @@ async function hasRlsComponents(config: RustupConfig): Promise<boolean> {
   }
 }
 
-async function installRlsComponents(config: RustupConfig) {
+export async function installRlsComponents(config: RustupConfig) {
   startSpinner('RLS', 'Installing components…');
 
   for (const component of REQUIRED_COMPONENTS) {
@@ -226,7 +264,9 @@ export function parseActiveToolchain(rustupOutput: string): string {
 export async function getVersion(config: RustupConfig): Promise<string> {
   const versionRegex = /rustup ([0-9]+\.[0-9]+\.[0-9]+)/;
 
-  const output = await withWsl(config.useWSL).exec(`${config.path} --version`);
+  const output = await withWsl(config.useWSL).execFile(config.path, [
+    '--version',
+  ]);
   const versionMatch = output.stdout.toString().match(versionRegex);
   if (versionMatch && versionMatch.length >= 2) {
     return versionMatch[1];
@@ -257,7 +297,7 @@ export function getActiveChannel(wsPath: string, config: RustupConfig): string {
   try {
     // `rustup show active-toolchain` is available since rustup 1.12.0
     activeChannel = withWsl(config.useWSL)
-      .execSync(`${config.path} show active-toolchain`, { cwd: wsPath })
+      .execSync(`${config.pathQuotes} show active-toolchain`, { cwd: wsPath })
       .toString()
       .trim();
     // Since rustup 1.17.0 if the active toolchain is the default, we're told
@@ -268,7 +308,7 @@ export function getActiveChannel(wsPath: string, config: RustupConfig): string {
   } catch (e) {
     // Possibly an old rustup version, so try rustup show
     const showOutput = withWsl(config.useWSL)
-      .execSync(`${config.path} show`, { cwd: wsPath })
+      .execSync(`${config.pathQuotes} show`, { cwd: wsPath })
       .toString();
     activeChannel = parseActiveToolchain(showOutput);
   }

src/tasks.ts

@@ -1,6 +1,7 @@
 import * as crypto from 'crypto';
 import {
   Disposable,
+  ProcessExecution,
   ShellExecution,
   Task,
   TaskGroup,
@@ -127,14 +128,17 @@ export async function runTaskCommand(
   displayName: string,
   folder?: WorkspaceFolder,
 ) {
+  if (!command) {
+    return;
+  }
   const uniqueId = crypto.randomBytes(20).toString();
 
   const task = new Task(
     { label: uniqueId, type: 'shell' },
     folder ? folder : workspace.workspaceFolders![0],
     displayName,
     TASK_SOURCE,
-    new ShellExecution(`${command} ${args.join(' ')}`, {
+    new ProcessExecution(command, args, {
       cwd: cwd || (folder && folder.uri.fsPath),
       env,
     }),

src/utils/child_process.ts

@@ -4,9 +4,11 @@ import * as util from 'util';
 import { modifyParametersForWSL } from './wslpath';
 
 const execAsync = util.promisify(child_process.exec);
+const execFile = util.promisify(child_process.execFile);
 
 export interface SpawnFunctions {
   exec: typeof execAsync;
+  execFile: typeof execFile;
   execSync: typeof child_process.execSync;
   spawn: typeof child_process.spawn;
   modifyArgs: (
@@ -19,12 +21,14 @@ export function withWsl(withWsl: boolean): SpawnFunctions {
   return withWsl
     ? {
         exec: withWslModifiedParameters(execAsync),
+        execFile: withWslModifiedParameters(execFile),
         execSync: withWslModifiedParameters(child_process.execSync),
         spawn: withWslModifiedParameters(child_process.spawn),
         modifyArgs: modifyParametersForWSL,
       }
     : {
         exec: execAsync,
+        execFile,
         execSync: child_process.execSync,
         spawn: child_process.spawn,
         modifyArgs: (command: string, args: string[]) => ({ command, args }),