From 2f0c2be7ac67bc196491cab23807c0a5ce310a89 Mon Sep 17 00:00:00 2001 From: Kallen Tu Date: Wed, 27 Dec 2023 00:49:17 +0000 Subject: [PATCH 01/10] Format record type annotations. --- lib/src/front_end/ast_node_visitor.dart | 57 +++++++- test/declaration/class.unit | 10 +- test/type/record.stmt | 184 ++++++++++++++++++++++++ test/type/record_comment.stmt | 65 +++++++++ 4 files changed, 312 insertions(+), 4 deletions(-) create mode 100644 test/type/record.stmt create mode 100644 test/type/record_comment.stmt diff --git a/lib/src/front_end/ast_node_visitor.dart b/lib/src/front_end/ast_node_visitor.dart index a1fc65a5..56a5de55 100644 --- a/lib/src/front_end/ast_node_visitor.dart +++ b/lib/src/front_end/ast_node_visitor.dart @@ -1511,19 +1511,70 @@ class AstNodeVisitor extends ThrowingAstVisitor with PieceFactory { @override Piece visitRecordTypeAnnotation(RecordTypeAnnotation node) { - throw UnimplementedError(); + var namedFields = node.namedFields; + var positionalFields = node.positionalFields; + + // Single positional record types always have a trailing comma. + var listStyle = node.positionalFields.length == 1 && namedFields == null + ? const ListStyle(commas: Commas.alwaysTrailing) + : const ListStyle(commas: Commas.trailing); + var builder = DelimitedListBuilder(this, listStyle); + + // If all parameters are optional, put the `{` right after `(`. + if (positionalFields.isEmpty && namedFields != null) { + builder.leftBracket( + node.leftParenthesis, + delimiter: namedFields.leftBracket, + ); + } else { + builder.leftBracket(node.leftParenthesis); + } + + for (var positionalField in positionalFields) { + builder.visit(positionalField); + } + + Token? rightDelimiter; + if (namedFields != null) { + // If this is the first optional parameter, put the delimiter before it. + if (positionalFields.isNotEmpty) { + builder.leftDelimiter(namedFields.leftBracket); + } + for (var namedField in namedFields.fields) { + builder.visit(namedField); + } + rightDelimiter = namedFields.rightBracket; + } + + builder.rightBracket(node.rightParenthesis, delimiter: rightDelimiter); + return buildPiece((b) { + b.add(builder.build()); + if (node.question case var question?) { + b.add(tokenPiece(question)); + } + }); } @override Piece visitRecordTypeAnnotationNamedField( RecordTypeAnnotationNamedField node) { - throw UnimplementedError(); + // TODO(tall): Format metadata. + if (node.metadata.isNotEmpty) throw UnimplementedError(); + return buildPiece((b) { + b.visit(node.type); + b.token(node.name, spaceBefore: true); + }); } @override Piece visitRecordTypeAnnotationPositionalField( RecordTypeAnnotationPositionalField node) { - throw UnimplementedError(); + // TODO(tall): Format metadata. + if (node.metadata.isNotEmpty) throw UnimplementedError(); + return buildPiece((b) { + b.visit(node.type); + b.token(node.name, spaceBefore: true); + }); } @override diff --git a/test/declaration/class.unit b/test/declaration/class.unit index a9a75c67..6a1291b1 100644 --- a/test/declaration/class.unit +++ b/test/declaration/class.unit @@ -73,4 +73,12 @@ abstract base mixin class C13 {} class SomeClass native "Zapp" { } <<< -class SomeClass native "Zapp" {} \ No newline at end of file +class SomeClass native "Zapp" {} +>>> Record type with multiple fields in parameter has no trailing comma. +class C { C((TypeName,TypeName,) this.record) {;} } +<<< +class C { + C((TypeName, TypeName) this.record) { + ; + } +} diff --git a/test/type/record.stmt b/test/type/record.stmt new file mode 100644 index 00000000..84cd6005 --- /dev/null +++ b/test/type/record.stmt @@ -0,0 +1,184 @@ +40 columns | +>>> Empty record. +( ) x; +<<< +() x; +>>> Empty nullable record type. +( ) ? x; +<<< +()? x; +>>> Nullable record type. +( int , bool ) ? x; +<<< +(int, bool)? x; +>>> Single positional field. +( int , ) x; +<<< +(int,) x; +>>> Single named field. +( { int n } ) x; +<<< +({int n}) x; +>>> Named positional fields. +( int value , String label) x; +<<< +(int value, String label) x; +>>> Unnamed positional fields. +( int , String ) x; +<<< +(int, String) x; +>>> Named fields. +( { int value , String label } ) x; +<<< +({int value, String label}) x; +>>> Split named positional fields. +( int longValue , String veryVeryLongLabel , ) x; +<<< +( + int longValue, + String veryVeryLongLabel, +) x; +>>> Unsplit unnamed positional fields have no trailing comma. +( int , String , ) x; +<<< +(int, String) x; +>>> Split only named fields. +( { int longValue , String veryLongLabel , } ) x; +<<< +({ + int longValue, + String veryLongLabel, +}) x; +>>> Empty record types don't split. +someLongFunctionName__________________(() x) {} +<<< +someLongFunctionName__________________( + () x, +) {} +>>> Unsplit short single positional field. +(TypeName, +) x; +<<< +(TypeName,) x; +>>> Unsplit single positional field. +function((VeryLongTypeName____________,) x) {;} +<<< +function( + (VeryLongTypeName____________,) x, +) { + ; +} +>>> Split single long positional field. +function((VeryLongTypeName___________________,) param) {;} +<<< +function( + ( + VeryLongTypeName___________________, + ) param, +) { + ; +} +>>> Split positional types. +(TypeName,TypeName,TypeName,TypeName) x; +<<< +( + TypeName, + TypeName, + TypeName, + TypeName, +) x; +>>> Split named types. +({TypeName a,TypeName b,TypeName c,TypeName d}) x; +<<< +({ + TypeName a, + TypeName b, + TypeName c, + TypeName d, +}) x; +>>> Split named if positional splits. +(TypeName,TypeName,TypeName,TypeName,{TypeName a,TypeName b}) x; +<<< +( + TypeName, + TypeName, + TypeName, + TypeName, { + TypeName a, + TypeName b, +}) x; +>>> Split positional if named splits. +(TypeName,TypeName,{TypeName a,TypeName b,TypeName c,TypeName d}) x; +<<< +( + TypeName, + TypeName, { + TypeName a, + TypeName b, + TypeName c, + TypeName d, +}) x; +>>> Single named field has no trailing comma. +({int n,}) x; +<<< +({int n}) x; +>>> Multiple positional fields have no trailing comma. +(int m, int n,) x; +<<< +(int m, int n) x; +>>> Split outer record if inner record splits. +((TypeName,TypeName,TypeName,TypeName),TypeName) x; +<<< +( + ( + TypeName, + TypeName, + TypeName, + TypeName, + ), + TypeName, +) x; +>>> Split outer type argument list if inner record splits. +Map map; +<<< +Map< + String, + ( + TypeName, + TypeName, + TypeName, + TypeName, + ) +> map; +>>> Split inside parameter list. +function((TypeName, TypeName, TypeName, TypeName, TypeName) record) {;} +<<< +function( + ( + TypeName, + TypeName, + TypeName, + TypeName, + TypeName, + ) record, +) { + ; +} +>>> Single positional has a trailing comma inside parameter list. +function((TypeName,) record) {;} +<<< +function((TypeName,) record) { + ; +} +>>> Named parameter has no trailing comma inside parameter list. +function(({TypeName param,}) record) {;} +<<< +function(({TypeName param}) record) { + ; +} +>>> Multiple positional fields have no trailing comma in parameter list. +function((TypeName,TypeName,) record,) {;} +<<< +function((TypeName, TypeName) record) { + ; +} diff --git a/test/type/record_comment.stmt b/test/type/record_comment.stmt new file mode 100644 index 00000000..4bb27584 --- /dev/null +++ b/test/type/record_comment.stmt @@ -0,0 +1,65 @@ +40 columns | +>>> Comment between the type and name of a field. +(int // comment +value, String label) x; +<<< +( + int // comment + value, + String label, +) x; +>>> Comment after field and comma. +(int value, // comment +String label) x; +<<< +( + int value, // comment + String label, +) x; +>>> Comment before field and comma. +(int value // comment +,String label) x; +<<< +( + int value, // comment + String label, +) x; +>>> Comment between positional and named delimiter. +(int value, // comment +{String label}) +x; +<<< +( + int value, { // comment + String label, +}) x; +>>> Comment after named left delimiter. +(int value, { // comment +String label}) +x; +<<< +( + int value, { // comment + String label, +}) x; +>>> Comment after named right delimiter. +(int value, {String label} // comment +) +x; +<<< +( + int value, { + String label, // comment +}) x; +>>> Comment between record type and nullable question mark. +(int value , ) // comment +? x; +<<< +(int value,) // comment +? x; +>>> Comment after record type. +(int value, String label) // comment +x; +<<< +(int value, String label) // comment +x; From b020a28b3eee7f7a087fd07070c0c583e30a67c2 Mon Sep 17 00:00:00 2001 From: Kallen Tu Date: Wed, 27 Dec 2023 00:51:39 +0000 Subject: [PATCH 02/10] Use local instead. --- lib/src/front_end/ast_node_visitor.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/src/front_end/ast_node_visitor.dart b/lib/src/front_end/ast_node_visitor.dart index 56a5de55..38c816ba 100644 --- a/lib/src/front_end/ast_node_visitor.dart +++ b/lib/src/front_end/ast_node_visitor.dart @@ -1515,7 +1515,7 @@ class AstNodeVisitor extends ThrowingAstVisitor with PieceFactory { var positionalFields = node.positionalFields; // Single positional record types always have a trailing comma. - var listStyle = node.positionalFields.length == 1 && namedFields == null + var listStyle = positionalFields.length == 1 && namedFields == null ? const ListStyle(commas: Commas.alwaysTrailing) : const ListStyle(commas: Commas.trailing); var builder = DelimitedListBuilder(this, listStyle); From 150d93ac1c0abb8046e9cd6ae0dce74c78ac2718 Mon Sep 17 00:00:00 2001 From: Kallen Tu Date: Wed, 3 Jan 2024 23:02:26 +0000 Subject: [PATCH 03/10] Rebase. --- test/declaration/class.unit | 8 ------ test/type/function.stmt | 52 +++++++++++++++++++++++++++++++++++++ test/type/record.stmt | 50 ++--------------------------------- 3 files changed, 54 insertions(+), 56 deletions(-) diff --git a/test/declaration/class.unit b/test/declaration/class.unit index 6a1291b1..504c7e9e 100644 --- a/test/declaration/class.unit +++ b/test/declaration/class.unit @@ -74,11 +74,3 @@ class SomeClass native "Zapp" { } <<< class SomeClass native "Zapp" {} ->>> Record type with multiple fields in parameter has no trailing comma. -class C { C((TypeName,TypeName,) this.record) {;} } -<<< -class C { - C((TypeName, TypeName) this.record) { - ; - } -} diff --git a/test/type/function.stmt b/test/type/function.stmt index dd0064b1..11261404 100644 --- a/test/type/function.stmt +++ b/test/type/function.stmt @@ -218,6 +218,58 @@ longMethod({ required int reallyLongParameterNameWow, }) {} +>>> Record type with multiple fields in parameter has no trailing comma. +function((TypeName, TypeName) parameter) { ; } +<<< +function( + (TypeName, TypeName) parameter, +) { + ; +} +>>> Split single long positional record type field. +function((VeryLongTypeName___________________,) param) {;} +<<< +function( + ( + VeryLongTypeName___________________, + ) param, +) { + ; +} +>>> Split inside parameter list with record type. +function((TypeName, TypeName, TypeName, TypeName, TypeName) record) {;} +<<< +function( + ( + TypeName, + TypeName, + TypeName, + TypeName, + TypeName, + ) record, +) { + ; +} +>>> Single positional has a trailing comma inside parameter list with record type. +function((TypeName,) record) {;} +<<< +function((TypeName,) record) { + ; +} +>>> Named parameter has no trailing comma inside parameter list with record type. +function(({TypeName param,}) record) {;} +<<< +function(({TypeName param}) record) { + ; +} +>>> Multiple positional fields have no trailing comma in parameter list with record type. +function((TypeName,TypeName,) record,) {;} +<<< +function((TypeName, TypeName) record) { + ; +} + + >>> Unsplit generic method instantiation. void main() => id < int > ; <<< diff --git a/test/type/record.stmt b/test/type/record.stmt index 84cd6005..527740cf 100644 --- a/test/type/record.stmt +++ b/test/type/record.stmt @@ -61,23 +61,9 @@ someLongFunctionName__________________( <<< (TypeName,) x; >>> Unsplit single positional field. -function((VeryLongTypeName____________,) x) {;} +(VeryLongTypeName________________,) x; <<< -function( - (VeryLongTypeName____________,) x, -) { - ; -} ->>> Split single long positional field. -function((VeryLongTypeName___________________,) param) {;} -<<< -function( - ( - VeryLongTypeName___________________, - ) param, -) { - ; -} +(VeryLongTypeName________________,) x; >>> Split positional types. (TypeName,TypeName,TypeName,TypeName) x; <<< @@ -150,35 +136,3 @@ Map< TypeName, ) > map; ->>> Split inside parameter list. -function((TypeName, TypeName, TypeName, TypeName, TypeName) record) {;} -<<< -function( - ( - TypeName, - TypeName, - TypeName, - TypeName, - TypeName, - ) record, -) { - ; -} ->>> Single positional has a trailing comma inside parameter list. -function((TypeName,) record) {;} -<<< -function((TypeName,) record) { - ; -} ->>> Named parameter has no trailing comma inside parameter list. -function(({TypeName param,}) record) {;} -<<< -function(({TypeName param}) record) { - ; -} ->>> Multiple positional fields have no trailing comma in parameter list. -function((TypeName,TypeName,) record,) {;} -<<< -function((TypeName, TypeName) record) { - ; -} From 88e55553a3cc0194f16a37469f3441f13a677078 Mon Sep 17 00:00:00 2001 From: Kallen Tu Date: Wed, 3 Jan 2024 23:04:00 +0000 Subject: [PATCH 04/10] Fix comment and use token() --- lib/src/front_end/ast_node_visitor.dart | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/lib/src/front_end/ast_node_visitor.dart b/lib/src/front_end/ast_node_visitor.dart index 38c816ba..fe74f937 100644 --- a/lib/src/front_end/ast_node_visitor.dart +++ b/lib/src/front_end/ast_node_visitor.dart @@ -1536,7 +1536,7 @@ class AstNodeVisitor extends ThrowingAstVisitor with PieceFactory { Token? rightDelimiter; if (namedFields != null) { - // If this is the first optional parameter, put the delimiter before it. + // If this is the first named field, put the delimiter before it. if (positionalFields.isNotEmpty) { builder.leftDelimiter(namedFields.leftBracket); } @@ -1549,9 +1549,7 @@ class AstNodeVisitor extends ThrowingAstVisitor with PieceFactory { builder.rightBracket(node.rightParenthesis, delimiter: rightDelimiter); return buildPiece((b) { b.add(builder.build()); - if (node.question case var question?) { - b.add(tokenPiece(question)); - } + b.token(node.question); }); } From 2602042db1338c112118cdba89e53c79016fd29b Mon Sep 17 00:00:00 2001 From: Kallen Tu Date: Wed, 3 Jan 2024 23:06:46 +0000 Subject: [PATCH 05/10] Added createRecordTypeField --- lib/src/front_end/ast_node_visitor.dart | 14 ++------------ lib/src/front_end/piece_factory.dart | 10 ++++++++++ 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/lib/src/front_end/ast_node_visitor.dart b/lib/src/front_end/ast_node_visitor.dart index fe74f937..6e547658 100644 --- a/lib/src/front_end/ast_node_visitor.dart +++ b/lib/src/front_end/ast_node_visitor.dart @@ -1556,23 +1556,13 @@ class AstNodeVisitor extends ThrowingAstVisitor with PieceFactory { @override Piece visitRecordTypeAnnotationNamedField( RecordTypeAnnotationNamedField node) { - // TODO(tall): Format metadata. - if (node.metadata.isNotEmpty) throw UnimplementedError(); - return buildPiece((b) { - b.visit(node.type); - b.token(node.name, spaceBefore: true); - }); + return createRecordTypeField(node); } @override Piece visitRecordTypeAnnotationPositionalField( RecordTypeAnnotationPositionalField node) { - // TODO(tall): Format metadata. - if (node.metadata.isNotEmpty) throw UnimplementedError(); - return buildPiece((b) { - b.visit(node.type); - b.token(node.name, spaceBefore: true); - }); + return createRecordTypeField(node); } @override diff --git a/lib/src/front_end/piece_factory.dart b/lib/src/front_end/piece_factory.dart index 687f7844..6fd39a8b 100644 --- a/lib/src/front_end/piece_factory.dart +++ b/lib/src/front_end/piece_factory.dart @@ -542,6 +542,16 @@ mixin PieceFactory { return builder.build(); } + /// Creates an [AdjacentPiece] for a given record type field. + Piece createRecordTypeField(RecordTypeAnnotationField node) { + // TODO(tall): Format metadata. + if (node.metadata.isNotEmpty) throw UnimplementedError(); + return buildPiece((b) { + b.visit(node.type); + b.token(node.name, spaceBefore: true); + }); + } + /// Creates a class, enum, extension, mixin, or mixin application class /// declaration. /// From 408dafa4fcebabde32c69207be86ce841c923124 Mon Sep 17 00:00:00 2001 From: Kallen Tu Date: Wed, 3 Jan 2024 23:40:09 +0000 Subject: [PATCH 06/10] Extracted parameter splitting to a helper. --- lib/src/front_end/piece_factory.dart | 63 ++++++++++++++++++---------- test/type/function.stmt | 3 ++ 2 files changed, 44 insertions(+), 22 deletions(-) diff --git a/lib/src/front_end/piece_factory.dart b/lib/src/front_end/piece_factory.dart index 6fd39a8b..048f9860 100644 --- a/lib/src/front_end/piece_factory.dart +++ b/lib/src/front_end/piece_factory.dart @@ -225,24 +225,14 @@ mixin PieceFactory { var builder = AdjacentBuilder(this); startFormalParameter(node, builder); builder.modifier(mutableKeyword); - builder.visit(type); - - Piece? typePiece; - if (type != null && name != null) { - typePiece = builder.build(); - } - - builder.token(fieldKeyword); - builder.token(period); - builder.token(name); - - // If we have both a type and name, allow splitting between them. - if (typePiece != null) { - var namePiece = builder.build(); - return VariablePiece(typePiece, [namePiece], hasType: true); - } - - return builder.build(); + return finishTypeAndName( + type, + name, + builder, + mutableKeyword: mutableKeyword, + fieldKeyword: fieldKeyword, + period: period, + ); } /// Creates a function, method, getter, or setter declaration. @@ -546,10 +536,11 @@ mixin PieceFactory { Piece createRecordTypeField(RecordTypeAnnotationField node) { // TODO(tall): Format metadata. if (node.metadata.isNotEmpty) throw UnimplementedError(); - return buildPiece((b) { - b.visit(node.type); - b.token(node.name, spaceBefore: true); - }); + return finishTypeAndName( + node.type, + node.name, + AdjacentBuilder(this), + ); } /// Creates a class, enum, extension, mixin, or mixin application class @@ -663,6 +654,34 @@ mixin PieceFactory { style: const ListStyle(commas: Commas.nonTrailing, splitCost: 3)); } + /// Creates a [VariablePiece] that allows splitting between a type and a name, + /// if they both exist. + /// + /// Otherwise, finishes building the existing [AdjacentPiece] with the + /// [builder]. + Piece finishTypeAndName( + TypeAnnotation? type, Token? name, AdjacentBuilder builder, + {Token? mutableKeyword, Token? fieldKeyword, Token? period}) { + builder.visit(type); + + Piece? typePiece; + if (type != null && name != null) { + typePiece = builder.build(); + } + + builder.token(fieldKeyword); + builder.token(period); + builder.token(name); + + // If we have both a type and name, allow splitting between them. + if (typePiece != null) { + var namePiece = builder.build(); + return VariablePiece(typePiece, [namePiece], hasType: true); + } + + return builder.build(); + } + /// Writes the parts of a formal parameter shared by all formal parameter /// types: metadata, `covariant`, etc. void startFormalParameter( diff --git a/test/type/function.stmt b/test/type/function.stmt index 11261404..42a9b147 100644 --- a/test/type/function.stmt +++ b/test/type/function.stmt @@ -268,6 +268,7 @@ function((TypeName,TypeName,) record,) {;} function((TypeName, TypeName) record) { ; } +<<<<<<< HEAD >>> Unsplit generic method instantiation. @@ -332,3 +333,5 @@ veryLongFunction( >, argument, ); +======= +>>>>>>> 3976436 (Extracted parameter splitting to a helper.) From f4c32a594f21f7e557e765446ccd950c7fd7d9cf Mon Sep 17 00:00:00 2001 From: Kallen Tu Date: Wed, 3 Jan 2024 23:44:54 +0000 Subject: [PATCH 07/10] Add test for splitting between type and name. --- test/type/record.stmt | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/test/type/record.stmt b/test/type/record.stmt index 527740cf..efdbf0be 100644 --- a/test/type/record.stmt +++ b/test/type/record.stmt @@ -31,6 +31,13 @@ ( { int value , String label } ) x; <<< ({int value, String label}) x; +>>> Split between the type and the name. +( VeryVeryLongType_____ veryLongName___________________ , ) x; +<<< +( + VeryVeryLongType_____ + veryLongName___________________, +) x; >>> Split named positional fields. ( int longValue , String veryVeryLongLabel , ) x; <<< From 220b57c60951a879160551c895b38828e8321360 Mon Sep 17 00:00:00 2001 From: Kallen Tu Date: Wed, 3 Jan 2024 23:46:14 +0000 Subject: [PATCH 08/10] Revert class.unit --- test/declaration/class.unit | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/declaration/class.unit b/test/declaration/class.unit index 504c7e9e..a9a75c67 100644 --- a/test/declaration/class.unit +++ b/test/declaration/class.unit @@ -73,4 +73,4 @@ abstract base mixin class C13 {} class SomeClass native "Zapp" { } <<< -class SomeClass native "Zapp" {} +class SomeClass native "Zapp" {} \ No newline at end of file From 11fa6fde06cea88db2cb91502fd47faf98b1fbd9 Mon Sep 17 00:00:00 2001 From: Kallen Tu Date: Mon, 8 Jan 2024 22:34:04 +0000 Subject: [PATCH 09/10] Updated comment. --- lib/src/front_end/ast_node_visitor.dart | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/src/front_end/ast_node_visitor.dart b/lib/src/front_end/ast_node_visitor.dart index 6e547658..4aed128d 100644 --- a/lib/src/front_end/ast_node_visitor.dart +++ b/lib/src/front_end/ast_node_visitor.dart @@ -1536,7 +1536,8 @@ class AstNodeVisitor extends ThrowingAstVisitor with PieceFactory { Token? rightDelimiter; if (namedFields != null) { - // If this is the first named field, put the delimiter before it. + // If we have both positional fields and named fields, then we need to add + // the left bracket delimiter before the first named field. if (positionalFields.isNotEmpty) { builder.leftDelimiter(namedFields.leftBracket); } From e07f0046244e254ea6b9db11e38a8ef03d069983 Mon Sep 17 00:00:00 2001 From: Kallen Tu Date: Mon, 8 Jan 2024 22:37:51 +0000 Subject: [PATCH 10/10] Rebase merge changes. --- test/type/function.stmt | 5 ----- 1 file changed, 5 deletions(-) diff --git a/test/type/function.stmt b/test/type/function.stmt index 42a9b147..75ad4c6d 100644 --- a/test/type/function.stmt +++ b/test/type/function.stmt @@ -268,9 +268,6 @@ function((TypeName,TypeName,) record,) {;} function((TypeName, TypeName) record) { ; } -<<<<<<< HEAD - - >>> Unsplit generic method instantiation. void main() => id < int > ; <<< @@ -333,5 +330,3 @@ veryLongFunction( >, argument, ); -======= ->>>>>>> 3976436 (Extracted parameter splitting to a helper.)