Skip to content

Commit

Permalink
Better formatting for long type parameter bounds. (#1576)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
munificent authored Oct 8, 2024
1 parent 618883f commit f408350
Show file tree
Hide file tree
Showing 4 changed files with 164 additions and 5 deletions.
20 changes: 15 additions & 5 deletions lib/src/front_end/ast_node_visitor.dart
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import '../piece/infix.dart';
import '../piece/list.dart';
import '../piece/piece.dart';
import '../piece/type.dart';
import '../piece/type_parameter_bound.dart';
import '../piece/variable.dart';
import '../profile.dart';
import '../source_code.dart';
Expand Down Expand Up @@ -1840,12 +1841,21 @@ final class AstNodeVisitor extends ThrowingAstVisitor<void> with PieceFactory {
@override
void visitTypeParameter(TypeParameter node) {
pieces.withMetadata(node.metadata, inlineMetadata: true, () {
pieces.token(node.name);
if (node.bound case var bound?) {
pieces.space();
pieces.token(node.extendsKeyword);
pieces.space();
pieces.visit(bound);
var typeParameterPiece = pieces.build(() {
pieces.token(node.name);
});

var boundPiece = pieces.build(() {
pieces.token(node.extendsKeyword);
pieces.space();
pieces.visit(bound);
});

pieces.add(TypeParameterBoundPiece(typeParameterPiece, boundPiece));
} else {
// No bound.
pieces.token(node.name);
}
});
}
Expand Down
65 changes: 65 additions & 0 deletions lib/src/piece/type_parameter_bound.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.
import '../back_end/code_writer.dart';
import '../constants.dart';
import 'piece.dart';

/// A piece for a type parameter and its bound.
///
/// Handles not splitting before `extends` if we can split inside the bound's
/// own type arguments, or splitting before `extends` if that isn't enough to
/// get it to fit.
final class TypeParameterBoundPiece extends Piece {
/// Split inside the type arguments of the bound, but not at `extends`, as in:
///
/// class C<
/// T extends Map<
/// LongKeyType,
/// LongValueType
/// >
/// >{}
static const State _insideBound = State(1);

/// Split at `extends`, like:
///
/// class C<
/// LongTypeParameters
/// extends LongBoundType
/// >{}
static const State _beforeExtends = State(2);

/// The type parameter name.
final Piece _typeParameter;

/// The bound with the preceding `extends` keyword.
final Piece _bound;

TypeParameterBoundPiece(this._typeParameter, this._bound);

@override
List<State> get additionalStates => const [_insideBound, _beforeExtends];

@override
bool allowNewlineInChild(State state, Piece child) => switch (state) {
State.unsplit => false,
_insideBound => child == _bound,
_beforeExtends => true,
_ => throw ArgumentError('Unexpected state.'),
};

@override
void format(CodeWriter writer, State state) {
if (state == _beforeExtends) writer.pushIndent(Indent.expression);
writer.format(_typeParameter);
writer.splitIf(state == _beforeExtends);
writer.format(_bound);
if (state == _beforeExtends) writer.popIndent();
}

@override
void forEachChild(void Function(Piece piece) callback) {
callback(_typeParameter);
callback(_bound);
}
}
17 changes: 17 additions & 0 deletions test/tall/declaration/class_type_parameter.unit
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,23 @@ class LongClassName<
Second,
Third
> {}
>>> Split inside bound.
class LongClassName<VeryLongTypeParameter extends VeryLongBoundName> {}
<<<
class LongClassName<
VeryLongTypeParameter
extends VeryLongBoundName
> {}
>>> Split at `extends` and inside bound type arguments.
class LongClassName<VeryLongTypeParameter extends
VeryLongBoundName<LongTypeArgument>> {}
<<<
class LongClassName<
VeryLongTypeParameter
extends VeryLongBoundName<
LongTypeArgument
>
> {}
>>> Split inside type parameter bound splits type parameters.
class LongClassName<T extends Map<LongTypeArgument, Another>> {}
<<<
Expand Down
67 changes: 67 additions & 0 deletions test/tall/regression/1500/1568.unit
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
>>> Conveniently fits in the new style.
abstract interface class TypeAnalyzerOperations<
TypeStructure extends SharedTypeStructure<TypeStructure>,
Variable extends Object,
TypeParameterStructure extends SharedTypeParameterStructure<TypeStructure>,
TypeDeclarationType extends Object,
TypeDeclaration extends Object>
implements FlowAnalysisOperations<Variable, SharedTypeView<TypeStructure>> {
// ...
}
<<<
abstract interface class TypeAnalyzerOperations<
TypeStructure extends SharedTypeStructure<TypeStructure>,
Variable extends Object,
TypeParameterStructure extends SharedTypeParameterStructure<TypeStructure>,
TypeDeclarationType extends Object,
TypeDeclaration extends Object
>
implements FlowAnalysisOperations<Variable, SharedTypeView<TypeStructure>> {
// ...
}
>>> Make some type arguments longer to force it to split.
abstract interface class TypeAnalyzerOperations<
TypeStructure extends SharedTypeStructure<TypeStructure>,
Variable extends Object,
TypeParameterStructure extends SharedTypeParameterStructure<LongerTypeStructure>,
TypeDeclarationType extends Object,
TypeDeclaration extends Object>
implements FlowAnalysisOperations<Variable, SharedTypeView<TypeStructure>> {
// ...
}
<<<
abstract interface class TypeAnalyzerOperations<
TypeStructure extends SharedTypeStructure<TypeStructure>,
Variable extends Object,
TypeParameterStructure
extends SharedTypeParameterStructure<LongerTypeStructure>,
TypeDeclarationType extends Object,
TypeDeclaration extends Object
>
implements FlowAnalysisOperations<Variable, SharedTypeView<TypeStructure>> {
// ...
}
>>> Even longer for more splitting.
abstract interface class TypeAnalyzerOperations<
TypeStructure extends SharedTypeStructure<TypeStructure>,
Variable extends Object,
TypeParameterStructure extends SharedTypeParameterStructure<LongerTypeStructure, AnotherTypeArgument>,
TypeDeclarationType extends Object,
TypeDeclaration extends Object>
implements FlowAnalysisOperations<Variable, SharedTypeView<TypeStructure>> {
// ...
}
<<<
abstract interface class TypeAnalyzerOperations<
TypeStructure extends SharedTypeStructure<TypeStructure>,
Variable extends Object,
TypeParameterStructure extends SharedTypeParameterStructure<
LongerTypeStructure,
AnotherTypeArgument
>,
TypeDeclarationType extends Object,
TypeDeclaration extends Object
>
implements FlowAnalysisOperations<Variable, SharedTypeView<TypeStructure>> {
// ...
}

0 comments on commit f408350

Please sign in to comment.