From f4083506e4fed27964a772995ef4758819b833da Mon Sep 17 00:00:00 2001 From: Bob Nystrom Date: Tue, 8 Oct 2024 16:38:38 -0700 Subject: [PATCH] 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. --- lib/src/front_end/ast_node_visitor.dart | 20 ++++-- lib/src/piece/type_parameter_bound.dart | 65 ++++++++++++++++++ .../declaration/class_type_parameter.unit | 17 +++++ test/tall/regression/1500/1568.unit | 67 +++++++++++++++++++ 4 files changed, 164 insertions(+), 5 deletions(-) create mode 100644 lib/src/piece/type_parameter_bound.dart create mode 100644 test/tall/regression/1500/1568.unit diff --git a/lib/src/front_end/ast_node_visitor.dart b/lib/src/front_end/ast_node_visitor.dart index 05587a04..c7e16893 100644 --- a/lib/src/front_end/ast_node_visitor.dart +++ b/lib/src/front_end/ast_node_visitor.dart @@ -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'; @@ -1840,12 +1841,21 @@ final class AstNodeVisitor extends ThrowingAstVisitor 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); } }); } diff --git a/lib/src/piece/type_parameter_bound.dart b/lib/src/piece/type_parameter_bound.dart new file mode 100644 index 00000000..3d7a5a7a --- /dev/null +++ b/lib/src/piece/type_parameter_bound.dart @@ -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 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); + } +} diff --git a/test/tall/declaration/class_type_parameter.unit b/test/tall/declaration/class_type_parameter.unit index 6e28b06b..065e1b6a 100644 --- a/test/tall/declaration/class_type_parameter.unit +++ b/test/tall/declaration/class_type_parameter.unit @@ -19,6 +19,23 @@ class LongClassName< Second, Third > {} +>>> Split inside bound. +class LongClassName {} +<<< +class LongClassName< + VeryLongTypeParameter + extends VeryLongBoundName +> {} +>>> Split at `extends` and inside bound type arguments. +class LongClassName> {} +<<< +class LongClassName< + VeryLongTypeParameter + extends VeryLongBoundName< + LongTypeArgument + > +> {} >>> Split inside type parameter bound splits type parameters. class LongClassName> {} <<< diff --git a/test/tall/regression/1500/1568.unit b/test/tall/regression/1500/1568.unit new file mode 100644 index 00000000..6530aed8 --- /dev/null +++ b/test/tall/regression/1500/1568.unit @@ -0,0 +1,67 @@ +>>> Conveniently fits in the new style. +abstract interface class TypeAnalyzerOperations< + TypeStructure extends SharedTypeStructure, + Variable extends Object, + TypeParameterStructure extends SharedTypeParameterStructure, + TypeDeclarationType extends Object, + TypeDeclaration extends Object> + implements FlowAnalysisOperations> { + // ... +} +<<< +abstract interface class TypeAnalyzerOperations< + TypeStructure extends SharedTypeStructure, + Variable extends Object, + TypeParameterStructure extends SharedTypeParameterStructure, + TypeDeclarationType extends Object, + TypeDeclaration extends Object +> + implements FlowAnalysisOperations> { + // ... +} +>>> Make some type arguments longer to force it to split. +abstract interface class TypeAnalyzerOperations< + TypeStructure extends SharedTypeStructure, + Variable extends Object, + TypeParameterStructure extends SharedTypeParameterStructure, + TypeDeclarationType extends Object, + TypeDeclaration extends Object> + implements FlowAnalysisOperations> { + // ... +} +<<< +abstract interface class TypeAnalyzerOperations< + TypeStructure extends SharedTypeStructure, + Variable extends Object, + TypeParameterStructure + extends SharedTypeParameterStructure, + TypeDeclarationType extends Object, + TypeDeclaration extends Object +> + implements FlowAnalysisOperations> { + // ... +} +>>> Even longer for more splitting. +abstract interface class TypeAnalyzerOperations< + TypeStructure extends SharedTypeStructure, + Variable extends Object, + TypeParameterStructure extends SharedTypeParameterStructure, + TypeDeclarationType extends Object, + TypeDeclaration extends Object> + implements FlowAnalysisOperations> { + // ... +} +<<< +abstract interface class TypeAnalyzerOperations< + TypeStructure extends SharedTypeStructure, + Variable extends Object, + TypeParameterStructure extends SharedTypeParameterStructure< + LongerTypeStructure, + AnotherTypeArgument + >, + TypeDeclarationType extends Object, + TypeDeclaration extends Object +> + implements FlowAnalysisOperations> { + // ... +} \ No newline at end of file