From a3e3c9af7c8937b096d1c618e0ed50f71be9de63 Mon Sep 17 00:00:00 2001 From: Kevin Moore Date: Wed, 11 Mar 2020 12:41:09 -0700 Subject: [PATCH] Streamlined error handling in all builders (#455) There should be no user-visible behavior change. --- source_gen/CHANGELOG.md | 5 ++ source_gen/lib/src/builder.dart | 37 ++++++------- source_gen/lib/src/generated_output.dart | 54 +++---------------- source_gen/pubspec.yaml | 2 +- source_gen/test/builder_test.dart | 37 ++++++------- .../test/generator_for_annotation_test.dart | 24 ++++----- 6 files changed, 55 insertions(+), 104 deletions(-) diff --git a/source_gen/CHANGELOG.md b/source_gen/CHANGELOG.md index 8bd199e3..716c201c 100644 --- 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 index 2227dc69..b8d62e96 100644 --- 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 _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 index d6a82a3a..3f5476c3 100644 --- a/source_gen/lib/src/generated_output.dart +++ b/source_gen/lib/src/generated_output.dart @@ -2,31 +2,20 @@ // 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; @@ -34,34 +23,3 @@ class GeneratedOutput { 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 index de726b31..924fc431 100644 --- 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 index a5cc25fa..c8a03b29 100644 --- 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().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 index b94f93e9..8def5cca 100644 --- 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().having( + (source) => source.message, + 'message', + 'not supported!', + ), + ), + ); }); } });