Keep comments when removing unnecessary lambda expressions (#76015)

src/Analyzers/CSharp/Analyzers/RemoveUnnecessaryLambdaExpression/CSharpRemoveUnnecessaryLambdaExpressionDiagnosticAnalyzer.cs

@@ -2,7 +2,6 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
-using System.Collections.Immutable;
 using System.Diagnostics.CodeAnalysis;
 using System.Linq;
 using System.Threading;
@@ -26,18 +25,15 @@ namespace Microsoft.CodeAnalysis.CSharp.RemoveUnnecessaryLambdaExpression;
 /// time.
 /// </summary>
 [DiagnosticAnalyzer(LanguageNames.CSharp)]
-internal sealed class CSharpRemoveUnnecessaryLambdaExpressionDiagnosticAnalyzer : AbstractBuiltInUnnecessaryCodeStyleDiagnosticAnalyzer
+internal sealed class CSharpRemoveUnnecessaryLambdaExpressionDiagnosticAnalyzer()
+    : AbstractBuiltInUnnecessaryCodeStyleDiagnosticAnalyzer(
+        IDEDiagnosticIds.RemoveUnnecessaryLambdaExpressionDiagnosticId,
+        EnforceOnBuildValues.RemoveUnnecessaryLambdaExpression,
+        CSharpCodeStyleOptions.PreferMethodGroupConversion,
+        fadingOption: null,
+        new LocalizableResourceString(nameof(CSharpAnalyzersResources.Remove_unnecessary_lambda_expression), CSharpAnalyzersResources.ResourceManager, typeof(CSharpAnalyzersResources)),
+        new LocalizableResourceString(nameof(CSharpAnalyzersResources.Lambda_expression_can_be_removed), CSharpAnalyzersResources.ResourceManager, typeof(CSharpAnalyzersResources)))
 {
-    public CSharpRemoveUnnecessaryLambdaExpressionDiagnosticAnalyzer()
-        : base(IDEDiagnosticIds.RemoveUnnecessaryLambdaExpressionDiagnosticId,
-               EnforceOnBuildValues.RemoveUnnecessaryLambdaExpression,
-               CSharpCodeStyleOptions.PreferMethodGroupConversion,
-               fadingOption: null,
-               new LocalizableResourceString(nameof(CSharpAnalyzersResources.Remove_unnecessary_lambda_expression), CSharpAnalyzersResources.ResourceManager, typeof(CSharpAnalyzersResources)),
-               new LocalizableResourceString(nameof(CSharpAnalyzersResources.Lambda_expression_can_be_removed), CSharpAnalyzersResources.ResourceManager, typeof(CSharpAnalyzersResources)))
-    {
-    }
-
     public override DiagnosticAnalyzerCategory GetAnalyzerCategory()
         => DiagnosticAnalyzerCategory.SemanticSpanAnalysis;
 
@@ -51,7 +47,7 @@ protected override void InitializeWorker(AnalysisContext context)
                 var conditionalAttributeType = context.Compilation.ConditionalAttribute();
 
                 context.RegisterSyntaxNodeAction(
-                    c => AnalyzeSyntax(c, expressionType, conditionalAttributeType),
+                    context => AnalyzeSyntax(context, expressionType, conditionalAttributeType),
                     SyntaxKind.SimpleLambdaExpression, SyntaxKind.ParenthesizedLambdaExpression, SyntaxKind.AnonymousMethodExpression);
             }
         });

src/Analyzers/CSharp/CodeFixes/RemoveUnnecessaryLambdaExpression/CSharpRemoveUnnecessaryLambdaExpressionCodeFixProvider.cs

@@ -3,17 +3,21 @@
 // See the LICENSE file in the project root for more information.
 
 using System;
+using System.Collections.Generic;
 using System.Collections.Immutable;
 using System.Composition;
+using System.Linq;
 using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.CodeAnalysis.CodeFixes;
 using Microsoft.CodeAnalysis.CSharp.Extensions;
 using Microsoft.CodeAnalysis.CSharp.Syntax;
 using Microsoft.CodeAnalysis.Diagnostics;
 using Microsoft.CodeAnalysis.Editing;
+using Microsoft.CodeAnalysis.Formatting;
 using Microsoft.CodeAnalysis.Host.Mef;
 using Microsoft.CodeAnalysis.Shared.Extensions;
+using Microsoft.CodeAnalysis.Utilities;
 
 namespace Microsoft.CodeAnalysis.CSharp.RemoveUnnecessaryLambdaExpression;
 
