From e335215b6d07d7a602efe4bd69fc870f1e8aa8d2 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 7 May 2024 20:22:48 -0700 Subject: [PATCH 01/13] Cleanup syntax first --- .../Core/Portable/CodeActions/CodeAction.cs | 28 +++++++----- .../DocumentBasedFixAllProvider.cs | 12 ++--- .../DocumentBasedFixAllProviderHelpers.cs | 45 +++++++++++++------ .../DocumentBasedFixAllProvider.cs | 27 +++-------- 4 files changed, 59 insertions(+), 53 deletions(-) diff --git a/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs b/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs index 28a3af953bf7f..86b127bc5bec1 100644 --- a/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs +++ b/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs @@ -518,23 +518,29 @@ protected virtual async Task PostProcessChangesAsync(Document document return document; } - internal static async Task CleanupDocumentAsync( - Document document, CodeCleanupOptions options, CancellationToken cancellationToken) + internal static async Task CleanupDocumentAsync(Document document, CodeCleanupOptions options, CancellationToken cancellationToken) { - document = await ImportAdder.AddImportsFromSymbolAnnotationAsync( - document, Simplifier.AddImportsAnnotation, options.AddImportOptions, cancellationToken).ConfigureAwait(false); - - document = await Simplifier.ReduceAsync(document, Simplifier.Annotation, options.SimplifierOptions, cancellationToken).ConfigureAwait(false); + var document1 = await CleanupSyntaxAsync(document, options, cancellationToken).ConfigureAwait(false); + var document2 = await CleanupSemanticsAsync(document1, options, cancellationToken).ConfigureAwait(false); + return document2; + } + internal static async Task CleanupSyntaxAsync(Document document, CodeCleanupOptions options, CancellationToken cancellationToken) + { // format any node with explicit formatter annotation - document = await Formatter.FormatAsync(document, Formatter.Annotation, options.FormattingOptions, cancellationToken).ConfigureAwait(false); + var document1 = await Formatter.FormatAsync(document, Formatter.Annotation, options.FormattingOptions, cancellationToken).ConfigureAwait(false); // format any elastic whitespace - document = await Formatter.FormatAsync(document, SyntaxAnnotation.ElasticAnnotation, options.FormattingOptions, cancellationToken).ConfigureAwait(false); - - document = await CaseCorrector.CaseCorrectAsync(document, CaseCorrector.Annotation, cancellationToken).ConfigureAwait(false); + var document2 = await Formatter.FormatAsync(document1, SyntaxAnnotation.ElasticAnnotation, options.FormattingOptions, cancellationToken).ConfigureAwait(false); + return document2; + } - return document; + 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); + return document3; } #region Factories for standard code actions diff --git a/src/Workspaces/Core/Portable/CodeFixes/FixAllOccurrences/DocumentBasedFixAllProvider.cs b/src/Workspaces/Core/Portable/CodeFixes/FixAllOccurrences/DocumentBasedFixAllProvider.cs index 823220fb6a546..00f95d649a3de 100644 --- a/src/Workspaces/Core/Portable/CodeFixes/FixAllOccurrences/DocumentBasedFixAllProvider.cs +++ b/src/Workspaces/Core/Portable/CodeFixes/FixAllOccurrences/DocumentBasedFixAllProvider.cs @@ -5,6 +5,7 @@ using System; using System.Collections.Generic; using System.Collections.Immutable; +using System.Threading; using System.Threading.Tasks; using Microsoft.CodeAnalysis.CodeActions; using Microsoft.CodeAnalysis.CodeFixesAndRefactorings; @@ -93,15 +94,8 @@ await RoslynParallel.ForEachAsync( return; var newDocument = await this.FixAllAsync(fixAllContext, document, documentDiagnostics).ConfigureAwait(false); - if (newDocument == null || newDocument == document) - return; - - // For documents that support syntax, grab the tree so that we can clean it up later. If it's a - // language that doesn't support that, then just grab the text. - var node = newDocument.SupportsSyntaxTree ? await newDocument.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false) : null; - var text = newDocument.SupportsSyntaxTree ? null : await newDocument.GetValueTextAsync(cancellationToken).ConfigureAwait(false); - - callback((document.Id, (node, text))); + await DocumentBasedFixAllProviderHelpers.CleanDocumentSyntaxAsync( + callback, document, newDocument, fixAllContext.State.CodeActionOptionsProvider, cancellationToken).ConfigureAwait(false); }).ConfigureAwait(false); } } diff --git a/src/Workspaces/Core/Portable/CodeFixesAndRefactorings/DocumentBasedFixAllProviderHelpers.cs b/src/Workspaces/Core/Portable/CodeFixesAndRefactorings/DocumentBasedFixAllProviderHelpers.cs index 1035dcc812517..789e70e80e383 100644 --- a/src/Workspaces/Core/Portable/CodeFixesAndRefactorings/DocumentBasedFixAllProviderHelpers.cs +++ b/src/Workspaces/Core/Portable/CodeFixesAndRefactorings/DocumentBasedFixAllProviderHelpers.cs @@ -7,6 +7,7 @@ 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; @@ -104,9 +105,9 @@ async Task CleanSolutionAsync(Solution dirtySolution, ImmutableArray CleanSolutionAsync(Solution dirtySolution, ImmutableArray CleanSolutionAsync(Solution dirtySolution, ImmutableArray - /// Dummy class just to get access to - /// - private class PostProcessCodeAction : CodeAction + public static async ValueTask CleanDocumentSyntaxAsync( + Action<(DocumentId documentId, (SyntaxNode? node, SourceText? text))> callback, + Document document, + Document? newDocument, + CodeActionOptionsProvider codeActionOptionsProvider, + CancellationToken cancellationToken) { - public static readonly PostProcessCodeAction Instance = new(); + if (newDocument == null || newDocument == document) + return; - public override string Title => ""; + // For documents that support syntax, grab the tree so that we can clean it up later. If it's a + // language that doesn't support that, then just grab the text. + if (newDocument.SupportsSyntaxTree) + { + 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); - public new Task PostProcessChangesAsync(Document document, CancellationToken cancellationToken) - => base.PostProcessChangesAsync(document, cancellationToken); + callback((document.Id, (node, text: null))); + } + else + { + var text = await newDocument.GetValueTextAsync(cancellationToken).ConfigureAwait(false); + callback((document.Id, (node: null, text))); + } } } diff --git a/src/Workspaces/Core/Portable/CodeRefactorings/FixAllOccurences/DocumentBasedFixAllProvider.cs b/src/Workspaces/Core/Portable/CodeRefactorings/FixAllOccurences/DocumentBasedFixAllProvider.cs index 7bb3a4c6c2526..5f2f642d39a1a 100644 --- a/src/Workspaces/Core/Portable/CodeRefactorings/FixAllOccurences/DocumentBasedFixAllProvider.cs +++ b/src/Workspaces/Core/Portable/CodeRefactorings/FixAllOccurences/DocumentBasedFixAllProvider.cs @@ -28,20 +28,15 @@ namespace Microsoft.CodeAnalysis.CodeRefactorings; /// /// TODO: Make public, tracked with https://github.com/dotnet/roslyn/issues/60703 /// -internal abstract class DocumentBasedFixAllProvider : FixAllProvider +internal abstract class DocumentBasedFixAllProvider(ImmutableArray supportedFixAllScopes) : FixAllProvider { - private readonly ImmutableArray _supportedFixAllScopes; + private readonly ImmutableArray _supportedFixAllScopes = supportedFixAllScopes; protected DocumentBasedFixAllProvider() : this(DefaultSupportedFixAllScopes) { } - protected DocumentBasedFixAllProvider(ImmutableArray supportedFixAllScopes) - { - _supportedFixAllScopes = supportedFixAllScopes; - } - /// /// Produce a suitable title for the fix-all this type creates in . Override this if customizing that title is desired. @@ -81,10 +76,9 @@ public sealed override IEnumerable GetSupportedFixAllScopes() GetFixedDocumentsAsync); /// - /// Attempts to apply fix all operations returning, for each updated document, either - /// the new syntax root for that document or its new text. Syntax roots are returned for documents that support - /// them, and are used to perform a final cleanup pass for formatting/simplication/etc. Text is returned for - /// documents that don't support syntax. + /// Attempts to apply fix all operations returning, for each updated document, either the new syntax root for that + /// document or its new text. Syntax roots are returned for documents that support them, and are used to perform a + /// final cleanup pass for formatting/simplification/etc. Text is returned for documents that don't support syntax. /// private async Task GetFixedDocumentsAsync( FixAllContext fixAllContext, Action<(DocumentId documentId, (SyntaxNode? node, SourceText? text))> callback) @@ -104,15 +98,8 @@ await RoslynParallel.ForEachAsync( { var (document, spans) = tuple; var newDocument = await this.FixAllAsync(fixAllContext, document, spans).ConfigureAwait(false); - if (newDocument == null || newDocument == document) - return; - - // For documents that support syntax, grab the tree so that we can clean it up later. If it's a - // language that doesn't support that, then just grab the text. - var node = newDocument.SupportsSyntaxTree ? await newDocument.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false) : null; - var text = newDocument.SupportsSyntaxTree ? null : await newDocument.GetValueTextAsync(cancellationToken).ConfigureAwait(false); - - callback((document.Id, (node, text))); + await DocumentBasedFixAllProviderHelpers.CleanDocumentSyntaxAsync( + callback, document, newDocument, fixAllContext.State.CodeActionOptionsProvider, cancellationToken).ConfigureAwait(false); }).ConfigureAwait(false); } } From 6c4bdf08e99f23203464c808b7f147e8d80e4071 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 7 May 2024 20:40:09 -0700 Subject: [PATCH 02/13] cleanup syntax last as well --- ...DelegateWithConditionalAccessTests_FixAllTests.cs | 6 ------ ...ressionForNullableTernaryConditionalCheckTests.cs | 2 +- .../Core/Portable/CodeActions/CodeAction.cs | 12 +++++++++++- .../FixAllOccurrences/DocumentBasedFixAllProvider.cs | 7 ++----- .../DocumentBasedFixAllProviderHelpers.cs | 12 ++++++++---- .../FixAllOccurences/DocumentBasedFixAllProvider.cs | 7 ++----- 6 files changed, 24 insertions(+), 22 deletions(-) diff --git a/src/Analyzers/CSharp/Tests/InvokeDelegateWithConditionalAccess/InvokeDelegateWithConditionalAccessTests_FixAllTests.cs b/src/Analyzers/CSharp/Tests/InvokeDelegateWithConditionalAccess/InvokeDelegateWithConditionalAccessTests_FixAllTests.cs index 5ccf06df80da4..d3d1a1daad986 100644 --- a/src/Analyzers/CSharp/Tests/InvokeDelegateWithConditionalAccess/InvokeDelegateWithConditionalAccessTests_FixAllTests.cs +++ b/src/Analyzers/CSharp/Tests/InvokeDelegateWithConditionalAccess/InvokeDelegateWithConditionalAccessTests_FixAllTests.cs @@ -46,7 +46,6 @@ class C void Goo() { a?.Invoke(); - a?.Invoke(); } } @@ -86,7 +85,6 @@ class C void Goo() { a?.Invoke(); - a?.Invoke(); } } @@ -126,7 +124,6 @@ class C void Goo() { a?.Invoke(); - a?.Invoke(); } } @@ -166,7 +163,6 @@ class C void Goo() { a?.Invoke(); - a?.Invoke(); } } @@ -206,7 +202,6 @@ class C void Goo() { a?.Invoke(); - a?.Invoke(); } } @@ -246,7 +241,6 @@ class C void Goo() { a?.Invoke(); - a?.Invoke(); } } diff --git a/src/Analyzers/CSharp/Tests/UseCoalesceExpression/UseCoalesceExpressionForNullableTernaryConditionalCheckTests.cs b/src/Analyzers/CSharp/Tests/UseCoalesceExpression/UseCoalesceExpressionForNullableTernaryConditionalCheckTests.cs index 6a8bc64328d97..0748cb0a8227b 100644 --- a/src/Analyzers/CSharp/Tests/UseCoalesceExpression/UseCoalesceExpressionForNullableTernaryConditionalCheckTests.cs +++ b/src/Analyzers/CSharp/Tests/UseCoalesceExpression/UseCoalesceExpressionForNullableTernaryConditionalCheckTests.cs @@ -163,7 +163,7 @@ class C void M(int? x, int? y) { var z1 = x ?? y; - var z2 = x ?? y ; + var z2 = x ?? y; } } """); diff --git a/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs b/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs index 86b127bc5bec1..cf70282edf2c5 100644 --- a/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs +++ b/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs @@ -520,7 +520,12 @@ protected virtual async Task PostProcessChangesAsync(Document document 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 + // 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; } @@ -540,7 +545,12 @@ internal static async Task CleanupSemanticsAsync(Document document, Co 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); - return document3; + + // 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 diff --git a/src/Workspaces/Core/Portable/CodeFixes/FixAllOccurrences/DocumentBasedFixAllProvider.cs b/src/Workspaces/Core/Portable/CodeFixes/FixAllOccurrences/DocumentBasedFixAllProvider.cs index 00f95d649a3de..88e8fc3f4d4da 100644 --- a/src/Workspaces/Core/Portable/CodeFixes/FixAllOccurrences/DocumentBasedFixAllProvider.cs +++ b/src/Workspaces/Core/Portable/CodeFixes/FixAllOccurrences/DocumentBasedFixAllProvider.cs @@ -5,11 +5,9 @@ using System; using System.Collections.Generic; using System.Collections.Immutable; -using System.Threading; using System.Threading.Tasks; using Microsoft.CodeAnalysis.CodeActions; using Microsoft.CodeAnalysis.CodeFixesAndRefactorings; -using Microsoft.CodeAnalysis.Shared.Extensions; using Microsoft.CodeAnalysis.Shared.Utilities; using Microsoft.CodeAnalysis.Text; using Roslyn.Utilities; @@ -75,7 +73,7 @@ public sealed override IEnumerable GetSupportedFixAllScopes() DetermineDiagnosticsAndGetFixedDocumentsAsync); private async Task DetermineDiagnosticsAndGetFixedDocumentsAsync( - FixAllContext fixAllContext, Action<(DocumentId documentId, (SyntaxNode? node, SourceText? text))> callback) + FixAllContext fixAllContext, Func onDocumentFixed) { var cancellationToken = fixAllContext.CancellationToken; @@ -94,8 +92,7 @@ await RoslynParallel.ForEachAsync( return; var newDocument = await this.FixAllAsync(fixAllContext, document, documentDiagnostics).ConfigureAwait(false); - await DocumentBasedFixAllProviderHelpers.CleanDocumentSyntaxAsync( - callback, document, newDocument, fixAllContext.State.CodeActionOptionsProvider, cancellationToken).ConfigureAwait(false); + await onDocumentFixed(document, newDocument).ConfigureAwait(false); }).ConfigureAwait(false); } } diff --git a/src/Workspaces/Core/Portable/CodeFixesAndRefactorings/DocumentBasedFixAllProviderHelpers.cs b/src/Workspaces/Core/Portable/CodeFixesAndRefactorings/DocumentBasedFixAllProviderHelpers.cs index 789e70e80e383..1705882691822 100644 --- a/src/Workspaces/Core/Portable/CodeFixesAndRefactorings/DocumentBasedFixAllProviderHelpers.cs +++ b/src/Workspaces/Core/Portable/CodeFixesAndRefactorings/DocumentBasedFixAllProviderHelpers.cs @@ -28,7 +28,7 @@ internal static class DocumentBasedFixAllProviderHelpers ImmutableArray fixAllContexts, IProgress progressTracker, string progressTrackerDescription, - Func, Task> getFixedDocumentsAsync) + Func, Task> getFixedDocumentsAsync) where TFixAllContext : IFixAllContext { var cancellationToken = originalFixAllContext.CancellationToken; @@ -60,7 +60,9 @@ internal static class DocumentBasedFixAllProviderHelpers Contract.ThrowIfFalse( fixAllContext.Scope is FixAllScope.Document or FixAllScope.Project or FixAllScope.ContainingMember or FixAllScope.ContainingType); - await args.getFixedDocumentsAsync(fixAllContext, callback).ConfigureAwait(false); + await args.getFixedDocumentsAsync(fixAllContext, + (originalDocument, newDocument) => CleanDocumentSyntaxAsync( + callback, originalDocument, newDocument, fixAllContext.State.CodeActionOptionsProvider, cancellationToken)).ConfigureAwait(false); }, consumeItems: static async (stream, args, cancellationToken) => { @@ -150,10 +152,11 @@ public static async ValueTask CleanDocumentSyntaxAsync( if (newDocument == null || newDocument == document) return; - // For documents that support syntax, grab the tree so that we can clean it up later. If it's a - // language that doesn't support that, then just grab the text. 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); @@ -162,6 +165,7 @@ public static async ValueTask CleanDocumentSyntaxAsync( } else { + // If it's a language that doesn't support that, then just grab the text. var text = await newDocument.GetValueTextAsync(cancellationToken).ConfigureAwait(false); callback((document.Id, (node: null, text))); } diff --git a/src/Workspaces/Core/Portable/CodeRefactorings/FixAllOccurences/DocumentBasedFixAllProvider.cs b/src/Workspaces/Core/Portable/CodeRefactorings/FixAllOccurences/DocumentBasedFixAllProvider.cs index 5f2f642d39a1a..6dbcf20bc1002 100644 --- a/src/Workspaces/Core/Portable/CodeRefactorings/FixAllOccurences/DocumentBasedFixAllProvider.cs +++ b/src/Workspaces/Core/Portable/CodeRefactorings/FixAllOccurences/DocumentBasedFixAllProvider.cs @@ -8,8 +8,6 @@ using System.Threading.Tasks; using Microsoft.CodeAnalysis.CodeActions; using Microsoft.CodeAnalysis.CodeFixesAndRefactorings; -using Microsoft.CodeAnalysis.PooledObjects; -using Microsoft.CodeAnalysis.Shared.Extensions; using Microsoft.CodeAnalysis.Shared.Utilities; using Microsoft.CodeAnalysis.Text; using Roslyn.Utilities; @@ -81,7 +79,7 @@ public sealed override IEnumerable GetSupportedFixAllScopes() /// final cleanup pass for formatting/simplification/etc. Text is returned for documents that don't support syntax. /// private async Task GetFixedDocumentsAsync( - FixAllContext fixAllContext, Action<(DocumentId documentId, (SyntaxNode? node, SourceText? text))> callback) + FixAllContext fixAllContext, Func onDocumentFixed) { Contract.ThrowIfFalse(fixAllContext.Scope is FixAllScope.Document or FixAllScope.Project or FixAllScope.ContainingMember or FixAllScope.ContainingType); @@ -98,8 +96,7 @@ await RoslynParallel.ForEachAsync( { var (document, spans) = tuple; var newDocument = await this.FixAllAsync(fixAllContext, document, spans).ConfigureAwait(false); - await DocumentBasedFixAllProviderHelpers.CleanDocumentSyntaxAsync( - callback, document, newDocument, fixAllContext.State.CodeActionOptionsProvider, cancellationToken).ConfigureAwait(false); + await onDocumentFixed(document, newDocument).ConfigureAwait(false); }).ConfigureAwait(false); } } From ed3233ddffee7c6d018baeee995bb289494259ea Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 7 May 2024 20:42:47 -0700 Subject: [PATCH 03/13] move function --- .../DocumentBasedFixAllProviderHelpers.cs | 52 +++++++++---------- 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/src/Workspaces/Core/Portable/CodeFixesAndRefactorings/DocumentBasedFixAllProviderHelpers.cs b/src/Workspaces/Core/Portable/CodeFixesAndRefactorings/DocumentBasedFixAllProviderHelpers.cs index 1705882691822..847fd166ef1a1 100644 --- a/src/Workspaces/Core/Portable/CodeFixesAndRefactorings/DocumentBasedFixAllProviderHelpers.cs +++ b/src/Workspaces/Core/Portable/CodeFixesAndRefactorings/DocumentBasedFixAllProviderHelpers.cs @@ -140,34 +140,34 @@ async Task CleanSolutionAsync(Solution dirtySolution, ImmutableArray callback, - Document document, - Document? newDocument, - CodeActionOptionsProvider codeActionOptionsProvider, - CancellationToken cancellationToken) - { - if (newDocument == null || newDocument == document) - return; - 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); - - callback((document.Id, (node, text: null))); - } - else + static async ValueTask CleanDocumentSyntaxAsync( + Action<(DocumentId documentId, (SyntaxNode? node, SourceText? text))> callback, + Document document, + Document? newDocument, + CodeActionOptionsProvider codeActionOptionsProvider, + CancellationToken cancellationToken) { - // If it's a language that doesn't support that, then just grab the text. - var text = await newDocument.GetValueTextAsync(cancellationToken).ConfigureAwait(false); - callback((document.Id, (node: null, text))); + if (newDocument == null || newDocument == document) + return; + + 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); + + callback((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); + callback((document.Id, (node: null, text))); + } } } } From e54a2a68ffce2f5a50c6089327822cdfbb9a93bd Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 7 May 2024 20:50:36 -0700 Subject: [PATCH 04/13] cleanup --- .../DocumentBasedFixAllProviderHelpers.cs | 43 +++++++++++-------- 1 file changed, 26 insertions(+), 17 deletions(-) diff --git a/src/Workspaces/Core/Portable/CodeFixesAndRefactorings/DocumentBasedFixAllProviderHelpers.cs b/src/Workspaces/Core/Portable/CodeFixesAndRefactorings/DocumentBasedFixAllProviderHelpers.cs index 847fd166ef1a1..981560ec81af2 100644 --- a/src/Workspaces/Core/Portable/CodeFixesAndRefactorings/DocumentBasedFixAllProviderHelpers.cs +++ b/src/Workspaces/Core/Portable/CodeFixesAndRefactorings/DocumentBasedFixAllProviderHelpers.cs @@ -60,21 +60,31 @@ internal static class DocumentBasedFixAllProviderHelpers Contract.ThrowIfFalse( fixAllContext.Scope is FixAllScope.Document or FixAllScope.Project or FixAllScope.ContainingMember or FixAllScope.ContainingType); - await args.getFixedDocumentsAsync(fixAllContext, - (originalDocument, newDocument) => CleanDocumentSyntaxAsync( - callback, originalDocument, newDocument, fixAllContext.State.CodeActionOptionsProvider, cancellationToken)).ConfigureAwait(false); + // Defer to the FixAllProvider to actually compute each fixed document. + 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 tupleOpt = await CleanDocumentSyntaxAsync( + originalDocument, newDocument, fixAllContext.State.CodeActionOptionsProvider, cancellationToken).ConfigureAwait(false); + if (tupleOpt.HasValue) + callback(tupleOpt.Value); + }).ConfigureAwait(false); }, consumeItems: static async (stream, args, cancellationToken) => { var currentSolution = args.solution; using var _ = ArrayBuilder.GetInstance(out var changedRootDocumentIds); - // 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 cleaned up. - // We don't cleanup the documents as they are created, or one at a time as we add them, as that - // would cause us to run cleanup on N different solution forks (which would be very expensive). - // Instead, by adding all the changed documents to one solution, and then cleaning *those* we - // only perform cleanup semantics on one forked solution. + // 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 + // up. We don't cleanup the documents as they are created, or one at a time as we add them, as that + // would cause us to run semantic cleanup on N different solution forks (which would be very + // expensive as we'd fork, produce semantics, fork, produce semantics, etc. etc.). Instead, by + // adding all the changed documents to one solution, and then cleaning *those* we only perform + // 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 @@ -107,9 +117,9 @@ async Task CleanSolutionAsync(Solution dirtySolution, ImmutableArray CleanSolutionAsync(Solution dirtySolution, ImmutableArray callback, + static async ValueTask<(DocumentId documentId, (SyntaxNode? node, SourceText? text))?> CleanDocumentSyntaxAsync( Document document, Document? newDocument, CodeActionOptionsProvider codeActionOptionsProvider, CancellationToken cancellationToken) { if (newDocument == null || newDocument == document) - return; + return null; if (newDocument.SupportsSyntaxTree) { @@ -160,13 +169,13 @@ static async ValueTask CleanDocumentSyntaxAsync( var cleanedDocument = await CodeAction.CleanupSyntaxAsync(newDocument, codeActionOptions, cancellationToken).ConfigureAwait(false); var node = await cleanedDocument.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false); - callback((document.Id, (node, text: null))); + 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); - callback((document.Id, (node: null, text))); + return (document.Id, (node: null, text)); } } } From ff73955b9f5007c9e80d5bfc0ff4c1a6ba081d20 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 7 May 2024 21:44:22 -0700 Subject: [PATCH 05/13] Update tests --- .../RemoveAsyncModifier/RemoveAsyncModifierTests.cs | 6 ++++-- .../CSharpTest/InlineTemporary/InlineTemporaryTests.cs | 10 +++++----- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/Analyzers/CSharp/Tests/RemoveAsyncModifier/RemoveAsyncModifierTests.cs b/src/Analyzers/CSharp/Tests/RemoveAsyncModifier/RemoveAsyncModifierTests.cs index d059a959a662b..47bfbcf7b3668 100644 --- a/src/Analyzers/CSharp/Tests/RemoveAsyncModifier/RemoveAsyncModifierTests.cs +++ b/src/Analyzers/CSharp/Tests/RemoveAsyncModifier/RemoveAsyncModifierTests.cs @@ -902,7 +902,8 @@ class C { public void M1() { - Func foo = x => { + Func foo = x => + { if (System.DateTime.Now.Ticks > 0) { return Task.CompletedTask; @@ -1040,7 +1041,8 @@ class C { public void M1() { - Func foo = () => { + Func foo = () => + { if (System.DateTime.Now.Ticks > 0) { return Task.CompletedTask; diff --git a/src/Features/CSharpTest/InlineTemporary/InlineTemporaryTests.cs b/src/Features/CSharpTest/InlineTemporary/InlineTemporaryTests.cs index d23dbb7d5117b..0ce28412e6c55 100644 --- a/src/Features/CSharpTest/InlineTemporary/InlineTemporaryTests.cs +++ b/src/Features/CSharpTest/InlineTemporary/InlineTemporaryTests.cs @@ -259,10 +259,10 @@ await TestFixOneAsync( x.ToString(); } """, - """ - { - 3.ToString(); } - """); + """ + { + 3.ToString(); } + """); } [Fact] @@ -690,7 +690,7 @@ class Program static void Main() { int x = 2; - Bar(x < x, x > 1+2); + Bar(x < x, x > 1 + 2); } static void Bar(object a, object b) From 40bc0bc998bddea68de36e422597920ee24191a5 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 7 May 2024 21:49:55 -0700 Subject: [PATCH 06/13] Fix feature --- ...SharpMakeMethodAsynchronousCodeFixProvider.cs | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/Analyzers/CSharp/CodeFixes/MakeMethodAsynchronous/CSharpMakeMethodAsynchronousCodeFixProvider.cs b/src/Analyzers/CSharp/CodeFixes/MakeMethodAsynchronous/CSharpMakeMethodAsynchronousCodeFixProvider.cs index b23010c17f6a6..7d56e87ff338e 100644 --- a/src/Analyzers/CSharp/CodeFixes/MakeMethodAsynchronous/CSharpMakeMethodAsynchronousCodeFixProvider.cs +++ b/src/Analyzers/CSharp/CodeFixes/MakeMethodAsynchronous/CSharpMakeMethodAsynchronousCodeFixProvider.cs @@ -92,7 +92,7 @@ private static MethodDeclarationSyntax FixMethod( CancellationToken cancellationToken) { var newReturnType = FixMethodReturnType(keepVoid, methodSymbol, method.ReturnType, knownTypes, cancellationToken); - var newModifiers = AddAsyncModifierWithCorrectedTrivia(method.Modifiers, ref newReturnType); + (var newModifiers, newReturnType) = AddAsyncModifierWithCorrectedTrivia(method.Modifiers, newReturnType); return method.WithReturnType(newReturnType).WithModifiers(newModifiers); } @@ -104,7 +104,7 @@ private static LocalFunctionStatementSyntax FixLocalFunction( CancellationToken cancellationToken) { var newReturnType = FixMethodReturnType(keepVoid, methodSymbol, localFunction.ReturnType, knownTypes, cancellationToken); - var newModifiers = AddAsyncModifierWithCorrectedTrivia(localFunction.Modifiers, ref newReturnType); + (var newModifiers, newReturnType) = AddAsyncModifierWithCorrectedTrivia(localFunction.Modifiers, newReturnType); return localFunction.WithReturnType(newReturnType).WithModifiers(newModifiers); } @@ -176,15 +176,17 @@ private static bool IsIEnumerable(ITypeSymbol returnType, KnownTaskTypes knownTy private static bool IsIEnumerator(ITypeSymbol returnType, KnownTaskTypes knownTypes) => returnType.OriginalDefinition.Equals(knownTypes.IEnumeratorOfTType); - private static SyntaxTokenList AddAsyncModifierWithCorrectedTrivia(SyntaxTokenList modifiers, ref TypeSyntax newReturnType) + private static (SyntaxTokenList newModifiers, TypeSyntax newReturnType) AddAsyncModifierWithCorrectedTrivia(SyntaxTokenList modifiers, TypeSyntax returnType) { if (modifiers.Any()) - return modifiers.Add(AsyncKeyword); + return (modifiers.Add(AsyncKeyword.WithoutTrivia().WithTrailingTrivia(Space)), returnType); // Move the leading trivia from the return type to the new modifiers list. - var result = TokenList(AsyncKeyword.WithLeadingTrivia(newReturnType.GetLeadingTrivia())); - newReturnType = newReturnType.WithoutLeadingTrivia(); - return result; + var newModifiers = TokenList(AsyncKeyword.WithoutTrivia() + .WithLeadingTrivia(returnType.GetLeadingTrivia()) + .WithTrailingTrivia(Space)); + var newReturnType = returnType.WithoutLeadingTrivia(); + return (newModifiers, newReturnType); } private static AnonymousFunctionExpressionSyntax FixAnonymousFunction(AnonymousFunctionExpressionSyntax anonymous) From 882f389542787f2d716d3f398ac8d149373ffebc Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 7 May 2024 21:51:14 -0700 Subject: [PATCH 07/13] simplify --- .../CSharpMakeMethodAsynchronousCodeFixProvider.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Analyzers/CSharp/CodeFixes/MakeMethodAsynchronous/CSharpMakeMethodAsynchronousCodeFixProvider.cs b/src/Analyzers/CSharp/CodeFixes/MakeMethodAsynchronous/CSharpMakeMethodAsynchronousCodeFixProvider.cs index 7d56e87ff338e..c24bdc78fec87 100644 --- a/src/Analyzers/CSharp/CodeFixes/MakeMethodAsynchronous/CSharpMakeMethodAsynchronousCodeFixProvider.cs +++ b/src/Analyzers/CSharp/CodeFixes/MakeMethodAsynchronous/CSharpMakeMethodAsynchronousCodeFixProvider.cs @@ -28,6 +28,8 @@ internal class CSharpMakeMethodAsynchronousCodeFixProvider : AbstractMakeMethodA private const string CS4034 = nameof(CS4034); // The 'await' operator can only be used within an async lambda expression. Consider marking this method with the 'async' modifier. private const string CS0246 = nameof(CS0246); // The type or namespace name 'await' could not be found + private static readonly SyntaxToken s_asyncKeywordWithSpace = AsyncKeyword.WithoutTrivia().WithTrailingTrivia(Space); + [ImportingConstructor] [SuppressMessage("RoslynDiagnosticsReliability", "RS0033:Importing constructor should be [Obsolete]", Justification = "Used in test code: https://github.com/dotnet/roslyn/issues/42814")] public CSharpMakeMethodAsynchronousCodeFixProvider() @@ -179,12 +181,10 @@ private static bool IsIEnumerator(ITypeSymbol returnType, KnownTaskTypes knownTy private static (SyntaxTokenList newModifiers, TypeSyntax newReturnType) AddAsyncModifierWithCorrectedTrivia(SyntaxTokenList modifiers, TypeSyntax returnType) { if (modifiers.Any()) - return (modifiers.Add(AsyncKeyword.WithoutTrivia().WithTrailingTrivia(Space)), returnType); + return (modifiers.Add(s_asyncKeywordWithSpace), returnType); // Move the leading trivia from the return type to the new modifiers list. - var newModifiers = TokenList(AsyncKeyword.WithoutTrivia() - .WithLeadingTrivia(returnType.GetLeadingTrivia()) - .WithTrailingTrivia(Space)); + var newModifiers = TokenList(s_asyncKeywordWithSpace.WithLeadingTrivia(returnType.GetLeadingTrivia())); var newReturnType = returnType.WithoutLeadingTrivia(); return (newModifiers, newReturnType); } From 325369827d8bf1421ea712976262c456fbde90b2 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 7 May 2024 21:52:24 -0700 Subject: [PATCH 08/13] fix tests --- ...esceExpressionForNullableTernaryConditionalCheckTests.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Analyzers/CSharp/Tests/UseCoalesceExpression/UseCoalesceExpressionForNullableTernaryConditionalCheckTests.cs b/src/Analyzers/CSharp/Tests/UseCoalesceExpression/UseCoalesceExpressionForNullableTernaryConditionalCheckTests.cs index 0748cb0a8227b..1cfa1624abc17 100644 --- a/src/Analyzers/CSharp/Tests/UseCoalesceExpression/UseCoalesceExpressionForNullableTernaryConditionalCheckTests.cs +++ b/src/Analyzers/CSharp/Tests/UseCoalesceExpression/UseCoalesceExpressionForNullableTernaryConditionalCheckTests.cs @@ -49,7 +49,7 @@ class C { void M(int? x, int? y) { - var z = x ?? y ; + var z = x ?? y; } } """); @@ -105,7 +105,7 @@ class C { void M(int? x, int? y) { - var z = (x + y) ?? y ; + var z = (x + y) ?? y; } } """); @@ -249,7 +249,7 @@ class C { void M(int? x, int? y) { - Expression> e = () => {|Warning:x ?? y|} ; + Expression> e = () => {|Warning:x ?? y|}; } } """); From 20ed180fa5ab5ebf3a8a857596f78d27e145cd03 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 7 May 2024 21:59:03 -0700 Subject: [PATCH 09/13] fix tests --- .../ConvertForEachToLinqQueryTests.cs | 30 ++++++++++--------- .../InlineTemporary/InlineTemporaryTests.cs | 2 +- .../CSharpTest/InvertIf/InvertIfTests.cs | 2 +- 3 files changed, 18 insertions(+), 16 deletions(-) diff --git a/src/Features/CSharpTest/ConvertLinq/ConvertForEachToLinqQueryTests.cs b/src/Features/CSharpTest/ConvertLinq/ConvertForEachToLinqQueryTests.cs index 5edb251a13646..959a63de1a199 100644 --- a/src/Features/CSharpTest/ConvertLinq/ConvertForEachToLinqQueryTests.cs +++ b/src/Features/CSharpTest/ConvertLinq/ConvertForEachToLinqQueryTests.cs @@ -4156,10 +4156,10 @@ List M(IEnumerable nums) return /*30*/ /* 1 *//* 2 *//* 3 *//* 4 */// 5 /*31*//* 6 */ (from/* 8 *//* 7 *//* 9 */x /* 10 */ in/* 11 */nums/* 12 */// 13 - /* 14 */// 15 - /* 16 *//* 17 */ - let y /* 18 */ = /* 19 */ x + 1/* 20 *///21 - select y)/* 24 *//*27*///28 + /* 14 */// 15 + /* 16 *//* 17 */ + let y /* 18 */ = /* 19 */ x + 1/* 20 *///21 + select y)/* 24 *//*27*///28 .ToList()/* 22 *//* 23 *//* 25 *///26 ; //32 } @@ -4205,7 +4205,7 @@ List M(IEnumerable nums) /*25*//* 14 */// 15 /* 6 */ (from/* 8 *//* 7 *//* 9 */x /* 10 */ in/* 11 */nums/* 12 */// 13 - select x + 1)/* 18 *//*21*///22 + select x + 1)/* 18 *//*21*///22 .ToList()/* 16 *//* 17 *//* 19 *///20 ; //26 } @@ -4223,7 +4223,8 @@ List M(IEnumerable nums) { /*23*/ return /*24*/ /* 1 *//* 2 *//* 3 *//* 4 */// 5 - /*25*/nums /* 12 */.Select( + /*25*/ + nums /* 12 */.Select( /* 6 *//* 7 *//* 14 */// 15 /* 9 */x /* 10 */ => x + 1/* 18 *//*21*///22 /* 8 *//* 11 */// 13 @@ -4268,7 +4269,7 @@ int M(IEnumerable nums) /*23*//* 14 */// 15 /* 6 */ (from/* 8 *//* 7 *//* 9 */x /* 10 */ in/* 11 */nums/* 12 */// 13 - select x)/* 10 *//*19*///20 + select x)/* 10 *//*19*///20 .Count()/* 16 *//* 17 *///18 ; //24 } @@ -4286,7 +4287,8 @@ int M(IEnumerable nums) { /*21*/ return /*22*/ /* 1 *//* 2 *//* 3 *//* 4 */// 5 - /*23*/nums /* 12 *//* 6 *//* 7 *//* 14 */// 15 + /*23*/ + nums /* 12 *//* 6 *//* 7 *//* 14 */// 15 /* 9 *//* 10 *//* 10 *//*19*///20 /* 8 *//* 11 */// 13 .Count()/* 16 *//* 17 *///18 @@ -4328,11 +4330,11 @@ void M(IEnumerable nums) { foreach (var (a /* 12 */ , b /*16*/ ) in /* 1 */from/* 2 */int /* 3 */ n1 /* 4 */in/* 5 */nums/* 6 */// 7 - /* 8*/// 9 - /* 10 *//* 11 */ - let a /* 12 */ = /* 13 */ n1 + n1/* 14*//* 15 */ - let b /*16*/ = /*17*/ n1 * n1/*18*///19 - select (a /* 12 */ , b /*16*/ )/*22*//*23*/) + /* 8*/// 9 + /* 10 *//* 11 */ + let a /* 12 */ = /* 13 */ n1 + n1/* 14*//* 15 */ + let b /*16*/ = /*17*/ n1 * n1/*18*///19 + select (a /* 12 */ , b /*16*/ )/*22*//*23*/) { /*20*/ Console.WriteLine(a + b);//21 @@ -4384,7 +4386,7 @@ void M(IEnumerable nums) /* 10 */ where/* 11 *//* 12 */n1 /* 13 */ > /* 14 */ 0/* 15 */// 16 select n1/* 4 *//* 21 */// 22 - /*23*//*24*/ + /*23*//*24*/ ) { /*19*/ diff --git a/src/Features/CSharpTest/InlineTemporary/InlineTemporaryTests.cs b/src/Features/CSharpTest/InlineTemporary/InlineTemporaryTests.cs index 0ce28412e6c55..4ed93be735cdc 100644 --- a/src/Features/CSharpTest/InlineTemporary/InlineTemporaryTests.cs +++ b/src/Features/CSharpTest/InlineTemporary/InlineTemporaryTests.cs @@ -260,7 +260,7 @@ await TestFixOneAsync( x.ToString(); } """, """ - { + { 3.ToString(); } """); } diff --git a/src/Features/CSharpTest/InvertIf/InvertIfTests.cs b/src/Features/CSharpTest/InvertIf/InvertIfTests.cs index 1fec5e17cbf1b..328d153609067 100644 --- a/src/Features/CSharpTest/InvertIf/InvertIfTests.cs +++ b/src/Features/CSharpTest/InvertIf/InvertIfTests.cs @@ -1082,7 +1082,7 @@ public async Task TestEmptyIf() { await TestInRegularAndScriptAsync( @"class C { void M(string s){ [||]if (s == ""a""){}else{ s = ""b""}}}", - @"class C { void M(string s){ if (s != ""a""){ s = ""b""}}}"); + @"class C { void M(string s){ if (s != ""a"") { s = ""b""}}}"); } [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/43224")] From 3eee53ee3c4a3e197e69152ac97c20ab2bbb3fcd Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 7 May 2024 22:05:44 -0700 Subject: [PATCH 10/13] fix tests --- .../InlineTemporary/InlineTemporaryTests.cs | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/src/Features/CSharpTest/InlineTemporary/InlineTemporaryTests.cs b/src/Features/CSharpTest/InlineTemporary/InlineTemporaryTests.cs index 4ed93be735cdc..438aebd5db404 100644 --- a/src/Features/CSharpTest/InlineTemporary/InlineTemporaryTests.cs +++ b/src/Features/CSharpTest/InlineTemporary/InlineTemporaryTests.cs @@ -6,7 +6,6 @@ using Microsoft.CodeAnalysis.CodeRefactorings; using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.CSharp.CodeRefactorings.InlineTemporary; -using Microsoft.CodeAnalysis.CSharp.Shared.Extensions; using Microsoft.CodeAnalysis.CSharp.Test.Utilities; using Microsoft.CodeAnalysis.Test.Utilities; using Microsoft.CodeAnalysis.UnitTests; @@ -16,7 +15,7 @@ namespace Microsoft.CodeAnalysis.Editor.CSharp.UnitTests.CodeRefactorings.InlineTemporary { [Trait(Traits.Feature, Traits.Features.CodeActionsInlineTemporary)] - public class InlineTemporaryTests : AbstractCSharpCodeActionTest_NoEditor + public sealed class InlineTemporaryTests : AbstractCSharpCodeActionTest_NoEditor { protected override CodeRefactoringProvider CreateCodeRefactoringProvider(TestWorkspace workspace, TestParameters parameters) => new CSharpInlineTemporaryCodeRefactoringProvider(); @@ -253,16 +252,23 @@ public void M() [Fact] public async Task Conversion_NoConversion() { - await TestFixOneAsync( + await TestAsync( """ - { int [||]x = 3; + class C + { + void F(){ int [||]x = 3; x.ToString(); } + } """, """ + class C { - 3.ToString(); } - """); + void F(){ + 3.ToString(); } + } + """, + CSharpParseOptions.Default); } [Fact] From 10a82a17220a0f7f3d9b8330a479cf2bdeb50551 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 7 May 2024 23:14:24 -0700 Subject: [PATCH 11/13] Fix formatting --- .../InlineMethod/VisualBasicInlineMethodTests.vb | 6 +++--- .../InlineMethod/AbstractInlineMethodRefactoringProvider.cs | 3 ++- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/EditorFeatures/VisualBasicTest/CodeActions/InlineMethod/VisualBasicInlineMethodTests.vb b/src/EditorFeatures/VisualBasicTest/CodeActions/InlineMethod/VisualBasicInlineMethodTests.vb index 035d4cdb4b5a6..5acc4c12278c1 100644 --- a/src/EditorFeatures/VisualBasicTest/CodeActions/InlineMethod/VisualBasicInlineMethodTests.vb +++ b/src/EditorFeatures/VisualBasicTest/CodeActions/InlineMethod/VisualBasicInlineMethodTests.vb @@ -795,8 +795,8 @@ Imports System Public Class TestClass Public Sub Caller(i As Integer, j As Integer) Dim x = Function() - Return i * j - End Function() + Return i * j + End Function() End Sub ## Private Function Callee(i As Integer, j As Integer) as Func(Of Integer) @@ -1287,7 +1287,7 @@ Public Class TestClass End Class", " Public Class TestClass Public Sub Caller(i As Integer) - Dim y = If (true, 10, 100) + Dim y = If(true, 10, 100) End Sub ## Private Function Callee(a As Boolean) As Integer diff --git a/src/Features/Core/Portable/InlineMethod/AbstractInlineMethodRefactoringProvider.cs b/src/Features/Core/Portable/InlineMethod/AbstractInlineMethodRefactoringProvider.cs index ede4efcdcb5d3..6415bb7c4d8e7 100644 --- a/src/Features/Core/Portable/InlineMethod/AbstractInlineMethodRefactoringProvider.cs +++ b/src/Features/Core/Portable/InlineMethod/AbstractInlineMethodRefactoringProvider.cs @@ -10,6 +10,7 @@ using Microsoft.CodeAnalysis.CodeActions; using Microsoft.CodeAnalysis.CodeRefactorings; using Microsoft.CodeAnalysis.Editing; +using Microsoft.CodeAnalysis.Formatting; using Microsoft.CodeAnalysis.LanguageService; using Microsoft.CodeAnalysis.Operations; using Microsoft.CodeAnalysis.Shared.Extensions; @@ -555,7 +556,7 @@ private async Task GetChangedCallerAsync(Document document, inlineExpression = (TExpressionSyntax)syntaxGenerator.AddParentheses( syntaxGenerator.CastExpression( GenerateTypeSyntax(calleeMethodSymbol.ReturnType, allowVar: false), - syntaxGenerator.AddParentheses(inlineMethodContext.InlineExpression))); + syntaxGenerator.AddParentheses(inlineMethodContext.InlineExpression.WithAdditionalAnnotations(Formatter.Annotation)))); } From 3a8fc26f01a749c68d546d9e11c2a54fe1ca0250 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 7 May 2024 23:15:04 -0700 Subject: [PATCH 12/13] Comment --- .../InlineMethod/AbstractInlineMethodRefactoringProvider.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Features/Core/Portable/InlineMethod/AbstractInlineMethodRefactoringProvider.cs b/src/Features/Core/Portable/InlineMethod/AbstractInlineMethodRefactoringProvider.cs index 6415bb7c4d8e7..418c10b2499b5 100644 --- a/src/Features/Core/Portable/InlineMethod/AbstractInlineMethodRefactoringProvider.cs +++ b/src/Features/Core/Portable/InlineMethod/AbstractInlineMethodRefactoringProvider.cs @@ -553,6 +553,8 @@ private async Task GetChangedCallerAsync(Document document, // After: // void Caller() { var x = ((Func)(() => 1))(); } // Func Callee() { return () => 1; } + // + // Also, ensure that the node is formatted properly at the destination location. inlineExpression = (TExpressionSyntax)syntaxGenerator.AddParentheses( syntaxGenerator.CastExpression( GenerateTypeSyntax(calleeMethodSymbol.ReturnType, allowVar: false), From 69ad1e2ef4c58fc51fe27fe10f280ae6790170d0 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 7 May 2024 23:17:48 -0700 Subject: [PATCH 13/13] Comment --- .../InlineMethod/AbstractInlineMethodRefactoringProvider.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Features/Core/Portable/InlineMethod/AbstractInlineMethodRefactoringProvider.cs b/src/Features/Core/Portable/InlineMethod/AbstractInlineMethodRefactoringProvider.cs index 418c10b2499b5..7779de2fa8b44 100644 --- a/src/Features/Core/Portable/InlineMethod/AbstractInlineMethodRefactoringProvider.cs +++ b/src/Features/Core/Portable/InlineMethod/AbstractInlineMethodRefactoringProvider.cs @@ -554,7 +554,9 @@ private async Task GetChangedCallerAsync(Document document, // void Caller() { var x = ((Func)(() => 1))(); } // Func Callee() { return () => 1; } // - // Also, ensure that the node is formatted properly at the destination location. + // Also, ensure that the node is formatted properly at the destination location. This is needed as the + // location of the destination node might be very different (indentation/nesting wise) from the original + // method where the inlined code is coming from. inlineExpression = (TExpressionSyntax)syntaxGenerator.AddParentheses( syntaxGenerator.CastExpression( GenerateTypeSyntax(calleeMethodSymbol.ReturnType, allowVar: false),