Skip to content

Commit 2f32bf8

Browse files
Fix OOP crash issue with copilot change analysis (#78931)
2 parents c6ef6e8 + a50f310 commit 2f32bf8

File tree

3 files changed

+35
-9
lines changed

3 files changed

+35
-9
lines changed

src/Features/Core/Portable/Copilot/CopilotChangeAnalysis.cs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,4 +55,6 @@ internal readonly record struct CopilotCodeFixAnalysis(
5555
[property: DataMember(Order = 3)] Dictionary<string, TimeSpan> DiagnosticIdToApplicationTime,
5656
[property: DataMember(Order = 4)] Dictionary<string, HashSet<string>> DiagnosticIdToProviderName,
5757
[property: DataMember(Order = 5)] Dictionary<string, TimeSpan> ProviderNameToApplicationTime,
58-
[property: DataMember(Order = 6)] Dictionary<string, bool> ProviderNameToHasConflict);
58+
[property: DataMember(Order = 6)] Dictionary<string, bool> ProviderNameToHasConflict,
59+
[property: DataMember(Order = 7)] Dictionary<string, int> ProviderNameToTotalCount,
60+
[property: DataMember(Order = 8)] Dictionary<string, int> ProviderNameToSuccessCount);

src/Features/Core/Portable/Copilot/CopilotChangeAnalysisUtilities.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,8 @@ public static IDisposable LogCopilotChangeAnalysis(
120120
d["CodeFixAnalysis_DiagnosticIdToProviderName"] = StringifyDictionary(analysisResult.CodeFixAnalysis.DiagnosticIdToProviderName);
121121
d["CodeFixAnalysis_ProviderNameToApplicationTime"] = StringifyDictionary(analysisResult.CodeFixAnalysis.ProviderNameToApplicationTime);
122122
d["CodeFixAnalysis_ProviderNameToHasConflict"] = StringifyDictionary(analysisResult.CodeFixAnalysis.ProviderNameToHasConflict);
123+
d["CodeFixAnalysis_ProviderNameToTotalCount"] = StringifyDictionary(analysisResult.CodeFixAnalysis.ProviderNameToTotalCount);
124+
d["CodeFixAnalysis_ProviderNameToSuccessCount"] = StringifyDictionary(analysisResult.CodeFixAnalysis.ProviderNameToSuccessCount);
123125
}, args: (featureId, accepted, proposalId, analysisResult)),
124126
cancellationToken);
125127
}

src/Features/Core/Portable/Copilot/ICopilotChangeAnalysisService.cs

