Skip to content

Inject logger directly into FolderContext #1826

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Inject logger directly into FolderContext #1826

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions src/FolderContext.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,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);
Expand Down Expand Up @@ -134,15 +135,16 @@ export class FolderContext implements vscode.Disposable {
linuxMain,
swiftPackage,
workspaceFolder,
workspaceContext
workspaceContext,
workspaceContext.logger
);

const error = await swiftPackage.error;
if (error) {
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
);
Expand Down Expand Up @@ -218,7 +220,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);
Expand Down
10 changes: 5 additions & 5 deletions src/sourcekit-lsp/LanguageClientManager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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 => {
Expand Down Expand Up @@ -550,7 +550,7 @@ export class LanguageClientManager implements vscode.Disposable {
}
})
.catch(reason => {
this.folderContext.workspaceContext.logger.error(reason);
this.folderContext.logger.error(reason);
void this.languageClient?.stop();
this.languageClient = undefined;
throw reason;
Expand Down
10 changes: 5 additions & 5 deletions test/integration-tests/FolderContext.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,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") &&
Expand Down Expand Up @@ -121,10 +121,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")
);

Expand Down Expand Up @@ -168,12 +168,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"
)
Expand Down
22 changes: 21 additions & 1 deletion test/unit-tests/sourcekit-lsp/LanguageClientManager.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,7 @@ suite("LanguageClientManager Suite", () => {
),
swiftVersion: new Version(6, 0, 0),
toolchain: instance(mockedToolchain),
logger: instance(mockLogger),
});
mockedWorkspace = mockObject<WorkspaceContext>({
globalToolchain: instance(mockedToolchain),
Expand Down Expand Up @@ -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(
Expand All @@ -267,6 +270,14 @@ suite("LanguageClientManager Suite", () => {
});

test("returns the a new language client for folders with different toolchains", async () => {
const differentToolchain = mockObject<SwiftToolchain>({
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<FolderContext>({
isRootFolder: false,
folder: vscode.Uri.file("/folder11"),
Expand All @@ -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(
Expand All @@ -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;
});
});

Expand Down Expand Up @@ -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<FolderContext>({
isRootFolder: false,
Expand All @@ -422,6 +437,8 @@ suite("LanguageClientManager Suite", () => {
},
workspaceContext: instance(mockedWorkspace),
swiftVersion: new Version(6, 0, 0),
toolchain: instance(mockedToolchain),
logger: instance(mockLogger),
});

new LanguageClientToolchainCoordinator(
Expand Down Expand Up @@ -893,6 +910,7 @@ suite("LanguageClientManager Suite", () => {
workspaceContext: instance(mockedWorkspace),
workspaceFolder,
toolchain: instance(mockedToolchain),
logger: instance(mockLogger),
});
mockedFolder.swiftVersion = mockedToolchain.swiftVersion;
mockedWorkspace = mockObject<WorkspaceContext>({
Expand All @@ -911,6 +929,7 @@ suite("LanguageClientManager Suite", () => {
workspaceContext: instance(mockedWorkspace),
toolchain: instance(mockedToolchain),
swiftVersion: mockedToolchain.swiftVersion,
logger: instance(mockLogger),
});
folder2 = mockObject<FolderContext>({
isRootFolder: false,
Expand All @@ -923,6 +942,7 @@ suite("LanguageClientManager Suite", () => {
workspaceContext: instance(mockedWorkspace),
toolchain: instance(mockedToolchain),
swiftVersion: mockedToolchain.swiftVersion,
logger: instance(mockLogger),
});
});

Expand Down