-
Notifications
You must be signed in to change notification settings - Fork 4.2k
Tweaks to existing "copilot completions" processing code to get it ready for auto-fixing copilot suggestions. #79061
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.
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
Changes from all commits
c1bc775
4fa5aa2
f8b927e
d3bc8a8
29335ba
94680d8
4529f2c
e0d0e7b
1eb7768
6471bb8
1cc2127
daee2c6
cf24809
6ab76b2
6987ce6
df2bd2b
1f25cd3
724f431
242d586
0906d40
c06d131
dc80262
fdbdba5
483bc78
f05e09b
f89f9c2
39dd692
95ec949
092b807
6485f02
b5bc4cc
fd9fce5
006f684
644412d
ea3e7c6
8ed74b8
ed224ca
cd20aab
fefcc9e
3f08874
ffecfcb
e3ca928
6c24a94
8a96a21
ce27d9e
c70648c
289fc9c
9d2c913
9818611
d5f9490
5daa687
2822fc9
f47f132
385ed4a
8b212c6
6680585
10402e7
9e7d073
24fcf8b
2515934
8e88735
4d5d88e
6b1f61b
bbbb52d
f8f6082
3339942
2e60831
ca44ecf
dad809b
1dd148e
f50d4dc
f9b0723
4163d96
507e54d
ebf1b1b
6e7683f
8eeaa75
389de64
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,62 @@ | ||
| // Licensed to the .NET Foundation under one or more agreements. | ||
| // The .NET Foundation licenses this file to you under the MIT license. | ||
| // See the LICENSE file in the project root for more information. | ||
|
|
||
| using System.Collections.Generic; | ||
| using System.Collections.Immutable; | ||
| using Microsoft.CodeAnalysis.Editor.Shared.Extensions; | ||
| using Microsoft.CodeAnalysis.PooledObjects; | ||
| using Microsoft.CodeAnalysis.Text; | ||
| using Microsoft.VisualStudio.Language.Proposals; | ||
| using Microsoft.VisualStudio.Text; | ||
|
|
||
| namespace Microsoft.CodeAnalysis.Copilot; | ||
|
|
||
| internal static class CopilotEditorUtilities | ||
| { | ||
| /// <summary> | ||
| /// Returns the single roslyn solution snapshot that is affected by the edits in the proposal. | ||
| /// </summary> | ||
| /// <remarks> | ||
| /// Will fail in the event that the edits touch multiple workspaces, or if the edits touch | ||
| /// multiple versions of a solution in a workspace (for example, edits to different versions of | ||
| /// the same <see cref="ITextSnapshot"/>. Will also fail if this edits non-roslyn files, or files | ||
| /// we cannot process semantics for (like non C#/VB files). | ||
| /// </remarks> | ||
| public static Solution? TryGetAffectedSolution(ProposalBase proposal) | ||
| { | ||
| Solution? solution = null; | ||
| foreach (var edit in proposal.Edits) | ||
| { | ||
| var document = edit.Span.Snapshot.GetOpenDocumentInCurrentContextWithChanges(); | ||
|
|
||
| // Edit touches a file roslyn doesn't know about. Don't touch this. | ||
| if (document is null) | ||
| return null; | ||
|
|
||
| // Only bother for languages we can actually process semantics for. | ||
| if (document.SupportsSemanticModel) | ||
| return null; | ||
|
|
||
| var currentSolution = document.Project.Solution; | ||
|
|
||
| // Edit touches multiple solutions. Don't bother with this for now for simplicity's sake. | ||
| if (solution != null && solution != currentSolution) | ||
| return null; | ||
|
|
||
| solution = currentSolution; | ||
| } | ||
|
|
||
| return solution; | ||
| } | ||
|
|
||
| /// <inheritdoc cref="CopilotUtilities.TryNormalizeCopilotTextChanges"/> | ||
| public static ImmutableArray<TextChange> TryGetNormalizedTextChanges(IEnumerable<ProposedEdit> edits) | ||
| { | ||
| using var _ = ArrayBuilder<TextChange>.GetInstance(out var textChanges); | ||
| foreach (var edit in edits) | ||
| textChanges.Add(new TextChange(edit.Span.Span.ToTextSpan(), edit.ReplacementText)); | ||
|
|
||
| return CopilotUtilities.TryNormalizeCopilotTextChanges(textChanges); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,6 +15,7 @@ | |
| using Microsoft.CodeAnalysis.Editor.Shared.Utilities; | ||
| using Microsoft.CodeAnalysis.Host.Mef; | ||
| using Microsoft.CodeAnalysis.Options; | ||
| using Microsoft.CodeAnalysis.Remote; | ||
| using Microsoft.CodeAnalysis.Shared.TestHooks; | ||
| using Microsoft.CodeAnalysis.Text; | ||
| using Microsoft.CodeAnalysis.Threading; | ||
|
|
@@ -106,6 +107,14 @@ private static async ValueTask ProcessCompletionEventAsync( | |
| const string featureId = "Completion"; | ||
| var proposalId = proposal.ProposalId; | ||
|
|
||
| var solution = CopilotEditorUtilities.TryGetAffectedSolution(proposal); | ||
| if (solution is null) | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. standard helper to make sure this is even a proposal we can consider fixing. will be used by new feature as well. |
||
| return; | ||
|
|
||
| // We're about to potentially make multiple calls to oop here. So keep a session alive to avoid | ||
| // resyncing any data unnecessary. | ||
| using var _1 = await RemoteKeepAliveSession.CreateAsync(solution, cancellationToken).ConfigureAwait(false); | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. somethign i noticed while doing work here. can help perf if copilot is suggesting changes to multiple documents. |
||
|
|
||
| foreach (var editGroup in proposal.Edits.GroupBy(e => e.Span.Snapshot)) | ||
| { | ||
| cancellationToken.ThrowIfCancellationRequested(); | ||
|
|
@@ -116,12 +125,10 @@ private static async ValueTask ProcessCompletionEventAsync( | |
| if (document is null) | ||
| continue; | ||
|
|
||
| using var _ = PooledObjects.ArrayBuilder<TextChange>.GetInstance(out var textChanges); | ||
| foreach (var edit in editGroup) | ||
| textChanges.Add(new TextChange(edit.Span.Span.ToTextSpan(), edit.ReplacementText)); | ||
| var normalizedEdits = CopilotEditorUtilities.TryGetNormalizedTextChanges(editGroup); | ||
|
|
||
| await CopilotChangeAnalysisUtilities.AnalyzeCopilotChangeAsync( | ||
| document, accepted, featureId, proposalId, textChanges, cancellationToken).ConfigureAwait(false); | ||
| document, accepted, featureId, proposalId, normalizedEdits, cancellationToken).ConfigureAwait(false); | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -72,7 +72,8 @@ public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context) | |
| searchOptions = searchOptions with { SearchNuGetPackages = false }; | ||
| } | ||
|
|
||
| var addImportOptions = await document.GetAddImportOptionsAsync(searchOptions, cancellationToken).ConfigureAwait(false); | ||
| var addImportOptions = await document.GetAddImportOptionsAsync( | ||
| searchOptions, cleanupDocument: true, cancellationToken).ConfigureAwait(false); | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. added a flag as copilot-fixup wants to add imports without that making other undesired changes to other parts of the document. (specifically, case correcting things in VB). |
||
|
|
||
| var fixesForDiagnostic = await addImportService.GetFixesForDiagnosticsAsync( | ||
| document, span, diagnostics, MaxResults, symbolSearchService, addImportOptions, packageSources, cancellationToken).ConfigureAwait(false); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ensures the edit is touching only files we can properly analyze within a single roslyn solution snapshot.