Better warnings and errors logging. (#4190)

* Better warnings and errors logging.

* Address review comments.

Author: davidmorgan

build_runner/CHANGELOG.md

@@ -1,5 +1,7 @@
 ## 2.7.3-wip
 
+- Make errors more prominent in log output, highlight in red.
+- Remove "deleting declared outputs" warning.
 - Bug fix: fix incorrect display of some "usage" messages.
 - Move code from `build_resolvers` into `build_runner`.
 - Move code from `build_runner_core` into `build_runner`.

build_runner/lib/src/generate/build_definition.dart

@@ -211,11 +211,6 @@ class _Loader {
     ReaderWriter readerWriter,
   ) async {
     if (conflictingAssets.isEmpty) return;
-
-    buildLog.info(
-      'Deleting ${conflictingAssets.length} declared outputs '
-      'which already existed on disk.',
-    );
     await Future.wait(conflictingAssets.map((id) => readerWriter.delete(id)));
   }
 }

build_runner/lib/src/logging/ansi_buffer.dart

@@ -14,6 +14,7 @@ class AnsiBuffer {
   static const String nbsp = '\u00A0';
   static const reset = '\x1B[0m';
   static const bold = '\x1B[1m';
+  static const boldRed = '\x1B[1;31m';
   static const _nbspCodeUnit = 0xA0;
   static const _spaceCodeUnit = 32;
 
@@ -127,7 +128,7 @@ class AnsiBuffer {
 
   /// Removes all ANSI constants from [string], for testing.
   static String removeAnsi(String string) =>
-      string.replaceAll(reset, '').replaceAll(bold, '');
+      string.replaceAll(reset, '').replaceAll(bold, '').replaceAll(boldRed, '');
 }
 
 /// A line for writing to an [AnsiBuffer].
@@ -167,7 +168,9 @@ class AnsiBufferLine {
 }
 
 bool _isAnsi(String item) =>
-    item == AnsiBuffer.reset || item == AnsiBuffer.bold;
+    item == AnsiBuffer.reset ||
+    item == AnsiBuffer.bold ||
+    item == AnsiBuffer.boldRed;
 
 bool get _showingAnsi =>
     buildLog.configuration.forceAnsiConsoleForTesting ??

build_runner/lib/src/logging/build_log.dart

@@ -386,7 +386,11 @@ class BuildLog {
       _display.block(render());
       _display.flush();
     } else {
-      _display.message(Severity.info, _status.join(''));
+      _display.message(
+        Severity.info,
+        // Removes ANSI codes if necessary.
+        AnsiBufferLine(_status).toString(),
+      );
     }
   }
 
