Merge pull request #767 from rust-lang/workspaces-refactor

Refactor workspace handling

Author: Xanewok

.gitignore

@@ -2,3 +2,4 @@ out
 node_modules
 .vscode-test
 *.vsix
+rls*.log

CHANGELOG.md

@@ -1,5 +1,7 @@
 ### Unreleased
 
+* Remove redundant `rust-client.nestedMultiRootConfigInOutermost` setting (originally used to work around non-multi-project limitations)
+* Ignore setting `rust-client.enableMultiProjectSetup` (it's always on by default)
 * Fix support for multiple VSCode workspaces
 
 ### 0.7.2 - 2020-04-17

fixtures/another-lib-project/Cargo.toml

@@ -0,0 +1,7 @@
+[package]
+name = "bare-lib-project"
+version = "0.1.0"
+authors = ["Example <[email protected]>"]
+edition = "2018"
+
+[dependencies]

fixtures/another-lib-project/src/lib.rs

@@ -0,0 +1,7 @@
+#[cfg(test)]
+mod tests {
+    #[test]
+    fn it_works() {
+        assert_eq!(2 + 2, 4);
+    }
+}

package.json

@@ -227,14 +227,9 @@
                     "description": "Traces the communication between VS Code and the Rust language server.",
                     "scope": "window"
                 },
