diff --git a/CHANGELOG.md b/CHANGELOG.md index 51ce9e86..97d86899 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 90d56364..7786d217 100644 --- a/lib/src/chunk.dart +++ b/lib/src/chunk.dart @@ -384,6 +384,20 @@ class Span extends FastHash { 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 @@ class SourceComment extends Selection { @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 @@ class SourceComment extends Selection { /// 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 c2ab67b9..71d955bc 100644 --- a/lib/src/chunk_builder.dart +++ b/lib/src/chunk_builder.dart @@ -253,8 +253,7 @@ class ChunkBuilder { // 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 @@ class ChunkBuilder { // // 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 @@ class ChunkBuilder { } _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 @@ class ChunkBuilder { // 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 @@ class ChunkBuilder { /// 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 @@ class ChunkBuilder { // 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/**/" 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 7ad10c9c..a74e2220 100644 --- a/lib/src/source_visitor.dart +++ b/lib/src/source_visitor.dart @@ -3770,9 +3770,18 @@ class SourceVisitor extends ThrowingAstVisitor { 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 d2e2f825..7bc89ecb 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 0f6c4a5f..fdf38360 100644 --- a/test/comments/classes.unit +++ b/test/comments/classes.unit @@ -33,9 +33,7 @@ class A { 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 67123f31..2b34b9ab 100644 --- a/test/comments/extensions.unit +++ b/test/comments/extensions.unit @@ -33,9 +33,7 @@ extension A on B { 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 00c235b2..45ac999a 100644 --- a/test/comments/functions.unit +++ b/test/comments/functions.unit @@ -143,4 +143,36 @@ function([ 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 d56ad602..6cc22372 100644 --- a/test/comments/statements.stmt +++ b/test/comments/statements.stmt @@ -54,4 +54,33 @@ while (true) { <<< 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 d1fe80f6..74a269a4 100644 --- a/test/comments/top_level.unit +++ b/test/comments/top_level.unit @@ -190,14 +190,12 @@ library a; /* comment */ import 'b.dart'; <<< 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 863284e4..ca2a2a22 100644 --- a/test/regression/0100/0178.unit +++ b/test/regression/0100/0178.unit @@ -6,8 +6,7 @@ void main() { } <<< 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 00000000..16fad4e8 --- /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 00000000..9c183272 --- /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 signOutGoogle() async {} + +/*Future signUp(email, password) async { + ; +}*/ +<<< +Future signOutGoogle() async {} + +/*Future signUp(email, password) async { + ; +}*/ \ No newline at end of file