Skip to content

Commit e7fe653

Browse files
authored
Compute TextChange based on syntax instead of text (#67120)
* Compute TextChange based on syntax instead of text * Check for empty change range before merging
1 parent 3119a8f commit e7fe653

File tree

2 files changed

+66
-29
lines changed

2 files changed

+66
-29
lines changed

src/EditorFeatures/CSharpTest/AutomaticCompletion/AutomaticBraceCompletionTests.cs

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,16 +3,12 @@
33
// See the LICENSE file in the project root for more information.
44

55
using System;
6-
using System.Collections.Generic;
7-
using Microsoft.CodeAnalysis.BraceCompletion;
86
using Microsoft.CodeAnalysis.CSharp.Formatting;
97
using Microsoft.CodeAnalysis.Editor.UnitTests.AutomaticCompletion;
108
using Microsoft.CodeAnalysis.Editor.UnitTests.CodeActions;
119
using Microsoft.CodeAnalysis.Editor.UnitTests.Workspaces;
1210
using Microsoft.CodeAnalysis.Formatting;
13-
using Microsoft.CodeAnalysis.Options;
1411
using Microsoft.CodeAnalysis.Test.Utilities;
15-
using Microsoft.VisualStudio.Text.Editor;
1612
using Roslyn.Test.Utilities;
1713
using Xunit;
1814
using static Microsoft.CodeAnalysis.BraceCompletion.AbstractBraceCompletionService;
@@ -1665,6 +1661,36 @@ public void man()
16651661
CheckReturn(session.Session, 12, expected);
16661662
}
16671663

