From 442970196d7daa7ca418e676f1f82e8f4caafe6d Mon Sep 17 00:00:00 2001 From: David Barbet Date: Wed, 2 Dec 2020 17:40:58 -0800 Subject: [PATCH] Extract out merge code into shared helper --- .../TextChangeRangeExtensions.cs | 338 ++++++++++++++++++ .../Core/Portable/Text/ChangedText.cs | 331 +---------------- .../AutomaticBraceCompletionTests.cs | 144 -------- .../CurlyBraceCompletionService.cs | 107 ++---- 4 files changed, 363 insertions(+), 557 deletions(-) diff --git a/src/Compilers/Core/Portable/InternalUtilities/TextChangeRangeExtensions.cs b/src/Compilers/Core/Portable/InternalUtilities/TextChangeRangeExtensions.cs index 14d426af23873..0521991ac2602 100644 --- a/src/Compilers/Core/Portable/InternalUtilities/TextChangeRangeExtensions.cs +++ b/src/Compilers/Core/Portable/InternalUtilities/TextChangeRangeExtensions.cs @@ -2,8 +2,12 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. +using System; using System.Collections.Generic; +using System.Collections.Immutable; +using System.Diagnostics; using System.Linq; +using Microsoft.CodeAnalysis.PooledObjects; using Microsoft.CodeAnalysis.Text; namespace Roslyn.Utilities @@ -94,5 +98,339 @@ internal static class TextChangeRangeExtensions return new TextChangeRange(TextSpan.FromBounds(currentStart, currentOldEnd), currentNewEnd - currentStart); } + + public static TextChangeRange ToTextChangeRange(this TextChange textChange) + { + return new TextChangeRange(textChange.Span, textChange.NewText?.Length ?? 0); + } + + /// + /// Merges the new change ranges into the old change ranges, adjusting the new ranges to be with respect to the original text + /// (with neither old or new changes applied) instead of with respect to the original text after "old changes" are applied. + /// + /// This may require splitting, concatenation, etc. of individual change ranges. + /// + /// + /// Both `oldChanges` and `newChanges` must contain non-overlapping spans in ascending order. + /// + public static ImmutableArray Merge(ImmutableArray oldChanges, ImmutableArray newChanges) + { + // Earlier steps are expected to prevent us from ever reaching this point with empty change sets. + if (oldChanges.IsEmpty) + { + throw new ArgumentException(nameof(oldChanges)); + } + + if (newChanges.IsEmpty) + { + throw new ArgumentException(nameof(newChanges)); + } + + var builder = ArrayBuilder.GetInstance(); + + var oldChange = oldChanges[0]; + var newChange = new UnadjustedNewChange(newChanges[0]); + + var oldIndex = 0; + var newIndex = 0; + + // The sum of characters inserted by old changes minus characters deleted by old changes. + // This value must be adjusted whenever characters from an old change are added to `builder`. + var oldDelta = 0; + + // In this loop we "zip" together potentially overlapping old and new changes. + // It's important that when overlapping changes are found, we don't consume past the end of the overlapping section until the next iteration. + // so that we don't miss scenarios where the section after the overlap we found itself overlaps with another change + // e.g.: + // [-------oldChange1------] + // [--newChange1--] [--newChange2--] + while (true) + { + if (oldChange.Span.Length == 0 && oldChange.NewLength == 0) + { + // old change does not insert or delete any characters, so it can be dropped to no effect. + if (tryGetNextOldChange()) continue; + else break; + } + else if (newChange.SpanLength == 0 && newChange.NewLength == 0) + { + // new change does not insert or delete any characters, so it can be dropped to no effect. + if (tryGetNextNewChange()) continue; + else break; + } + else if (newChange.SpanEnd <= oldChange.Span.Start + oldDelta) + { + // new change is entirely before old change, so just take the new change + // old[--------] + // new[--------] + adjustAndAddNewChange(builder, oldDelta, newChange); + if (tryGetNextNewChange()) continue; + else break; + } + else if (newChange.SpanStart >= oldChange.NewEnd() + oldDelta) + { + // new change is entirely after old change, so just take the old change + // old[--------] + // new[--------] + addAndAdjustOldDelta(builder, ref oldDelta, oldChange); + if (tryGetNextOldChange()) continue; + else break; + } + else if (newChange.SpanStart < oldChange.Span.Start + oldDelta) + { + // new change starts before old change, but the new change deletion overlaps with the old change insertion + // note: 'd' represents a deleted character, 'a' represents a character inserted by an old change, and 'b' represents a character inserted by a new change. + // + // old|dddddd| + // |aaaaaa| + // --------------- + // new|dddddd| + // |bbbbbb| + + // align the new change and old change start by consuming the part of the new deletion before the old change + // (this only deletes characters of the original text) + // + // old|dddddd| + // |aaaaaa| + // --------------- + // new|ddd| + // |bbbbbb| + var newChangeLeadingDeletion = oldChange.Span.Start + oldDelta - newChange.SpanStart; + adjustAndAddNewChange(builder, oldDelta, new UnadjustedNewChange(newChange.SpanStart, newChangeLeadingDeletion, newLength: 0)); + newChange = new UnadjustedNewChange(oldChange.Span.Start + oldDelta, newChange.SpanLength - newChangeLeadingDeletion, newChange.NewLength); + continue; + } + else if (newChange.SpanStart > oldChange.Span.Start + oldDelta) + { + // new change starts after old change, but overlaps + // + // old|dddddd| + // |aaaaaa| + // --------------- + // new|dddddd| + // |bbbbbb| + + // align the old change to the new change by consuming the part of the old change which is before the new change. + // + // old|ddd| + // |aaa| + // --------------- + // new|dddddd| + // |bbbbbb| + + var oldChangeLeadingInsertion = newChange.SpanStart - (oldChange.Span.Start + oldDelta); + // we must make sure to delete at most as many characters as the entire oldChange deletes + var oldChangeLeadingDeletion = Math.Min(oldChange.Span.Length, oldChangeLeadingInsertion); + addAndAdjustOldDelta(builder, ref oldDelta, new TextChangeRange(new TextSpan(oldChange.Span.Start, oldChangeLeadingDeletion), oldChangeLeadingInsertion)); + oldChange = new TextChangeRange(new TextSpan(newChange.SpanStart - oldDelta, oldChange.Span.Length - oldChangeLeadingDeletion), oldChange.NewLength - oldChangeLeadingInsertion); + continue; + } + else + { + // old and new change start at same adjusted position + Debug.Assert(newChange.SpanStart == oldChange.Span.Start + oldDelta); + + if (newChange.SpanLength <= oldChange.NewLength) + { + // new change deletes fewer characters than old change inserted + // + // old|dddddd| + // |aaaaaa| + // --------------- + // new|ddd| + // |bbbbbb| + + // - apply the new change deletion to the old change insertion + // + // old|dddddd| + // |aaa| + // --------------- + // new|| + // |bbbbbb| + // + // - move the new change insertion forward by the same amount as its consumed deletion to remain aligned with the old change. + // (because the old change and new change have the same adjusted start position, the new change insertion appears directly before the old change insertion in the final text) + // + // old|dddddd| + // |aaa| + // --------------- + // new|| + // |bbbbbb| + + oldChange = new TextChangeRange(oldChange.Span, oldChange.NewLength - newChange.SpanLength); + + // the new change deletion is equal to the subset of the old change insertion that we are consuming this iteration + oldDelta = oldDelta + newChange.SpanLength; + + // since the new change insertion occurs before the old change, consume it now + newChange = new UnadjustedNewChange(newChange.SpanEnd, spanLength: 0, newChange.NewLength); + adjustAndAddNewChange(builder, oldDelta, newChange); + if (tryGetNextNewChange()) continue; + else break; + } + else + { + // new change deletes more characters than old change inserted + // + // old|d| + // |aa| + // --------------- + // new|ddd| + // |bbb| + + // merge the old change into the new change: + // - new change deletion deletes all of the old change insertion. reduce the new change deletion accordingly + // + // old|d| + // || + // --------------- + // new|d| + // |bbb| + // + // - old change deletion is simply added to the new change deletion. + // + // old|| + // || + // --------------- + // new|dd| + // |bbb| + // + // - new change is moved to put its adjusted position equal to the old change we just merged in + // + // old|| + // || + // --------------- + // new|dd| + // |bbb| + + // adjust the oldDelta to reflect that the old change has been consumed + oldDelta = oldDelta - oldChange.Span.Length + oldChange.NewLength; + + var newDeletion = newChange.SpanLength + oldChange.Span.Length - oldChange.NewLength; + newChange = new UnadjustedNewChange(oldChange.Span.Start + oldDelta, newDeletion, newChange.NewLength); + if (tryGetNextOldChange()) continue; + else break; + } + } + } + + // there may be remaining old changes or remaining new changes (not both, and not neither) + switch (oldIndex == oldChanges.Length, newIndex == newChanges.Length) + { + case (true, true): + case (false, false): + throw new InvalidOperationException(); + } + + while (oldIndex < oldChanges.Length) + { + addAndAdjustOldDelta(builder, ref oldDelta, oldChange); + tryGetNextOldChange(); + } + + while (newIndex < newChanges.Length) + { + adjustAndAddNewChange(builder, oldDelta, newChange); + tryGetNextNewChange(); + } + + return builder.ToImmutableAndFree(); + + bool tryGetNextOldChange() + { + oldIndex++; + if (oldIndex < oldChanges.Length) + { + oldChange = oldChanges[oldIndex]; + return true; + } + else + { + oldChange = default; + return false; + } + } + + bool tryGetNextNewChange() + { + newIndex++; + if (newIndex < newChanges.Length) + { + newChange = new UnadjustedNewChange(newChanges[newIndex]); + return true; + } + else + { + newChange = default; + return false; + } + } + + static void addAndAdjustOldDelta(ArrayBuilder builder, ref int oldDelta, TextChangeRange oldChange) + { + // modify oldDelta to reflect characters deleted and inserted by an old change + oldDelta = oldDelta - oldChange.Span.Length + oldChange.NewLength; + add(builder, oldChange); + } + + static void adjustAndAddNewChange(ArrayBuilder builder, int oldDelta, UnadjustedNewChange newChange) + { + // unadjusted new change is relative to the original text with old changes applied. Subtract oldDelta to make it relative to the original text. + add(builder, new TextChangeRange(new TextSpan(newChange.SpanStart - oldDelta, newChange.SpanLength), newChange.NewLength)); + } + + static void add(ArrayBuilder builder, TextChangeRange change) + { + if (builder.Count > 0) + { + var last = builder[^1]; + if (last.Span.End == change.Span.Start) + { + // merge changes together if they are adjacent + builder[^1] = new TextChangeRange(new TextSpan(last.Span.Start, last.Span.Length + change.Span.Length), last.NewLength + change.NewLength); + return; + } + else if (last.Span.End > change.Span.Start) + { + throw new ArgumentOutOfRangeException(nameof(change)); + } + + } + + builder.Add(change); + } + } + + /// + /// Represents a new change being processed by . + /// Such a new change must be adjusted before being added to the result list. + /// + /// + /// A value of this type may represent the intermediate state of merging of an old change into an unadjusted new change, + /// resulting in a temporary unadjusted new change whose is negative (not valid) until it is adjusted. + /// This tends to happen when we need to merge an old change deletion into a new change near the beginning of the text. (see TextChangeTests.Fuzz_4) + /// + private readonly struct UnadjustedNewChange + { + public readonly int SpanStart { get; } + public readonly int SpanLength { get; } + public readonly int NewLength { get; } + + public int SpanEnd => SpanStart + SpanLength; + + public UnadjustedNewChange(int spanStart, int spanLength, int newLength) + { + SpanStart = spanStart; + SpanLength = spanLength; + NewLength = newLength; + } + + public UnadjustedNewChange(TextChangeRange range) + : this(range.Span.Start, range.Span.Length, range.NewLength) + { + } + } + + private static int NewEnd(this TextChangeRange range) => range.Span.Start + range.NewLength; } } diff --git a/src/Compilers/Core/Portable/Text/ChangedText.cs b/src/Compilers/Core/Portable/Text/ChangedText.cs index b20bd10baa0d0..090d3dae0f9a9 100644 --- a/src/Compilers/Core/Portable/Text/ChangedText.cs +++ b/src/Compilers/Core/Portable/Text/ChangedText.cs @@ -257,339 +257,12 @@ private static ImmutableArray Merge(IReadOnlyList - /// Represents a new change being processed by . - /// Such a new change must be adjusted before being added to the result list. - /// - /// - /// A value of this type may represent the intermediate state of merging of an old change into an unadjusted new change, - /// resulting in a temporary unadjusted new change whose is negative (not valid) until it is adjusted. - /// This tends to happen when we need to merge an old change deletion into a new change near the beginning of the text. (see TextChangeTests.Fuzz_4) - /// - private readonly struct UnadjustedNewChange - { - public readonly int SpanStart { get; } - public readonly int SpanLength { get; } - public readonly int NewLength { get; } - - public int SpanEnd => SpanStart + SpanLength; - - public UnadjustedNewChange(int spanStart, int spanLength, int newLength) - { - SpanStart = spanStart; - SpanLength = spanLength; - NewLength = newLength; - } - - public UnadjustedNewChange(TextChangeRange range) - : this(range.Span.Start, range.Span.Length, range.NewLength) - { - } - } - - /// - /// Merges the new change ranges into the old change ranges, adjusting the new ranges to be with respect to the original text - /// (with neither old or new changes applied) instead of with respect to the original text after "old changes" are applied. - /// - /// This may require splitting, concatenation, etc. of individual change ranges. - /// - /// - /// Both `oldChanges` and `newChanges` must contain non-overlapping spans in ascending order. - /// - private static ImmutableArray Merge(ImmutableArray oldChanges, ImmutableArray newChanges) - { - // Earlier steps are expected to prevent us from ever reaching this point with empty change sets. - if (oldChanges.IsEmpty) - { - throw new ArgumentException(nameof(oldChanges)); - } - - if (newChanges.IsEmpty) - { - throw new ArgumentException(nameof(newChanges)); - } - - var builder = ArrayBuilder.GetInstance(); - - var oldChange = oldChanges[0]; - var newChange = new UnadjustedNewChange(newChanges[0]); - - var oldIndex = 0; - var newIndex = 0; - - // The sum of characters inserted by old changes minus characters deleted by old changes. - // This value must be adjusted whenever characters from an old change are added to `builder`. - var oldDelta = 0; - - // In this loop we "zip" together potentially overlapping old and new changes. - // It's important that when overlapping changes are found, we don't consume past the end of the overlapping section until the next iteration. - // so that we don't miss scenarios where the section after the overlap we found itself overlaps with another change - // e.g.: - // [-------oldChange1------] - // [--newChange1--] [--newChange2--] - while (true) - { - if (oldChange.Span.Length == 0 && oldChange.NewLength == 0) - { - // old change does not insert or delete any characters, so it can be dropped to no effect. - if (tryGetNextOldChange()) continue; - else break; - } - else if (newChange.SpanLength == 0 && newChange.NewLength == 0) - { - // new change does not insert or delete any characters, so it can be dropped to no effect. - if (tryGetNextNewChange()) continue; - else break; - } - else if (newChange.SpanEnd <= oldChange.Span.Start + oldDelta) - { - // new change is entirely before old change, so just take the new change - // old[--------] - // new[--------] - adjustAndAddNewChange(builder, oldDelta, newChange); - if (tryGetNextNewChange()) continue; - else break; - } - else if (newChange.SpanStart >= oldChange.NewEnd + oldDelta) - { - // new change is entirely after old change, so just take the old change - // old[--------] - // new[--------] - addAndAdjustOldDelta(builder, ref oldDelta, oldChange); - if (tryGetNextOldChange()) continue; - else break; - } - else if (newChange.SpanStart < oldChange.Span.Start + oldDelta) - { - // new change starts before old change, but the new change deletion overlaps with the old change insertion - // note: 'd' represents a deleted character, 'a' represents a character inserted by an old change, and 'b' represents a character inserted by a new change. - // - // old|dddddd| - // |aaaaaa| - // --------------- - // new|dddddd| - // |bbbbbb| - - // align the new change and old change start by consuming the part of the new deletion before the old change - // (this only deletes characters of the original text) - // - // old|dddddd| - // |aaaaaa| - // --------------- - // new|ddd| - // |bbbbbb| - var newChangeLeadingDeletion = oldChange.Span.Start + oldDelta - newChange.SpanStart; - adjustAndAddNewChange(builder, oldDelta, new UnadjustedNewChange(newChange.SpanStart, newChangeLeadingDeletion, newLength: 0)); - newChange = new UnadjustedNewChange(oldChange.Span.Start + oldDelta, newChange.SpanLength - newChangeLeadingDeletion, newChange.NewLength); - continue; - } - else if (newChange.SpanStart > oldChange.Span.Start + oldDelta) - { - // new change starts after old change, but overlaps - // - // old|dddddd| - // |aaaaaa| - // --------------- - // new|dddddd| - // |bbbbbb| - - // align the old change to the new change by consuming the part of the old change which is before the new change. - // - // old|ddd| - // |aaa| - // --------------- - // new|dddddd| - // |bbbbbb| - - var oldChangeLeadingInsertion = newChange.SpanStart - (oldChange.Span.Start + oldDelta); - // we must make sure to delete at most as many characters as the entire oldChange deletes - var oldChangeLeadingDeletion = Math.Min(oldChange.Span.Length, oldChangeLeadingInsertion); - addAndAdjustOldDelta(builder, ref oldDelta, new TextChangeRange(new TextSpan(oldChange.Span.Start, oldChangeLeadingDeletion), oldChangeLeadingInsertion)); - oldChange = new TextChangeRange(new TextSpan(newChange.SpanStart - oldDelta, oldChange.Span.Length - oldChangeLeadingDeletion), oldChange.NewLength - oldChangeLeadingInsertion); - continue; - } - else - { - // old and new change start at same adjusted position - Debug.Assert(newChange.SpanStart == oldChange.Span.Start + oldDelta); - - if (newChange.SpanLength <= oldChange.NewLength) - { - // new change deletes fewer characters than old change inserted - // - // old|dddddd| - // |aaaaaa| - // --------------- - // new|ddd| - // |bbbbbb| - - // - apply the new change deletion to the old change insertion - // - // old|dddddd| - // |aaa| - // --------------- - // new|| - // |bbbbbb| - // - // - move the new change insertion forward by the same amount as its consumed deletion to remain aligned with the old change. - // (because the old change and new change have the same adjusted start position, the new change insertion appears directly before the old change insertion in the final text) - // - // old|dddddd| - // |aaa| - // --------------- - // new|| - // |bbbbbb| - - oldChange = new TextChangeRange(oldChange.Span, oldChange.NewLength - newChange.SpanLength); - - // the new change deletion is equal to the subset of the old change insertion that we are consuming this iteration - oldDelta = oldDelta + newChange.SpanLength; - - // since the new change insertion occurs before the old change, consume it now - newChange = new UnadjustedNewChange(newChange.SpanEnd, spanLength: 0, newChange.NewLength); - adjustAndAddNewChange(builder, oldDelta, newChange); - if (tryGetNextNewChange()) continue; - else break; - } - else - { - // new change deletes more characters than old change inserted - // - // old|d| - // |aa| - // --------------- - // new|ddd| - // |bbb| - - // merge the old change into the new change: - // - new change deletion deletes all of the old change insertion. reduce the new change deletion accordingly - // - // old|d| - // || - // --------------- - // new|d| - // |bbb| - // - // - old change deletion is simply added to the new change deletion. - // - // old|| - // || - // --------------- - // new|dd| - // |bbb| - // - // - new change is moved to put its adjusted position equal to the old change we just merged in - // - // old|| - // || - // --------------- - // new|dd| - // |bbb| - - // adjust the oldDelta to reflect that the old change has been consumed - oldDelta = oldDelta - oldChange.Span.Length + oldChange.NewLength; - - var newDeletion = newChange.SpanLength + oldChange.Span.Length - oldChange.NewLength; - newChange = new UnadjustedNewChange(oldChange.Span.Start + oldDelta, newDeletion, newChange.NewLength); - if (tryGetNextOldChange()) continue; - else break; - } - } - } - - // there may be remaining old changes or remaining new changes (not both, and not neither) - switch (oldIndex == oldChanges.Length, newIndex == newChanges.Length) - { - case (true, true): - case (false, false): - throw new InvalidOperationException(); - } - - while (oldIndex < oldChanges.Length) - { - addAndAdjustOldDelta(builder, ref oldDelta, oldChange); - tryGetNextOldChange(); - } - - while (newIndex < newChanges.Length) - { - adjustAndAddNewChange(builder, oldDelta, newChange); - tryGetNextNewChange(); - } - - return builder.ToImmutableAndFree(); - - bool tryGetNextOldChange() - { - oldIndex++; - if (oldIndex < oldChanges.Length) - { - oldChange = oldChanges[oldIndex]; - return true; - } - else - { - oldChange = default; - return false; - } - } - - bool tryGetNextNewChange() - { - newIndex++; - if (newIndex < newChanges.Length) - { - newChange = new UnadjustedNewChange(newChanges[newIndex]); - return true; - } - else - { - newChange = default; - return false; - } - } - - static void addAndAdjustOldDelta(ArrayBuilder builder, ref int oldDelta, TextChangeRange oldChange) - { - // modify oldDelta to reflect characters deleted and inserted by an old change - oldDelta = oldDelta - oldChange.Span.Length + oldChange.NewLength; - add(builder, oldChange); - } - - static void adjustAndAddNewChange(ArrayBuilder builder, int oldDelta, UnadjustedNewChange newChange) - { - // unadjusted new change is relative to the original text with old changes applied. Subtract oldDelta to make it relative to the original text. - add(builder, new TextChangeRange(new TextSpan(newChange.SpanStart - oldDelta, newChange.SpanLength), newChange.NewLength)); - } - - static void add(ArrayBuilder builder, TextChangeRange change) - { - if (builder.Count > 0) - { - var last = builder[^1]; - if (last.Span.End == change.Span.Start) - { - // merge changes together if they are adjacent - builder[^1] = new TextChangeRange(new TextSpan(last.Span.Start, last.Span.Length + change.Span.Length), last.NewLength + change.NewLength); - return; - } - else if (last.Span.End > change.Span.Start) - { - throw new ArgumentOutOfRangeException(nameof(change)); - } - - } - - builder.Add(change); - } - } - /// /// Computes line starts faster given already computed line starts from text before the change. /// @@ -694,7 +367,7 @@ protected override TextLineCollection GetLinesCore() internal static class TestAccessor { public static ImmutableArray Merge(ImmutableArray oldChanges, ImmutableArray newChanges) - => ChangedText.Merge(oldChanges, newChanges); + => TextChangeRangeExtensions.Merge(oldChanges, newChanges); } } } diff --git a/src/EditorFeatures/CSharpTest/AutomaticCompletion/AutomaticBraceCompletionTests.cs b/src/EditorFeatures/CSharpTest/AutomaticCompletion/AutomaticBraceCompletionTests.cs index 06022927ada5e..2d19ec09a903e 100644 --- a/src/EditorFeatures/CSharpTest/AutomaticCompletion/AutomaticBraceCompletionTests.cs +++ b/src/EditorFeatures/CSharpTest/AutomaticCompletion/AutomaticBraceCompletionTests.cs @@ -1348,150 +1348,6 @@ public void CurlyBraceFormatting_InsertsCorrectNewLine() CheckReturn(session.Session, 4, result: "class C\r{\r\r}"); } - [Fact, Trait(Traits.Feature, Traits.Features.AutomaticCompletion)] - public void CurlyBraceFormattingEditMerge_FormattingChangeEntirelyBefore() - { - var originalText = SourceText.From("aaaa"); - - // Text should be aabaa - var insertionTextChange = new TextChange(new TextSpan(2, 0), "b"); - var incrementalText = originalText.WithChanges(insertionTextChange); - - // Formatting change before insertion location - // Replace first 'a' with 'cc' to become ccabaa - var formattingTextChange = new TextChange(new TextSpan(0, 1), "cc"); - incrementalText = incrementalText.WithChanges(formattingTextChange); - - var mergedChanges = MergeFormatChangesIntoNewLineChange(insertionTextChange, ImmutableArray.Create(formattingTextChange)); - var mergedText = originalText.WithChanges(mergedChanges); - - Assert.Equal(insertionTextChange, mergedChanges[0]); - Assert.Equal(formattingTextChange, mergedChanges[1]); - Assert.Equal(incrementalText.ToString(), mergedText.ToString()); - } - - [Fact, Trait(Traits.Feature, Traits.Features.AutomaticCompletion)] - public void CurlyBraceFormattingEditMerge_FormattingChangeEntirelyAfter() - { - var originalText = SourceText.From("aaaa"); - - // Text should be aabaa - var insertionTextChange = new TextChange(new TextSpan(2, 0), "b"); - var incrementalText = originalText.WithChanges(insertionTextChange); - - // Formatting change entirely after insertion location - // Replace last 'a' with 'cc' to become aabacc - var formattingTextChange = new TextChange(new TextSpan(4, 1), "cc"); - incrementalText = incrementalText.WithChanges(formattingTextChange); - - var mergedChanges = MergeFormatChangesIntoNewLineChange(insertionTextChange, ImmutableArray.Create(formattingTextChange)); - var mergedText = originalText.WithChanges(mergedChanges); - - Assert.Equal(insertionTextChange, mergedChanges[0]); - // Formatting edit should be shifted to be relative to original text - Assert.Equal(new TextChange(new TextSpan(3, 1), "cc"), mergedChanges[1]); - Assert.Equal(incrementalText.ToString(), mergedText.ToString()); - } - - [Fact, Trait(Traits.Feature, Traits.Features.AutomaticCompletion)] - public void CurlyBraceFormattingEditMerge_OverlapsWithEndOfFormattingChange() - { - var originalText = SourceText.From("aaaa"); - - // Text should be aa#$aa - var insertionTextChange = new TextChange(new TextSpan(2, 0), "#$"); - var incrementalText = originalText.WithChanges(insertionTextChange); - - // Formatting starting before insertion (with overlap), ends inside insertion - // Replace 'a#' with 'cc' to become acc$aa - var formattingTextChange = new TextChange(new TextSpan(1, 2), "cc"); - incrementalText = incrementalText.WithChanges(formattingTextChange); - - var mergedChanges = MergeFormatChangesIntoNewLineChange(insertionTextChange, ImmutableArray.Create(formattingTextChange)); - var mergedText = originalText.WithChanges(mergedChanges); - - // The overlapping text is removed from the initial edit as it is covered by the formatting change. - Assert.Equal(new TextChange(new TextSpan(2, 0), "$"), mergedChanges[0]); - // The formatting change span is modified to end before the insertion edit. - Assert.Equal(new TextChange(new TextSpan(1, 1), "cc"), mergedChanges[1]); - Assert.Equal(incrementalText.ToString(), mergedText.ToString()); - } - - [Fact, Trait(Traits.Feature, Traits.Features.AutomaticCompletion)] - public void CurlyBraceFormattingEditMerge_OverlapsWithBeginningOfFormattingChange() - { - var originalText = SourceText.From("aaaa"); - - // Text should be aa#$aa - var insertionTextChange = new TextChange(new TextSpan(2, 0), "#$"); - var incrementalText = originalText.WithChanges(insertionTextChange); - - // Formatting starting inside insertion (with overlap), ends outside insertion - // Replace '$a' with 'cc' to become aa#cca - var formattingTextChange = new TextChange(new TextSpan(3, 2), "cc"); - incrementalText = incrementalText.WithChanges(formattingTextChange); - - var mergedChanges = MergeFormatChangesIntoNewLineChange(insertionTextChange, ImmutableArray.Create(formattingTextChange)); - var mergedText = originalText.WithChanges(mergedChanges); - - // The overlapping text is removed from the initial edit as it is covered by the formatting change. - Assert.Equal(new TextChange(new TextSpan(2, 0), "#"), mergedChanges[0]); - // The formatting change span is modified to end before the insertion edit. - Assert.Equal(new TextChange(new TextSpan(2, 1), "cc"), mergedChanges[1]); - Assert.Equal(incrementalText.ToString(), mergedText.ToString()); - } - - [Fact, Trait(Traits.Feature, Traits.Features.AutomaticCompletion)] - public void CurlyBraceFormattingEditMerge_NewLineInsideFormattingChange() - { - var originalText = SourceText.From("aaaa"); - - // Text should be aa#$aa - var insertionTextChange = new TextChange(new TextSpan(2, 0), "#$"); - var incrementalText = originalText.WithChanges(insertionTextChange); - - // Insertion change is entirely inside the formatting change. - // Replace '#$' with 'ccc' to become aacccaa - var formattingTextChange = new TextChange(new TextSpan(2, 2), "ccc"); - incrementalText = incrementalText.WithChanges(formattingTextChange); - - var mergedChanges = MergeFormatChangesIntoNewLineChange(insertionTextChange, ImmutableArray.Create(formattingTextChange)); - var mergedText = originalText.WithChanges(mergedChanges); - - // Only one edit is returned with the new text. - Assert.Equal(new TextChange(new TextSpan(2, 0), "ccc"), mergedChanges.Single()); - Assert.Equal(incrementalText.ToString(), mergedText.ToString()); - } - - [Fact, Trait(Traits.Feature, Traits.Features.AutomaticCompletion)] - public void CurlyBraceFormattingEditMerge_FormattingBeforeAndAfter() - { - var originalText = SourceText.From("aaaa"); - - // Text should be aa#$%aa - var insertionTextChange = new TextChange(new TextSpan(2, 0), "#$%"); - var incrementalText = originalText.WithChanges(insertionTextChange); - - // Multiple changes formatting changes before and after the insertion. - // Result should be ccdd$eeff - var entirelyBeforeChange = new TextChange(new TextSpan(0, 1), "cc"); - var partiallyBeforeChange = new TextChange(new TextSpan(1, 2), "dd"); - var partiallyAfterChange = new TextChange(new TextSpan(4, 2), "ee"); - var entirelyAfterChange = new TextChange(new TextSpan(6, 1), "ff"); - var changes = ImmutableArray.Create(entirelyBeforeChange, partiallyBeforeChange, partiallyAfterChange, entirelyAfterChange); - incrementalText = incrementalText.WithChanges(changes); - - var mergedChanges = MergeFormatChangesIntoNewLineChange(insertionTextChange, changes); - var mergedText = originalText.WithChanges(mergedChanges); - - Assert.Equal(new TextChange(new TextSpan(2, 0), "$"), mergedChanges[0]); - Assert.Equal(new TextChange(new TextSpan(0, 1), "cc"), mergedChanges[1]); - Assert.Equal(new TextChange(new TextSpan(1, 1), "dd"), mergedChanges[2]); - Assert.Equal(new TextChange(new TextSpan(2, 1), "ee"), mergedChanges[3]); - Assert.Equal(new TextChange(new TextSpan(3, 1), "ff"), mergedChanges[4]); - Assert.Equal(incrementalText.ToString(), mergedText.ToString()); - } - internal static Holder CreateSession(string code, Dictionary optionSet = null) { return CreateSession( diff --git a/src/Features/CSharp/Portable/BraceCompletion/CurlyBraceCompletionService.cs b/src/Features/CSharp/Portable/BraceCompletion/CurlyBraceCompletionService.cs index 429cc95581c1d..11fe1b620ab34 100644 --- a/src/Features/CSharp/Portable/BraceCompletion/CurlyBraceCompletionService.cs +++ b/src/Features/CSharp/Portable/BraceCompletion/CurlyBraceCompletionService.cs @@ -133,7 +133,7 @@ public override Task AllowOverTypeAsync(BraceCompletionContext context, Ca // The new line edit is calculated against the original text, d0, to get text d1. // The formatting edits are calculated against d1 to get text d2. // Merge the formatting and new line edits into a set of whitespace only text edits that all apply to d0. - var overallChanges = newLineEdit != null ? MergeFormatChangesIntoNewLineChange(newLineEdit.Value, formattingChanges) : formattingChanges; + var overallChanges = newLineEdit != null ? GetMergedChanges(newLineEdit.Value, formattingChanges, formattedText) : formattingChanges; return new BraceCompletionResult(overallChanges, caretPosition); static bool HasExistingEmptyLineBetweenBraces(int openingPoint, int closingPoint, SourceText sourceText) @@ -163,6 +163,28 @@ static LinePosition GetIndentedLinePosition(Document document, SourceText source var indentedLinePosition = new LinePosition(lineNumber, totalOffset); return indentedLinePosition; } + + static ImmutableArray GetMergedChanges(TextChange newLineEdit, ImmutableArray formattingChanges, SourceText formattedText) + { + var newRanges = TextChangeRangeExtensions.Merge( + ImmutableArray.Create(newLineEdit.ToTextChangeRange()), + formattingChanges.Select(f => f.ToTextChangeRange()).ToImmutableArray()); + + using var _ = ArrayBuilder.GetInstance(out var mergedChanges); + var amountToShift = 0; + foreach (var newRange in newRanges) + { + var newTextChangeSpan = newRange.Span; + // Get the text to put in the text change by looking at the span in the formatted text. + // As the new range start is relative to the original text, we need to adjust it assuming the previous changes were applied + // to get the correct start location in the formatted text. + var newTextChangeText = formattedText.GetSubText(new TextSpan(newRange.Span.Start + amountToShift, newRange.NewLength)).ToString(); + amountToShift += (newRange.NewLength - newRange.Span.Length); + mergedChanges.Add(new TextChange(newTextChangeSpan, newTextChangeText)); + } + + return mergedChanges.ToImmutable(); + } } public override async Task CanProvideBraceCompletionAsync(char brace, int openingPosition, Document document, CancellationToken cancellationToken) @@ -182,89 +204,6 @@ protected override bool IsValidOpeningBraceToken(SyntaxToken token) protected override bool IsValidClosingBraceToken(SyntaxToken token) => token.IsKind(SyntaxKind.CloseBraceToken); - /// - /// Given the original text, a text edit to insert a new line to the original text, and - /// a set of formatting text edits that apply to the text with the new line inserted. - /// Modify the formatting edits to be relative to the original text and return the set - /// of edits that can be applied to the original text to get the new line + formatted text. - /// - /// Visible for testing. - /// - internal static ImmutableArray MergeFormatChangesIntoNewLineChange(TextChange newLineEdit, ImmutableArray formattingEdits) - { - using var _ = ArrayBuilder.GetInstance(out var overallChanges); - - // There is always text in the new line edit as we construct it above. - var newLineText = newLineEdit.NewText!; - var newLineTextStart = newLineEdit.Span.Start; - var newLineTextEnd = newLineEdit.Span.End + newLineText.Length; - var newLineTextAfterMerge = newLineText; - foreach (var formattingEdit in formattingEdits) - { - var formattingEditText = formattingEdit.NewText ?? string.Empty; - if (formattingEdit.Span.End < newLineTextStart) - { - // The formatting change replacement span is entirely before where we added the new line, just take the change - // since the spans are already relative to the original text. - overallChanges.Add(formattingEdit); - } - else if (formattingEdit.Span.Start > newLineTextEnd) - { - // The formatting change replacement span is entirely after the text inserted by the new line change. - // We need to adjust the span by the amount that was inserted by the new line to make the change relative to the original text. - var adjustedFormatChange = new TextChange(new TextSpan(formattingEdit.Span.Start - newLineText.Length, formattingEdit.Span.Length), formattingEditText); - overallChanges.Add(adjustedFormatChange); - } - else - { - // The formatting change modifies locations that were inserted by the new line edit. - // There are three cases that cover the different types of overlap. - - // Case 1: The new line text is entirely contained within the formatting change replacement span. - // The formatting change text therefore has all of the new line text that should be inserted. - // Remove the new line edit and modify the formatting change so that the end is relative to the original text. - if (newLineTextStart >= formattingEdit.Span.Start && newLineTextEnd <= formattingEdit.Span.End) - { - newLineTextAfterMerge = string.Empty; - var adjustedFormatChange = new TextChange(new TextSpan(formattingEdit.Span.Start, formattingEdit.Span.Length - newLineText.Length), formattingEditText); - overallChanges.Add(adjustedFormatChange); - } - // Case 2: The end of the formatting change span overlaps with the beginning of the new line text. - // Remove the overlapping text from the new line edit (the text is included in the formatting change). - // Modify the formatting change span to be everything up to the new line edit start so there is not overlap. - else if (newLineTextStart >= formattingEdit.Span.Start) - { - var overlappingAmount = formattingEdit.Span.End - newLineTextStart; - var adjustedFormatChange = new TextChange(new TextSpan(formattingEdit.Span.Start, formattingEdit.Span.Length - overlappingAmount), formattingEditText); - - // Remove the overlap at the beginning of the new line text. - newLineTextAfterMerge = newLineTextAfterMerge.Remove(0, overlappingAmount); - overallChanges.Add(adjustedFormatChange); - } - // Case 3: The beginning of the formatting change span overlaps with the end of the new line text. - // Remove the overlapping text from the new line edit (the text is included in the formatting change). - // Modify the formatting change span to be everything after the new line edit end (and make the endpoint relative to the original text). - else - { - var overlappingAmount = newLineTextEnd - formattingEdit.Span.Start; - var adjustedFormatChange = new TextChange(new TextSpan(newLineEdit.Span.End, formattingEdit.Span.Length - overlappingAmount), formattingEditText); - - // Remove the overlap at the end of the new line text. - newLineTextAfterMerge = newLineTextAfterMerge.Substring(0, newLineTextAfterMerge.Length - overlappingAmount); - overallChanges.Add(adjustedFormatChange); - } - } - } - - if (newLineTextAfterMerge != string.Empty) - { - // Ensure the new line change comes before formatting changes in case of ties. - overallChanges.Insert(0, new TextChange(newLineEdit.Span, newLineTextAfterMerge)); - } - - return overallChanges.ToImmutable(); - } - private static bool ContainsOnlyWhitespace(SourceText text, int openingPosition, int closingBraceEndPoint) { // Set the start point to the character after the opening brace.