Don't unnecessarily split argument lists with block comments.
A very subtle bug in the comment handling would cause a /* */ inline
comment to force the surrounding code to full split when not needed in
some cases.
This fixes that issue. It also slightly tweaks the formatting of code
containing inline block comments in some rare cases. In practice, the
only code I see affected by that is formatter tests. In the real world,
all affected code is related to the bug itself and is fixed by this
change.
Fix #837.
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 51ce9e8..97d8689 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,3 +1,7 @@
+# 2.0.2-dev
+
+* Don't unnecessarily split argument lists with `/* */` comments (#837).
+
# 2.0.1
* Support triple-shift `>>>` and `>>>=` operators (#992).
diff --git a/lib/src/chunk.dart b/lib/src/chunk.dart
index 90d5636..7786d21 100644
--- a/lib/src/chunk.dart
+++ b/lib/src/chunk.dart
@@ -384,6 +384,20 @@
String toString() => '$id\$$cost';
}
+enum CommentType {
+ /// A `///` or `/**` doc comment.
+ doc,
+
+ /// A non-doc line comment.
+ line,
+
+ /// A `/* ... */` comment that should be on its own line.
+ block,
+
+ /// A `/* ... */` comment that can share a line with other code.
+ inlineBlock,
+}
+
/// A comment in the source, with a bit of information about the surrounding
/// whitespace.
class SourceComment extends Selection {
@@ -391,15 +405,15 @@
@override
final String text;
+ /// What kind of comment this is.
+ final CommentType type;
+
/// The number of newlines between the comment or token preceding this comment
/// and the beginning of this one.
///
/// Will be zero if the comment is a trailing one.
int linesBefore;
- /// Whether this comment is a line comment.
- final bool isLineComment;
-
/// Whether this comment starts at column one in the source.
///
/// Comments that start at the start of the line will not be indented in the
@@ -407,9 +421,6 @@
/// re-indented.
final bool flushLeft;
- /// Whether this comment is an inline block comment.
- bool get isInline => linesBefore == 0 && !isLineComment;
-
- SourceComment(this.text, this.linesBefore,
- {required this.isLineComment, required this.flushLeft});
+ SourceComment(this.text, this.type, this.linesBefore,
+ {required this.flushLeft});
}
diff --git a/lib/src/chunk_builder.dart b/lib/src/chunk_builder.dart
index c2ab67b..71d955b 100644
--- a/lib/src/chunk_builder.dart
+++ b/lib/src/chunk_builder.dart
@@ -253,8 +253,7 @@
// Edge case: if the comments are completely inline (i.e. just a series of
// block comments with no newlines before, after, or between them), then
// they will eat any pending newlines. Make sure that doesn't happen by
- // putting the pending whitespace before the first comment and moving them
- // to their own line. Turns this:
+ // putting the pending whitespace before the first comment. Turns this:
//
// library foo; /* a */ /* b */ import 'a.dart';
//
@@ -262,13 +261,11 @@
//
// library foo;
//
- // /* a */ /* b */
- // import 'a.dart';
+ // /* a */ /* b */ import 'a.dart';
if (linesBeforeToken == 0 &&
- comments.every((comment) => comment.isInline) &&
- _pendingWhitespace.minimumLines > 0) {
+ _pendingWhitespace.minimumLines > comments.first.linesBefore &&
+ comments.every((comment) => comment.type == CommentType.inlineBlock)) {
comments.first.linesBefore = _pendingWhitespace.minimumLines;
- linesBeforeToken = 1;
}
// Write each comment and the whitespace between them.
@@ -284,10 +281,11 @@
}
_emitPendingWhitespace();
- if (comment.linesBefore == 0) {
+ // See if the comment should follow text on the current line.
+ if (comment.linesBefore == 0 || comment.type == CommentType.inlineBlock) {
// If we're sitting on a split, move the comment before it to adhere it
// to the preceding text.
- if (_shouldMoveCommentBeforeSplit(comment.text)) {
+ if (_shouldMoveCommentBeforeSplit(comment)) {
_chunks.last.allowText();
}
@@ -295,7 +293,7 @@
// space before it or not.
if (_needsSpaceBeforeComment(comment)) _writeText(' ');
} else {
- // The comment starts a line, so make sure it stays on its own line.
+ // The comment is not inline, so start a new line.
_writeHardSplit(
flushLeft: comment.flushLeft,
isDouble: comment.linesBefore > 1,
@@ -743,17 +741,23 @@
/// Returns `true` if the last chunk is a split that should be moved after the
/// comment that is about to be written.
- bool _shouldMoveCommentBeforeSplit(String comment) {
+ bool _shouldMoveCommentBeforeSplit(SourceComment comment) {
// Not if there is nothing before it.
if (_chunks.isEmpty) return false;
+ // Don't move a comment to a preceding line.
+ if (comment.linesBefore != 0) return false;
+
// Multi-line comments are always pushed to the next line.
- if (comment.contains('\n')) return false;
+ if (comment.type == CommentType.doc) return false;
+ if (comment.type == CommentType.block) return false;
var text = _chunks.last.text;
// A block comment following a comma probably refers to the following item.
- if (text.endsWith(',') && comment.startsWith('/*')) return false;
+ if (text.endsWith(',') && comment.type == CommentType.inlineBlock) {
+ return false;
+ }
// If the text before the split is an open grouping character, it looks
// better to keep it with the elements than with the bracket itself.
@@ -788,13 +792,11 @@
// Not at the start of a line.
if (!_chunks.last.canAddText) return false;
- var text = _chunks.last.text;
- if (text.endsWith('\n')) return false;
-
// Always put a space before line comments.
- if (comment.isLineComment) return true;
+ if (comment.type == CommentType.line) return true;
// Magic generic method comments like "Foo/*<T>*/" don't get spaces.
+ var text = _chunks.last.text;
if (_isGenericMethodComment(comment) &&
_trailingIdentifierChar.hasMatch(text)) {
return false;
diff --git a/lib/src/source_visitor.dart b/lib/src/source_visitor.dart
index 7ad10c9..f5b4c61 100644
--- a/lib/src/source_visitor.dart
+++ b/lib/src/source_visitor.dart
@@ -3770,8 +3770,17 @@
if (comment == token.precedingComments) linesBefore = 2;
}
- var sourceComment = SourceComment(text, linesBefore,
- isLineComment: comment.type == TokenType.SINGLE_LINE_COMMENT,
+ var type = CommentType.block;
+ if (text.startsWith('///') && !text.startsWith('////') ||
+ text.startsWith('/**')) {
+ type = CommentType.doc;
+ } else if (comment.type == TokenType.SINGLE_LINE_COMMENT) {
+ type = CommentType.line;
+ } else if (commentLine == previousLine || commentLine == tokenLine) {
+ type = CommentType.inlineBlock;
+ }
+
+ var sourceComment = SourceComment(text, type, linesBefore,
flushLeft: flushLeft);
// If this comment contains either of the selection endpoints, mark them
diff --git a/pubspec.yaml b/pubspec.yaml
index d2e2f82..7bc89ec 100644
--- a/pubspec.yaml
+++ b/pubspec.yaml
@@ -1,6 +1,6 @@
name: dart_style
# Note: See tool/grind.dart for how to bump the version.
-version: 2.0.1
+version: 2.0.2-dev
description: >-
Opinionated, automatic Dart source code formatter.
Provides an API and a CLI tool.
diff --git a/test/comments/classes.unit b/test/comments/classes.unit
index 0f6c4a5..fdf3836 100644
--- a/test/comments/classes.unit
+++ b/test/comments/classes.unit
@@ -33,9 +33,7 @@
class A {
/* comment */}
<<<
-class A {
- /* comment */
-}
+class A {/* comment */}
>>> inline block comment
class A { /* comment */ }
<<<
diff --git a/test/comments/extensions.unit b/test/comments/extensions.unit
index 67123f3..2b34b9a 100644
--- a/test/comments/extensions.unit
+++ b/test/comments/extensions.unit
@@ -33,9 +33,7 @@
extension A on B {
/* comment */}
<<<
-extension A on B {
- /* comment */
-}
+extension A on B {/* comment */}
>>> inline block comment
extension A on B { /* comment */ }
<<<
diff --git a/test/comments/functions.unit b/test/comments/functions.unit
index 00c235b..45ac999 100644
--- a/test/comments/functions.unit
+++ b/test/comments/functions.unit
@@ -143,4 +143,36 @@
parameter,
] /* c */) {
;
+}
+>>>
+function(
+ /* comment */ int a, int b, int c,
+ [direction]) {
+ ;
+}
+<<<
+function(
+ /* comment */ int a, int b, int c,
+ [direction]) {
+ ;
+}
+>>>
+function(
+ /* comment */ int a, int b, int c) {
+ ;
+}
+<<<
+function(
+ /* comment */ int a, int b, int c) {
+ ;
+}
+>>>
+function(
+ /* comment */ int a, int b, int c, int d) {
+ ;
+}
+<<<
+function(/* comment */ int a, int b,
+ int c, int d) {
+ ;
}
\ No newline at end of file
diff --git a/test/comments/statements.stmt b/test/comments/statements.stmt
index d56ad60..6cc2237 100644
--- a/test/comments/statements.stmt
+++ b/test/comments/statements.stmt
@@ -54,4 +54,33 @@
<<<
while (true) {
// comment
+}
+>>>
+main() {
+ /* comment */ statement;
+}
+<<<
+main() {
+ /* comment */ statement;
+}
+>>>
+main() {
+ code;
+
+ /* comment */ statement;
+}
+<<<
+main() {
+ code;
+
+ /* comment */ statement;
+}
+>>>
+main() {
+ while (b)
+ /*unreachable*/ {}
+}
+<<<
+main() {
+ while (b) /*unreachable*/ {}
}
\ No newline at end of file
diff --git a/test/comments/top_level.unit b/test/comments/top_level.unit
index d1fe80f..74a269a 100644
--- a/test/comments/top_level.unit
+++ b/test/comments/top_level.unit
@@ -190,14 +190,12 @@
<<<
library a;
-/* comment */
-import 'b.dart';
+/* comment */ import 'b.dart';
>>> inline block comment between directives
import 'a.dart'; /* comment */ import 'b.dart';
<<<
import 'a.dart';
-/* comment */
-import 'b.dart';
+/* comment */ import 'b.dart';
>>> block comment between directives
import 'a.dart'; /* comment */
import 'b.dart';
diff --git a/test/regression/0100/0178.unit b/test/regression/0100/0178.unit
index 863284e..ca2a2a2 100644
--- a/test/regression/0100/0178.unit
+++ b/test/regression/0100/0178.unit
@@ -6,8 +6,7 @@
}
<<<
import 'dart:async';
-/* soup */
-import 'dart:io';
+/* soup */ import 'dart:io';
void main() {
print('hi');
diff --git a/test/regression/0800/0837.unit b/test/regression/0800/0837.unit
new file mode 100644
index 0000000..16fad4e
--- /dev/null
+++ b/test/regression/0800/0837.unit
@@ -0,0 +1,45 @@
+>>>
+String var80 = () => ('Ss\u2665\u26659d').replanullceRange( { }, var3,/* aaaaaaaaaaaaaaaaaaaaaaaaaaexportaaa */-9223372036854775807, -64, arexternalg3: '\u26650P'..[(var2 << var5..ff8[var2aoeu])] = { });
+<<<
+String var80 = () => ('Ss\u2665\u26659d').replanullceRange({}, var3,
+ /* aaaaaaaaaaaaaaaaaaaaaaaaaaexportaaa */ -9223372036854775807, -64,
+ arexternalg3: '\u26650P'
+ ..[(var2 << var5
+ ..ff8[var2aoeu])] = {});
+>>>
+String var80 = () => ('Ss\u2665\u26659d').replanullceRange({}, var3,
+ /* aaaaaaaaaaaaaaaaaaaaaaaaaaexportaaa */ -9223372036854775807, -64,
+ arexternalg3: '\u26650P'
+ ..[(var2 << var5
+ ..ff8[var2aoeu])] = {});
+<<<
+String var80 = () => ('Ss\u2665\u26659d').replanullceRange({}, var3,
+ /* aaaaaaaaaaaaaaaaaaaaaaaaaaexportaaa */ -9223372036854775807, -64,
+ arexternalg3: '\u26650P'
+ ..[(var2 << var5
+ ..ff8[var2aoeu])] = {});
+>>>
+doString var80 = ('Ss\u2665\u26659d').replaceRange( { }, var3,/* aaaaaaaaaaaaaaaaaaaaaaaaaaaaa static*/-9223372036854775807, -64, arg3: '\u26650P');
+<<<
+doString var80 = ('Ss\u2665\u26659d').replaceRange({}, var3,
+ /* aaaaaaaaaaaaaaaaaaaaaaaaaaaaa static*/ -9223372036854775807, -64,
+ arg3: '\u26650P');
+>>>
+doString var80 = ('Ss\u2665\u26659d').replaceRange({}, var3,
+ /* aaaaaaaaaaaaaaaaaaaaaaaaaaaaa static*/ -9223372036854775807, -64,
+ arg3: '\u26650P');
+<<<
+doString var80 = ('Ss\u2665\u26659d').replaceRange({}, var3,
+ /* aaaaaaaaaaaaaaaaaaaaaaaaaaaaa static*/ -9223372036854775807, -64,
+ arg3: '\u26650P');
+>>>
+doubnullle var70 = foooooooooooooooooooooooooooooo( /* aaaaaaaaaaaaaaaaaaaaaaaaaaaa */var8, var8);
+<<<
+doubnullle var70 = foooooooooooooooooooooooooooooo(
+ /* aaaaaaaaaaaaaaaaaaaaaaaaaaaa */ var8, var8);
+>>>
+doubnullle var70 = foooooooooooooooooooooooooooooo(
+ /* aaaaaaaaaaaaaaaaaaaaaaaaaaaa */ var8, var8);
+<<<
+doubnullle var70 = foooooooooooooooooooooooooooooo(
+ /* aaaaaaaaaaaaaaaaaaaaaaaaaaaa */ var8, var8);
\ No newline at end of file
diff --git a/test/regression/other/block_comment.unit b/test/regression/other/block_comment.unit
new file mode 100644
index 0000000..9c18327
--- /dev/null
+++ b/test/regression/other/block_comment.unit
@@ -0,0 +1,35 @@
+>>>
+setSelectionRange(
+ /* TextInputElement | TextAreaElement */ Element input, int start, int end,
+ [direction]) {
+ ;
+}
+<<<
+setSelectionRange(
+ /* TextInputElement | TextAreaElement */ Element input, int start, int end,
+ [direction]) {
+ ;
+}
+>>>
+setSelectionRange(/* TextInputElement | TextAreaElement */ Element input, int start, int end,
+ [direction]) {
+ ;
+}
+<<<
+setSelectionRange(
+ /* TextInputElement | TextAreaElement */ Element input, int start, int end,
+ [direction]) {
+ ;
+}
+>>>
+Future<void> signOutGoogle() async {}
+
+/*Future <FirebaseUser> signUp(email, password) async {
+ ;
+}*/
+<<<
+Future<void> signOutGoogle() async {}
+
+/*Future <FirebaseUser> signUp(email, password) async {
+ ;
+}*/
\ No newline at end of file
