Skip to content

Commit d34343c

Browse files
committed
Rework things some:
- Move the tests to tall. The old short style formatter doesn't support new syntax and doesn't need to be tested. - Add support for passing experiment flags with tests. The old way of doing this was to just hardcode experiments inside the formatter when a new feature was supported. That made sense when the formatter didn't know about language version, but now that it does, it should probably require experiment flags explicitly and tests for new features should enable them. - Move the code to handle "? into visitMapLiteralEntry(). The AST node does have tokens for "?" and that way there's less special-case code getting threaded through PieceFactory. - Add some tests for comments appearing around "?". (Comments tend to be a tricky corner of the formatter, so we test them heavily.)
1 parent 9368c17 commit d34343c

File tree

10 files changed

+108
-58
lines changed

10 files changed

+108
-58
lines changed

example/format.dart

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,8 @@ Future<void> _runTest(String path, int line,
7878
var formatter = DartFormatter(
7979
languageVersion: formatTest.languageVersion,
8080
pageWidth: testFile.pageWidth,
81-
indent: formatTest.leadingIndent);
81+
indent: formatTest.leadingIndent,
82+
experimentFlags: formatTest.experimentFlags);
8283

8384
var actual = formatter.formatSource(formatTest.input);
8485

lib/src/front_end/ast_node_visitor.dart

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1251,8 +1251,24 @@ final class AstNodeVisitor extends ThrowingAstVisitor<void> with PieceFactory {
12511251

12521252
@override
12531253
void visitMapLiteralEntry(MapLiteralEntry node) {
1254-
writeAssignment(node.key, node.separator, node.value,
1255-
includeLeftHandSideQuestion: true, includeRightHandSideQuestion: true);
1254+
var leftPiece = pieces.build(() {
1255+
pieces.token(node.keyQuestion);
1256+
pieces.visit(node.key);
1257+
});
1258+
1259+
var operatorPiece = tokenPiece(node.separator);
1260+
1261+
var rightPiece = pieces.build(() {
1262+
pieces.token(node.valueQuestion);
1263+
pieces.visit(node.value, context: NodeContext.assignment);
1264+
});
1265+
1266+
pieces.add(AssignPiece(
1267+
left: leftPiece,
1268+
operatorPiece,
1269+
rightPiece,
1270+
canBlockSplitLeft: node.key.canBlockSplit,
1271+
canBlockSplitRight: node.value.canBlockSplit));
12561272
}
12571273

12581274
@override

lib/src/front_end/piece_factory.dart

Lines changed: 4 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1338,10 +1338,7 @@ mixin PieceFactory {
13381338
AstNode leftHandSide, Token operator, AstNode rightHandSide,
13391339
{bool includeComma = false,
13401340
bool canBlockSplitLeft = false,
1341-
NodeContext leftHandSideContext = NodeContext.none,
1342-
bool includeLeftHandSideQuestion = false,
1343-
bool includeRightHandSideQuestion = false,
1344-
String? rightHandSidePrefix}) {
1341+
NodeContext leftHandSideContext = NodeContext.none}) {
13451342
// If an operand can have block formatting, then a newline in it doesn't
13461343
// force the operator to split, as in:
13471344
//
@@ -1386,19 +1383,15 @@ mixin PieceFactory {
13861383
_ => false
13871384
};
13881385

1389-
var leftPiece = nodePiece(leftHandSide,
1390-
questionBefore: includeLeftHandSideQuestion,
1391-
context: leftHandSideContext);
1386+
var leftPiece = nodePiece(leftHandSide, context: leftHandSideContext);
13921387

13931388
var operatorPiece = pieces.build(() {
13941389
if (operator.type != TokenType.COLON) pieces.space();
13951390
pieces.token(operator);
13961391
});
13971392

13981393
var rightPiece = nodePiece(rightHandSide,
1399-
commaAfter: includeComma,
1400-
questionBefore: includeRightHandSideQuestion,
1401-
context: NodeContext.assignment);
1394+
commaAfter: includeComma, context: NodeContext.assignment);
14021395