1664+
[WpfFact, WorkItem("https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1758005")]
1665+
public void NoFormattingAfterNewlineIfOptionsDisabled()
1666+
{
1667+
var code = @"namespace NS1
1668+
$$";
1669+
1670+
var expected = @"namespace NS1
1671+
{}";
1672+
1673+
var expectedAfterReturn = @"namespace NS1
1674+
{
1675+
1676+
}";
1677+
1678+
// Those option ensures no additional formatting would happen around added braces, including indention of added newline
1679+
var globalOptions = new OptionsCollection(LanguageNames.CSharp)
1680+
{
1681+
{ FormattingOptions2.SmartIndent, FormattingOptions2.IndentStyle.None },
1682+
{ AutoFormattingOptionsStorage.FormatOnCloseBrace, false },
1683+
};
1684+
1685+
using var session = CreateSession(code, globalOptions);
1686+
Assert.NotNull(session);
1687+
1688+
CheckStart(session.Session);
1689+
Assert.Equal(expected, session.Session.SubjectBuffer.CurrentSnapshot.GetText());
1690+
1691+
CheckReturn(session.Session, 0, expectedAfterReturn);
1692+
}
1693+
16681694
internal static Holder CreateSession(string code, OptionsCollection? globalOptions = null)
16691695
{
16701696
return CreateSession(

src/Features/CSharp/Portable/BraceCompletion/AbstractCurlyBraceOrBracketCompletionService.cs

Lines changed: 36 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
using System.Linq;
99
using System.Threading;
1010
using Microsoft.CodeAnalysis.BraceCompletion;
11-
using Microsoft.CodeAnalysis.Editing;
1211
using Microsoft.CodeAnalysis.Formatting;
1312
using Microsoft.CodeAnalysis.Formatting.Rules;
1413
using Microsoft.CodeAnalysis.Indentation;
@@ -25,7 +24,8 @@ internal abstract class AbstractCurlyBraceOrBracketCompletionService : AbstractC
2524
/// Annotation used to find the closing brace location after formatting changes are applied.
2625
/// The closing brace location is then used as the caret location.
2726
/// </summary>
28-
private static readonly SyntaxAnnotation s_closingBraceSyntaxAnnotation = new(nameof(s_closingBraceSyntaxAnnotation));
27+
private static readonly SyntaxAnnotation s_closingBraceFormatAnnotation = new(nameof(s_closingBraceFormatAnnotation));
28+
private static readonly SyntaxAnnotation s_closingBraceNewlineAnnotation = new(nameof(s_closingBraceNewlineAnnotation));
2929

3030
protected abstract ImmutableArray<AbstractFormattingRule> GetBraceFormattingIndentationRulesAfterReturn(IndentationOptions options);
3131

@@ -115,18 +115,17 @@ private static bool ContainsOnlyWhitespace(SourceText text, int openingPosition,
115115
if (closingPointLine - openingPointLine == 1)
116116
{
117117
// Handling syntax tree directly to avoid parsing in potentially UI blocking code-path
118-
var closingToken = document.Root.FindTokenOnLeftOfPosition(context.ClosingPoint);
119-
var newLineString = options.FormattingOptions.NewLine;
120-
newLineEdit = new TextChange(new TextSpan(closingToken.FullSpan.Start, 0), newLineString);
118+
var closingToken = FindClosingBraceToken(document.Root, closingPoint);
119+
var annotatedNewline = SyntaxFactory.EndOfLine(options.FormattingOptions.NewLine).WithAdditionalAnnotations(s_closingBraceNewlineAnnotation);
120+
var newClosingToken = closingToken.WithPrependedLeadingTrivia(SpecializedCollections.SingletonEnumerable(annotatedNewline));
121121

122-
var generator = document.LanguageServices.GetRequiredService<SyntaxGeneratorInternal>();
123-
var endOfLine = generator.EndOfLine(newLineString);
124-
125-
var rootToFormat = document.Root.ReplaceToken(closingToken, closingToken.WithPrependedLeadingTrivia(endOfLine));
122+
var rootToFormat = document.Root.ReplaceToken(closingToken, newClosingToken);
123+
annotatedNewline = rootToFormat.GetAnnotatedTrivia(s_closingBraceNewlineAnnotation).Single();
126124
document = document.WithChangedRoot(rootToFormat, cancellationToken);
127125

128-
// Modify the closing point location to adjust for the newly inserted line.
129-
closingPoint += newLineString.Length;
126+
// Calculate text change for adding a newline and adjust closing point location.
127+
closingPoint = annotatedNewline.Token.Span.End;
128+
newLineEdit = new TextChange(new TextSpan(annotatedNewline.SpanStart, 0), annotatedNewline.ToString());
130129
}
131130

132131
// Format the text that contains the newly inserted line.
@@ -147,11 +146,7 @@ private static bool ContainsOnlyWhitespace(SourceText text, int openingPosition,
147146
// Set the caret position to the properly indented column in the desired line.
148147
var caretPosition = GetIndentedLinePosition(newDocument, newDocument.Text, desiredCaretLine.LineNumber, options, cancellationToken);
149148

150-
// The new line edit is calculated against the original text, d0, to get text d1.
151-
// The formatting edits are calculated against d1 to get text d2.
152-
// Merge the formatting and new line edits into a set of whitespace only text edits that all apply to d0.
153-
var overallChanges = newLineEdit != null ? GetMergedChanges(newLineEdit.Value, formattingChanges, newDocument.Text) : formattingChanges;
154-
return new BraceCompletionResult(overallChanges, caretPosition);
149+
return new BraceCompletionResult(GetMergedChanges(newLineEdit, formattingChanges, newDocument.Text), caretPosition);
155150

156151
static TextLine GetLineBetweenCurlys(int closingPosition, SourceText text)
157152
{
@@ -165,16 +160,27 @@ static LinePosition GetIndentedLinePosition(ParsedDocument document, SourceText
165160
var indentation = indentationService.GetIndentation(document, lineNumber, options, cancellationToken);
166161

167162
var baseLinePosition = sourceText.Lines.GetLinePosition(indentation.BasePosition);
168-
var offsetOfBacePosition = baseLinePosition.Character;
169-
var totalOffset = offsetOfBacePosition + indentation.Offset;
163+
var offsetOfBasePosition = baseLinePosition.Character;
164+
var totalOffset = offsetOfBasePosition + indentation.Offset;
170165
var indentedLinePosition = new LinePosition(lineNumber, totalOffset);
171166
return indentedLinePosition;
172167
}
173168

174-
static ImmutableArray<TextChange> GetMergedChanges(TextChange newLineEdit, ImmutableArray<TextChange> formattingChanges, SourceText formattedText)
169+
static ImmutableArray<TextChange> GetMergedChanges(TextChange? newLineEdit, ImmutableArray<TextChange> formattingChanges, SourceText formattedText)
175170
{
171+
// The new line edit is calculated against the original text, d0, to get text d1.
172+
// The formatting edits are calculated against d1 to get text d2.
173+
// Merge the formatting and new line edits into a set of whitespace only text edits that all apply to d0.
174+
if (!newLineEdit.HasValue)
175+
return formattingChanges;
176+
177+
// Depending on options, we might not get any formatting change.
178+
// In this case, the newline edit is the only change.
179+
if (formattingChanges.IsEmpty)
180+
return ImmutableArray.Create(newLineEdit.Value);
181+
176182
var newRanges = TextChangeRangeExtensions.Merge(
177-
ImmutableArray.Create(newLineEdit.ToTextChangeRange()),
183+
ImmutableArray.Create(newLineEdit.Value.ToTextChangeRange()),
178184
formattingChanges.SelectAsArray(f => f.ToTextChangeRange()));
179185

180186
using var _ = ArrayBuilder<TextChange>.GetInstance(out var mergedChanges);
@@ -247,19 +253,24 @@ static ImmutableArray<TextChange> GetMergedChanges(TextChange newLineEdit, Immut
247253
}
248254

249255
var newRoot = result.GetFormattedRoot(cancellationToken);
250-
var newClosingPoint = newRoot.GetAnnotatedTokens(s_closingBraceSyntaxAnnotation).Single().SpanStart + 1;
256+
var newClosingPoint = newRoot.GetAnnotatedTokens(s_closingBraceFormatAnnotation).Single().SpanStart + 1;
251257

252258
var textChanges = result.GetTextChanges(cancellationToken).ToImmutableArray();
253259
return (newRoot, textChanges, newClosingPoint);
254260

255261
SyntaxNode GetSyntaxRootWithAnnotatedClosingBrace(SyntaxNode originalRoot, int closingBraceEndPoint)
256262
{
257-
var closeBraceToken = originalRoot.FindToken(closingBraceEndPoint - 1);
258-
Debug.Assert(IsValidClosingBraceToken(closeBraceToken));
259-
260-
var newCloseBraceToken = closeBraceToken.WithAdditionalAnnotations(s_closingBraceSyntaxAnnotation);
263+
var closeBraceToken = FindClosingBraceToken(originalRoot, closingBraceEndPoint);
264+
var newCloseBraceToken = closeBraceToken.WithAdditionalAnnotations(s_closingBraceFormatAnnotation);
261265
return originalRoot.ReplaceToken(closeBraceToken, newCloseBraceToken);
262266
}
263267
}
268+
269+
private SyntaxToken FindClosingBraceToken(SyntaxNode root, int closingBraceEndPoint)
270+
{
271+
var closeBraceToken = root.FindToken(closingBraceEndPoint - 1);
272+
Debug.Assert(IsValidClosingBraceToken(closeBraceToken));
273+
return closeBraceToken;
274+
}
264275
}
265276
}

0 commit comments

Comments
 (0)