-                "rust-client.nestedMultiRootConfigInOutermost": {
-                    "type": "boolean",
-                    "default": true,
-                    "description": "If one root workspace folder is nested in another root folder, look for the Rust config in the outermost root."
-                },
                 "rust-client.enableMultiProjectSetup": {
-                    "type": "boolean",
-                    "default": false,
+                    "type": ["boolean", "null"],
+                    "default": null,
                     "description": "Allow multiple projects in the same folder, along with removing the constraint that the cargo.toml must be located at the root. (Experimental: might not work for certain setups)"
                 },
                 "rust.sysroot": {

src/configuration.ts

@@ -118,13 +118,6 @@ export class RLSConfiguration {
     return this.configuration.get<string>('rust-client.rlsPath');
   }
 
-  public get multiProjectEnabled(): boolean {
-    return this.configuration.get<boolean>(
-      'rust-client.enableMultiProjectSetup',
-      false,
-    );
-  }
-
   // Added ignoreChannel for readChannel function. Otherwise we end in an infinite loop.
   public rustupConfig(ignoreChannel: boolean = false): RustupConfig {
     return {

src/extension.ts

@@ -3,6 +3,7 @@ import * as fs from 'fs';
 import * as path from 'path';
 import {
   commands,
+  ConfigurationTarget,
   Disposable,
   ExtensionContext,
   IndentAction,
@@ -28,8 +29,11 @@ import { checkForRls, ensureToolchain, rustupUpdate } from './rustup';
 import { startSpinner, stopSpinner } from './spinner';
 import { activateTaskProvider, Execution, runRlsCommand } from './tasks';
 import { withWsl } from './utils/child_process';
+import {
+  getOuterMostWorkspaceFolder,
+  nearestParentWorkspace,
+} from './utils/workspace';
 import { uriWindowsToWsl, uriWslToWindows } from './utils/wslpath';
-import * as workspace_util from './workspace_util';
 
 /**
  * Parameter type to `window/progress` request as issued by the RLS.
@@ -55,6 +59,33 @@ export async function activate(context: ExtensionContext) {
   // Installed listeners don't fire immediately for already opened files, so
   // trigger an open event manually to fire up RLS instances where needed
   workspace.textDocuments.forEach(whenOpeningTextDocument);
+
+  // Migrate the users of multi-project setup for RLS to disable the setting
+  // entirely (it's always on now)
+  const config = workspace.getConfiguration();
+  if (
+    typeof config.get<boolean | null>(
+      'rust-client.enableMultiProjectSetup',
+      null,
+    ) === 'boolean'
+  ) {
+    window
+      .showWarningMessage(
+        'The multi-project setup for RLS is always enabled, so the `rust-client.enableMultiProjectSetup` setting is now redundant',
+        { modal: false },
+        { title: 'Remove' },
+      )
+      .then(value => {
+        if (value && value.title === 'Remove') {
+          return config.update(
+            'rust-client.enableMultiProjectSetup',
+            null,
+            ConfigurationTarget.Global,
+          );
+        }
+        return;
+      });
+  }
 }
 
 export async function deactivate() {
@@ -67,25 +98,12 @@ function whenOpeningTextDocument(document: TextDocument) {
     return;
   }
 
-  const uri = document.uri;
-  let folder = workspace.getWorkspaceFolder(uri);
+  let folder = workspace.getWorkspaceFolder(document.uri);
   if (!folder) {
     return;
   }
 
-  const inMultiProjectMode = workspace
-    .getConfiguration()
-    .get<boolean>('rust-client.enableMultiProjectSetup', false);
-
-  const inNestedOuterProjectMode = workspace
-    .getConfiguration()
-    .get<boolean>('rust-client.nestedMultiRootConfigInOutermost', true);
-
-  if (inMultiProjectMode) {
-    folder = workspace_util.nearestParentWorkspace(folder, document.uri.fsPath);
-  } else if (inNestedOuterProjectMode) {
-    folder = getOuterMostWorkspaceFolder(folder);
-  }
+  folder = nearestParentWorkspace(folder, document.uri.fsPath);
 
   if (!folder) {
     stopSpinner(`RLS: Cargo.toml missing`);
@@ -105,46 +123,7 @@ function whenOpeningTextDocument(document: TextDocument) {
   }
 }
 
-// This is an intermediate, lazy cache used by `getOuterMostWorkspaceFolder`
-// and cleared when VSCode workspaces change.
-let _sortedWorkspaceFolders: string[] | undefined;
-
-function sortedWorkspaceFolders(): string[] {
-  // TODO: decouple the global state such that it can be moved to workspace_util
-  if (!_sortedWorkspaceFolders && workspace.workspaceFolders) {
-    _sortedWorkspaceFolders = workspace.workspaceFolders
-      .map(folder => {
-        let result = folder.uri.toString();
-        if (result.charAt(result.length - 1) !== '/') {
-          result = result + '/';
-        }
-        return result;
-      })
-      .sort((a, b) => {
-        return a.length - b.length;
-      });
-  }
-  return _sortedWorkspaceFolders || [];
-}
-
-function getOuterMostWorkspaceFolder(folder: WorkspaceFolder): WorkspaceFolder {
-  // TODO: decouple the global state such that it can be moved to workspace_util
-  const sorted = sortedWorkspaceFolders();
-  for (const element of sorted) {
-    let uri = folder.uri.toString();
-    if (uri.charAt(uri.length - 1) !== '/') {
-      uri = uri + '/';
-    }
-    if (uri.startsWith(element)) {
-      return workspace.getWorkspaceFolder(Uri.parse(element)) || folder;
-    }
-  }
-  return folder;
-}
-
 function whenChangingWorkspaceFolders(e: WorkspaceFoldersChangeEvent) {
-  _sortedWorkspaceFolders = undefined;
-
   // If a VSCode workspace has been added, check to see if it is part of an existing one, and
   // if not, and it is a Rust project (i.e., has a Cargo.toml), then create a new client.
   for (let folder of e.added) {
@@ -193,10 +172,6 @@ class ClientWorkspace {
   }
 
   public async start() {
-    if (!this.config.multiProjectEnabled) {
-      warnOnMissingCargoToml();
-    }
-
     startSpinner('RLS', 'Starting');
 
     const serverOptions: ServerOptions = async () => {
@@ -450,16 +425,6 @@ class ClientWorkspace {
   }
 }
 
-async function warnOnMissingCargoToml() {
-  const files = await workspace.findFiles('Cargo.toml');
-
-  if (files.length < 1) {
-    window.showWarningMessage(
-      'A Cargo.toml file must be at the root of the workspace in order to support all features. Alternatively set rust-client.enableMultiProjectSetup=true in settings.',
-    );
-  }
-}
-
 /**
  * Tracks the most current VSCode workspace as opened by the user. Used by the
  * commands to know in which workspace these should be executed.

src/workspace_util.ts renamed to src/utils/workspace.ts

@@ -1,6 +1,6 @@
 import * as fs from 'fs';
 import * as path from 'path';
-import { Uri, WorkspaceFolder } from 'vscode';
+import { Uri, workspace, WorkspaceFolder } from 'vscode';
 
 // searches up the folder structure until it finds a Cargo.toml
 export function nearestParentWorkspace(
@@ -44,3 +44,30 @@ export function nearestParentWorkspace(
 
   return curWorkspace;
 }
+
+export function getOuterMostWorkspaceFolder(
+  folder: WorkspaceFolder,
+): WorkspaceFolder {
+  const sortedFoldersByPrefix = (workspace.workspaceFolders || [])
+    .map(folder => normalizeUriToPathPrefix(folder.uri))
+    .sort((a, b) => a.length - b.length);
+
+  const uri = normalizeUriToPathPrefix(folder.uri);
+
+  const outermostPath = sortedFoldersByPrefix.find(pre => uri.startsWith(pre));
+  return outermostPath
+    ? workspace.getWorkspaceFolder(Uri.parse(outermostPath)) || folder
+    : folder;
+}
+
+/**
+ * Transforms a given URI to a path prefix, namely ensures that each path
+ * segment ends with a path separator `/`.
+ */
+function normalizeUriToPathPrefix(uri: Uri): string {
+  let result = uri.toString();
+  if (result.charAt(result.length - 1) !== '/') {
+    result = result + '/';
+  }
+  return result;
+}

test/suite/extension.test.ts

@@ -10,16 +10,24 @@ const fixtureDir = path.resolve(
 
 suite('Extension Tests', () => {
   test('cargo tasks are auto-detected', async () => {
-    const projectPath = path.join(fixtureDir, 'bare-lib-project');
+    const projectPath = fixtureDir;
     const projectUri = Uri.file(projectPath);
+    const projects = [
+      path.join(projectPath, 'bare-lib-project'),
+      path.join(projectPath, 'another-lib-project'),
+    ];
 
     await vscode.commands.executeCommand('vscode.openFolder', projectUri);
     await vscode.workspace.openTextDocument(
-      Uri.file(path.join(projectPath, 'src', 'lib.rs')),
+      Uri.file(path.join(projects[0], 'src', 'lib.rs')),
+    );
+    await vscode.workspace.openTextDocument(
+      Uri.file(path.join(projects[1], 'src', 'lib.rs')),
     );
 
     const expected = [
-      { subcommand: 'build', group: vscode.TaskGroup.Build },
+      { subcommand: 'build', group: vscode.TaskGroup.Build, cwd: projects[0] },
+      { subcommand: 'build', group: vscode.TaskGroup.Build, cwd: projects[1] },
       { subcommand: 'check', group: vscode.TaskGroup.Build },
       { subcommand: 'test', group: vscode.TaskGroup.Test },
       { subcommand: 'clean', group: vscode.TaskGroup.Clean },
@@ -28,13 +36,18 @@ suite('Extension Tests', () => {
 
     const tasks = await vscode.tasks.fetchTasks();
 
-    for (const { subcommand, group } of expected) {
+    for (const { subcommand, group, cwd } of expected) {
       assert(
         tasks.some(
           task =>
             task.definition.type === 'cargo' &&
             task.definition.subcommand === subcommand &&
-            task.group === group,
+            task.group === group &&
+            (!cwd ||
+              cwd ===
+                (task.execution &&
+                  task.execution.options &&
+                  task.execution.options.cwd)),
         ),
       );
     }