Skip to content

Commit

Permalink
Fix LF line-ending auto format bug (#10802)
Browse files Browse the repository at this point in the history
* Implemented a line normalization function that prevents the language server from sending /r to LF line ending docs

* check if indentation location has been processed

* added LF line ending document to all previous razor formatting test cases

* skipping some LF line ending formatting tests. Created an issue to track the progress
  • Loading branch information
jordi1215 authored Sep 4, 2024
1 parent de7eddc commit 148d71a
Show file tree
Hide file tree
Showing 6 changed files with 107 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -296,4 +296,33 @@ public static TextEdit[] MinimizeTextEdits(this SourceText text, TextEdit[] edit
var cleanEdits = cleanChanges.Select(text.GetTextEdit).ToArray();
return cleanEdits;
}

/// <summary>
/// Determines if the given <see cref="SourceText"/> has more LF line endings ('\n') than CRLF line endings ('\r\n').
/// </summary>
/// <param name="text">The <see cref="SourceText"/> to examine.</param>
/// <returns>
/// <c>true</c> if the <see cref="SourceText"/> is deemed to use LF line endings; otherwise, <c>false</c>.
/// </returns>
public static bool HasLFLineEndings(this SourceText text)
{
var crlfCount = 0;
var lfCount = 0;

foreach (var line in text.Lines)
{
var lineBreakSpan = TextSpan.FromBounds(line.End, line.EndIncludingLineBreak);
var lineBreak = line.Text?.ToString(lineBreakSpan) ?? string.Empty;
if (lineBreak == "\r\n")
{
crlfCount++;
}
else if (lineBreak == "\n")
{
lfCount++;
}
}

return lfCount > crlfCount;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,10 @@ protected async Task<List<TextChange>> AdjustIndentationAsync(FormattingContext
}

var scopeOwner = syntaxTreeRoot.FindInnermostNode(originalLocation);
sourceMappingIndentations[originalLocation] = new IndentationData(indentation);
if (!sourceMappingIndentations.ContainsKey(originalLocation))
{
sourceMappingIndentations[originalLocation] = new IndentationData(indentation);
}

// For @section blocks we have special handling to add a fake source mapping/significant location at the end of the
// section, to return the indentation back to before the start of the section block.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,8 @@ public async Task<TextEdit[]> GetDocumentFormattingEditsAsync(
? result
: result.Where(e => range.LineOverlapsWith(e.Range)).ToArray();

return originalText.MinimizeTextEdits(filteredEdits);
var normalizedEdits = NormalizeLineEndings(originalText, filteredEdits);
return originalText.MinimizeTextEdits(normalizedEdits);
}

public Task<TextEdit[]> GetCSharpOnTypeFormattingEditsAsync(DocumentContext documentContext, RazorFormattingOptions options, int hostDocumentIndex, char triggerCharacter, CancellationToken cancellationToken)
Expand Down Expand Up @@ -279,4 +280,22 @@ private static void UnwrapCSharpSnippets(TextEdit[] razorEdits)
edit.NewText = edit.NewText.Replace("/*$0*/", "$0");
}
}

/// <summary>
/// This method counts the occurrences of CRLF and LF line endings in the original text.
/// If LF line endings are more prevalent, it removes any CR characters from the text edits
/// to ensure consistency with the LF style.
/// </summary>
private TextEdit[] NormalizeLineEndings(SourceText originalText, TextEdit[] edits)
{
if (originalText.HasLFLineEndings())
{
foreach (var edit in edits)
{
edit.NewText = edit.NewText.Replace("\r", "");
}
}

return edits;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -1699,7 +1699,8 @@ await RunFormattingTestAsync(
private IEnumerable<int> _items = new[] { 1, 2, 3, 4, 5 };
}
""",
tagHelpers: GetComponentWithCascadingTypeParameter());
tagHelpers: GetComponentWithCascadingTypeParameter(),
skipFlipLineEndingTest: true);
}

[Fact]
Expand Down Expand Up @@ -1789,7 +1790,8 @@ await RunFormattingTestAsync(
private IEnumerable<long> _items2 = new long[] { 1, 2, 3, 4, 5 };
}
""",
tagHelpers: GetComponentWithTwoCascadingTypeParameter());
tagHelpers: GetComponentWithTwoCascadingTypeParameter(),
skipFlipLineEndingTest: true); // tracked by https://github.com/dotnet/razor/issues/10836
}

