From 5f0719ab6cd20ecc65f43c4f2d452c924fb42a3f Mon Sep 17 00:00:00 2001 From: Paul LeMarquand Date: Mon, 8 Sep 2025 13:36:27 -0400 Subject: [PATCH] Inject logger directly into FolderContext This change moves toward decoupling FolderContext from WorkspaceContext by making logger dependency explicit rather than accessed through context chain. Part of broader effort to remove FolderContext dependency on WorkspaceContext. --- src/FolderContext.ts | 10 +++++---- src/sourcekit-lsp/LanguageClientManager.ts | 12 +++++----- test/integration-tests/FolderContext.test.ts | 10 ++++----- .../LanguageClientManager.test.ts | 22 ++++++++++++++++++- 4 files changed, 37 insertions(+), 17 deletions(-) diff --git a/src/FolderContext.ts b/src/FolderContext.ts index 7ed08a2ea..a1ee652e0 100644 --- a/src/FolderContext.ts +++ b/src/FolderContext.ts @@ -51,7 +51,8 @@ export class FolderContext implements vscode.Disposable { public linuxMain: LinuxMain, public swiftPackage: SwiftPackage, public workspaceFolder: vscode.WorkspaceFolder, - public workspaceContext: WorkspaceContext + public workspaceContext: WorkspaceContext, + public logger: SwiftLogger ) { this.packageWatcher = new PackageWatcher(this, workspaceContext.logger); this.backgroundCompilation = new BackgroundCompilation(this); @@ -146,7 +147,8 @@ export class FolderContext implements vscode.Disposable { linuxMain, swiftPackage, workspaceFolder, - workspaceContext + workspaceContext, + workspaceContext.logger ); const error = await swiftPackage.error; @@ -154,7 +156,7 @@ export class FolderContext implements vscode.Disposable { void vscode.window.showErrorMessage( `Failed to load ${folderContext.name}/Package.swift: ${error.message}` ); - workspaceContext.logger.info( + folderContext.logger.info( `Failed to load Package.swift: ${error.message}`, folderContext.name ); @@ -230,7 +232,7 @@ export class FolderContext implements vscode.Disposable { this.testExplorer = new TestExplorer( this, this.workspaceContext.tasks, - this.workspaceContext.logger, + this.logger, this.workspaceContext.onDidChangeSwiftFiles.bind(this.workspaceContext) ); this.testExplorerResolver?.(this.testExplorer); diff --git a/src/sourcekit-lsp/LanguageClientManager.ts b/src/sourcekit-lsp/LanguageClientManager.ts index 13ec7d139..182a60e27 100644 --- a/src/sourcekit-lsp/LanguageClientManager.ts +++ b/src/sourcekit-lsp/LanguageClientManager.ts @@ -168,7 +168,7 @@ export class LanguageClientManager implements vscode.Disposable { // Swift versions prior to 5.6 don't support file changes, so need to restart // lSP server when a file is either created or deleted if (this.swiftVersion.isLessThan(new Version(5, 6, 0))) { - folderContext.workspaceContext.logger.debug("LSP: Adding new/delete file handlers"); + folderContext.logger.debug("LSP: Adding new/delete file handlers"); // restart LSP server on creation of a new file const onDidCreateFileDisposable = vscode.workspace.onDidCreateFiles(() => { void this.restart(); @@ -382,7 +382,7 @@ export class LanguageClientManager implements vscode.Disposable { if (reason.message === "Stopping the server timed out") { await this.setupLanguageClient(workspaceFolder); } - this.folderContext.workspaceContext.logger.error(reason); + this.folderContext.logger.error(reason); }); await this.restartedPromise; } @@ -504,13 +504,13 @@ export class LanguageClientManager implements vscode.Disposable { }); }); if (client.clientOptions.workspaceFolder) { - this.folderContext.workspaceContext.logger.info( + this.folderContext.logger.info( `SourceKit-LSP setup for ${FolderContext.uriName( client.clientOptions.workspaceFolder.uri )}` ); } else { - this.folderContext.workspaceContext.logger.info(`SourceKit-LSP setup`); + this.folderContext.logger.info(`SourceKit-LSP setup`); } client.onNotification(SourceKitLogMessageNotification.type, params => { @@ -551,9 +551,7 @@ export class LanguageClientManager implements vscode.Disposable { // do nothing, the experimental capability is not supported } } catch (reason) { - this.folderContext.workspaceContext.logger.error( - `Error starting SourceKit-LSP: ${reason}` - ); + this.folderContext.logger.error(`Error starting SourceKit-LSP: ${reason}`); if (this.languageClient?.state === State.Running) { await this.languageClient?.stop(); } diff --git a/test/integration-tests/FolderContext.test.ts b/test/integration-tests/FolderContext.test.ts index c1e624486..4d6be38f8 100644 --- a/test/integration-tests/FolderContext.test.ts +++ b/test/integration-tests/FolderContext.test.ts @@ -63,7 +63,7 @@ suite("FolderContext Error Handling Test Suite", () => { "Should fallback to global toolchain when user dismisses dialog" ); - const errorLogs = workspaceContext.logger.logs.filter( + const errorLogs = folderContext.logger.logs.filter( log => log.includes("Failed to discover Swift toolchain") && log.includes("package2") && @@ -118,10 +118,10 @@ suite("FolderContext Error Handling Test Suite", () => { ); // Assert: Should log both failure and success - const failureLogs = workspaceContext.logger.logs.filter(log => + const failureLogs = folderContext.logger.logs.filter(log => log.includes("Failed to discover Swift toolchain for package2") ); - const successLogs = workspaceContext.logger.logs.filter(log => + const successLogs = folderContext.logger.logs.filter(log => log.includes("Successfully created toolchain for package2 after user selection") ); @@ -161,12 +161,12 @@ suite("FolderContext Error Handling Test Suite", () => { "Should retry toolchain creation after user selection" ); - const initialFailureLogs = workspaceContext.logger.logs.filter(log => + const initialFailureLogs = folderContext.logger.logs.filter(log => log.includes( "Failed to discover Swift toolchain for package2: Error: Initial toolchain failure" ) ); - const retryFailureLogs = workspaceContext.logger.logs.filter(log => + const retryFailureLogs = folderContext.logger.logs.filter(log => log.includes( "Failed to create toolchain for package2 even after user selection: Error: Retry toolchain failure" ) diff --git a/test/unit-tests/sourcekit-lsp/LanguageClientManager.test.ts b/test/unit-tests/sourcekit-lsp/LanguageClientManager.test.ts index 0fe5b4b5d..7dfbc01d1 100644 --- a/test/unit-tests/sourcekit-lsp/LanguageClientManager.test.ts +++ b/test/unit-tests/sourcekit-lsp/LanguageClientManager.test.ts @@ -140,6 +140,7 @@ suite("LanguageClientManager Suite", () => { ), swiftVersion: new Version(6, 0, 0), toolchain: instance(mockedToolchain), + logger: instance(mockLogger), }); mockedWorkspace = mockObject({ globalToolchain: instance(mockedToolchain), @@ -251,6 +252,8 @@ suite("LanguageClientManager Suite", () => { }, workspaceContext: instance(mockedWorkspace), swiftVersion: mockedFolder.swiftVersion, + toolchain: instance(mockedToolchain), + logger: instance(mockLogger), }); mockedWorkspace.folders.push(instance(newFolder)); const factory = new LanguageClientToolchainCoordinator( @@ -267,6 +270,14 @@ suite("LanguageClientManager Suite", () => { }); test("returns the a new language client for folders with different toolchains", async () => { + const differentToolchain = mockObject({ + swiftVersion: new Version(6, 1, 0), + buildFlags: mockedBuildFlags as unknown as BuildFlags, + getToolchainExecutable: mockFn(s => + s.withArgs("sourcekit-lsp").returns("/path/to/toolchain/bin/sourcekit-lsp") + ), + }); + const newFolder = mockObject({ isRootFolder: false, folder: vscode.Uri.file("/folder11"), @@ -277,6 +288,8 @@ suite("LanguageClientManager Suite", () => { }, workspaceContext: instance(mockedWorkspace), swiftVersion: new Version(6, 1, 0), + toolchain: instance(differentToolchain), + logger: instance(mockLogger), }); mockedWorkspace.folders.push(instance(newFolder)); const factory = new LanguageClientToolchainCoordinator( @@ -289,7 +302,7 @@ suite("LanguageClientManager Suite", () => { const sut2 = factory.get(instance(newFolder)); expect(sut1).to.not.equal(sut2, "Expected different LanguageClients to be returned"); - expect(languageClientFactoryMock.createLanguageClient).to.have.been.calledOnce; + expect(languageClientFactoryMock.createLanguageClient).to.have.been.calledTwice; }); }); @@ -411,6 +424,8 @@ suite("LanguageClientManager Suite", () => { }, workspaceContext: instance(mockedWorkspace), swiftVersion: new Version(6, 0, 0), + toolchain: instance(mockedToolchain), + logger: instance(mockLogger), }); const folder2 = mockObject({ isRootFolder: false, @@ -422,6 +437,8 @@ suite("LanguageClientManager Suite", () => { }, workspaceContext: instance(mockedWorkspace), swiftVersion: new Version(6, 0, 0), + toolchain: instance(mockedToolchain), + logger: instance(mockLogger), }); new LanguageClientToolchainCoordinator( @@ -893,6 +910,7 @@ suite("LanguageClientManager Suite", () => { workspaceContext: instance(mockedWorkspace), workspaceFolder, toolchain: instance(mockedToolchain), + logger: instance(mockLogger), }); mockedFolder.swiftVersion = mockedToolchain.swiftVersion; mockedWorkspace = mockObject({ @@ -911,6 +929,7 @@ suite("LanguageClientManager Suite", () => { workspaceContext: instance(mockedWorkspace), toolchain: instance(mockedToolchain), swiftVersion: mockedToolchain.swiftVersion, + logger: instance(mockLogger), }); folder2 = mockObject({ isRootFolder: false, @@ -923,6 +942,7 @@ suite("LanguageClientManager Suite", () => { workspaceContext: instance(mockedWorkspace), toolchain: instance(mockedToolchain), swiftVersion: mockedToolchain.swiftVersion, + logger: instance(mockLogger), }); });