Lines changed: 30 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
using Microsoft.CodeAnalysis.CodeActions;
1313
using Microsoft.CodeAnalysis.CodeFixes;
1414
using Microsoft.CodeAnalysis.Diagnostics;
15+
using Microsoft.CodeAnalysis.ErrorReporting;
1516
using Microsoft.CodeAnalysis.Host;
1617
using Microsoft.CodeAnalysis.Host.Mef;
1718
using Microsoft.CodeAnalysis.PooledObjects;
@@ -255,30 +256,44 @@ private async Task<CopilotCodeFixAnalysis> ComputeCodeFixAnalysisAsync(
255256
var diagnosticIdToProviderName = new Dictionary<string, HashSet<string>>();
256257
var providerNameToApplicationTime = new Dictionary<string, TimeSpan>();
257258
var providerNameToHasConflict = new Dictionary<string, bool>();
259+
var providerNameToTotalCount = new Dictionary<string, int>();
260+
var providerNameToSuccessCount = new Dictionary<string, int>();
258261

259262
var totalApplicationTimeStopWatch = SharedStopwatch.StartNew();
260-
await ProducerConsumer<(CodeFixCollection collection, TimeSpan elapsedTime)>.RunParallelAsync(
263+
await ProducerConsumer<(CodeFixCollection collection, bool success, TimeSpan elapsedTime)>.RunParallelAsync(
261264
codeFixCollections,
262265
produceItems: static async (codeFixCollection, callback, args, cancellationToken) =>
263266
{
264-
var (@this, solution, _, _, _, _, _) = args;
267+
var (@this, solution, _, _, _, _, _, _, _) = args;
265268
var firstAction = GetFirstAction(codeFixCollection.Fixes[0]);
266269

267270
var applicationTimeStopWatch = SharedStopwatch.StartNew();
268-
var result = await firstAction.GetPreviewOperationsAsync(solution, cancellationToken).ConfigureAwait(false);
269-
callback((codeFixCollection, applicationTimeStopWatch.Elapsed));
271+
var success = true;
272+
try
273+
{
274+
await firstAction
275+
.GetPreviewOperationsAsync(solution, cancellationToken)
276+
.ConfigureAwait(false);
277+
}
278+
catch (Exception e) when (FatalError.ReportAndCatchUnlessCanceled(e, cancellationToken))
279+
{
280+
success = false;
281+
}
282+
283+
callback((codeFixCollection, success, applicationTimeStopWatch.Elapsed));
270284
},
271285
consumeItems: static async (values, args, cancellationToken) =>
272286
{
273-
var (@this, solution, diagnosticIdToCount, diagnosticIdToApplicationTime, diagnosticIdToProviderName, providerNameToApplicationTime, providerNameToHasConflict) = args;
287+
var (@this, solution, diagnosticIdToCount, diagnosticIdToApplicationTime, diagnosticIdToProviderName,
288+
providerNameToApplicationTime, providerNameToHasConflict, providerNameToTotalCount, providerNameToSuccessCount) = args;
274289

275290
// Track which text span each code fix says it will be fixing. We can use this to efficiently determine
276291
// which codefixes 'conflict' with some other codefix (in that that multiple features think they can fix
277292
// the same span of code). We would need some mechanism to determine which we would prefer to take in
278293
// order to have a good experience in such a case.
279294
var intervalTree = new SimpleMutableIntervalTree<CodeFixCollection, CodeFixCollectionIntervalIntrospector>(new CodeFixCollectionIntervalIntrospector());
280295

281-
await foreach (var (codeFixCollection, applicationTime) in values)
296+
await foreach (var (codeFixCollection, success, applicationTime) in values)
282297
{
283298
var diagnosticId = codeFixCollection.FirstDiagnostic.Id;
284299
var providerName = GetProviderName(codeFixCollection);
@@ -287,6 +302,10 @@ private async Task<CopilotCodeFixAnalysis> ComputeCodeFixAnalysisAsync(
287302
IncrementElapsedTime(diagnosticIdToApplicationTime, diagnosticId, applicationTime);
288303
diagnosticIdToProviderName.MultiAdd(diagnosticId, providerName);
289304
IncrementElapsedTime(providerNameToApplicationTime, providerName, applicationTime);
305+
IncrementCount(providerNameToTotalCount, providerName);
306+
307+
if (success)
308+
IncrementCount(providerNameToSuccessCount, providerName);
290309

291310
intervalTree.AddIntervalInPlace(codeFixCollection);
292311
}
@@ -311,7 +330,8 @@ private async Task<CopilotCodeFixAnalysis> ComputeCodeFixAnalysisAsync(
311330
providerNameToHasConflict[providerName] = storedHasConflictValue || newHasConflictValue;
312331
}
313332
},
314-
args: (@this: this, newDocument.Project.Solution, diagnosticIdToCount, diagnosticIdToApplicationTime, diagnosticIdToProviderName, providerNameToApplicationTime, providerNameToHasConflict),
333+
args: (@this: this, newDocument.Project.Solution, diagnosticIdToCount, diagnosticIdToApplicationTime, diagnosticIdToProviderName,
334+
providerNameToApplicationTime, providerNameToHasConflict, providerNameToTotalCount, providerNameToSuccessCount),
315335
cancellationToken).ConfigureAwait(false);
316336
var totalApplicationTime = totalApplicationTimeStopWatch.Elapsed;
317337

@@ -322,7 +342,9 @@ private async Task<CopilotCodeFixAnalysis> ComputeCodeFixAnalysisAsync(
322342
diagnosticIdToApplicationTime,
323343
diagnosticIdToProviderName,
324344
providerNameToApplicationTime,
325-
providerNameToHasConflict);
345+
providerNameToHasConflict,
346+
providerNameToTotalCount,
347+
providerNameToSuccessCount);
326348

327349
Task<ImmutableArray<CodeFixCollection>> ComputeCodeFixCollectionsAsync()
328350
{

0 commit comments

Comments
 (0)