Address PR feedback from Cyrus and Heejae

src/EditorFeatures/CSharpTest/ChangeSignature/ChangeSignature_Formatting.cs

@@ -346,6 +346,71 @@ class CustomAttribute : System.Attribute
             await TestChangeSignatureViaCommandAsync(LanguageNames.CSharp, markup, updatedSignature: updatedSignature, expectedUpdatedInvocationDocumentCode: expectedUpdatedCode);
         }
 
+        [WpfFact, Trait(Traits.Feature, Traits.Features.ChangeSignature)]
+        [WorkItem(28156, "https://github.com/dotnet/roslyn/issues/28156")]
+        public async Task ChangeSignature_Formatting_Attribute_KeepTrivia_RemovingSecond()
+        {
+            var markup = @"
+[Custom(
+    1, 2)]
+class CustomAttribute : System.Attribute
+{
+    public $$CustomAttribute(int x, int y) { }
+}";
+            var updatedSignature = new[] { 0 };
+            var expectedUpdatedCode = @"
+[Custom(
+    1)]
+class CustomAttribute : System.Attribute
+{
+    public CustomAttribute(int x) { }
+}";
+            await TestChangeSignatureViaCommandAsync(LanguageNames.CSharp, markup, updatedSignature: updatedSignature, expectedUpdatedInvocationDocumentCode: expectedUpdatedCode);
+        }
+
+        [WpfFact, Trait(Traits.Feature, Traits.Features.ChangeSignature)]
+        [WorkItem(28156, "https://github.com/dotnet/roslyn/issues/28156")]
+        public async Task ChangeSignature_Formatting_Attribute_KeepTrivia_RemovingBoth()
+        {
+            var markup = @"
+[Custom(
+    1, 2)]
+class CustomAttribute : System.Attribute
+{
+    public $$CustomAttribute(int x, int y) { }
+}";
+            var updatedSignature = new int[] { };
+            var expectedUpdatedCode = @"
+[Custom(
+)]
+class CustomAttribute : System.Attribute
+{
+    public CustomAttribute() { }
+}";
+            await TestChangeSignatureViaCommandAsync(LanguageNames.CSharp, markup, updatedSignature: updatedSignature, expectedUpdatedInvocationDocumentCode: expectedUpdatedCode);
+        }
+
+        [WpfFact, Trait(Traits.Feature, Traits.Features.ChangeSignature)]
+        [WorkItem(28156, "https://github.com/dotnet/roslyn/issues/28156")]
+        public async Task ChangeSignature_Formatting_Attribute_KeepTrivia_RemovingBeforeNewlineComma()
+        {
+            var markup = @"
+[Custom(1
+    , 2, 3)]
+class CustomAttribute : System.Attribute
+{
+    public $$CustomAttribute(int x, int y, int z) { }
+}";
+            var updatedSignature = new[] { 1, 2 };
+            var expectedUpdatedCode = @"
+[Custom(2, 3)]
+class CustomAttribute : System.Attribute
+{
+    public CustomAttribute(int y, int z) { }
+}";
+            await TestChangeSignatureViaCommandAsync(LanguageNames.CSharp, markup, updatedSignature: updatedSignature, expectedUpdatedInvocationDocumentCode: expectedUpdatedCode);
+        }
+
         [WorkItem(946220, "http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/946220")]
         [WpfFact, Trait(Traits.Feature, Traits.Features.ChangeSignature)]
         public async Task ChangeSignature_Formatting_LambdaAsArgument()

src/Features/CSharp/Portable/ChangeSignature/CSharpChangeSignatureService.cs

@@ -406,9 +406,9 @@ private SyntaxNode GetNodeContainingTargetNode(SyntaxNode matchingNode)
             var reorderedParameters = updatedSignature.UpdatedConfiguration.ToListOfParameters();
 
             var newParameters = new List<T>();
-            int index = 0;
-            foreach (var newParam in reorderedParameters)
+            for (var index = 0; index < reorderedParameters.Count; index++)
             {
+                var newParam = reorderedParameters[index];
                 var pos = originalParameters.IndexOf(newParam);
                 var param = list[pos];
 
@@ -416,7 +416,6 @@ private SyntaxNode GetNodeContainingTargetNode(SyntaxNode matchingNode)
                 param = TransferLeadingWhitespaceTrivia(param, list[index]);
 
                 newParameters.Add(param);
-                index++;
             }
 
             var numSeparatorsToSkip = originalParameters.Count - reorderedParameters.Count;
@@ -433,7 +432,7 @@ private SyntaxNode GetNodeContainingTargetNode(SyntaxNode matchingNode)
 
             if (oldOnlyHasWhitespaceTrivia && newOnlyHasWhitespaceTrivia)
             {
-                return newArgument.WithLeadingTrivia(oldTrivia);
+                newArgument = newArgument.WithLeadingTrivia(oldTrivia);
             }
 
             return newArgument;
@@ -449,7 +448,8 @@ private SyntaxNode GetNodeContainingTargetNode(SyntaxNode matchingNode)
             var numSeparatorsToSkip = arguments.Count - newArguments.Count;
 
             // copy whitespace trivia from original position
-            var newArgumentsWithTrivia = TransferLeadingWhitespaceTrivia(newArguments.Select(a => (AttributeArgumentSyntax)(UnifiedArgumentSyntax)a), arguments);
+            var newArgumentsWithTrivia = TransferLeadingWhitespaceTrivia(
+                newArguments.Select(a => (AttributeArgumentSyntax)(UnifiedArgumentSyntax)a), arguments);
 
             return SyntaxFactory.SeparatedList(newArgumentsWithTrivia, GetSeparators(arguments, numSeparatorsToSkip));
         }
@@ -463,7 +463,9 @@ private SyntaxNode GetNodeContainingTargetNode(SyntaxNode matchingNode)
         {
             var newArguments = PermuteArguments(document, declarationSymbol, arguments.Select(a => UnifiedArgumentSyntax.Create(a)).ToList(), updatedSignature, isReducedExtensionMethod);
 
-            var newArgumentsWithTrivia = TransferLeadingWhitespaceTrivia(newArguments.Select(a => (ArgumentSyntax)(UnifiedArgumentSyntax)a), arguments); // copy whitespace trivia from original position
+            // copy whitespace trivia from original position
+            var newArgumentsWithTrivia = TransferLeadingWhitespaceTrivia(
+                newArguments.Select(a => (ArgumentSyntax)(UnifiedArgumentSyntax)a), arguments);
 
             var numSeparatorsToSkip = arguments.Count - newArguments.Count;
             return SyntaxFactory.SeparatedList(newArgumentsWithTrivia, GetSeparators(arguments, numSeparatorsToSkip));