Add style fix to remove "new" keyword. (flutter#695)

Author: munificent

CHANGELOG.md

@@ -1,7 +1,8 @@
 # 1.1.0-dev
 
 * Add support for "style fixes", opt-in non-whitespace changes.
-* Add fix to convert ":" to "=" as the named parameter default value separator.
+* Add fix to convert `:` to `=` as the named parameter default value separator.
+* Add fix to remove `new` keywords.
 
 # 1.0.14
 

lib/src/chunk_builder.dart

@@ -200,6 +200,24 @@ class ChunkBuilder {
       }
     }
 
+    // Edge case: if the previous output was also from a call to
+    // [writeComments()] which ended with a line comment, force a newline.
+    // Normally, comments are strictly interleaved with tokens and you never
+    // get two sequences of comments in a row. However, when applying a fix
+    // that removes a token (like `new`), it's possible to get two sets of
+    // comments in a row, as in:
+    //
+    //     // a
+    //     new // b
+    //     Foo();
+    //
+    // When that happens, we need to make sure the preserve the split at the
+    // end of the first sequence of comments if there is one.
+    if (_pendingWhitespace == null) {
+      comments.first.linesBefore = 1;
+      _pendingWhitespace = Whitespace.none;
+    }
+
     // 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
@@ -215,11 +233,10 @@ class ChunkBuilder {
     //     /* a */ /* b */
     //     import 'a.dart';
     if (linesBeforeToken == 0 &&
-        comments.every((comment) => comment.isInline)) {
-      if (_pendingWhitespace.minimumLines > 0) {
-        comments.first.linesBefore = _pendingWhitespace.minimumLines;
-        linesBeforeToken = 1;
-      }
+        comments.every((comment) => comment.isInline) &&
+        _pendingWhitespace.minimumLines > 0) {
+      comments.first.linesBefore = _pendingWhitespace.minimumLines;
+      linesBeforeToken = 1;
     }
 
     // Write each comment and the whitespace between them.

lib/src/source_visitor.dart

@@ -1443,7 +1443,23 @@ class SourceVisitor extends ThrowingAstVisitor {
 
   visitInstanceCreationExpression(InstanceCreationExpression node) {
     builder.startSpan();
-    token(node.keyword, after: space);
+
+    var includeKeyword = true;
+
+    if (node.keyword != null) {
+      if (node.keyword.keyword == Keyword.NEW &&
+          _formatter.fixes.contains(StyleFix.optionalNew)) {
+        includeKeyword = false;
+      }
+    }
+
+    if (includeKeyword) {
+      token(node.keyword, after: space);
+    } else {
+      // Don't lose comments before the discarded keyword, if any.
+      writePrecedingCommentsAndNewlines(node.keyword);
+    }
+
     builder.startSpan(Cost.constructorName);
 
     // Start the expression nesting for the argument list here, in case this

lib/src/style_fix.dart

@@ -9,7 +9,10 @@ class StyleFix {
       "named-default-separator",
       'Use "=" as the separator before named parameter default values.');
 
-  static const all = const [namedDefaultSeparator];
+  static const optionalNew =
+      const StyleFix._("optional-new", 'Remove "new" keyword.');
+
+  static const all = const [namedDefaultSeparator, optionalNew];
 
   final String name;
   final String description;

pubspec.lock

@@ -366,4 +366,4 @@ packages:
     source: hosted
     version: "2.1.13"
 sdks:
-  dart: ">=2.0.0-dev.54.0 <=2.0.0-edge.ba342a94ecd08c183b6ea0b4dab33e47bb8dadd5"
+  dart: ">=2.0.0-dev.54.0 <=2.0.0-edge.9ff283218499ccc46fe7bb59f6aa4a40f31331c0"

test/command_line_test.dart

@@ -234,22 +234,39 @@ void main() {
   });
 
   group("fix", () {
-    // TODO(rnystrom): This will get more useful when other fixes are supported.
     test("--fix applies all fixes", () async {
       var process = await runFormatter(["--fix"]);
-      process.stdin.writeln("foo({a:1}) {}");
+      process.stdin.writeln("foo({a:1}) {");
+      process.stdin.writeln("  new Bar();}");
       await process.stdin.close();
 
-      expect(await process.stdout.next, "foo({a = 1}) {}");
+      expect(await process.stdout.next, "foo({a = 1}) {");
+      expect(await process.stdout.next, "  Bar();");
+      expect(await process.stdout.next, "}");
       await process.shouldExit(0);
     });
 
     test("--fix-named-default-separator", () async {
       var process = await runFormatter(["--fix-named-default-separator"]);
-      process.stdin.writeln("foo({a:1}) {}");
+      process.stdin.writeln("foo({a:1}) {");
+      process.stdin.writeln("  new Bar();}");
       await process.stdin.close();
 
-      expect(await process.stdout.next, "foo({a = 1}) {}");
+      expect(await process.stdout.next, "foo({a = 1}) {");
+      expect(await process.stdout.next, "  new Bar();");
+      expect(await process.stdout.next, "}");
+      await process.shouldExit(0);
+    });
+
+    test("--fix-optional-new", () async {
+      var process = await runFormatter(["--fix-optional-new"]);
+      process.stdin.writeln("foo({a:1}) {");
+      process.stdin.writeln("  new Bar();}");
+      await process.stdin.close();
+
+      expect(await process.stdout.next, "foo({a: 1}) {");
+      expect(await process.stdout.next, "  Bar();");
+      expect(await process.stdout.next, "}");
       await process.shouldExit(0);
     });
 

test/fix_test.dart

@@ -14,4 +14,5 @@ import 'utils.dart';
 void main() {
   testFile(
       "fixes/named_default_separator.unit", [StyleFix.namedDefaultSeparator]);
+  testFile("fixes/optional_new.stmt", [StyleFix.optionalNew]);
 }

test/fixes/optional_new.stmt

@@ -0,0 +1,67 @@
+40 columns                              |
+>>>
+{
+  new A(new A.named());
+  new A<int>(new A<int>.named());
+  // Don't touch const.
+  const A();
+  const A.named();
+}
+<<<
+{
+  A(A.named());
+  A<int>(A<int>.named());
+  // Don't touch const.
+  const A();
+  const A.named();
+}
+>>> preserves block comments before
+/* a */    /* b */ new A();
+<<<
+/* a */ /* b */ A();
+>>> preserves block comments after
+new /* a */    /* b */A();
+<<<
+/* a */ /* b */ A();
+>>> preserves line comments before
+// a
+// b
+new A();
+<<<
+// a
+// b
+A();
+>>> preserves line comments after
+new // a
+// b
+A();
+<<<
+// a
+// b
+A();
+>>> merge surrounding comments
+{
+  /* a */ new /* b */ A();
+  /* a */ new // b
+  A();
+  // a
+  new /* b */ A();
+  // a
+  new // b
+  A();
+}
+<<<
+{
+  /* a */ /* b */ A();
+  /* a */ // b
+  A();
+  // a
+  /* b */ A();
+  // a
+  // b
+  A();
+}
+>>> handle already-removed keyword
+A<int>(A<int>.named());
+<<<
+A<int>(A<int>.named());