Add support for trailing commas in Emitter (#376)

Author: srawlins

CHANGELOG.md

@@ -3,6 +3,8 @@
 * Only emit `late` keyword when using null safety syntax.
 * Use implicit `const` when assigning to a `declareConst` variable.
 * Deprecate `assignVar`, `assignConst`, and `assignFinal`.
+* Add trailing commas to any parameter list, argument list, or collection
+  literal which has more than one element.
 
 ## 4.2.0
 

lib/src/emitter.dart

@@ -213,12 +213,15 @@ class DartEmitter extends Object
         ..write(spec.name);
     }
     output.write('(');
+    final hasMultipleParameters =
+        spec.requiredParameters.length + spec.optionalParameters.length > 1;
     if (spec.requiredParameters.isNotEmpty) {
       var count = 0;
       for (final p in spec.requiredParameters) {
         count++;
         _visitParameter(p, output);
-        if (spec.requiredParameters.length != count ||
+        if (hasMultipleParameters ||
+            spec.requiredParameters.length != count ||
             spec.optionalParameters.isNotEmpty) {
           output.write(', ');
         }
@@ -235,7 +238,7 @@ class DartEmitter extends Object
       for (final p in spec.optionalParameters) {
         count++;
         _visitParameter(p, output, optional: true, named: named);
-        if (spec.optionalParameters.length != count) {
+        if (hasMultipleParameters || spec.optionalParameters.length != count) {
           output.write(', ');
         }
       }
@@ -453,20 +456,30 @@ class DartEmitter extends Object
       out.write('>');
     }
     out.write('(');
+    final needsTrailingComma = spec.requiredParameters.length +
+            spec.optionalParameters.length +
+            spec.namedRequiredParameters.length +
+            spec.namedParameters.length >
+        1;
     visitAll<Reference>(spec.requiredParameters, out, (spec) {
       spec.accept(this, out);
     });
     final hasNamedParameters = spec.namedRequiredParameters.isNotEmpty ||
         spec.namedParameters.isNotEmpty;
     if (spec.requiredParameters.isNotEmpty &&
-        (spec.optionalParameters.isNotEmpty || hasNamedParameters)) {
+        (needsTrailingComma ||
+            spec.optionalParameters.isNotEmpty ||
+            hasNamedParameters)) {
       out.write(', ');
     }
     if (spec.optionalParameters.isNotEmpty) {
       out.write('[');
       visitAll<Reference>(spec.optionalParameters, out, (spec) {
         spec.accept(this, out);
       });
+      if (needsTrailingComma) {
+        out.write(', ');
+      }
       out.write(']');
     } else if (hasNamedParameters) {
       out.write('{');
@@ -487,6 +500,9 @@ class DartEmitter extends Object
           ..write(' ')
           ..write(name);
       });
+      if (needsTrailingComma) {
+        out.write(', ');
+      }
       out.write('}');
     }
     out.write(')');
@@ -526,12 +542,15 @@ class DartEmitter extends Object
       }
       visitTypeParameters(spec.types.map((r) => r.type), output);
       output.write('(');
+      final hasMultipleParameters =
+          spec.requiredParameters.length + spec.optionalParameters.length > 1;
       if (spec.requiredParameters.isNotEmpty) {
         var count = 0;
         for (final p in spec.requiredParameters) {
           count++;
           _visitParameter(p, output);
-          if (spec.requiredParameters.length != count ||
+          if (hasMultipleParameters ||
+              spec.requiredParameters.length != count ||
               spec.optionalParameters.isNotEmpty) {
             output.write(', ');
           }
@@ -548,7 +567,8 @@ class DartEmitter extends Object
         for (final p in spec.optionalParameters) {
           count++;
           _visitParameter(p, output, optional: true, named: named);
-          if (spec.optionalParameters.length != count) {
+          if (hasMultipleParameters ||
+              spec.optionalParameters.length != count) {
             output.write(', ');
           }
         }

lib/src/specs/expression.dart

@@ -497,6 +497,11 @@ abstract class ExpressionEmitter implements ExpressionVisitor<StringSink> {
           ..write(': ');
         expression.namedArguments[name]!.accept(this, out);
       });
+      final argumentCount = expression.positionalArguments.length +
+          expression.namedArguments.length;
+      if (argumentCount > 1) {
+        out.write(', ');
+      }
       return out..write(')');
     });
   }
@@ -535,6 +540,9 @@ abstract class ExpressionEmitter implements ExpressionVisitor<StringSink> {
       visitAll<Object?>(expression.values, out, (value) {
         _acceptLiteral(value, out);
       });
+      if (expression.values.length > 1) {
+        out.write(', ');
+      }
       return out..write(']');
     });
   }
@@ -556,6 +564,9 @@ abstract class ExpressionEmitter implements ExpressionVisitor<StringSink> {
       visitAll<Object?>(expression.values, out, (value) {
         _acceptLiteral(value, out);
       });
+      if (expression.values.length > 1) {
+        out.write(', ');
+      }
       return out..write('}');
     });
   }
@@ -585,6 +596,9 @@ abstract class ExpressionEmitter implements ExpressionVisitor<StringSink> {
         out.write(': ');
         _acceptLiteral(value, out);
       });
+      if (expression.values.length > 1) {
+        out.write(', ');
+      }
       return out..write('}');
     });
   }

test/const_test.dart

@@ -17,15 +17,15 @@ void main() {
 
   test('expression', () {
     expect(constMap, equalsDart(r'''
-          const {'list': [], 'duration': Duration()}'''));
+          const {'list': [], 'duration': Duration(), }'''));
   });
 
   test('assignConst', () {
     expect(
       // ignore: deprecated_member_use_from_same_package
       constMap.assignConst('constField'),
       equalsDart(r'''
-          const constField = {'list': [], 'duration': Duration()}''',
+          const constField = {'list': [], 'duration': Duration(), }''',
           DartEmitter.scoped()),
     );
   });
