Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Run phases of code cleanup in parallel #73386

Merged
merged 36 commits into from
May 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
389c000
Run phases of code cleanup in parallel
CyrusNajmabadi May 8, 2024
07c8ae7
Merge branch 'cleanupSyntax' into cleanupParallel
CyrusNajmabadi May 8, 2024
73fe5f3
Merge branch 'main' into cleanupParallel
CyrusNajmabadi May 8, 2024
a5ee414
Use helper
CyrusNajmabadi May 8, 2024
688fa6f
More parallel
CyrusNajmabadi May 8, 2024
e42a945
Add helper
CyrusNajmabadi May 8, 2024
57e9943
Share code
CyrusNajmabadi May 8, 2024
5dfe9ee
Merge remote-tracking branch 'upstream/main' into cleanupParallel
CyrusNajmabadi May 8, 2024
b77e7d9
Fixup feature tests
CyrusNajmabadi May 8, 2024
7de9185
Simplify
CyrusNajmabadi May 8, 2024
d1437df
Fix issue with improper formatting in introduce-variable
CyrusNajmabadi May 8, 2024
6973de7
Fix test
CyrusNajmabadi May 8, 2024
964b677
Merge branch 'introVarFormatting' into cleanupParallel
CyrusNajmabadi May 8, 2024
333e6df
Cleanup
CyrusNajmabadi May 8, 2024
88ed323
inline method
CyrusNajmabadi May 8, 2024
d10a8c1
Don't root documents
CyrusNajmabadi May 8, 2024
efb7a2b
Fixes
CyrusNajmabadi May 8, 2024
6e78281
Simplify
CyrusNajmabadi May 8, 2024
0f6439c
Only process roslyn docs with trees
CyrusNajmabadi May 8, 2024
c76dd4e
Simplify
CyrusNajmabadi May 8, 2024
4b47c49
Make helper
CyrusNajmabadi May 8, 2024
a203ee6
Share code
CyrusNajmabadi May 8, 2024
bb28d31
cleanup
CyrusNajmabadi May 8, 2024
5b01471
in progress
CyrusNajmabadi May 8, 2024
6d52ccb
Simplify
CyrusNajmabadi May 8, 2024
cdf1375
Cleanup
CyrusNajmabadi May 8, 2024
2d535e6
No need for check
CyrusNajmabadi May 8, 2024
334606e
Simplify
CyrusNajmabadi May 8, 2024
5f3d8e4
Proper nrt
CyrusNajmabadi May 8, 2024
6d439cc
Make private
CyrusNajmabadi May 8, 2024
8a07c5f
Docs
CyrusNajmabadi May 8, 2024
316d473
Simplify
CyrusNajmabadi May 8, 2024
6b881cf
Simplify
CyrusNajmabadi May 8, 2024
af1bd47
Break out file
CyrusNajmabadi May 8, 2024
d056c8b
Move back
CyrusNajmabadi May 8, 2024
de6a2bb
inline
CyrusNajmabadi May 8, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
""";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ namespace Microsoft.CodeAnalysis.IntroduceVariable;

internal partial class AbstractIntroduceVariableService<TService, TExpressionSyntax, TTypeSyntax, TTypeDeclarationSyntax, TQueryExpressionSyntax, TNameSyntax>
{
internal abstract class AbstractIntroduceVariableCodeAction : CodeAction
private abstract class AbstractIntroduceVariableCodeAction : CodeAction
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

merge of #73392 into this PR. once that goes in, this will disappear from this PR.

{
private readonly bool _allOccurrences;
private readonly bool _isConstant;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<TService, TExpressionSyntax, TTypeSyntax, TTypeDeclarationSyntax, TQueryExpressionSyntax, TNameSyntax>
{
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)
{
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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<TService, TExpressionSyntax, TTypeSyntax, TTypeDeclarationSyntax, TQueryExpressionSyntax, TNameSyntax>
{
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<Document> 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;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
98 changes: 18 additions & 80 deletions src/Workspaces/Core/Portable/CodeActions/CodeAction.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,11 @@
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;
Expand All @@ -32,7 +29,7 @@ namespace Microsoft.CodeAnalysis.CodeActions;
/// <summary>
/// An action produced by a <see cref="CodeFixProvider"/> or a <see cref="CodeRefactoringProvider"/>.
/// </summary>
public abstract class CodeAction
public abstract partial class CodeAction
{
private static readonly Dictionary<Type, bool> s_isNonProgressGetChangedSolutionAsyncOverridden = [];
private static readonly Dictionary<Type, bool> s_isNonProgressComputeOperationsAsyncOverridden = [];
Expand Down Expand Up @@ -248,7 +245,7 @@ private protected virtual async Task<ImmutableArray<CodeActionOperation>> GetOpe

if (operations != null)
{
return await this.PostProcessAsync(originalSolution, operations, cancellationToken).ConfigureAwait(false);
return await PostProcessAsync(originalSolution, operations, cancellationToken).ConfigureAwait(false);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

became a static method.

}

return [];
Expand All @@ -269,7 +266,7 @@ internal async Task<ImmutableArray<CodeActionOperation>> GetPreviewOperationsAsy

if (operations != null)
{
return await this.PostProcessAsync(originalSolution, operations, cancellationToken).ConfigureAwait(false);
return await PostProcessAsync(originalSolution, operations, cancellationToken).ConfigureAwait(false);
}

return [];
Expand Down Expand Up @@ -408,7 +405,7 @@ protected virtual Task<Document> GetChangedDocumentAsync(IProgress<CodeAnalysisP
return solution;
}

return await this.PostProcessChangesAsync(originalSolution, solution, cancellationToken).ConfigureAwait(false);
return await PostProcessChangesAsync(originalSolution, solution, cancellationToken).ConfigureAwait(false);
}

internal Task<Document> GetChangedDocumentInternalAsync(CancellationToken cancellation)
Expand All @@ -420,19 +417,21 @@ internal Task<Document> GetChangedDocumentInternalAsync(CancellationToken cancel
/// <param name="operations">A list of operations.</param>
/// <param name="cancellationToken">A cancellation token.</param>
/// <returns>A new list of operations with post processing steps applied to any <see cref="ApplyChangesOperation"/>'s.</returns>
#pragma warning disable CA1822 // Mark members as static. This is a public API.
protected Task<ImmutableArray<CodeActionOperation>> PostProcessAsync(IEnumerable<CodeActionOperation> operations, CancellationToken cancellationToken)
=> PostProcessAsync(originalSolution: null!, operations, cancellationToken);
#pragma warning restore CA1822 // Mark members as static
=> PostProcessAsync(originalSolution: null, operations, cancellationToken);

internal async Task<ImmutableArray<CodeActionOperation>> PostProcessAsync(
Solution originalSolution, IEnumerable<CodeActionOperation> operations, CancellationToken cancellationToken)
internal static async Task<ImmutableArray<CodeActionOperation>> PostProcessAsync(
Solution? originalSolution, IEnumerable<CodeActionOperation> operations, CancellationToken cancellationToken)
{
using var result = TemporaryArray<CodeActionOperation>.Empty;

foreach (var op in operations)
{
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
{
Expand All @@ -448,11 +447,13 @@ internal async Task<ImmutableArray<CodeActionOperation>> PostProcessAsync(
/// </summary>
/// <param name="changedSolution">The solution changed by the <see cref="CodeAction"/>.</param>
/// <param name="cancellationToken">A cancellation token</param>
#pragma warning disable CA1822 // Mark members as static. This is a public API.
protected Task<Solution> 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<Solution> PostProcessChangesAsync(
Solution originalSolution,
private static async Task<Solution> PostProcessChangesAsync(
Solution? originalSolution,
Solution changedSolution,
CancellationToken cancellationToken)
{
Expand All @@ -461,38 +462,10 @@ internal async Task<Solution> 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 solutionChanges = changedSolution.GetChanges(originalSolution);

var processedSolution = changedSolution;

// process changed projects
foreach (var projectChanges in solutionChanges.GetProjectChanges())
{
var documentsToProcess = projectChanges.GetChangedDocuments(onlyGetDocumentsWithTextChanges: true).Concat(
projectChanges.GetAddedDocuments());

foreach (var documentId in documentsToProcess)
{
var document = processedSolution.GetRequiredDocument(documentId);
var processedDocument = await PostProcessChangesAsync(document, cancellationToken).ConfigureAwait(false);
processedSolution = processedDocument.Project.Solution;
}
}

// process completely new projects too
foreach (var addedProject in solutionChanges.GetAddedProjects())
{
var documentsToProcess = addedProject.DocumentIds;

foreach (var documentId in documentsToProcess)
{
var document = processedSolution.GetRequiredDocument(documentId);
var processedDocument = await PostProcessChangesAsync(document, cancellationToken).ConfigureAwait(false);
processedSolution = processedDocument.Project.Solution;
}
}

return processedSolution;
var globalOptions = changedSolution.Services.GetService<ILegacyGlobalCleanCodeGenerationOptionsWorkspaceService>();
var fallbackOptions = globalOptions?.Provider ?? CodeActionOptions.DefaultProvider;
return await CleanSyntaxAndSemanticsAsync(originalSolution, changedSolution, fallbackOptions, CodeAnalysisProgress.None, cancellationToken).ConfigureAwait(false);
}

/// <summary>
Expand All @@ -518,41 +491,6 @@ protected virtual async Task<Document> PostProcessChangesAsync(Document document
return document;
}

internal static async Task<Document> CleanupDocumentAsync(Document document, CodeCleanupOptions options, CancellationToken cancellationToken)
{
// 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);

// Now, do the semantic cleaning pass.
var document2 = await CleanupSemanticsAsync(document1, options, cancellationToken).ConfigureAwait(false);
return document2;
}

internal static async Task<Document> CleanupSyntaxAsync(Document document, CodeCleanupOptions options, CancellationToken cancellationToken)
{
// 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 async Task<Document> 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);

// 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);

return document4;
}

#region Factories for standard code actions

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Loading
Loading