Merge pull request #47723 from Cosifne/dev/shech/RemoveUnusedParameterFix

Sort the code fix action for UnusedValueCodeFixProvider

src/Analyzers/CSharp/Tests/RemoveUnusedParametersAndValues/RemoveUnusedValueAssignmentTests.cs

@@ -8683,5 +8683,56 @@ class C
 ";
             await TestExactActionSetOfferedAsync(source, new[] { CodeFixesResources.Remove_redundant_assignment });
         }
+
+        [WorkItem(38507, "https://github.com/dotnet/roslyn/issues/46251")]
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsRemoveUnusedValues)]
+        public async Task TestCodeFixForAllInDocumentForNestedDiagnostic()
+        {
+            var source = @"
+using System;
+namespace ConsoleApp
+{
+	public static class ConsoleApp
+    {
+		public static void Main(string[] args)
+        {
+            {|FixAllInDocument:Foo(() => { Bar(); return true; })|};
+        }
+
+        public static bool Foo(Func<bool> func)
+        {
+            return func. Invoke();
+        }
+
+        public static bool Bar()
+        {
+            return true;
+        }
+	}
+}";
+            var expected = @"
+using System;
+namespace ConsoleApp
+{
+	public static class ConsoleApp
+    {
+		public static void Main(string[] args)
+        {
+            _ = Foo(() => { _ = Bar(); return true; });
+        }
+
+        public static bool Foo(Func<bool> func)
+        {
+            return func. Invoke();
+        }
+
+        public static bool Bar()
+        {
+            return true;
+        }
+	}
+}";
+            await TestInRegularAndScriptAsync(source, expected, options: PreferDiscard).ConfigureAwait(false);
+        }
     }
 }

src/Analyzers/CSharp/Tests/RemoveUnusedParametersAndValues/RemoveUnusedValueExpressionStatementTests.cs

@@ -446,8 +446,8 @@ public C()
 
     void M(int unused1, int unused2)
     {{
-        {fix2} = M2();
-        {fix3} = M2();           // Another instance in same code block
+        {fix3} = M2();
+        {fix2} = M2();           // Another instance in same code block
         _ = M2();       // Already fixed
         var x = M2();   // Different unused value diagnostic
     }}

src/Analyzers/Core/CodeFixes/RemoveUnusedParametersAndValues/AbstractRemoveUnusedValuesCodeFixProvider.cs

