From 23cc0716724c0dfca85f268c86a7ca58328bda17 Mon Sep 17 00:00:00 2001 From: Kori Francis Date: Wed, 2 Sep 2026 15:47:49 -0400 Subject: [PATCH] Keep a string literal's own line endings when printing it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An interpolated string is printed as one verbatim run rather than as a token, so `TokenPrinter.EmitVerbatimRange` split it at every newline and re-issued each break with the configured `end_of_line`. Those newlines are not layout: they are characters of the value the literal builds, and they are part of the token's own text. Rewriting them changed the program, and the re-parse comparer correctly reported it as `formatting changed token N` — identical-looking on both sides, because the only difference is a `\r`. Every other multi-line literal survives because Roslyn hands it over as a single token, which `TokenPrinter.Print` emits as one text leaf. `"""` and `@""` were therefore fine and `$"""` and `$@""` were not, which is what issue #85 observed. `Doc` gains three sentinels for `B` on a literal line — the configured ending, LF, or CRLF — and `DocArena.SourceLine` emits one carrying the ending the source had at that point. `EmitVerbatimRange` uses it behind `preserveLineEndings`, which only the interpolated-string printer sets: comments, doc comments and disabled `#if` branches want their endings normalised with everything else, and keep it. That removes the reason `DocPrinter.Print` forced a round-trip parse on every mixed-ending file containing `@"` or `"""`. With the rewrite gone there is nothing for the second parse to find, so the guard goes with it — 64 of 144 forced re-parses became 0 on curb's own sources, with output byte-identical. Conformance on elastic/docs-builder (1,285 files) holds at 100% in all four configurations: reflow off, reflow on, width 120, and preserving. --- src/Nullean.Curb.Core/Documents/Doc.cs | 20 ++++++++- src/Nullean.Curb.Core/Documents/DocArena.cs | 11 +++++ src/Nullean.Curb.Core/Documents/DocDumper.cs | 7 +++- .../Printing/CSharp/Printers.Expressions.cs | 3 +- .../Printing/CSharp/TokenPrinter.cs | 23 +++++++++-- src/Nullean.Curb.Core/Printing/DocPrinter.cs | 41 +++++-------------- .../Formatting/Options/CoreOptionTests.cs | 24 +++++++++++ .../Printing/RoundTripRiskTests.cs | 31 +++++++++++--- 8 files changed, 116 insertions(+), 44 deletions(-) diff --git a/src/Nullean.Curb.Core/Documents/Doc.cs b/src/Nullean.Curb.Core/Documents/Doc.cs index 0d141e4..791055a 100644 --- a/src/Nullean.Curb.Core/Documents/Doc.cs +++ b/src/Nullean.Curb.Core/Documents/Doc.cs @@ -88,7 +88,8 @@ internal enum LineType : byte /// /// Always a newline, emitted at column 0 with no indent. Used inside verbatim and raw string - /// literals, whose content must not be re-indented. + /// literals, whose content must not be re-indented. B picks the ending — see + /// . /// Literal = 3, } @@ -169,6 +170,23 @@ internal readonly struct Doc /// Sentinel for meaning "dedent to column 0". public const int IndentToRoot = int.MinValue; + /// + /// B on a line: write the configured end_of_line. + /// + /// + /// The default, and what a break between two lines of a comment, a doc comment or a disabled + /// #if branch wants: those newlines are layout, so normalising them is the whole point. + /// A newline that is part of a string literal's value is not layout, and asks for one of the two + /// below instead — rewriting it changes what the program computes. + /// + public const int ConfiguredEnding = 0; + + /// B on a line: write \n whatever the configuration says. + public const int LfEnding = 1; + + /// B on a line: write \r\n whatever the configuration says. + public const int CrLfEnding = 2; + public readonly DocKind Kind; public readonly DocFlags Flags; diff --git a/src/Nullean.Curb.Core/Documents/DocArena.cs b/src/Nullean.Curb.Core/Documents/DocArena.cs index d85af6c..d98ef26 100644 --- a/src/Nullean.Curb.Core/Documents/DocArena.cs +++ b/src/Nullean.Curb.Core/Documents/DocArena.cs @@ -89,6 +89,17 @@ public void ExternalText(string text) /// Always a newline emitted at column 0, for content that must not be re-indented. public void LiteralLine(DocFlags flags = DocFlags.None) => AddLine(LineType.Literal, flags); + /// + /// A literal line that reproduces the ending the source had here rather than the configured one. + /// + /// + /// For newlines that are part of a string literal's value. A raw or verbatim string's line + /// endings are content: rewriting them changes the string the program builds, and changes the + /// token's own text, which is why the re-parse comparer reports it as a changed token. + /// + public void SourceLine(bool crLf) => + Add(new Doc(DocKind.Line, a: (int)LineType.Literal, b: crLf ? Doc.CrLfEnding : Doc.LfEnding)); + /// /// A newline, but only when the group names ended up broken. /// diff --git a/src/Nullean.Curb.Core/Documents/DocDumper.cs b/src/Nullean.Curb.Core/Documents/DocDumper.cs index 382b1d0..7029854 100644 --- a/src/Nullean.Curb.Core/Documents/DocDumper.cs +++ b/src/Nullean.Curb.Core/Documents/DocDumper.cs @@ -50,7 +50,12 @@ private static void Write(DocArena arena, ReadOnlySpan source, StringBuild LineType.Normal => "line", LineType.Soft => "softline", LineType.Hard => "hardline", - LineType.Literal => "literalline", + LineType.Literal => doc.B switch + { + Doc.LfEnding => "literalline lf", + Doc.CrLfEnding => "literalline crlf", + _ => "literalline", + }, _ => "line?", }); AppendFlags(output, doc.Flags); diff --git a/src/Nullean.Curb.Core/Printing/CSharp/Printers.Expressions.cs b/src/Nullean.Curb.Core/Printing/CSharp/Printers.Expressions.cs index 0b121eb..500d726 100644 --- a/src/Nullean.Curb.Core/Printing/CSharp/Printers.Expressions.cs +++ b/src/Nullean.Curb.Core/Printing/CSharp/Printers.Expressions.cs @@ -26,8 +26,9 @@ private static void VerbatimContent(SyntaxNode node, PrintContext context) if (first.RawKind != 0) TokenPrinter.PrintLeadingTrivia(first, context); + // preserveLineEndings: the run is a string literal, so its newlines are part of the value. var span = node.Span; - TokenPrinter.EmitVerbatimRange(context, span.Start, span.Length); + TokenPrinter.EmitVerbatimRange(context, span.Start, span.Length, preserveLineEndings: true); if (last.RawKind != 0) TokenPrinter.PrintTrailingTrivia(last, context); diff --git a/src/Nullean.Curb.Core/Printing/CSharp/TokenPrinter.cs b/src/Nullean.Curb.Core/Printing/CSharp/TokenPrinter.cs index 1c86466..aa486df 100644 --- a/src/Nullean.Curb.Core/Printing/CSharp/TokenPrinter.cs +++ b/src/Nullean.Curb.Core/Printing/CSharp/TokenPrinter.cs @@ -555,7 +555,16 @@ private static void EmitVerbatimBlock(SyntaxTrivia trivia, PrintContext context) /// Emits characters from exactly as written, /// splitting on newlines so that the printer does not re-indent them. /// - internal static void EmitVerbatimRange(PrintContext context, int start, int length) + /// + /// says the newlines in the range belong to the content + /// rather than to the layout, which is the case inside a string literal: an interpolated string + /// is printed as one verbatim run, and its line endings are characters of the value it builds. + /// Re-issuing them as the configured end_of_line rewrites the literal — an LF raw string + /// in a file formatted to CRLF came back with its string changed, and the re-parse comparer + /// reported it as a changed token (issue #85). Comments and disabled #if branches want the + /// opposite, and leave it false so their endings normalise with everything else. + /// + internal static void EmitVerbatimRange(PrintContext context, int start, int length, bool preserveLineEndings = false) { var source = context.Text; var arena = context.Arena; @@ -568,13 +577,19 @@ internal static void EmitVerbatimRange(PrintContext context, int start, int leng continue; var lineLength = i - lineStart; - // Drop a \r that belongs to this \n; the printer emits the configured line ending. - if (lineLength > 0 && source[lineStart + lineLength - 1] == '\r') + // Drop a \r that belongs to this \n; the printer, not the text leaf, emits the ending. + var crLf = lineLength > 0 && source[lineStart + lineLength - 1] == '\r'; + if (crLf) lineLength--; if (lineLength > 0) arena.SourceText(lineStart, lineLength); - arena.LiteralLine(); + + if (preserveLineEndings) + arena.SourceLine(crLf); + else + arena.LiteralLine(); + lineStart = i + 1; } diff --git a/src/Nullean.Curb.Core/Printing/DocPrinter.cs b/src/Nullean.Curb.Core/Printing/DocPrinter.cs index 6e74c02..49390e5 100644 --- a/src/Nullean.Curb.Core/Printing/DocPrinter.cs +++ b/src/Nullean.Curb.Core/Printing/DocPrinter.cs @@ -84,16 +84,7 @@ public void Print(DocArena arena, ReadOnlyMemory source, in FormatOptions _depth = 0; _lastSourceEnd = -1; _insideLineComment = false; - - // Verbatim runs are re-emitted line by line with the configured ending, so when that differs - // from the source's own ending the content of a multi-line string literal changes. The - // boundary tracking below cannot see that — but only files that actually contain a verbatim - // or raw string literal are at risk, so check for the relevant delimiters before forcing a - // round-trip parse. - var sourceNewLine = source.Span.IndexOf('\n'); - var sourceUsesCrLf = sourceNewLine > 0 && source.Span[sourceNewLine - 1] == '\r'; - RoundTripAtRisk = sourceNewLine >= 0 && sourceUsesCrLf != (_endOfLine == "\r\n") - && HasVerbatimOrRawString(source.Span); + RoundTripAtRisk = false; _breaks.Run(arena); EnsureGroupModes(arena.Count); @@ -368,7 +359,15 @@ private void PrintLine(in Doc doc, in Scope scope) // indent because whatever it sits inside (a raw string) owns its own leading whitespace. if (type == LineType.Literal) { - _output.Append(_endOfLine); + // B says whose ending this is. A break between two lines of a comment or a disabled #if + // branch takes the configured one; a newline inside a string literal keeps the source's, + // because there it is a character of the value rather than layout. + _output.Append(doc.B switch + { + Doc.LfEnding => "\n", + Doc.CrLfEnding => "\r\n", + _ => _endOfLine, + }); _column = 0; _insideLineComment = false; return; @@ -474,26 +473,6 @@ private void TrackRoundTripRisk(in Doc doc, ReadOnlySpan text) RoundTripAtRisk = true; } - /// - /// Returns true when the source contains at least one verbatim or raw string delimiter. - /// Only such strings have their line endings rewritten by the verbatim-run emitter, so only - /// they can have their token text changed when the configured line ending differs from the - /// source's own. - /// - private static bool HasVerbatimOrRawString(ReadOnlySpan source) - { - // Fast exit: no double-quote means no string literals at all. - if (source.IndexOf('"') < 0) - return false; - - // @"..." and $@"..." both contain the two-character sequence @" (in $@" it appears at - // offset 1), so one search covers both. @$"..." has @ followed by $ followed by ", so - // it requires its own check. Raw string literals start with """. - return source.Contains("@\"", StringComparison.Ordinal) - || source.Contains("@$\"", StringComparison.Ordinal) - || source.Contains("\"\"\"", StringComparison.Ordinal); - } - private void Emit(ReadOnlySpan text, DocFlags flags, bool suppressWidth) { _output.Append(text); diff --git a/tests/Nullean.Curb.Tests/Formatting/Options/CoreOptionTests.cs b/tests/Nullean.Curb.Tests/Formatting/Options/CoreOptionTests.cs index dd2487f..b750402 100644 --- a/tests/Nullean.Curb.Tests/Formatting/Options/CoreOptionTests.cs +++ b/tests/Nullean.Curb.Tests/Formatting/Options/CoreOptionTests.cs @@ -123,6 +123,30 @@ public Task Unset_end_of_line_matches_the_platform() => FormatsExactly( $"public class C{Environment.NewLine}{{{Environment.NewLine}}}{Environment.NewLine}", editorConfig: "end_of_line = auto"); + // A string literal's newlines are characters of its value, not layout, so end_of_line does not + // reach inside one. Re-issuing them with the configured ending changed both the string the + // program builds and the token's own text, which made the file fail its own round-trip check + // (issue #85). Interpolated strings are the case that regressed: they print as one verbatim run + // rather than as a single token, so they took the line-splitting path the others never did. + + [Test] + public Task An_interpolated_raw_string_keeps_its_own_line_endings() => FormatsExactly( + "class A\n{\n string M(string x)\n {\n return $\"\"\"\n {x}\n \"\"\";\n }\n}", + "class A\r\n{\r\n string M(string x)\r\n {\r\n return $\"\"\"\n {x}\n \"\"\";\r\n }\r\n}\r\n", + editorConfig: "end_of_line = crlf"); + + [Test] + public Task An_interpolated_verbatim_string_keeps_its_own_line_endings() => FormatsExactly( + "class A\r\n{\r\n string M(int x)\r\n {\r\n return $@\"one{x}\r\ntwo\";\r\n }\r\n}", + "class A\n{\n string M(int x)\n {\n return $@\"one{x}\r\ntwo\";\n }\n}\n", + editorConfig: "end_of_line = lf"); + + [Test] + public Task A_raw_string_keeps_its_own_line_endings() => FormatsExactly( + "class A\n{\n string M()\n {\n return \"\"\"\n b\n \"\"\";\n }\n}", + "class A\r\n{\r\n string M()\r\n {\r\n return \"\"\"\n b\n \"\"\";\r\n }\r\n}\r\n", + editorConfig: "end_of_line = crlf"); + // ---- insert_final_newline ------------------------------------------------------------------ [Test] diff --git a/tests/Nullean.Curb.Tests/Printing/RoundTripRiskTests.cs b/tests/Nullean.Curb.Tests/Printing/RoundTripRiskTests.cs index ea7a07f..0791bd1 100644 --- a/tests/Nullean.Curb.Tests/Printing/RoundTripRiskTests.cs +++ b/tests/Nullean.Curb.Tests/Printing/RoundTripRiskTests.cs @@ -100,20 +100,39 @@ public async Task Does_not_fire_when_a_line_comment_is_closed_by_a_break() } [Test] - public async Task Fires_when_line_endings_are_being_rewritten() + public async Task Does_not_fire_merely_because_the_file_holds_a_multi_line_literal() { - // Verbatim string content is re-emitted with the configured ending, which changes the value - // of a multi-line literal. Only files that actually contain a verbatim or raw string need - // the second parse — the guard prevents unnecessary re-parses on files that have none. + // A verbatim string's newlines are content, and DocArena.SourceLine now reproduces them, so + // a file whose endings differ from the configured one no longer has its literals rewritten + // and no longer owes a second parse for that reason alone. Before that, every mixed-ending + // file containing @" or """ paid for one. var arena = new DocArena(); arena.SourceText(0, 3); using var output = new OutputBuffer(); var printer = new DocPrinter(); - // Source contains @" so HasVerbatimOrRawString fires; CRLF source with LF target triggers the risk. printer.Print(arena, "@\"a\r\nb\"".AsMemory(), new FormatOptions { EndOfLine = EndOfLine.Lf }, output); - printer.RoundTripAtRisk.Should().BeTrue(); + printer.RoundTripAtRisk.Should().BeFalse(); + await Task.CompletedTask; + } + + [Test] + public async Task A_source_line_keeps_its_own_ending_rather_than_the_configured_one() + { + var arena = new DocArena(); + arena.SourceText(0, 3); + arena.SourceLine(crLf: true); + arena.SourceText(4, 3); + arena.SourceLine(crLf: false); + arena.SourceText(8, 1); + + using var output = new OutputBuffer(); + var printer = new DocPrinter(); + printer.Print(arena, Source.AsMemory(), Options, output); + + // Options say LF; both endings come from the document, not from the configuration. + output.ToString().Should().Be("out\r\nvar\na"); await Task.CompletedTask; }