Skip to content

Commit f408350

Browse files
authored
Better formatting for long type parameter bounds. (#1576)
Better formatting for long type parameter bounds. The tall style was already doing a better job on #1568 than the old short style does. (I don't know why it leaves an overly long line there. It seems like a bug but perhaps a nasty subtle one in the solver.) But it still didn't allow splitting before `extends` in a bound which could leave long lines in cases where the type parameter and bound name are both quite long. Rare, but we may as well do a better job. So this introduces a new piece that allows splitting before `extends` if needed. Fix #1568.
1 parent 618883f commit f408350

File tree

4 files changed

+164
-5
lines changed

4 files changed

+164
-5
lines changed

lib/src/front_end/ast_node_visitor.dart

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import '../piece/infix.dart';
1717
import '../piece/list.dart';
1818
import '../piece/piece.dart';
1919
import '../piece/type.dart';
20+
import '../piece/type_parameter_bound.dart';
2021
import '../piece/variable.dart';
2122
import '../profile.dart';
2223
import '../source_code.dart';
@@ -1840,12 +1841,21 @@ final class AstNodeVisitor extends ThrowingAstVisitor<void> with PieceFactory {
18401841
@override
18411842
void visitTypeParameter(TypeParameter node) {
18421843
pieces.withMetadata(node.metadata, inlineMetadata: true, () {
1843-
pieces.token(node.name);
18441844
if (node.bound case var bound?) {
1845-
pieces.space();
1846-
pieces.token(node.extendsKeyword);
1847-
pieces.space();
1848-
pieces.visit(bound);
1845+
var typeParameterPiece = pieces.build(() {
1846+
pieces.token(node.name);
1847+
});
1848+
1849+
var boundPiece = pieces.build(() {
1850+
pieces.token(node.extendsKeyword);
1851+
pieces.space();
1852+
pieces.visit(bound);
1853+
});
1854+
1855+
pieces.add(TypeParameterBoundPiece(typeParameterPiece, boundPiece));
1856+
} else {
1857+
// No bound.
1858+
pieces.token(node.name);
18491859
}
18501860
});
18511861
}
Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
import '../back_end/code_writer.dart';
5+
import '../constants.dart';
6+
import 'piece.dart';
7+
8+
/// A piece for a type parameter and its bound.
9+
///
10+
/// Handles not splitting before `extends` if we can split inside the bound's
11+
/// own type arguments, or splitting before `extends` if that isn't enough to
12+
/// get it to fit.
13+
final class TypeParameterBoundPiece extends Piece {
14+
/// Split inside the type arguments of the bound, but not at `extends`, as in:
15+
///
16+
/// class C<
17+
/// T extends Map<
18+
/// LongKeyType,
19+
/// LongValueType
20+
/// >
21+
/// >{}
22+
static const State _insideBound = State(1);
23+
24+
/// Split at `extends`, like:
25+
///
26+
/// class C<
27+
/// LongTypeParameters
28+
/// extends LongBoundType
29+
/// >{}
30+
static const State _beforeExtends = State(2);
31+
32+
/// The type parameter name.
33+
final Piece _typeParameter;
34+
35+
/// The bound with the preceding `extends` keyword.
36+
final Piece _bound;
37+
38+
TypeParameterBoundPiece(this._typeParameter, this._bound);
39+
40+
@override
41+
List<State> get additionalStates => const [_insideBound, _beforeExtends];
42+
43+
@override
44+
bool allowNewlineInChild(State state, Piece child) => switch (state) {
45+
State.unsplit => false,
46+
_insideBound => child == _bound,
47+
_beforeExtends => true,
48+
_ => throw ArgumentError('Unexpected state.'),
49+
};
50+
51+
@override
52+
void format(CodeWriter writer, State state) {
53+
if (state == _beforeExtends) writer.pushIndent(Indent.expression);
54+
writer.format(_typeParameter);
55+
writer.splitIf(state == _beforeExtends);
56+
writer.format(_bound);
57+
if (state == _beforeExtends) writer.popIndent();
58+
}
59+
60+
@override
61+
void forEachChild(void Function(Piece piece) callback) {
62+
callback(_typeParameter);
63+
callback(_bound);
64+
}
65+
}

test/tall/declaration/class_type_parameter.unit

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,23 @@ class LongClassName<
1919
Second,
2020
Third
2121
> {}
22+
>>> Split inside bound.
23+
class LongClassName<VeryLongTypeParameter extends VeryLongBoundName> {}
24+
<<<
25+
class LongClassName<
26+
VeryLongTypeParameter
27+
extends VeryLongBoundName
28+
> {}
29+
>>> Split at `extends` and inside bound type arguments.
30+
class LongClassName<VeryLongTypeParameter extends
31+
VeryLongBoundName<LongTypeArgument>> {}
32+
<<<
33+
class LongClassName<
34+
VeryLongTypeParameter
35+
extends VeryLongBoundName<
36+
LongTypeArgument
37+
>
38+
> {}
2239
>>> Split inside type parameter bound splits type parameters.
2340
class LongClassName<T extends Map<LongTypeArgument, Another>> {}
2441
<<<
Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
>>> Conveniently fits in the new style.
2+
abstract interface class TypeAnalyzerOperations<
3+
TypeStructure extends SharedTypeStructure<TypeStructure>,
4+
Variable extends Object,
5+
TypeParameterStructure extends SharedTypeParameterStructure<TypeStructure>,
6+
TypeDeclarationType extends Object,
7+
TypeDeclaration extends Object>
8+
implements FlowAnalysisOperations<Variable, SharedTypeView<TypeStructure>> {
9+
// ...
10+
}
11+
<<<
12+
abstract interface class TypeAnalyzerOperations<
13+
TypeStructure extends SharedTypeStructure<TypeStructure>,
14+
Variable extends Object,
15+
TypeParameterStructure extends SharedTypeParameterStructure<TypeStructure>,
16+
TypeDeclarationType extends Object,
17+
TypeDeclaration extends Object
18+
>
19+
implements FlowAnalysisOperations<Variable, SharedTypeView<TypeStructure>> {
20+
// ...
21+
}
22+
>>> Make some type arguments longer to force it to split.
23+
abstract interface class TypeAnalyzerOperations<
24+
TypeStructure extends SharedTypeStructure<TypeStructure>,
25+
Variable extends Object,
26+
TypeParameterStructure extends SharedTypeParameterStructure<LongerTypeStructure>,
27+
TypeDeclarationType extends Object,
28+
TypeDeclaration extends Object>
29+
implements FlowAnalysisOperations<Variable, SharedTypeView<TypeStructure>> {
30+
// ...
31+
}
32+
<<<
33+
abstract interface class TypeAnalyzerOperations<
34+
TypeStructure extends SharedTypeStructure<TypeStructure>,
35+
Variable extends Object,
36+
TypeParameterStructure
37+
extends SharedTypeParameterStructure<LongerTypeStructure>,
38+
TypeDeclarationType extends Object,
39+
TypeDeclaration extends Object
40+
>
41+
implements FlowAnalysisOperations<Variable, SharedTypeView<TypeStructure>> {
42+
// ...
43+
}
44+
>>> Even longer for more splitting.
45+
abstract interface class TypeAnalyzerOperations<
46+
TypeStructure extends SharedTypeStructure<TypeStructure>,
47+
Variable extends Object,
48+
TypeParameterStructure extends SharedTypeParameterStructure<LongerTypeStructure, AnotherTypeArgument>,
49+
TypeDeclarationType extends Object,
50+
TypeDeclaration extends Object>
51+
implements FlowAnalysisOperations<Variable, SharedTypeView<TypeStructure>> {
52+
// ...
53+
}
54+
<<<
55+
abstract interface class TypeAnalyzerOperations<
56+
TypeStructure extends SharedTypeStructure<TypeStructure>,
57+
Variable extends Object,
58+
TypeParameterStructure extends SharedTypeParameterStructure<
59+
LongerTypeStructure,
60+
AnotherTypeArgument
61+
>,
62+
TypeDeclarationType extends Object,
63+
TypeDeclaration extends Object
64+
>
65+
implements FlowAnalysisOperations<Variable, SharedTypeView<TypeStructure>> {
66+
// ...
67+
}

0 commit comments

Comments
 (0)