@@ -295,11 +295,10 @@ protected sealed override async Task FixAllAsync(Document document, ImmutableArr
                 var semanticModel = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false);
                 foreach (var diagnosticsToFix in diagnosticsGroupedByMember)
                 {
-                    var orderedDiagnostics = diagnosticsToFix.OrderBy(d => d.Location.SourceSpan.Start);
                     var containingMemberDeclaration = diagnosticsToFix.Key;
                     using var nameGenerator = new UniqueVariableNameGenerator(containingMemberDeclaration, semanticModel, semanticFacts, cancellationToken);
 
-                    await FixAllAsync(diagnosticId, orderedDiagnostics, document, semanticModel, root, containingMemberDeclaration, preference,
+                    await FixAllAsync(diagnosticId, diagnosticsToFix.Select(d => d), document, semanticModel, root, containingMemberDeclaration, preference,
                         removeAssignments, nameGenerator, editor, syntaxFacts, cancellationToken).ConfigureAwait(false);
                 }
 
@@ -320,7 +319,7 @@ protected sealed override async Task FixAllAsync(Document document, ImmutableArr
 
         private async Task FixAllAsync(
             string diagnosticId,
-            IOrderedEnumerable<Diagnostic> diagnostics,
+            IEnumerable<Diagnostic> diagnostics,
             Document document,
             SemanticModel semanticModel,
             SyntaxNode root,
@@ -335,12 +334,17 @@ protected sealed override async Task FixAllAsync(Document document, ImmutableArr
             switch (diagnosticId)
             {
                 case IDEDiagnosticIds.ExpressionValueIsUnusedDiagnosticId:
-                    FixAllExpressionValueIsUnusedDiagnostics(diagnostics, semanticModel, root,
+                    // Make sure the inner diagnostics are placed first
+                    FixAllExpressionValueIsUnusedDiagnostics(diagnostics.OrderByDescending(d => d.Location.SourceSpan.Start), semanticModel, root,
                         preference, nameGenerator, editor, syntaxFacts);
                     break;
 
                 case IDEDiagnosticIds.ValueAssignedIsUnusedDiagnosticId:
-                    await FixAllValueAssignedIsUnusedDiagnosticsAsync(diagnostics, document, semanticModel, root, containingMemberDeclaration,
+                    // Make sure the diagnostics are placed in order.
+                    // Example: 
+                    // int a = 0; int b = 1;
+                    // After fix it would be int a; int b;
+                    await FixAllValueAssignedIsUnusedDiagnosticsAsync(diagnostics.OrderBy(d => d.Location.SourceSpan.Start), document, semanticModel, root, containingMemberDeclaration,
                         preference, removeAssignments, nameGenerator, editor, syntaxFacts, cancellationToken).ConfigureAwait(false);
                     break;
 
@@ -361,7 +365,9 @@ protected sealed override async Task FixAllAsync(Document document, ImmutableArr
             // This method applies the code fix for diagnostics reported for expression statement dropping values.
             // We replace each flagged expression statement with an assignment to a discard variable or a new unused local,
             // based on the user's preference.
-
+            // Note: The diagnostic order here should be inner first and outer second.
+            // Example: Foo1(() => { Foo2(); })
+            // Foo2() should be the first in this case.
             foreach (var diagnostic in diagnostics)
             {
                 var expressionStatement = root.FindNode(diagnostic.Location.SourceSpan).FirstAncestorOrSelf<TExpressionStatementSyntax>();
@@ -370,27 +376,35 @@ protected sealed override async Task FixAllAsync(Document document, ImmutableArr
                     continue;
                 }
 
-                var expression = syntaxFacts.GetExpressionOfExpressionStatement(expressionStatement);
                 switch (preference)
                 {
                     case UnusedValuePreference.DiscardVariable:
                         Debug.Assert(semanticModel.Language != LanguageNames.VisualBasic);
-                        var discardAssignmentExpression = (TExpressionSyntax)editor.Generator.AssignmentStatement(
-                                                                left: editor.Generator.IdentifierName(AbstractRemoveUnusedParametersAndValuesDiagnosticAnalyzer.DiscardVariableName),
-                                                                right: expression.WithoutTrivia())
-                                                            .WithTriviaFrom(expression)
-                                                            .WithAdditionalAnnotations(Simplifier.Annotation, Formatter.Annotation);
-                        editor.ReplaceNode(expression, discardAssignmentExpression);
+                        var expression = syntaxFacts.GetExpressionOfExpressionStatement(expressionStatement);
+                        editor.ReplaceNode(expression, (node, generator) =>
+                        {
+                            var discardAssignmentExpression = (TExpressionSyntax)generator.AssignmentStatement(
+                                                                    left: generator.IdentifierName(AbstractRemoveUnusedParametersAndValuesDiagnosticAnalyzer.DiscardVariableName),
+                                                                    right: node.WithoutTrivia())
+                                                                .WithTriviaFrom(node)
+                                                                .WithAdditionalAnnotations(Simplifier.Annotation, Formatter.Annotation);
+                            return discardAssignmentExpression;
+                        });
                         break;
 
                     case UnusedValuePreference.UnusedLocalVariable:
-                        // Add Simplifier annotation so that 'var'/explicit type is correctly added based on user options.
-                        var localDecl = editor.Generator.LocalDeclarationStatement(
-                                            name: nameGenerator.GenerateUniqueNameAtSpanStart(expressionStatement).ValueText,
-                                            initializer: expression.WithoutLeadingTrivia())
-                                        .WithTriviaFrom(expressionStatement)
-                                        .WithAdditionalAnnotations(Simplifier.Annotation, Formatter.Annotation);
-                        editor.ReplaceNode(expressionStatement, localDecl);
+                        var name = nameGenerator.GenerateUniqueNameAtSpanStart(expressionStatement).ValueText;
+                        editor.ReplaceNode(expressionStatement, (node, generator) =>
+                        {
+                            var expression = syntaxFacts.GetExpressionOfExpressionStatement(node);
+                            // Add Simplifier annotation so that 'var'/explicit type is correctly added based on user options.
+                            var localDecl = editor.Generator.LocalDeclarationStatement(
+                                                name: name,
+                                                initializer: expression.WithoutLeadingTrivia())
+                                            .WithTriviaFrom(node)
+                                            .WithAdditionalAnnotations(Simplifier.Annotation, Formatter.Annotation);
+                            return localDecl;
+                        });
                         break;
                 }
             }