@@ -34,7 +34,7 @@ void main() {
     expect(
       declareConst('constField').assign(constMap),
       equalsDart(r'''
-          const constField = {'list': [], 'duration': Duration()}''',
+          const constField = {'list': [], 'duration': Duration(), }''',
           DartEmitter.scoped()),
     );
   });
@@ -43,7 +43,7 @@ void main() {
     expect(
         declareVar('varField').assign(constMap),
         equalsDart(r'''
-          var varField = const {'list': [], 'duration': Duration()}''',
+          var varField = const {'list': [], 'duration': Duration(), }''',
             DartEmitter.scoped()));
   });
 

test/e2e/injection_test.dart

@@ -45,7 +45,7 @@ void main() {
           final Module _module;
 
           @override
-          Thing getThing() => Thing(_module.get1(), _module.get2());
+          Thing getThing() => Thing(_module.get1(), _module.get2(), );
         }
       '''),
     );
@@ -59,7 +59,7 @@ void main() {
           final _i2.Module _module;
 
           @override
-          _i3.Thing getThing() => _i3.Thing(_module.get1(), _module.get2());
+          _i3.Thing getThing() => _i3.Thing(_module.get1(), _module.get2(), );
         }
       ''', DartEmitter(allocator: Allocator.simplePrefixing())),
     );

test/specs/class_test.dart

@@ -312,7 +312,7 @@ void main() {
     );
   });
 
-  test('should create a class with method parameters', () {
+  test('should create a class with a constructor with parameters', () {
     expect(
       Class((b) => b
         ..name = 'Foo'
@@ -328,7 +328,7 @@ void main() {
           ])))),
       equalsDart(r'''
         class Foo {
-          Foo(a, b, {c});
+          Foo(a, b, {c, });
         }
       '''),
     );
@@ -355,7 +355,7 @@ void main() {
           ])))),
       equalsDart(r'''
         class Foo {
-          Foo(this.a, this.b, {this.c});
+          Foo(this.a, this.b, {this.c, });
         }
       '''),
     );
@@ -382,7 +382,7 @@ void main() {
           ])))),
       equalsDart(r'''
         class Foo {
-          Foo(super.a, super.b, {super.c});
+          Foo(super.a, super.b, {super.c, });
         }
       '''),
     );

test/specs/code/expression_test.dart

@@ -96,7 +96,7 @@ void main() {
         refer('three'): 3,
         refer('Map').newInstance([]): null,
       }),
-      equalsDart(r"{1: 'one', 2: two, three: 3, Map(): null}"),
+      equalsDart(r"{1: 'one', 2: two, three: 3, Map(): null, }"),
     );
   });
 
@@ -110,7 +110,7 @@ void main() {
         null,
         refer('Map').newInstance([])
       ]),
-      equalsDart('[[], {}, true, null, Map()]'),
+      equalsDart('[[], {}, true, null, Map(), ]'),
     );
   });
 
@@ -125,7 +125,7 @@ void main() {
         null,
         refer('Map').newInstance([])
       ]),
-      equalsDart('{[], {}, true, null, Map()}'),
+      equalsDart('{[], {}, true, null, Map(), }'),
     );
   });
 
@@ -190,7 +190,7 @@ void main() {
         literal(2),
         literal(3),
       ]),
-      equalsDart('foo(1, 2, 3)'),
+      equalsDart('foo(1, 2, 3, )'),
     );
   });
 
@@ -209,7 +209,7 @@ void main() {
         'bar': literal(1),
         'baz': literal(2),
       }),
-      equalsDart('foo(bar: 1, baz: 2)'),
+      equalsDart('foo(bar: 1, baz: 2, )'),
     );
   });
 
@@ -221,7 +221,7 @@ void main() {
         'bar': literal(2),
         'baz': literal(3),
       }),
-      equalsDart('foo(1, bar: 2, baz: 3)'),
+      equalsDart('foo(1, bar: 2, baz: 3, )'),
     );
   });
 
@@ -287,7 +287,7 @@ void main() {
       FunctionType((b) => b
         ..requiredParameters.add(refer('String'))
         ..optionalParameters.add(refer('int'))),
-      equalsDart('Function(String, [int])'),
+      equalsDart('Function(String, [int, ])'),
     );
   });
 
@@ -298,7 +298,7 @@ void main() {
           'x': refer('int'),
           'y': refer('int'),
         })),
-      equalsDart('Function({int x, int y})'),
+      equalsDart('Function({int x, int y, })'),
     );
   });
 
@@ -313,7 +313,7 @@ void main() {
         ..namedParameters.addAll({
           'y': refer('int'),
         })),
-      equalsDart('Function({required int x, int y})'),
+      equalsDart('Function({required int x, int y, })'),
     );
   });
 
@@ -324,7 +324,7 @@ void main() {
           'x': refer('int'),
           'y': refer('int'),
         })),
-      equalsDart('Function({required int x, required int y})'),
+      equalsDart('Function({required int x, required int y, })'),
     );
   });
 
@@ -373,7 +373,7 @@ void main() {
         literalString('foo'),
         Method((b) => b..body = literalTrue.code).closure,
       ]),
-      equalsDart("map.putIfAbsent('foo', () => true)"),
+      equalsDart("map.putIfAbsent('foo', () => true, )"),
     );
   });
 
@@ -385,7 +385,7 @@ void main() {
           ..types.add(refer('T'))
           ..body = literalTrue.code).genericClosure,
       ]),
-      equalsDart("map.putIfAbsent('foo', <T>() => true)"),
+      equalsDart("map.putIfAbsent('foo', <T>() => true, )"),
     );
   });