Streamlined error handling in all builders (#455)

There should be no user-visible behavior change.
dart-lang · Mar 11, 2020 · a3e3c9a · a3e3c9a
1 parent 18e7244
commit a3e3c9a
Show file tree

Hide file tree

Showing 6 changed files with 55 additions and 104 deletions.
diff --git a/source_gen/CHANGELOG.md b/source_gen/CHANGELOG.md
@@ -1,3 +1,8 @@
+## 0.9.6
+
+* Streamlined error handling in all builders. There should be no user-visible
+  behavior change.
+
 ## 0.9.5
 
 * Add support for an `ignore_for_file` option to `combining_builder`.

diff --git a/source_gen/lib/src/builder.dart b/source_gen/lib/src/builder.dart
@@ -126,7 +126,9 @@ class _Builder extends Builder {
         ..writeln()
         ..writeln(_headerLine)
         ..writeAll(
-            LineSplitter.split(item.toString()).map((line) => '// $line\n'))
+          LineSplitter.split(item.generatorDescription)
+              .map((line) => '// $line\n'),
+        )
         ..writeln(_headerLine)
         ..writeln()
         ..writeln(item.output);
@@ -289,28 +291,23 @@ Stream<GeneratedOutput> _generate(
   final libraryReader = LibraryReader(library);
   for (var i = 0; i < generators.length; i++) {
     final gen = generators[i];
-    try {
-      var msg = 'Running $gen';
-      if (generators.length > 1) {
-        msg = '$msg - ${i + 1} of ${generators.length}';
-      }
-      log.fine(msg);
-      var createdUnit = await gen.generate(libraryReader, buildStep);
-
-      if (createdUnit == null) {
-        continue;
-      }
+    var msg = 'Running $gen';
+    if (generators.length > 1) {
+      msg = '$msg - ${i + 1} of ${generators.length}';
+    }
+    log.fine(msg);
+    var createdUnit = await gen.generate(libraryReader, buildStep);
 
-      createdUnit = createdUnit.trim();
-      if (createdUnit.isEmpty) {
-        continue;
-      }
+    if (createdUnit == null) {
+      continue;
+    }
 
-      yield GeneratedOutput(gen, createdUnit);
-    } catch (e, stack) {
-      log.severe('Error running $gen', e, stack);
-      yield GeneratedOutput.fromError(gen, e, stack);
+    createdUnit = createdUnit.trim();
+    if (createdUnit.isEmpty) {
+      continue;
     }
+
+    yield GeneratedOutput(gen, createdUnit);
   }
 }
 

diff --git a/source_gen/lib/src/generated_output.dart b/source_gen/lib/src/generated_output.dart
@@ -2,66 +2,24 @@
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
-import 'dart:convert';
-
 import 'generator.dart';
 
 class GeneratedOutput {
   final String output;
-  final Generator generator;
-  final dynamic error;
-  final StackTrace stackTrace;
-
-  bool get isError => error != null;
+  final String generatorDescription;
 
-  GeneratedOutput(this.generator, this.output)
-      : error = null,
-        stackTrace = null,
-        assert(output != null),
+  GeneratedOutput(Generator generator, this.output)
+      : assert(output != null),
         assert(output.isNotEmpty),
         // assuming length check is cheaper than simple string equality
-        assert(output.length == output.trim().length);
+        assert(output.length == output.trim().length),
+        generatorDescription = _toString(generator);
 
-  GeneratedOutput.fromError(this.generator, this.error, this.stackTrace)
-      : output = _outputFromError(error);
-
-  @override
-  String toString() {
+  static String _toString(Generator generator) {
     final output = generator.toString();
     if (output.endsWith('Generator')) {
       return output;
     }
     return 'Generator: $output';
   }
 }
-
-String _outputFromError(Object error) {
-  final buffer = StringBuffer();
-
-  _commentWithHeader(_errorHeader, error.toString(), buffer);
-
-  if (error is InvalidGenerationSourceError && error.todo.isNotEmpty) {
-    _commentWithHeader(_todoHeader, error.todo, buffer);
-  }
-
-  return buffer.toString();
-}
-
-void _commentWithHeader(String header, String content, StringSink buffer) {
-  final lines = const LineSplitter().convert(content);
-
-  buffer
-    ..writeAll([_commentPrefix, header, lines.first])
-    ..writeln();
-
-  final blankPrefix = ''.padLeft(header.length, ' ');
-  for (var i = 1; i < lines.length; i++) {
-    buffer
-      ..writeAll([_commentPrefix, blankPrefix, lines[i]])
-      ..writeln();
-  }
-}
-
-const _commentPrefix = '// ';
-const _errorHeader = 'Error: ';
-const _todoHeader = 'TODO: ';
diff --git a/source_gen/pubspec.yaml b/source_gen/pubspec.yaml
@@ -1,5 +1,5 @@
 name: source_gen
-version: 0.9.5
+version: 0.9.6-dev
 description: Automated source code generation for Dart.
 homepage: https://github.com/dart-lang/source_gen
 

diff --git a/source_gen/test/builder_test.dart b/source_gen/test/builder_test.dart
@@ -114,11 +114,21 @@ void main() {
   test('handle generator errors well', () async {
     final srcs = _createPackageStub(testLibContent: _testLibContentWithError);
     final builder = PartBuilder([const CommentGenerator()], '.foo.dart');
-    await testBuilder(builder, srcs, generateFor: {
-      '$_pkgName|lib/test_lib.dart'
-    }, outputs: {
-      '$_pkgName|lib/test_lib.foo.dart': _testGenPartContentError,
-    });
+
+    await expectLater(
+      () => testBuilder(
+        builder,
+        srcs,
+        generateFor: {'$_pkgName|lib/test_lib.dart'},
+      ),
+      throwsA(
+        isA<InvalidGenerationSourceError>().having(
+          (source) => source.message,
+          'message',
+          "Don't use classes with the word 'Error' in the name",
+        ),
+      ),
+    );
   });
 
   test('warns when a non-standalone builder does not see "part"', () async {
@@ -587,23 +597,6 @@ part of test_lib;
 // Code for "class Customer"
 ''';
 
-const _testGenPartContentError = r'''// GENERATED CODE - DO NOT MODIFY BY HAND
-
-part of test_lib;
-
-// **************************************************************************
-// CommentGenerator
-// **************************************************************************
-
-// Error: Don't use classes with the word 'Error' in the name
-//        package:pkg/test_lib.dart:4:7
-//          ╷
-//        4 │ class MyGoodError { }
-//          │       ^^^^^^^^^^^
-//          ╵
-// TODO: Rename MyGoodError to something else.
-''';
-
 const _testGenNoLibrary = r'''// GENERATED CODE - DO NOT MODIFY BY HAND
 
 part of 'test_lib.dart';

diff --git a/source_gen/test/generator_for_annotation_test.dart b/source_gen/test/generator_for_annotation_test.dart
@@ -4,13 +4,11 @@
 
 // The first test that runs `testBuilder` takes a LOT longer than the rest.
 @Timeout.factor(3)
-
 import 'package:analyzer/dart/element/element.dart';
 import 'package:build/build.dart';
 import 'package:build_test/build_test.dart';
-import 'package:test/test.dart';
-
 import 'package:source_gen/source_gen.dart';
+import 'package:test/test.dart';
 
 void main() {
   group('skips output if per-annotation output is', () {
@@ -58,17 +56,17 @@ void main() {
     }.entries) {
       test(entry.key, () async {
         final builder = LibraryBuilder(entry.value);
-        await testBuilder(builder, _inputMap, outputs: {
-          'a|lib/file.g.dart': r'''
-// GENERATED CODE - DO NOT MODIFY BY HAND
 
-// **************************************************************************
-// FailingGenerator
-// **************************************************************************
-
-// Error: Bad state: not supported!
-'''
-        });
+        await expectLater(
+          () => testBuilder(builder, _inputMap),
+          throwsA(
+            isA<StateError>().having(
+              (source) => source.message,
+              'message',
+              'not supported!',
+            ),
+          ),
+        );
       });
     }
   });