Fix the batch pragma suppression fixer to handle the case where multiple...

Fix the batch pragma suppression fixer to handle the case where multiple diagnostics with different IDs are being suppressed on the same token

Current logic in batch pragma fixer merges individual fixes in a document by applying each pragma fix sequentially and keeping track of new diagnostics spans for remaining diagnostics. The latter logic that computes new spans was using out of date spans, causing the subsequent fixes to be incorrectly applied. This change fixes that logic.

Fixes #6455

src/EditorFeatures/CSharpTest/Diagnostics/Suppression/SuppressionTest_FixMultipleTests.cs

@@ -113,8 +113,8 @@ class Class2
 #pragma warning disable InfoDiagnostic // InfoDiagnostic Title
 #pragma warning disable InfoDiagnostic2 // InfoDiagnostic2 Title
 class Class1
-#pragma warning restore InfoDiagnostic // InfoDiagnostic Title
 #pragma warning restore InfoDiagnostic2 // InfoDiagnostic2 Title
+#pragma warning restore InfoDiagnostic // InfoDiagnostic Title
 {
     int Method()
     {
@@ -125,8 +125,8 @@ int Method()
 #pragma warning disable InfoDiagnostic // InfoDiagnostic Title
 #pragma warning disable InfoDiagnostic2 // InfoDiagnostic2 Title
 class Class2
-#pragma warning restore InfoDiagnostic // InfoDiagnostic Title
 #pragma warning restore InfoDiagnostic2 // InfoDiagnostic2 Title
+#pragma warning restore InfoDiagnostic // InfoDiagnostic Title
 {
 }
 class Class3 { }

src/Features/Core/Portable/CodeFixes/Suppression/AbstractSuppressionCodeFixProvider.PragmaBatchFixHelpers.cs

@@ -94,19 +94,22 @@ private static class PragmaBatchFixHelpers
                             newSuppressionFix.Action.GetCodeActions().OfType<IPragmaBasedCodeAction>().SingleOrDefault();
                         if (newPragmaAction != null)
                         {
-                            // Get the changed document with pragma suppression add/removals.
+                            // Get the text changes with pragma suppression add/removals.
                             // Note: We do it one token at a time to ensure we get single text change in the new document, otherwise UpdateDiagnosticSpans won't function as expected.
                             // Update the diagnostics spans based on the text changes.
-                            var startTokenChanges = await GetChangedDocumentAsync(newPragmaAction, currentDocument, diagnostics, currentDiagnosticSpans,
+                            var startTokenChanges = await GetTextChangesAsync(newPragmaAction, currentDocument, diagnostics, currentDiagnosticSpans,
                                 includeStartTokenChange: true, includeEndTokenChange: false, cancellationToken: cancellationToken).ConfigureAwait(false);
 
-                            var endTokenChanges = await GetChangedDocumentAsync(newPragmaAction, currentDocument, diagnostics, currentDiagnosticSpans,
+                            var endTokenChanges = await GetTextChangesAsync(newPragmaAction, currentDocument, diagnostics, currentDiagnosticSpans,
                                 includeStartTokenChange: false, includeEndTokenChange: true, cancellationToken: cancellationToken).ConfigureAwait(false);
 
                             var currentText = await currentDocument.GetTextAsync(cancellationToken).ConfigureAwait(false);
                             var orderedChanges = startTokenChanges.Concat(endTokenChanges).OrderBy(change => change.Span).Distinct();
                             var newText = currentText.WithChanges(orderedChanges);
                             currentDocument = currentDocument.WithText(newText);
+
+                            // Update the diagnostics spans based on the text changes.
+                            UpdateDiagnosticSpans(diagnostics, currentDiagnosticSpans, orderedChanges);
                         }
                     }
                 }
@@ -114,7 +117,7 @@ private static class PragmaBatchFixHelpers
                 return currentDocument;
             }
 
-            private static async Task<IEnumerable<TextChange>> GetChangedDocumentAsync(
+            private static async Task<IEnumerable<TextChange>> GetTextChangesAsync(
                 IPragmaBasedCodeAction pragmaAction,
                 Document currentDocument,
                 ImmutableArray<Diagnostic> diagnostics,
@@ -123,71 +126,59 @@ private static class PragmaBatchFixHelpers
                 bool includeEndTokenChange,
                 CancellationToken cancellationToken)
             {
-                var newDocument = await pragmaAction.GetChangedDocumentAsync(includeStartTokenChange, includeEndTokenChange, cancellationToken).ConfigureAwait(false);
-                
-                // Update the diagnostics spans based on the text changes.
-                var textChanges = await newDocument.GetTextChangesAsync(currentDocument, cancellationToken).ConfigureAwait(false);
-                foreach (var textChange in textChanges)
-                {
-                    UpdateDiagnosticSpans(diagnostics, currentDiagnosticSpans, textChange);
-                }
-
-                return textChanges;
+                var newDocument = await pragmaAction.GetChangedDocumentAsync(includeStartTokenChange, includeEndTokenChange, cancellationToken).ConfigureAwait(false);                
+                return await newDocument.GetTextChangesAsync(currentDocument, cancellationToken).ConfigureAwait(false);
             }
 
-            private static async Task UpdateDiagnosticSpansAsync(Document currentDocument, Document newDocument, ImmutableArray<Diagnostic> diagnostics, Dictionary<Diagnostic, TextSpan> currentDiagnosticSpans, CancellationToken cancellationToken)
+            private static void UpdateDiagnosticSpans(ImmutableArray<Diagnostic> diagnostics, Dictionary<Diagnostic, TextSpan> currentDiagnosticSpans, IEnumerable<TextChange> textChanges)
             {
-                // Update the diagnostics spans based on the text changes.
-                var textChanges = await newDocument.GetTextChangesAsync(currentDocument, cancellationToken).ConfigureAwait(false);
-                foreach (var textChange in textChanges)
-                {
-                    UpdateDiagnosticSpans(diagnostics, currentDiagnosticSpans, textChange);
-                }
-            }
-
-            private static void UpdateDiagnosticSpans(ImmutableArray<Diagnostic> diagnostics, Dictionary<Diagnostic, TextSpan> currentDiagnosticSpans, TextChange textChange)
-            {
-                var isAdd = textChange.Span.Length == 0;
-                Func<TextSpan, bool> isPriorSpan = span => span.End <= textChange.Span.Start;
-                Func<TextSpan, bool> isFollowingSpan = span => span.Start >= textChange.Span.End;
-                Func<TextSpan, bool> isEnclosingSpan = span => span.Contains(textChange.Span);
+                Func<TextSpan, TextChange, bool> isPriorSpan = (span, textChange) => span.End <= textChange.Span.Start;
+                Func<TextSpan, TextChange, bool> isFollowingSpan = (span, textChange) => span.Start >= textChange.Span.End;
+                Func<TextSpan, TextChange, bool> isEnclosingSpan = (span, textChange) => span.Contains(textChange.Span);
 
                 foreach (var diagnostic in diagnostics)
                 {
-                    TextSpan currentSpan;
-                    if (!currentDiagnosticSpans.TryGetValue(diagnostic, out currentSpan))
-                    {
-                        continue;
-                    }
-
-                    if (isPriorSpan(currentSpan))
+                    // We use 'originalSpan' to identify if the diagnostic is prior/following/enclosing with respect to each text change.
+                    // We use 'currentSpan' to track updates made to the originalSpan by each text change.
+                    TextSpan originalSpan;
+                    if (!currentDiagnosticSpans.TryGetValue(diagnostic, out originalSpan))
                     {
-                        // Prior span, needs no update.
                         continue;
                     }
 
-                    var delta = textChange.NewText.Length - textChange.Span.Length;
-                    if (delta != 0)
+                    var currentSpan = originalSpan;
+                    foreach (var textChange in textChanges)
                     {
-                        if (isFollowingSpan(currentSpan))
-                        {
-                            // Following span.
-                            var newStart = currentSpan.Start + delta;
-                            var newSpan = new TextSpan(newStart, currentSpan.Length);
-                            currentDiagnosticSpans[diagnostic] = newSpan;
-                        }
-                        else if (isEnclosingSpan(currentSpan))
+                        if (isPriorSpan(originalSpan, textChange))
                         {
-                            // Enclosing span.
-                            var newLength = currentSpan.Length + delta;
-                            var newSpan = new TextSpan(currentSpan.Start, newLength);
-                            currentDiagnosticSpans[diagnostic] = newSpan;
+                            // Prior span, needs no update.
+                            continue;
                         }
-                        else
+
+                        var delta = textChange.NewText.Length - textChange.Span.Length;
+                        if (delta != 0)
                         {
-                            // Overlapping span.
-                            // Drop conflicting diagnostics.
-                            currentDiagnosticSpans.Remove(diagnostic);
+                            if (isFollowingSpan(originalSpan, textChange))
+                            {
+                                // Following span.
+                                var newStart = currentSpan.Start + delta;
+                                currentSpan = new TextSpan(newStart, currentSpan.Length);
+                                currentDiagnosticSpans[diagnostic] = currentSpan;
+                            }
+                            else if (isEnclosingSpan(originalSpan, textChange))
+                            {
+                                // Enclosing span.
+                                var newLength = currentSpan.Length + delta;
+                                currentSpan = new TextSpan(currentSpan.Start, newLength);
+                                currentDiagnosticSpans[diagnostic] = currentSpan;
+                            }
+                            else
+                            {
+                                // Overlapping span.
+                                // Drop conflicting diagnostics.
+                                currentDiagnosticSpans.Remove(diagnostic);
+                                break;
+                            }
                         }
                     }
                 }

src/Features/Core/Portable/CodeFixes/Suppression/AbstractSuppressionCodeFixProvider.PragmaHelpers.cs

@@ -79,9 +79,7 @@ private static int GetPositionForPragmaInsertion(ImmutableArray<SyntaxTrivia> tr
 
                     walkedPastDiagnosticSpan = walkedPastDiagnosticSpan || shouldConsiderTrivia(trivia);
                     seenEndOfLineTrivia = seenEndOfLineTrivia ||
-                        (fixer.IsEndOfLine(trivia) || 
-                         (trivia.HasStructure &&
-                          trivia.GetStructure().DescendantTrivia().Any(t => fixer.IsEndOfLine(t))));
+                        IsEndOfLineOrContainsEndOfLine(trivia, fixer);
 
                     if (walkedPastDiagnosticSpan && seenEndOfLineTrivia)
                     {
@@ -108,7 +106,7 @@ internal static SyntaxToken GetNewStartTokenWithAddedPragma(SyntaxToken startTok
                 bool needsLeadingEOL;
                 if (index > 0)
                 {
-                    needsLeadingEOL = !fixer.IsEndOfLine(insertAfterTrivia);
+                    needsLeadingEOL = !IsEndOfLineOrHasTrailingEndOfLine(insertAfterTrivia, fixer);
                 }
                 else if (startToken.FullSpan.Start == 0)
                 {
@@ -126,6 +124,24 @@ internal static SyntaxToken GetNewStartTokenWithAddedPragma(SyntaxToken startTok
                 return startToken.WithLeadingTrivia(trivia.InsertRange(index, pragmaTrivia));
             }
 
+            private static bool IsEndOfLineOrHasLeadingEndOfLine(SyntaxTrivia trivia, AbstractSuppressionCodeFixProvider fixer)
+            {
+                return fixer.IsEndOfLine(trivia) ||
+                    (trivia.HasStructure && fixer.IsEndOfLine(trivia.GetStructure().DescendantTrivia().FirstOrDefault()));
+            }
+
+            private static bool IsEndOfLineOrHasTrailingEndOfLine(SyntaxTrivia trivia, AbstractSuppressionCodeFixProvider fixer)
+            {
+                return fixer.IsEndOfLine(trivia) ||
+                    (trivia.HasStructure && fixer.IsEndOfLine(trivia.GetStructure().DescendantTrivia().LastOrDefault()));
+            }
+
+            private static bool IsEndOfLineOrContainsEndOfLine(SyntaxTrivia trivia, AbstractSuppressionCodeFixProvider fixer)
+            {
+                return fixer.IsEndOfLine(trivia) ||
+                    (trivia.HasStructure && trivia.GetStructure().DescendantTrivia().Any(t => fixer.IsEndOfLine(t)));
+            }
+
             internal static SyntaxToken GetNewEndTokenWithAddedPragma(SyntaxToken endToken, TextSpan currentDiagnosticSpan, Diagnostic diagnostic, AbstractSuppressionCodeFixProvider fixer, Func<SyntaxNode, SyntaxNode> formatNode, bool isRemoveSuppression = false)
             {
                 ImmutableArray<SyntaxTrivia> trivia;
@@ -145,7 +161,7 @@ internal static SyntaxToken GetNewEndTokenWithAddedPragma(SyntaxToken endToken,
                 bool needsTrailingEOL;
                 if (index < trivia.Length)
                 {
-                    needsTrailingEOL = !fixer.IsEndOfLine(insertBeforeTrivia);
+                    needsTrailingEOL = !IsEndOfLineOrHasLeadingEndOfLine(insertBeforeTrivia, fixer);
                 }
                 else if (isEOF)
                 {