diff --git a/src/EditorFeatures/CSharpTest/Diagnostics/Suppression/SuppressionTest_FixMultipleTests.cs b/src/EditorFeatures/CSharpTest/Diagnostics/Suppression/SuppressionTest_FixMultipleTests.cs index 9297a1073f2580a35d3f1d8d3203a1f99bdcab05..c3f62ea38d963a101c469aa26ed5a1a5003bb429 100644 --- a/src/EditorFeatures/CSharpTest/Diagnostics/Suppression/SuppressionTest_FixMultipleTests.cs +++ b/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 { } diff --git a/src/Features/Core/Portable/CodeFixes/Suppression/AbstractSuppressionCodeFixProvider.PragmaBatchFixHelpers.cs b/src/Features/Core/Portable/CodeFixes/Suppression/AbstractSuppressionCodeFixProvider.PragmaBatchFixHelpers.cs index e6510604166bdc3692bf2ca50ac45538b0c630a3..15bfb14bb674ec80909199c8f3fbdc97a27ff66d 100644 --- a/src/Features/Core/Portable/CodeFixes/Suppression/AbstractSuppressionCodeFixProvider.PragmaBatchFixHelpers.cs +++ b/src/Features/Core/Portable/CodeFixes/Suppression/AbstractSuppressionCodeFixProvider.PragmaBatchFixHelpers.cs @@ -94,19 +94,22 @@ private static class PragmaBatchFixHelpers newSuppressionFix.Action.GetCodeActions().OfType().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> GetChangedDocumentAsync( + private static async Task> GetTextChangesAsync( IPragmaBasedCodeAction pragmaAction, Document currentDocument, ImmutableArray 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 diagnostics, Dictionary currentDiagnosticSpans, CancellationToken cancellationToken) + private static void UpdateDiagnosticSpans(ImmutableArray diagnostics, Dictionary currentDiagnosticSpans, IEnumerable 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 diagnostics, Dictionary currentDiagnosticSpans, TextChange textChange) - { - var isAdd = textChange.Span.Length == 0; - Func isPriorSpan = span => span.End <= textChange.Span.Start; - Func isFollowingSpan = span => span.Start >= textChange.Span.End; - Func isEnclosingSpan = span => span.Contains(textChange.Span); + Func isPriorSpan = (span, textChange) => span.End <= textChange.Span.Start; + Func isFollowingSpan = (span, textChange) => span.Start >= textChange.Span.End; + Func 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; + } } } } diff --git a/src/Features/Core/Portable/CodeFixes/Suppression/AbstractSuppressionCodeFixProvider.PragmaHelpers.cs b/src/Features/Core/Portable/CodeFixes/Suppression/AbstractSuppressionCodeFixProvider.PragmaHelpers.cs index 67d0c69fe67b309b1b35dc00541cb524d378341a..89b630c423d0b7592980b9777ebaba36c38f9821 100644 --- a/src/Features/Core/Portable/CodeFixes/Suppression/AbstractSuppressionCodeFixProvider.PragmaHelpers.cs +++ b/src/Features/Core/Portable/CodeFixes/Suppression/AbstractSuppressionCodeFixProvider.PragmaHelpers.cs @@ -79,9 +79,7 @@ private static int GetPositionForPragmaInsertion(ImmutableArray 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 formatNode, bool isRemoveSuppression = false) { ImmutableArray 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) {