Merge pull request #1031 from dart-lang/837-block-comment-splits
Don't unnecessarily split argument lists with block comments.
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..a74e222 100644
--- a/lib/src/source_visitor.dart
+++ b/lib/src/source_visitor.dart
@@ -3770,9 +3770,18 @@
if (comment == token.precedingComments) linesBefore = 2;
}
- var sourceComment = SourceComment(text, linesBefore,
- isLineComment: comment.type == TokenType.SINGLE_LINE_COMMENT,
- flushLeft: flushLeft);
+ 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
// in the comment.
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