[Fact]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,14 +49,26 @@ private protected async Task RunFormattingTestAsync(
ImmutableArray<TagHelperDescriptor> tagHelpers = default,
bool allowDiagnostics = false,
RazorLSPOptions? razorLSPOptions = null,
bool inGlobalNamespace = false)
bool inGlobalNamespace = false,
bool skipFlipLineEndingTest = false)
{
// Run with and without forceRuntimeCodeGeneration
await RunFormattingTestAsync(input, expected, tabSize, insertSpaces, fileKind, tagHelpers, allowDiagnostics, razorLSPOptions, inGlobalNamespace, forceRuntimeCodeGeneration: true);
await RunFormattingTestAsync(input, expected, tabSize, insertSpaces, fileKind, tagHelpers, allowDiagnostics, razorLSPOptions, inGlobalNamespace, forceRuntimeCodeGeneration: false);
await RunFormattingTestInternalAsync(input, expected, tabSize, insertSpaces, fileKind, tagHelpers, allowDiagnostics, razorLSPOptions, inGlobalNamespace, forceRuntimeCodeGeneration: true);
await RunFormattingTestInternalAsync(input, expected, tabSize, insertSpaces, fileKind, tagHelpers, allowDiagnostics, razorLSPOptions, inGlobalNamespace, forceRuntimeCodeGeneration: false);

// some tests are failing, skip for now, tracked by https://github.com/dotnet/razor/issues/10836
if (!skipFlipLineEndingTest)
{
// flip the line endings of the stings (LF to CRLF and vice versa) and run again
input = FlipLineEndings(input);
expected = FlipLineEndings(expected);

await RunFormattingTestInternalAsync(input, expected, tabSize, insertSpaces, fileKind, tagHelpers, allowDiagnostics, razorLSPOptions, inGlobalNamespace, forceRuntimeCodeGeneration: true);
await RunFormattingTestInternalAsync(input, expected, tabSize, insertSpaces, fileKind, tagHelpers, allowDiagnostics, razorLSPOptions, inGlobalNamespace, forceRuntimeCodeGeneration: false);
}
}

private async Task RunFormattingTestAsync(string input, string expected, int tabSize, bool insertSpaces, string? fileKind, ImmutableArray<TagHelperDescriptor> tagHelpers, bool allowDiagnostics, RazorLSPOptions? razorLSPOptions, bool inGlobalNamespace, bool forceRuntimeCodeGeneration)
private async Task RunFormattingTestInternalAsync(string input, string expected, int tabSize, bool insertSpaces, string? fileKind, ImmutableArray<TagHelperDescriptor> tagHelpers, bool allowDiagnostics, RazorLSPOptions? razorLSPOptions, bool inGlobalNamespace, bool forceRuntimeCodeGeneration)
{
// Arrange
fileKind ??= FileKinds.Component;
Expand Down Expand Up @@ -351,4 +363,26 @@ internal static IDocumentSnapshot CreateDocumentSnapshot(string path, ImmutableA
});
return documentSnapshot.Object;
}

private static string FlipLineEndings(string input)
{
if (string.IsNullOrEmpty(input))
{
return input;
}

var hasCRLF = input.Contains("\r\n");
var hasLF = !hasCRLF && input.Contains("\n");

if (hasCRLF)
{
return input.Replace("\r\n", "\n");
}
else if (hasLF)
{
return input.Replace("\n", "\r\n");
}

return input;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -487,7 +487,8 @@ await RunFormattingTestAsync(
}
</GridTable>
""",
tagHelpers: tagHelpers);
tagHelpers: tagHelpers,
skipFlipLineEndingTest: true); // tracked by https://github.com/dotnet/razor/issues/10836
}

[Fact]
Expand Down Expand Up @@ -595,7 +596,8 @@ await RunFormattingTestAsync(
@{
<p></p>
}
""");
""",
skipFlipLineEndingTest: true); // tracked by https://github.com/dotnet/razor/issues/10836
}

[Fact]
Expand Down Expand Up @@ -1316,7 +1318,8 @@ await RunFormattingTestAsync(
public bool VarBool { get; set; }
}
""",
fileKind: FileKinds.Component);
fileKind: FileKinds.Component,
skipFlipLineEndingTest: true); // tracked by https://github.com/dotnet/razor/issues/10836
}

[Fact]
Expand Down Expand Up @@ -1428,7 +1431,8 @@ await RunFormattingTestAsync(
public bool VarBool { get; set; }
}
""",
fileKind: FileKinds.Component);
fileKind: FileKinds.Component,
skipFlipLineEndingTest: true); // tracked by https://github.com/dotnet/razor/issues/10836
}

[Fact]
Expand Down Expand Up @@ -1486,7 +1490,8 @@ await RunFormattingTestAsync(
public bool VarBool { get; set; }
}
""",
fileKind: FileKinds.Component);
fileKind: FileKinds.Component,
skipFlipLineEndingTest: true); // tracked by https://github.com/dotnet/razor/issues/10836
}

[Fact]
Expand Down Expand Up @@ -1794,7 +1799,8 @@ await RunFormattingTestAsync(
}
</Select>
""",
tagHelpers: CreateTagHelpers());
tagHelpers: CreateTagHelpers(),
skipFlipLineEndingTest: true); // tracked by https://github.com/dotnet/razor/issues/10836

ImmutableArray<TagHelperDescriptor> CreateTagHelpers()
{
Expand Down

0 comments on commit 148d71a

Please sign in to comment.