Skip to content

Commit 9df3a17

Browse files
benmcmorranbenmcmorran
authored
Always use related files API for Copilot completions (#12848)
1 parent 038a5ee commit 9df3a17

File tree

3 files changed

+7
-166
lines changed

3 files changed

+7
-166
lines changed

Diff for: ‎Extension/src/LanguageServer/copilotProviders.ts

-31
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,9 @@
66

77
import * as vscode from 'vscode';
88
import * as util from '../common';
9-
import * as telemetry from '../telemetry';
109
import { ChatContextResult, GetIncludesResult } from './client';
1110
import { getActiveClient } from './extension';
1211

13-
let isRelatedFilesApiEnabled: boolean | undefined;
14-
1512
export interface CopilotTrait {
1613
name: string;
1714
value: string;
@@ -31,10 +28,6 @@ export interface CopilotApi {
3128
}
3229

3330
export async function registerRelatedFilesProvider(): Promise<void> {
34-
if (!await getIsRelatedFilesApiEnabled()) {
35-
return;
36-
}
37-
3831
const api = await getCopilotApi();
3932
if (util.extensionContext && api) {
4033
try {
@@ -79,12 +72,6 @@ export async function registerRelatedFilesProvider(): Promise<void> {
7972
}
8073
}
8174

82-
export async function registerRelatedFilesCommands(commandDisposables: vscode.Disposable[], enabled: boolean): Promise<void> {
83-
if (await getIsRelatedFilesApiEnabled()) {
84-
commandDisposables.push(vscode.commands.registerCommand('C_Cpp.getIncludes', enabled ? (maxDepth: number) => getIncludes(maxDepth) : () => Promise.resolve()));
85-
}
86-
}
87-
8875
async function getIncludesWithCancellation(maxDepth: number, token: vscode.CancellationToken): Promise<GetIncludesResult> {
8976
const activeClient = getActiveClient();
9077
const includes = await activeClient.getIncludes(maxDepth, token);
@@ -98,24 +85,6 @@ async function getIncludesWithCancellation(maxDepth: number, token: vscode.Cance
9885
return includes;
9986
}
10087

101-
async function getIncludes(maxDepth: number): Promise<GetIncludesResult> {
102-
const tokenSource = new vscode.CancellationTokenSource();
103-
try {
104-
const includes = await getIncludesWithCancellation(maxDepth, tokenSource.token);
105-
return includes;
106-
} finally {
107-
tokenSource.dispose();
108-
}
109-
}
110-
111-
async function getIsRelatedFilesApiEnabled(): Promise<boolean> {
112-
if (isRelatedFilesApiEnabled === undefined) {
113-
isRelatedFilesApiEnabled = await telemetry.isExperimentEnabled("CppToolsRelatedFilesApi");
114-
}
115-
116-
return isRelatedFilesApiEnabled;
117-
}
118-
11988
export async function getCopilotApi(): Promise<CopilotApi | undefined> {
12089
const copilotExtension = vscode.extensions.getExtension<CopilotApi>('github.copilot');
12190
if (!copilotExtension) {

Diff for: ‎Extension/src/LanguageServer/extension.ts

+1-3
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ import * as telemetry from '../telemetry';
2323
import { Client, DefaultClient, DoxygenCodeActionCommandArguments, openFileVersions } from './client';
2424
import { ClientCollection } from './clientCollection';
2525
import { CodeActionDiagnosticInfo, CodeAnalysisDiagnosticIdentifiersAndUri, codeAnalysisAllFixes, codeAnalysisCodeToFixes, codeAnalysisFileToCodeActions } from './codeAnalysis';
26-
import { registerRelatedFilesCommands, registerRelatedFilesProvider } from './copilotProviders';
26+
import { registerRelatedFilesProvider } from './copilotProviders';
2727
import { CppBuildTaskProvider } from './cppBuildTaskProvider';
2828
import { getCustomConfigProviders } from './customProviders';
2929
import { getLanguageConfig } from './languageConfig';
@@ -411,8 +411,6 @@ export async function registerCommands(enabled: boolean): Promise<void> {
411411
commandDisposables.push(vscode.commands.registerCommand('C_Cpp.ExtractToFreeFunction', enabled ? () => onExtractToFunction(true, false) : onDisabledCommand));
412412
commandDisposables.push(vscode.commands.registerCommand('C_Cpp.ExtractToMemberFunction', enabled ? () => onExtractToFunction(false, true) : onDisabledCommand));
413413
commandDisposables.push(vscode.commands.registerCommand('C_Cpp.ExpandSelection', enabled ? (r: Range) => onExpandSelection(r) : onDisabledCommand));
414-
415-
await registerRelatedFilesCommands(commandDisposables, enabled);
416414
}
417415

418416
function onDisabledCommand() {

Diff for: ‎Extension/test/scenarios/SingleRootProject/tests/copilotProviders.test.ts

+6-132
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,9 @@ import * as util from '../../../../src/common';
1212
import { ChatContextResult, DefaultClient, GetIncludesResult } from '../../../../src/LanguageServer/client';
1313
import { CopilotApi, CopilotTrait } from '../../../../src/LanguageServer/copilotProviders';
1414
import * as extension from '../../../../src/LanguageServer/extension';
15-
import * as telemetry from '../../../../src/telemetry';
1615

1716
describe('registerRelatedFilesProvider', () => {
1817
let moduleUnderTest: any;
19-
let getIsRelatedFilesApiEnabledStub: sinon.SinonStub;
2018
let mockCopilotApi: sinon.SinonStubbedInstance<CopilotApi>;
2119
let getActiveClientStub: sinon.SinonStub;
2220
let activeClientStub: sinon.SinonStubbedInstance<DefaultClient>;
@@ -32,7 +30,6 @@ describe('registerRelatedFilesProvider', () => {
3230
{} // Stub if you need to, or keep the object empty
3331
);
3432

35-
getIsRelatedFilesApiEnabledStub = sinon.stub(telemetry, 'isExperimentEnabled');
3633
sinon.stub(util, 'extensionContext').value({ extension: { id: 'test-extension-id' } });
3734

3835
class MockCopilotApi implements CopilotApi {
@@ -71,11 +68,10 @@ describe('registerRelatedFilesProvider', () => {
7168
sinon.restore();
7269
});
7370

74-
const arrange = ({ cppToolsRelatedFilesApi, vscodeExtension, getIncludeFiles, chatContext, rootUri, flags }:
75-
{ cppToolsRelatedFilesApi: boolean; vscodeExtension?: vscode.Extension<unknown>; getIncludeFiles?: GetIncludesResult; chatContext?: ChatContextResult; rootUri?: vscode.Uri; flags?: Record<string, unknown> } =
76-
{ cppToolsRelatedFilesApi: false, vscodeExtension: undefined, getIncludeFiles: undefined, chatContext: undefined, rootUri: undefined, flags: {} }
71+
const arrange = ({ vscodeExtension, getIncludeFiles, chatContext, rootUri, flags }:
72+
{ vscodeExtension?: vscode.Extension<unknown>; getIncludeFiles?: GetIncludesResult; chatContext?: ChatContextResult; rootUri?: vscode.Uri; flags?: Record<string, unknown> } =
73+
{ vscodeExtension: undefined, getIncludeFiles: undefined, chatContext: undefined, rootUri: undefined, flags: {} }
7774
) => {
78-
getIsRelatedFilesApiEnabledStub.withArgs('CppToolsRelatedFilesApi').resolves(cppToolsRelatedFilesApi);
7975
activeClientStub.getIncludes.resolves(getIncludeFiles);
8076
activeClientStub.getChatContext.resolves(chatContext);
8177
sinon.stub(activeClientStub, 'RootUri').get(() => rootUri);
@@ -95,31 +91,19 @@ describe('registerRelatedFilesProvider', () => {
9591
vscodeGetExtensionsStub = sinon.stub(vscode.extensions, 'getExtension').returns(vscodeExtension);
9692
};
9793

98-
it('should not register provider if CppToolsRelatedFilesApi is not enabled', async () => {
94+
it('should register provider', async () => {
9995
arrange(
100-
{ cppToolsRelatedFilesApi: false }
96+
{ vscodeExtension: vscodeExtension }
10197
);
10298

10399
await moduleUnderTest.registerRelatedFilesProvider();
104100

105-
ok(getIsRelatedFilesApiEnabledStub.calledOnce, 'getIsRelatedFilesApiEnabled should be called once');
106-
});
107-
108-
it('should register provider if CppToolsRelatedFilesApi is enabled', async () => {
109-
arrange(
110-
{ cppToolsRelatedFilesApi: true, vscodeExtension: vscodeExtension }
111-
);
112-
113-
await moduleUnderTest.registerRelatedFilesProvider();
114-
115-
ok(getIsRelatedFilesApiEnabledStub.calledOnce, 'getIsRelatedFilesApiEnabled should be called once');
116101
ok(vscodeGetExtensionsStub.calledOnce, 'vscode.extensions.getExtension should be called once');
117102
ok(mockCopilotApi.registerRelatedFilesProvider.calledWithMatch(sinon.match({ extensionId: 'test-extension-id', languageId: sinon.match.in(['c', 'cpp', 'cuda-cpp']) })), 'registerRelatedFilesProvider should be called with the correct providerId and languageId');
118103
});
119104

120105
it('should not add #cpp traits when ChatContext isn\'t available.', async () => {
121106
arrange({
122-
cppToolsRelatedFilesApi: true,
123107
vscodeExtension: vscodeExtension,
124108
getIncludeFiles: { includedFiles: ['c:\\system\\include\\vector', 'c:\\system\\include\\string', 'C:\\src\\my_project\\foo.h'] },
125109
chatContext: undefined,
@@ -130,7 +114,6 @@ describe('registerRelatedFilesProvider', () => {
130114

131115
const result = await callbackPromise;
132116

133-
ok(getIsRelatedFilesApiEnabledStub.calledOnce, 'getIsRelatedFilesApiEnabled should be called once');
134117
ok(vscodeGetExtensionsStub.calledOnce, 'vscode.extensions.getExtension should be called once');
135118
ok(mockCopilotApi.registerRelatedFilesProvider.calledWithMatch(sinon.match({ extensionId: 'test-extension-id', languageId: sinon.match.in(['c', 'cpp', 'cuda-cpp']) })), 'registerRelatedFilesProvider should be called with the correct providerId and languageId');
136119
ok(getActiveClientStub.callCount !== 0, 'getActiveClient should be called');
@@ -143,7 +126,6 @@ describe('registerRelatedFilesProvider', () => {
143126

144127
it('should not add #cpp traits when copilotcppTraits flag is false.', async () => {
145128
arrange({
146-
cppToolsRelatedFilesApi: true,
147129
vscodeExtension: vscodeExtension,
148130
getIncludeFiles: { includedFiles: ['c:\\system\\include\\vector', 'c:\\system\\include\\string', 'C:\\src\\my_project\\foo.h'] },
149131
chatContext: {
@@ -160,7 +142,6 @@ describe('registerRelatedFilesProvider', () => {
160142

161143
const result = await callbackPromise;
162144

163-
ok(getIsRelatedFilesApiEnabledStub.calledOnce, 'getIsRelatedFilesApiEnabled should be called once');
164145
ok(vscodeGetExtensionsStub.calledOnce, 'vscode.extensions.getExtension should be called once');
165146
ok(mockCopilotApi.registerRelatedFilesProvider.calledWithMatch(sinon.match({ extensionId: 'test-extension-id', languageId: sinon.match.in(['c', 'cpp', 'cuda-cpp']) })), 'registerRelatedFilesProvider should be called with the correct providerId and languageId');
166147
ok(getActiveClientStub.callCount !== 0, 'getActiveClient should be called');
@@ -173,7 +154,6 @@ describe('registerRelatedFilesProvider', () => {
173154

174155
it('should add #cpp traits when copilotcppTraits flag is true.', async () => {
175156
arrange({
176-
cppToolsRelatedFilesApi: true,
177157
vscodeExtension: vscodeExtension,
178158
getIncludeFiles: { includedFiles: ['c:\\system\\include\\vector', 'c:\\system\\include\\string', 'C:\\src\\my_project\\foo.h'] },
179159
chatContext: {
@@ -190,7 +170,6 @@ describe('registerRelatedFilesProvider', () => {
190170

191171
const result = await callbackPromise;
192172

193-
ok(getIsRelatedFilesApiEnabledStub.calledOnce, 'getIsRelatedFilesApiEnabled should be called once');
194173
ok(vscodeGetExtensionsStub.calledOnce, 'vscode.extensions.getExtension should be called once');
195174
ok(mockCopilotApi.registerRelatedFilesProvider.calledThrice, 'registerRelatedFilesProvider should be called three times');
196175
ok(mockCopilotApi.registerRelatedFilesProvider.calledWithMatch(sinon.match({ extensionId: 'test-extension-id', languageId: sinon.match.in(['c', 'cpp', 'cuda-cpp']) })), 'registerRelatedFilesProvider should be called with the correct providerId and languageId');
@@ -226,7 +205,6 @@ describe('registerRelatedFilesProvider', () => {
226205
it('should exclude #cpp traits per copilotcppExcludeTraits.', async () => {
227206
const excludeTraits = ['compiler', 'targetPlatform'];
228207
arrange({
229-
cppToolsRelatedFilesApi: true,
230208
vscodeExtension: vscodeExtension,
231209
getIncludeFiles: { includedFiles: ['c:\\system\\include\\vector', 'c:\\system\\include\\string', 'C:\\src\\my_project\\foo.h'] },
232210
chatContext: {
@@ -243,7 +221,6 @@ describe('registerRelatedFilesProvider', () => {
243221

244222
const result = await callbackPromise;
245223

246-
ok(getIsRelatedFilesApiEnabledStub.calledOnce, 'getIsRelatedFilesApiEnabled should be called once');
247224
ok(vscodeGetExtensionsStub.calledOnce, 'vscode.extensions.getExtension should be called once');
248225
ok(mockCopilotApi.registerRelatedFilesProvider.calledThrice, 'registerRelatedFilesProvider should be called three times');
249226
ok(mockCopilotApi.registerRelatedFilesProvider.calledWithMatch(sinon.match({ extensionId: 'test-extension-id', languageId: sinon.match.in(['c', 'cpp', 'cuda-cpp']) })), 'registerRelatedFilesProvider should be called with the correct providerId and languageId');
@@ -258,114 +235,11 @@ describe('registerRelatedFilesProvider', () => {
258235
});
259236

260237
it('should handle errors during provider registration', async () => {
261-
arrange(
262-
{ cppToolsRelatedFilesApi: true }
263-
);
238+
arrange({});
264239

265240
await moduleUnderTest.registerRelatedFilesProvider();
266241

267-
ok(getIsRelatedFilesApiEnabledStub.calledOnce, 'getIsRelatedFilesApiEnabled should be called once');
268242
ok(vscodeGetExtensionsStub.calledOnce, 'vscode.extensions.getExtension should be called once');
269243
ok(mockCopilotApi.registerRelatedFilesProvider.notCalled, 'registerRelatedFilesProvider should not be called');
270244
});
271245
});
272-
273-
describe('registerRelatedFilesCommands', () => {
274-
let moduleUnderTest: any;
275-
let getIsRelatedFilesApiEnabledStub: sinon.SinonStub;
276-
let registerCommandStub: sinon.SinonStub;
277-
let commandDisposables: vscode.Disposable[];
278-
let getActiveClientStub: sinon.SinonStub;
279-
let activeClientStub: sinon.SinonStubbedInstance<DefaultClient>;
280-
let getIncludesResult: Promise<GetIncludesResult | undefined> = Promise.resolve(undefined);
281-
282-
beforeEach(() => {
283-
proxyquire.noPreserveCache(); // Tells proxyquire to not fetch the module from cache
284-
// Ensures that each test has a freshly loaded instance of moduleUnderTest
285-
moduleUnderTest = proxyquire(
286-
'../../../../src/LanguageServer/copilotProviders',
287-
{} // Stub if you need to, or keep the object empty
288-
);
289-
290-
activeClientStub = sinon.createStubInstance(DefaultClient);
291-
getActiveClientStub = sinon.stub(extension, 'getActiveClient').returns(activeClientStub);
292-
activeClientStub.getIncludes.resolves({ includedFiles: [] });
293-
getIsRelatedFilesApiEnabledStub = sinon.stub(telemetry, 'isExperimentEnabled');
294-
registerCommandStub = sinon.stub(vscode.commands, 'registerCommand');
295-
commandDisposables = [];
296-
});
297-
298-
afterEach(() => {
299-
sinon.restore();
300-
});
301-
302-
const arrange = ({ cppToolsRelatedFilesApi, getIncludeFiles, rootUri }:
303-
{ cppToolsRelatedFilesApi: boolean; getIncludeFiles?: GetIncludesResult; rootUri?: vscode.Uri } =
304-
{ cppToolsRelatedFilesApi: false, getIncludeFiles: undefined, rootUri: undefined }
305-
) => {
306-
getIsRelatedFilesApiEnabledStub.withArgs('CppToolsRelatedFilesApi').resolves(cppToolsRelatedFilesApi);
307-
activeClientStub.getIncludes.resolves(getIncludeFiles);
308-
sinon.stub(activeClientStub, 'RootUri').get(() => rootUri);
309-
registerCommandStub.callsFake((command: string, callback: (maxDepth: number) => Promise<GetIncludesResult>) => {
310-
getIncludesResult = callback(1);
311-
});
312-
};
313-
314-
it('should register C_Cpp.getIncludes command if CppToolsRelatedFilesApi is enabled', async () => {
315-
arrange({ cppToolsRelatedFilesApi: true });
316-
317-
await moduleUnderTest.registerRelatedFilesCommands(commandDisposables, true);
318-
319-
ok(getIsRelatedFilesApiEnabledStub.calledOnce, 'getIsRelatedFilesApiEnabled should be called once');
320-
ok(registerCommandStub.calledOnce, 'registerCommand should be called once');
321-
ok(commandDisposables.length === 1, 'commandDisposables should have one disposable');
322-
ok(registerCommandStub.calledWithMatch('C_Cpp.getIncludes', sinon.match.func), 'registerCommand should be called with "C_Cpp.getIncludes" and a function');
323-
});
324-
325-
it('should register C_Cpp.getIncludes command that can handle requests properly', async () => {
326-
arrange({
327-
cppToolsRelatedFilesApi: true,
328-
getIncludeFiles: { includedFiles: ['c:\\system\\include\\vector', 'c:\\system\\include\\string', 'C:\\src\\my_project\\foo1.h', 'C:\\src\\my_project\\foo2.h'] },
329-
rootUri: vscode.Uri.file('C:\\src\\my_project')
330-
});
331-
await moduleUnderTest.registerRelatedFilesCommands(commandDisposables, true);
332-
333-
const includesResult = await getIncludesResult;
334-
335-
ok(getIsRelatedFilesApiEnabledStub.calledOnce, 'getIsRelatedFilesApiEnabled should be called once');
336-
ok(getActiveClientStub.calledOnce, 'getActiveClient should be called once');
337-
ok(includesResult, 'includesResult should be defined');
338-
ok(includesResult?.includedFiles.length === 2, 'includesResult should have 2 included files');
339-
ok(includesResult?.includedFiles[0] === 'C:\\src\\my_project\\foo1.h', 'includesResult should have "C:\\src\\my_project\\foo1.h"');
340-
ok(includesResult?.includedFiles[1] === 'C:\\src\\my_project\\foo2.h', 'includesResult should have "C:\\src\\my_project\\foo2.h"');
341-
});
342-
343-
it('should not register C_Cpp.getIncludes command if CppToolsRelatedFilesApi is not enabled', async () => {
344-
arrange({
345-
cppToolsRelatedFilesApi: false
346-
});
347-
348-
await moduleUnderTest.registerRelatedFilesCommands(commandDisposables, true);
349-
350-
ok(getIsRelatedFilesApiEnabledStub.calledOnce, 'getIsRelatedFilesApiEnabled should be called once');
351-
ok(registerCommandStub.notCalled, 'registerCommand should not be called');
352-
ok(commandDisposables.length === 0, 'commandDisposables should be empty');
353-
});
354-
355-
it('should register C_Cpp.getIncludes as no-op command if enabled is false', async () => {
356-
arrange({
357-
cppToolsRelatedFilesApi: true,
358-
getIncludeFiles: { includedFiles: ['c:\\system\\include\\vector', 'c:\\system\\include\\string', 'C:\\src\\my_project\\foo.h'] },
359-
rootUri: vscode.Uri.file('C:\\src\\my_project')
360-
});
361-
await moduleUnderTest.registerRelatedFilesCommands(commandDisposables, false);
362-
363-
const includesResult = await getIncludesResult;
364-
365-
ok(getIsRelatedFilesApiEnabledStub.calledOnce, 'getIsRelatedFilesApiEnabled should be called once');
366-
ok(registerCommandStub.calledOnce, 'registerCommand should be called once');
367-
ok(commandDisposables.length === 1, 'commandDisposables should have one disposable');
368-
ok(includesResult === undefined, 'includesResult should be undefined');
369-
ok(registerCommandStub.calledWithMatch('C_Cpp.getIncludes', sinon.match.func), 'registerCommand should be called with "C_Cpp.getIncludes" and a function');
370-
});
371-
});

0 commit comments

Comments
 (0)