Skip to content

Make sure swiftc diagnostics are cleaned up after new swift commands #915

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

Merged
merged 1 commit into from
Jun 20, 2024
Merged

Make sure swiftc diagnostics are cleaned up after new swift commands #915

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
9 changes: 3 additions & 6 deletions src/DiagnosticsManager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,12 +190,13 @@ export class DiagnosticsManager implements vscode.Disposable {
}

private removeSwiftcDiagnostics() {
this.diagnosticCollection.forEach((uri, diagnostics) => {
this.allDiagnostics.forEach((diagnostics, path) => {
const newDiagnostics = diagnostics.slice();
this.removeDiagnostics(newDiagnostics, d => this.isSwiftc(d));
if (diagnostics.length !== newDiagnostics.length) {
this.diagnosticCollection.set(uri, newDiagnostics);
this.allDiagnostics.set(path, newDiagnostics);
}
this.updateDiagnosticsCollection(vscode.Uri.file(path));
});
}

Expand Down Expand Up @@ -243,10 +244,6 @@ export class DiagnosticsManager implements vscode.Disposable {
return configuration.diagnosticsCollection !== "onlySourceKit";
}

private includeSourceKitDiagnostics(): boolean {
return configuration.diagnosticsCollection !== "onlySwiftc";
}

private parseDiagnostics(swiftExecution: SwiftExecution): Promise<DiagnosticsMap> {
return new Promise<DiagnosticsMap>(res => {
const diagnostics = new Map();
Expand Down
174 changes: 81 additions & 93 deletions test/suite/DiagnosticsManager.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,32 @@ suite("DiagnosticsManager Test Suite", async function () {
assert.equal(diagnostics.length, 1);
assertHasDiagnostic(mainUri, outputDiagnostic);
});

test("New set of swiftc diagnostics clear old list", async () => {
let fixture = testSwiftTask("swift", ["build"], workspaceFolder, toolchain);
await vscode.tasks.executeTask(fixture.task);
let diagnosticsPromise = waitForDiagnostics([mainUri]);
// Wait to spawn before writing
fixture.process.write(`${mainUri.fsPath}:13:5: error: Cannot find 'foo' in scope`);
fixture.process.close(1);
await waitForNoRunningTasks();
await diagnosticsPromise;
let diagnostics = vscode.languages.getDiagnostics(mainUri);
// Should only include one
assert.equal(diagnostics.length, 1);
assertHasDiagnostic(mainUri, outputDiagnostic);

// Run again but no diagnostics returned
fixture = testSwiftTask("swift", ["build"], workspaceFolder, toolchain);
await vscode.tasks.executeTask(fixture.task);
diagnosticsPromise = waitForDiagnostics([mainUri]);
fixture.process.close(0);
await waitForNoRunningTasks();
await diagnosticsPromise;
diagnostics = vscode.languages.getDiagnostics(mainUri);
// Should have cleaned up
assert.equal(diagnostics.length, 0);
});
});
});

Expand Down Expand Up @@ -346,30 +372,6 @@ suite("DiagnosticsManager Test Suite", async function () {
assertHasDiagnostic(mainUri, sourcekitErrorDiagnostic);
assertHasDiagnostic(mainUri, sourcekitWarningDiagnostic);
});

test("fix swiftc diagnostic", async () => {
// Add initial diagnostics
workspaceContext.diagnostics.handleDiagnostics(
mainUri,
DiagnosticsManager.sourcekit,
[sourcekitErrorDiagnostic, sourcekitWarningDiagnostic]
);
workspaceContext.diagnostics.handleDiagnostics(mainUri, DiagnosticsManager.swiftc, [
swiftcErrorDiagnostic,
]);

// Have SourceKit some have been fixed
workspaceContext.diagnostics.handleDiagnostics(
mainUri,
DiagnosticsManager.sourcekit,
[sourcekitWarningDiagnostic]
);

// check kept all
assertWithoutDiagnostic(mainUri, swiftcErrorDiagnostic);
assertWithoutDiagnostic(mainUri, sourcekitErrorDiagnostic);
assertHasDiagnostic(mainUri, sourcekitWarningDiagnostic);
});
});

suite("keepSourceKit", () => {
Expand Down Expand Up @@ -477,30 +479,6 @@ suite("DiagnosticsManager Test Suite", async function () {
// kept unique swiftc diagnostic
assertHasDiagnostic(mainUri, swiftcWarningDiagnostic);
});

test("fix swiftc diagnostic", async () => {
// Add initial diagnostics
workspaceContext.diagnostics.handleDiagnostics(
mainUri,
DiagnosticsManager.sourcekit,
[sourcekitErrorDiagnostic, sourcekitWarningDiagnostic]
);
workspaceContext.diagnostics.handleDiagnostics(mainUri, DiagnosticsManager.swiftc, [
swiftcErrorDiagnostic,
]);

// Have SourceKit some have been fixed
workspaceContext.diagnostics.handleDiagnostics(
mainUri,
DiagnosticsManager.sourcekit,
[sourcekitWarningDiagnostic]
);

// check kept all
assertWithoutDiagnostic(mainUri, swiftcErrorDiagnostic);
assertWithoutDiagnostic(mainUri, sourcekitErrorDiagnostic);
assertHasDiagnostic(mainUri, sourcekitWarningDiagnostic);
});
});

