Remove semicolons from where they don't belong. (#33)

* Remove semicolons from where they don't belong.

* Update changelog

* Cleanup Changelog

Author: alorenzen

CHANGELOG.md

@@ -1,3 +1,21 @@
+## 1.0.0-alpha+5
+
+- MethodBuilder with no statements will create an empty block instead of a
+semicolon.
+
+```dart
+// main() {}
+method('main')
+```
+
+- Fix lambdas and closures to not include a trailing semicolon when used as an
+expression.
+
+```dart
+ // () => false
+ new MethodBuilder.closure(returns: literal(false));
+```
+
 ## 1.0.0-alpha+4
 
 - Add support for latest `pkg/analyzer`.

lib/src/builders/method.dart

@@ -326,14 +326,18 @@ class _LambdaMethodBuilder extends Object
 
   @override
   Expression buildExpression([Scope scope]) {
+    return _buildExpression(scope, isStatement: false);
+  }
+
+  FunctionExpression _buildExpression(Scope scope, {bool isStatement}) {
     return new FunctionExpression(
       null,
       _property != Keyword.GET ? buildParameterList(scope) : null,
       new ExpressionFunctionBody(
         null,
         null,
         _expression.buildExpression(scope),
-        $semicolon,
+        isStatement ? $semicolon : null,
       ),
     );
   }
@@ -347,7 +351,7 @@ class _LambdaMethodBuilder extends Object
       _returnType?.buildType(scope),
       _property != null ? new KeywordToken(_property, 0) : null,
       stringIdentifier(_name),
-      buildExpression(scope),
+      _buildExpression(scope, isStatement: true),
     );
   }
 
@@ -407,9 +411,7 @@ class _MethodBuilderImpl extends Object
     return new FunctionExpression(
       null,
       _property != Keyword.GET ? buildParameterList(scope) : null,
-      !hasStatements
-          ? new EmptyFunctionBody($semicolon)
-          : new BlockFunctionBody(
+      new BlockFunctionBody(
               null,
               null,
               buildBlock(scope),
@@ -443,15 +445,16 @@ class _MethodBuilderImpl extends Object
       identifier(scope, _name),
       null,
       _property != Keyword.GET ? buildParameterList(scope) : null,
-      !hasStatements
-          ? new EmptyFunctionBody($semicolon)
-          : new BlockFunctionBody(
-              null,
-              null,
-              buildBlock(scope),
-            ),
+      new BlockFunctionBody(
+        null,
+        null,
+        buildBlock(scope),
+      ),
     );
   }
+
+  @override
+  CompilationUnitMember buildTopLevelAst([Scope scope]) => buildFunction(scope);
 }
 
 class _NamedParameterWrapper

pubspec.yaml

@@ -1,5 +1,5 @@
 name: code_builder
-version: 1.0.0-alpha+4
+version: 1.0.0-alpha+5
 description: A fluent API for generating Dart code
 author: Dart Team <[email protected]>
 homepage: https://github.com/dart-lang/code_builder

test/builders/file_test.dart

@@ -58,5 +58,14 @@ void main() {
         '''),
       );
     });
+
+    group('$LibraryBuilder', () {
+      test('should handle empty methods', () {
+        expect(
+            new LibraryBuilder()
+              ..addMember(new MethodBuilder.returnVoid('main')),
+            equalsSource('void main() {}'));
+      });
+    });
   });
 }

test/builders/method_test.dart

@@ -12,7 +12,7 @@ void main() {
     expect(
       method('main'),
       equalsSource(r'''
-        main();
+        main() {}
       '''),
     );
   });
@@ -23,7 +23,7 @@ void main() {
         lib$core.$void,
       ]).buildMethod(false).toSource(),
       equalsIgnoringWhitespace(r'''
-        void main();
+        void main() {}
       '''),
     );
   });
@@ -34,7 +34,7 @@ void main() {
         parameter('args', [lib$core.List]),
       ]).buildMethod(false).toSource(),
       equalsIgnoringWhitespace(r'''
-        main(List args);
+        main(List args) {}
       '''),
     );
   });
@@ -47,7 +47,7 @@ void main() {
         parameter('c').asOptional(),
       ]),
       equalsSource(r'''
-        main(a, b, [c]);
+        main(a, b, [c]) {}
       '''),
     );
   });
@@ -60,7 +60,7 @@ void main() {
         parameter('c').asOptional(literal(true)),
       ]),
       equalsSource(r'''
-        main(a, b, [c = true]);
+        main(a, b, [c = true]) {}
       '''),
     );
   });
@@ -72,7 +72,7 @@ void main() {
         named(parameter('b').asOptional(literal(true))),
       ]).buildMethod(false).toSource(),
       equalsIgnoringWhitespace(r'''
-        main({a, b : true});
+        main({a, b : true}) {}
       '''),
     );
   });
@@ -170,18 +170,38 @@ void main() {
     );
   });
 
-  test('should emit a closure', () {
-    final closure = new MethodBuilder.closure(
-      returns: literal(false).or(reference('defaultTo')),
-      returnType: lib$core.bool,
-    )..addPositional(parameter('defaultTo', [lib$core.bool]));
-    // Should be usable as an expression/parameter itself.
-    expect(closure, const isInstanceOf<ExpressionBuilder>());
-    expect(
-      closure,
-      equalsSource(r'''
-        (bool defaultTo) => false || defaultTo;
+  group('closure', () {
+    MethodBuilder closure;
+    setUp(() {
+      closure = new MethodBuilder.closure(
+        returns: literal(false).or(reference('defaultTo')),
+        returnType: lib$core.bool,
+      )..addPositional(parameter('defaultTo', [lib$core.bool]));
+    });
+
+    test('should emit a closure', () {
+      // Should be usable as an expression/parameter itself.
+      expect(closure, const isInstanceOf<ExpressionBuilder>());
+      expect(
+        closure,
+        equalsSource(r'''
+        (bool defaultTo) => false || defaultTo
       '''),
-    );
+      );
+
+      test('should treat closure as expression', () {
+        expect(
+            list([true, false]).invoke('where', [closure]),
+            equalsSource(
+                '[true, false].where((bool defaultTo) => false || defaultTo)'));
+      });
+
+      test('should emit a closure as a function in a library', () {
+        final library = new LibraryBuilder();
+        library.addMember(closure);
+        expect(
+            library, equalsSource('(bool defaultTo) => false || defaultTo;'));
+      });
+    });
   });
 }