From d3f213f18dc8777898a3d30b9d56ef93c440955b Mon Sep 17 00:00:00 2001 From: Amin Yahyaabadi Date: Thu, 10 Jun 2021 04:47:08 -0500 Subject: [PATCH 01/10] feat: allow overriding determineProjectPath --- lib/auto-languageclient.ts | 12 +++++++++++- lib/server-manager.ts | 13 +++---------- 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/lib/auto-languageclient.ts b/lib/auto-languageclient.ts index 66c15343..410e1011 100644 --- a/lib/auto-languageclient.ts +++ b/lib/auto-languageclient.ts @@ -308,7 +308,8 @@ export default class AutoLanguageClient { (e) => this.shouldStartForEditor(e), (filepath) => this.filterChangeWatchedFiles(filepath), this.reportBusyWhile, - this.getServerName() + this.getServerName(), + this.determineProjectPath ) this._serverManager.startListening() process.on("exit", () => this.exitCleanup.bind(this)) @@ -477,6 +478,15 @@ export default class AutoLanguageClient { this.logger.debug(`exit: code ${code} signal ${signal}`) } + /** (Optional) Finds the project path. If there is a custom logic for finding projects override this method. */ + protected determineProjectPath(textEditor: TextEditor): string | null { + const filePath = textEditor.getPath() + if (filePath == null) { + return null + } + return this._serverManager.getNormalizedProjectPaths().find((d) => filePath.startsWith(d)) || null + } + /** * The function called whenever the spawned server returns `data` in `stderr` Extend (call super.onSpawnStdErrData) or * override this if you need custom stderr data handling diff --git a/lib/server-manager.ts b/lib/server-manager.ts index e078a3e3..a5109ee9 100644 --- a/lib/server-manager.ts +++ b/lib/server-manager.ts @@ -47,7 +47,8 @@ export class ServerManager { private _startForEditor: (editor: TextEditor) => boolean, private _changeWatchedFileFilter: (filePath: string) => boolean, private _reportBusyWhile: ReportBusyWhile, - private _languageServerName: string + private _languageServerName: string, + private _determineProjectPath: (textEditor: TextEditor) => string | null ) { this.updateNormalizedProjectPaths() } @@ -121,7 +122,7 @@ export class ServerManager { textEditor: TextEditor, { shouldStart }: { shouldStart?: boolean } = { shouldStart: false } ): Promise { - const finalProjectPath = this.determineProjectPath(textEditor) + const finalProjectPath = this._determineProjectPath(textEditor) if (finalProjectPath == null) { // Files not yet saved have no path return null @@ -245,14 +246,6 @@ export class ServerManager { }) } - public determineProjectPath(textEditor: TextEditor): string | null { - const filePath = textEditor.getPath() - if (filePath == null) { - return null - } - return this._normalizedProjectPaths.find((d) => filePath.startsWith(d)) || null - } - public updateNormalizedProjectPaths(): void { this._normalizedProjectPaths = atom.project.getPaths().map(normalizePath) } From 360d4fb0be17f2f12d64ac26654f2f4d5bb74c1c Mon Sep 17 00:00:00 2001 From: Amin Yahyaabadi Date: Thu, 10 Jun 2021 07:31:01 -0500 Subject: [PATCH 02/10] feat: support out-of-project files Co-Authored-By: Alex Butler --- lib/auto-languageclient.ts | 25 ++++++++++++++++++++++--- lib/server-manager.ts | 12 ++++++++++++ 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/lib/auto-languageclient.ts b/lib/auto-languageclient.ts index 410e1011..4dc910e5 100644 --- a/lib/auto-languageclient.ts +++ b/lib/auto-languageclient.ts @@ -26,7 +26,13 @@ import * as Utils from "./utils" import { Socket } from "net" import { LanguageClientConnection } from "./languageclient" import { ConsoleLogger, FilteredLogger, Logger } from "./logger" -import { LanguageServerProcess, ServerManager, ActiveServer } from "./server-manager.js" +import { + LanguageServerProcess, + ServerManager, + ActiveServer, + normalizePath, + considerAdditionalPath, +} from "./server-manager.js" import { Disposable, CompositeDisposable, Point, Range, TextEditor } from "atom" import * as ac from "atom/autocomplete-plus" import { basename } from "path" @@ -391,6 +397,7 @@ export default class AutoLanguageClient { connection, capabilities: initializeResponse.capabilities, disposable: new CompositeDisposable(), + additionalPaths: new Set(), } this.postInitialization(newServer) connection.initialized() @@ -481,10 +488,22 @@ export default class AutoLanguageClient { /** (Optional) Finds the project path. If there is a custom logic for finding projects override this method. */ protected determineProjectPath(textEditor: TextEditor): string | null { const filePath = textEditor.getPath() - if (filePath == null) { + // TODO can filePath be null + if (filePath === null || filePath === undefined) { return null } - return this._serverManager.getNormalizedProjectPaths().find((d) => filePath.startsWith(d)) || null + const projectPath = this._serverManager.getNormalizedProjectPaths().find((d) => filePath.startsWith(d)) + if (projectPath !== undefined) { + return projectPath + } + + const serverWithClaim = this._serverManager + .getActiveServers() + .find((server) => server.additionalPaths?.has(path.dirname(filePath))) + if (serverWithClaim !== undefined) { + return normalizePath(serverWithClaim.projectPath) + } + return null } /** diff --git a/lib/server-manager.ts b/lib/server-manager.ts index a5109ee9..289f92d2 100644 --- a/lib/server-manager.ts +++ b/lib/server-manager.ts @@ -22,6 +22,8 @@ export interface ActiveServer { process: LanguageServerProcess connection: ls.LanguageClientConnection capabilities: ls.ServerCapabilities + /** Out of project directories that this server can also support. */ + additionalPaths?: Set } interface RestartCounter { @@ -343,3 +345,13 @@ export function normalizedProjectPathToWorkspaceFolder(normalizedProjectPath: st export function normalizePath(projectPath: string): string { return !projectPath.endsWith(path.sep) ? path.join(projectPath, path.sep) : projectPath } + +/** Considers a path for inclusion in `additionalPaths`. */ +export function considerAdditionalPath( + server: ActiveServer & { additionalPaths: Set }, + additionalPath: string +): void { + if (!additionalPath.startsWith(server.projectPath)) { + server.additionalPaths.add(additionalPath) + } +} From e7e7828418b0beaab305f6694bc0ecc8675ee365 Mon Sep 17 00:00:00 2001 From: Amin Yahyaabadi Date: Thu, 10 Jun 2021 07:35:13 -0500 Subject: [PATCH 03/10] feat: populate additionalPaths based on definitions Co-Authored-By: Alex Butler --- lib/auto-languageclient.ts | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/lib/auto-languageclient.ts b/lib/auto-languageclient.ts index 4dc910e5..26fd7cdc 100644 --- a/lib/auto-languageclient.ts +++ b/lib/auto-languageclient.ts @@ -697,7 +697,23 @@ export default class AutoLanguageClient { } this.definitions = this.definitions || new DefinitionAdapter() - return this.definitions.getDefinition(server.connection, server.capabilities, this.getLanguageName(), editor, point) + const query = await this.definitions.getDefinition( + server.connection, + server.capabilities, + this.getLanguageName(), + editor, + point + ) + + if (query !== null && this.supportsDefinitionsForAdditionalPaths && server.additionalPaths !== undefined) { + // populate additionalPaths based on definitions + // Indicates that the language server can support LSP functionality for out of project files indicated by `textDocument/definition` responses. + for (const def of query.definitions) { + considerAdditionalPath(server as ActiveServer & { additionalPaths: Set }, path.dirname(def.path)) + } + } + + return query } // Outline View via LS documentSymbol--------------------------------- @@ -984,6 +1000,14 @@ export default class AutoLanguageClient { .forEach((line) => this.logger.warn(`stderr ${line}`)) } + /** + * Indicates that the language server can support LSP functionality for out of project files indicated by + * `textDocument/definition` responses. Set it to `true` if the server supports this feature. + * + * @default `false` + */ + protected supportsDefinitionsForAdditionalPaths: boolean = false + private getServerAdapter( server: ActiveServer, adapter: T From 47773e9d3b0d05fee35875143ac7bd199b97a878 Mon Sep 17 00:00:00 2001 From: Amin Yahyaabadi Date: Thu, 10 Jun 2021 07:44:07 -0500 Subject: [PATCH 04/10] fix: remove supportsDefinitionsForAdditionalPaths function This check is not necessary --- lib/auto-languageclient.ts | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/lib/auto-languageclient.ts b/lib/auto-languageclient.ts index 26fd7cdc..3d2e4139 100644 --- a/lib/auto-languageclient.ts +++ b/lib/auto-languageclient.ts @@ -705,7 +705,7 @@ export default class AutoLanguageClient { point ) - if (query !== null && this.supportsDefinitionsForAdditionalPaths && server.additionalPaths !== undefined) { + if (query !== null && server.additionalPaths !== undefined) { // populate additionalPaths based on definitions // Indicates that the language server can support LSP functionality for out of project files indicated by `textDocument/definition` responses. for (const def of query.definitions) { @@ -1000,14 +1000,6 @@ export default class AutoLanguageClient { .forEach((line) => this.logger.warn(`stderr ${line}`)) } - /** - * Indicates that the language server can support LSP functionality for out of project files indicated by - * `textDocument/definition` responses. Set it to `true` if the server supports this feature. - * - * @default `false` - */ - protected supportsDefinitionsForAdditionalPaths: boolean = false - private getServerAdapter( server: ActiveServer, adapter: T From e22025c57f034fe898bfa706adb298b527c07e06 Mon Sep 17 00:00:00 2001 From: Amin Yahyaabadi Date: Fri, 11 Jun 2021 13:37:33 -0500 Subject: [PATCH 05/10] test: refactor setupClient and setupServerManager --- test/auto-languageclient.test.ts | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/test/auto-languageclient.test.ts b/test/auto-languageclient.test.ts index f1cadebc..eae3b4b8 100644 --- a/test/auto-languageclient.test.ts +++ b/test/auto-languageclient.test.ts @@ -12,20 +12,26 @@ function mockEditor(uri: string, scopeName: string): any { } } +function setupClient() { + atom.workspace.getTextEditors().forEach((editor) => editor.destroy()) + atom.project.getPaths().forEach((project) => atom.project.removePath(project)) + const client = new FakeAutoLanguageClient() + client.activate() + return client +} + +function setupServerManager(client = setupClient()) { + /* eslint-disable-next-line dot-notation */ + const serverManager = client["_serverManager"] + return serverManager +} + describe("AutoLanguageClient", () => { describe("ServerManager", () => { describe("WorkspaceFolders", () => { - let client: FakeAutoLanguageClient let serverManager: ServerManager - beforeEach(() => { - atom.workspace.getTextEditors().forEach((editor) => editor.destroy()) - atom.project.getPaths().forEach((project) => atom.project.removePath(project)) - client = new FakeAutoLanguageClient() - client.activate() - - /* eslint-disable-next-line dot-notation */ - serverManager = client["_serverManager"] + serverManager = setupServerManager() }) afterEach(() => { From a5932d6c14942fd689ffbcd642774fac160894d6 Mon Sep 17 00:00:00 2001 From: Amin Yahyaabadi Date: Fri, 11 Jun 2021 13:40:28 -0500 Subject: [PATCH 06/10] test: add tests for determineProjectPath --- test/auto-languageclient.test.ts | 73 +++++++++++++++++++++++++++++++- 1 file changed, 71 insertions(+), 2 deletions(-) diff --git a/test/auto-languageclient.test.ts b/test/auto-languageclient.test.ts index eae3b4b8..a5bf23ca 100644 --- a/test/auto-languageclient.test.ts +++ b/test/auto-languageclient.test.ts @@ -1,7 +1,14 @@ +import { TextEditor } from "atom" import AutoLanguageClient from "../lib/auto-languageclient" -import { projectPathToWorkspaceFolder, ServerManager } from "../lib/server-manager" +import { + projectPathToWorkspaceFolder, + ServerManager, + ActiveServer, + considerAdditionalPath, + normalizePath, +} from "../lib/server-manager" import { FakeAutoLanguageClient } from "./helpers" -import { dirname } from "path" +import { dirname, join } from "path" function mockEditor(uri: string, scopeName: string): any { return { @@ -27,6 +34,68 @@ function setupServerManager(client = setupClient()) { } describe("AutoLanguageClient", () => { + describe("determineProjectPath", () => { + it("returns null when a single file is open", async () => { + const client = setupClient() + const textEditor = (await atom.workspace.open(__filename)) as TextEditor + /* eslint-disable-next-line dot-notation */ + const projectPath = client["determineProjectPath"](textEditor) + expect(projectPath).toBeNull() + }) + it("returns the project path when a file of that project is open", async () => { + const client = setupClient() + const serverManager = setupServerManager(client) + + const projectPath = __dirname + + // gives the open workspace folder + atom.project.addPath(projectPath) + await serverManager.startServer(projectPath) + + const textEditor = (await atom.workspace.open(__filename)) as TextEditor + /* eslint-disable-next-line dot-notation */ + expect(client["determineProjectPath"](textEditor)).toBe(normalizePath(projectPath)) + }) + it("returns the project path when an external file is open and it is not in additional paths", async () => { + const client = setupClient() + const serverManager = setupServerManager(client) + + const projectPath = __dirname + const externalDir = join(dirname(projectPath), "lib") + const externalFile = join(externalDir, "main.js") + + // gives the open workspace folder + atom.project.addPath(projectPath) + await serverManager.startServer(projectPath) + + const textEditor = (await atom.workspace.open(externalFile)) as TextEditor + /* eslint-disable-next-line dot-notation */ + expect(client["determineProjectPath"](textEditor)).toBeNull() + }) + it("returns the project path when an external file is open and it is in additional paths", async () => { + const client = setupClient() + const serverManager = setupServerManager(client) + + const projectPath = __dirname + const externalDir = join(dirname(projectPath), "lib") + const externalFile = join(externalDir, "main.js") + + // gives the open workspace folder + atom.project.addPath(projectPath) + await serverManager.startServer(projectPath) + + // get server + const server = serverManager.getActiveServers()[0] + expect(typeof server.additionalPaths).toBe("object") // Set() + // add additional path + considerAdditionalPath(server as ActiveServer & { additionalPaths: Set }, externalDir) + expect(server.additionalPaths?.has(externalDir)).toBeTrue() + + const textEditor = (await atom.workspace.open(externalFile)) as TextEditor + /* eslint-disable-next-line dot-notation */ + expect(client["determineProjectPath"](textEditor)).toBe(normalizePath(projectPath)) + }) + }) describe("ServerManager", () => { describe("WorkspaceFolders", () => { let serverManager: ServerManager From 19b6848c1032f4bc6803bcb0e65ae02c48a3fff7 Mon Sep 17 00:00:00 2001 From: Amin Yahyaabadi Date: Fri, 11 Jun 2021 14:55:44 -0500 Subject: [PATCH 07/10] test: share the resources to prevent MaxListenersExceededWarning error --- test/auto-languageclient.test.ts | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/test/auto-languageclient.test.ts b/test/auto-languageclient.test.ts index a5bf23ca..c3976d08 100644 --- a/test/auto-languageclient.test.ts +++ b/test/auto-languageclient.test.ts @@ -56,7 +56,9 @@ describe("AutoLanguageClient", () => { /* eslint-disable-next-line dot-notation */ expect(client["determineProjectPath"](textEditor)).toBe(normalizePath(projectPath)) }) - it("returns the project path when an external file is open and it is not in additional paths", async () => { + it("returns the project path for an external file if it is in additional paths", async () => { + // "returns the project path when an external file is open and it is not in additional paths" + const client = setupClient() const serverManager = setupServerManager(client) @@ -68,21 +70,12 @@ describe("AutoLanguageClient", () => { atom.project.addPath(projectPath) await serverManager.startServer(projectPath) - const textEditor = (await atom.workspace.open(externalFile)) as TextEditor + let textEditor = (await atom.workspace.open(externalFile)) as TextEditor /* eslint-disable-next-line dot-notation */ expect(client["determineProjectPath"](textEditor)).toBeNull() - }) - it("returns the project path when an external file is open and it is in additional paths", async () => { - const client = setupClient() - const serverManager = setupServerManager(client) + textEditor.destroy() - const projectPath = __dirname - const externalDir = join(dirname(projectPath), "lib") - const externalFile = join(externalDir, "main.js") - - // gives the open workspace folder - atom.project.addPath(projectPath) - await serverManager.startServer(projectPath) + // "returns the project path when an external file is open and it is in additional paths" // get server const server = serverManager.getActiveServers()[0] @@ -91,9 +84,10 @@ describe("AutoLanguageClient", () => { considerAdditionalPath(server as ActiveServer & { additionalPaths: Set }, externalDir) expect(server.additionalPaths?.has(externalDir)).toBeTrue() - const textEditor = (await atom.workspace.open(externalFile)) as TextEditor + textEditor = (await atom.workspace.open(externalFile)) as TextEditor /* eslint-disable-next-line dot-notation */ expect(client["determineProjectPath"](textEditor)).toBe(normalizePath(projectPath)) + textEditor.destroy() }) }) describe("ServerManager", () => { From bb2eefc8296cfab2b2a9e73dfdbcdab1111014ef Mon Sep 17 00:00:00 2001 From: Amin Yahyaabadi Date: Fri, 11 Jun 2021 15:05:01 -0500 Subject: [PATCH 08/10] test: skip some of the tests on macos --- test/auto-languageclient.test.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/test/auto-languageclient.test.ts b/test/auto-languageclient.test.ts index c3976d08..e4d2bcca 100644 --- a/test/auto-languageclient.test.ts +++ b/test/auto-languageclient.test.ts @@ -43,6 +43,10 @@ describe("AutoLanguageClient", () => { expect(projectPath).toBeNull() }) it("returns the project path when a file of that project is open", async () => { + // macos has issues with handling too much resources + if (process.platform === "darwin") { + return + } const client = setupClient() const serverManager = setupServerManager(client) @@ -57,6 +61,11 @@ describe("AutoLanguageClient", () => { expect(client["determineProjectPath"](textEditor)).toBe(normalizePath(projectPath)) }) it("returns the project path for an external file if it is in additional paths", async () => { + // macos has issues with handling too much resources + if (process.platform === "darwin") { + return + } + // "returns the project path when an external file is open and it is not in additional paths" const client = setupClient() From 50fe3d8d6fb73a8d01c79990e928ea4fb175ef38 Mon Sep 17 00:00:00 2001 From: Amin Yahyaabadi Date: Fri, 11 Jun 2021 16:45:53 -0500 Subject: [PATCH 09/10] test: merge the tests to share resources to fix macos crashes --- test/auto-languageclient.test.ts | 36 +++++++++++--------------------- 1 file changed, 12 insertions(+), 24 deletions(-) diff --git a/test/auto-languageclient.test.ts b/test/auto-languageclient.test.ts index e4d2bcca..8de4563d 100644 --- a/test/auto-languageclient.test.ts +++ b/test/auto-languageclient.test.ts @@ -35,43 +35,31 @@ function setupServerManager(client = setupClient()) { describe("AutoLanguageClient", () => { describe("determineProjectPath", () => { - it("returns null when a single file is open", async () => { - const client = setupClient() - const textEditor = (await atom.workspace.open(__filename)) as TextEditor - /* eslint-disable-next-line dot-notation */ - const projectPath = client["determineProjectPath"](textEditor) - expect(projectPath).toBeNull() - }) - it("returns the project path when a file of that project is open", async () => { - // macos has issues with handling too much resources - if (process.platform === "darwin") { - return - } + it("returns the project path for an internal or an external file in the project", async () => { const client = setupClient() const serverManager = setupServerManager(client) + // "returns null when a single file is open" + + let textEditor = (await atom.workspace.open(__filename)) as TextEditor + /* eslint-disable-next-line dot-notation */ + expect(client["determineProjectPath"](textEditor)).toBeNull() + textEditor.destroy() + + // "returns the project path when a file of that project is open" const projectPath = __dirname // gives the open workspace folder atom.project.addPath(projectPath) await serverManager.startServer(projectPath) - const textEditor = (await atom.workspace.open(__filename)) as TextEditor + textEditor = (await atom.workspace.open(__filename)) as TextEditor /* eslint-disable-next-line dot-notation */ expect(client["determineProjectPath"](textEditor)).toBe(normalizePath(projectPath)) - }) - it("returns the project path for an external file if it is in additional paths", async () => { - // macos has issues with handling too much resources - if (process.platform === "darwin") { - return - } + textEditor.destroy() // "returns the project path when an external file is open and it is not in additional paths" - const client = setupClient() - const serverManager = setupServerManager(client) - - const projectPath = __dirname const externalDir = join(dirname(projectPath), "lib") const externalFile = join(externalDir, "main.js") @@ -79,7 +67,7 @@ describe("AutoLanguageClient", () => { atom.project.addPath(projectPath) await serverManager.startServer(projectPath) - let textEditor = (await atom.workspace.open(externalFile)) as TextEditor + textEditor = (await atom.workspace.open(externalFile)) as TextEditor /* eslint-disable-next-line dot-notation */ expect(client["determineProjectPath"](textEditor)).toBeNull() textEditor.destroy() From 61fec5883557b00cad6e97e4cda6059ebddf230d Mon Sep 17 00:00:00 2001 From: Amin Yahyaabadi Date: Fri, 11 Jun 2021 16:56:15 -0500 Subject: [PATCH 10/10] test: skip the whole test on macos --- test/auto-languageclient.test.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/test/auto-languageclient.test.ts b/test/auto-languageclient.test.ts index 8de4563d..183b7759 100644 --- a/test/auto-languageclient.test.ts +++ b/test/auto-languageclient.test.ts @@ -36,6 +36,11 @@ function setupServerManager(client = setupClient()) { describe("AutoLanguageClient", () => { describe("determineProjectPath", () => { it("returns the project path for an internal or an external file in the project", async () => { + if (process.platform === "darwin") { + // there is nothing OS specific about the code. It just hits the limits that MacOS can handle in this test + pending("skipped on MacOS") + return + } const client = setupClient() const serverManager = setupServerManager(client)