suite("keepSwiftc", () => {
Expand Down Expand Up @@ -583,30 +561,6 @@ suite("DiagnosticsManager Test Suite", async function () {
// kept unique SourceKit diagnostic
assertHasDiagnostic(mainUri, sourcekitWarningDiagnostic);
});

test("fix swiftc diagnostic", async () => {
// Add initial diagnostics
workspaceContext.diagnostics.handleDiagnostics(
mainUri,
DiagnosticsManager.sourcekit,
[sourcekitErrorDiagnostic, sourcekitWarningDiagnostic]
);
workspaceContext.diagnostics.handleDiagnostics(mainUri, DiagnosticsManager.swiftc, [
swiftcErrorDiagnostic,
]);

// Have SourceKit some have been fixed
workspaceContext.diagnostics.handleDiagnostics(
mainUri,
DiagnosticsManager.sourcekit,
[sourcekitWarningDiagnostic]
);

// check kept all
assertWithoutDiagnostic(mainUri, swiftcErrorDiagnostic);
assertWithoutDiagnostic(mainUri, sourcekitErrorDiagnostic);
assertHasDiagnostic(mainUri, sourcekitWarningDiagnostic);
});
});

suite("onlySourceKit", () => {
Expand Down Expand Up @@ -734,30 +688,64 @@ suite("DiagnosticsManager Test Suite", async function () {
assertWithoutDiagnostic(mainUri, sourcekitErrorDiagnostic);
assertWithoutDiagnostic(mainUri, sourcekitWarningDiagnostic);
});
});

test("fix swiftc diagnostic", async () => {
// Add initial diagnostics
workspaceContext.diagnostics.handleDiagnostics(
mainUri,
DiagnosticsManager.sourcekit,
[sourcekitErrorDiagnostic, sourcekitWarningDiagnostic]
);
workspaceContext.diagnostics.handleDiagnostics(mainUri, DiagnosticsManager.swiftc, [
swiftcErrorDiagnostic,
]);
test("SourceKit removes swiftc diagnostic (SourceKit shows first)", async () => {
// Add initial diagnostics
workspaceContext.diagnostics.handleDiagnostics(mainUri, DiagnosticsManager.sourcekit, [
sourcekitErrorDiagnostic,
sourcekitWarningDiagnostic,
]);
workspaceContext.diagnostics.handleDiagnostics(mainUri, DiagnosticsManager.swiftc, [
swiftcErrorDiagnostic,
]);

// Have SourceKit indicate some have been fixed
workspaceContext.diagnostics.handleDiagnostics(mainUri, DiagnosticsManager.sourcekit, [
sourcekitWarningDiagnostic,
]);

// check cleaned up stale error
assertWithoutDiagnostic(mainUri, swiftcErrorDiagnostic);
assertWithoutDiagnostic(mainUri, sourcekitErrorDiagnostic);
assertHasDiagnostic(mainUri, sourcekitWarningDiagnostic);
});

// Have SourceKit some have been fixed
workspaceContext.diagnostics.handleDiagnostics(
mainUri,
DiagnosticsManager.sourcekit,
[sourcekitWarningDiagnostic]
);
test("SourceKit removes swiftc diagnostic (swiftc shows first)", async () => {
// Add initial diagnostics
workspaceContext.diagnostics.handleDiagnostics(mainUri, DiagnosticsManager.swiftc, [
swiftcErrorDiagnostic,
swiftcWarningDiagnostic,
]);
workspaceContext.diagnostics.handleDiagnostics(mainUri, DiagnosticsManager.sourcekit, [
sourcekitErrorDiagnostic,
]);

// Have SourceKit indicate has been fixed
workspaceContext.diagnostics.handleDiagnostics(
mainUri,
DiagnosticsManager.sourcekit,
[]
);

// check kept all
assertWithoutDiagnostic(mainUri, swiftcErrorDiagnostic);
assertWithoutDiagnostic(mainUri, sourcekitErrorDiagnostic);
assertWithoutDiagnostic(mainUri, sourcekitWarningDiagnostic);
});
// check cleaned up stale error
assertWithoutDiagnostic(mainUri, swiftcErrorDiagnostic);
assertWithoutDiagnostic(mainUri, sourcekitErrorDiagnostic);
assertHasDiagnostic(mainUri, swiftcWarningDiagnostic);
});

test("don't remove swiftc diagnostics when SourceKit never matched", async () => {
workspaceContext.diagnostics.handleDiagnostics(mainUri, DiagnosticsManager.swiftc, [
swiftcErrorDiagnostic,
]);

workspaceContext.diagnostics.handleDiagnostics(mainUri, DiagnosticsManager.sourcekit, [
sourcekitWarningDiagnostic,
]);

// Should not have cleaned up swiftc error
assertHasDiagnostic(mainUri, swiftcErrorDiagnostic);
assertHasDiagnostic(mainUri, sourcekitWarningDiagnostic);
});
});
});