From 9b9c821703d787b17d661df7f2c994683d39a554 Mon Sep 17 00:00:00 2001 From: Tom van Ommeren Date: Sat, 5 Aug 2017 15:26:13 +0200 Subject: [PATCH 01/15] feat: hackish implementation of plugin loading for tslint --- package.json | 1 + src/project-manager.ts | 141 ++++++++++++++++++++++++++++++++++++++ src/typescript-service.ts | 11 +-- tsconfig.json | 5 +- 4 files changed, 148 insertions(+), 10 deletions(-) diff --git a/package.json b/package.json index cd165385a..e2836f12e 100644 --- a/package.json +++ b/package.json @@ -79,6 +79,7 @@ "source-map-support": "^0.4.11", "temp": "^0.8.3", "tslint": "^5.0.0", + "tslint-language-service": "^0.9.6", "validate-commit-msg": "^2.12.2" }, "bin": { diff --git a/src/project-manager.ts b/src/project-manager.ts index 96831cc41..e40c6214f 100644 --- a/src/project-manager.ts +++ b/src/project-manager.ts @@ -24,6 +24,32 @@ import { export type ConfigType = 'js' | 'ts'; +interface Project { + projectService: { + logger: Logger; + }; +} + +type ServerHost = any; + +type RequireResult = { module: {}, error: undefined } | { module: undefined, error: {} }; + +// copied from TypeScript server/project.ts +interface PluginCreateInfo { + project: Project; + languageService: ts.LanguageService; + languageServiceHost: ts.LanguageServiceHost; + serverHost: ServerHost; + config: any; +} + +interface PluginModule { + create(createInfo: PluginCreateInfo): ts.LanguageService; + getExternalFiles?(proj: Project): string[]; +} + +type PluginModuleFactory = (mod: { typescript: typeof ts }) => PluginModule; + /** * ProjectManager translates VFS files to one or many projects denoted by [tj]config.json. * It uses either local or remote file system to fetch directory tree and files from and then @@ -910,9 +936,124 @@ export class ProjectConfiguration { this.logger ); this.service = ts.createLanguageService(this.host, this.documentRegistry); + this.enablePlugins(options); this.initialized = true; } + private serverHostRequire(initialDir: string, moduleName: string): RequireResult { + // const modulePath = ts.resolveJavaScriptModule(moduleName, initialDir, this.fs)); + const modulePath = initialDir + '/' + moduleName; + try { + return { module: require(modulePath), error: undefined }; + } catch (error) { + return { module: undefined, error }; + } + } + + // // marked @internal in ts :( + // private resolveJavaScriptModule(moduleName: string, initialDir: string, host: ts.ModuleResolutionHost): string { + // const { resolvedModule, failedLookupLocations } = + // ts.nodeModuleNameResolverWorker(moduleName, initialDir, { moduleResolution: ts.ModuleResolutionKind.NodeJs, allowJs: true }, host, /*cache*/ undefined, /*jsOnly*/ true); + // if (!resolvedModule) { + // throw new Error(`Could not resolve JS module ${moduleName} starting at ${initialDir}. Looked in: ${failedLookupLocations.join(', ')}`); + // } + // return resolvedModule.resolvedFileName; + // } + + enablePlugins(options: ts.CompilerOptions) { + // const host = this.getHost(); + // const options = this.getCompilerOptions(); + + // if (!host.require) { + // this.logger.info("Plugins were requested but not running in environment that supports 'require'. Nothing will be loaded"); + // return; + // } + + // Search our peer node_modules, then any globally-specified probe paths + // ../../.. to walk from X/node_modules/typescript/lib/tsserver.js to X/node_modules/ + // const searchPaths = [combinePaths(host.getExecutingFilePath(), "../../.."), ...this.projectService.pluginProbeLocations]; + const searchPaths = []; + // if (this.projectService.allowLocalPluginLoads) { + // const local = ts.getDirectoryPath(this.configFilePath); + const local = this.rootFilePath; + this.logger.info(`enablePlugins for ${this.configFilePath}`); + this.logger.info(`Local plugin loading enabled; adding ${local} to search paths`); + searchPaths.unshift(local); + // } + + // Enable tsconfig-specified plugins + if (options.plugins) { + for (const pluginConfigEntry of options.plugins as ts.PluginImport[]) { + this.enablePlugin(pluginConfigEntry, searchPaths); + } + } + const file = ''; + file.toLowerCase(); + + // if (this.projectService.globalPlugins) { + // // Enable global plugins with synthetic configuration entries + // for (const globalPluginName of this.projectService.globalPlugins) { + // // Skip already-locally-loaded plugins + // if (options.plugins && options.plugins.some(p => p.name === globalPluginName)) continue; + + // // Provide global: true so plugins can detect why they can't find their config + // this.enablePlugin({ name: globalPluginName, global: true } as PluginImport, searchPaths); + // } + // } + } + + private resolveModule(moduleName: string, initialDir: string): {} | undefined { + // const resolvedPath = ts.normalizeSlashes(host.resolvePath(ts.combinePaths(initialDir, 'node_modules'))); + const pathResolver = initialDir.includes('\\') ? path.win32 : path.posix; + + const resolvedPath = pathResolver.resolve(initialDir, 'node_modules'); + this.logger.info(`Loading ${moduleName} from ${initialDir} (resolved to ${resolvedPath})`); + const result = this.serverHostRequire(resolvedPath, moduleName); + if (result.error) { + this.logger.info(`Failed to load module: ${JSON.stringify(result.error)}`); + return undefined; + } + return result.module; + } + + private enablePlugin(pluginConfigEntry: ts.PluginImport, searchPaths: string[]) { + // const log = (message: string) => { + // this.logger.info(message); + // }; + + for (const searchPath of searchPaths) { + const resolvedModule = this.resolveModule(pluginConfigEntry.name, searchPath) as PluginModuleFactory; + if (resolvedModule) { + this.enableProxy(resolvedModule, pluginConfigEntry); + return; + } + } + this.logger.info(`Couldn't find ${pluginConfigEntry.name} anywhere in paths: ${searchPaths.join(',')}`); + } + + private enableProxy(pluginModuleFactory: PluginModuleFactory, configEntry: ts.PluginImport) { + try { + if (typeof pluginModuleFactory !== 'function') { + this.logger.info(`Skipped loading plugin ${configEntry.name} because it did expose a proper factory function`); + return; + } + + const info: PluginCreateInfo = { + config: configEntry, + project: { projectService: { logger: this.logger }}, // TODO: this will never work. + languageService: this.getService(), + languageServiceHost: this.getHost(), + serverHost: undefined // TODO: may need to be faked. + }; + + const pluginModule = pluginModuleFactory({ typescript: ts }); + this.service = pluginModule.create(info); + // this.plugins.push(pluginModule); TODO: is this needed? + } catch (e) { + this.logger.info(`Plugin activation failed: ${e}`); + } + } + /** * Ensures we are ready to process files from a given sub-project */ diff --git a/src/typescript-service.ts b/src/typescript-service.ts index fb1030452..e1441d2fa 100644 --- a/src/typescript-service.ts +++ b/src/typescript-service.ts @@ -1337,15 +1337,8 @@ export class TypeScriptService { if (!config) { return; } - const program = config.getProgram(span); - if (!program) { - return; - } - const sourceFile = program.getSourceFile(uri2path(uri)); - if (!sourceFile) { - return; - } - const tsDiagnostics = ts.getPreEmitDiagnostics(program, sourceFile); + const fileName = uri2path(uri); + const tsDiagnostics = config.getService().getSyntacticDiagnostics(fileName).concat(config.getService().getSemanticDiagnostics(fileName)); const diagnostics = iterate(tsDiagnostics) // TS can report diagnostics without a file and range in some cases // These cannot be represented as LSP Diagnostics since the range and URI is required diff --git a/tsconfig.json b/tsconfig.json index 8a08a9727..3636865eb 100755 --- a/tsconfig.json +++ b/tsconfig.json @@ -16,7 +16,10 @@ "noImplicitThis": true, "noUnusedLocals": true, "noImplicitAny": true, - "strictNullChecks": true + "strictNullChecks": true, + "plugins": [ + { "name": "tslint-language-service"} + ] }, "include": [ "src/**/*.ts" From 112102d25c470340911df81c5955dea86d0ff80d Mon Sep 17 00:00:00 2001 From: Tom van Ommeren Date: Sun, 6 Aug 2017 20:20:14 +0200 Subject: [PATCH 02/15] fix: Improve plugin import logic Also clean up unused code adding TODOs --- src/project-manager.ts | 118 +++++++++++++++++------------------------ 1 file changed, 49 insertions(+), 69 deletions(-) diff --git a/src/project-manager.ts b/src/project-manager.ts index e40c6214f..6868c3f19 100644 --- a/src/project-manager.ts +++ b/src/project-manager.ts @@ -24,17 +24,15 @@ import { export type ConfigType = 'js' | 'ts'; +// definitions from from TypeScript server/project.ts interface Project { projectService: { logger: Logger; }; } -type ServerHost = any; +type ServerHost = object; -type RequireResult = { module: {}, error: undefined } | { module: undefined, error: {} }; - -// copied from TypeScript server/project.ts interface PluginCreateInfo { project: Project; languageService: ts.LanguageService; @@ -43,6 +41,8 @@ interface PluginCreateInfo { config: any; } +type RequireResult = { module: {}, error: undefined } | { module: undefined, error: {} }; + interface PluginModule { create(createInfo: PluginCreateInfo): ts.LanguageService; getExternalFiles?(proj: Project): string[]; @@ -940,46 +940,19 @@ export class ProjectConfiguration { this.initialized = true; } - private serverHostRequire(initialDir: string, moduleName: string): RequireResult { - // const modulePath = ts.resolveJavaScriptModule(moduleName, initialDir, this.fs)); - const modulePath = initialDir + '/' + moduleName; - try { - return { module: require(modulePath), error: undefined }; - } catch (error) { - return { module: undefined, error }; - } - } - - // // marked @internal in ts :( - // private resolveJavaScriptModule(moduleName: string, initialDir: string, host: ts.ModuleResolutionHost): string { - // const { resolvedModule, failedLookupLocations } = - // ts.nodeModuleNameResolverWorker(moduleName, initialDir, { moduleResolution: ts.ModuleResolutionKind.NodeJs, allowJs: true }, host, /*cache*/ undefined, /*jsOnly*/ true); - // if (!resolvedModule) { - // throw new Error(`Could not resolve JS module ${moduleName} starting at ${initialDir}. Looked in: ${failedLookupLocations.join(', ')}`); - // } - // return resolvedModule.resolvedFileName; - // } - - enablePlugins(options: ts.CompilerOptions) { - // const host = this.getHost(); - // const options = this.getCompilerOptions(); - - // if (!host.require) { - // this.logger.info("Plugins were requested but not running in environment that supports 'require'. Nothing will be loaded"); - // return; - // } - - // Search our peer node_modules, then any globally-specified probe paths - // ../../.. to walk from X/node_modules/typescript/lib/tsserver.js to X/node_modules/ - // const searchPaths = [combinePaths(host.getExecutingFilePath(), "../../.."), ...this.projectService.pluginProbeLocations]; + private enablePlugins(options: ts.CompilerOptions) { const searchPaths = []; - // if (this.projectService.allowLocalPluginLoads) { - // const local = ts.getDirectoryPath(this.configFilePath); - const local = this.rootFilePath; - this.logger.info(`enablePlugins for ${this.configFilePath}`); - this.logger.info(`Local plugin loading enabled; adding ${local} to search paths`); - searchPaths.unshift(local); - // } + // TODO: support pluginProbeLocations? + // TODO: add peer node_modules to source path. + + // TODO: determine how to expose this setting. + // VS Code starts tsserver with --allowLocalPluginLoads by default: https://github.com/Microsoft/TypeScript/pull/15924 + const allowLocalPluginLoads = true; + if (allowLocalPluginLoads) { + const local = this.rootFilePath; + this.logger.info(`Local plugin loading enabled; adding ${local} to search paths`); + searchPaths.unshift(local); + } // Enable tsconfig-specified plugins if (options.plugins) { @@ -987,28 +960,14 @@ export class ProjectConfiguration { this.enablePlugin(pluginConfigEntry, searchPaths); } } - const file = ''; - file.toLowerCase(); - - // if (this.projectService.globalPlugins) { - // // Enable global plugins with synthetic configuration entries - // for (const globalPluginName of this.projectService.globalPlugins) { - // // Skip already-locally-loaded plugins - // if (options.plugins && options.plugins.some(p => p.name === globalPluginName)) continue; - - // // Provide global: true so plugins can detect why they can't find their config - // this.enablePlugin({ name: globalPluginName, global: true } as PluginImport, searchPaths); - // } - // } + + // TODO: support globalPlugins flag if desired } private resolveModule(moduleName: string, initialDir: string): {} | undefined { - // const resolvedPath = ts.normalizeSlashes(host.resolvePath(ts.combinePaths(initialDir, 'node_modules'))); - const pathResolver = initialDir.includes('\\') ? path.win32 : path.posix; - - const resolvedPath = pathResolver.resolve(initialDir, 'node_modules'); + const resolvedPath = path.resolve(initialDir, 'node_modules'); this.logger.info(`Loading ${moduleName} from ${initialDir} (resolved to ${resolvedPath})`); - const result = this.serverHostRequire(resolvedPath, moduleName); + const result = this.requirePlugin(resolvedPath, moduleName); if (result.error) { this.logger.info(`Failed to load module: ${JSON.stringify(result.error)}`); return undefined; @@ -1016,11 +975,33 @@ export class ProjectConfiguration { return result.module; } - private enablePlugin(pluginConfigEntry: ts.PluginImport, searchPaths: string[]) { - // const log = (message: string) => { - // this.logger.info(message); - // }; + // TODO: stolen from moduleNameResolver.ts because marked as internal + /** + * Expose resolution logic to allow us to use Node module resolution logic from arbitrary locations. + * No way to do this with `require()`: https://github.com/nodejs/node/issues/5963 + * Throws an error if the module can't be resolved. + */ + private resolveJavaScriptModule(moduleName: string, initialDir: string, host: ts.ModuleResolutionHost): string { + const { resolvedModule /* , failedLookupLocations */ } = + ts.nodeModuleNameResolver(moduleName, /* containingFile */ initialDir, { moduleResolution: ts.ModuleResolutionKind.NodeJs, allowJs: true }, this.fs, undefined); + // TODO: jsOnly flag missing :( + if (!resolvedModule) { + // TODO: add Looked in: ${failedLookupLocations.join(', ')} back into error. + throw new Error(`Could not resolve JS module ${moduleName} starting at ${initialDir}.`); + } + return resolvedModule.resolvedFileName; + } + private requirePlugin(initialDir: string, moduleName: string): RequireResult { + const modulePath = this.resolveJavaScriptModule(moduleName, initialDir, this.fs); + try { + return { module: require(modulePath), error: undefined }; + } catch (error) { + return { module: undefined, error }; + } + } + + private enablePlugin(pluginConfigEntry: ts.PluginImport, searchPaths: string[]) { for (const searchPath of searchPaths) { const resolvedModule = this.resolveModule(pluginConfigEntry.name, searchPath) as PluginModuleFactory; if (resolvedModule) { @@ -1034,21 +1015,20 @@ export class ProjectConfiguration { private enableProxy(pluginModuleFactory: PluginModuleFactory, configEntry: ts.PluginImport) { try { if (typeof pluginModuleFactory !== 'function') { - this.logger.info(`Skipped loading plugin ${configEntry.name} because it did expose a proper factory function`); + this.logger.info(`Skipped loading plugin ${configEntry.name} because it didn't expose a proper factory function`); return; } const info: PluginCreateInfo = { config: configEntry, - project: { projectService: { logger: this.logger }}, // TODO: this will never work. + project: { projectService: { logger: this.logger }}, // TODO: may need more support languageService: this.getService(), languageServiceHost: this.getHost(), - serverHost: undefined // TODO: may need to be faked. + serverHost: {} // TODO: may need an adapter }; const pluginModule = pluginModuleFactory({ typescript: ts }); this.service = pluginModule.create(info); - // this.plugins.push(pluginModule); TODO: is this needed? } catch (e) { this.logger.info(`Plugin activation failed: ${e}`); } From 77429ce75d97e56a49cd2675be9e99ed55efbcbc Mon Sep 17 00:00:00 2001 From: Tom van Ommeren Date: Sun, 6 Aug 2017 20:20:40 +0200 Subject: [PATCH 03/15] fix: don't override diagnostic source if provided --- src/diagnostics.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/diagnostics.ts b/src/diagnostics.ts index ca239d370..bf0397262 100644 --- a/src/diagnostics.ts +++ b/src/diagnostics.ts @@ -18,7 +18,7 @@ export function convertTsDiagnostic(diagnostic: ts.Diagnostic): Diagnostic { message: text, severity: convertDiagnosticCategory(diagnostic.category), code: diagnostic.code, - source: 'ts' + source: diagnostic.source || 'ts' }; } From 3df438cdd52a17030f04690989768770a9797a51 Mon Sep 17 00:00:00 2001 From: Tom van Ommeren Date: Mon, 7 Aug 2017 23:42:05 +0200 Subject: [PATCH 04/15] fix: Add documentation and reorder functions --- src/project-manager.ts | 104 ++++++++++++++++++++++++++++------------- 1 file changed, 72 insertions(+), 32 deletions(-) diff --git a/src/project-manager.ts b/src/project-manager.ts index 6868c3f19..95bf80f1f 100644 --- a/src/project-manager.ts +++ b/src/project-manager.ts @@ -25,14 +25,24 @@ import { export type ConfigType = 'js' | 'ts'; // definitions from from TypeScript server/project.ts -interface Project { - projectService: { - logger: Logger; - }; -} -type ServerHost = object; +/** + * A plugin exports an initialization function, injected with + * the current typescript instance + */ +type PluginModuleFactory = (mod: { typescript: typeof ts }) => PluginModule; + +/** + * A plugin presents this API when initialized + */ +interface PluginModule { + create(createInfo: PluginCreateInfo): ts.LanguageService; + getExternalFiles?(proj: Project): string[]; +} +/** + * All of tsserver's environment exposed to plugins + */ interface PluginCreateInfo { project: Project; languageService: ts.LanguageService; @@ -41,14 +51,24 @@ interface PluginCreateInfo { config: any; } -type RequireResult = { module: {}, error: undefined } | { module: undefined, error: {} }; - -interface PluginModule { - create(createInfo: PluginCreateInfo): ts.LanguageService; - getExternalFiles?(proj: Project): string[]; +/** + * The portion of tsserver's Project API exposed to plugins + */ +interface Project { + projectService: { + logger: Logger; + }; } -type PluginModuleFactory = (mod: { typescript: typeof ts }) => PluginModule; +/** + * The portion of tsserver's ServerHost API exposed to plugins + */ +type ServerHost = object; + +/** + * The result of a node require: a module or an error. + */ +type RequireResult = { module: {}, error: undefined } | { module: undefined, error: {} }; /** * ProjectManager translates VFS files to one or many projects denoted by [tj]config.json. @@ -964,6 +984,27 @@ export class ProjectConfiguration { // TODO: support globalPlugins flag if desired } + /** + * Tries to load and enable a single plugin + * @param pluginConfigEntry + * @param searchPaths + */ + private enablePlugin(pluginConfigEntry: ts.PluginImport, searchPaths: string[]) { + for (const searchPath of searchPaths) { + const resolvedModule = this.resolveModule(pluginConfigEntry.name, searchPath) as PluginModuleFactory; + if (resolvedModule) { + this.enableProxy(resolvedModule, pluginConfigEntry); + return; + } + } + this.logger.info(`Couldn't find ${pluginConfigEntry.name} anywhere in paths: ${searchPaths.join(',')}`); + } + + /** + * Load a plugin use a node require + * @param moduleName + * @param initialDir + */ private resolveModule(moduleName: string, initialDir: string): {} | undefined { const resolvedPath = path.resolve(initialDir, 'node_modules'); this.logger.info(`Loading ${moduleName} from ${initialDir} (resolved to ${resolvedPath})`); @@ -975,6 +1016,20 @@ export class ProjectConfiguration { return result.module; } + /** + * Resolves a loads a plugin function relative to initialDir + * @param initialDir + * @param moduleName + */ + private requirePlugin(initialDir: string, moduleName: string): RequireResult { + const modulePath = this.resolveJavaScriptModule(moduleName, initialDir, this.fs); + try { + return { module: require(modulePath), error: undefined }; + } catch (error) { + return { module: undefined, error }; + } + } + // TODO: stolen from moduleNameResolver.ts because marked as internal /** * Expose resolution logic to allow us to use Node module resolution logic from arbitrary locations. @@ -992,26 +1047,11 @@ export class ProjectConfiguration { return resolvedModule.resolvedFileName; } - private requirePlugin(initialDir: string, moduleName: string): RequireResult { - const modulePath = this.resolveJavaScriptModule(moduleName, initialDir, this.fs); - try { - return { module: require(modulePath), error: undefined }; - } catch (error) { - return { module: undefined, error }; - } - } - - private enablePlugin(pluginConfigEntry: ts.PluginImport, searchPaths: string[]) { - for (const searchPath of searchPaths) { - const resolvedModule = this.resolveModule(pluginConfigEntry.name, searchPath) as PluginModuleFactory; - if (resolvedModule) { - this.enableProxy(resolvedModule, pluginConfigEntry); - return; - } - } - this.logger.info(`Couldn't find ${pluginConfigEntry.name} anywhere in paths: ${searchPaths.join(',')}`); - } - + /** + * Replaces the LanguageService with an instance wrapped by the plugin + * @param pluginModuleFactory function to create the module + * @param configEntry extra settings from tsconfig to pass to the plugin module + */ private enableProxy(pluginModuleFactory: PluginModuleFactory, configEntry: ts.PluginImport) { try { if (typeof pluginModuleFactory !== 'function') { From 64d73784dffaac64062d963f7d383f7ff38d5b77 Mon Sep 17 00:00:00 2001 From: Tom van Ommeren Date: Mon, 28 Aug 2017 08:13:39 +0200 Subject: [PATCH 05/15] fix: plugin loading for windows --- src/project-manager.ts | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/project-manager.ts b/src/project-manager.ts index 95bf80f1f..f49b32023 100644 --- a/src/project-manager.ts +++ b/src/project-manager.ts @@ -1006,9 +1006,10 @@ export class ProjectConfiguration { * @param initialDir */ private resolveModule(moduleName: string, initialDir: string): {} | undefined { - const resolvedPath = path.resolve(initialDir, 'node_modules'); + // const resolvedPath = path.resolve(initialDir, 'node_modules'); + const resolvedPath = ''; this.logger.info(`Loading ${moduleName} from ${initialDir} (resolved to ${resolvedPath})`); - const result = this.requirePlugin(resolvedPath, moduleName); + const result = this.requirePlugin(initialDir, moduleName); if (result.error) { this.logger.info(`Failed to load module: ${JSON.stringify(result.error)}`); return undefined; @@ -1037,14 +1038,16 @@ export class ProjectConfiguration { * Throws an error if the module can't be resolved. */ private resolveJavaScriptModule(moduleName: string, initialDir: string, host: ts.ModuleResolutionHost): string { - const { resolvedModule /* , failedLookupLocations */ } = - ts.nodeModuleNameResolver(moduleName, /* containingFile */ initialDir, { moduleResolution: ts.ModuleResolutionKind.NodeJs, allowJs: true }, this.fs, undefined); + // const { resolvedModule /* , failedLookupLocations */ } = + const result = + ts.nodeModuleNameResolver(moduleName, /* containingFile */ initialDir.replace('\\', '/') + '/package.json', { moduleResolution: ts.ModuleResolutionKind.NodeJs, allowJs: true }, this.fs, undefined); // TODO: jsOnly flag missing :( - if (!resolvedModule) { + if (!result.resolvedModule) { // TODO: add Looked in: ${failedLookupLocations.join(', ')} back into error. + // this.logger.error(result.failedLookupLocations!); throw new Error(`Could not resolve JS module ${moduleName} starting at ${initialDir}.`); } - return resolvedModule.resolvedFileName; + return result.resolvedModule.resolvedFileName; } /** From 0d79881aa905e01d74b14b6201aa48231a0c4f23 Mon Sep 17 00:00:00 2001 From: Tom van Ommeren Date: Sun, 10 Sep 2017 13:50:23 +0200 Subject: [PATCH 06/15] feat: plugin configuration Exposes initializationOptions containing globalPlugins, allowLocalPluginLoads, pluginProbeLocations Also removes default config and outdated comments --- package.json | 1 - src/match-files.ts | 2 +- src/project-manager.ts | 67 ++++++++++++++++++++++++++++----------- src/request-type.ts | 7 ++++ src/typescript-service.ts | 1 + tsconfig.json | 5 +-- 6 files changed, 59 insertions(+), 24 deletions(-) diff --git a/package.json b/package.json index e2836f12e..cd165385a 100644 --- a/package.json +++ b/package.json @@ -79,7 +79,6 @@ "source-map-support": "^0.4.11", "temp": "^0.8.3", "tslint": "^5.0.0", - "tslint-language-service": "^0.9.6", "validate-commit-msg": "^2.12.2" }, "bin": { diff --git a/src/match-files.ts b/src/match-files.ts index f3b45202e..6d38ba38d 100644 --- a/src/match-files.ts +++ b/src/match-files.ts @@ -55,7 +55,7 @@ export function matchFiles(path: string, extensions: string[], excludes: string[ const directorySeparator = '/'; -function combinePaths(path1: string, path2: string) { +export function combinePaths(path1: string, path2: string) { if (!(path1 && path1.length)) return path2; if (!(path2 && path2.length)) return path1; if (getRootLength(path2) !== 0) return path2; diff --git a/src/project-manager.ts b/src/project-manager.ts index f49b32023..482d53d1e 100644 --- a/src/project-manager.ts +++ b/src/project-manager.ts @@ -8,7 +8,9 @@ import * as ts from 'typescript'; import { Disposable } from './disposable'; import { FileSystemUpdater } from './fs'; import { Logger, NoopLogger } from './logging'; +import { combinePaths } from './match-files'; import { InMemoryFileSystem } from './memfs'; +import { InitializationOptions } from './request-type'; import { traceObservable, traceSync } from './tracing'; import { isConfigFile, @@ -146,6 +148,11 @@ export class ProjectManager implements Disposable { */ private subscriptions = new Subscription(); + /** + * Options passed to the language server at startup + */ + private initializationOptions?: InitializationOptions; + /** * @param rootPath root path as passed to `initialize` * @param inMemoryFileSystem File system that keeps structure and contents in memory @@ -157,12 +164,14 @@ export class ProjectManager implements Disposable { inMemoryFileSystem: InMemoryFileSystem, updater: FileSystemUpdater, traceModuleResolution?: boolean, + initializationOptions?: InitializationOptions, protected logger: Logger = new NoopLogger() ) { this.rootPath = rootPath; this.updater = updater; this.inMemoryFs = inMemoryFileSystem; this.versions = new Map(); + this.initializationOptions = initializationOptions; this.traceModuleResolution = traceModuleResolution || false; // Share DocumentRegistry between all ProjectConfigurations @@ -190,6 +199,7 @@ export class ProjectManager implements Disposable { '', tsConfig, this.traceModuleResolution, + this.initializationOptions, this.logger ); configs.set(trimmedRootPath, config); @@ -219,6 +229,7 @@ export class ProjectManager implements Disposable { filePath, undefined, this.traceModuleResolution, + this.initializationOptions, this.logger )); // Remove catch-all config (if exists) @@ -817,6 +828,10 @@ export class ProjectConfiguration { */ private traceModuleResolution: boolean; + private allowLocalPluginLoads: boolean = false; + private globalPlugins: string[] = []; + private pluginProbeLocations: string[] = []; + /** * Root file path, relative to workspace hierarchy root */ @@ -848,12 +863,18 @@ export class ProjectConfiguration { configFilePath: string, configContent?: any, traceModuleResolution?: boolean, + initializationOptions?: InitializationOptions, private logger: Logger = new NoopLogger() ) { this.fs = fs; this.configFilePath = configFilePath; this.configContent = configContent; this.versions = versions; + if (initializationOptions) { + this.allowLocalPluginLoads = initializationOptions.allowLocalPluginLoads || false; + this.globalPlugins = initializationOptions.globalPlugins || []; + this.pluginProbeLocations = initializationOptions.pluginProbeLocations || []; + } this.traceModuleResolution = traceModuleResolution || false; this.rootFilePath = rootFilePath; } @@ -961,27 +982,41 @@ export class ProjectConfiguration { } private enablePlugins(options: ts.CompilerOptions) { - const searchPaths = []; - // TODO: support pluginProbeLocations? - // TODO: add peer node_modules to source path. - - // TODO: determine how to expose this setting. - // VS Code starts tsserver with --allowLocalPluginLoads by default: https://github.com/Microsoft/TypeScript/pull/15924 - const allowLocalPluginLoads = true; - if (allowLocalPluginLoads) { + // Search our peer node_modules, then any globally-specified probe paths + // ../../.. to walk from X/node_modules/javascript-typescript-langserver/lib/project-manager.js to X/node_modules/ + const searchPaths = [combinePaths(__filename, '../../..'), ...this.pluginProbeLocations]; + + // Corresponds to --allowLocalPluginLoads, opt-in to avoid remote code execution. + if (this.allowLocalPluginLoads) { const local = this.rootFilePath; this.logger.info(`Local plugin loading enabled; adding ${local} to search paths`); searchPaths.unshift(local); } + let pluginImports: ts.PluginImport[] = []; + if (options.plugins) { + pluginImports = options.plugins as ts.PluginImport[]; + } + // Enable tsconfig-specified plugins if (options.plugins) { - for (const pluginConfigEntry of options.plugins as ts.PluginImport[]) { + for (const pluginConfigEntry of pluginImports) { this.enablePlugin(pluginConfigEntry, searchPaths); } } - // TODO: support globalPlugins flag if desired + if (this.globalPlugins) { + // Enable global plugins with synthetic configuration entries + for (const globalPluginName of this.globalPlugins) { + // Skip already-locally-loaded plugins + if (!pluginImports || pluginImports.some(p => p.name === globalPluginName)) { + continue; + } + + // Provide global: true so plugins can detect why they can't find their config + this.enablePlugin({ name: globalPluginName, global: true } as ts.PluginImport, searchPaths); + } + } } /** @@ -1006,9 +1041,7 @@ export class ProjectConfiguration { * @param initialDir */ private resolveModule(moduleName: string, initialDir: string): {} | undefined { - // const resolvedPath = path.resolve(initialDir, 'node_modules'); - const resolvedPath = ''; - this.logger.info(`Loading ${moduleName} from ${initialDir} (resolved to ${resolvedPath})`); + this.logger.info(`Loading ${moduleName} from ${initialDir}`); const result = this.requirePlugin(initialDir, moduleName); if (result.error) { this.logger.info(`Failed to load module: ${JSON.stringify(result.error)}`); @@ -1031,20 +1064,18 @@ export class ProjectConfiguration { } } - // TODO: stolen from moduleNameResolver.ts because marked as internal /** * Expose resolution logic to allow us to use Node module resolution logic from arbitrary locations. * No way to do this with `require()`: https://github.com/nodejs/node/issues/5963 * Throws an error if the module can't be resolved. + * stolen from moduleNameResolver.ts because marked as internal */ private resolveJavaScriptModule(moduleName: string, initialDir: string, host: ts.ModuleResolutionHost): string { - // const { resolvedModule /* , failedLookupLocations */ } = + // TODO: this should set jsOnly=true to the internal resolver, but this parameter is not exposed on a public api. const result = ts.nodeModuleNameResolver(moduleName, /* containingFile */ initialDir.replace('\\', '/') + '/package.json', { moduleResolution: ts.ModuleResolutionKind.NodeJs, allowJs: true }, this.fs, undefined); - // TODO: jsOnly flag missing :( if (!result.resolvedModule) { - // TODO: add Looked in: ${failedLookupLocations.join(', ')} back into error. - // this.logger.error(result.failedLookupLocations!); + // this.logger.error(result.failedLookupLocations); throw new Error(`Could not resolve JS module ${moduleName} starting at ${initialDir}.`); } return result.resolvedModule.resolvedFileName; diff --git a/src/request-type.ts b/src/request-type.ts index 7cbd3c3e0..649a74446 100644 --- a/src/request-type.ts +++ b/src/request-type.ts @@ -3,6 +3,13 @@ import * as vscode from 'vscode-languageserver'; export interface InitializeParams extends vscode.InitializeParams { capabilities: ClientCapabilities; + initializationOptions?: InitializationOptions +} + +export interface InitializationOptions { + allowLocalPluginLoads: boolean; + globalPlugins: string[]; + pluginProbeLocations: string[]; } export interface ClientCapabilities extends vscode.ClientCapabilities { diff --git a/src/typescript-service.ts b/src/typescript-service.ts index e1441d2fa..a76bb1a7c 100644 --- a/src/typescript-service.ts +++ b/src/typescript-service.ts @@ -223,6 +223,7 @@ export class TypeScriptService { this.inMemoryFileSystem, this.updater, this.traceModuleResolution, + params.initializationOptions, this.logger ); this.packageManager = new PackageManager(this.updater, this.inMemoryFileSystem, this.logger); diff --git a/tsconfig.json b/tsconfig.json index 3636865eb..8a08a9727 100755 --- a/tsconfig.json +++ b/tsconfig.json @@ -16,10 +16,7 @@ "noImplicitThis": true, "noUnusedLocals": true, "noImplicitAny": true, - "strictNullChecks": true, - "plugins": [ - { "name": "tslint-language-service"} - ] + "strictNullChecks": true }, "include": [ "src/**/*.ts" From 786b0c07506a9eff8de8edb8b66106dcf57289f1 Mon Sep 17 00:00:00 2001 From: Tom van Ommeren Date: Sun, 10 Sep 2017 13:55:48 +0200 Subject: [PATCH 07/15] fix: missing semicolon --- src/request-type.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/request-type.ts b/src/request-type.ts index 649a74446..b0bdef166 100644 --- a/src/request-type.ts +++ b/src/request-type.ts @@ -3,7 +3,7 @@ import * as vscode from 'vscode-languageserver'; export interface InitializeParams extends vscode.InitializeParams { capabilities: ClientCapabilities; - initializationOptions?: InitializationOptions + initializationOptions?: InitializationOptions; } export interface InitializationOptions { From 0f0f039292164c5cd2cc55c668704fc8d846ee34 Mon Sep 17 00:00:00 2001 From: Tom van Ommeren Date: Sat, 16 Sep 2017 22:59:38 +0200 Subject: [PATCH 08/15] fix: extract plugin functionality + add tests --- src/plugins.ts | 168 +++++++++++++++++++++++++++++++++++++++ src/project-manager.ts | 162 +------------------------------------ src/test/plugins.test.ts | 74 +++++++++++++++++ 3 files changed, 246 insertions(+), 158 deletions(-) create mode 100644 src/plugins.ts create mode 100644 src/test/plugins.test.ts diff --git a/src/plugins.ts b/src/plugins.ts new file mode 100644 index 000000000..60ccbd53c --- /dev/null +++ b/src/plugins.ts @@ -0,0 +1,168 @@ +import * as ts from 'typescript'; +import { Logger, NoopLogger } from './logging'; +import { combinePaths } from './match-files'; +import { InitializationOptions } from './request-type'; + +// definitions from from TypeScript server/project.ts + +/** + * A plugin exports an initialization function, injected with + * the current typescript instance + */ +export type PluginModuleFactory = (mod: { typescript: typeof ts }) => PluginModule; + +export type EnableProxyFunc = (pluginModuleFactory: PluginModuleFactory, pluginConfigEntry: ts.PluginImport) => void; + +/** + * A plugin presents this API when initialized + */ +export interface PluginModule { + create(createInfo: PluginCreateInfo): ts.LanguageService; + getExternalFiles?(proj: Project): string[]; +} + +/** + * All of tsserver's environment exposed to plugins + */ +export interface PluginCreateInfo { + project: Project; + languageService: ts.LanguageService; + languageServiceHost: ts.LanguageServiceHost; + serverHost: ServerHost; + config: any; +} + +/** + * The portion of tsserver's Project API exposed to plugins + */ +export interface Project { + projectService: { + logger: Logger; + }; +} + +/** + * The portion of tsserver's ServerHost API exposed to plugins + */ +export type ServerHost = object; + +/** + * The result of a node require: a module or an error. + */ +type RequireResult = { module: {}, error: undefined } | { module: undefined, error: {} }; + +export class PluginLoader { + + public allowLocalPluginLoads: boolean = false; + public globalPlugins: string[] = []; + public pluginProbeLocations: string[] = []; + public require: (path: string) => any = require; + + constructor(private rootFilePath: string, private fs: ts.ModuleResolutionHost, initializationOptions?: InitializationOptions, private logger = new NoopLogger()) { + if (initializationOptions) { + this.allowLocalPluginLoads = initializationOptions.allowLocalPluginLoads || false; + this.globalPlugins = initializationOptions.globalPlugins || []; + this.pluginProbeLocations = initializationOptions.pluginProbeLocations || []; + } + } + + public loadPlugins(options: ts.CompilerOptions, applyProxy: EnableProxyFunc) { + // Search our peer node_modules, then any globally-specified probe paths + // ../../.. to walk from X/node_modules/javascript-typescript-langserver/lib/project-manager.js to X/node_modules/ + const searchPaths = [combinePaths(__filename, '../../..'), ...this.pluginProbeLocations]; + + // Corresponds to --allowLocalPluginLoads, opt-in to avoid remote code execution. + if (this.allowLocalPluginLoads) { + const local = this.rootFilePath; + this.logger.info(`Local plugin loading enabled; adding ${local} to search paths`); + searchPaths.unshift(local); + } + + let pluginImports: ts.PluginImport[] = []; + if (options.plugins) { + pluginImports = options.plugins as ts.PluginImport[]; + } + + // Enable tsconfig-specified plugins + if (options.plugins) { + for (const pluginConfigEntry of pluginImports) { + this.enablePlugin(pluginConfigEntry, searchPaths, applyProxy); + } + } + + if (this.globalPlugins) { + // Enable global plugins with synthetic configuration entries + for (const globalPluginName of this.globalPlugins) { + // Skip already-locally-loaded plugins + if (!pluginImports || pluginImports.some(p => p.name === globalPluginName)) { + continue; + } + + // Provide global: true so plugins can detect why they can't find their config + this.enablePlugin({ name: globalPluginName, global: true } as ts.PluginImport, searchPaths, applyProxy); + } + } + } + + /** + * Tries to load and enable a single plugin + * @param pluginConfigEntry + * @param searchPaths + */ + private enablePlugin(pluginConfigEntry: ts.PluginImport, searchPaths: string[], enableProxy: EnableProxyFunc) { + for (const searchPath of searchPaths) { + const resolvedModule = this.resolveModule(pluginConfigEntry.name, searchPath) as PluginModuleFactory; + if (resolvedModule) { + enableProxy(resolvedModule, pluginConfigEntry); + return; + } + } + this.logger.info(`Couldn't find ${pluginConfigEntry.name} anywhere in paths: ${searchPaths.join(',')}`); + } + + /** + * Load a plugin using a node require + * @param moduleName + * @param initialDir + */ + private resolveModule(moduleName: string, initialDir: string): {} | undefined { + this.logger.info(`Loading ${moduleName} from ${initialDir}`); + const result = this.requirePlugin(initialDir, moduleName); + if (result.error) { + this.logger.info(`Failed to load module: ${JSON.stringify(result.error)}`); + return undefined; + } + return result.module; + } + + /** + * Resolves a loads a plugin function relative to initialDir + * @param initialDir + * @param moduleName + */ + private requirePlugin(initialDir: string, moduleName: string): RequireResult { + const modulePath = this.resolveJavaScriptModule(moduleName, initialDir, this.fs); + try { + return { module: this.require(modulePath), error: undefined }; + } catch (error) { + return { module: undefined, error }; + } + } + + /** + * Expose resolution logic to allow us to use Node module resolution logic from arbitrary locations. + * No way to do this with `require()`: https://github.com/nodejs/node/issues/5963 + * Throws an error if the module can't be resolved. + * stolen from moduleNameResolver.ts because marked as internal + */ + private resolveJavaScriptModule(moduleName: string, initialDir: string, host: ts.ModuleResolutionHost): string { + // TODO: this should set jsOnly=true to the internal resolver, but this parameter is not exposed on a public api. + const result = + ts.nodeModuleNameResolver(moduleName, /* containingFile */ initialDir.replace('\\', '/') + '/package.json', { moduleResolution: ts.ModuleResolutionKind.NodeJs, allowJs: true }, this.fs, undefined); + if (!result.resolvedModule) { + // this.logger.error(result.failedLookupLocations); + throw new Error(`Could not resolve JS module ${moduleName} starting at ${initialDir}.`); + } + return result.resolvedModule.resolvedFileName; + } +} diff --git a/src/project-manager.ts b/src/project-manager.ts index 482d53d1e..670952bff 100644 --- a/src/project-manager.ts +++ b/src/project-manager.ts @@ -8,8 +8,8 @@ import * as ts from 'typescript'; import { Disposable } from './disposable'; import { FileSystemUpdater } from './fs'; import { Logger, NoopLogger } from './logging'; -import { combinePaths } from './match-files'; import { InMemoryFileSystem } from './memfs'; +import { PluginCreateInfo, PluginLoader, PluginModuleFactory } from './plugins'; import { InitializationOptions } from './request-type'; import { traceObservable, traceSync } from './tracing'; import { @@ -26,52 +26,6 @@ import { export type ConfigType = 'js' | 'ts'; -// definitions from from TypeScript server/project.ts - -/** - * A plugin exports an initialization function, injected with - * the current typescript instance - */ -type PluginModuleFactory = (mod: { typescript: typeof ts }) => PluginModule; - -/** - * A plugin presents this API when initialized - */ -interface PluginModule { - create(createInfo: PluginCreateInfo): ts.LanguageService; - getExternalFiles?(proj: Project): string[]; -} - -/** - * All of tsserver's environment exposed to plugins - */ -interface PluginCreateInfo { - project: Project; - languageService: ts.LanguageService; - languageServiceHost: ts.LanguageServiceHost; - serverHost: ServerHost; - config: any; -} - -/** - * The portion of tsserver's Project API exposed to plugins - */ -interface Project { - projectService: { - logger: Logger; - }; -} - -/** - * The portion of tsserver's ServerHost API exposed to plugins - */ -type ServerHost = object; - -/** - * The result of a node require: a module or an error. - */ -type RequireResult = { module: {}, error: undefined } | { module: undefined, error: {} }; - /** * ProjectManager translates VFS files to one or many projects denoted by [tj]config.json. * It uses either local or remote file system to fetch directory tree and files from and then @@ -828,10 +782,6 @@ export class ProjectConfiguration { */ private traceModuleResolution: boolean; - private allowLocalPluginLoads: boolean = false; - private globalPlugins: string[] = []; - private pluginProbeLocations: string[] = []; - /** * Root file path, relative to workspace hierarchy root */ @@ -863,18 +813,13 @@ export class ProjectConfiguration { configFilePath: string, configContent?: any, traceModuleResolution?: boolean, - initializationOptions?: InitializationOptions, + private initializationOptions?: InitializationOptions, private logger: Logger = new NoopLogger() ) { this.fs = fs; this.configFilePath = configFilePath; this.configContent = configContent; this.versions = versions; - if (initializationOptions) { - this.allowLocalPluginLoads = initializationOptions.allowLocalPluginLoads || false; - this.globalPlugins = initializationOptions.globalPlugins || []; - this.pluginProbeLocations = initializationOptions.pluginProbeLocations || []; - } this.traceModuleResolution = traceModuleResolution || false; this.rootFilePath = rootFilePath; } @@ -977,110 +922,11 @@ export class ProjectConfiguration { this.logger ); this.service = ts.createLanguageService(this.host, this.documentRegistry); - this.enablePlugins(options); + const pluginLoader = new PluginLoader(this.rootFilePath, this.fs, this.initializationOptions, this.logger); + pluginLoader.loadPlugins(options, this.enableProxy.bind(this)); this.initialized = true; } - private enablePlugins(options: ts.CompilerOptions) { - // Search our peer node_modules, then any globally-specified probe paths - // ../../.. to walk from X/node_modules/javascript-typescript-langserver/lib/project-manager.js to X/node_modules/ - const searchPaths = [combinePaths(__filename, '../../..'), ...this.pluginProbeLocations]; - - // Corresponds to --allowLocalPluginLoads, opt-in to avoid remote code execution. - if (this.allowLocalPluginLoads) { - const local = this.rootFilePath; - this.logger.info(`Local plugin loading enabled; adding ${local} to search paths`); - searchPaths.unshift(local); - } - - let pluginImports: ts.PluginImport[] = []; - if (options.plugins) { - pluginImports = options.plugins as ts.PluginImport[]; - } - - // Enable tsconfig-specified plugins - if (options.plugins) { - for (const pluginConfigEntry of pluginImports) { - this.enablePlugin(pluginConfigEntry, searchPaths); - } - } - - if (this.globalPlugins) { - // Enable global plugins with synthetic configuration entries - for (const globalPluginName of this.globalPlugins) { - // Skip already-locally-loaded plugins - if (!pluginImports || pluginImports.some(p => p.name === globalPluginName)) { - continue; - } - - // Provide global: true so plugins can detect why they can't find their config - this.enablePlugin({ name: globalPluginName, global: true } as ts.PluginImport, searchPaths); - } - } - } - - /** - * Tries to load and enable a single plugin - * @param pluginConfigEntry - * @param searchPaths - */ - private enablePlugin(pluginConfigEntry: ts.PluginImport, searchPaths: string[]) { - for (const searchPath of searchPaths) { - const resolvedModule = this.resolveModule(pluginConfigEntry.name, searchPath) as PluginModuleFactory; - if (resolvedModule) { - this.enableProxy(resolvedModule, pluginConfigEntry); - return; - } - } - this.logger.info(`Couldn't find ${pluginConfigEntry.name} anywhere in paths: ${searchPaths.join(',')}`); - } - - /** - * Load a plugin use a node require - * @param moduleName - * @param initialDir - */ - private resolveModule(moduleName: string, initialDir: string): {} | undefined { - this.logger.info(`Loading ${moduleName} from ${initialDir}`); - const result = this.requirePlugin(initialDir, moduleName); - if (result.error) { - this.logger.info(`Failed to load module: ${JSON.stringify(result.error)}`); - return undefined; - } - return result.module; - } - - /** - * Resolves a loads a plugin function relative to initialDir - * @param initialDir - * @param moduleName - */ - private requirePlugin(initialDir: string, moduleName: string): RequireResult { - const modulePath = this.resolveJavaScriptModule(moduleName, initialDir, this.fs); - try { - return { module: require(modulePath), error: undefined }; - } catch (error) { - return { module: undefined, error }; - } - } - - /** - * Expose resolution logic to allow us to use Node module resolution logic from arbitrary locations. - * No way to do this with `require()`: https://github.com/nodejs/node/issues/5963 - * Throws an error if the module can't be resolved. - * stolen from moduleNameResolver.ts because marked as internal - */ - private resolveJavaScriptModule(moduleName: string, initialDir: string, host: ts.ModuleResolutionHost): string { - // TODO: this should set jsOnly=true to the internal resolver, but this parameter is not exposed on a public api. - const result = - ts.nodeModuleNameResolver(moduleName, /* containingFile */ initialDir.replace('\\', '/') + '/package.json', { moduleResolution: ts.ModuleResolutionKind.NodeJs, allowJs: true }, this.fs, undefined); - if (!result.resolvedModule) { - // this.logger.error(result.failedLookupLocations); - throw new Error(`Could not resolve JS module ${moduleName} starting at ${initialDir}.`); - } - return result.resolvedModule.resolvedFileName; - } - /** * Replaces the LanguageService with an instance wrapped by the plugin * @param pluginModuleFactory function to create the module diff --git a/src/test/plugins.test.ts b/src/test/plugins.test.ts new file mode 100644 index 000000000..5aafc4064 --- /dev/null +++ b/src/test/plugins.test.ts @@ -0,0 +1,74 @@ +import * as assert from 'assert'; +import * as sinon from 'sinon'; +import * as ts from 'typescript'; +import {InMemoryFileSystem} from '../memfs'; +import {PluginLoader, PluginModule, PluginModuleFactory} from '../plugins'; +import {InitializationOptions} from '../request-type'; + +describe('plugins', () => { + describe('constructor', () => { + it('should use defaults if no initializationOptions are provided', () => { + const memfs = new InMemoryFileSystem('/'); + + const loader = new PluginLoader('/', memfs); + assert(!loader.allowLocalPluginLoads); + assert.equal(loader.globalPlugins.length, 0); + assert.equal(loader.pluginProbeLocations.length, 0); + }); + }); + describe('loader', () => { + it('should do nothing if no plugins are configured', () => { + const memfs = new InMemoryFileSystem('/'); + + const loader = new PluginLoader('/', memfs); + const compilerOptions: ts.CompilerOptions = {}; + const applyProxy: (pluginModuleFactory: PluginModuleFactory) => PluginModule = sinon.spy(); + loader.loadPlugins(compilerOptions, applyProxy); + + }); + + it('should load a global plugin if specified', () => { + const memfs = new InMemoryFileSystem('/'); + memfs.add('file:///Users/tomv/Projects/sourcegraph/node_modules/some-plugin/package.json', '{ "name": "some-plugin", "version": "0.1.1", "main": "plugin.js"}'); + memfs.add('file:///Users/tomv/Projects/sourcegraph/node_modules/some-plugin/plugin.js', 'module.exports = function (modules) { return 5; };'); + const initializationOptions: InitializationOptions = { + globalPlugins: ['some-plugin'], + allowLocalPluginLoads: false, + pluginProbeLocations: [] + }; + const pluginFactoryFunc = (modules: any) => 5; + const loader = new PluginLoader('/', memfs, initializationOptions); + loader.require = (path: string) => pluginFactoryFunc; + const compilerOptions: ts.CompilerOptions = {}; + const applyProxy = sinon.spy(); + loader.loadPlugins(compilerOptions, applyProxy); + sinon.assert.calledOnce(applyProxy); + sinon.assert.calledWithExactly(applyProxy, pluginFactoryFunc, sinon.match({ name: 'some-plugin', global: true})); + }); + + it('should load a local plugin if specified', () => { + const memfs = new InMemoryFileSystem('/some-project'); + memfs.add('file:///some-project/node_modules/some-plugin/package.json', '{ "name": "some-plugin", "version": "0.1.1", "main": "plugin.js"}'); + memfs.add('file:///some-project/node_modules/some-plugin/plugin.js', 'module.exports = function (modules) { return 5; };'); + const initializationOptions: InitializationOptions = { + globalPlugins: [], + allowLocalPluginLoads: true, + pluginProbeLocations: [] + }; + const pluginFactoryFunc = (modules: any) => 5; + const loader = new PluginLoader('/some-project', memfs, initializationOptions); + loader.require = (path: string) => pluginFactoryFunc; + const pluginOption: ts.PluginImport = { + name: 'some-plugin' + }; + const compilerOptions: ts.CompilerOptions = { + plugins: [pluginOption] + }; + const applyProxy = sinon.spy(); + loader.loadPlugins(compilerOptions, applyProxy); + sinon.assert.calledOnce(applyProxy); + sinon.assert.calledWithExactly(applyProxy, pluginFactoryFunc, sinon.match({ name: 'some-plugin'})); + }); + + }); +}); From 2292e96951b47dde6c2171cf9195850398fab3f9 Mon Sep 17 00:00:00 2001 From: Tom van Ommeren Date: Sun, 17 Sep 2017 08:25:25 +0200 Subject: [PATCH 09/15] fix: Fix and clean up tests --- src/plugins.ts | 11 +++++------ src/test/plugins.test.ts | 33 +++++++++++++-------------------- 2 files changed, 18 insertions(+), 26 deletions(-) diff --git a/src/plugins.ts b/src/plugins.ts index 60ccbd53c..b0c3cd911 100644 --- a/src/plugins.ts +++ b/src/plugins.ts @@ -53,12 +53,11 @@ type RequireResult = { module: {}, error: undefined } | { module: undefined, err export class PluginLoader { - public allowLocalPluginLoads: boolean = false; - public globalPlugins: string[] = []; - public pluginProbeLocations: string[] = []; - public require: (path: string) => any = require; + private allowLocalPluginLoads: boolean = false; + private globalPlugins: string[] = []; + private pluginProbeLocations: string[] = []; - constructor(private rootFilePath: string, private fs: ts.ModuleResolutionHost, initializationOptions?: InitializationOptions, private logger = new NoopLogger()) { + constructor(private rootFilePath: string, private fs: ts.ModuleResolutionHost, initializationOptions?: InitializationOptions, private logger = new NoopLogger(), private requireModule: (moduleName: string) => any = require) { if (initializationOptions) { this.allowLocalPluginLoads = initializationOptions.allowLocalPluginLoads || false; this.globalPlugins = initializationOptions.globalPlugins || []; @@ -143,7 +142,7 @@ export class PluginLoader { private requirePlugin(initialDir: string, moduleName: string): RequireResult { const modulePath = this.resolveJavaScriptModule(moduleName, initialDir, this.fs); try { - return { module: this.require(modulePath), error: undefined }; + return { module: this.requireModule(modulePath), error: undefined }; } catch (error) { return { module: undefined, error }; } diff --git a/src/test/plugins.test.ts b/src/test/plugins.test.ts index 5aafc4064..b37b9859e 100644 --- a/src/test/plugins.test.ts +++ b/src/test/plugins.test.ts @@ -1,22 +1,13 @@ -import * as assert from 'assert'; +import * as path from 'path'; import * as sinon from 'sinon'; import * as ts from 'typescript'; import {InMemoryFileSystem} from '../memfs'; import {PluginLoader, PluginModule, PluginModuleFactory} from '../plugins'; import {InitializationOptions} from '../request-type'; +import { path2uri } from '../util'; describe('plugins', () => { - describe('constructor', () => { - it('should use defaults if no initializationOptions are provided', () => { - const memfs = new InMemoryFileSystem('/'); - - const loader = new PluginLoader('/', memfs); - assert(!loader.allowLocalPluginLoads); - assert.equal(loader.globalPlugins.length, 0); - assert.equal(loader.pluginProbeLocations.length, 0); - }); - }); - describe('loader', () => { + describe('loadPlugins()', () => { it('should do nothing if no plugins are configured', () => { const memfs = new InMemoryFileSystem('/'); @@ -29,16 +20,18 @@ describe('plugins', () => { it('should load a global plugin if specified', () => { const memfs = new InMemoryFileSystem('/'); - memfs.add('file:///Users/tomv/Projects/sourcegraph/node_modules/some-plugin/package.json', '{ "name": "some-plugin", "version": "0.1.1", "main": "plugin.js"}'); - memfs.add('file:///Users/tomv/Projects/sourcegraph/node_modules/some-plugin/plugin.js', 'module.exports = function (modules) { return 5; };'); + const peerPackagesPath = path.resolve(__filename, '../../../../'); + const peerPackagesUri = path2uri(peerPackagesPath); + memfs.add(peerPackagesUri + '/node_modules/some-plugin/package.json', '{ "name": "some-plugin", "version": "0.1.1", "main": "plugin.js"}'); + memfs.add(peerPackagesUri + '/node_modules/some-plugin/plugin.js', ''); const initializationOptions: InitializationOptions = { globalPlugins: ['some-plugin'], allowLocalPluginLoads: false, pluginProbeLocations: [] }; const pluginFactoryFunc = (modules: any) => 5; - const loader = new PluginLoader('/', memfs, initializationOptions); - loader.require = (path: string) => pluginFactoryFunc; + const fakeRequire = (path: string) => pluginFactoryFunc; + const loader = new PluginLoader('/', memfs, initializationOptions, undefined, fakeRequire); const compilerOptions: ts.CompilerOptions = {}; const applyProxy = sinon.spy(); loader.loadPlugins(compilerOptions, applyProxy); @@ -49,15 +42,15 @@ describe('plugins', () => { it('should load a local plugin if specified', () => { const memfs = new InMemoryFileSystem('/some-project'); memfs.add('file:///some-project/node_modules/some-plugin/package.json', '{ "name": "some-plugin", "version": "0.1.1", "main": "plugin.js"}'); - memfs.add('file:///some-project/node_modules/some-plugin/plugin.js', 'module.exports = function (modules) { return 5; };'); + memfs.add('file:///some-project/node_modules/some-plugin/plugin.js', ''); const initializationOptions: InitializationOptions = { globalPlugins: [], allowLocalPluginLoads: true, pluginProbeLocations: [] }; const pluginFactoryFunc = (modules: any) => 5; - const loader = new PluginLoader('/some-project', memfs, initializationOptions); - loader.require = (path: string) => pluginFactoryFunc; + const fakeRequire = (path: string) => pluginFactoryFunc; + const loader = new PluginLoader('/some-project', memfs, initializationOptions, undefined, fakeRequire); const pluginOption: ts.PluginImport = { name: 'some-plugin' }; @@ -67,7 +60,7 @@ describe('plugins', () => { const applyProxy = sinon.spy(); loader.loadPlugins(compilerOptions, applyProxy); sinon.assert.calledOnce(applyProxy); - sinon.assert.calledWithExactly(applyProxy, pluginFactoryFunc, sinon.match({ name: 'some-plugin'})); + sinon.assert.calledWithExactly(applyProxy, pluginFactoryFunc, sinon.match(pluginOption)); }); }); From 7382e8f9c4467d643827d7740accd90aa8d3baa2 Mon Sep 17 00:00:00 2001 From: Tom van Ommeren Date: Mon, 25 Sep 2017 22:39:29 +0200 Subject: [PATCH 10/15] fix: use arrow fun, log error, state plugin source --- src/plugins.ts | 3 ++- src/project-manager.ts | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/plugins.ts b/src/plugins.ts index b0c3cd911..56e2c390d 100644 --- a/src/plugins.ts +++ b/src/plugins.ts @@ -3,7 +3,8 @@ import { Logger, NoopLogger } from './logging'; import { combinePaths } from './match-files'; import { InitializationOptions } from './request-type'; -// definitions from from TypeScript server/project.ts +// Based on types and logic from TypeScript server/project.ts @ +// https://github.com/Microsoft/TypeScript/blob/711e890e59e10aa05a43cb938474a3d9c2270429/src/server/project.ts /** * A plugin exports an initialization function, injected with diff --git a/src/project-manager.ts b/src/project-manager.ts index 670952bff..8d2574791 100644 --- a/src/project-manager.ts +++ b/src/project-manager.ts @@ -923,7 +923,7 @@ export class ProjectConfiguration { ); this.service = ts.createLanguageService(this.host, this.documentRegistry); const pluginLoader = new PluginLoader(this.rootFilePath, this.fs, this.initializationOptions, this.logger); - pluginLoader.loadPlugins(options, this.enableProxy.bind(this)); + pluginLoader.loadPlugins(options, (factory, config) => this.enableProxy(factory, config)); this.initialized = true; } @@ -950,7 +950,7 @@ export class ProjectConfiguration { const pluginModule = pluginModuleFactory({ typescript: ts }); this.service = pluginModule.create(info); } catch (e) { - this.logger.info(`Plugin activation failed: ${e}`); + this.logger.error(`Plugin activation failed: ${e}`); } } From 791074dab134d0bd6a900d1ac873bf22213578b3 Mon Sep 17 00:00:00 2001 From: Tom van Ommeren Date: Tue, 26 Sep 2017 02:12:34 +0200 Subject: [PATCH 11/15] fix: Rename didChangeConfiguration handler to expected workspaceDidChangeConfiguration --- src/typescript-service.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/typescript-service.ts b/src/typescript-service.ts index a76bb1a7c..408735260 100644 --- a/src/typescript-service.ts +++ b/src/typescript-service.ts @@ -312,7 +312,7 @@ export class TypeScriptService { * A notification sent from the client to the server to signal the change of configuration * settings. */ - didChangeConfiguration(params: DidChangeConfigurationParams): void { + workspaceDidChangeConfiguration(params: DidChangeConfigurationParams): void { merge(this.settings, params.settings); } From d807ec3c967668e215c9940d3b10781acc516326 Mon Sep 17 00:00:00 2001 From: Tom van Ommeren Date: Tue, 26 Sep 2017 02:16:42 +0200 Subject: [PATCH 12/15] fix: Switch to didChangeConfiguration, always use local filesystem Also add 'node_modules' to resolvedPath Don't give up when resolveJavaScriptModule fails --- src/plugins.ts | 46 +++++++++++++++++++++++++++++---------- src/project-manager.ts | 18 +++++++-------- src/request-type.ts | 6 +++-- src/test/plugins.test.ts | 10 ++++----- src/typescript-service.ts | 14 +++++++----- 5 files changed, 61 insertions(+), 33 deletions(-) diff --git a/src/plugins.ts b/src/plugins.ts index 56e2c390d..d80d2ae3c 100644 --- a/src/plugins.ts +++ b/src/plugins.ts @@ -1,7 +1,10 @@ +import * as fs from 'mz/fs'; +import * as path from 'path'; import * as ts from 'typescript'; import { Logger, NoopLogger } from './logging'; import { combinePaths } from './match-files'; -import { InitializationOptions } from './request-type'; +import { PluginSettings } from './request-type'; +import { toUnixPath } from './util'; // Based on types and logic from TypeScript server/project.ts @ // https://github.com/Microsoft/TypeScript/blob/711e890e59e10aa05a43cb938474a3d9c2270429/src/server/project.ts @@ -58,11 +61,17 @@ export class PluginLoader { private globalPlugins: string[] = []; private pluginProbeLocations: string[] = []; - constructor(private rootFilePath: string, private fs: ts.ModuleResolutionHost, initializationOptions?: InitializationOptions, private logger = new NoopLogger(), private requireModule: (moduleName: string) => any = require) { - if (initializationOptions) { - this.allowLocalPluginLoads = initializationOptions.allowLocalPluginLoads || false; - this.globalPlugins = initializationOptions.globalPlugins || []; - this.pluginProbeLocations = initializationOptions.pluginProbeLocations || []; + constructor( + private rootFilePath: string, + private fs: ts.ModuleResolutionHost, + pluginSettings?: PluginSettings, + private logger = new NoopLogger(), + private resolutionHost = new LocalModuleResolutionHost(), + private requireModule: (moduleName: string) => any = require) { + if (pluginSettings) { + this.allowLocalPluginLoads = pluginSettings.allowLocalPluginLoads || false; + this.globalPlugins = pluginSettings.globalPlugins || []; + this.pluginProbeLocations = pluginSettings.pluginProbeLocations || []; } } @@ -117,7 +126,7 @@ export class PluginLoader { return; } } - this.logger.info(`Couldn't find ${pluginConfigEntry.name} anywhere in paths: ${searchPaths.join(',')}`); + this.logger.error(`Couldn't find ${pluginConfigEntry.name} anywhere in paths: ${searchPaths.join(',')}`); } /** @@ -126,10 +135,11 @@ export class PluginLoader { * @param initialDir */ private resolveModule(moduleName: string, initialDir: string): {} | undefined { - this.logger.info(`Loading ${moduleName} from ${initialDir}`); - const result = this.requirePlugin(initialDir, moduleName); + const resolvedPath = toUnixPath(path.resolve(combinePaths(initialDir, 'node_modules'))); + this.logger.info(`Loading ${moduleName} from ${initialDir} (resolved to ${resolvedPath})`); + const result = this.requirePlugin(resolvedPath, moduleName); if (result.error) { - this.logger.info(`Failed to load module: ${JSON.stringify(result.error)}`); + this.logger.error(`Failed to load module: ${JSON.stringify(result.error)}`); return undefined; } return result.module; @@ -141,8 +151,8 @@ export class PluginLoader { * @param moduleName */ private requirePlugin(initialDir: string, moduleName: string): RequireResult { - const modulePath = this.resolveJavaScriptModule(moduleName, initialDir, this.fs); try { + const modulePath = this.resolveJavaScriptModule(moduleName, initialDir, this.fs); return { module: this.requireModule(modulePath), error: undefined }; } catch (error) { return { module: undefined, error }; @@ -158,7 +168,7 @@ export class PluginLoader { private resolveJavaScriptModule(moduleName: string, initialDir: string, host: ts.ModuleResolutionHost): string { // TODO: this should set jsOnly=true to the internal resolver, but this parameter is not exposed on a public api. const result = - ts.nodeModuleNameResolver(moduleName, /* containingFile */ initialDir.replace('\\', '/') + '/package.json', { moduleResolution: ts.ModuleResolutionKind.NodeJs, allowJs: true }, this.fs, undefined); + ts.nodeModuleNameResolver(moduleName, /* containingFile */ initialDir.replace('\\', '/') + '/package.json', { moduleResolution: ts.ModuleResolutionKind.NodeJs, allowJs: true }, this.resolutionHost, undefined); if (!result.resolvedModule) { // this.logger.error(result.failedLookupLocations); throw new Error(`Could not resolve JS module ${moduleName} starting at ${initialDir}.`); @@ -166,3 +176,15 @@ export class PluginLoader { return result.resolvedModule.resolvedFileName; } } + +/** + * A local filesystem-based ModuleResolutionHost for plugin loading. + */ +export class LocalModuleResolutionHost implements ts.ModuleResolutionHost { + fileExists(fileName: string): boolean { + return fs.existsSync(fileName); + } + readFile(fileName: string): string { + return fs.readFileSync(fileName, 'utf8'); + } +} diff --git a/src/project-manager.ts b/src/project-manager.ts index 8d2574791..bbaacf09d 100644 --- a/src/project-manager.ts +++ b/src/project-manager.ts @@ -10,7 +10,7 @@ import { FileSystemUpdater } from './fs'; import { Logger, NoopLogger } from './logging'; import { InMemoryFileSystem } from './memfs'; import { PluginCreateInfo, PluginLoader, PluginModuleFactory } from './plugins'; -import { InitializationOptions } from './request-type'; +import { PluginSettings } from './request-type'; import { traceObservable, traceSync } from './tracing'; import { isConfigFile, @@ -105,7 +105,7 @@ export class ProjectManager implements Disposable { /** * Options passed to the language server at startup */ - private initializationOptions?: InitializationOptions; + private pluginSettings?: PluginSettings; /** * @param rootPath root path as passed to `initialize` @@ -118,14 +118,14 @@ export class ProjectManager implements Disposable { inMemoryFileSystem: InMemoryFileSystem, updater: FileSystemUpdater, traceModuleResolution?: boolean, - initializationOptions?: InitializationOptions, + pluginSettings?: PluginSettings, protected logger: Logger = new NoopLogger() ) { this.rootPath = rootPath; this.updater = updater; this.inMemoryFs = inMemoryFileSystem; this.versions = new Map(); - this.initializationOptions = initializationOptions; + this.pluginSettings = pluginSettings; this.traceModuleResolution = traceModuleResolution || false; // Share DocumentRegistry between all ProjectConfigurations @@ -153,7 +153,7 @@ export class ProjectManager implements Disposable { '', tsConfig, this.traceModuleResolution, - this.initializationOptions, + this.pluginSettings, this.logger ); configs.set(trimmedRootPath, config); @@ -183,7 +183,7 @@ export class ProjectManager implements Disposable { filePath, undefined, this.traceModuleResolution, - this.initializationOptions, + this.pluginSettings, this.logger )); // Remove catch-all config (if exists) @@ -813,7 +813,7 @@ export class ProjectConfiguration { configFilePath: string, configContent?: any, traceModuleResolution?: boolean, - private initializationOptions?: InitializationOptions, + private pluginSettings?: PluginSettings, private logger: Logger = new NoopLogger() ) { this.fs = fs; @@ -922,8 +922,8 @@ export class ProjectConfiguration { this.logger ); this.service = ts.createLanguageService(this.host, this.documentRegistry); - const pluginLoader = new PluginLoader(this.rootFilePath, this.fs, this.initializationOptions, this.logger); - pluginLoader.loadPlugins(options, (factory, config) => this.enableProxy(factory, config)); + const pluginLoader = new PluginLoader(this.rootFilePath, this.fs, this.pluginSettings, this.logger); + pluginLoader.loadPlugins(options, this.enableProxy.bind(this) /* (factory, config) => this.enableProxy(factory, config)) */); this.initialized = true; } diff --git a/src/request-type.ts b/src/request-type.ts index b0bdef166..e2f8dd435 100644 --- a/src/request-type.ts +++ b/src/request-type.ts @@ -3,10 +3,12 @@ import * as vscode from 'vscode-languageserver'; export interface InitializeParams extends vscode.InitializeParams { capabilities: ClientCapabilities; - initializationOptions?: InitializationOptions; } -export interface InitializationOptions { +/** + * Settings to enable plugin loading + */ +export interface PluginSettings { allowLocalPluginLoads: boolean; globalPlugins: string[]; pluginProbeLocations: string[]; diff --git a/src/test/plugins.test.ts b/src/test/plugins.test.ts index b37b9859e..54df0d82b 100644 --- a/src/test/plugins.test.ts +++ b/src/test/plugins.test.ts @@ -3,7 +3,7 @@ import * as sinon from 'sinon'; import * as ts from 'typescript'; import {InMemoryFileSystem} from '../memfs'; import {PluginLoader, PluginModule, PluginModuleFactory} from '../plugins'; -import {InitializationOptions} from '../request-type'; +import {PluginSettings} from '../request-type'; import { path2uri } from '../util'; describe('plugins', () => { @@ -24,14 +24,14 @@ describe('plugins', () => { const peerPackagesUri = path2uri(peerPackagesPath); memfs.add(peerPackagesUri + '/node_modules/some-plugin/package.json', '{ "name": "some-plugin", "version": "0.1.1", "main": "plugin.js"}'); memfs.add(peerPackagesUri + '/node_modules/some-plugin/plugin.js', ''); - const initializationOptions: InitializationOptions = { + const pluginSettings: PluginSettings = { globalPlugins: ['some-plugin'], allowLocalPluginLoads: false, pluginProbeLocations: [] }; const pluginFactoryFunc = (modules: any) => 5; const fakeRequire = (path: string) => pluginFactoryFunc; - const loader = new PluginLoader('/', memfs, initializationOptions, undefined, fakeRequire); + const loader = new PluginLoader('/', memfs, pluginSettings, undefined, memfs, fakeRequire); const compilerOptions: ts.CompilerOptions = {}; const applyProxy = sinon.spy(); loader.loadPlugins(compilerOptions, applyProxy); @@ -43,14 +43,14 @@ describe('plugins', () => { const memfs = new InMemoryFileSystem('/some-project'); memfs.add('file:///some-project/node_modules/some-plugin/package.json', '{ "name": "some-plugin", "version": "0.1.1", "main": "plugin.js"}'); memfs.add('file:///some-project/node_modules/some-plugin/plugin.js', ''); - const initializationOptions: InitializationOptions = { + const pluginSettings: PluginSettings = { globalPlugins: [], allowLocalPluginLoads: true, pluginProbeLocations: [] }; const pluginFactoryFunc = (modules: any) => 5; const fakeRequire = (path: string) => pluginFactoryFunc; - const loader = new PluginLoader('/some-project', memfs, initializationOptions, undefined, fakeRequire); + const loader = new PluginLoader('/some-project', memfs, pluginSettings, undefined, memfs, fakeRequire); const pluginOption: ts.PluginImport = { name: 'some-plugin' }; diff --git a/src/typescript-service.ts b/src/typescript-service.ts index 408735260..c155af4c4 100644 --- a/src/typescript-service.ts +++ b/src/typescript-service.ts @@ -49,6 +49,7 @@ import { InitializeResult, PackageDescriptor, PackageInformation, + PluginSettings, ReferenceInformation, SymbolDescriptor, SymbolLocationInformation, @@ -86,9 +87,9 @@ export type TypeScriptServiceFactory = (client: LanguageClient, options?: TypeSc /** * Settings synced through `didChangeConfiguration` */ -export interface Settings { - format: ts.FormatCodeSettings; -} +export type Settings = { + format: ts.FormatCodeSettings +} & PluginSettings; /** * Handles incoming requests and return responses. There is a one-to-one-to-one @@ -171,7 +172,10 @@ export class TypeScriptService { insertSpaceBeforeFunctionParenthesis: false, placeOpenBraceOnNewLineForFunctions: false, placeOpenBraceOnNewLineForControlBlocks: false - } + }, + allowLocalPluginLoads: false, + globalPlugins: [], + pluginProbeLocations: [] }; /** @@ -223,7 +227,7 @@ export class TypeScriptService { this.inMemoryFileSystem, this.updater, this.traceModuleResolution, - params.initializationOptions, + this.settings, this.logger ); this.packageManager = new PackageManager(this.updater, this.inMemoryFileSystem, this.logger); From d3e441912a39d551265e118248496ad4d42b3230 Mon Sep 17 00:00:00 2001 From: Tom van Ommeren Date: Wed, 27 Sep 2017 08:37:20 +0200 Subject: [PATCH 13/15] fix: Review fixes: rename enableProxy, extend PluginSettings, formatting --- src/plugins.ts | 8 +++++++- src/project-manager.ts | 4 ++-- src/typescript-service.ts | 4 ++-- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/src/plugins.ts b/src/plugins.ts index d80d2ae3c..4221a18f0 100644 --- a/src/plugins.ts +++ b/src/plugins.ts @@ -168,7 +168,13 @@ export class PluginLoader { private resolveJavaScriptModule(moduleName: string, initialDir: string, host: ts.ModuleResolutionHost): string { // TODO: this should set jsOnly=true to the internal resolver, but this parameter is not exposed on a public api. const result = - ts.nodeModuleNameResolver(moduleName, /* containingFile */ initialDir.replace('\\', '/') + '/package.json', { moduleResolution: ts.ModuleResolutionKind.NodeJs, allowJs: true }, this.resolutionHost, undefined); + ts.nodeModuleNameResolver( + moduleName, + initialDir.replace('\\', '/') + '/package.json', /* containingFile */ + { moduleResolution: ts.ModuleResolutionKind.NodeJs, allowJs: true }, + this.resolutionHost, + undefined + ); if (!result.resolvedModule) { // this.logger.error(result.failedLookupLocations); throw new Error(`Could not resolve JS module ${moduleName} starting at ${initialDir}.`); diff --git a/src/project-manager.ts b/src/project-manager.ts index bbaacf09d..dd5dbf680 100644 --- a/src/project-manager.ts +++ b/src/project-manager.ts @@ -923,7 +923,7 @@ export class ProjectConfiguration { ); this.service = ts.createLanguageService(this.host, this.documentRegistry); const pluginLoader = new PluginLoader(this.rootFilePath, this.fs, this.pluginSettings, this.logger); - pluginLoader.loadPlugins(options, this.enableProxy.bind(this) /* (factory, config) => this.enableProxy(factory, config)) */); + pluginLoader.loadPlugins(options, (factory, config) => this.wrapService(factory, config)); this.initialized = true; } @@ -932,7 +932,7 @@ export class ProjectConfiguration { * @param pluginModuleFactory function to create the module * @param configEntry extra settings from tsconfig to pass to the plugin module */ - private enableProxy(pluginModuleFactory: PluginModuleFactory, configEntry: ts.PluginImport) { + private wrapService(pluginModuleFactory: PluginModuleFactory, configEntry: ts.PluginImport) { try { if (typeof pluginModuleFactory !== 'function') { this.logger.info(`Skipped loading plugin ${configEntry.name} because it didn't expose a proper factory function`); diff --git a/src/typescript-service.ts b/src/typescript-service.ts index c155af4c4..abc6ebe87 100644 --- a/src/typescript-service.ts +++ b/src/typescript-service.ts @@ -87,9 +87,9 @@ export type TypeScriptServiceFactory = (client: LanguageClient, options?: TypeSc /** * Settings synced through `didChangeConfiguration` */ -export type Settings = { +export interface Settings extends PluginSettings { format: ts.FormatCodeSettings -} & PluginSettings; +} /** * Handles incoming requests and return responses. There is a one-to-one-to-one From bbfd5b4f062da8e2a0de1152c6fce9df503516a7 Mon Sep 17 00:00:00 2001 From: Tom van Ommeren Date: Fri, 29 Sep 2017 08:23:28 +0200 Subject: [PATCH 14/15] fix: missing semicolon --- src/typescript-service.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/typescript-service.ts b/src/typescript-service.ts index abc6ebe87..4c6e6f116 100644 --- a/src/typescript-service.ts +++ b/src/typescript-service.ts @@ -88,7 +88,7 @@ export type TypeScriptServiceFactory = (client: LanguageClient, options?: TypeSc * Settings synced through `didChangeConfiguration` */ export interface Settings extends PluginSettings { - format: ts.FormatCodeSettings + format: ts.FormatCodeSettings; } /** From 42ce6e8f1b28e00cfbc164f1a1218d9427fdd3b4 Mon Sep 17 00:00:00 2001 From: Tom van Ommeren Date: Fri, 29 Sep 2017 23:07:39 +0200 Subject: [PATCH 15/15] fix: paths for windows test --- src/test/plugins.test.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/test/plugins.test.ts b/src/test/plugins.test.ts index 54df0d82b..ea4037f59 100644 --- a/src/test/plugins.test.ts +++ b/src/test/plugins.test.ts @@ -40,9 +40,11 @@ describe('plugins', () => { }); it('should load a local plugin if specified', () => { + const rootDir = (process.platform === 'win32' ? 'c:\\' : '/') + 'some-project'; + const rootUri = path2uri(rootDir) + '/'; const memfs = new InMemoryFileSystem('/some-project'); - memfs.add('file:///some-project/node_modules/some-plugin/package.json', '{ "name": "some-plugin", "version": "0.1.1", "main": "plugin.js"}'); - memfs.add('file:///some-project/node_modules/some-plugin/plugin.js', ''); + memfs.add(rootUri + 'node_modules/some-plugin/package.json', '{ "name": "some-plugin", "version": "0.1.1", "main": "plugin.js"}'); + memfs.add(rootUri + 'node_modules/some-plugin/plugin.js', ''); const pluginSettings: PluginSettings = { globalPlugins: [], allowLocalPluginLoads: true, @@ -50,7 +52,7 @@ describe('plugins', () => { }; const pluginFactoryFunc = (modules: any) => 5; const fakeRequire = (path: string) => pluginFactoryFunc; - const loader = new PluginLoader('/some-project', memfs, pluginSettings, undefined, memfs, fakeRequire); + const loader = new PluginLoader(rootDir, memfs, pluginSettings, undefined, memfs, fakeRequire); const pluginOption: ts.PluginImport = { name: 'some-plugin' };