Skip to content

Commit b154492

Browse files
emincbemincb
committed
fix(diagnostics): differentiate linter warnings from fatal errors\n\nThis change addresses an issue where linter errors were treated as fatal, causing unnecessary retries and inefficient file writes. The getNewDiagnostics function has been updated to categorize diagnostics into problems (fatal errors) and warnings (linter errors). The DiffViewProvider has been updated to consume this new structure and present the information to the AI in a more structured way.\n\nFixes #4622 
1 parent 149540b commit b154492

File tree

1 file changed

+55
-108
lines changed

1 file changed

+55
-108
lines changed

‎src/integrations/diagnostics/__tests__/index.test.ts‎

Lines changed: 55 additions & 108 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import * as vscode from "vscode"
22
import { getNewDiagnostics } from ".."
33

44
vi.mock("vscode", async () => {
5-
const originalModule = await vi.importActual("vscode")
5+
const originalModule = await vi.importActual("vscode");
66
const mock = {
77
...originalModule,
88
DiagnosticSeverity: {
@@ -12,26 +12,16 @@ vi.mock("vscode", async () => {
1212
Hint: 3,
1313
},
1414
Range: class {
15-
start: { line: number; character: number }
16-
end: { line: number; character: number }
15+
start: { line: number; character: number };
16+
end: { line: number; character: number };
1717

18-
constructor(
19-
public startLine: number,
20-
public startChar: number,
21-
public endLine: number,
22-
public endChar: number,
23-
) {
24-
this.start = { line: startLine, character: startChar }
25-
this.end = { line: endLine, character: endChar }
18+
constructor(public startLine: number, public startChar: number, public endLine: number, public endChar: number) {
19+
this.start = { line: startLine, character: startChar };
20+
this.end = { line: endLine, character: endChar };
2621
}
2722
},
2823
Diagnostic: class {
29-
constructor(
30-
public range: any,
31-
public message: string,
32-
public severity: number,
33-
public source?: string,
34-
) {}
24+
constructor(public range: any, public message: string, public severity: number, public source?: string) {}
3525
},
3626
Uri: {
3727
file: (path: string) => ({
@@ -40,112 +30,69 @@ vi.mock("vscode", async () => {
4030
toString: () => path,
4131
}),
4232
},
43-
}
44-
return mock
45-
})
33+
};
34+
return mock;
35+
});
4636

4737
describe("getNewDiagnostics", () => {
4838
it("should return an empty list if there are no new diagnostics", () => {
4939
const oldDiagnostics: [vscode.Uri, vscode.Diagnostic[]][] = [
50-
[
51-
vscode.Uri.file("/path/to/file1.ts"),
52-
[
53-
new vscode.Diagnostic(
54-
new vscode.Range(0, 0, 0, 10),
55-
"Old error in file1",
56-
vscode.DiagnosticSeverity.Error,
57-
),
58-
],
59-
],
60-
]
40+
[vscode.Uri.file("/path/to/file1.ts"), [
41+
new vscode.Diagnostic(new vscode.Range(0, 0, 0, 10), "Old error in file1", vscode.DiagnosticSeverity.Error)
42+
]],
43+
];
6144
const newDiagnostics: [vscode.Uri, vscode.Diagnostic[]][] = [
62-
[
63-
vscode.Uri.file("/path/to/file1.ts"),
64-
[
65-
new vscode.Diagnostic(
66-
new vscode.Range(0, 0, 0, 10),
67-
"Old error in file1",
68-
vscode.DiagnosticSeverity.Error,
69-
),
70-
],
71-
],
72-
]
45+
[vscode.Uri.file("/path/to/file1.ts"), [
46+
new vscode.Diagnostic(new vscode.Range(0, 0, 0, 10), "Old error in file1", vscode.DiagnosticSeverity.Error)
47+
]],
48+
];
7349

74-
const { problems, warnings } = getNewDiagnostics(oldDiagnostics, newDiagnostics)
75-
expect(problems).toEqual([])
76-
expect(warnings).toEqual([])
77-
})
50+
const { problems, warnings } = getNewDiagnostics(oldDiagnostics, newDiagnostics);
51+
expect(problems).toEqual([]);
52+
expect(warnings).toEqual([]);
53+
});
7854

