Skip to content

Commit

Permalink
Don't split return type if there is a comment after function metadata.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
munificent committed Oct 14, 2024
1 parent 7d4acb8 commit bee1256
Show file tree
Hide file tree
Showing 4 changed files with 49 additions and 2 deletions.
16 changes: 15 additions & 1 deletion lib/src/front_end/piece_factory.dart
Original file line number Diff line number Diff line change
Expand Up @@ -616,6 +616,19 @@ mixin PieceFactory {
return;
}

// Hoist any comments before the function so they don't force a split
// between the return type and function. In most cases, this doesn't matter
// because the [SequenceBuilder] for the surrounding code will separate out
// the leading comment. But if there is a metadata annotation followed by
// a comment, then the function, then the comment doesn't get captured by
// the [SequenceBuilder], as in:
//
// @meta
// // Weird place for comment.
// int f() {}
var leadingComments =
pieces.takeCommentsBefore(returnType.firstNonCommentToken);

var returnTypePiece = pieces.build(() {
for (var keyword in modifiers) {
pieces.modifier(keyword);
Expand All @@ -628,7 +641,8 @@ mixin PieceFactory {
writeFunction();
});

pieces.add(VariablePiece(returnTypePiece, [signature], hasType: true));
pieces.add(prependLeadingComments(leadingComments,
VariablePiece(returnTypePiece, [signature], hasType: true)));
}

/// If [parameter] has a [defaultValue] then writes a piece for the parameter
Expand Down
8 changes: 8 additions & 0 deletions test/tall/function/comment.unit
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,12 @@ main() {
// nested
}
}
}
>>> Don't split return type if comment before.
// Comment.
int f() {;}
<<<
// Comment.
int f() {
;
}
12 changes: 11 additions & 1 deletion test/tall/function/metadata.unit
Original file line number Diff line number Diff line change
Expand Up @@ -95,4 +95,14 @@ f([
@metadata
@another(argument, argument)
callback() = constantFunction,
]) {}
]) {}
>>> Don't split return type if comment after metadata.
@meta
// Comment.
int f() {;}
<<<
@meta
// Comment.
int f() {
;
}
15 changes: 15 additions & 0 deletions test/tall/regression/other/dart.unit
Original file line number Diff line number Diff line change
Expand Up @@ -39,4 +39,19 @@ main() {
.transform(utf8.decoder)
.transform(const LineSplitter())
.asBroadcastStream();
}
>>> Don't split return type if comment after metadata annotation.
class Benchmark {
@override
// A rate of one run per 2s, with a millisecond of noise. Some variation is
// needed for Golem's noise-based filtering and regression detection.
double
measure() => (2000 + Random().nextDouble() - 0.5) * 1000;
}
<<<
class Benchmark {
@override
// A rate of one run per 2s, with a millisecond of noise. Some variation is
// needed for Golem's noise-based filtering and regression detection.
double measure() => (2000 + Random().nextDouble() - 0.5) * 1000;
}

0 comments on commit bee1256

Please sign in to comment.