Preserve trivia when simplifier removes unnecessary this / Me keywords.

Fixes dotnet#50
shyamnamboodiripad · Apr 14, 2015 · 83a9545 · 83a9545
1 parent 4d691ac
commit 83a9545
Show file tree

Hide file tree

Showing 3 changed files with 60 additions and 2 deletions.
diff --git a/src/EditorFeatures/Test2/Simplification/TypeNameSimplifierTest.vb b/src/EditorFeatures/Test2/Simplification/TypeNameSimplifierTest.vb
@@ -1426,6 +1426,42 @@ public class C
             Test(input, expected)
         End Sub
 
+        <Fact, Trait(Traits.Feature, Traits.Features.Simplification)>
+        <WorkItem(50, "https://github.com/dotnet/roslyn/issues/50")>
+        Public Sub TestCSRemoveThisPreservesTrivia()
+            Dim input =
+<Workspace>
+    <Project Language="C#" CommonReferences="true">
+        <Document>
+class C1
+{
+    int _field;
+
+    void M()
+    {
+        this /*comment 1*/ . /* comment 2 */ {|SimplifyParent:_field|} /* comment 3 */ = 0;
+    }
+}
+        </Document>
+    </Project>
+</Workspace>
+
+            Dim expected =
+<code>
+class C1
+{
+    int _field;
+
+    void M()
+    {
+         /*comment 1*/  /* comment 2 */ _field /* comment 3 */ = 0;
+    }
+}
+</code>
+
+            Test(input, expected)
+        End Sub
+
         <WorkItem(649385)>
         <Fact, Trait(Traits.Feature, Traits.Features.Simplification)>
         Public Sub CSharpSimplifyToVarCorrect()

diff --git a/src/Workspaces/CSharp/Portable/Extensions/ExpressionSyntaxExtensions.cs b/src/Workspaces/CSharp/Portable/Extensions/ExpressionSyntaxExtensions.cs
@@ -773,7 +773,19 @@ private static bool TryReduce(
                 }
             }
 
-            replacementNode = memberAccess.Name.WithLeadingTrivia(memberAccess.GetLeadingTrivia()).WithTrailingTrivia(memberAccess.GetTrailingTrivia());
+            // We want to include any user-typed trivia that may be present between the 'Expression', 'OperatorToken' and 'Identifier' of the MemberAccessExpression.
+            // However, we don't want to include any elastic trivia that may have been introduced by the expander in these locations. This is to avoid triggering
+            // aggressive formatting. Otherwise, formatter will see this elastic trivia added by the expander and use that as a cue to introduce unnecessary blank lines
+            // etc. around the user's original code.
+            Func<SyntaxTrivia, bool> isNotElastic = t => !t.IsElastic();
+            replacementNode = memberAccess.Name
+                .WithLeadingTrivia(
+                    memberAccess.GetLeadingTrivia()
+                    .AddRange(memberAccess.Expression.GetTrailingTrivia().Where(isNotElastic))
+                    .AddRange(memberAccess.OperatorToken.LeadingTrivia.Where(isNotElastic))
+                    .AddRange(memberAccess.OperatorToken.TrailingTrivia.Where(isNotElastic))
+                    .AddRange(memberAccess.Name.GetLeadingTrivia().Where(isNotElastic)))
+                .WithTrailingTrivia(memberAccess.GetTrailingTrivia());
             issueSpan = memberAccess.Expression.Span;
 
             if (replacementNode == null)

diff --git a/src/Workspaces/VisualBasic/Portable/Extensions/ExpressionSyntaxExtensions.vb b/src/Workspaces/VisualBasic/Portable/Extensions/ExpressionSyntaxExtensions.vb
@@ -1018,12 +1018,22 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Extensions
                     End If
                 End If
 
+                ' We want to include any user-typed trivia that may be present between the 'Expression', 'OperatorToken' and 'Identifier' of the MemberAccessExpression.
+                ' However, we don't want to include any elastic trivia that may have been introduced by the expander in these locations. This is to avoid triggering
+                ' aggressive formatting. Otherwise, formatter will see this elastic trivia added by the expander And use that as a cue to introduce unnecessary blank lines
+                ' etc. around the user's original code.
+                Dim isNotElastic = Function(t As SyntaxTrivia) Not t.IsElastic()
                 replacementNode = memberAccess.Name
                 replacementNode = DirectCast(replacementNode, SimpleNameSyntax) _
                     .WithIdentifier(VisualBasicSimplificationService.TryEscapeIdentifierToken(
                         memberAccess.Name.Identifier,
                         semanticModel)) _
-                            .WithLeadingTrivia(memberAccess.GetLeadingTrivia())
+                    .WithLeadingTrivia(memberAccess.GetLeadingTrivia() _
+                        .AddRange(memberAccess.Expression.GetTrailingTrivia().Where(isNotElastic)) _
+                        .AddRange(memberAccess.OperatorToken.LeadingTrivia.Where(isNotElastic)) _
+                        .AddRange(memberAccess.OperatorToken.TrailingTrivia.Where(isNotElastic)) _
+                        .AddRange(memberAccess.Name.GetLeadingTrivia().Where(isNotElastic))) _
+                    .WithTrailingTrivia(memberAccess.GetTrailingTrivia())
                 issueSpan = memberAccess.Expression.Span
 
                 If memberAccess.CanReplaceWithReducedName(replacementNode, semanticModel, cancellationToken) Then