Skip to content

Commit bee1256

Browse files
committed
Don't split return type if there is a comment after function metadata.
This situation rarely occurs in practice, since users typically put the comment before the metadata too (which is why we never noticed this bug until now). But if it does occur, it's definitely wrong to force the return type to split unnecessarily.
1 parent 7d4acb8 commit bee1256

File tree

4 files changed

+49
-2
lines changed

4 files changed

+49
-2
lines changed

lib/src/front_end/piece_factory.dart

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -616,6 +616,19 @@ mixin PieceFactory {
616616
return;
617617
}
618618

619+
// Hoist any comments before the function so they don't force a split
620+
// between the return type and function. In most cases, this doesn't matter
621+
// because the [SequenceBuilder] for the surrounding code will separate out
622+
// the leading comment. But if there is a metadata annotation followed by
623+
// a comment, then the function, then the comment doesn't get captured by
624+
// the [SequenceBuilder], as in:
625+
//
626+
// @meta
627+
// // Weird place for comment.
628+
// int f() {}
629+
var leadingComments =
630+
pieces.takeCommentsBefore(returnType.firstNonCommentToken);
631+
619632
var returnTypePiece = pieces.build(() {
620633
for (var keyword in modifiers) {
621634
pieces.modifier(keyword);
@@ -628,7 +641,8 @@ mixin PieceFactory {
628641
writeFunction();
629642
});
630643

631-
pieces.add(VariablePiece(returnTypePiece, [signature], hasType: true));
644+
pieces.add(prependLeadingComments(leadingComments,
645+
VariablePiece(returnTypePiece, [signature], hasType: true)));
632646
}
633647

634648
/// If [parameter] has a [defaultValue] then writes a piece for the parameter

test/tall/function/comment.unit

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,4 +21,12 @@ main() {
2121
// nested
2222
}
2323
}
24+
}
25+
>>> Don't split return type if comment before.
26+
// Comment.
27+
int f() {;}
28+
<<<
29+
// Comment.
30+
int f() {
31+
;
2432
}

test/tall/function/metadata.unit

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,4 +95,14 @@ f([
9595
@metadata
9696
@another(argument, argument)
9797
callback() = constantFunction,
98-
]) {}
98+
]) {}
99+
>>> Don't split return type if comment after metadata.
100+
@meta
101+
// Comment.
102+
int f() {;}
103+
<<<
104+
@meta
105+
// Comment.
106+
int f() {
107+
;
108+
}

test/tall/regression/other/dart.unit

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,4 +39,19 @@ main() {
3939
.transform(utf8.decoder)
4040
.transform(const LineSplitter())
4141
.asBroadcastStream();
42+
}
43+
>>> Don't split return type if comment after metadata annotation.
44+
class Benchmark {
45+
@override
46+
// A rate of one run per 2s, with a millisecond of noise. Some variation is
47+
// needed for Golem's noise-based filtering and regression detection.
48+
double
49+
measure() => (2000 + Random().nextDouble() - 0.5) * 1000;
50+
}
51+
<<<
52+
class Benchmark {
53+
@override
54+
// A rate of one run per 2s, with a millisecond of noise. Some variation is
55+
// needed for Golem's noise-based filtering and regression detection.
56+
double measure() => (2000 + Random().nextDouble() - 0.5) * 1000;
4257
}

0 commit comments

Comments
 (0)