Skip to content

Commit 4b2224f

Browse files
committed
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.
1 parent b8d08c5 commit 4b2224f

File tree

4 files changed

+37
-15
lines changed

4 files changed

+37
-15
lines changed

src/FolderContext.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,8 @@ export class FolderContext implements vscode.Disposable {
5050
public linuxMain: LinuxMain,
5151
public swiftPackage: SwiftPackage,
5252
public workspaceFolder: vscode.WorkspaceFolder,
53-
public workspaceContext: WorkspaceContext
53+
public workspaceContext: WorkspaceContext,
54+
public logger: SwiftLogger
5455
) {
5556
this.packageWatcher = new PackageWatcher(this, workspaceContext.logger);
5657
this.backgroundCompilation = new BackgroundCompilation(this);
@@ -134,15 +135,16 @@ export class FolderContext implements vscode.Disposable {
134135
linuxMain,
135136
swiftPackage,
136137
workspaceFolder,
137-
workspaceContext
138+
workspaceContext,
139+
workspaceContext.logger
138140
);
139141

140142
const error = await swiftPackage.error;
141143
if (error) {
142144
void vscode.window.showErrorMessage(
143145
`Failed to load ${folderContext.name}/Package.swift: ${error.message}`
144146
);
145-
workspaceContext.logger.info(
147+
folderContext.logger.info(
146148
`Failed to load Package.swift: ${error.message}`,
147149
folderContext.name
148150
);
@@ -218,7 +220,7 @@ export class FolderContext implements vscode.Disposable {
218220
this.testExplorer = new TestExplorer(
219221
this,
220222
this.workspaceContext.tasks,
221-
this.workspaceContext.logger,
223+
this.logger,
222224
this.workspaceContext.onDidChangeSwiftFiles.bind(this.workspaceContext)
223225
);
224226
this.testExplorerResolver?.(this.testExplorer);

src/sourcekit-lsp/LanguageClientManager.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ export class LanguageClientManager implements vscode.Disposable {
168168
// Swift versions prior to 5.6 don't support file changes, so need to restart
169169
// lSP server when a file is either created or deleted
170170
if (this.swiftVersion.isLessThan(new Version(5, 6, 0))) {
171-
folderContext.workspaceContext.logger.debug("LSP: Adding new/delete file handlers");
171+
folderContext.logger.debug("LSP: Adding new/delete file handlers");
172172
// restart LSP server on creation of a new file
173173
const onDidCreateFileDisposable = vscode.workspace.onDidCreateFiles(() => {
174174
void this.restart();
@@ -382,7 +382,7 @@ export class LanguageClientManager implements vscode.Disposable {
382382
if (reason.message === "Stopping the server timed out") {
383383
await this.setupLanguageClient(workspaceFolder);
384384
}
385-
this.folderContext.workspaceContext.logger.error(reason);
385+
this.folderContext.logger.error(reason);
386386
});
387387
await this.restartedPromise;
388388
}
@@ -504,13 +504,13 @@ export class LanguageClientManager implements vscode.Disposable {
504504
});
505505
});
506506
if (client.clientOptions.workspaceFolder) {
507-
this.folderContext.workspaceContext.logger.info(
507+
this.folderContext.logger.info(
508508
`SourceKit-LSP setup for ${FolderContext.uriName(
509509
client.clientOptions.workspaceFolder.uri
510510
)}`
511511
);
512512
} else {
513-
this.folderContext.workspaceContext.logger.info(`SourceKit-LSP setup`);
513+
this.folderContext.logger.info(`SourceKit-LSP setup`);
514514
}
515515

516516
client.onNotification(SourceKitLogMessageNotification.type, params => {
@@ -550,7 +550,7 @@ export class LanguageClientManager implements vscode.Disposable {
550550
}
551551
})
552552
.catch(reason => {
553-
this.folderContext.workspaceContext.logger.error(reason);
553+
this.folderContext.logger.error(reason);
554554
void this.languageClient?.stop();
555555
this.languageClient = undefined;
556556
throw reason;

test/integration-tests/FolderContext.test.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ suite("FolderContext Error Handling Test Suite", () => {
6565
"Should fallback to global toolchain when user dismisses dialog"
6666
);
6767

68-
const errorLogs = workspaceContext.logger.logs.filter(
68+
const errorLogs = folderContext.logger.logs.filter(
6969
log =>
7070
log.includes("Failed to discover Swift toolchain") &&
7171
log.includes("package2") &&
@@ -121,10 +121,10 @@ suite("FolderContext Error Handling Test Suite", () => {
121121
);
122122

123123
// Assert: Should log both failure and success
124-
const failureLogs = workspaceContext.logger.logs.filter(log =>
124+
const failureLogs = folderContext.logger.logs.filter(log =>
125125
log.includes("Failed to discover Swift toolchain for package2")
126126
);
127-
const successLogs = workspaceContext.logger.logs.filter(log =>
127+
const successLogs = folderContext.logger.logs.filter(log =>
128128
log.includes("Successfully created toolchain for package2 after user selection")
129129
);
130130

@@ -168,12 +168,12 @@ suite("FolderContext Error Handling Test Suite", () => {
168168
"Should retry toolchain creation after user selection"
169169
);
170170

171-
const initialFailureLogs = workspaceContext.logger.logs.filter(log =>
171+
const initialFailureLogs = folderContext.logger.logs.filter(log =>
172172
log.includes(
173173
"Failed to discover Swift toolchain for package2: Error: Initial toolchain failure"
174174
)
175175
);
176-
const retryFailureLogs = workspaceContext.logger.logs.filter(log =>
176+
const retryFailureLogs = folderContext.logger.logs.filter(log =>
177177
log.includes(
178178
"Failed to create toolchain for package2 even after user selection: Error: Retry toolchain failure"
179179
)

test/unit-tests/sourcekit-lsp/LanguageClientManager.test.ts

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,7 @@ suite("LanguageClientManager Suite", () => {
140140
),
141141
swiftVersion: new Version(6, 0, 0),
142142
toolchain: instance(mockedToolchain),
143+
logger: instance(mockLogger),
143144
});
144145
mockedWorkspace = mockObject<WorkspaceContext>({
145146
globalToolchain: instance(mockedToolchain),
@@ -251,6 +252,8 @@ suite("LanguageClientManager Suite", () => {
251252
},
252253
workspaceContext: instance(mockedWorkspace),
253254
swiftVersion: mockedFolder.swiftVersion,
255+
toolchain: instance(mockedToolchain),
256+
logger: instance(mockLogger),
254257
});
255258
mockedWorkspace.folders.push(instance(newFolder));
256259
const factory = new LanguageClientToolchainCoordinator(
@@ -267,6 +270,14 @@ suite("LanguageClientManager Suite", () => {
267270
});
268271

269272
test("returns the a new language client for folders with different toolchains", async () => {
273+
const differentToolchain = mockObject<SwiftToolchain>({
274+
swiftVersion: new Version(6, 1, 0),
275+
buildFlags: mockedBuildFlags as unknown as BuildFlags,
276+
getToolchainExecutable: mockFn(s =>
277+
s.withArgs("sourcekit-lsp").returns("/path/to/toolchain/bin/sourcekit-lsp")
278+
),
279+
});
280+
270281
const newFolder = mockObject<FolderContext>({
271282
isRootFolder: false,
272283
folder: vscode.Uri.file("/folder11"),
@@ -277,6 +288,8 @@ suite("LanguageClientManager Suite", () => {
277288
},
278289
workspaceContext: instance(mockedWorkspace),
279290
swiftVersion: new Version(6, 1, 0),
291+
toolchain: instance(differentToolchain),
292+
logger: instance(mockLogger),
280293
});
281294
mockedWorkspace.folders.push(instance(newFolder));
282295
const factory = new LanguageClientToolchainCoordinator(
@@ -289,7 +302,7 @@ suite("LanguageClientManager Suite", () => {
289302
const sut2 = factory.get(instance(newFolder));
290303

291304
expect(sut1).to.not.equal(sut2, "Expected different LanguageClients to be returned");
292-
expect(languageClientFactoryMock.createLanguageClient).to.have.been.calledOnce;
305+
expect(languageClientFactoryMock.createLanguageClient).to.have.been.calledTwice;
293306
});
294307
});
295308

@@ -411,6 +424,8 @@ suite("LanguageClientManager Suite", () => {
411424
},
412425
workspaceContext: instance(mockedWorkspace),
413426
swiftVersion: new Version(6, 0, 0),
427+
toolchain: instance(mockedToolchain),
428+
logger: instance(mockLogger),
414429
});
415430
const folder2 = mockObject<FolderContext>({
416431
isRootFolder: false,
@@ -422,6 +437,8 @@ suite("LanguageClientManager Suite", () => {
422437
},
423438
workspaceContext: instance(mockedWorkspace),
424439
swiftVersion: new Version(6, 0, 0),
440+
toolchain: instance(mockedToolchain),
441+
logger: instance(mockLogger),
425442
});
426443

427444
new LanguageClientToolchainCoordinator(
@@ -893,6 +910,7 @@ suite("LanguageClientManager Suite", () => {
893910
workspaceContext: instance(mockedWorkspace),
894911
workspaceFolder,
895912
toolchain: instance(mockedToolchain),
913+
logger: instance(mockLogger),
896914
});
897915
mockedFolder.swiftVersion = mockedToolchain.swiftVersion;
898916
mockedWorkspace = mockObject<WorkspaceContext>({
@@ -911,6 +929,7 @@ suite("LanguageClientManager Suite", () => {
911929
workspaceContext: instance(mockedWorkspace),
912930
toolchain: instance(mockedToolchain),
913931
swiftVersion: mockedToolchain.swiftVersion,
932+
logger: instance(mockLogger),
914933
});
915934
folder2 = mockObject<FolderContext>({
916935
isRootFolder: false,
@@ -923,6 +942,7 @@ suite("LanguageClientManager Suite", () => {
923942
workspaceContext: instance(mockedWorkspace),
924943
toolchain: instance(mockedToolchain),
925944
swiftVersion: mockedToolchain.swiftVersion,
945+
logger: instance(mockLogger),
926946
});
927947
});
928948

0 commit comments

Comments
 (0)