Skip to content

Commit 0382f35

Browse files
benibenjpierceboggan
benibenj
authored and
pierceboggan
committed
Rely more on observable document for better notebook support (#519)
* rely more on observable document due to notebooks * set debounceDelayMs to 0
1 parent 59b6ba3 commit 0382f35

File tree

8 files changed

+187
-405
lines changed

8 files changed

+187
-405
lines changed

‎src/extension/inlineEdits/test/vscode-node/diagnosticsCollection.spec.ts

Lines changed: 44 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -5,43 +5,24 @@
55

66
import * as assert from 'assert';
77
import { suite, test } from 'vitest';
8+
import { DiagnosticData } from '../../../../platform/inlineEdits/common/dataTypes/diagnosticData';
9+
import { URI } from '../../../../util/vs/base/common/uri';
810
import { StringEdit } from '../../../../util/vs/editor/common/core/edits/stringEdit';
9-
import { Range } from '../../../../util/vs/editor/common/core/range';
1011
import { OffsetRange } from '../../../../util/vs/editor/common/core/ranges/offsetRange';
1112
import { StringText } from '../../../../util/vs/editor/common/core/text/abstractText';
12-
import { Diagnostic, DiagnosticSeverity } from '../../vscode-node/features/diagnosticsBasedCompletions/diagnosticsCompletions';
13+
import { Diagnostic } from '../../vscode-node/features/diagnosticsBasedCompletions/diagnosticsCompletions';
1314
import { DiagnosticsCollection } from '../../vscode-node/features/diagnosticsCompletionProcessor';
1415

15-
// Helper function to create a mock VS Code diagnostic
16-
function createMockVSCodeDiagnostic(
17-
message: string,
18-
range: Range,
19-
severity: number = 0,
20-
source?: string,
21-
code?: string | number
22-
): any {
23-
return {
24-
message,
25-
range: {
26-
start: { line: range.startLineNumber - 1, character: range.startColumn - 1 },
27-
end: { line: range.endLineNumber - 1, character: range.endColumn - 1 }
28-
},
29-
severity,
30-
source,
31-
code
32-
};
33-
}
34-
3516
// Helper function to create a Diagnostic from a mock VS Code diagnostic
36-
function createDiagnostic(
37-
message: string,
38-
range: Range,
39-
severity: DiagnosticSeverity = DiagnosticSeverity.Error,
40-
source?: string,
41-
code?: string | number
42-
): Diagnostic {
43-
const mockVSCodeDiagnostic = createMockVSCodeDiagnostic(message, range, severity, source, code);
44-
return Diagnostic.fromVSCodeDiagnostic(mockVSCodeDiagnostic);
17+
function createDiagnostic(message: string, range: OffsetRange): Diagnostic {
18+
return new Diagnostic(new DiagnosticData(
19+
URI.parse('file:///test/document.ts'),
20+
message,
21+
'error',
22+
range,
23+
undefined,
24+
undefined
25+
));
4526
}
4627

4728
suite('DiagnosticsCollection', () => {
@@ -54,7 +35,7 @@ suite('DiagnosticsCollection', () => {
5435
const collection = new DiagnosticsCollection();
5536
const diagnostic = createDiagnostic(
5637
'Test error',
57-
new Range(1, 1, 1, 5)
38+
new OffsetRange(0, 4)
5839
);
5940

6041
const result = collection.isEqualAndUpdate([diagnostic]);
@@ -63,8 +44,8 @@ suite('DiagnosticsCollection', () => {
6344
});
6445
test('isEqualAndUpdate should return true when diagnostics are equal', () => {
6546
const collection = new DiagnosticsCollection();
66-
const diagnostic1 = createDiagnostic('Test error', new Range(1, 1, 1, 5));
67-
const diagnostic2 = createDiagnostic('Test error', new Range(1, 1, 1, 5));
47+
const diagnostic1 = createDiagnostic('Test error', new OffsetRange(0, 4));
48+
const diagnostic2 = createDiagnostic('Test error', new OffsetRange(0, 4));
6849

6950
collection.isEqualAndUpdate([diagnostic1]);
7051
const result = collection.isEqualAndUpdate([diagnostic2]);
@@ -73,8 +54,8 @@ suite('DiagnosticsCollection', () => {
7354
});
7455
test('isEqualAndUpdate should return false when a diagnostics is invalidated', () => {
7556
const collection = new DiagnosticsCollection();
76-
const diagnostic1 = createDiagnostic('Test error', new Range(1, 1, 1, 5));
77-
const diagnostic2 = createDiagnostic('Test error', new Range(1, 1, 1, 5));
57+
const diagnostic1 = createDiagnostic('Test error', new OffsetRange(0, 4));
58+
const diagnostic2 = createDiagnostic('Test error', new OffsetRange(0, 4));
7859

7960
collection.isEqualAndUpdate([diagnostic1]);
8061

@@ -88,12 +69,12 @@ suite('DiagnosticsCollection', () => {
8869
suite('applyEdit', () => {
8970
test('should invalidate when typing numbers at the end of a diagnostic range', () => {
9071
const collection = new DiagnosticsCollection();
91-
const diagnostic = createDiagnostic('Test error', new Range(1, 13, 1, 17)); // "test" = positions 12-15 (1-based: 13-17)
72+
const diagnostic = createDiagnostic('Test error', new OffsetRange(12, 17)); // "test" = positions 12-15 (0-based)
9273
collection.isEqualAndUpdate([diagnostic]);
9374

9475
// Replace "test" with "test123"
9576
const before = new StringText('hello world test');
96-
const edit = StringEdit.replace(new OffsetRange(12, 16), 'test123'); // 0-based: 12-15
77+
const edit = StringEdit.replace(new OffsetRange(12, 17), 'test123'); // 0-based: 12-16
9778
const after = edit.applyOnText(before);
9879

9980
const hasInvalidated = collection.applyEdit(before, edit, after);
@@ -103,7 +84,7 @@ suite('DiagnosticsCollection', () => {
10384

10485
test('should invalidate diagnostic when range shrinks', () => {
10586
const collection = new DiagnosticsCollection();
106-
const diagnostic = createDiagnostic('Test error', new Range(1, 7, 1, 12)); // "world"
87+
const diagnostic = createDiagnostic('Test error', new OffsetRange(6, 11)); // "world"
10788
collection.isEqualAndUpdate([diagnostic]);
10889

10990
// Create an edit that removes "w"
@@ -119,7 +100,7 @@ suite('DiagnosticsCollection', () => {
119100

120101
test('should update range when content stays the same and range length unchanged', () => {
121102
const collection = new DiagnosticsCollection();
122-
const diagnostic = createDiagnostic('Test error', new Range(1, 13, 1, 17));
103+
const diagnostic = createDiagnostic('Test error', new OffsetRange(12, 16));
123104
collection.isEqualAndUpdate([diagnostic]);
124105

125106
// Insert " big" without touching the diagnostic range
@@ -135,7 +116,7 @@ suite('DiagnosticsCollection', () => {
135116

136117
test('should invalidate diagnostic when content at range changes with same length', () => {
137118
const collection = new DiagnosticsCollection();
138-
const diagnostic = createDiagnostic('Test error', new Range(1, 13, 1, 17)); // "test"
119+
const diagnostic = createDiagnostic('Test error', new OffsetRange(12, 16)); // "test"
139120
collection.isEqualAndUpdate([diagnostic]);
140121

141122
// Replace "test" with "best"
@@ -150,7 +131,7 @@ suite('DiagnosticsCollection', () => {
150131
});
151132
test('should handle range growth with same prefix content', () => {
152133
const collection = new DiagnosticsCollection();
153-
const diagnostic = createDiagnostic('Test error', new Range(1, 13, 1, 17));
134+
const diagnostic = createDiagnostic('Test error', new OffsetRange(12, 16));
154135
collection.isEqualAndUpdate([diagnostic]);
155136

156137
// "test" becomes "test!" (non-alphanumeric edge)
@@ -164,13 +145,13 @@ suite('DiagnosticsCollection', () => {
164145
assert.strictEqual(diagnostic.isValid(), true);
165146

166147
// Range should still point to the original "test" part
167-
assert.strictEqual(diagnostic.range.startColumn, 13);
168-
assert.strictEqual(diagnostic.range.endColumn, 17);
148+
assert.strictEqual(diagnostic.range.start, 12);
149+
assert.strictEqual(diagnostic.range.endExclusive, 16);
169150
});
170151

171152
test('should handle range growth with same suffix content', () => {
172153
const collection = new DiagnosticsCollection();
173-
const diagnostic = createDiagnostic('Test error', new Range(1, 13, 1, 17)); // "test"
154+
const diagnostic = createDiagnostic('Test error', new OffsetRange(12, 16)); // "test"
174155
collection.isEqualAndUpdate([diagnostic]);
175156

176157
const before = new StringText('hello world test');
@@ -179,16 +160,13 @@ suite('DiagnosticsCollection', () => {
179160

180161
const hasInvalidated = collection.applyEdit(before, edit, after);
181162

182-
assert.strictEqual(hasInvalidated, false);
183-
assert.strictEqual(diagnostic.isValid(), true);
184-
// Range should point to the suffix "test" part
185-
assert.strictEqual(diagnostic.range.startColumn, 15); // 13 + 2 ("ab")
186-
assert.strictEqual(diagnostic.range.endColumn, 19); // 17 + 2 ("ab")
163+
assert.strictEqual(hasInvalidated, true);
164+
assert.strictEqual(diagnostic.isValid(), false);
187165
});
188166

189167
test('should invalidate when edge character is alphanumeric with prefix match', () => {
190168
const collection = new DiagnosticsCollection();
191-
const diagnostic = createDiagnostic('Test error', new Range(1, 13, 1, 17)); // "test"
169+
const diagnostic = createDiagnostic('Test error', new OffsetRange(12, 16)); // "test"
192170
collection.isEqualAndUpdate([diagnostic]);
193171

194172
const before = new StringText('hello world test');
@@ -205,14 +183,13 @@ suite('DiagnosticsCollection', () => {
205183

206184
test('should not invalidate when edge character is non-alphanumeric with prefix match', () => {
207185
const collection = new DiagnosticsCollection();
208-
const diagnostic = createDiagnostic('Test error', new Range(1, 13, 1, 17)); // "test" = positions 12-15 (1-based: 13-17)
186+
const diagnostic = createDiagnostic('Test error', new OffsetRange(12, 16)); // "test" = positions 12-15 (0-based)
209187
collection.isEqualAndUpdate([diagnostic]);
210188

211-
const before = new StringText('hello world test');
212-
const after = new StringText('hello world test!'); // "test" becomes "test!" (non-alphanumeric edge)
213-
214189
// Replace "test" with "test!"
190+
const before = new StringText('hello world test');
215191
const edit = StringEdit.replace(new OffsetRange(12, 16), 'test!'); // 0-based: 12-15
192+
const after = edit.applyOnText(before);
216193

217194
const hasInvalidated = collection.applyEdit(before, edit, after);
218195

@@ -222,15 +199,13 @@ suite('DiagnosticsCollection', () => {
222199

223200
test('should handle multiple diagnostics correctly', () => {
224201
const collection = new DiagnosticsCollection();
225-
const diagnostic1 = createDiagnostic('Error 1', new Range(1, 1, 1, 6)); // "hello" = positions 0-4 (1-based: 1-5), but using 6 for end
226-
const diagnostic2 = createDiagnostic('Error 2', new Range(1, 13, 1, 17)); // "test" = positions 12-15 (1-based: 13-17)
202+
const diagnostic1 = createDiagnostic('Error 1', new OffsetRange(0, 5)); // "hello" = positions 0-4 (0-based)
203+
const diagnostic2 = createDiagnostic('Error 2', new OffsetRange(12, 16)); // "test" = positions 12-15 (0-based)
227204
collection.isEqualAndUpdate([diagnostic1, diagnostic2]);
228205

229206
const before = new StringText('hello world test');
230-
const after = new StringText('hello big world test');
231-
232-
// Insert "big " at position 6 (0-based)
233207
const edit = StringEdit.replace(new OffsetRange(6, 6), 'big ');
208+
const after = edit.applyOnText(before);
234209

235210
const hasInvalidated = collection.applyEdit(before, edit, after);
236211

@@ -239,17 +214,17 @@ suite('DiagnosticsCollection', () => {
239214
assert.strictEqual(diagnostic2.isValid(), true);
240215

241216
// First diagnostic range should be unchanged
242-
assert.strictEqual(diagnostic1.range.startColumn, 1);
243-
assert.strictEqual(diagnostic1.range.endColumn, 6);
217+
assert.strictEqual(diagnostic1.range.start, 0);
218+
assert.strictEqual(diagnostic1.range.endExclusive, 5);
244219

245220
// Second diagnostic range should be shifted by 4 positions ("big ")
246-
assert.strictEqual(diagnostic2.range.startColumn, 17); // 13 + 4
247-
assert.strictEqual(diagnostic2.range.endColumn, 21); // 17 + 4
221+
assert.strictEqual(diagnostic2.range.start, 16);
222+
assert.strictEqual(diagnostic2.range.endExclusive, 20);
248223
});
249224

250225
test('should handle edge case with empty edge character', () => {
251226
const collection = new DiagnosticsCollection();
252-
const diagnostic = createDiagnostic('Test error', new Range(1, 13, 1, 17)); // "test" = positions 12-15 (1-based: 13-17)
227+
const diagnostic = createDiagnostic('Test error', new OffsetRange(12, 16)); // "test" = positions 12-15 (0-based)
253228
collection.isEqualAndUpdate([diagnostic]);
254229

255230
const before = new StringText('hello world test');
@@ -267,7 +242,7 @@ suite('DiagnosticsCollection', () => {
267242

268243
test('should handle suffix match with non-alphanumeric edge character', () => {
269244
const collection = new DiagnosticsCollection();
270-
const diagnostic = createDiagnostic('Test error', new Range(1, 13, 1, 17)); // "test" = positions 12-15 (1-based: 13-17)
245+
const diagnostic = createDiagnostic('Test error', new OffsetRange(12, 16)); // "test" = positions 12-15 (0-based)
271246
collection.isEqualAndUpdate([diagnostic]);
272247

273248
const before = new StringText('hello world test');
@@ -281,13 +256,13 @@ suite('DiagnosticsCollection', () => {
281256
assert.strictEqual(hasInvalidated, false);
282257
assert.strictEqual(diagnostic.isValid(), true);
283258
// Range should point to the suffix "test" part
284-
assert.strictEqual(diagnostic.range.startColumn, 14); // 13 + 1 (".")
285-
assert.strictEqual(diagnostic.range.endColumn, 18); // 17 + 1 (".")
259+
assert.strictEqual(diagnostic.range.start, 13);
260+
assert.strictEqual(diagnostic.range.endExclusive, 17); // 17 + 1 (".")
286261
});
287262

288263
test('should handle case where newOffsetRange is null', () => {
289264
const collection = new DiagnosticsCollection();
290-
const diagnostic = createDiagnostic('Test error', new Range(1, 13, 1, 17)); // "test" = positions 12-15 (1-based: 13-17)
265+
const diagnostic = createDiagnostic('Test error', new OffsetRange(12, 16)); // "test" = positions 12-15 (0-based)
291266
collection.isEqualAndUpdate([diagnostic]);
292267

293268
// Mock applyEditsToRanges to return null (would happen if range is completely removed)

‎src/extension/inlineEdits/vscode-node/features/diagnosticsBasedCompletions/anyDiagnosticsCompletionProvider.ts

Lines changed: 23 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,15 @@
33
* Licensed under the MIT License. See License.txt in the project root for license information.
44
*--------------------------------------------------------------------------------------------*/
55

6+
import { CodeActionData } from '../../../../../platform/inlineEdits/common/dataTypes/codeActionData';
67
import { LanguageId } from '../../../../../platform/inlineEdits/common/dataTypes/languageId';
78
import { ITracer } from '../../../../../util/common/tracing';
89
import { CancellationToken } from '../../../../../util/vs/base/common/cancellation';
910
import { TextReplacement } from '../../../../../util/vs/editor/common/core/edits/textEdit';
1011
import { Position } from '../../../../../util/vs/editor/common/core/position';
1112
import { INextEditDisplayLocation } from '../../../node/nextEditResult';
1213
import { IVSCodeObservableDocument } from '../../parts/vscodeWorkspace';
13-
import { CodeAction, Diagnostic, DiagnosticCompletionItem, DiagnosticInlineEditRequestLogContext, getCodeActionsForDiagnostic, IDiagnosticCodeAction, IDiagnosticCompletionProvider, isDiagnosticWithinDistance, log, logList } from './diagnosticsCompletions';
14+
import { Diagnostic, DiagnosticCompletionItem, DiagnosticInlineEditRequestLogContext, IDiagnosticCodeAction, IDiagnosticCompletionProvider, isDiagnosticWithinDistance, log, logList } from './diagnosticsCompletions';
1415

1516
interface IAnyCodeAction extends IDiagnosticCodeAction {
1617
type: string;
@@ -23,14 +24,19 @@ export class AnyDiagnosticCompletionItem extends DiagnosticCompletionItem {
2324
constructor(
2425
codeAction: IAnyCodeAction,
2526
diagnostic: Diagnostic,
26-
private readonly _nextEditDisplayLocation: INextEditDisplayLocation | undefined,
27+
private readonly _nextEditDisplayLabel: string | undefined,
2728
workspaceDocument: IVSCodeObservableDocument,
2829
) {
2930
super(codeAction.type, diagnostic, codeAction.edit, workspaceDocument);
3031
}
3132

3233
protected override _getDisplayLocation(): INextEditDisplayLocation | undefined {
33-
return this._nextEditDisplayLocation;
34+
if (!this._nextEditDisplayLabel) {
35+
return undefined;
36+
}
37+
38+
const transformer = this._workspaceDocument.value.get().getTransformer();
39+
return { range: transformer.getRange(this.diagnostic.range), label: this._nextEditDisplayLabel };
3440
}
3541
}
3642

@@ -42,14 +48,14 @@ export class AnyDiagnosticCompletionProvider implements IDiagnosticCompletionPro
4248

4349
constructor(private readonly _tracer: ITracer) { }
4450

45-
public providesCompletionsForDiagnostic(diagnostic: Diagnostic, language: LanguageId, pos: Position): boolean {
46-
return isDiagnosticWithinDistance(diagnostic, pos, 5);
51+
public providesCompletionsForDiagnostic(workspaceDocument: IVSCodeObservableDocument, diagnostic: Diagnostic, language: LanguageId, pos: Position): boolean {
52+
return isDiagnosticWithinDistance(workspaceDocument, diagnostic, pos, 5);
4753
}
4854

4955
async provideDiagnosticCompletionItem(workspaceDocument: IVSCodeObservableDocument, sortedDiagnostics: Diagnostic[], pos: Position, logContext: DiagnosticInlineEditRequestLogContext, token: CancellationToken): Promise<AnyDiagnosticCompletionItem | null> {
5056

5157
for (const diagnostic of sortedDiagnostics) {
52-
const availableCodeActions = await getCodeActionsForDiagnostic(diagnostic, workspaceDocument, token);
58+
const availableCodeActions = await workspaceDocument.getCodeActions(diagnostic.range, 3, token);
5359
if (availableCodeActions === undefined) {
5460
log(`Fetching code actions likely timed out for \`${diagnostic.message}\``, logContext, this._tracer);
5561
continue;
@@ -62,29 +68,28 @@ export class AnyDiagnosticCompletionProvider implements IDiagnosticCompletionPro
6268

6369
logList(`Found the following code action which fix \`${diagnostic.message}\``, codeActionsFixingCodeAction, logContext, this._tracer);
6470

65-
const filteredCodeActionsWithEdit = filterCodeActions(codeActionsFixingCodeAction, workspaceDocument);
71+
const filteredCodeActionsWithEdit = filterCodeActions(codeActionsFixingCodeAction);
6672

6773
if (filteredCodeActionsWithEdit.length === 0) {
6874
continue;
6975
}
7076

7177
const codeAction = filteredCodeActionsWithEdit[0];
72-
const edits = codeAction.getEditForWorkspaceDocument(workspaceDocument);
73-
if (!edits) { continue; }
78+
if (!codeAction.edits) { continue; }
7479

75-
const joinedEdit = TextReplacement.joinReplacements(edits, workspaceDocument.value.get());
80+
const joinedEdit = TextReplacement.joinReplacements(codeAction.edits, workspaceDocument.value.get());
7681
const anyCodeAction: IAnyCodeAction = {
7782
edit: joinedEdit,
7883
type: getSanitizedCodeActionTitle(codeAction)
7984
};
8085

81-
let displayLocation: INextEditDisplayLocation | undefined;
86+
let displayLocationLabel: string | undefined;
8287
const editDistance = Math.abs(joinedEdit.range.startLineNumber - pos.lineNumber);
8388
if (editDistance > 12) {
84-
displayLocation = { range: diagnostic.range, label: codeAction.title };
89+
displayLocationLabel = codeAction.title;
8590
}
8691

87-
const item = new AnyDiagnosticCompletionItem(anyCodeAction, diagnostic, displayLocation, workspaceDocument);
92+
const item = new AnyDiagnosticCompletionItem(anyCodeAction, diagnostic, displayLocationLabel, workspaceDocument);
8893
log(`Created Completion Item for diagnostic: ${diagnostic.message}: ${item.toLineEdit().toString()}`);
8994
return item;
9095
}
@@ -95,18 +100,17 @@ export class AnyDiagnosticCompletionProvider implements IDiagnosticCompletionPro
95100
completionItemRejected(item: AnyDiagnosticCompletionItem): void { }
96101
}
97102

98-
function doesCodeActionFixDiagnostics(action: CodeAction, diagnostic: Diagnostic): boolean {
99-
const CodeActionFixedDiagnostics = [...action.diagnostics, ...action.getDiagnosticsReferencedInCommand()];
100-
return CodeActionFixedDiagnostics.some(d => diagnostic.equals(d));
103+
function doesCodeActionFixDiagnostics(action: CodeActionData, diagnostic: Diagnostic): boolean {
104+
return action.diagnostics.some(d => diagnostic.data.message === d.message && diagnostic.data.range.equals(d.range));
101105
}
102106

103-
function getSanitizedCodeActionTitle(action: CodeAction): string {
107+
function getSanitizedCodeActionTitle(action: CodeActionData): string {
104108
return action.title.replace(/(["'])(.*?)\1/g, '$1...$1');
105109
}
106110

107-
function filterCodeActions(codeActionsWithEdit: CodeAction[], workspaceDocument: IVSCodeObservableDocument): CodeAction[] {
111+
function filterCodeActions(codeActionsWithEdit: CodeActionData[]): CodeActionData[] {
108112
return codeActionsWithEdit.filter(action => {
109-
const edit = action.getEditForWorkspaceDocument(workspaceDocument);
113+
const edit = action.edits;
110114
if (!edit) { return false; }
111115

112116
if (action.title === 'Infer parameter types from usage') {

0 commit comments

Comments
 (0)