Skip to content

Commit 5b89991

Browse files
munificentmunificent
authored
Format typedefs. (#1339)
I also resurrected the example code used for debugging that was inadvertently deleted by 6249194.
1 parent 34dd26f commit 5b89991

File tree

6 files changed

+280
-12
lines changed

6 files changed

+280
-12
lines changed

‎example/format.dart

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,48 @@ void main(List<String> args) {
1717
debug.tracePieceBuilder = true;
1818
debug.traceSolver = true;
1919

20+
_formatStmt('''
21+
1 + 2;
22+
''');
23+
24+
_formatUnit('''
25+
class C {}
26+
''');
27+
2028
_runTest('selection/selection.stmt', 2);
2129
}
2230

31+
void _formatStmt(String source, {bool tall = true, int pageWidth = 40}) {
32+
_runFormatter(source, pageWidth, tall: tall, isCompilationUnit: false);
33+
}
34+
35+
void _formatUnit(String source, {bool tall = true, int pageWidth = 40}) {
36+
_runFormatter(source, pageWidth, tall: tall, isCompilationUnit: true);
37+
}
38+
39+
void _runFormatter(String source, int pageWidth,
40+
{required bool tall, required bool isCompilationUnit}) {
41+
try {
42+
var formatter = DartFormatter(
43+
pageWidth: pageWidth,
44+
experimentFlags: [if (tall) tallStyleExperimentFlag]);
45+
46+
String result;
47+
if (isCompilationUnit) {
48+
result = formatter.format(source);
49+
} else {
50+
result = formatter.formatStatement(source);
51+
}
52+
53+
_drawRuler('before', pageWidth);
54+
print(source);
55+
_drawRuler('after', pageWidth);
56+
print(result);
57+
} on FormatterException catch (error) {
58+
print(error.message());
59+
}
60+
}
61+
2362
void _drawRuler(String label, int width) {
2463
var padding = ' ' * (width - label.length - 1);
2564
print('$label:$padding|');

‎lib/src/front_end/ast_node_visitor.dart

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -871,7 +871,16 @@ class AstNodeVisitor extends ThrowingAstVisitor<Piece> with PieceFactory {
871871

872872
@override
873873
Piece visitFunctionTypeAlias(FunctionTypeAlias node) {
874-
throw UnimplementedError();
874+
if (node.metadata.isNotEmpty) throw UnimplementedError();
875+
876+
return buildPiece((b) {
877+
b.token(node.typedefKeyword);
878+
b.space();
879+
b.token(node.name);
880+
b.visit(node.typeParameters);
881+
b.visit(node.parameters);
882+
b.token(node.semicolon);
883+
});
875884
}
876885

877886
@override
@@ -893,7 +902,22 @@ class AstNodeVisitor extends ThrowingAstVisitor<Piece> with PieceFactory {
893902

894903
@override
895904
Piece visitGenericTypeAlias(GenericTypeAlias node) {
896-
throw UnimplementedError();
905+
if (node.metadata.isNotEmpty) throw UnimplementedError();
906+
907+
return buildPiece((b) {
908+
b.token(node.typedefKeyword);
909+
b.space();
910+
b.token(node.name);
911+
b.visit(node.typeParameters);
912+
b.space();
913+
b.token(node.equals);
914+
// Don't bother allowing splitting after the `=`. It's always better to
915+
// split inside the type parameter, type argument, or parameter lists of
916+
// the typedef or the aliased type.
917+
b.space();
918+
b.visit(node.type);
919+
b.token(node.semicolon);
920+
});
897921
}
898922

899923
@override

‎lib/src/front_end/piece_factory.dart

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -261,7 +261,7 @@ mixin PieceFactory {
261261
/// getter or setter declaration.
262262
Piece createFunction(
263263
{List<Token?> modifiers = const [],
264-
AstNode? returnType,
264+
TypeAnnotation? returnType,
265265
Token? operatorKeyword,
266266
Token? propertyKeyword,
267267
Token? name,
@@ -288,7 +288,9 @@ mixin PieceFactory {
288288

289289
var bodyPiece = createFunctionBody(body);
290290

291-
return FunctionPiece(returnTypePiece, signature, bodyPiece);
291+
return FunctionPiece(returnTypePiece, signature,
292+
isReturnTypeFunctionType: returnType is GenericFunctionType,
293+
body: bodyPiece);
292294
}
293295

294296
/// Creates a piece for a function, method, or constructor body.
@@ -334,7 +336,8 @@ mixin PieceFactory {
334336
builder.visit(parameters);
335337
builder.token(question);
336338

337-
return FunctionPiece(returnTypePiece, builder.build());
339+
return FunctionPiece(returnTypePiece, builder.build(),
340+
isReturnTypeFunctionType: returnType is GenericFunctionType);
338341
}
339342

340343
/// Creates a [TryPiece] for try statement.

‎lib/src/piece/function.dart

Lines changed: 33 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,6 @@ import 'piece.dart';
99
///
1010
/// Handles splitting between the return type and the rest of the function.
1111
class FunctionPiece extends Piece {
12-
static const _splitAfterReturnType = State(1, cost: 2);
13-
1412
/// The return type annotation, if any.
1513
final Piece? _returnType;
1614

@@ -21,24 +19,52 @@ class FunctionPiece extends Piece {
2119
/// If this is a function declaration with a (non-empty `;`) body, the body.
2220
final Piece? _body;
2321

24-
FunctionPiece(this._returnType, this._signature, [this._body]);
22+
/// Whether the return type is a function type.
23+
///
24+
/// When it is, allow splitting fairly cheaply because the return type is
25+
/// usually pretty big and looks good on its own line, or at least better
26+
/// then splitting inside the return type's parameter list. Prefers:
27+
///
28+
/// Function(int x, int y)
29+
/// returnFunction() { ... }
30+
///
31+
/// Over:
32+
///
33+
/// Function(
34+
/// int x,
35+
/// int y,
36+
/// ) returnFunction() { ... }
37+
///
38+
/// If the return type is *not* a function type, is almost always looks worse
39+
/// to split at the return type, so make that high cost.
40+
final bool _isReturnTypeFunctionType;
41+
42+
FunctionPiece(this._returnType, this._signature,
43+
{required bool isReturnTypeFunctionType, Piece? body})
44+
: _body = body,
45+
_isReturnTypeFunctionType = isReturnTypeFunctionType;
2546

2647
@override
27-
List<State> get additionalStates =>
28-
[if (_returnType != null) _splitAfterReturnType];
48+
List<State> get additionalStates => [if (_returnType != null) State.split];
49+
50+
@override
51+
int stateCost(State state) {
52+
if (state == State.split) return _isReturnTypeFunctionType ? 1 : 4;
53+
return super.stateCost(state);
54+
}
2955

3056
@override
3157
void format(CodeWriter writer, State state) {
3258
if (_returnType case var returnType?) {
3359
// A split inside the return type forces splitting after the return type.
34-
writer.setAllowNewlines(state == _splitAfterReturnType);
60+
writer.setAllowNewlines(state == State.split);
3561

3662
writer.format(returnType);
3763

3864
// A split in the type parameters or parameters does not force splitting
3965
// after the return type.
4066
writer.setAllowNewlines(true);
41-
writer.splitIf(state == _splitAfterReturnType);
67+
writer.splitIf(state == State.split);
4268
}
4369

4470
writer.format(_signature);

‎test/declaration/typedef.unit

Lines changed: 147 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,147 @@
1+
40 columns |
2+
>>> Old generic typedef syntax.
3+
typedef Foo < T ,S >(T t,S s);
4+
<<<
5+
typedef Foo<T, S>(T t, S s);
6+
>>> Unsplit generic function type.
7+
typedef F = A Function < A , B > ( A a , B b ) ;
8+
<<<
9+
typedef F = A Function<A, B>(A a, B b);
10+
>>> Split in function type parameter list.
11+
typedef SomeFunc=ReturnType Function(int param, double other);
12+
<<<
13+
typedef SomeFunc = ReturnType Function(
14+
int param,
15+
double other,
16+
);
17+
>>> Nested function type.
18+
typedef SomeFunc = Function(int first, Function(int first, bool second, String third) second, String third);
19+
<<<
20+
typedef SomeFunc = Function(
21+
int first,
22+
Function(
23+
int first,
24+
bool second,
25+
String third,
26+
) second,
27+
String third,
28+
);
29+
>>> Generic typedef.
30+
typedef Foo < A , B > = Function ( A a , B b ) ;
31+
<<<
32+
typedef Foo<A, B> = Function(A a, B b);
33+
>>> Non-function typedef.
34+
typedef Foo = Bar;
35+
<<<
36+
typedef Foo = Bar;
37+
>>> Non-function typedef of generic type.
38+
typedef Json = Map < String , Object ? > ;
39+
<<<
40+
typedef Json = Map<String, Object?>;
41+
>>> Split function type parameters.
42+
typedef G = T Function<TypeOne, TypeTwo, TypeThree>();
43+
<<<
44+
typedef G = T Function<
45+
TypeOne,
46+
TypeTwo,
47+
TypeThree
48+
>();
49+
>>> Split function type and value parameters.
50+
typedef G = T Function<TypeOne, TypeTwo, TypeThree>(TypeOne one, TypeTwo two, TypeThree three);
51+
<<<
52+
typedef G = T Function<
53+
TypeOne,
54+
TypeTwo,
55+
TypeThree
56+
>(
57+
TypeOne one,
58+
TypeTwo two,
59+
TypeThree three,
60+
);
61+
>>> Split typedef type parameters and function parameters.
62+
typedef LongfunctionType<First, Second, Third, Fourth, Fifth, Sixth> = Function<Seventh>(First first, Second second, Third third, Fourth fourth);
63+
<<<
64+
typedef LongfunctionType<
65+
First,
66+
Second,
67+
Third,
68+
Fourth,
69+
Fifth,
70+
Sixth
71+
> = Function<Seventh>(
72+
First first,
73+
Second second,
74+
Third third,
75+
Fourth fourth,
76+
);
77+
>>> Split typedef type parameters.
78+
typedef LongfunctionType<First, Second, Third, Fourth, Fifth, Sixth> = Type;
79+
<<<
80+
typedef LongfunctionType<
81+
First,
82+
Second,
83+
Third,
84+
Fourth,
85+
Fifth,
86+
Sixth
87+
> = Type;
88+
>>> All three parameter lists split.
89+
typedef LongfunctionType<First, Second, Third, Fourth, Fifth, Sixth> = Function<Seventh, Eighth, Ninth, Tenth, Eleventh, Twelfth, Thirteenth>(First first, Second second, Third third, Fourth fourth);
90+
<<<
91+
typedef LongfunctionType<
92+
First,
93+
Second,
94+
Third,
95+
Fourth,
96+
Fifth,
97+
Sixth
98+
> = Function<
99+
Seventh,
100+
Eighth,
101+
Ninth,
102+
Tenth,
103+
Eleventh,
104+
Twelfth,
105+
Thirteenth
106+
>(
107+
First first,
108+
Second second,
109+
Third third,
110+
Fourth fourth,
111+
);
112+
>>> Split in non-function type argument list.
113+
typedef G = SomeType<TypeOne, TypeTwo, TypeThree>;
114+
<<<
115+
typedef G = SomeType<
116+
TypeOne,
117+
TypeTwo,
118+
TypeThree
119+
>;
120+
>>> Split typedef type parameters and non-function type arguments.
121+
typedef LongGenericType<First, Second, Third, Fourth, Fifth, Sixth> = AnotherType<First, Second, Third, Fourth>;
122+
<<<
123+
typedef LongGenericType<
124+
First,
125+
Second,
126+
Third,
127+
Fourth,
128+
Fifth,
129+
Sixth
130+
> = AnotherType<
131+
First,
132+
Second,
133+
Third,
134+
Fourth
135+
>;
136+
>>> Prefer splitting parameter list over type parameter list.
137+
typedef Generic = R Function<T, R>(T param);
138+
<<<
139+
typedef Generic = R Function<T, R>(
140+
T param,
141+
);
142+
>>>
143+
typedef Generic<T, R> = R Function(T param);
144+
<<<
145+
typedef Generic<T, R> = R Function(
146+
T param,
147+
);

‎test/declaration/typedef_comment.unit

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
40 columns |
2+
>>> Line comment after `typedef`.
3+
typedef // c
4+
Alias = Type;
5+
<<<
6+
typedef // c
7+
Alias = Type;
8+
>>> Line comment after name.
9+
typedef Alias // c
10+
= Type;
11+
<<<
12+
typedef Alias // c
13+
= Type;
14+
>>> Line comment after `=`.
15+
typedef Alias = // c
16+
Type;
17+
<<<
18+
typedef Alias = // c
19+
Type;
20+
>>> Line comment after type.
21+
typedef Alias = Type // c
22+
;
23+
<<<
24+
typedef Alias = Type // c
25+
;
26+
>>> Line comment after `;`.
27+
typedef Alias = Type; // c
28+
<<<
29+
typedef Alias = Type; // c

0 commit comments

Comments
 (0)