@@ -456,23 +460,33 @@ class BuildLog {
         .fold(0, max);
     final indent = maxProgressWidth + 1;
 
-    for (final entry in displayedProgressEntries) {
-      final phaseName = entry.key;
+    if (displayedProgressEntries.isNotEmpty) {
+      for (final entry in displayedProgressEntries) {
+        final phaseName = entry.key;
 
-      result.write(_renderPhase(phaseName).withHangingIndent(indent));
+        result.write(_renderPhase(phaseName).withHangingIndent(indent));
+      }
+      result.writeEmptyLine();
     }
 
-    if (displayedProgressEntries.isNotEmpty) {
-      result.writeLine([]);
+    final renderedMessages = _messages.render();
+
+    // Log output blocks with no errors show before the status line.
+    if (renderedMessages.nonFailureLines.isNotEmpty) {
+      for (final line in renderedMessages.nonFailureLines) {
+        result.write(line);
+      }
+      result.writeEmptyLine();
     }
+
     if (_status.isNotEmpty) {
       result.writeLine(_status);
     }
 
-    final renderedMessages = _messages.render();
-    if (renderedMessages.isNotEmpty) {
-      result.writeEmptyLine();
-      for (final line in renderedMessages) {
+    // Log output blocks that do have errors show after the status line.
+    if (renderedMessages.failureLines.isNotEmpty) {
+      if (_status.isNotEmpty) result.writeEmptyLine();
+      for (final line in renderedMessages.failureLines) {
         result.write(line);
       }
     }

build_runner/lib/src/logging/build_log_messages.dart

@@ -51,8 +51,8 @@ class BuildLogMessages {
   bool hasMessages({required String? phaseName}) =>
       _phaseNamesWithMessages.contains(phaseName);
 
-  List<AnsiBufferLine> render() {
-    final result = <AnsiBufferLine>[];
+  RenderResult render() {
+    final result = RenderResult();
 
     final buildRunnerCategories = <MapEntry<_MessageCategory, List<Message>>>[];
     for (final entry in _messageByCategory.entries) {
@@ -72,40 +72,51 @@ class BuildLogMessages {
     return result;
   }
 
-  List<AnsiBufferLine> _renderCategory(
+  RenderResult _renderCategory(
     MapEntry<_MessageCategory, List<Message>> entry,
   ) {
-    final result = <AnsiBufferLine>[];
     final category = entry.key;
     final context = category.context;
+
+    final messages = _messageByCategory[category]!;
+    final failed = messages.any(
+      (message) => message.severity == Severity.error,
+    );
+
+    final result = <AnsiBufferLine>[];
     result.add(
       AnsiBufferLine([
-        'log output for ',
-        AnsiBuffer.bold,
-        category.phaseName ?? 'build_runner',
-        AnsiBuffer.reset,
         if (context != null) ...[
-          ' on ',
-          AnsiBuffer.bold,
+          failed ? AnsiBuffer.boldRed : AnsiBuffer.bold,
           context,
           AnsiBuffer.reset,
+          ' ',
         ],
+        AnsiBuffer.bold,
+        category.phaseName ?? 'build_runner',
+        AnsiBuffer.reset,
       ]),
     );
 
-    for (final message in _messageByCategory[category]!) {
+    for (final message in messages) {
       var first = true;
+      final isError = message.severity == Severity.error;
       for (final line in message.text.split('\n')) {
         result.add(
           AnsiBufferLine([
+            if (isError) AnsiBuffer.boldRed,
             first ? message.severity.prefix : '  ',
+            if (isError) AnsiBuffer.reset,
             line,
           ], hangingIndent: 2),
         );
         first = false;
       }
     }
-    return result;
+
+    return failed
+        ? RenderResult.failed(result)
+        : RenderResult.succeeded(result);
   }
 }
 
@@ -177,3 +188,24 @@ enum Severity {
     Severity.error => 'E ',
   };
 }
+
+class RenderResult {
+  /// Rendered messages block that does not contain any errors.
+  final List<AnsiBufferLine> nonFailureLines;
+
+  /// Rendered messages block that does contain errors.
+  final List<AnsiBufferLine> failureLines;
+
+  RenderResult() : nonFailureLines = [], failureLines = [];
+  RenderResult.failed(List<AnsiBufferLine> lines)
+    : failureLines = lines,
+      nonFailureLines = [];
+  RenderResult.succeeded(List<AnsiBufferLine> lines)
+    : nonFailureLines = lines,
+      failureLines = [];
+
+  void addAll(RenderResult other) {
+    nonFailureLines.addAll(other.nonFailureLines);
+    failureLines.addAll(other.failureLines);
+  }
+}

build_runner/test/logging/build_log_console_mode_test.dart

@@ -62,7 +62,7 @@ columns because it will not fit.'''),
         padLinesRight('''
 Some setup.
 
-log output for build_runner
+build_runner
   Some info.
 W A warning.
 W Another warning.
@@ -238,15 +238,16 @@ Building, full build.'''),
 0s builder1 on 10 inputs; pkg|lib/l0.dart
 0s builder2 on 15 inputs
 
-Building, full build.
-
-log output for builder1 on lib/l0.dart
+lib/l0.dart builder1
   Some info.
   Some more info.
-log output for builder2 on lib/l1.dart
+
+Building, full build.
+
+lib/l1.dart builder2
 W A warning.
 E An error.
-log output for builder2 on lib/l3.dart
+lib/l3.dart builder2
 E An error.'''),
       );
 
@@ -275,17 +276,18 @@ E An error.'''),
 0s builder2 on 15 inputs
 0s builder1 (lazy): 1 no-op
 
-Building, full build.
-
-log output for builder1 on lib/l0.dart
+lib/l0.dart builder1
   Some info.
   Some more info.
-log output for builder2 on lib/l1.dart
+
+Building, full build.
+
+lib/l1.dart builder2
 W A warning.
 E An error.
-log output for builder2 on lib/l3.dart
+lib/l3.dart builder2
 E An error.
-log output for builder1 (lazy) on lib/l3.dart
+lib/l3.dart builder1 (lazy)
 E An error.'''),
       );
     });
@@ -343,11 +345,12 @@ E An error.'''),
 0s builder1 on 1 input: 1 output
 0s builder2 on 1 input: 1 output
 
+lib/l0.dart builder1
+  Some info.
+
 Built with build_runner in 0s with warnings; wrote 2 outputs.
 
-log output for builder1 on lib/l0.dart
-  Some info.
-log output for builder2 on lib/l0.dart
+lib/l0.dart builder2
 W A warning.
 E An error.'''),
       );

build_runner/test/logging/build_log_messages_test.dart

@@ -95,19 +95,22 @@ void main() {
         context: 'file1',
         severity: Severity.info,
       );
-      expect(messages.render().map((l) => l.toString()).toList(), [
-        'log output for phase1 on file1',
-        'E message1',
-        'W message3',
-        'log output for phase2 on file1',
+      final rendered = messages.render();
+      expect(rendered.nonFailureLines.map((l) => l.toString()).toList(), [
+        'file1 phase2',
         '  message2',
         '  message6',
-        'log output for phase1 on file2',
+        'file2 phase1',
         '  message4',
-        'log output for build_runner',
+        'build_runner',
         'W message0',
         '  message5',
       ]);
+      expect(rendered.failureLines.map((l) => l.toString()).toList(), [
+        'file1 phase1',
+        'E message1',
+        'W message3',
+      ]);
     });
   });
 }