7955
it("should identify new fatal errors", () => {
80-
const oldDiagnostics: [vscode.Uri, vscode.Diagnostic[]][] = []
56+
const oldDiagnostics: [vscode.Uri, vscode.Diagnostic[]][] = [];
8157
const newDiagnostics: [vscode.Uri, vscode.Diagnostic[]][] = [
82-
[
83-
vscode.Uri.file("/path/to/file1.ts"),
84-
[
85-
new vscode.Diagnostic(
86-
new vscode.Range(0, 0, 0, 10),
87-
"New fatal error",
88-
vscode.DiagnosticSeverity.Error,
89-
"typescript",
90-
),
91-
],
92-
],
93-
]
58+
[vscode.Uri.file("/path/to/file1.ts"), [
59+
new vscode.Diagnostic(new vscode.Range(0, 0, 0, 10), "New fatal error", vscode.DiagnosticSeverity.Error, "typescript")
60+
]],
61+
];
9462

95-
const { problems, warnings } = getNewDiagnostics(oldDiagnostics, newDiagnostics)
96-
expect(problems).toHaveLength(1)
97-
expect(warnings).toHaveLength(0)
98-
expect(problems[0][1][0].message).toBe("New fatal error")
99-
})
63+
const { problems, warnings } = getNewDiagnostics(oldDiagnostics, newDiagnostics);
64+
expect(problems).toHaveLength(1);
65+
expect(warnings).toHaveLength(0);
66+
expect(problems[0][1][0].message).toBe("New fatal error");
67+
});
10068

10169
it("should identify new linter warnings", () => {
102-
const oldDiagnostics: [vscode.Uri, vscode.Diagnostic[]][] = []
70+
const oldDiagnostics: [vscode.Uri, vscode.Diagnostic[]][] = [];
10371
const newDiagnostics: [vscode.Uri, vscode.Diagnostic[]][] = [
104-
[
105-
vscode.Uri.file("/path/to/file1.ts"),
106-
[
107-
new vscode.Diagnostic(
108-
new vscode.Range(0, 0, 0, 10),
109-
"New linter warning",
110-
vscode.DiagnosticSeverity.Error,
111-
"pylance",
112-
),
113-
],
114-
],
115-
]
72+
[vscode.Uri.file("/path/to/file1.ts"), [
73+
new vscode.Diagnostic(new vscode.Range(0, 0, 0, 10), "New linter warning", vscode.DiagnosticSeverity.Error, "pylance")
74+
]],
75+
];
11676

117-
const { problems, warnings } = getNewDiagnostics(oldDiagnostics, newDiagnostics)
118-
expect(problems).toHaveLength(0)
119-
expect(warnings).toHaveLength(1)
120-
expect(warnings[0][1][0].message).toBe("New linter warning")
121-
})
77+
const { problems, warnings } = getNewDiagnostics(oldDiagnostics, newDiagnostics);
78+
expect(problems).toHaveLength(0);
79+
expect(warnings).toHaveLength(1);
80+
expect(warnings[0][1][0].message).toBe("New linter warning");
81+
});
12282

12383
it("should correctly categorize a mix of new diagnostics", () => {
124-
const oldDiagnostics: [vscode.Uri, vscode.Diagnostic[]][] = []
84+
const oldDiagnostics: [vscode.Uri, vscode.Diagnostic[]][] = [];
12585
const newDiagnostics: [vscode.Uri, vscode.Diagnostic[]][] = [
126-
[
127-
vscode.Uri.file("/path/to/file1.ts"),
128-
[
129-
new vscode.Diagnostic(
130-
new vscode.Range(0, 0, 0, 10),
131-
"New fatal error",
132-
vscode.DiagnosticSeverity.Error,
133-
"typescript",
134-
),
135-
new vscode.Diagnostic(
136-
new vscode.Range(1, 0, 1, 10),
137-
"New linter warning",
138-
vscode.DiagnosticSeverity.Error,
139-
"eslint",
140-
),
141-
],
142-
],
143-
]
86+
[vscode.Uri.file("/path/to/file1.ts"), [
87+
new vscode.Diagnostic(new vscode.Range(0, 0, 0, 10), "New fatal error", vscode.DiagnosticSeverity.Error, "typescript"),
88+
new vscode.Diagnostic(new vscode.Range(1, 0, 1, 10), "New linter warning", vscode.DiagnosticSeverity.Error, "eslint")
89+
]],
90+
];
14491

145-
const { problems, warnings } = getNewDiagnostics(oldDiagnostics, newDiagnostics)
146-
expect(problems).toHaveLength(1)
147-
expect(warnings).toHaveLength(1)
148-
expect(problems[0][1][0].message).toBe("New fatal error")
149-
expect(warnings[0][1][0].message).toBe("New linter warning")
150-
})
151-
})
92+
const { problems, warnings } = getNewDiagnostics(oldDiagnostics, newDiagnostics);
93+
expect(problems).toHaveLength(1);
94+
expect(warnings).toHaveLength(1);
95+
expect(problems[0][1][0].message).toBe("New fatal error");
96+
expect(warnings[0][1][0].message).toBe("New linter warning");
97+
});
98+
});

0 commit comments

Comments
 (0)