Skip to content

Commit 1cf42fa

Browse files
authored
Use the right line-ending when writing multi-line strings. (#1519)
Use the right line-ending when writing multi-line strings. There were tests for this already, but those tests were only run using the short style, so I also fixed that test file to run all of those tests in both styles. Fix #1504.
1 parent fbf2ce9 commit 1cf42fa

File tree

3 files changed

+59
-30
lines changed

3 files changed

+59
-30
lines changed

lib/src/back_end/code.dart

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -54,13 +54,19 @@ final class GroupCode extends Code {
5454

5555
/// Traverse the [Code] tree and build the final formatted string.
5656
///
57+
/// Whenever a newline is written, writes [lineEnding]. If omitted, defaults
58+
/// to '\n'.
59+
///
5760
/// Returns the formatted string and the selection markers if there are any.
58-
({String code, int? selectionStart, int? selectionEnd}) build() {
61+
({String code, int? selectionStart, int? selectionEnd}) build(
62+
[String? lineEnding]) {
63+
lineEnding ??= '\n';
64+
5965
var buffer = StringBuffer();
6066
int? selectionStart;
6167
int? selectionEnd;
6268

63-
_build(buffer, (marker, offset) {
69+
_build(buffer, lineEnding, (marker, offset) {
6470
if (marker == _Marker.start) {
6571
selectionStart = offset;
6672
} else {
@@ -75,16 +81,16 @@ final class GroupCode extends Code {
7581
);
7682
}
7783

78-
void _build(StringBuffer buffer,
84+
void _build(StringBuffer buffer, String lineEnding,
7985
void Function(_Marker marker, int offset) markSelection) {
8086
for (var i = 0; i < _children.length; i++) {
8187
var child = _children[i];
8288
switch (child) {
8389
case _NewlineCode():
8490
// Don't write any leading newlines at the top of the buffer.
8591
if (i > 0) {
86-
buffer.writeln();
87-
if (child._blank) buffer.writeln();
92+
buffer.write(lineEnding);
93+
if (child._blank) buffer.write(lineEnding);
8894
}
8995

9096
buffer.write(_indents[child._indent] ?? (' ' * child._indent));
@@ -93,7 +99,7 @@ final class GroupCode extends Code {
9399
buffer.write(child._text);
94100

95101
case GroupCode():
96-
child._build(buffer, markSelection);
102+
child._build(buffer, lineEnding, markSelection);
97103

98104
case _MarkerCode():
99105
markSelection(child._marker, buffer.length + child._offset);

lib/src/front_end/piece_writer.dart

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -387,10 +387,11 @@ class PieceWriter {
387387
Profile.begin('PieceWriter.finish() format piece tree');
388388

389389
var cache = SolutionCache();
390-
var formatter = Solver(cache,
390+
var solver = Solver(cache,
391391
pageWidth: _formatter.pageWidth, leadingIndent: _formatter.indent);
392-
var solution = formatter.format(rootPiece);
393-
var (:code, :selectionStart, :selectionEnd) = solution.code.build();
392+
var solution = solver.format(rootPiece);
393+
var (:code, :selectionStart, :selectionEnd) =
394+
solution.code.build(_formatter.lineEnding);
394395

395396
Profile.end('PieceWriter.finish() format piece tree');
396397

test/dart_formatter_test.dart

Lines changed: 43 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -3,41 +3,62 @@
33
// BSD-style license that can be found in the LICENSE file.
44

55
import 'package:dart_style/dart_style.dart';
6+
import 'package:dart_style/src/constants.dart';
67
import 'package:pub_semver/pub_semver.dart';
78
import 'package:test/test.dart';
89

910
void main() async {
11+
group('short style', () {
12+
_runTests(isTall: false);
13+
});
14+
15+
group('tall style', () {
16+
_runTests(isTall: true);
17+
});
18+
}
19+
20+
/// Run all of the DartFormatter tests either using short or tall style.
21+
void _runTests({required bool isTall}) {
22+
DartFormatter makeFormatter(
23+
{Version? languageVersion, int? indent, String? lineEnding}) {
24+
return DartFormatter(
25+
languageVersion: languageVersion,
26+
indent: indent,
27+
lineEnding: lineEnding,
28+
experimentFlags: [if (isTall) tallStyleExperimentFlag]);
29+
}
30+
1031
group('language version', () {
1132
test('defaults to latest if omitted', () {
12-
var formatter = DartFormatter();
33+
var formatter = makeFormatter();
1334
expect(formatter.languageVersion, DartFormatter.latestLanguageVersion);
1435
});
1536

1637
test('defaults to latest if null', () {
17-
var formatter = DartFormatter(languageVersion: null);
38+
var formatter = makeFormatter(languageVersion: null);
1839
expect(formatter.languageVersion, DartFormatter.latestLanguageVersion);
1940
});
2041

2142
test('parses at given older language version', () {
2243
// Use a language version before patterns were supported and a pattern
2344
// is an error.
24-
var formatter = DartFormatter(languageVersion: Version(2, 19, 0));
45+
var formatter = makeFormatter(languageVersion: Version(2, 19, 0));
2546
expect(() => formatter.format('main() {switch (o) {case var x: break;}}'),
2647
throwsA(isA<FormatterException>()));
2748
});
2849

2950
test('parses at given newer language version', () {
3051
// Use a language version after patterns were supported and `1 + 2` is an
3152
// error.
32-
var formatter = DartFormatter(languageVersion: Version(3, 0, 0));
53+
var formatter = makeFormatter(languageVersion: Version(3, 0, 0));
3354
expect(() => formatter.format('main() {switch (o) {case 1+2: break;}}'),
3455
throwsA(isA<FormatterException>()));
3556
});
3657

3758
test('@dart comment overrides version', () {
3859
// Use a language version after patterns were supported and `1 + 2` is an
3960
// error.
40-
var formatter = DartFormatter(languageVersion: Version(3, 0, 0));
61+
var formatter = makeFormatter(languageVersion: Version(3, 0, 0));
4162

4263
// But then have the code opt to the older version.
4364
const before = '''
@@ -60,22 +81,22 @@ main() {
6081
});
6182

6283
test('throws a FormatterException on failed parse', () {
63-
var formatter = DartFormatter();
84+
var formatter = makeFormatter();
6485
expect(() => formatter.format('wat?!'), throwsA(isA<FormatterException>()));
6586
});
6687

6788
test('FormatterException.message() does not throw', () {
6889
// This is a regression test for #358 where an error whose position is
6990
// past the end of the source caused FormatterException to throw.
7091
expect(
71-
() => DartFormatter().format('library'),
92+
() => makeFormatter().format('library'),
7293
throwsA(isA<FormatterException>().having(
7394
(e) => e.message(), 'message', contains('Could not format'))));
7495
});
7596

7697
test('FormatterException describes parse errors', () {
7798
expect(() {
78-
DartFormatter().format('''
99+
makeFormatter().format('''
79100
80101
var a = some error;
81102
@@ -92,33 +113,33 @@ main() {
92113
});
93114

94115
test('adds newline to unit', () {
95-
expect(DartFormatter().format('var x = 1;'), equals('var x = 1;\n'));
116+
expect(makeFormatter().format('var x = 1;'), equals('var x = 1;\n'));
96117
});
97118

98119
test('adds newline to unit after trailing comment', () {
99-
expect(DartFormatter().format('library foo; //zamm'),
120+
expect(makeFormatter().format('library foo; //zamm'),
100121
equals('library foo; //zamm\n'));
101122
});
102123

103124
test('removes extra newlines', () {
104-
expect(DartFormatter().format('var x = 1;\n\n\n'), equals('var x = 1;\n'));
125+
expect(makeFormatter().format('var x = 1;\n\n\n'), equals('var x = 1;\n'));
105126
});
106127

107128
test('does not add newline to statement', () {
108-
expect(DartFormatter().formatStatement('var x = 1;'), equals('var x = 1;'));
129+
expect(makeFormatter().formatStatement('var x = 1;'), equals('var x = 1;'));
109130
});
110131

111132
test('fails if anything is after the statement', () {
112133
expect(
113-
() => DartFormatter().formatStatement('var x = 1;;'),
134+
() => makeFormatter().formatStatement('var x = 1;;'),
114135
throwsA(isA<FormatterException>()
115136
.having((e) => e.errors.length, 'errors.length', equals(1))
116137
.having((e) => e.errors.first.offset, 'errors.length.first.offset',
117138
equals(10))));
118139
});
119140

120141
test('preserves initial indent', () {
121-
var formatter = DartFormatter(indent: 3);
142+
var formatter = makeFormatter(indent: 3);
122143
expect(
123144
formatter.formatStatement('if (foo) {bar;}'),
124145
equals(' if (foo) {\n'
@@ -131,29 +152,30 @@ main() {
131152
// Use zero width no-break space character as the line ending. We have
132153
// to use a whitespace character for the line ending as the formatter
133154
// will throw an error if it accidentally makes non-whitespace changes
134-
// as will occur
155+
// as would occur if we used a non-whitespace character as the line
156+
// ending.
135157
var lineEnding = '\t';
136-
expect(DartFormatter(lineEnding: lineEnding).format('var i = 1;'),
158+
expect(makeFormatter(lineEnding: lineEnding).format('var i = 1;'),
137159
equals('var i = 1;\t'));
138160
});
139161

140162
test('infers \\r\\n if the first newline uses that', () {
141-
expect(DartFormatter().format('var\r\ni\n=\n1;\n'),
163+
expect(makeFormatter().format('var\r\ni\n=\n1;\n'),
142164
equals('var i = 1;\r\n'));
143165
});
144166

145167
test('infers \\n if the first newline uses that', () {
146-
expect(DartFormatter().format('var\ni\r\n=\r\n1;\r\n'),
168+
expect(makeFormatter().format('var\ni\r\n=\r\n1;\r\n'),
147169
equals('var i = 1;\n'));
148170
});
149171

150172
test('defaults to \\n if there are no newlines', () {
151-
expect(DartFormatter().format('var i =1;'), equals('var i = 1;\n'));
173+
expect(makeFormatter().format('var i =1;'), equals('var i = 1;\n'));
152174
});
153175

154176
test('handles Windows line endings in multiline strings', () {
155177
expect(
156-
DartFormatter(lineEnding: '\r\n').formatStatement(' """first\r\n'
178+
makeFormatter(lineEnding: '\r\n').formatStatement(' """first\r\n'
157179
'second\r\n'
158180
'third""" ;'),
159181
equals('"""first\r\n'
@@ -165,7 +187,7 @@ main() {
165187
test('throws an UnexpectedOutputException on non-whitespace changes', () {
166188
// Use an invalid line ending character to ensure the formatter will
167189
// attempt to make non-whitespace changes.
168-
var formatter = DartFormatter(lineEnding: '%');
190+
var formatter = makeFormatter(lineEnding: '%');
169191
expect(() => formatter.format('var i = 1;'),
170192
throwsA(isA<UnexpectedOutputException>()));
171193
});

0 commit comments

Comments
 (0)