From 389c000c16c6e62a3aa68a545a41ce5c395796f2 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Wed, 8 May 2024 00:04:17 -0700 Subject: [PATCH 01/32] Run phases of code cleanup in parallel --- .../Core/Portable/CodeActions/CodeAction.cs | 85 +++++++++++++++++-- .../DocumentBasedFixAllProviderHelpers.cs | 61 ++++++------- 2 files changed, 103 insertions(+), 43 deletions(-) diff --git a/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs b/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs index cf70282edf2c5..aa5b808f01b0e 100644 --- a/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs +++ b/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs @@ -20,8 +20,10 @@ using Microsoft.CodeAnalysis.Host.Mef; using Microsoft.CodeAnalysis.Internal.Log; using Microsoft.CodeAnalysis.Options; +using Microsoft.CodeAnalysis.Remote; using Microsoft.CodeAnalysis.Shared.Collections; using Microsoft.CodeAnalysis.Shared.Extensions; +using Microsoft.CodeAnalysis.Shared.Utilities; using Microsoft.CodeAnalysis.Simplification; using Microsoft.CodeAnalysis.Tags; using Microsoft.CodeAnalysis.Telemetry; @@ -542,15 +544,84 @@ internal static async Task CleanupSyntaxAsync(Document document, CodeC internal static async Task CleanupSemanticsAsync(Document document, CodeCleanupOptions options, CancellationToken cancellationToken) { - var document1 = await ImportAdder.AddImportsFromSymbolAnnotationAsync(document, Simplifier.AddImportsAnnotation, options.AddImportOptions, cancellationToken).ConfigureAwait(false); - var document2 = await Simplifier.ReduceAsync(document1, Simplifier.Annotation, options.SimplifierOptions, cancellationToken).ConfigureAwait(false); - var document3 = await CaseCorrector.CaseCorrectAsync(document2, CaseCorrector.Annotation, cancellationToken).ConfigureAwait(false); + var newSolution = await CleanupSemanticsAsync( + document.Project.Solution, [(document.Id, options)], CodeAnalysisProgress.None, cancellationToken).ConfigureAwait(false); + return newSolution.GetRequiredDocument(document.Id); + } + + internal static async Task CleanupSemanticsAsync( + Solution solution, + ImmutableArray<(DocumentId documentId, CodeCleanupOptions options)> documentIdsAndOptions, + IProgress progress, + CancellationToken cancellationToken) + { + // We do this in 4 serialized passes. This allows us to process all documents in parallel, while only forking + // the solution 4 times *total* (instead of 4 times *per* document). + progress.AddItems(documentIdsAndOptions.Length * 4); + + // First, add all missing imports to all changed documents. + var solution1 = await RunParallelCleanupPassAsync( + solution, static (document, options, cancellationToken) => + ImportAdder.AddImportsFromSymbolAnnotationAsync(document, Simplifier.AddImportsAnnotation, options.AddImportOptions, cancellationToken)).ConfigureAwait(false); + + // Then simplify any expanded constructs. + var solution2 = await RunParallelCleanupPassAsync( + solution1, static (document, options, cancellationToken) => + Simplifier.ReduceAsync(document, Simplifier.Annotation, options.SimplifierOptions, cancellationToken)).ConfigureAwait(false); + + // Do any necessary case correction for VB files. + var solution3 = await RunParallelCleanupPassAsync( + solution2, static (document, options, cancellationToken) => + CaseCorrector.CaseCorrectAsync(document, CaseCorrector.Annotation, cancellationToken)).ConfigureAwait(false); + + // Finally, after doing the semantic cleanup, do another syntax cleanup pass to ensure that the tree is in a + // good state. The semantic cleanup passes may have introduced new nodes with elastic trivia that have to be + // cleaned. + var solution4 = await RunParallelCleanupPassAsync( + solution3, static (document, options, cancellationToken) => + CleanupSyntaxAsync(document, options, cancellationToken)).ConfigureAwait(false); + + return solution4; + + async Task RunParallelCleanupPassAsync( + Solution solution, Func> cleanupDocumentAsync) + { + // We're about to making a ton of calls to this new solution, including expensive oop calls to get up to + // date compilations, skeletons and SG docs. Create and pin this solution so that all remote calls operate + // on the same fork and do not cause the forked solution to be created and dropped repeatedly. + using var _ = await RemoteKeepAliveSession.CreateAsync(solution, cancellationToken).ConfigureAwait(false); + + return await ProducerConsumer<(DocumentId documentId, SyntaxNode newRoot)>.RunParallelAsync( + source: documentIdsAndOptions, + produceItems: static async (documentIdAndOptions, callback, args, cancellationToken) => + { + // As we finish each document, update our progress. + using var _ = args.progress.ItemCompletedScope(); + + var (documentId, options) = documentIdAndOptions; + + // Fetch the current state of the document from this fork of the solution. + var document = args.solution.GetRequiredDocument(documentId); - // After doing the semantic cleanup, do another syntax cleanup pass to ensure that the tree is in a good state. - // The semantic cleanup passes may have introduced new nodes with elastic trivia that have to be cleaned. - var document4 = await CleanupSyntaxAsync(document3, options, cancellationToken).ConfigureAwait(false); + // Now, perform the requested cleanup pass on it. + var cleanedDocument = await args.cleanupDocumentAsync(document, options, cancellationToken).ConfigureAwait(false); - return document4; + // Now get the cleaned root and pass it back to the consumer. + var newRoot = await cleanedDocument.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false); + callback((documentId, newRoot)); + }, + consumeItems: static async (stream, args, cancellationToken) => + { + // Grab all the cleaned roots and produce the new solution snapshot from that. + var currentSolution = args.solution; + await foreach (var (documentId, newRoot) in stream) + currentSolution = currentSolution.WithDocumentSyntaxRoot(documentId, newRoot); + + return currentSolution; + }, + args: (solution, progress, cleanupDocumentAsync), + cancellationToken).ConfigureAwait(false); + } } #region Factories for standard code actions diff --git a/src/Workspaces/Core/Portable/CodeFixesAndRefactorings/DocumentBasedFixAllProviderHelpers.cs b/src/Workspaces/Core/Portable/CodeFixesAndRefactorings/DocumentBasedFixAllProviderHelpers.cs index b2169de3a1599..d7aa7ead24f4e 100644 --- a/src/Workspaces/Core/Portable/CodeFixesAndRefactorings/DocumentBasedFixAllProviderHelpers.cs +++ b/src/Workspaces/Core/Portable/CodeFixesAndRefactorings/DocumentBasedFixAllProviderHelpers.cs @@ -110,45 +110,34 @@ async Task CleanSolutionAsync(Solution dirtySolution, ImmutableArray.RunParallelAsync( - source: changedRootDocumentIds, - produceItems: static async (documentId, callback, args, cancellationToken) => - { - using var _ = args.progressTracker.ItemCompletedScope(); + progressTracker.Report(CodeAnalysisProgress.Description(WorkspacesResources.Running_code_cleanup_on_fixed_documents)); - var dirtyDocument = args.dirtySolution.GetRequiredDocument(documentId); - var codeCleanupOptions = await dirtyDocument.GetCodeCleanupOptionsAsync( - args.originalFixAllContext.State.CodeActionOptionsProvider, cancellationToken).ConfigureAwait(false); + // Next, go and semantically cleanup any trees we inserted. Do this in parallel across all the documents + // that were fixed and resulted in a new tree (as opposed to new text). + var documentIdsAndOptions = new FixedSizeArrayBuilder<(DocumentId documentId, CodeCleanupOptions options)>(changedRootDocumentIds.Length); + foreach (var documentId in changedRootDocumentIds) + { + var document = dirtySolution.GetRequiredDocument(documentId); + var codeActionOptions = await document.GetCodeCleanupOptionsAsync( + originalFixAllContext.State.CodeActionOptionsProvider, cancellationToken).ConfigureAwait(false); + documentIdsAndOptions.Add((documentId, codeActionOptions)); + } - // Only have to cleanup semantics here. Syntax was already done in CleanDocumentSyntaxAsync. - var cleanedDocument = await CodeAction.CleanupSemanticsAsync(dirtyDocument, codeCleanupOptions, cancellationToken).ConfigureAwait(false); - var cleanedText = await cleanedDocument.GetValueTextAsync(cancellationToken).ConfigureAwait(false); - callback((dirtyDocument.Id, cleanedText)); - }, - consumeItems: static async (results, args, cancellationToken) => - { - // Finally, apply the cleaned documents to the solution. - var finalSolution = args.dirtySolution; - await foreach (var (docId, cleanedText) in results) - finalSolution = finalSolution.WithDocumentText(docId, cleanedText); + var solutionWithCleanedRoots = await CodeAction.CleanupSemanticsAsync( + dirtySolution, documentIdsAndOptions.MoveToImmutable(), progressTracker, cancellationToken).ConfigureAwait(false); - return finalSolution; - }, - args: (originalFixAllContext, dirtySolution, progressTracker), - cancellationToken).ConfigureAwait(false); + // Once we clean the document, we get the text of it and insert that back into the final solution. This way + // we can release both the original fixed tree, and the cleaned tree (both of which can be much more + // expensive than just text). + var finalSolution = dirtySolution; + foreach (var changedRootDocumentId in changedRootDocumentIds) + { + var cleanedDocument = solutionWithCleanedRoots.GetRequiredDocument(changedRootDocumentId); + var cleanedText = await cleanedDocument.GetValueTextAsync(cancellationToken).ConfigureAwait(false); + finalSolution = finalSolution.WithDocumentText(changedRootDocumentId, cleanedText); + } + + return finalSolution; } static async ValueTask<(DocumentId documentId, (SyntaxNode? node, SourceText? text))?> CleanDocumentSyntaxAsync( From a5ee41498c02f54e16d53c5d6572f6858233eb38 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Wed, 8 May 2024 11:05:25 -0700 Subject: [PATCH 02/32] Use helper --- ...e.IntroduceVariableAllOccurrenceCodeAction.cs | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-) diff --git a/src/Features/Core/Portable/IntroduceVariable/AbstractIntroduceVariableService.IntroduceVariableAllOccurrenceCodeAction.cs b/src/Features/Core/Portable/IntroduceVariable/AbstractIntroduceVariableService.IntroduceVariableAllOccurrenceCodeAction.cs index 7800caa54892a..1e02f4aa8f5f6 100644 --- a/src/Features/Core/Portable/IntroduceVariable/AbstractIntroduceVariableService.IntroduceVariableAllOccurrenceCodeAction.cs +++ b/src/Features/Core/Portable/IntroduceVariable/AbstractIntroduceVariableService.IntroduceVariableAllOccurrenceCodeAction.cs @@ -2,14 +2,9 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. -#nullable disable - using System.Threading; using System.Threading.Tasks; -using Microsoft.CodeAnalysis.CaseCorrection; using Microsoft.CodeAnalysis.CodeCleanup; -using Microsoft.CodeAnalysis.Formatting; -using Microsoft.CodeAnalysis.Simplification; namespace Microsoft.CodeAnalysis.IntroduceVariable; @@ -17,7 +12,7 @@ internal partial class AbstractIntroduceVariableService PostProcessChangesAsync(Document document, CancellationToken cancellationToken) - { - document = await Simplifier.ReduceAsync(document, Simplifier.Annotation, Options.SimplifierOptions, cancellationToken).ConfigureAwait(false); - document = await Formatter.FormatAsync(document, Formatter.Annotation, Options.FormattingOptions, cancellationToken).ConfigureAwait(false); - document = await CaseCorrector.CaseCorrectAsync(document, CaseCorrector.Annotation, cancellationToken).ConfigureAwait(false); - return document; - } + protected override Task PostProcessChangesAsync(Document document, CancellationToken cancellationToken) + => CleanupSemanticsAsync(document, this.Options, cancellationToken); } } From 688fa6f5f368736476d7a7ef9cdf9c2b4b1ba815 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Wed, 8 May 2024 11:28:28 -0700 Subject: [PATCH 03/32] More parallel --- .../Core/Portable/CodeActions/CodeAction.cs | 90 +++++++++++-------- 1 file changed, 53 insertions(+), 37 deletions(-) diff --git a/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs b/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs index aa5b808f01b0e..17154e00bb9ed 100644 --- a/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs +++ b/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs @@ -20,6 +20,7 @@ using Microsoft.CodeAnalysis.Host.Mef; using Microsoft.CodeAnalysis.Internal.Log; using Microsoft.CodeAnalysis.Options; +using Microsoft.CodeAnalysis.PooledObjects; using Microsoft.CodeAnalysis.Remote; using Microsoft.CodeAnalysis.Shared.Collections; using Microsoft.CodeAnalysis.Shared.Extensions; @@ -27,6 +28,7 @@ using Microsoft.CodeAnalysis.Simplification; using Microsoft.CodeAnalysis.Tags; using Microsoft.CodeAnalysis.Telemetry; +using Microsoft.CodeAnalysis.Text; using Roslyn.Utilities; namespace Microsoft.CodeAnalysis.CodeActions; @@ -250,7 +252,7 @@ private protected virtual async Task> GetOpe if (operations != null) { - return await this.PostProcessAsync(originalSolution, operations, cancellationToken).ConfigureAwait(false); + return await PostProcessAsync(originalSolution, operations, cancellationToken).ConfigureAwait(false); } return []; @@ -271,7 +273,7 @@ internal async Task> GetPreviewOperationsAsy if (operations != null) { - return await this.PostProcessAsync(originalSolution, operations, cancellationToken).ConfigureAwait(false); + return await PostProcessAsync(originalSolution, operations, cancellationToken).ConfigureAwait(false); } return []; @@ -410,7 +412,7 @@ protected virtual Task GetChangedDocumentAsync(IProgress GetChangedDocumentInternalAsync(CancellationToken cancellation) @@ -422,10 +424,12 @@ internal Task GetChangedDocumentInternalAsync(CancellationToken cancel /// A list of operations. /// A cancellation token. /// A new list of operations with post processing steps applied to any 's. +#pragma warning disable CA1822 // Mark members as static. This is a public API. protected Task> PostProcessAsync(IEnumerable operations, CancellationToken cancellationToken) +#pragma warning restore CA1822 // Mark members as static => PostProcessAsync(originalSolution: null!, operations, cancellationToken); - internal async Task> PostProcessAsync( + internal static async Task> PostProcessAsync( Solution originalSolution, IEnumerable operations, CancellationToken cancellationToken) { using var result = TemporaryArray.Empty; @@ -434,7 +438,7 @@ internal async Task> PostProcessAsync( { if (op is ApplyChangesOperation ac) { - result.Add(new ApplyChangesOperation(await this.PostProcessChangesAsync(originalSolution, ac.ChangedSolution, cancellationToken).ConfigureAwait(false))); + result.Add(new ApplyChangesOperation(await PostProcessChangesAsync(originalSolution, ac.ChangedSolution, cancellationToken).ConfigureAwait(false))); } else { @@ -450,10 +454,12 @@ internal async Task> PostProcessAsync( /// /// The solution changed by the . /// A cancellation token +#pragma warning disable CA1822 // Mark members as static. This is a public API. protected Task PostProcessChangesAsync(Solution changedSolution, CancellationToken cancellationToken) - => PostProcessChangesAsync(originalSolution: null!, changedSolution, cancellationToken); +#pragma warning restore CA1822 // Mark members as static + => PostProcessChangesAsync(originalSolution: null!, changedSolution, cancellationToken); - internal async Task PostProcessChangesAsync( + internal static async Task PostProcessChangesAsync( Solution originalSolution, Solution changedSolution, CancellationToken cancellationToken) @@ -465,36 +471,40 @@ internal async Task PostProcessChangesAsync( originalSolution ??= changedSolution.Workspace.CurrentSolution; var solutionChanges = changedSolution.GetChanges(originalSolution); - var processedSolution = changedSolution; + var documentIdsAndOptions = await solutionChanges + .GetProjectChanges() + .SelectMany(p => p.GetChangedDocuments(onlyGetDocumentsWithTextChanges: true).Concat(p.GetAddedDocuments())) + .Concat(solutionChanges.GetAddedProjects().SelectMany(p => p.DocumentIds)) + .SelectAsArrayAsync(async documentId => (documentId, options: await GetCodeCleanupOptionAsync(changedSolution.GetRequiredDocument(documentId), cancellationToken).ConfigureAwait(false))).ConfigureAwait(false); - // process changed projects - foreach (var projectChanges in solutionChanges.GetProjectChanges()) - { - var documentsToProcess = projectChanges.GetChangedDocuments(onlyGetDocumentsWithTextChanges: true).Concat( - projectChanges.GetAddedDocuments()); - - foreach (var documentId in documentsToProcess) + // Do an initial pass where we cleanup syntax. + var syntaxCleanedSolution = await ProducerConsumer.RunParallelAsync( + source: documentIdsAndOptions, + produceItems: static async (documentIdAndOptions, callback, changedSolution, cancellationToken) => { - var document = processedSolution.GetRequiredDocument(documentId); - var processedDocument = await PostProcessChangesAsync(document, cancellationToken).ConfigureAwait(false); - processedSolution = processedDocument.Project.Solution; - } - } + var document = changedSolution.GetRequiredDocument(documentIdAndOptions.documentId); + var newDoc = await CleanupSyntaxAsync(document, documentIdAndOptions.options, cancellationToken).ConfigureAwait(false); + }, + consumeItems: static async (stream, changedSolution, cancellationToken) => + { + var currentSolution = changedSolution; + await foreach (var document in stream) + { + currentSolution = document.SupportsSyntaxTree + ? currentSolution.WithDocumentSyntaxRoot(document.Id, await document.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false)) + : currentSolution.WithDocumentText(document.Id, await document.GetValueTextAsync(cancellationToken).ConfigureAwait(false)); + } - // process completely new projects too - foreach (var addedProject in solutionChanges.GetAddedProjects()) - { - var documentsToProcess = addedProject.DocumentIds; + return currentSolution; + }, + args: changedSolution, + cancellationToken).ConfigureAwait(false); - foreach (var documentId in documentsToProcess) - { - var document = processedSolution.GetRequiredDocument(documentId); - var processedDocument = await PostProcessChangesAsync(document, cancellationToken).ConfigureAwait(false); - processedSolution = processedDocument.Project.Solution; - } - } + // Then do a pass where we cleanup semantics. + var semanticCleanedSolution = await CleanupSemanticsAsync( + syntaxCleanedSolution, documentIdsAndOptions, CodeAnalysisProgress.None, cancellationToken).ConfigureAwait(false); - return processedSolution; + return semanticCleanedSolution; } /// @@ -509,17 +519,23 @@ protected virtual async Task PostProcessChangesAsync(Document document { if (document.SupportsSyntaxTree) { - // TODO: avoid ILegacyGlobalCodeActionOptionsWorkspaceService https://github.com/dotnet/roslyn/issues/60777 - var globalOptions = document.Project.Solution.Services.GetService(); - var fallbackOptions = globalOptions?.Provider ?? CodeActionOptions.DefaultProvider; - - var options = await document.GetCodeCleanupOptionsAsync(fallbackOptions, cancellationToken).ConfigureAwait(false); + var options = await GetCodeCleanupOptionAsync(document, cancellationToken).ConfigureAwait(false); return await CleanupDocumentAsync(document, options, cancellationToken).ConfigureAwait(false); } return document; } + private static async Task GetCodeCleanupOptionAsync(Document document, CancellationToken cancellationToken) + { + // TODO: avoid ILegacyGlobalCodeActionOptionsWorkspaceService https://github.com/dotnet/roslyn/issues/60777 + var globalOptions = document.Project.Solution.Services.GetService(); + var fallbackOptions = globalOptions?.Provider ?? CodeActionOptions.DefaultProvider; + + var options = await document.GetCodeCleanupOptionsAsync(fallbackOptions, cancellationToken).ConfigureAwait(false); + return options; + } + internal static async Task CleanupDocumentAsync(Document document, CodeCleanupOptions options, CancellationToken cancellationToken) { // First, do a syntax pass. Ensuring that things are formatted correctly based on the original nodes and From e42a945a56aff9c94639e7c22862c6825e383e00 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Wed, 8 May 2024 11:33:05 -0700 Subject: [PATCH 04/32] Add helper --- .../Core/Portable/CodeActions/CodeAction.cs | 26 ++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs b/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs index 17154e00bb9ed..9e8fce40ea63a 100644 --- a/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs +++ b/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs @@ -9,6 +9,7 @@ using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.Linq; +using System.Reflection.Metadata; using System.Threading; using System.Threading.Tasks; using Microsoft.CodeAnalysis.CaseCorrection; @@ -459,6 +460,20 @@ protected Task PostProcessChangesAsync(Solution changedSolution, Cance #pragma warning restore CA1822 // Mark members as static => PostProcessChangesAsync(originalSolution: null!, changedSolution, cancellationToken); + internal static async Task> GetDocumentIdsAndOptionsAsync( + Solution solution, CodeCleanupOptionsProvider optionsProvider, ImmutableArray documentIds, CancellationToken cancellationToken) + { + var documentIdsAndOptions = new FixedSizeArrayBuilder<(DocumentId documentId, CodeCleanupOptions options)>(documentIds.Length); + foreach (var documentId in documentIds) + { + var document = solution.GetRequiredDocument(documentId); + var codeActionOptions = await document.GetCodeCleanupOptionsAsync(optionsProvider, cancellationToken).ConfigureAwait(false); + documentIdsAndOptions.Add((documentId, codeActionOptions)); + } + + return documentIdsAndOptions.MoveToImmutable(); + } + internal static async Task PostProcessChangesAsync( Solution originalSolution, Solution changedSolution, @@ -471,11 +486,16 @@ internal static async Task PostProcessChangesAsync( originalSolution ??= changedSolution.Workspace.CurrentSolution; var solutionChanges = changedSolution.GetChanges(originalSolution); - var documentIdsAndOptions = await solutionChanges + var documentIds = solutionChanges .GetProjectChanges() .SelectMany(p => p.GetChangedDocuments(onlyGetDocumentsWithTextChanges: true).Concat(p.GetAddedDocuments())) .Concat(solutionChanges.GetAddedProjects().SelectMany(p => p.DocumentIds)) - .SelectAsArrayAsync(async documentId => (documentId, options: await GetCodeCleanupOptionAsync(changedSolution.GetRequiredDocument(documentId), cancellationToken).ConfigureAwait(false))).ConfigureAwait(false); + .ToImmutableArray(); + + var globalOptions = changedSolution.Services.GetService(); + var fallbackOptions = globalOptions?.Provider ?? CodeActionOptions.DefaultProvider; + var documentIdsAndOptions = await GetDocumentIdsAndOptionsAsync( + changedSolution, fallbackOptions, documentIds, cancellationToken).ConfigureAwait(false); // Do an initial pass where we cleanup syntax. var syntaxCleanedSolution = await ProducerConsumer.RunParallelAsync( @@ -483,7 +503,7 @@ internal static async Task PostProcessChangesAsync( produceItems: static async (documentIdAndOptions, callback, changedSolution, cancellationToken) => { var document = changedSolution.GetRequiredDocument(documentIdAndOptions.documentId); - var newDoc = await CleanupSyntaxAsync(document, documentIdAndOptions.options, cancellationToken).ConfigureAwait(false); + var newDoc = await CleanupSyntaxAsync(document, documentIdAndOptions.codeCleanupOptions, cancellationToken).ConfigureAwait(false); }, consumeItems: static async (stream, changedSolution, cancellationToken) => { From 57e9943b81095fe521c8b0cf0b3fc5cfc37e3849 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Wed, 8 May 2024 11:34:46 -0700 Subject: [PATCH 05/32] Share code --- .../DocumentBasedFixAllProviderHelpers.cs | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/src/Workspaces/Core/Portable/CodeFixesAndRefactorings/DocumentBasedFixAllProviderHelpers.cs b/src/Workspaces/Core/Portable/CodeFixesAndRefactorings/DocumentBasedFixAllProviderHelpers.cs index d7aa7ead24f4e..849d478acea9b 100644 --- a/src/Workspaces/Core/Portable/CodeFixesAndRefactorings/DocumentBasedFixAllProviderHelpers.cs +++ b/src/Workspaces/Core/Portable/CodeFixesAndRefactorings/DocumentBasedFixAllProviderHelpers.cs @@ -114,17 +114,11 @@ async Task CleanSolutionAsync(Solution dirtySolution, ImmutableArray(changedRootDocumentIds.Length); - foreach (var documentId in changedRootDocumentIds) - { - var document = dirtySolution.GetRequiredDocument(documentId); - var codeActionOptions = await document.GetCodeCleanupOptionsAsync( - originalFixAllContext.State.CodeActionOptionsProvider, cancellationToken).ConfigureAwait(false); - documentIdsAndOptions.Add((documentId, codeActionOptions)); - } + var documentIdsAndOptions = await CodeAction.GetDocumentIdsAndOptionsAsync( + dirtySolution, originalFixAllContext.State.CodeActionOptionsProvider, changedRootDocumentIds, cancellationToken).ConfigureAwait(false); var solutionWithCleanedRoots = await CodeAction.CleanupSemanticsAsync( - dirtySolution, documentIdsAndOptions.MoveToImmutable(), progressTracker, cancellationToken).ConfigureAwait(false); + dirtySolution, documentIdsAndOptions, progressTracker, cancellationToken).ConfigureAwait(false); // Once we clean the document, we get the text of it and insert that back into the final solution. This way // we can release both the original fixed tree, and the cleaned tree (both of which can be much more From b77e7d91c693380c3238bfc93b83a9a714a6cddf Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Wed, 8 May 2024 11:39:23 -0700 Subject: [PATCH 06/32] Fixup feature tests --- .../CSharpTest/IntroduceVariable/IntroduceVariableTests.cs | 2 +- .../Core/Portable/CodeActions/CodeActionWithOptions.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Features/CSharpTest/IntroduceVariable/IntroduceVariableTests.cs b/src/Features/CSharpTest/IntroduceVariable/IntroduceVariableTests.cs index 82cf8a801c445..dd30dd77f6304 100644 --- a/src/Features/CSharpTest/IntroduceVariable/IntroduceVariableTests.cs +++ b/src/Features/CSharpTest/IntroduceVariable/IntroduceVariableTests.cs @@ -5977,7 +5977,7 @@ void M() { int x = 1; int {|Rename:y1|} = C.y; - var t = new { y= y1, y= y1 }; // this is an error already + var t = new { y = y1, y = y1 }; // this is an error already } } """; diff --git a/src/Workspaces/Core/Portable/CodeActions/CodeActionWithOptions.cs b/src/Workspaces/Core/Portable/CodeActions/CodeActionWithOptions.cs index 5914b7fbd7a7b..0f8c3b5471b5c 100644 --- a/src/Workspaces/Core/Portable/CodeActions/CodeActionWithOptions.cs +++ b/src/Workspaces/Core/Portable/CodeActions/CodeActionWithOptions.cs @@ -44,7 +44,7 @@ public abstract class CodeActionWithOptions : CodeAction if (operations != null) { - operations = await this.PostProcessAsync(originalSolution, operations, cancellationToken).ConfigureAwait(false); + operations = await PostProcessAsync(originalSolution, operations, cancellationToken).ConfigureAwait(false); } return operations; From 7de918556ef283c7a9a4047c03e3454e5d82cb97 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Wed, 8 May 2024 11:42:06 -0700 Subject: [PATCH 07/32] Simplify --- ...oduceVariableService.AbstractIntroduceVariableCodeAction.cs | 2 +- ...VariableService.IntroduceVariableAllOccurrenceCodeAction.cs | 3 --- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/src/Features/Core/Portable/IntroduceVariable/AbstractIntroduceVariableService.AbstractIntroduceVariableCodeAction.cs b/src/Features/Core/Portable/IntroduceVariable/AbstractIntroduceVariableService.AbstractIntroduceVariableCodeAction.cs index 89515c43368be..0ae59492bed8c 100644 --- a/src/Features/Core/Portable/IntroduceVariable/AbstractIntroduceVariableService.AbstractIntroduceVariableCodeAction.cs +++ b/src/Features/Core/Portable/IntroduceVariable/AbstractIntroduceVariableService.AbstractIntroduceVariableCodeAction.cs @@ -16,7 +16,7 @@ namespace Microsoft.CodeAnalysis.IntroduceVariable; internal partial class AbstractIntroduceVariableService { - internal abstract class AbstractIntroduceVariableCodeAction : CodeAction + private abstract class AbstractIntroduceVariableCodeAction : CodeAction { private readonly bool _allOccurrences; private readonly bool _isConstant; diff --git a/src/Features/Core/Portable/IntroduceVariable/AbstractIntroduceVariableService.IntroduceVariableAllOccurrenceCodeAction.cs b/src/Features/Core/Portable/IntroduceVariable/AbstractIntroduceVariableService.IntroduceVariableAllOccurrenceCodeAction.cs index 1e02f4aa8f5f6..5ea06a2325b22 100644 --- a/src/Features/Core/Portable/IntroduceVariable/AbstractIntroduceVariableService.IntroduceVariableAllOccurrenceCodeAction.cs +++ b/src/Features/Core/Portable/IntroduceVariable/AbstractIntroduceVariableService.IntroduceVariableAllOccurrenceCodeAction.cs @@ -24,8 +24,5 @@ public IntroduceVariableAllOccurrenceCodeAction( : base(service, document, options, expression, allOccurrences, isConstant, isLocal, isQueryLocal) { } - - protected override Task PostProcessChangesAsync(Document document, CancellationToken cancellationToken) - => CleanupSemanticsAsync(document, this.Options, cancellationToken); } } From d1437dfd589d240b46c79ce5d94496413a3efd6e Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Wed, 8 May 2024 11:44:28 -0700 Subject: [PATCH 08/32] Fix issue with improper formatting in introduce-variable --- ...ice.AbstractIntroduceVariableCodeAction.cs | 2 +- ...ractIntroduceVariableService.CodeAction.cs | 25 +++++------- ...ntroduceVariableAllOccurrenceCodeAction.cs | 38 +++++-------------- ...ntroduceVariableCodeRefactoringProvider.cs | 13 ++----- 4 files changed, 25 insertions(+), 53 deletions(-) diff --git a/src/Features/Core/Portable/IntroduceVariable/AbstractIntroduceVariableService.AbstractIntroduceVariableCodeAction.cs b/src/Features/Core/Portable/IntroduceVariable/AbstractIntroduceVariableService.AbstractIntroduceVariableCodeAction.cs index 89515c43368be..0ae59492bed8c 100644 --- a/src/Features/Core/Portable/IntroduceVariable/AbstractIntroduceVariableService.AbstractIntroduceVariableCodeAction.cs +++ b/src/Features/Core/Portable/IntroduceVariable/AbstractIntroduceVariableService.AbstractIntroduceVariableCodeAction.cs @@ -16,7 +16,7 @@ namespace Microsoft.CodeAnalysis.IntroduceVariable; internal partial class AbstractIntroduceVariableService { - internal abstract class AbstractIntroduceVariableCodeAction : CodeAction + private abstract class AbstractIntroduceVariableCodeAction : CodeAction { private readonly bool _allOccurrences; private readonly bool _isConstant; diff --git a/src/Features/Core/Portable/IntroduceVariable/AbstractIntroduceVariableService.CodeAction.cs b/src/Features/Core/Portable/IntroduceVariable/AbstractIntroduceVariableService.CodeAction.cs index e7cbeb85533ae..ab8443bec9cff 100644 --- a/src/Features/Core/Portable/IntroduceVariable/AbstractIntroduceVariableService.CodeAction.cs +++ b/src/Features/Core/Portable/IntroduceVariable/AbstractIntroduceVariableService.CodeAction.cs @@ -2,27 +2,22 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. -#nullable disable - using Microsoft.CodeAnalysis.CodeCleanup; namespace Microsoft.CodeAnalysis.IntroduceVariable; internal partial class AbstractIntroduceVariableService { - private class IntroduceVariableCodeAction : AbstractIntroduceVariableCodeAction + private sealed class IntroduceVariableCodeAction( + TService service, + SemanticDocument document, + CodeCleanupOptions options, + TExpressionSyntax expression, + bool allOccurrences, + bool isConstant, + bool isLocal, + bool isQueryLocal) : AbstractIntroduceVariableCodeAction( + service, document, options, expression, allOccurrences, isConstant, isLocal, isQueryLocal) { - internal IntroduceVariableCodeAction( - TService service, - SemanticDocument document, - CodeCleanupOptions options, - TExpressionSyntax expression, - bool allOccurrences, - bool isConstant, - bool isLocal, - bool isQueryLocal) - : base(service, document, options, expression, allOccurrences, isConstant, isLocal, isQueryLocal) - { - } } } diff --git a/src/Features/Core/Portable/IntroduceVariable/AbstractIntroduceVariableService.IntroduceVariableAllOccurrenceCodeAction.cs b/src/Features/Core/Portable/IntroduceVariable/AbstractIntroduceVariableService.IntroduceVariableAllOccurrenceCodeAction.cs index 7800caa54892a..258785ef00557 100644 --- a/src/Features/Core/Portable/IntroduceVariable/AbstractIntroduceVariableService.IntroduceVariableAllOccurrenceCodeAction.cs +++ b/src/Features/Core/Portable/IntroduceVariable/AbstractIntroduceVariableService.IntroduceVariableAllOccurrenceCodeAction.cs @@ -2,40 +2,22 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. -#nullable disable - -using System.Threading; -using System.Threading.Tasks; -using Microsoft.CodeAnalysis.CaseCorrection; using Microsoft.CodeAnalysis.CodeCleanup; -using Microsoft.CodeAnalysis.Formatting; -using Microsoft.CodeAnalysis.Simplification; namespace Microsoft.CodeAnalysis.IntroduceVariable; internal partial class AbstractIntroduceVariableService { - private class IntroduceVariableAllOccurrenceCodeAction : AbstractIntroduceVariableCodeAction + private class IntroduceVariableAllOccurrenceCodeAction( + TService service, + SemanticDocument document, + CodeCleanupOptions options, + TExpressionSyntax expression, + bool allOccurrences, + bool isConstant, + bool isLocal, + bool isQueryLocal) : AbstractIntroduceVariableCodeAction( + service, document, options, expression, allOccurrences, isConstant, isLocal, isQueryLocal) { - internal IntroduceVariableAllOccurrenceCodeAction( - TService service, - SemanticDocument document, - CodeCleanupOptions options, - TExpressionSyntax expression, - bool allOccurrences, - bool isConstant, - bool isLocal, - bool isQueryLocal) - : base(service, document, options, expression, allOccurrences, isConstant, isLocal, isQueryLocal) - { - } - - protected override async Task PostProcessChangesAsync(Document document, CancellationToken cancellationToken) - { - document = await Simplifier.ReduceAsync(document, Simplifier.Annotation, Options.SimplifierOptions, cancellationToken).ConfigureAwait(false); - document = await Formatter.FormatAsync(document, Formatter.Annotation, Options.FormattingOptions, cancellationToken).ConfigureAwait(false); - document = await CaseCorrector.CaseCorrectAsync(document, CaseCorrector.Annotation, cancellationToken).ConfigureAwait(false); - return document; - } } } diff --git a/src/Features/Core/Portable/IntroduceVariable/IntroduceVariableCodeRefactoringProvider.cs b/src/Features/Core/Portable/IntroduceVariable/IntroduceVariableCodeRefactoringProvider.cs index 121a6645a8aec..b6755b077c613 100644 --- a/src/Features/Core/Portable/IntroduceVariable/IntroduceVariableCodeRefactoringProvider.cs +++ b/src/Features/Core/Portable/IntroduceVariable/IntroduceVariableCodeRefactoringProvider.cs @@ -15,16 +15,11 @@ namespace Microsoft.CodeAnalysis.IntroduceVariable; [ExtensionOrder(After = PredefinedCodeRefactoringProviderNames.ConvertAnonymousTypeToClass)] [ExtensionOrder(After = PredefinedCodeRefactoringProviderNames.InvertConditional)] [ExtensionOrder(After = PredefinedCodeRefactoringProviderNames.InvertLogical)] -[ExportCodeRefactoringProvider(LanguageNames.CSharp, LanguageNames.VisualBasic, - Name = PredefinedCodeRefactoringProviderNames.IntroduceVariable), Shared] -internal class IntroduceVariableCodeRefactoringProvider : CodeRefactoringProvider +[ExportCodeRefactoringProvider(LanguageNames.CSharp, LanguageNames.VisualBasic, Name = PredefinedCodeRefactoringProviderNames.IntroduceVariable), Shared] +[method: ImportingConstructor] +[method: SuppressMessage("RoslynDiagnosticsReliability", "RS0033:Importing constructor should be [Obsolete]", Justification = "Used in test code: https://github.com/dotnet/roslyn/issues/42814")] +internal sealed class IntroduceVariableCodeRefactoringProvider() : CodeRefactoringProvider { - [ImportingConstructor] - [SuppressMessage("RoslynDiagnosticsReliability", "RS0033:Importing constructor should be [Obsolete]", Justification = "Used in test code: https://github.com/dotnet/roslyn/issues/42814")] - public IntroduceVariableCodeRefactoringProvider() - { - } - public override async Task ComputeRefactoringsAsync(CodeRefactoringContext context) { var (document, textSpan, cancellationToken) = context; From 6973de76748131338f6eb19f3a132c094ea8e2e3 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Wed, 8 May 2024 11:46:08 -0700 Subject: [PATCH 09/32] Fix test --- .../CSharpTest/IntroduceVariable/IntroduceVariableTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Features/CSharpTest/IntroduceVariable/IntroduceVariableTests.cs b/src/Features/CSharpTest/IntroduceVariable/IntroduceVariableTests.cs index 82cf8a801c445..dd30dd77f6304 100644 --- a/src/Features/CSharpTest/IntroduceVariable/IntroduceVariableTests.cs +++ b/src/Features/CSharpTest/IntroduceVariable/IntroduceVariableTests.cs @@ -5977,7 +5977,7 @@ void M() { int x = 1; int {|Rename:y1|} = C.y; - var t = new { y= y1, y= y1 }; // this is an error already + var t = new { y = y1, y = y1 }; // this is an error already } } """; From 333e6dfdd3252c28789b7b0b71c81ae881ce2006 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Wed, 8 May 2024 11:49:59 -0700 Subject: [PATCH 10/32] Cleanup --- src/Workspaces/Core/Portable/CodeActions/CodeAction.cs | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs b/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs index 9e8fce40ea63a..0880701071373 100644 --- a/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs +++ b/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs @@ -9,7 +9,6 @@ using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.Linq; -using System.Reflection.Metadata; using System.Threading; using System.Threading.Tasks; using Microsoft.CodeAnalysis.CaseCorrection; @@ -21,7 +20,6 @@ using Microsoft.CodeAnalysis.Host.Mef; using Microsoft.CodeAnalysis.Internal.Log; using Microsoft.CodeAnalysis.Options; -using Microsoft.CodeAnalysis.PooledObjects; using Microsoft.CodeAnalysis.Remote; using Microsoft.CodeAnalysis.Shared.Collections; using Microsoft.CodeAnalysis.Shared.Extensions; @@ -29,7 +27,6 @@ using Microsoft.CodeAnalysis.Simplification; using Microsoft.CodeAnalysis.Tags; using Microsoft.CodeAnalysis.Telemetry; -using Microsoft.CodeAnalysis.Text; using Roslyn.Utilities; namespace Microsoft.CodeAnalysis.CodeActions; From 88ed32302e901d0717312b0db6da49edf8282582 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Wed, 8 May 2024 11:52:13 -0700 Subject: [PATCH 11/32] inline method --- .../Core/Portable/CodeActions/CodeAction.cs | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs b/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs index 0880701071373..1c9c51f31adde 100644 --- a/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs +++ b/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs @@ -536,23 +536,17 @@ protected virtual async Task PostProcessChangesAsync(Document document { if (document.SupportsSyntaxTree) { - var options = await GetCodeCleanupOptionAsync(document, cancellationToken).ConfigureAwait(false); + // TODO: avoid ILegacyGlobalCodeActionOptionsWorkspaceService https://github.com/dotnet/roslyn/issues/60777 + var globalOptions = document.Project.Solution.Services.GetService(); + var fallbackOptions = globalOptions?.Provider ?? CodeActionOptions.DefaultProvider; + + var options = await document.GetCodeCleanupOptionsAsync(fallbackOptions, cancellationToken).ConfigureAwait(false); return await CleanupDocumentAsync(document, options, cancellationToken).ConfigureAwait(false); } return document; } - private static async Task GetCodeCleanupOptionAsync(Document document, CancellationToken cancellationToken) - { - // TODO: avoid ILegacyGlobalCodeActionOptionsWorkspaceService https://github.com/dotnet/roslyn/issues/60777 - var globalOptions = document.Project.Solution.Services.GetService(); - var fallbackOptions = globalOptions?.Provider ?? CodeActionOptions.DefaultProvider; - - var options = await document.GetCodeCleanupOptionsAsync(fallbackOptions, cancellationToken).ConfigureAwait(false); - return options; - } - internal static async Task CleanupDocumentAsync(Document document, CodeCleanupOptions options, CancellationToken cancellationToken) { // First, do a syntax pass. Ensuring that things are formatted correctly based on the original nodes and From d10a8c1621b491f4ee53aebd54bb1dc9e42595f3 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Wed, 8 May 2024 11:59:22 -0700 Subject: [PATCH 12/32] Don't root documents --- .../Core/Portable/CodeActions/CodeAction.cs | 20 +++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs b/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs index 1c9c51f31adde..e5eca531da3df 100644 --- a/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs +++ b/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs @@ -27,6 +27,7 @@ using Microsoft.CodeAnalysis.Simplification; using Microsoft.CodeAnalysis.Tags; using Microsoft.CodeAnalysis.Telemetry; +using Microsoft.CodeAnalysis.Text; using Roslyn.Utilities; namespace Microsoft.CodeAnalysis.CodeActions; @@ -495,21 +496,28 @@ internal static async Task PostProcessChangesAsync( changedSolution, fallbackOptions, documentIds, cancellationToken).ConfigureAwait(false); // Do an initial pass where we cleanup syntax. - var syntaxCleanedSolution = await ProducerConsumer.RunParallelAsync( + var syntaxCleanedSolution = await ProducerConsumer<(DocumentId documentId, (SyntaxNode? node, SourceText? text))>.RunParallelAsync( source: documentIdsAndOptions, produceItems: static async (documentIdAndOptions, callback, changedSolution, cancellationToken) => { var document = changedSolution.GetRequiredDocument(documentIdAndOptions.documentId); - var newDoc = await CleanupSyntaxAsync(document, documentIdAndOptions.codeCleanupOptions, cancellationToken).ConfigureAwait(false); + var cleanedDocument = await CleanupSyntaxAsync(document, documentIdAndOptions.codeCleanupOptions, cancellationToken).ConfigureAwait(false); + if (cleanedDocument is null || cleanedDocument == document) + return; + + var newRoot = cleanedDocument.SupportsSyntaxTree ? await cleanedDocument.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false) : null; + var newText = cleanedDocument.SupportsSyntaxTree ? null : await cleanedDocument.GetValueTextAsync(cancellationToken).ConfigureAwait(false); + callback((document.Id, (newRoot, newText))); }, consumeItems: static async (stream, changedSolution, cancellationToken) => { var currentSolution = changedSolution; - await foreach (var document in stream) + await foreach (var (documentId, (newRoot, newText)) in stream) { - currentSolution = document.SupportsSyntaxTree - ? currentSolution.WithDocumentSyntaxRoot(document.Id, await document.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false)) - : currentSolution.WithDocumentText(document.Id, await document.GetValueTextAsync(cancellationToken).ConfigureAwait(false)); + var document = currentSolution.GetRequiredDocument(documentId); + currentSolution = newRoot != null + ? currentSolution.WithDocumentSyntaxRoot(document.Id, newRoot) + : currentSolution.WithDocumentText(document.Id, newText!); } return currentSolution; From efb7a2b008ef44806a3c8b25b790c4ce8f3c3e4e Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Wed, 8 May 2024 12:01:30 -0700 Subject: [PATCH 13/32] Fixes --- .../Core/Portable/CodeActions/CodeAction.cs | 32 +++++++++++++------ 1 file changed, 22 insertions(+), 10 deletions(-) diff --git a/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs b/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs index e5eca531da3df..a43e9d6d35b58 100644 --- a/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs +++ b/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs @@ -555,8 +555,11 @@ protected virtual async Task PostProcessChangesAsync(Document document return document; } - internal static async Task CleanupDocumentAsync(Document document, CodeCleanupOptions options, CancellationToken cancellationToken) + internal static async ValueTask CleanupDocumentAsync(Document document, CodeCleanupOptions options, CancellationToken cancellationToken) { + if (!document.SupportsSyntaxTree) + return document; + // First, do a syntax pass. Ensuring that things are formatted correctly based on the original nodes and // tokens. We want to do this prior to cleaning semantics as semantic cleanup can change the shape of the tree // (for example, by removing tokens), which can then cause formatting to not work as expected. @@ -567,8 +570,11 @@ internal static async Task CleanupDocumentAsync(Document document, Cod return document2; } - internal static async Task CleanupSyntaxAsync(Document document, CodeCleanupOptions options, CancellationToken cancellationToken) + internal static async ValueTask CleanupSyntaxAsync(Document document, CodeCleanupOptions options, CancellationToken cancellationToken) { + if (!document.SupportsSyntaxTree) + return document; + // format any node with explicit formatter annotation var document1 = await Formatter.FormatAsync(document, Formatter.Annotation, options.FormattingOptions, cancellationToken).ConfigureAwait(false); @@ -577,8 +583,11 @@ internal static async Task CleanupSyntaxAsync(Document document, CodeC return document2; } - internal static async Task CleanupSemanticsAsync(Document document, CodeCleanupOptions options, CancellationToken cancellationToken) + internal static async ValueTask CleanupSemanticsAsync(Document document, CodeCleanupOptions options, CancellationToken cancellationToken) { + if (!document.SupportsSyntaxTree) + return document; + var newSolution = await CleanupSemanticsAsync( document.Project.Solution, [(document.Id, options)], CodeAnalysisProgress.None, cancellationToken).ConfigureAwait(false); return newSolution.GetRequiredDocument(document.Id); @@ -613,8 +622,8 @@ internal static async Task CleanupSemanticsAsync( // good state. The semantic cleanup passes may have introduced new nodes with elastic trivia that have to be // cleaned. var solution4 = await RunParallelCleanupPassAsync( - solution3, static (document, options, cancellationToken) => - CleanupSyntaxAsync(document, options, cancellationToken)).ConfigureAwait(false); + solution3, static async (document, options, cancellationToken) => + await CleanupSyntaxAsync(document, options, cancellationToken).ConfigureAwait(false)).ConfigureAwait(false); return solution4; @@ -638,12 +647,15 @@ async Task RunParallelCleanupPassAsync( // Fetch the current state of the document from this fork of the solution. var document = args.solution.GetRequiredDocument(documentId); - // Now, perform the requested cleanup pass on it. - var cleanedDocument = await args.cleanupDocumentAsync(document, options, cancellationToken).ConfigureAwait(false); + if (document.SupportsSyntaxTree) + { + // Now, perform the requested cleanup pass on it. + var cleanedDocument = await args.cleanupDocumentAsync(document, options, cancellationToken).ConfigureAwait(false); - // Now get the cleaned root and pass it back to the consumer. - var newRoot = await cleanedDocument.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false); - callback((documentId, newRoot)); + // Now get the cleaned root and pass it back to the consumer. + var newRoot = await cleanedDocument.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false); + callback((documentId, newRoot)); + } }, consumeItems: static async (stream, args, cancellationToken) => { From 6e7828173fc943d9933797ff6584b0a737162ea2 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Wed, 8 May 2024 12:04:39 -0700 Subject: [PATCH 14/32] Simplify --- .../Core/Portable/CodeActions/CodeAction.cs | 27 ++++++++++--------- 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs b/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs index a43e9d6d35b58..a1a3d7a77a11b 100644 --- a/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs +++ b/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs @@ -20,6 +20,7 @@ using Microsoft.CodeAnalysis.Host.Mef; using Microsoft.CodeAnalysis.Internal.Log; using Microsoft.CodeAnalysis.Options; +using Microsoft.CodeAnalysis.PooledObjects; using Microsoft.CodeAnalysis.Remote; using Microsoft.CodeAnalysis.Shared.Collections; using Microsoft.CodeAnalysis.Shared.Extensions; @@ -461,15 +462,19 @@ protected Task PostProcessChangesAsync(Solution changedSolution, Cance internal static async Task> GetDocumentIdsAndOptionsAsync( Solution solution, CodeCleanupOptionsProvider optionsProvider, ImmutableArray documentIds, CancellationToken cancellationToken) { - var documentIdsAndOptions = new FixedSizeArrayBuilder<(DocumentId documentId, CodeCleanupOptions options)>(documentIds.Length); + using var _ = ArrayBuilder<(DocumentId documentId, CodeCleanupOptions options)>.GetInstance(documentIds.Length, out var documentIdsAndOptions); foreach (var documentId in documentIds) { var document = solution.GetRequiredDocument(documentId); - var codeActionOptions = await document.GetCodeCleanupOptionsAsync(optionsProvider, cancellationToken).ConfigureAwait(false); - documentIdsAndOptions.Add((documentId, codeActionOptions)); + + if (document.SupportsSyntaxTree) + { + var codeActionOptions = await document.GetCodeCleanupOptionsAsync(optionsProvider, cancellationToken).ConfigureAwait(false); + documentIdsAndOptions.Add((documentId, codeActionOptions)); + } } - return documentIdsAndOptions.MoveToImmutable(); + return documentIdsAndOptions.ToImmutableAndClear(); } internal static async Task PostProcessChangesAsync( @@ -496,28 +501,26 @@ internal static async Task PostProcessChangesAsync( changedSolution, fallbackOptions, documentIds, cancellationToken).ConfigureAwait(false); // Do an initial pass where we cleanup syntax. - var syntaxCleanedSolution = await ProducerConsumer<(DocumentId documentId, (SyntaxNode? node, SourceText? text))>.RunParallelAsync( + var syntaxCleanedSolution = await ProducerConsumer<(DocumentId documentId, SyntaxNode newRoot)>.RunParallelAsync( source: documentIdsAndOptions, produceItems: static async (documentIdAndOptions, callback, changedSolution, cancellationToken) => { var document = changedSolution.GetRequiredDocument(documentIdAndOptions.documentId); + Contract.ThrowIfFalse(document.SupportsSyntaxTree, "GetDocumentIdsAndOptionsAsync should only be returning documents that support syntax"); var cleanedDocument = await CleanupSyntaxAsync(document, documentIdAndOptions.codeCleanupOptions, cancellationToken).ConfigureAwait(false); if (cleanedDocument is null || cleanedDocument == document) return; - var newRoot = cleanedDocument.SupportsSyntaxTree ? await cleanedDocument.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false) : null; - var newText = cleanedDocument.SupportsSyntaxTree ? null : await cleanedDocument.GetValueTextAsync(cancellationToken).ConfigureAwait(false); - callback((document.Id, (newRoot, newText))); + var newRoot = await cleanedDocument.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false); + callback((document.Id, newRoot)); }, consumeItems: static async (stream, changedSolution, cancellationToken) => { var currentSolution = changedSolution; - await foreach (var (documentId, (newRoot, newText)) in stream) + await foreach (var (documentId, newRoot) in stream) { var document = currentSolution.GetRequiredDocument(documentId); - currentSolution = newRoot != null - ? currentSolution.WithDocumentSyntaxRoot(document.Id, newRoot) - : currentSolution.WithDocumentText(document.Id, newText!); + currentSolution = currentSolution.WithDocumentSyntaxRoot(document.Id, newRoot); } return currentSolution; From 0f6439c6683206dac4ccc5fa2af2b2d183516be4 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Wed, 8 May 2024 12:05:26 -0700 Subject: [PATCH 15/32] Only process roslyn docs with trees --- .../DocumentBasedFixAllProviderHelpers.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Workspaces/Core/Portable/CodeFixesAndRefactorings/DocumentBasedFixAllProviderHelpers.cs b/src/Workspaces/Core/Portable/CodeFixesAndRefactorings/DocumentBasedFixAllProviderHelpers.cs index 849d478acea9b..f45050f528095 100644 --- a/src/Workspaces/Core/Portable/CodeFixesAndRefactorings/DocumentBasedFixAllProviderHelpers.cs +++ b/src/Workspaces/Core/Portable/CodeFixesAndRefactorings/DocumentBasedFixAllProviderHelpers.cs @@ -124,11 +124,11 @@ async Task CleanSolutionAsync(Solution dirtySolution, ImmutableArray Date: Wed, 8 May 2024 12:10:00 -0700 Subject: [PATCH 16/32] Simplify --- .../Core/Portable/CodeActions/CodeAction.cs | 8 +++---- .../DocumentBasedFixAllProviderHelpers.cs | 23 ++++++++----------- 2 files changed, 14 insertions(+), 17 deletions(-) diff --git a/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs b/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs index a1a3d7a77a11b..bee6f9660a5ee 100644 --- a/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs +++ b/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs @@ -459,7 +459,7 @@ protected Task PostProcessChangesAsync(Solution changedSolution, Cance #pragma warning restore CA1822 // Mark members as static => PostProcessChangesAsync(originalSolution: null!, changedSolution, cancellationToken); - internal static async Task> GetDocumentIdsAndOptionsAsync( + internal static async Task> GetDocumentIdsAndOptionsToCleanAsync( Solution solution, CodeCleanupOptionsProvider optionsProvider, ImmutableArray documentIds, CancellationToken cancellationToken) { using var _ = ArrayBuilder<(DocumentId documentId, CodeCleanupOptions options)>.GetInstance(documentIds.Length, out var documentIdsAndOptions); @@ -497,12 +497,12 @@ internal static async Task PostProcessChangesAsync( var globalOptions = changedSolution.Services.GetService(); var fallbackOptions = globalOptions?.Provider ?? CodeActionOptions.DefaultProvider; - var documentIdsAndOptions = await GetDocumentIdsAndOptionsAsync( + var documentIdsAndOptionsToClean = await GetDocumentIdsAndOptionsToCleanAsync( changedSolution, fallbackOptions, documentIds, cancellationToken).ConfigureAwait(false); // Do an initial pass where we cleanup syntax. var syntaxCleanedSolution = await ProducerConsumer<(DocumentId documentId, SyntaxNode newRoot)>.RunParallelAsync( - source: documentIdsAndOptions, + source: documentIdsAndOptionsToClean, produceItems: static async (documentIdAndOptions, callback, changedSolution, cancellationToken) => { var document = changedSolution.GetRequiredDocument(documentIdAndOptions.documentId); @@ -530,7 +530,7 @@ internal static async Task PostProcessChangesAsync( // Then do a pass where we cleanup semantics. var semanticCleanedSolution = await CleanupSemanticsAsync( - syntaxCleanedSolution, documentIdsAndOptions, CodeAnalysisProgress.None, cancellationToken).ConfigureAwait(false); + syntaxCleanedSolution, documentIdsAndOptionsToClean, CodeAnalysisProgress.None, cancellationToken).ConfigureAwait(false); return semanticCleanedSolution; } diff --git a/src/Workspaces/Core/Portable/CodeFixesAndRefactorings/DocumentBasedFixAllProviderHelpers.cs b/src/Workspaces/Core/Portable/CodeFixesAndRefactorings/DocumentBasedFixAllProviderHelpers.cs index f45050f528095..742b05795e29a 100644 --- a/src/Workspaces/Core/Portable/CodeFixesAndRefactorings/DocumentBasedFixAllProviderHelpers.cs +++ b/src/Workspaces/Core/Portable/CodeFixesAndRefactorings/DocumentBasedFixAllProviderHelpers.cs @@ -40,10 +40,10 @@ internal static class DocumentBasedFixAllProviderHelpers // One work item for each context. progressTracker.AddItems(fixAllContexts.Length); - var (dirtySolution, changedRootDocumentIds) = await GetInitialUncleanedSolutionAsync().ConfigureAwait(false); - return await CleanSolutionAsync(dirtySolution, changedRootDocumentIds).ConfigureAwait(false); + var (dirtySolution, changedDocumentIds) = await GetInitialUncleanedSolutionAsync().ConfigureAwait(false); + return await CleanSolutionAsync(dirtySolution, changedDocumentIds).ConfigureAwait(false); - async Task<(Solution dirtySolution, ImmutableArray changedRootDocumentIds)> GetInitialUncleanedSolutionAsync() + async Task<(Solution dirtySolution, ImmutableArray changedDocumentIds)> GetInitialUncleanedSolutionAsync() { // First, iterate over all contexts, and collect all the changes for each of them. We'll be making a lot of // calls to the remote server to compute diagnostics and changes. So keep a single connection alive to it @@ -76,7 +76,7 @@ await args.getFixedDocumentsAsync( consumeItems: static async (stream, args, cancellationToken) => { var currentSolution = args.solution; - using var _ = ArrayBuilder.GetInstance(out var changedRootDocumentIds); + using var _ = ArrayBuilder.GetInstance(out var changedDocumentIds); // Next, go and insert those all into the solution so all the docs in this particular project point // at the new trees (or text). At this point though, the trees have not been semantically cleaned @@ -87,25 +87,22 @@ await args.getFixedDocumentsAsync( // cleanup semantics on one forked solution. await foreach (var (docId, (newRoot, newText)) in stream) { - // If we produced a new root (as opposed to new text), keep track of that doc-id so that we - // can clean this doc later. - if (newRoot != null) - changedRootDocumentIds.Add(docId); + changedDocumentIds.Add(docId); currentSolution = newRoot != null ? currentSolution.WithDocumentSyntaxRoot(docId, newRoot) : currentSolution.WithDocumentText(docId, newText!); } - return (currentSolution, changedRootDocumentIds.ToImmutableAndClear()); + return (currentSolution, changedDocumentIds.ToImmutableAndClear()); }, args: (getFixedDocumentsAsync, progressTracker, solution), cancellationToken).ConfigureAwait(false); } - async Task CleanSolutionAsync(Solution dirtySolution, ImmutableArray changedRootDocumentIds) + async Task CleanSolutionAsync(Solution dirtySolution, ImmutableArray changedDocumentIds) { - if (changedRootDocumentIds.IsEmpty) + if (changedDocumentIds.IsEmpty) return dirtySolution; // Clear out the progress so far. We're starting a new progress pass for the final cleanup. @@ -114,8 +111,8 @@ async Task CleanSolutionAsync(Solution dirtySolution, ImmutableArray Date: Wed, 8 May 2024 12:15:48 -0700 Subject: [PATCH 17/32] Make helper --- .../Core/Portable/CodeActions/CodeAction.cs | 23 +++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs b/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs index bee6f9660a5ee..8c6345a163151 100644 --- a/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs +++ b/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs @@ -487,6 +487,23 @@ internal static async Task PostProcessChangesAsync( // underneath us). But it's the only option we have for the compat case with existing public extension // points. originalSolution ??= changedSolution.Workspace.CurrentSolution; + + var globalOptions = changedSolution.Services.GetService(); + var fallbackOptions = globalOptions?.Provider ?? CodeActionOptions.DefaultProvider; + return await PostProcessChangesAsync(originalSolution, changedSolution, fallbackOptions, CodeAnalysisProgress.None, cancellationToken).ConfigureAwait(false); + } + + internal static async Task PostProcessChangesAsync( + Solution originalSolution, + Solution changedSolution, + CodeCleanupOptionsProvider codeCleanupOptions, + IProgress progress, + CancellationToken cancellationToken) + { + // originalSolution is only null on backward compatible codepaths. In that case, we get the workspace's + // current solution. This is not ideal (as that is a mutable field that could be changing out from + // underneath us). But it's the only option we have for the compat case with existing public extension + // points. var solutionChanges = changedSolution.GetChanges(originalSolution); var documentIds = solutionChanges @@ -495,10 +512,8 @@ internal static async Task PostProcessChangesAsync( .Concat(solutionChanges.GetAddedProjects().SelectMany(p => p.DocumentIds)) .ToImmutableArray(); - var globalOptions = changedSolution.Services.GetService(); - var fallbackOptions = globalOptions?.Provider ?? CodeActionOptions.DefaultProvider; var documentIdsAndOptionsToClean = await GetDocumentIdsAndOptionsToCleanAsync( - changedSolution, fallbackOptions, documentIds, cancellationToken).ConfigureAwait(false); + changedSolution, codeCleanupOptions, documentIds, cancellationToken).ConfigureAwait(false); // Do an initial pass where we cleanup syntax. var syntaxCleanedSolution = await ProducerConsumer<(DocumentId documentId, SyntaxNode newRoot)>.RunParallelAsync( @@ -530,7 +545,7 @@ internal static async Task PostProcessChangesAsync( // Then do a pass where we cleanup semantics. var semanticCleanedSolution = await CleanupSemanticsAsync( - syntaxCleanedSolution, documentIdsAndOptionsToClean, CodeAnalysisProgress.None, cancellationToken).ConfigureAwait(false); + syntaxCleanedSolution, documentIdsAndOptionsToClean, progress, cancellationToken).ConfigureAwait(false); return semanticCleanedSolution; } From a203ee6af814e7f5b3226c28bd3b975860211c9f Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Wed, 8 May 2024 12:23:20 -0700 Subject: [PATCH 18/32] Share code --- .../Core/Portable/CodeActions/CodeAction.cs | 27 ++-- .../DocumentBasedFixAllProviderHelpers.cs | 123 +++++++----------- 2 files changed, 60 insertions(+), 90 deletions(-) diff --git a/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs b/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs index 8c6345a163151..638d1da8ed1e2 100644 --- a/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs +++ b/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs @@ -490,27 +490,32 @@ internal static async Task PostProcessChangesAsync( var globalOptions = changedSolution.Services.GetService(); var fallbackOptions = globalOptions?.Provider ?? CodeActionOptions.DefaultProvider; - return await PostProcessChangesAsync(originalSolution, changedSolution, fallbackOptions, CodeAnalysisProgress.None, cancellationToken).ConfigureAwait(false); + return await CleanSyntaxAndSemanticsAsync(originalSolution, changedSolution, fallbackOptions, CodeAnalysisProgress.None, cancellationToken).ConfigureAwait(false); } - internal static async Task PostProcessChangesAsync( + internal static ImmutableArray GetAllChangedOrAddedDocumentIds( Solution originalSolution, - Solution changedSolution, - CodeCleanupOptionsProvider codeCleanupOptions, - IProgress progress, - CancellationToken cancellationToken) + Solution changedSolution) { - // originalSolution is only null on backward compatible codepaths. In that case, we get the workspace's - // current solution. This is not ideal (as that is a mutable field that could be changing out from - // underneath us). But it's the only option we have for the compat case with existing public extension - // points. var solutionChanges = changedSolution.GetChanges(originalSolution); - var documentIds = solutionChanges .GetProjectChanges() .SelectMany(p => p.GetChangedDocuments(onlyGetDocumentsWithTextChanges: true).Concat(p.GetAddedDocuments())) .Concat(solutionChanges.GetAddedProjects().SelectMany(p => p.DocumentIds)) .ToImmutableArray(); + return documentIds; + } + + internal static async Task CleanSyntaxAndSemanticsAsync( + Solution originalSolution, + Solution changedSolution, + CodeCleanupOptionsProvider codeCleanupOptions, + IProgress progress, + CancellationToken cancellationToken) + { + var solutionChanges = changedSolution.GetChanges(originalSolution); + + var documentIds = GetAllChangedOrAddedDocumentIds(originalSolution, changedSolution); var documentIdsAndOptionsToClean = await GetDocumentIdsAndOptionsToCleanAsync( changedSolution, codeCleanupOptions, documentIds, cancellationToken).ConfigureAwait(false); diff --git a/src/Workspaces/Core/Portable/CodeFixesAndRefactorings/DocumentBasedFixAllProviderHelpers.cs b/src/Workspaces/Core/Portable/CodeFixesAndRefactorings/DocumentBasedFixAllProviderHelpers.cs index 742b05795e29a..1ab7fc697cf88 100644 --- a/src/Workspaces/Core/Portable/CodeFixesAndRefactorings/DocumentBasedFixAllProviderHelpers.cs +++ b/src/Workspaces/Core/Portable/CodeFixesAndRefactorings/DocumentBasedFixAllProviderHelpers.cs @@ -4,12 +4,9 @@ using System; using System.Collections.Immutable; -using System.Threading; using System.Threading.Tasks; using Microsoft.CodeAnalysis.CodeActions; -using Microsoft.CodeAnalysis.CodeCleanup; using Microsoft.CodeAnalysis.CodeFixes; -using Microsoft.CodeAnalysis.PooledObjects; using Microsoft.CodeAnalysis.Remote; using Microsoft.CodeAnalysis.Shared.Extensions; using Microsoft.CodeAnalysis.Shared.Utilities; @@ -35,20 +32,44 @@ internal static class DocumentBasedFixAllProviderHelpers progressTracker.Report(CodeAnalysisProgress.Description(progressTrackerDescription)); - var solution = originalFixAllContext.Solution; + var originalSolution = originalFixAllContext.Solution; // One work item for each context. progressTracker.AddItems(fixAllContexts.Length); - var (dirtySolution, changedDocumentIds) = await GetInitialUncleanedSolutionAsync().ConfigureAwait(false); - return await CleanSolutionAsync(dirtySolution, changedDocumentIds).ConfigureAwait(false); + // Do the initial pass to fixup documents. + var dirtySolution = await GetInitialUncleanedSolutionAsync(originalSolution).ConfigureAwait(false); + + // Now do a pass to clean the fixed documents. + progressTracker.Report(CodeAnalysisProgress.Clear()); + progressTracker.Report(CodeAnalysisProgress.Description(WorkspacesResources.Running_code_cleanup_on_fixed_documents)); + + var cleanedSolution = await CodeAction.CleanSyntaxAndSemanticsAsync( + originalSolution, + dirtySolution, + originalFixAllContext.State.CodeActionOptionsProvider, + progressTracker, + cancellationToken).ConfigureAwait(false); + + // Once we clean the document, we get the text of it and insert that back into the final solution. This way + // we can release both the original fixed tree, and the cleaned tree (both of which can be much more + // expensive than just text). + var finalSolution = cleanedSolution; + foreach (var documentId in CodeAction.GetAllChangedOrAddedDocumentIds(originalSolution, finalSolution)) + { + var cleanedDocument = finalSolution.GetRequiredDocument(documentId); + var cleanedText = await cleanedDocument.GetValueTextAsync(cancellationToken).ConfigureAwait(false); + finalSolution = finalSolution.WithDocumentText(documentId, cleanedText); + } + + return finalSolution; - async Task<(Solution dirtySolution, ImmutableArray changedDocumentIds)> GetInitialUncleanedSolutionAsync() + async Task GetInitialUncleanedSolutionAsync(Solution originalSolution) { // First, iterate over all contexts, and collect all the changes for each of them. We'll be making a lot of // calls to the remote server to compute diagnostics and changes. So keep a single connection alive to it // so we never resync or recompute anything. - using var _ = await RemoteKeepAliveSession.CreateAsync(solution, cancellationToken).ConfigureAwait(false); + using var _ = await RemoteKeepAliveSession.CreateAsync(originalSolution, cancellationToken).ConfigureAwait(false); return await ProducerConsumer<(DocumentId documentId, (SyntaxNode? node, SourceText? text))>.RunParallelAsync( source: fixAllContexts, @@ -65,18 +86,23 @@ await args.getFixedDocumentsAsync( fixAllContext, async (originalDocument, newDocument) => { - // As the FixAllProvider informs us about fixed documents, go and clean them up - // syntactically, and then invoke the callback to put into the channel for consumption. - var tuple = await CleanDocumentSyntaxAsync( - originalDocument, newDocument, fixAllContext.State.CodeActionOptionsProvider, cancellationToken).ConfigureAwait(false); - if (tuple.HasValue) - callback(tuple.Value); + //// As the FixAllProvider informs us about fixed documents, go and clean them up + //// syntactically, and then invoke the callback to put into the channel for consumption. + //var tuple = await CleanDocumentSyntaxAsync( + // originalDocument, newDocument, fixAllContext.State.CodeActionOptionsProvider, cancellationToken).ConfigureAwait(false); + //if (tuple.HasValue) + // callback(tuple.Value); + if (newDocument == null || newDocument == originalDocument) + return; + + var newRoot = newDocument.SupportsSyntaxTree ? await newDocument.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false) : null; + var newText = newDocument.SupportsSyntaxTree ? null : await newDocument.GetValueTextAsync(cancellationToken).ConfigureAwait(false); + callback((newDocument.Id, (newRoot, newText))); }).ConfigureAwait(false); }, consumeItems: static async (stream, args, cancellationToken) => { - var currentSolution = args.solution; - using var _ = ArrayBuilder.GetInstance(out var changedDocumentIds); + var currentSolution = args.originalSolution; // Next, go and insert those all into the solution so all the docs in this particular project point // at the new trees (or text). At this point though, the trees have not been semantically cleaned @@ -87,76 +113,15 @@ await args.getFixedDocumentsAsync( // cleanup semantics on one forked solution. await foreach (var (docId, (newRoot, newText)) in stream) { - changedDocumentIds.Add(docId); - currentSolution = newRoot != null ? currentSolution.WithDocumentSyntaxRoot(docId, newRoot) : currentSolution.WithDocumentText(docId, newText!); } - return (currentSolution, changedDocumentIds.ToImmutableAndClear()); + return currentSolution; }, - args: (getFixedDocumentsAsync, progressTracker, solution), + args: (getFixedDocumentsAsync, progressTracker, originalSolution), cancellationToken).ConfigureAwait(false); } - - async Task CleanSolutionAsync(Solution dirtySolution, ImmutableArray changedDocumentIds) - { - if (changedDocumentIds.IsEmpty) - return dirtySolution; - - // Clear out the progress so far. We're starting a new progress pass for the final cleanup. - progressTracker.Report(CodeAnalysisProgress.Clear()); - progressTracker.Report(CodeAnalysisProgress.Description(WorkspacesResources.Running_code_cleanup_on_fixed_documents)); - - // Next, go and semantically cleanup any trees we inserted. Do this in parallel across all the documents - // that were fixed and resulted in a new tree (as opposed to new text). - var documentIdsAndOptions = await CodeAction.GetDocumentIdsAndOptionsToCleanAsync( - dirtySolution, originalFixAllContext.State.CodeActionOptionsProvider, changedDocumentIds, cancellationToken).ConfigureAwait(false); - - var solutionWithCleanedRoots = await CodeAction.CleanupSemanticsAsync( - dirtySolution, documentIdsAndOptions, progressTracker, cancellationToken).ConfigureAwait(false); - - // Once we clean the document, we get the text of it and insert that back into the final solution. This way - // we can release both the original fixed tree, and the cleaned tree (both of which can be much more - // expensive than just text). - var finalSolution = dirtySolution; - foreach (var (documentId, _) in documentIdsAndOptions) - { - var cleanedDocument = solutionWithCleanedRoots.GetRequiredDocument(documentId); - var cleanedText = await cleanedDocument.GetValueTextAsync(cancellationToken).ConfigureAwait(false); - finalSolution = finalSolution.WithDocumentText(documentId, cleanedText); - } - - return finalSolution; - } - - static async ValueTask<(DocumentId documentId, (SyntaxNode? node, SourceText? text))?> CleanDocumentSyntaxAsync( - Document document, - Document? newDocument, - CodeActionOptionsProvider codeActionOptionsProvider, - CancellationToken cancellationToken) - { - if (newDocument == null || newDocument == document) - return null; - - if (newDocument.SupportsSyntaxTree) - { - // For documents that support syntax, grab the tree so that we can clean it. We do the formatting up front - // ensuring that we have well-formatted syntax trees in the solution to work with. A later pass will do the - // semantic cleanup on all documents in parallel. - var codeActionOptions = await newDocument.GetCodeCleanupOptionsAsync(codeActionOptionsProvider, cancellationToken).ConfigureAwait(false); - var cleanedDocument = await CodeAction.CleanupSyntaxAsync(newDocument, codeActionOptions, cancellationToken).ConfigureAwait(false); - var node = await cleanedDocument.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false); - - return (document.Id, (node, text: null)); - } - else - { - // If it's a language that doesn't support that, then just grab the text. - var text = await newDocument.GetValueTextAsync(cancellationToken).ConfigureAwait(false); - return (document.Id, (node: null, text)); - } - } } } From bb28d319a92b096b83c648f7fe395d192020474f Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Wed, 8 May 2024 12:24:06 -0700 Subject: [PATCH 19/32] cleanup --- .../DocumentBasedFixAllProviderHelpers.cs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/Workspaces/Core/Portable/CodeFixesAndRefactorings/DocumentBasedFixAllProviderHelpers.cs b/src/Workspaces/Core/Portable/CodeFixesAndRefactorings/DocumentBasedFixAllProviderHelpers.cs index 1ab7fc697cf88..2383629bbdc42 100644 --- a/src/Workspaces/Core/Portable/CodeFixesAndRefactorings/DocumentBasedFixAllProviderHelpers.cs +++ b/src/Workspaces/Core/Portable/CodeFixesAndRefactorings/DocumentBasedFixAllProviderHelpers.cs @@ -86,12 +86,6 @@ await args.getFixedDocumentsAsync( fixAllContext, async (originalDocument, newDocument) => { - //// As the FixAllProvider informs us about fixed documents, go and clean them up - //// syntactically, and then invoke the callback to put into the channel for consumption. - //var tuple = await CleanDocumentSyntaxAsync( - // originalDocument, newDocument, fixAllContext.State.CodeActionOptionsProvider, cancellationToken).ConfigureAwait(false); - //if (tuple.HasValue) - // callback(tuple.Value); if (newDocument == null || newDocument == originalDocument) return; From 5b014712fc1df95309ce60c563e00a59cd31f7c5 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Wed, 8 May 2024 12:27:12 -0700 Subject: [PATCH 20/32] in progress --- .../Core/Portable/CodeActions/CodeAction.cs | 43 +++++++++---------- 1 file changed, 20 insertions(+), 23 deletions(-) diff --git a/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs b/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs index 638d1da8ed1e2..3e0b208d837ae 100644 --- a/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs +++ b/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs @@ -459,24 +459,6 @@ protected Task PostProcessChangesAsync(Solution changedSolution, Cance #pragma warning restore CA1822 // Mark members as static => PostProcessChangesAsync(originalSolution: null!, changedSolution, cancellationToken); - internal static async Task> GetDocumentIdsAndOptionsToCleanAsync( - Solution solution, CodeCleanupOptionsProvider optionsProvider, ImmutableArray documentIds, CancellationToken cancellationToken) - { - using var _ = ArrayBuilder<(DocumentId documentId, CodeCleanupOptions options)>.GetInstance(documentIds.Length, out var documentIdsAndOptions); - foreach (var documentId in documentIds) - { - var document = solution.GetRequiredDocument(documentId); - - if (document.SupportsSyntaxTree) - { - var codeActionOptions = await document.GetCodeCleanupOptionsAsync(optionsProvider, cancellationToken).ConfigureAwait(false); - documentIdsAndOptions.Add((documentId, codeActionOptions)); - } - } - - return documentIdsAndOptions.ToImmutableAndClear(); - } - internal static async Task PostProcessChangesAsync( Solution originalSolution, Solution changedSolution, @@ -509,18 +491,16 @@ internal static ImmutableArray GetAllChangedOrAddedDocumentIds( internal static async Task CleanSyntaxAndSemanticsAsync( Solution originalSolution, Solution changedSolution, - CodeCleanupOptionsProvider codeCleanupOptions, + CodeCleanupOptionsProvider optionsProvider, IProgress progress, CancellationToken cancellationToken) { var solutionChanges = changedSolution.GetChanges(originalSolution); var documentIds = GetAllChangedOrAddedDocumentIds(originalSolution, changedSolution); + var documentIdsAndOptionsToClean = await GetDocumentIdsAndOptionsToCleanAsync().ConfigureAwait(false); - var documentIdsAndOptionsToClean = await GetDocumentIdsAndOptionsToCleanAsync( - changedSolution, codeCleanupOptions, documentIds, cancellationToken).ConfigureAwait(false); - - // Do an initial pass where we cleanup syntax. + // Do an initial pass where we cleanup syntax only. var syntaxCleanedSolution = await ProducerConsumer<(DocumentId documentId, SyntaxNode newRoot)>.RunParallelAsync( source: documentIdsAndOptionsToClean, produceItems: static async (documentIdAndOptions, callback, changedSolution, cancellationToken) => @@ -553,6 +533,23 @@ internal static async Task CleanSyntaxAndSemanticsAsync( syntaxCleanedSolution, documentIdsAndOptionsToClean, progress, cancellationToken).ConfigureAwait(false); return semanticCleanedSolution; + + async Task> GetDocumentIdsAndOptionsToCleanAsync() + { + using var _ = ArrayBuilder<(DocumentId documentId, CodeCleanupOptions options)>.GetInstance(documentIds.Length, out var documentIdsAndOptions); + foreach (var documentId in documentIds) + { + var document = changedSolution.GetRequiredDocument(documentId); + + if (document.SupportsSyntaxTree) + { + var codeActionOptions = await document.GetCodeCleanupOptionsAsync(optionsProvider, cancellationToken).ConfigureAwait(false); + documentIdsAndOptions.Add((documentId, codeActionOptions)); + } + } + + return documentIdsAndOptions.ToImmutableAndClear(); + } } /// From 6d52ccba1dd2633c7e77bb167a9b46445c92eacd Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Wed, 8 May 2024 12:27:56 -0700 Subject: [PATCH 21/32] Simplify --- src/Workspaces/Core/Portable/CodeActions/CodeAction.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs b/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs index 3e0b208d837ae..da6808bc42db8 100644 --- a/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs +++ b/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs @@ -671,6 +671,8 @@ async Task RunParallelCleanupPassAsync( { // Now, perform the requested cleanup pass on it. var cleanedDocument = await args.cleanupDocumentAsync(document, options, cancellationToken).ConfigureAwait(false); + if (cleanedDocument is null || cleanedDocument == document) + return; // Now get the cleaned root and pass it back to the consumer. var newRoot = await cleanedDocument.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false); From cdf1375a3f4bee829ece9acc3d4aa89cf258415b Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Wed, 8 May 2024 12:29:43 -0700 Subject: [PATCH 22/32] Cleanup --- .../Core/Portable/CodeActions/CodeAction.cs | 114 ++++++++++-------- 1 file changed, 63 insertions(+), 51 deletions(-) diff --git a/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs b/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs index da6808bc42db8..4de608c6fa617 100644 --- a/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs +++ b/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs @@ -625,72 +625,84 @@ internal static async Task CleanupSemanticsAsync( // First, add all missing imports to all changed documents. var solution1 = await RunParallelCleanupPassAsync( - solution, static (document, options, cancellationToken) => - ImportAdder.AddImportsFromSymbolAnnotationAsync(document, Simplifier.AddImportsAnnotation, options.AddImportOptions, cancellationToken)).ConfigureAwait(false); + solution, documentIdsAndOptions, progress, + static (document, options, cancellationToken) => + ImportAdder.AddImportsFromSymbolAnnotationAsync(document, Simplifier.AddImportsAnnotation, options.AddImportOptions, cancellationToken), + cancellationToken).ConfigureAwait(false); // Then simplify any expanded constructs. var solution2 = await RunParallelCleanupPassAsync( - solution1, static (document, options, cancellationToken) => - Simplifier.ReduceAsync(document, Simplifier.Annotation, options.SimplifierOptions, cancellationToken)).ConfigureAwait(false); + solution1, documentIdsAndOptions, progress, + static (document, options, cancellationToken) => + Simplifier.ReduceAsync(document, Simplifier.Annotation, options.SimplifierOptions, cancellationToken), + cancellationToken).ConfigureAwait(false); // Do any necessary case correction for VB files. var solution3 = await RunParallelCleanupPassAsync( - solution2, static (document, options, cancellationToken) => - CaseCorrector.CaseCorrectAsync(document, CaseCorrector.Annotation, cancellationToken)).ConfigureAwait(false); + solution2, documentIdsAndOptions, progress, + static (document, options, cancellationToken) => + CaseCorrector.CaseCorrectAsync(document, CaseCorrector.Annotation, cancellationToken), + cancellationToken).ConfigureAwait(false); // Finally, after doing the semantic cleanup, do another syntax cleanup pass to ensure that the tree is in a // good state. The semantic cleanup passes may have introduced new nodes with elastic trivia that have to be // cleaned. var solution4 = await RunParallelCleanupPassAsync( - solution3, static async (document, options, cancellationToken) => - await CleanupSyntaxAsync(document, options, cancellationToken).ConfigureAwait(false)).ConfigureAwait(false); + solution3, documentIdsAndOptions, progress, + static async (document, options, cancellationToken) => + await CleanupSyntaxAsync(document, options, cancellationToken).ConfigureAwait(false), + cancellationToken).ConfigureAwait(false); return solution4; + } - async Task RunParallelCleanupPassAsync( - Solution solution, Func> cleanupDocumentAsync) - { - // We're about to making a ton of calls to this new solution, including expensive oop calls to get up to - // date compilations, skeletons and SG docs. Create and pin this solution so that all remote calls operate - // on the same fork and do not cause the forked solution to be created and dropped repeatedly. - using var _ = await RemoteKeepAliveSession.CreateAsync(solution, cancellationToken).ConfigureAwait(false); - - return await ProducerConsumer<(DocumentId documentId, SyntaxNode newRoot)>.RunParallelAsync( - source: documentIdsAndOptions, - produceItems: static async (documentIdAndOptions, callback, args, cancellationToken) => - { - // As we finish each document, update our progress. - using var _ = args.progress.ItemCompletedScope(); - - var (documentId, options) = documentIdAndOptions; - - // Fetch the current state of the document from this fork of the solution. - var document = args.solution.GetRequiredDocument(documentId); - - if (document.SupportsSyntaxTree) - { - // Now, perform the requested cleanup pass on it. - var cleanedDocument = await args.cleanupDocumentAsync(document, options, cancellationToken).ConfigureAwait(false); - if (cleanedDocument is null || cleanedDocument == document) - return; - - // Now get the cleaned root and pass it back to the consumer. - var newRoot = await cleanedDocument.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false); - callback((documentId, newRoot)); - } - }, - consumeItems: static async (stream, args, cancellationToken) => + private static async Task RunParallelCleanupPassAsync( + Solution solution, + ImmutableArray<(DocumentId documentId, CodeCleanupOptions options)> documentIdsAndOptions, + IProgress progress, + Func> cleanupDocumentAsync, + CancellationToken cancellationToken) + { + // We're about to making a ton of calls to this new solution, including expensive oop calls to get up to + // date compilations, skeletons and SG docs. Create and pin this solution so that all remote calls operate + // on the same fork and do not cause the forked solution to be created and dropped repeatedly. + using var _ = await RemoteKeepAliveSession.CreateAsync(solution, cancellationToken).ConfigureAwait(false); + + return await ProducerConsumer<(DocumentId documentId, SyntaxNode newRoot)>.RunParallelAsync( + source: documentIdsAndOptions, + produceItems: static async (documentIdAndOptions, callback, args, cancellationToken) => + { + // As we finish each document, update our progress. + using var _ = args.progress.ItemCompletedScope(); + + var (documentId, options) = documentIdAndOptions; + + // Fetch the current state of the document from this fork of the solution. + var document = args.solution.GetRequiredDocument(documentId); + + if (document.SupportsSyntaxTree) { - // Grab all the cleaned roots and produce the new solution snapshot from that. - var currentSolution = args.solution; - await foreach (var (documentId, newRoot) in stream) - currentSolution = currentSolution.WithDocumentSyntaxRoot(documentId, newRoot); - - return currentSolution; - }, - args: (solution, progress, cleanupDocumentAsync), - cancellationToken).ConfigureAwait(false); - } + // Now, perform the requested cleanup pass on it. + var cleanedDocument = await args.cleanupDocumentAsync(document, options, cancellationToken).ConfigureAwait(false); + if (cleanedDocument is null || cleanedDocument == document) + return; + + // Now get the cleaned root and pass it back to the consumer. + var newRoot = await cleanedDocument.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false); + callback((documentId, newRoot)); + } + }, + consumeItems: static async (stream, args, cancellationToken) => + { + // Grab all the cleaned roots and produce the new solution snapshot from that. + var currentSolution = args.solution; + await foreach (var (documentId, newRoot) in stream) + currentSolution = currentSolution.WithDocumentSyntaxRoot(documentId, newRoot); + + return currentSolution; + }, + args: (solution, progress, cleanupDocumentAsync), + cancellationToken).ConfigureAwait(false); } #region Factories for standard code actions From 2d535e696f824288dcaece03e192d9725beaa113 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Wed, 8 May 2024 12:32:28 -0700 Subject: [PATCH 23/32] No need for check --- .../Core/Portable/CodeActions/CodeAction.cs | 20 +++++++++---------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs b/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs index 4de608c6fa617..8b60e21d140d3 100644 --- a/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs +++ b/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs @@ -679,18 +679,16 @@ private static async Task RunParallelCleanupPassAsync( // Fetch the current state of the document from this fork of the solution. var document = args.solution.GetRequiredDocument(documentId); + Contract.ThrowIfFalse(document.SupportsSyntaxTree, "GetDocumentIdsAndOptionsAsync should only be returning documents that support syntax"); - if (document.SupportsSyntaxTree) - { - // Now, perform the requested cleanup pass on it. - var cleanedDocument = await args.cleanupDocumentAsync(document, options, cancellationToken).ConfigureAwait(false); - if (cleanedDocument is null || cleanedDocument == document) - return; - - // Now get the cleaned root and pass it back to the consumer. - var newRoot = await cleanedDocument.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false); - callback((documentId, newRoot)); - } + // Now, perform the requested cleanup pass on it. + var cleanedDocument = await args.cleanupDocumentAsync(document, options, cancellationToken).ConfigureAwait(false); + if (cleanedDocument is null || cleanedDocument == document) + return; + + // Now get the cleaned root and pass it back to the consumer. + var newRoot = await cleanedDocument.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false); + callback((documentId, newRoot)); }, consumeItems: static async (stream, args, cancellationToken) => { From 334606e056ae0e090a0e1d9f744ed6bfea083319 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Wed, 8 May 2024 12:37:41 -0700 Subject: [PATCH 24/32] Simplify --- .../Core/Portable/CodeActions/CodeAction.cs | 94 ++++++------------- 1 file changed, 31 insertions(+), 63 deletions(-) diff --git a/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs b/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs index 8b60e21d140d3..dd88f934a3402 100644 --- a/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs +++ b/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs @@ -495,44 +495,14 @@ internal static async Task CleanSyntaxAndSemanticsAsync( IProgress progress, CancellationToken cancellationToken) { - var solutionChanges = changedSolution.GetChanges(originalSolution); - var documentIds = GetAllChangedOrAddedDocumentIds(originalSolution, changedSolution); var documentIdsAndOptionsToClean = await GetDocumentIdsAndOptionsToCleanAsync().ConfigureAwait(false); - // Do an initial pass where we cleanup syntax only. - var syntaxCleanedSolution = await ProducerConsumer<(DocumentId documentId, SyntaxNode newRoot)>.RunParallelAsync( - source: documentIdsAndOptionsToClean, - produceItems: static async (documentIdAndOptions, callback, changedSolution, cancellationToken) => - { - var document = changedSolution.GetRequiredDocument(documentIdAndOptions.documentId); - Contract.ThrowIfFalse(document.SupportsSyntaxTree, "GetDocumentIdsAndOptionsAsync should only be returning documents that support syntax"); - var cleanedDocument = await CleanupSyntaxAsync(document, documentIdAndOptions.codeCleanupOptions, cancellationToken).ConfigureAwait(false); - if (cleanedDocument is null || cleanedDocument == document) - return; - - var newRoot = await cleanedDocument.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false); - callback((document.Id, newRoot)); - }, - consumeItems: static async (stream, changedSolution, cancellationToken) => - { - var currentSolution = changedSolution; - await foreach (var (documentId, newRoot) in stream) - { - var document = currentSolution.GetRequiredDocument(documentId); - currentSolution = currentSolution.WithDocumentSyntaxRoot(document.Id, newRoot); - } - - return currentSolution; - }, - args: changedSolution, - cancellationToken).ConfigureAwait(false); - // Then do a pass where we cleanup semantics. - var semanticCleanedSolution = await CleanupSemanticsAsync( - syntaxCleanedSolution, documentIdsAndOptionsToClean, progress, cancellationToken).ConfigureAwait(false); + var cleanedSolution = await CleanupSyntaxAndSemanticsAsync( + changedSolution, documentIdsAndOptionsToClean, progress, cancellationToken).ConfigureAwait(false); - return semanticCleanedSolution; + return cleanedSolution; async Task> GetDocumentIdsAndOptionsToCleanAsync() { @@ -580,17 +550,16 @@ internal static async ValueTask CleanupDocumentAsync(Document document if (!document.SupportsSyntaxTree) return document; - // First, do a syntax pass. Ensuring that things are formatted correctly based on the original nodes and - // tokens. We want to do this prior to cleaning semantics as semantic cleanup can change the shape of the tree - // (for example, by removing tokens), which can then cause formatting to not work as expected. - var document1 = await CleanupSyntaxAsync(document, options, cancellationToken).ConfigureAwait(false); + var cleanedSolution = await CleanupSyntaxAndSemanticsAsync( + document.Project.Solution, + [(document.Id, options)], + CodeAnalysisProgress.None, + cancellationToken).ConfigureAwait(false); - // Now, do the semantic cleaning pass. - var document2 = await CleanupSemanticsAsync(document1, options, cancellationToken).ConfigureAwait(false); - return document2; + return cleanedSolution.GetRequiredDocument(document.Id); } - internal static async ValueTask CleanupSyntaxAsync(Document document, CodeCleanupOptions options, CancellationToken cancellationToken) + private static async ValueTask CleanupSyntaxAsync(Document document, CodeCleanupOptions options, CancellationToken cancellationToken) { if (!document.SupportsSyntaxTree) return document; @@ -603,43 +572,42 @@ internal static async ValueTask CleanupSyntaxAsync(Document document, return document2; } - internal static async ValueTask CleanupSemanticsAsync(Document document, CodeCleanupOptions options, CancellationToken cancellationToken) - { - if (!document.SupportsSyntaxTree) - return document; - - var newSolution = await CleanupSemanticsAsync( - document.Project.Solution, [(document.Id, options)], CodeAnalysisProgress.None, cancellationToken).ConfigureAwait(false); - return newSolution.GetRequiredDocument(document.Id); - } - - internal static async Task CleanupSemanticsAsync( + internal static async Task CleanupSyntaxAndSemanticsAsync( Solution solution, ImmutableArray<(DocumentId documentId, CodeCleanupOptions options)> documentIdsAndOptions, IProgress progress, CancellationToken cancellationToken) { - // We do this in 4 serialized passes. This allows us to process all documents in parallel, while only forking - // the solution 4 times *total* (instead of 4 times *per* document). - progress.AddItems(documentIdsAndOptions.Length * 4); + // We do this in 5 serialized passes. This allows us to process all documents in parallel, while only forking + // the solution 5 times *total* (instead of 5 times *per* document). + progress.AddItems(documentIdsAndOptions.Length * 5); - // First, add all missing imports to all changed documents. + // First, ensure that everything is formatted as the feature asked for. We want to do this prior to doing + // semantic cleanup as the semantic cleanup passes may end up making changes that end up dropping some of the + // formatting/elastic annotations that the feature wanted respected. var solution1 = await RunParallelCleanupPassAsync( solution, documentIdsAndOptions, progress, - static (document, options, cancellationToken) => - ImportAdder.AddImportsFromSymbolAnnotationAsync(document, Simplifier.AddImportsAnnotation, options.AddImportOptions, cancellationToken), + static async (document, options, cancellationToken) => + await CleanupSyntaxAsync(document, options, cancellationToken).ConfigureAwait(false), cancellationToken).ConfigureAwait(false); - // Then simplify any expanded constructs. + // Then add all missing imports to all changed documents. var solution2 = await RunParallelCleanupPassAsync( solution1, documentIdsAndOptions, progress, static (document, options, cancellationToken) => - Simplifier.ReduceAsync(document, Simplifier.Annotation, options.SimplifierOptions, cancellationToken), + ImportAdder.AddImportsFromSymbolAnnotationAsync(document, Simplifier.AddImportsAnnotation, options.AddImportOptions, cancellationToken), cancellationToken).ConfigureAwait(false); - // Do any necessary case correction for VB files. + // Then simplify any expanded constructs. var solution3 = await RunParallelCleanupPassAsync( solution2, documentIdsAndOptions, progress, + static (document, options, cancellationToken) => + Simplifier.ReduceAsync(document, Simplifier.Annotation, options.SimplifierOptions, cancellationToken), + cancellationToken).ConfigureAwait(false); + + // The do any necessary case correction for VB files. + var solution4 = await RunParallelCleanupPassAsync( + solution3, documentIdsAndOptions, progress, static (document, options, cancellationToken) => CaseCorrector.CaseCorrectAsync(document, CaseCorrector.Annotation, cancellationToken), cancellationToken).ConfigureAwait(false); @@ -647,8 +615,8 @@ internal static async Task CleanupSemanticsAsync( // Finally, after doing the semantic cleanup, do another syntax cleanup pass to ensure that the tree is in a // good state. The semantic cleanup passes may have introduced new nodes with elastic trivia that have to be // cleaned. - var solution4 = await RunParallelCleanupPassAsync( - solution3, documentIdsAndOptions, progress, + var solution5 = await RunParallelCleanupPassAsync( + solution4, documentIdsAndOptions, progress, static async (document, options, cancellationToken) => await CleanupSyntaxAsync(document, options, cancellationToken).ConfigureAwait(false), cancellationToken).ConfigureAwait(false); From 5f3d8e4f135fa6e61b7844e7f7662f3c36c73c8e Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Wed, 8 May 2024 12:39:49 -0700 Subject: [PATCH 25/32] Proper nrt --- src/Workspaces/Core/Portable/CodeActions/CodeAction.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs b/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs index dd88f934a3402..75b27082fe312 100644 --- a/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs +++ b/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs @@ -427,10 +427,10 @@ internal Task GetChangedDocumentInternalAsync(CancellationToken cancel #pragma warning disable CA1822 // Mark members as static. This is a public API. protected Task> PostProcessAsync(IEnumerable operations, CancellationToken cancellationToken) #pragma warning restore CA1822 // Mark members as static - => PostProcessAsync(originalSolution: null!, operations, cancellationToken); + => PostProcessAsync(originalSolution: null, operations, cancellationToken); internal static async Task> PostProcessAsync( - Solution originalSolution, IEnumerable operations, CancellationToken cancellationToken) + Solution? originalSolution, IEnumerable operations, CancellationToken cancellationToken) { using var result = TemporaryArray.Empty; @@ -457,10 +457,10 @@ internal static async Task> PostProcessAsync #pragma warning disable CA1822 // Mark members as static. This is a public API. protected Task PostProcessChangesAsync(Solution changedSolution, CancellationToken cancellationToken) #pragma warning restore CA1822 // Mark members as static - => PostProcessChangesAsync(originalSolution: null!, changedSolution, cancellationToken); + => PostProcessChangesAsync(originalSolution: null, changedSolution, cancellationToken); - internal static async Task PostProcessChangesAsync( - Solution originalSolution, + private static async Task PostProcessChangesAsync( + Solution? originalSolution, Solution changedSolution, CancellationToken cancellationToken) { From 6d439cc52e52fc091c4e20e459178a882eb386ac Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Wed, 8 May 2024 12:41:39 -0700 Subject: [PATCH 26/32] Make private --- src/Workspaces/Core/Portable/CodeActions/CodeAction.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs b/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs index 75b27082fe312..70fc130f4d0cd 100644 --- a/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs +++ b/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs @@ -572,7 +572,7 @@ private static async ValueTask CleanupSyntaxAsync(Document document, C return document2; } - internal static async Task CleanupSyntaxAndSemanticsAsync( + private static async Task CleanupSyntaxAndSemanticsAsync( Solution solution, ImmutableArray<(DocumentId documentId, CodeCleanupOptions options)> documentIdsAndOptions, IProgress progress, From 8a07c5f2840026584eaa0701676095f71509dcdf Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Wed, 8 May 2024 12:42:04 -0700 Subject: [PATCH 27/32] Docs --- src/Workspaces/Core/Portable/CodeActions/CodeAction.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs b/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs index 70fc130f4d0cd..8864dae8484b8 100644 --- a/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs +++ b/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs @@ -511,6 +511,7 @@ internal static async Task CleanSyntaxAndSemanticsAsync( { var document = changedSolution.GetRequiredDocument(documentId); + // Only care about documents that support syntax. Non-C#/VB files can't be cleaned. if (document.SupportsSyntaxTree) { var codeActionOptions = await document.GetCodeCleanupOptionsAsync(optionsProvider, cancellationToken).ConfigureAwait(false); From 316d4737d84c17ab5bb1782b2111f13f70e8e721 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Wed, 8 May 2024 12:45:44 -0700 Subject: [PATCH 28/32] Simplify --- .../Core/Portable/CodeActions/CodeAction.cs | 74 ++++++++----------- 1 file changed, 30 insertions(+), 44 deletions(-) diff --git a/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs b/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs index 8864dae8484b8..4198501c19c55 100644 --- a/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs +++ b/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs @@ -560,7 +560,7 @@ internal static async ValueTask CleanupDocumentAsync(Document document return cleanedSolution.GetRequiredDocument(document.Id); } - private static async ValueTask CleanupSyntaxAsync(Document document, CodeCleanupOptions options, CancellationToken cancellationToken) + private static async Task CleanupSyntaxAsync(Document document, CodeCleanupOptions options, CancellationToken cancellationToken) { if (!document.SupportsSyntaxTree) return document; @@ -579,50 +579,36 @@ private static async Task CleanupSyntaxAndSemanticsAsync( IProgress progress, CancellationToken cancellationToken) { - // We do this in 5 serialized passes. This allows us to process all documents in parallel, while only forking - // the solution 5 times *total* (instead of 5 times *per* document). - progress.AddItems(documentIdsAndOptions.Length * 5); - - // First, ensure that everything is formatted as the feature asked for. We want to do this prior to doing - // semantic cleanup as the semantic cleanup passes may end up making changes that end up dropping some of the - // formatting/elastic annotations that the feature wanted respected. - var solution1 = await RunParallelCleanupPassAsync( - solution, documentIdsAndOptions, progress, - static async (document, options, cancellationToken) => - await CleanupSyntaxAsync(document, options, cancellationToken).ConfigureAwait(false), - cancellationToken).ConfigureAwait(false); - - // Then add all missing imports to all changed documents. - var solution2 = await RunParallelCleanupPassAsync( - solution1, documentIdsAndOptions, progress, - static (document, options, cancellationToken) => - ImportAdder.AddImportsFromSymbolAnnotationAsync(document, Simplifier.AddImportsAnnotation, options.AddImportOptions, cancellationToken), - cancellationToken).ConfigureAwait(false); - - // Then simplify any expanded constructs. - var solution3 = await RunParallelCleanupPassAsync( - solution2, documentIdsAndOptions, progress, - static (document, options, cancellationToken) => - Simplifier.ReduceAsync(document, Simplifier.Annotation, options.SimplifierOptions, cancellationToken), - cancellationToken).ConfigureAwait(false); - - // The do any necessary case correction for VB files. - var solution4 = await RunParallelCleanupPassAsync( - solution3, documentIdsAndOptions, progress, - static (document, options, cancellationToken) => - CaseCorrector.CaseCorrectAsync(document, CaseCorrector.Annotation, cancellationToken), - cancellationToken).ConfigureAwait(false); - - // Finally, after doing the semantic cleanup, do another syntax cleanup pass to ensure that the tree is in a - // good state. The semantic cleanup passes may have introduced new nodes with elastic trivia that have to be - // cleaned. - var solution5 = await RunParallelCleanupPassAsync( - solution4, documentIdsAndOptions, progress, - static async (document, options, cancellationToken) => - await CleanupSyntaxAsync(document, options, cancellationToken).ConfigureAwait(false), - cancellationToken).ConfigureAwait(false); + // We do this in N serialized passes. This allows us to process all documents in parallel, while only forking + // the solution N times *total* (instead of N times *per* document). + + Func>[] cleanupPasses = [ + // First, ensure that everything is formatted as the feature asked for. We want to do this prior to doing + // semantic cleanup as the semantic cleanup passes may end up making changes that end up dropping some of + // the formatting/elastic annotations that the feature wanted respected. + static (document, options, cancellationToken) => CleanupSyntaxAsync(document, options, cancellationToken), + // Then add all missing imports to all changed documents. + static (document, options, cancellationToken) => ImportAdder.AddImportsFromSymbolAnnotationAsync(document, Simplifier.AddImportsAnnotation, options.AddImportOptions, cancellationToken), + // Then simplify any expanded constructs. + static (document, options, cancellationToken) => Simplifier.ReduceAsync(document, Simplifier.Annotation, options.SimplifierOptions, cancellationToken), + // The do any necessary case correction for VB files. + static (document, options, cancellationToken) => CaseCorrector.CaseCorrectAsync(document, CaseCorrector.Annotation, cancellationToken), + // Finally, after doing the semantic cleanup, do another syntax cleanup pass to ensure that the tree is in a + // good state. The semantic cleanup passes may have introduced new nodes with elastic trivia that have to be + // cleaned. + static (document, options, cancellationToken) => CleanupSyntaxAsync(document, options, cancellationToken), + ]; + + progress.AddItems(documentIdsAndOptions.Length * cleanupPasses.Length); + + var currentSolution = solution; + foreach (var cleanupPass in cleanupPasses) + { + currentSolution = await RunParallelCleanupPassAsync( + currentSolution, documentIdsAndOptions, progress, cleanupPass, cancellationToken).ConfigureAwait(false); + } - return solution4; + return currentSolution; } private static async Task RunParallelCleanupPassAsync( From 6b881cf529893027dcdf4f57e29782b1d861ed3d Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Wed, 8 May 2024 12:47:08 -0700 Subject: [PATCH 29/32] Simplify --- .../Core/Portable/CodeActions/CodeAction.cs | 47 ++++++++++--------- 1 file changed, 25 insertions(+), 22 deletions(-) diff --git a/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs b/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs index 4198501c19c55..3876f8de5f1b4 100644 --- a/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs +++ b/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs @@ -573,36 +573,39 @@ private static async Task CleanupSyntaxAsync(Document document, CodeCl return document2; } + /// + /// We do cleanup in N serialized passes. This allows us to process all documents in parallel, while only forking + /// the solution N times *total* (instead of N times *per* document). + /// + private static readonly ImmutableArray>> s_cleanupPasses = + [ + // First, ensure that everything is formatted as the feature asked for. We want to do this prior to doing + // semantic cleanup as the semantic cleanup passes may end up making changes that end up dropping some of + // the formatting/elastic annotations that the feature wanted respected. + static (document, options, cancellationToken) => CleanupSyntaxAsync(document, options, cancellationToken), + // Then add all missing imports to all changed documents. + static (document, options, cancellationToken) => ImportAdder.AddImportsFromSymbolAnnotationAsync(document, Simplifier.AddImportsAnnotation, options.AddImportOptions, cancellationToken), + // Then simplify any expanded constructs. + static (document, options, cancellationToken) => Simplifier.ReduceAsync(document, Simplifier.Annotation, options.SimplifierOptions, cancellationToken), + // The do any necessary case correction for VB files. + static (document, options, cancellationToken) => CaseCorrector.CaseCorrectAsync(document, CaseCorrector.Annotation, cancellationToken), + // Finally, after doing the semantic cleanup, do another syntax cleanup pass to ensure that the tree is in a + // good state. The semantic cleanup passes may have introduced new nodes with elastic trivia that have to be + // cleaned. + static (document, options, cancellationToken) => CleanupSyntaxAsync(document, options, cancellationToken), + ]; + private static async Task CleanupSyntaxAndSemanticsAsync( Solution solution, ImmutableArray<(DocumentId documentId, CodeCleanupOptions options)> documentIdsAndOptions, IProgress progress, CancellationToken cancellationToken) { - // We do this in N serialized passes. This allows us to process all documents in parallel, while only forking - // the solution N times *total* (instead of N times *per* document). - - Func>[] cleanupPasses = [ - // First, ensure that everything is formatted as the feature asked for. We want to do this prior to doing - // semantic cleanup as the semantic cleanup passes may end up making changes that end up dropping some of - // the formatting/elastic annotations that the feature wanted respected. - static (document, options, cancellationToken) => CleanupSyntaxAsync(document, options, cancellationToken), - // Then add all missing imports to all changed documents. - static (document, options, cancellationToken) => ImportAdder.AddImportsFromSymbolAnnotationAsync(document, Simplifier.AddImportsAnnotation, options.AddImportOptions, cancellationToken), - // Then simplify any expanded constructs. - static (document, options, cancellationToken) => Simplifier.ReduceAsync(document, Simplifier.Annotation, options.SimplifierOptions, cancellationToken), - // The do any necessary case correction for VB files. - static (document, options, cancellationToken) => CaseCorrector.CaseCorrectAsync(document, CaseCorrector.Annotation, cancellationToken), - // Finally, after doing the semantic cleanup, do another syntax cleanup pass to ensure that the tree is in a - // good state. The semantic cleanup passes may have introduced new nodes with elastic trivia that have to be - // cleaned. - static (document, options, cancellationToken) => CleanupSyntaxAsync(document, options, cancellationToken), - ]; - - progress.AddItems(documentIdsAndOptions.Length * cleanupPasses.Length); + // One item per document per cleanup pass. + progress.AddItems(documentIdsAndOptions.Length * s_cleanupPasses.Length); var currentSolution = solution; - foreach (var cleanupPass in cleanupPasses) + foreach (var cleanupPass in s_cleanupPasses) { currentSolution = await RunParallelCleanupPassAsync( currentSolution, documentIdsAndOptions, progress, cleanupPass, cancellationToken).ConfigureAwait(false); From af1bd473ff0d879b0cc7520045f6e86ec5861a73 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Wed, 8 May 2024 12:50:49 -0700 Subject: [PATCH 30/32] Break out file --- .../Core/Portable/CodeActions/CodeAction.cs | 199 +----------------- .../CodeActions/CodeAction_Cleanup.cs | 188 +++++++++++++++++ 2 files changed, 190 insertions(+), 197 deletions(-) create mode 100644 src/Workspaces/Core/Portable/CodeActions/CodeAction_Cleanup.cs diff --git a/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs b/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs index 3876f8de5f1b4..1e20c922bbed5 100644 --- a/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs +++ b/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs @@ -8,27 +8,18 @@ using System.ComponentModel; using System.Diagnostics; using System.Diagnostics.CodeAnalysis; -using System.Linq; using System.Threading; using System.Threading.Tasks; -using Microsoft.CodeAnalysis.CaseCorrection; -using Microsoft.CodeAnalysis.CodeCleanup; using Microsoft.CodeAnalysis.CodeFixes; using Microsoft.CodeAnalysis.CodeRefactorings; -using Microsoft.CodeAnalysis.Editing; using Microsoft.CodeAnalysis.Formatting; using Microsoft.CodeAnalysis.Host.Mef; using Microsoft.CodeAnalysis.Internal.Log; using Microsoft.CodeAnalysis.Options; -using Microsoft.CodeAnalysis.PooledObjects; -using Microsoft.CodeAnalysis.Remote; using Microsoft.CodeAnalysis.Shared.Collections; using Microsoft.CodeAnalysis.Shared.Extensions; -using Microsoft.CodeAnalysis.Shared.Utilities; -using Microsoft.CodeAnalysis.Simplification; using Microsoft.CodeAnalysis.Tags; using Microsoft.CodeAnalysis.Telemetry; -using Microsoft.CodeAnalysis.Text; using Roslyn.Utilities; namespace Microsoft.CodeAnalysis.CodeActions; @@ -36,7 +27,7 @@ namespace Microsoft.CodeAnalysis.CodeActions; /// /// An action produced by a or a . /// -public abstract class CodeAction +public abstract partial class CodeAction { private static readonly Dictionary s_isNonProgressGetChangedSolutionAsyncOverridden = []; private static readonly Dictionary s_isNonProgressComputeOperationsAsyncOverridden = []; @@ -457,7 +448,7 @@ internal static async Task> PostProcessAsync #pragma warning disable CA1822 // Mark members as static. This is a public API. protected Task PostProcessChangesAsync(Solution changedSolution, CancellationToken cancellationToken) #pragma warning restore CA1822 // Mark members as static - => PostProcessChangesAsync(originalSolution: null, changedSolution, cancellationToken); + => PostProcessChangesAsync(originalSolution: null, changedSolution, cancellationToken); private static async Task PostProcessChangesAsync( Solution? originalSolution, @@ -475,192 +466,6 @@ private static async Task PostProcessChangesAsync( return await CleanSyntaxAndSemanticsAsync(originalSolution, changedSolution, fallbackOptions, CodeAnalysisProgress.None, cancellationToken).ConfigureAwait(false); } - internal static ImmutableArray GetAllChangedOrAddedDocumentIds( - Solution originalSolution, - Solution changedSolution) - { - var solutionChanges = changedSolution.GetChanges(originalSolution); - var documentIds = solutionChanges - .GetProjectChanges() - .SelectMany(p => p.GetChangedDocuments(onlyGetDocumentsWithTextChanges: true).Concat(p.GetAddedDocuments())) - .Concat(solutionChanges.GetAddedProjects().SelectMany(p => p.DocumentIds)) - .ToImmutableArray(); - return documentIds; - } - - internal static async Task CleanSyntaxAndSemanticsAsync( - Solution originalSolution, - Solution changedSolution, - CodeCleanupOptionsProvider optionsProvider, - IProgress progress, - CancellationToken cancellationToken) - { - var documentIds = GetAllChangedOrAddedDocumentIds(originalSolution, changedSolution); - var documentIdsAndOptionsToClean = await GetDocumentIdsAndOptionsToCleanAsync().ConfigureAwait(false); - - // Then do a pass where we cleanup semantics. - var cleanedSolution = await CleanupSyntaxAndSemanticsAsync( - changedSolution, documentIdsAndOptionsToClean, progress, cancellationToken).ConfigureAwait(false); - - return cleanedSolution; - - async Task> GetDocumentIdsAndOptionsToCleanAsync() - { - using var _ = ArrayBuilder<(DocumentId documentId, CodeCleanupOptions options)>.GetInstance(documentIds.Length, out var documentIdsAndOptions); - foreach (var documentId in documentIds) - { - var document = changedSolution.GetRequiredDocument(documentId); - - // Only care about documents that support syntax. Non-C#/VB files can't be cleaned. - if (document.SupportsSyntaxTree) - { - var codeActionOptions = await document.GetCodeCleanupOptionsAsync(optionsProvider, cancellationToken).ConfigureAwait(false); - documentIdsAndOptions.Add((documentId, codeActionOptions)); - } - } - - return documentIdsAndOptions.ToImmutableAndClear(); - } - } - - /// - /// Apply post processing steps to a single document: - /// Reducing nodes annotated with - /// Formatting nodes annotated with - /// - /// The document changed by the . - /// A cancellation token. - /// A document with the post processing changes applied. - protected virtual async Task PostProcessChangesAsync(Document document, CancellationToken cancellationToken) - { - if (document.SupportsSyntaxTree) - { - // TODO: avoid ILegacyGlobalCodeActionOptionsWorkspaceService https://github.com/dotnet/roslyn/issues/60777 - var globalOptions = document.Project.Solution.Services.GetService(); - var fallbackOptions = globalOptions?.Provider ?? CodeActionOptions.DefaultProvider; - - var options = await document.GetCodeCleanupOptionsAsync(fallbackOptions, cancellationToken).ConfigureAwait(false); - return await CleanupDocumentAsync(document, options, cancellationToken).ConfigureAwait(false); - } - - return document; - } - - internal static async ValueTask CleanupDocumentAsync(Document document, CodeCleanupOptions options, CancellationToken cancellationToken) - { - if (!document.SupportsSyntaxTree) - return document; - - var cleanedSolution = await CleanupSyntaxAndSemanticsAsync( - document.Project.Solution, - [(document.Id, options)], - CodeAnalysisProgress.None, - cancellationToken).ConfigureAwait(false); - - return cleanedSolution.GetRequiredDocument(document.Id); - } - - private static async Task CleanupSyntaxAsync(Document document, CodeCleanupOptions options, CancellationToken cancellationToken) - { - if (!document.SupportsSyntaxTree) - return document; - - // format any node with explicit formatter annotation - var document1 = await Formatter.FormatAsync(document, Formatter.Annotation, options.FormattingOptions, cancellationToken).ConfigureAwait(false); - - // format any elastic whitespace - var document2 = await Formatter.FormatAsync(document1, SyntaxAnnotation.ElasticAnnotation, options.FormattingOptions, cancellationToken).ConfigureAwait(false); - return document2; - } - - /// - /// We do cleanup in N serialized passes. This allows us to process all documents in parallel, while only forking - /// the solution N times *total* (instead of N times *per* document). - /// - private static readonly ImmutableArray>> s_cleanupPasses = - [ - // First, ensure that everything is formatted as the feature asked for. We want to do this prior to doing - // semantic cleanup as the semantic cleanup passes may end up making changes that end up dropping some of - // the formatting/elastic annotations that the feature wanted respected. - static (document, options, cancellationToken) => CleanupSyntaxAsync(document, options, cancellationToken), - // Then add all missing imports to all changed documents. - static (document, options, cancellationToken) => ImportAdder.AddImportsFromSymbolAnnotationAsync(document, Simplifier.AddImportsAnnotation, options.AddImportOptions, cancellationToken), - // Then simplify any expanded constructs. - static (document, options, cancellationToken) => Simplifier.ReduceAsync(document, Simplifier.Annotation, options.SimplifierOptions, cancellationToken), - // The do any necessary case correction for VB files. - static (document, options, cancellationToken) => CaseCorrector.CaseCorrectAsync(document, CaseCorrector.Annotation, cancellationToken), - // Finally, after doing the semantic cleanup, do another syntax cleanup pass to ensure that the tree is in a - // good state. The semantic cleanup passes may have introduced new nodes with elastic trivia that have to be - // cleaned. - static (document, options, cancellationToken) => CleanupSyntaxAsync(document, options, cancellationToken), - ]; - - private static async Task CleanupSyntaxAndSemanticsAsync( - Solution solution, - ImmutableArray<(DocumentId documentId, CodeCleanupOptions options)> documentIdsAndOptions, - IProgress progress, - CancellationToken cancellationToken) - { - // One item per document per cleanup pass. - progress.AddItems(documentIdsAndOptions.Length * s_cleanupPasses.Length); - - var currentSolution = solution; - foreach (var cleanupPass in s_cleanupPasses) - { - currentSolution = await RunParallelCleanupPassAsync( - currentSolution, documentIdsAndOptions, progress, cleanupPass, cancellationToken).ConfigureAwait(false); - } - - return currentSolution; - } - - private static async Task RunParallelCleanupPassAsync( - Solution solution, - ImmutableArray<(DocumentId documentId, CodeCleanupOptions options)> documentIdsAndOptions, - IProgress progress, - Func> cleanupDocumentAsync, - CancellationToken cancellationToken) - { - // We're about to making a ton of calls to this new solution, including expensive oop calls to get up to - // date compilations, skeletons and SG docs. Create and pin this solution so that all remote calls operate - // on the same fork and do not cause the forked solution to be created and dropped repeatedly. - using var _ = await RemoteKeepAliveSession.CreateAsync(solution, cancellationToken).ConfigureAwait(false); - - return await ProducerConsumer<(DocumentId documentId, SyntaxNode newRoot)>.RunParallelAsync( - source: documentIdsAndOptions, - produceItems: static async (documentIdAndOptions, callback, args, cancellationToken) => - { - // As we finish each document, update our progress. - using var _ = args.progress.ItemCompletedScope(); - - var (documentId, options) = documentIdAndOptions; - - // Fetch the current state of the document from this fork of the solution. - var document = args.solution.GetRequiredDocument(documentId); - Contract.ThrowIfFalse(document.SupportsSyntaxTree, "GetDocumentIdsAndOptionsAsync should only be returning documents that support syntax"); - - // Now, perform the requested cleanup pass on it. - var cleanedDocument = await args.cleanupDocumentAsync(document, options, cancellationToken).ConfigureAwait(false); - if (cleanedDocument is null || cleanedDocument == document) - return; - - // Now get the cleaned root and pass it back to the consumer. - var newRoot = await cleanedDocument.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false); - callback((documentId, newRoot)); - }, - consumeItems: static async (stream, args, cancellationToken) => - { - // Grab all the cleaned roots and produce the new solution snapshot from that. - var currentSolution = args.solution; - await foreach (var (documentId, newRoot) in stream) - currentSolution = currentSolution.WithDocumentSyntaxRoot(documentId, newRoot); - - return currentSolution; - }, - args: (solution, progress, cleanupDocumentAsync), - cancellationToken).ConfigureAwait(false); - } - #region Factories for standard code actions /// diff --git a/src/Workspaces/Core/Portable/CodeActions/CodeAction_Cleanup.cs b/src/Workspaces/Core/Portable/CodeActions/CodeAction_Cleanup.cs new file mode 100644 index 0000000000000..8fcb2aaab9835 --- /dev/null +++ b/src/Workspaces/Core/Portable/CodeActions/CodeAction_Cleanup.cs @@ -0,0 +1,188 @@ +// 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; +using System.Collections.Immutable; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.CodeAnalysis.CaseCorrection; +using Microsoft.CodeAnalysis.CodeCleanup; +using Microsoft.CodeAnalysis.Editing; +using Microsoft.CodeAnalysis.Formatting; +using Microsoft.CodeAnalysis.Options; +using Microsoft.CodeAnalysis.PooledObjects; +using Microsoft.CodeAnalysis.Remote; +using Microsoft.CodeAnalysis.Shared.Extensions; +using Microsoft.CodeAnalysis.Shared.Utilities; +using Microsoft.CodeAnalysis.Simplification; +using Roslyn.Utilities; + +namespace Microsoft.CodeAnalysis.CodeActions; + +public abstract partial class CodeAction +{ + /// + /// We do cleanup in N serialized passes. This allows us to process all documents in parallel, while only forking + /// the solution N times *total* (instead of N times *per* document). + /// + private static readonly ImmutableArray>> s_cleanupPasses = + [ + // First, ensure that everything is formatted as the feature asked for. We want to do this prior to doing + // semantic cleanup as the semantic cleanup passes may end up making changes that end up dropping some of + // the formatting/elastic annotations that the feature wanted respected. + static (document, options, cancellationToken) => CleanupSyntaxAsync(document, options, cancellationToken), + // Then add all missing imports to all changed documents. + static (document, options, cancellationToken) => ImportAdder.AddImportsFromSymbolAnnotationAsync(document, Simplifier.AddImportsAnnotation, options.AddImportOptions, cancellationToken), + // Then simplify any expanded constructs. + static (document, options, cancellationToken) => Simplifier.ReduceAsync(document, Simplifier.Annotation, options.SimplifierOptions, cancellationToken), + // The do any necessary case correction for VB files. + static (document, options, cancellationToken) => CaseCorrector.CaseCorrectAsync(document, CaseCorrector.Annotation, cancellationToken), + // Finally, after doing the semantic cleanup, do another syntax cleanup pass to ensure that the tree is in a + // good state. The semantic cleanup passes may have introduced new nodes with elastic trivia that have to be + // cleaned. + static (document, options, cancellationToken) => CleanupSyntaxAsync(document, options, cancellationToken), + ]; + + internal static ImmutableArray GetAllChangedOrAddedDocumentIds( + Solution originalSolution, + Solution changedSolution) + { + var solutionChanges = changedSolution.GetChanges(originalSolution); + var documentIds = solutionChanges + .GetProjectChanges() + .SelectMany(p => p.GetChangedDocuments(onlyGetDocumentsWithTextChanges: true).Concat(p.GetAddedDocuments())) + .Concat(solutionChanges.GetAddedProjects().SelectMany(p => p.DocumentIds)) + .ToImmutableArray(); + return documentIds; + } + + internal static async Task CleanSyntaxAndSemanticsAsync( + Solution originalSolution, + Solution changedSolution, + CodeCleanupOptionsProvider optionsProvider, + IProgress progress, + CancellationToken cancellationToken) + { + var documentIds = GetAllChangedOrAddedDocumentIds(originalSolution, changedSolution); + var documentIdsAndOptionsToClean = await GetDocumentIdsAndOptionsToCleanAsync().ConfigureAwait(false); + + // Then do a pass where we cleanup semantics. + var cleanedSolution = await CleanupSyntaxAndSemanticsAsync( + changedSolution, documentIdsAndOptionsToClean, progress, cancellationToken).ConfigureAwait(false); + + return cleanedSolution; + + async Task> GetDocumentIdsAndOptionsToCleanAsync() + { + using var _ = ArrayBuilder<(DocumentId documentId, CodeCleanupOptions options)>.GetInstance(documentIds.Length, out var documentIdsAndOptions); + foreach (var documentId in documentIds) + { + var document = changedSolution.GetRequiredDocument(documentId); + + // Only care about documents that support syntax. Non-C#/VB files can't be cleaned. + if (document.SupportsSyntaxTree) + { + var codeActionOptions = await document.GetCodeCleanupOptionsAsync(optionsProvider, cancellationToken).ConfigureAwait(false); + documentIdsAndOptions.Add((documentId, codeActionOptions)); + } + } + + return documentIdsAndOptions.ToImmutableAndClear(); + } + } + + internal static async ValueTask CleanupDocumentAsync(Document document, CodeCleanupOptions options, CancellationToken cancellationToken) + { + if (!document.SupportsSyntaxTree) + return document; + + var cleanedSolution = await CleanupSyntaxAndSemanticsAsync( + document.Project.Solution, + [(document.Id, options)], + CodeAnalysisProgress.None, + cancellationToken).ConfigureAwait(false); + + return cleanedSolution.GetRequiredDocument(document.Id); + } + + private static async Task CleanupSyntaxAsync(Document document, CodeCleanupOptions options, CancellationToken cancellationToken) + { + if (!document.SupportsSyntaxTree) + return document; + + // format any node with explicit formatter annotation + var document1 = await Formatter.FormatAsync(document, Formatter.Annotation, options.FormattingOptions, cancellationToken).ConfigureAwait(false); + + // format any elastic whitespace + var document2 = await Formatter.FormatAsync(document1, SyntaxAnnotation.ElasticAnnotation, options.FormattingOptions, cancellationToken).ConfigureAwait(false); + return document2; + } + + private static async Task CleanupSyntaxAndSemanticsAsync( + Solution solution, + ImmutableArray<(DocumentId documentId, CodeCleanupOptions options)> documentIdsAndOptions, + IProgress progress, + CancellationToken cancellationToken) + { + // One item per document per cleanup pass. + progress.AddItems(documentIdsAndOptions.Length * s_cleanupPasses.Length); + + var currentSolution = solution; + foreach (var cleanupPass in s_cleanupPasses) + { + currentSolution = await RunParallelCleanupPassAsync( + currentSolution, documentIdsAndOptions, progress, cleanupPass, cancellationToken).ConfigureAwait(false); + } + + return currentSolution; + } + + private static async Task RunParallelCleanupPassAsync( + Solution solution, + ImmutableArray<(DocumentId documentId, CodeCleanupOptions options)> documentIdsAndOptions, + IProgress progress, + Func> cleanupDocumentAsync, + CancellationToken cancellationToken) + { + // We're about to making a ton of calls to this new solution, including expensive oop calls to get up to + // date compilations, skeletons and SG docs. Create and pin this solution so that all remote calls operate + // on the same fork and do not cause the forked solution to be created and dropped repeatedly. + using var _ = await RemoteKeepAliveSession.CreateAsync(solution, cancellationToken).ConfigureAwait(false); + + return await ProducerConsumer<(DocumentId documentId, SyntaxNode newRoot)>.RunParallelAsync( + source: documentIdsAndOptions, + produceItems: static async (documentIdAndOptions, callback, args, cancellationToken) => + { + // As we finish each document, update our progress. + using var _ = args.progress.ItemCompletedScope(); + + var (documentId, options) = documentIdAndOptions; + + // Fetch the current state of the document from this fork of the solution. + var document = args.solution.GetRequiredDocument(documentId); + Contract.ThrowIfFalse(document.SupportsSyntaxTree, "GetDocumentIdsAndOptionsAsync should only be returning documents that support syntax"); + + // Now, perform the requested cleanup pass on it. + var cleanedDocument = await args.cleanupDocumentAsync(document, options, cancellationToken).ConfigureAwait(false); + if (cleanedDocument is null || cleanedDocument == document) + return; + + // Now get the cleaned root and pass it back to the consumer. + var newRoot = await cleanedDocument.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false); + callback((documentId, newRoot)); + }, + consumeItems: static async (stream, args, cancellationToken) => + { + // Grab all the cleaned roots and produce the new solution snapshot from that. + var currentSolution = args.solution; + await foreach (var (documentId, newRoot) in stream) + currentSolution = currentSolution.WithDocumentSyntaxRoot(documentId, newRoot); + + return currentSolution; + }, + args: (solution, progress, cleanupDocumentAsync), + cancellationToken).ConfigureAwait(false); + } +} From d056c8b3edae5dc42c998676f4d3f0bbec7e4659 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Wed, 8 May 2024 12:52:07 -0700 Subject: [PATCH 31/32] Move back --- .../Core/Portable/CodeActions/CodeAction.cs | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs b/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs index 1e20c922bbed5..dac6f076718ea 100644 --- a/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs +++ b/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs @@ -10,6 +10,7 @@ using System.Diagnostics.CodeAnalysis; using System.Threading; using System.Threading.Tasks; +using Microsoft.CodeAnalysis.CodeCleanup; using Microsoft.CodeAnalysis.CodeFixes; using Microsoft.CodeAnalysis.CodeRefactorings; using Microsoft.CodeAnalysis.Formatting; @@ -18,6 +19,7 @@ using Microsoft.CodeAnalysis.Options; using Microsoft.CodeAnalysis.Shared.Collections; using Microsoft.CodeAnalysis.Shared.Extensions; +using Microsoft.CodeAnalysis.Simplification; using Microsoft.CodeAnalysis.Tags; using Microsoft.CodeAnalysis.Telemetry; using Roslyn.Utilities; @@ -466,6 +468,29 @@ private static async Task PostProcessChangesAsync( return await CleanSyntaxAndSemanticsAsync(originalSolution, changedSolution, fallbackOptions, CodeAnalysisProgress.None, cancellationToken).ConfigureAwait(false); } + /// + /// Apply post processing steps to a single document: + /// Reducing nodes annotated with + /// Formatting nodes annotated with + /// + /// The document changed by the . + /// A cancellation token. + /// A document with the post processing changes applied. + protected virtual async Task PostProcessChangesAsync(Document document, CancellationToken cancellationToken) + { + if (document.SupportsSyntaxTree) + { + // TODO: avoid ILegacyGlobalCodeActionOptionsWorkspaceService https://github.com/dotnet/roslyn/issues/60777 + var globalOptions = document.Project.Solution.Services.GetService(); + var fallbackOptions = globalOptions?.Provider ?? CodeActionOptions.DefaultProvider; + + var options = await document.GetCodeCleanupOptionsAsync(fallbackOptions, cancellationToken).ConfigureAwait(false); + return await CleanupDocumentAsync(document, options, cancellationToken).ConfigureAwait(false); + } + + return document; + } + #region Factories for standard code actions /// From de6a2bbf01c0d042d90aea75367fd8cb5e878a4c Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Wed, 8 May 2024 12:56:06 -0700 Subject: [PATCH 32/32] inline --- .../CodeActions/CodeAction_Cleanup.cs | 116 ++++++++---------- 1 file changed, 54 insertions(+), 62 deletions(-) diff --git a/src/Workspaces/Core/Portable/CodeActions/CodeAction_Cleanup.cs b/src/Workspaces/Core/Portable/CodeActions/CodeAction_Cleanup.cs index 8fcb2aaab9835..18ca9283f4f0c 100644 --- a/src/Workspaces/Core/Portable/CodeActions/CodeAction_Cleanup.cs +++ b/src/Workspaces/Core/Portable/CodeActions/CodeAction_Cleanup.cs @@ -45,6 +45,18 @@ public abstract partial class CodeAction static (document, options, cancellationToken) => CleanupSyntaxAsync(document, options, cancellationToken), ]; + private static async Task CleanupSyntaxAsync(Document document, CodeCleanupOptions options, CancellationToken cancellationToken) + { + Contract.ThrowIfFalse(document.SupportsSyntaxTree); + + // format any node with explicit formatter annotation + var document1 = await Formatter.FormatAsync(document, Formatter.Annotation, options.FormattingOptions, cancellationToken).ConfigureAwait(false); + + // format any elastic whitespace + var document2 = await Formatter.FormatAsync(document1, SyntaxAnnotation.ElasticAnnotation, options.FormattingOptions, cancellationToken).ConfigureAwait(false); + return document2; + } + internal static ImmutableArray GetAllChangedOrAddedDocumentIds( Solution originalSolution, Solution changedSolution) @@ -69,7 +81,7 @@ internal static async Task CleanSyntaxAndSemanticsAsync( var documentIdsAndOptionsToClean = await GetDocumentIdsAndOptionsToCleanAsync().ConfigureAwait(false); // Then do a pass where we cleanup semantics. - var cleanedSolution = await CleanupSyntaxAndSemanticsAsync( + var cleanedSolution = await RunAllCleanupPassesInOrderAsync( changedSolution, documentIdsAndOptionsToClean, progress, cancellationToken).ConfigureAwait(false); return cleanedSolution; @@ -98,7 +110,7 @@ internal static async ValueTask CleanupDocumentAsync(Document document if (!document.SupportsSyntaxTree) return document; - var cleanedSolution = await CleanupSyntaxAndSemanticsAsync( + var cleanedSolution = await RunAllCleanupPassesInOrderAsync( document.Project.Solution, [(document.Id, options)], CodeAnalysisProgress.None, @@ -107,20 +119,7 @@ internal static async ValueTask CleanupDocumentAsync(Document document return cleanedSolution.GetRequiredDocument(document.Id); } - private static async Task CleanupSyntaxAsync(Document document, CodeCleanupOptions options, CancellationToken cancellationToken) - { - if (!document.SupportsSyntaxTree) - return document; - - // format any node with explicit formatter annotation - var document1 = await Formatter.FormatAsync(document, Formatter.Annotation, options.FormattingOptions, cancellationToken).ConfigureAwait(false); - - // format any elastic whitespace - var document2 = await Formatter.FormatAsync(document1, SyntaxAnnotation.ElasticAnnotation, options.FormattingOptions, cancellationToken).ConfigureAwait(false); - return document2; - } - - private static async Task CleanupSyntaxAndSemanticsAsync( + private static async Task RunAllCleanupPassesInOrderAsync( Solution solution, ImmutableArray<(DocumentId documentId, CodeCleanupOptions options)> documentIdsAndOptions, IProgress progress, @@ -131,58 +130,51 @@ private static async Task CleanupSyntaxAndSemanticsAsync( var currentSolution = solution; foreach (var cleanupPass in s_cleanupPasses) - { - currentSolution = await RunParallelCleanupPassAsync( - currentSolution, documentIdsAndOptions, progress, cleanupPass, cancellationToken).ConfigureAwait(false); - } + currentSolution = await RunParallelCleanupPassAsync(currentSolution, cleanupPass).ConfigureAwait(false); return currentSolution; - } - private static async Task RunParallelCleanupPassAsync( - Solution solution, - ImmutableArray<(DocumentId documentId, CodeCleanupOptions options)> documentIdsAndOptions, - IProgress progress, - Func> cleanupDocumentAsync, - CancellationToken cancellationToken) - { - // We're about to making a ton of calls to this new solution, including expensive oop calls to get up to - // date compilations, skeletons and SG docs. Create and pin this solution so that all remote calls operate - // on the same fork and do not cause the forked solution to be created and dropped repeatedly. - using var _ = await RemoteKeepAliveSession.CreateAsync(solution, cancellationToken).ConfigureAwait(false); - - return await ProducerConsumer<(DocumentId documentId, SyntaxNode newRoot)>.RunParallelAsync( - source: documentIdsAndOptions, - produceItems: static async (documentIdAndOptions, callback, args, cancellationToken) => - { - // As we finish each document, update our progress. - using var _ = args.progress.ItemCompletedScope(); + async Task RunParallelCleanupPassAsync( + Solution solution, Func> cleanupDocumentAsync) + { + // We're about to making a ton of calls to this new solution, including expensive oop calls to get up to + // date compilations, skeletons and SG docs. Create and pin this solution so that all remote calls operate + // on the same fork and do not cause the forked solution to be created and dropped repeatedly. + using var _ = await RemoteKeepAliveSession.CreateAsync(solution, cancellationToken).ConfigureAwait(false); + + return await ProducerConsumer<(DocumentId documentId, SyntaxNode newRoot)>.RunParallelAsync( + source: documentIdsAndOptions, + produceItems: static async (documentIdAndOptions, callback, args, cancellationToken) => + { + // As we finish each document, update our progress. + using var _ = args.progress.ItemCompletedScope(); - var (documentId, options) = documentIdAndOptions; + var (documentId, options) = documentIdAndOptions; - // Fetch the current state of the document from this fork of the solution. - var document = args.solution.GetRequiredDocument(documentId); - Contract.ThrowIfFalse(document.SupportsSyntaxTree, "GetDocumentIdsAndOptionsAsync should only be returning documents that support syntax"); + // Fetch the current state of the document from this fork of the solution. + var document = args.solution.GetRequiredDocument(documentId); + Contract.ThrowIfFalse(document.SupportsSyntaxTree, "GetDocumentIdsAndOptionsAsync should only be returning documents that support syntax"); - // Now, perform the requested cleanup pass on it. - var cleanedDocument = await args.cleanupDocumentAsync(document, options, cancellationToken).ConfigureAwait(false); - if (cleanedDocument is null || cleanedDocument == document) - return; + // Now, perform the requested cleanup pass on it. + var cleanedDocument = await args.cleanupDocumentAsync(document, options, cancellationToken).ConfigureAwait(false); + if (cleanedDocument is null || cleanedDocument == document) + return; - // Now get the cleaned root and pass it back to the consumer. - var newRoot = await cleanedDocument.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false); - callback((documentId, newRoot)); - }, - consumeItems: static async (stream, args, cancellationToken) => - { - // Grab all the cleaned roots and produce the new solution snapshot from that. - var currentSolution = args.solution; - await foreach (var (documentId, newRoot) in stream) - currentSolution = currentSolution.WithDocumentSyntaxRoot(documentId, newRoot); - - return currentSolution; - }, - args: (solution, progress, cleanupDocumentAsync), - cancellationToken).ConfigureAwait(false); + // Now get the cleaned root and pass it back to the consumer. + var newRoot = await cleanedDocument.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false); + callback((documentId, newRoot)); + }, + consumeItems: static async (stream, args, cancellationToken) => + { + // Grab all the cleaned roots and produce the new solution snapshot from that. + var currentSolution = args.solution; + await foreach (var (documentId, newRoot) in stream) + currentSolution = currentSolution.WithDocumentSyntaxRoot(documentId, newRoot); + + return currentSolution; + }, + args: (solution, progress, cleanupDocumentAsync), + cancellationToken).ConfigureAwait(false); + } } }