14031396
pieces.add(AssignPiece(
14041397
left: leftPiece,
@@ -1527,9 +1520,7 @@ mixin PieceFactory {
15271520
/// If [commaAfter] is `true`, looks for a comma token after [node] and
15281521
/// writes it to the piece as well.
15291522
Piece nodePiece(AstNode node,
1530-
{bool commaAfter = false,
1531-
bool questionBefore = false,
1532-
NodeContext context = NodeContext.none}) {
1523+
{bool commaAfter = false, NodeContext context = NodeContext.none}) {
15331524
var result = pieces.build(() {
15341525
visitNode(node, context);
15351526
});
@@ -1542,14 +1533,6 @@ mixin PieceFactory {
15421533
}
15431534
}
15441535

1545-
if (questionBefore) {
1546-
var previousToken = node.beginToken.previous!;
1547-
if (previousToken.lexeme == '?') {
1548-
var question = tokenPiece(previousToken);
1549-
result = AdjacentPiece([question, result]);
1550-
}
1551-
}
1552-
15531536
return result;
15541537
}
15551538

lib/src/testing/test_file.dart

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import '../../dart_style.dart';
1111

1212
final _indentPattern = RegExp(r'\(indent (\d+)\)');
1313
final _versionPattern = RegExp(r'\(version (\d+)\.(\d+)\)');
14+
final _experimentPattern = RegExp(r'\(experiment ([a-z-]+)\)');
1415
final _unicodeUnescapePattern = RegExp(r'×([0-9a-fA-F]{2,4})');
1516
final _unicodeEscapePattern = RegExp('[\x0a\x0c\x0d]');
1617

@@ -111,6 +112,14 @@ final class TestFile {
111112
return '';
112113
});
113114

115+
// Let the test enable experiments for features that are supported but not
116+
// released yet.
117+
var experiments = <String>[];
118+
description = description.replaceAllMapped(_experimentPattern, (match) {
119+
experiments.add(match[1]!);
120+
return '';
121+
});
122+
114123
var inputComments = readComments();
115124

116125
var inputBuffer = StringBuffer();
@@ -160,6 +169,7 @@ final class TestFile {
160169
lineNumber,
161170
languageVersion,
162171
leadingIndent,
172+
experiments,
163173
inputComments,
164174
outputComments));
165175
}
@@ -217,6 +227,9 @@ final class FormatTest {
217227
/// line.
218228
final int leadingIndent;
219229

230+
/// Experiments that should be enabled when running this test.
231+
final List<String> experimentFlags;
232+
220233
FormatTest(
221234
this.input,
222235
this.output,
@@ -225,6 +238,7 @@ final class FormatTest {
225238
this.line,
226239
this.languageVersion,
227240
this.leadingIndent,
241+
this.experimentFlags,
228242
this.inputComments,
229243
this.outputComments);
230244

pubspec.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ environment:
99
sdk: "^3.4.0"
1010

1111
dependencies:
12-
analyzer: ">=6.5.0 <8.0.0"
12+
analyzer: ">=7.3.0 <8.0.0"
1313
args: ">=1.0.0 <3.0.0"
1414
collection: "^1.17.0"
1515
package_config: ^2.1.0

test/short/whitespace/collections.stmt

Lines changed: 1 addition & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -92,34 +92,4 @@ f() async {
9292
<<<
9393
f() async {
9494
var l = [await for (x in y) x];
95-
}
96-
>>> null-aware elements
97-
foo1(int? x) => [
98-
?x];
99-
<<<
100-
foo1(int? x) => [?x];
101-
>>>
102-
foo2(int? x) => {
103-
?x};
104-
<<<
105-
foo2(int? x) => {?x};
106-
>>>
107-
bar0() => {
108-
"key": #value};
109-
<<<
110-
bar0() => {"key": #value};
111-
>>>
112-
bar1(int? x) => {
113-
?x: "value"};
114-
<<<
115-
bar1(int? x) => {?x: "value"};
116-
>>>
117-
bar2(String? y) => {
118-
"key": ?y};
119-
<<<
120-
bar2(String? y) => {"key": ?y};
121-
>>>
122-
bar3(int? x, String? y) => {
123-
?x: ?y};
124-
<<<
125-
bar3(int? x, String? y) => {?x: ?y};
95+
}
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
40 columns |
2+
>>> (experiment null-aware-elements) List element.
3+
var list = [ ? x ];
4+
<<<
5+
var list = [?x];
6+
>>> (experiment null-aware-elements) Set element.
7+
var set = { ? x };
8+
<<<
9+
var set = {?x};
10+
>>> (experiment null-aware-elements) Map key.
11+
var map = { ? key : value};
12+
<<<
13+
var map = {?key: value};
14+
>>> (experiment null-aware-elements) Map value.
15+
var map = { key: ? value };
16+
<<<
17+
var map = {key: ?value};
18+
>>> (experiment null-aware-elements) Both key and value.
19+
var map = { ? key : ? value };
20+
<<<
21+
var map = {?key: ?value};
22+
>>> (experiment null-aware-elements) Split inside element.
23+
var list = [?(veryLongExpression +thatIsForcedToSplit)];
24+
<<<
25+
var list = [
26+
?(veryLongExpression +
27+
thatIsForcedToSplit),
28+
];
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
40 columns |
2+
>>> (experiment null-aware-elements) Inline comment after `?`.
3+
var list = [ ? /* c */ x ];
4+
<<<
5+
var list = [? /* c */ x];
6+
>>> (experiment null-aware-elements)
7+
var map = { ? /* c */ key : ? /* c */ value };
8+
<<<
9+
var map = {
10+
? /* c */ key: ? /* c */ value,
11+
};
12+
>>> (experiment null-aware-elements) Line comment after `?`.
13+
var list = [ ? // c
14+
x ];
15+
<<<
16+
### This is an odd place for a comment so the formatting is odd, but we want to
17+
### at least pin it down with a test.
18+
var list = [
19+
? // c
20+
x,
21+
];
22+
>>> (experiment null-aware-elements)
23+
var map = { ? // c
24+
key : ? // c
25+
value };
26+
<<<
27+
### This is an odd place for a comment so the formatting is odd, but we want to
28+
### at least pin it down with a test.
29+
var map = {
30+
? // c
31+
key:
32+
? // c
33+
value,
34+
};

test/utils.dart

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,8 @@ void _testFile(TestFile testFile) {
130130
var formatter = DartFormatter(
131131
languageVersion: formatTest.languageVersion,
132132
pageWidth: testFile.pageWidth,
133-
indent: formatTest.leadingIndent);
133+
indent: formatTest.leadingIndent,
134+
experimentFlags: formatTest.experimentFlags);
134135

135136
var actual = _validateFormat(
136137
formatter,

tool/update_tests.dart

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,8 @@ Future<void> _updateTestFile(TestFile testFile) async {
9292
var formatter = DartFormatter(
9393
languageVersion: formatTest.languageVersion,
9494
pageWidth: testFile.pageWidth,
95-
indent: formatTest.leadingIndent);
95+
indent: formatTest.leadingIndent,
96+
experimentFlags: formatTest.experimentFlags);
9697

9798
var actual = formatter.formatSource(formatTest.input);
9899

@@ -110,6 +111,8 @@ Future<void> _updateTestFile(TestFile testFile) async {
110111
: DartFormatter.latestShortStyleLanguageVersion;
111112

112113
var descriptionParts = [
114+
for (var experiment in formatTest.experimentFlags)
115+
'(experiment $experiment)',
113116
if (formatTest.leadingIndent != 0) '(indent ${formatTest.leadingIndent})',
114117
if (formatTest.languageVersion != defaultLanguageVersion)
115118
'(version ${formatTest.languageVersion.major}.'

0 commit comments

Comments
 (0)