Replace LS bundling with extension API

.gitignore

@@ -42,4 +42,3 @@ ptvsd*.log
 pydevd*.log
 nodeLanguageServer/**
 nodeLanguageServer.*/**
-bundledLanguageServer/**

src/client/activation/common/downloader.ts

@@ -59,9 +59,9 @@ export class LanguageServerDownloader implements ILanguageServerDownloader {
     }
 
     public async downloadLanguageServer(destinationFolder: string, resource: Resource): Promise<void> {
-        if (this.lsFolderService.isBundled()) {
-            // Sanity check; a bundled LS should never be downloaded.
-            traceError('Attempted to download bundled language server');
+        if (await this.lsFolderService.skipDownload()) {
+            // Sanity check; this case should not be hit if skipDownload is true elsewhere.
+            traceError('Attempted to download with skipDownload true.');
             return;
         }
 

src/client/activation/common/languageServerFolderService.ts

@@ -26,7 +26,7 @@ export abstract class LanguageServerFolderService implements ILanguageServerFold
         @unmanaged() protected readonly languageServerFolder: string
     ) {}
 
-    public isBundled(): boolean {
+    public async skipDownload(): Promise<boolean> {
         return false;
     }
 

src/client/activation/node/languageClientFactory.ts

@@ -31,9 +31,13 @@ export class NodeLanguageClientFactory implements ILanguageClientFactory {
         const commandArgs = (clientOptions.connectionOptions
             ?.cancellationStrategy as FileBasedCancellationStrategy).getCommandLineArguments();
 
-        const languageServerFolder = await this.languageServerFolderService.getLanguageServerFolderName(resource);
-        const bundlePath = path.join(EXTENSION_ROOT_DIR, languageServerFolder, 'server.bundle.js');
-        const nonBundlePath = path.join(EXTENSION_ROOT_DIR, languageServerFolder, 'server.js');
+        const folderName = await this.languageServerFolderService.getLanguageServerFolderName(resource);
+        const languageServerFolder = path.isAbsolute(folderName)
+            ? folderName
+            : path.join(EXTENSION_ROOT_DIR, folderName);
+
+        const bundlePath = path.join(languageServerFolder, 'server.bundle.js');
+        const nonBundlePath = path.join(languageServerFolder, 'server.js');
         const modulePath = (await this.fs.fileExists(nonBundlePath)) ? nonBundlePath : bundlePath;
         const debugOptions = { execArgv: ['--nolazy', '--inspect=6600'] };
 

src/client/activation/node/languageServerFolderService.ts

@@ -3,23 +3,16 @@
 
 'use strict';
 
+import * as assert from 'assert';
 import { inject, injectable } from 'inversify';
-import * as semver from 'semver';
-import { IApplicationEnvironment, IWorkspaceService } from '../../common/application/types';
+import * as path from 'path';
+import { SemVer } from 'semver';
+import { IWorkspaceService } from '../../common/application/types';
 import { NugetPackage } from '../../common/nuget/types';
-import { IConfigurationService, Resource } from '../../common/types';
+import { IConfigurationService, IExtensions, Resource } from '../../common/types';
 import { IServiceContainer } from '../../ioc/types';
-import { traceWarning } from '../../logging';
 import { LanguageServerFolderService } from '../common/languageServerFolderService';
-import {
-    BundledLanguageServerFolder,
-    FolderVersionPair,
-    ILanguageServerFolderService,
-    NodeLanguageServerFolder
-} from '../types';
-
-// Must match languageServerVersion* keys in package.json
-export const NodeLanguageServerVersionKey = 'languageServerVersionV2';
+import { FolderVersionPair, ILanguageServerFolderService, NodeLanguageServerFolder } from '../types';
 
 class FallbackNodeLanguageServerFolderService extends LanguageServerFolderService {
     constructor(serviceContainer: IServiceContainer) {
@@ -31,62 +24,95 @@ class FallbackNodeLanguageServerFolderService extends LanguageServerFolderServic
     }
 }
 
+// Exported for testing.
+export interface ILanguageServerFolder {
+    path: string;
+    version: string; // SemVer, in string form to avoid cross-extension type issues.
+}
+
+// Exported for testing.
+export interface ILSExtensionApi {
+    languageServerFolder?(): Promise<ILanguageServerFolder>;
+}
+
 @injectable()
 export class NodeLanguageServerFolderService implements ILanguageServerFolderService {
-    private readonly _bundledVersion: semver.SemVer | undefined;
     private readonly fallback: FallbackNodeLanguageServerFolderService;
 
     constructor(
         @inject(IServiceContainer) serviceContainer: IServiceContainer,
-        @inject(IConfigurationService) configService: IConfigurationService,
-        @inject(IWorkspaceService) workspaceService: IWorkspaceService,
-        @inject(IApplicationEnvironment) appEnv: IApplicationEnvironment
+        @inject(IConfigurationService) private configService: IConfigurationService,
+        @inject(IWorkspaceService) private workspaceService: IWorkspaceService,
+        @inject(IExtensions) readonly extensions: IExtensions
     ) {
         this.fallback = new FallbackNodeLanguageServerFolderService(serviceContainer);
-
-        // downloadLanguageServer is a bit of a misnomer; if false then this indicates that a local
-        // development copy should be run instead of a "real" build, telemetry discarded, etc.
-        // So, we require it to be true, even though in the bundled case no real download happens.
-        if (
-            configService.getSettings().downloadLanguageServer &&
-            !workspaceService.getConfiguration('python').get<string>('packageName')
-        ) {
-            const ver = appEnv.packageJson[NodeLanguageServerVersionKey] as string;
-            this._bundledVersion = semver.parse(ver) || undefined;
-            if (this._bundledVersion === undefined) {
-                traceWarning(
-                    `invalid language server version ${ver} in package.json (${NodeLanguageServerVersionKey})`
-                );
-            }
-        }
-    }
-
-    public get bundledVersion(): semver.SemVer | undefined {
-        return this._bundledVersion;
     }
 
-    public isBundled(): boolean {
-        return this._bundledVersion !== undefined;
+    public async skipDownload(): Promise<boolean> {
+        return (await this.lsExtensionApi()) !== undefined;
     }
 
     public async getLanguageServerFolderName(resource: Resource): Promise<string> {
-        if (this._bundledVersion) {
-            return BundledLanguageServerFolder;
+        const lsf = await this.languageServerFolder();
+        if (lsf) {
+            assert.ok(path.isAbsolute(lsf.path));
+            return lsf.path;
         }
         return this.fallback.getLanguageServerFolderName(resource);
     }
 
     public async getLatestLanguageServerVersion(resource: Resource): Promise<NugetPackage | undefined> {
-        if (this._bundledVersion) {
+        if (await this.lsExtensionApi()) {
             return undefined;
         }
         return this.fallback.getLatestLanguageServerVersion(resource);
     }
 
     public async getCurrentLanguageServerDirectory(): Promise<FolderVersionPair | undefined> {
-        if (this._bundledVersion) {
-            return { path: BundledLanguageServerFolder, version: this._bundledVersion };
+        const lsf = await this.languageServerFolder();
+        if (lsf) {
+            assert.ok(path.isAbsolute(lsf.path));
+            return {
+                path: lsf.path,
+                version: new SemVer(lsf.version)
+            };
         }
         return this.fallback.getCurrentLanguageServerDirectory();
     }
+
+    protected async languageServerFolder(): Promise<ILanguageServerFolder | undefined> {
+        const extension = await this.lsExtensionApi();
+        if (!extension?.languageServerFolder) {
+            return undefined;
+        }
+        return extension.languageServerFolder();
+    }
+
+    private async lsExtensionApi(): Promise<ILSExtensionApi | undefined> {
+        // downloadLanguageServer is a bit of a misnomer; if false then this indicates that a local
+        // development copy should be run instead of a "real" build, telemetry discarded, etc.
+        // So, we require it to be true, even though in the pinned case no real download happens.
+        if (
+            !this.configService.getSettings().downloadLanguageServer ||
+            this.workspaceService.getConfiguration('python').get<string>('packageName')
+        ) {
+            return undefined;
+        }
+
+        const extensionName = this.workspaceService.getConfiguration('python').get<string>('lsExtensionName');
+        if (!extensionName) {
+            return undefined;
+        }
+
+        const extension = this.extensions.getExtension<ILSExtensionApi>(extensionName);
+        if (!extension) {
+            return undefined;
+        }
+
+        if (!extension.isActive) {
+            return extension.activate();
+        }
+
+        return extension.exports;
+    }
 }

src/client/activation/types.ts

@@ -72,7 +72,6 @@ export enum LanguageServerType {
 
 export const DotNetLanguageServerFolder = 'languageServer';
 export const NodeLanguageServerFolder = 'nodeLanguageServer';
-export const BundledLanguageServerFolder = 'bundledLanguageServer';
 
 // tslint:disable-next-line: interface-name
 export interface DocumentHandler {
@@ -117,7 +116,7 @@ export interface ILanguageServerFolderService {
     getLanguageServerFolderName(resource: Resource): Promise<string>;
     getLatestLanguageServerVersion(resource: Resource): Promise<NugetPackage | undefined>;
     getCurrentLanguageServerDirectory(): Promise<FolderVersionPair | undefined>;
-    isBundled(): boolean;
+    skipDownload(): Promise<boolean>;
 }
 
 export const ILanguageServerDownloader = Symbol('ILanguageServerDownloader');

src/test/activation/languageServer/downloader.unit.test.ts

@@ -321,7 +321,7 @@ suite('Language Server Activation - Downloader', () => {
             );
         });
         test('Display error message if LS downloading fails', async () => {
-            folderService.setup((f) => f.isBundled()).returns(() => false);
+            folderService.setup((f) => f.skipDownload()).returns(async () => false);
             const pkg = makePkgInfo('ls', 'xyz');
             folderService.setup((f) => f.getLatestLanguageServerVersion(resource)).returns(() => Promise.resolve(pkg));
             output.setup((o) => o.appendLine(LanguageService.downloadFailedOutputMessage()));
@@ -345,7 +345,7 @@ suite('Language Server Activation - Downloader', () => {
             platformData.verifyAll();
         });
         test('Display error message if LS extraction fails', async () => {
-            folderService.setup((f) => f.isBundled()).returns(() => false);
+            folderService.setup((f) => f.skipDownload()).returns(async () => false);
             const pkg = makePkgInfo('ls', 'xyz');
             folderService.setup((f) => f.getLatestLanguageServerVersion(resource)).returns(() => Promise.resolve(pkg));
             output.setup((o) => o.appendLine(LanguageService.extractionFailedOutputMessage()));
@@ -369,7 +369,7 @@ suite('Language Server Activation - Downloader', () => {
             platformData.verifyAll();
         });
         test('No download if bundled', async () => {
-            folderService.setup((f) => f.isBundled()).returns(() => true);
+            folderService.setup((f) => f.skipDownload()).returns(async () => true);
 
             await languageServerBundledTest.downloadLanguageServer('', resource);
 

src/test/activation/languageServer/languageServerFolderService.unit.test.ts

@@ -268,14 +268,15 @@ suite('Language Server Folder Service', () => {
         });
     });
 
-    suite('Method isBundled()', () => {
+    suite('Method skipDownload()', () => {
         setup(() => {
             serviceContainer = TypeMoq.Mock.ofType<IServiceContainer>();
             languageServerFolderService = new DotNetLanguageServerFolderService(serviceContainer.object);
         });
 
-        test('isBundled is false', () => {
-            expect(languageServerFolderService.isBundled()).to.be.equal(false, 'isBundled should be false');
+        test('skipDownload is false', async () => {
+            const skipDownload = await languageServerFolderService.skipDownload();
+            expect(skipDownload).to.be.equal(false, 'skipDownload should be false');
         });
     });
 });