@@ -39,7 +43,9 @@ protected override Task FixAllAsync(
     {
         foreach (var diagnostic in diagnostics)
         {
-            var anonymousFunction = diagnostic.AdditionalLocations[0].FindNode(getInnermostNodeForTie: true, cancellationToken);
+            var anonymousFunction = diagnostic.AdditionalLocations[0].FindNode(getInnermostNodeForTie: true, cancellationToken) as AnonymousFunctionExpressionSyntax;
+            if (anonymousFunction is null)
+                continue;
 
             editor.ReplaceNode(anonymousFunction,
                 (current, generator) =>
@@ -52,8 +58,32 @@ protected override Task FixAllAsync(
 
                     return current;
                 });
+
+            // If the inner invocation has important trivia on it, move it to the container of the anonymous function.
+            if (TryGetAnonymousFunctionInvocation(anonymousFunction, out var invocation, out _) &&
+                invocation.GetLeadingTrivia().Any(t => t.IsSingleOrMultiLineComment()))
+            {
+                var containingStatement = anonymousFunction.AncestorsAndSelf().OfType<StatementSyntax>().FirstOrDefault();
+                if (containingStatement != null)
+                {
+                    editor.ReplaceNode(containingStatement,
+                        (current, generator) => current
+                            .WithPrependedLeadingTrivia(TakeComments(invocation.GetLeadingTrivia()))
+                            .WithAdditionalAnnotations(Formatter.Annotation));
+                }
+            }
         }
 
         return Task.CompletedTask;
     }
+
+    private static IEnumerable<SyntaxTrivia> TakeComments(SyntaxTriviaList triviaList)
+    {
+        var lastComment = triviaList.Last(t => t.IsSingleOrMultiLineComment());
+        var lastIndex = triviaList.IndexOf(lastComment) + 1;
+        if (lastIndex < triviaList.Count && triviaList[lastIndex].IsEndOfLine())
+            lastIndex++;
+
+        return triviaList.Take(lastIndex);
+    }
 }

src/Analyzers/CSharp/Tests/RemoveUnnecessaryLambdaExpression/RemoveUnnecessaryLambdaExpressionTests.cs

@@ -2,6 +2,7 @@
 // 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.Diagnostics.CodeAnalysis;
 using System.Threading.Tasks;
 using Microsoft.CodeAnalysis.CodeStyle;
 using Microsoft.CodeAnalysis.CSharp;
@@ -19,11 +20,11 @@ namespace Microsoft.CodeAnalysis.Editor.CSharp.UnitTests.RemoveUnnecessaryLambda
    CSharpRemoveUnnecessaryLambdaExpressionCodeFixProvider>;
 
 [Trait(Traits.Feature, Traits.Features.CodeActionsRemoveUnnecessaryLambdaExpression)]
-public class RemoveUnnecessaryLambdaExpressionTests
+public sealed class RemoveUnnecessaryLambdaExpressionTests
 {
     private static async Task TestInRegularAndScriptAsync(
-        string testCode,
-        string fixedCode,
+        [StringSyntax(PredefinedEmbeddedLanguageNames.CSharpTest)] string testCode,
+        [StringSyntax(PredefinedEmbeddedLanguageNames.CSharpTest)] string fixedCode,
         LanguageVersion version = LanguageVersion.CSharp12,
         OutputKind? outputKind = null)
     {
@@ -40,7 +41,7 @@ private static async Task TestInRegularAndScriptAsync(
     }
 
     private static Task TestMissingInRegularAndScriptAsync(
-        string testCode,
+        [StringSyntax(PredefinedEmbeddedLanguageNames.CSharpTest)] string testCode,
         LanguageVersion version = LanguageVersion.CSharp12,
         OutputKind? outputKind = null)
         => TestInRegularAndScriptAsync(testCode, testCode, version, outputKind);
@@ -2034,4 +2035,44 @@ public string ToStr(int x)
             }
             """);
     }
+
+    [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/71300")]
+    public async Task PreserveComment()
+    {
+        await TestInRegularAndScriptAsync("""
+            using System;
+
+            class C
+            {
+                void M1()
+                {
+                    M2([|() =>
+                    {
+                        // I hope M2 doesn't call M1!
+                        |]M1();
+                    });
+                }
+
+                void M2(Action a)
+                {
+                }
+            }
+            """,
+            """
+            using System;
+
+            class C
+            {
+                void M1()
+                {
+                    // I hope M2 doesn't call M1!
+                    M2(M1);
+                }
+
+                void M2(Action a)
+                {
+                }
+            }
+            """);
+    }
 }

src/EditorFeatures/CSharpTest/QuickInfo/SemanticQuickInfoSourceTests.cs

@@ -516,7 +516,9 @@ void M()
 
         // SingleLine doc comment with '\r' line separators
         await TestAsync("""
-            ///<summary>Hello!            ///</summary>            class C { void M() { $$C obj; } }
+            ///<summary>Hello!
+            ///</summary>
+            class C { void M() { $$C obj; } }
             """,
             MainDescription("class C"),
             Documentation("Hello!"));
@@ -632,7 +634,12 @@ void M()
 
         // Multiline doc comment with '\r' line separators
         await TestAsync("""
-            /**            * <summary>            * Hello!            * </summary>            */            class C { void M() { $$C obj; } }
+            /**
+            * <summary>
+            * Hello!
+            * </summary>
+            */
+            class C { void M() { $$C obj; } }
             """,
             MainDescription("class C"),
             Documentation("Hello!"));