[flutter_tools] Unified analyze error log format (#61146)

* Unified analyze error log format

* Fix review issues

packages/flutter_tools/lib/src/commands/analyze_base.dart

@@ -17,6 +17,7 @@ import '../base/platform.dart';
 import '../base/terminal.dart';
 import '../base/utils.dart';
 import '../cache.dart';
+import '../dart/analysis.dart';
 import '../globals.dart' as globals;
 
 /// Common behavior for `flutter analyze` and `flutter analyze --watch`
@@ -84,7 +85,16 @@ abstract class AnalyzeBase {
     logger.printStatus('Analysis benchmark written to $benchmarkOut ($data).');
   }
 
+  bool get isFlutterRepo => argResults['flutter-repo'] as bool;
+  String get sdkPath => argResults['dart-sdk'] as String ?? artifacts.getArtifactPath(Artifact.engineDartSdkPath);
   bool get isBenchmarking => argResults['benchmark'] as bool;
+  bool get isDartDocs => argResults['dartdocs'] as bool;
+
+  static int countMissingDartDocs(List<AnalysisError> errors) {
+    return errors.where((AnalysisError error) {
+      return error.code == 'public_member_api_docs';
+    }).length;
+  }
 
   static String generateDartDocMessage(int undocumentedMembers) {
     String dartDocMessage;
@@ -103,6 +113,41 @@ abstract class AnalyzeBase {
 
     return dartDocMessage;
   }
+
+  /// Generate an analysis summary for both [AnalyzeOnce], [AnalyzeContinuously].
+  static String generateErrorsMessage({
+    @required int issueCount,
+    int issueDiff,
+    int files,
+    @required String seconds,
+    int undocumentedMembers = 0,
+    String dartDocMessage = '',
+  }) {
+    final StringBuffer errorsMessage = StringBuffer(issueCount > 0
+      ? '$issueCount ${pluralize('issue', issueCount)} found.'
+      : 'No issues found!');
+
+    // Only [AnalyzeContinuously] has issueDiff message.
+    if (issueDiff != null) {
+      if (issueDiff > 0) {
+        errorsMessage.write(' ($issueDiff new)');
+      } else if (issueDiff < 0) {
+        errorsMessage.write(' (${-issueDiff} fixed)');
+      }
+    }
+
+    // Only [AnalyzeContinuously] has files message.
+    if (files != null) {
+      errorsMessage.write(' • analyzed $files ${pluralize('file', files)}');
+    }
+
+    if (undocumentedMembers > 0) {
+      errorsMessage.write(' (ran in ${seconds}s; $dartDocMessage)');
+    } else {
+      errorsMessage.write(' (ran in ${seconds}s;)');
+    }
+    return errorsMessage.toString();
+  }
 }
 
 class PackageDependency {

packages/flutter_tools/lib/src/commands/analyze_continuously.dart

@@ -15,7 +15,6 @@ import '../base/io.dart';
 import '../base/logger.dart';
 import '../base/platform.dart';
 import '../base/terminal.dart';
-import '../base/utils.dart';
 import '../dart/analysis.dart';
 import 'analyze_base.dart';
 
@@ -56,7 +55,7 @@ class AnalyzeContinuously extends AnalyzeBase {
   Future<void> analyze() async {
     List<String> directories;
 
-    if (argResults['flutter-repo'] as bool) {
+    if (isFlutterRepo) {
       final PackageDependencyTracker dependencies = PackageDependencyTracker();
       dependencies.checkForConflictingDependencies(repoPackages, dependencies);
 
@@ -72,9 +71,6 @@ class AnalyzeContinuously extends AnalyzeBase {
       analysisTarget = fileSystem.currentDirectory.path;
     }
 
-    final String sdkPath = argResults['dart-sdk'] as String ??
-      artifacts.getArtifactPath(Artifact.engineDartSdkPath);
-
     final AnalysisServer server = AnalysisServer(
       sdkPath,
       directories,
@@ -131,10 +127,8 @@ class AnalyzeContinuously extends AnalyzeBase {
       int issueCount = errors.length;
 
       // count missing dartdocs
-      final int undocumentedMembers = errors.where((AnalysisError error) {
-        return error.code == 'public_member_api_docs';
-      }).length;
-      if (!(argResults['dartdocs'] as bool)) {
+      final int undocumentedMembers = AnalyzeBase.countMissingDartDocs(errors);
+      if (!isDartDocs) {
         errors.removeWhere((AnalysisError error) => error.code == 'public_member_api_docs');
         issueCount -= undocumentedMembers;
       }
@@ -150,32 +144,20 @@ class AnalyzeContinuously extends AnalyzeBase {
 
       dumpErrors(errors.map<String>((AnalysisError error) => error.toLegacyString()));
 
-      // Print an analysis summary.
-      String errorsMessage;
       final int issueDiff = issueCount - lastErrorCount;
       lastErrorCount = issueCount;
-
-      if (firstAnalysis) {
-        errorsMessage = '$issueCount ${pluralize('issue', issueCount)} found';
-      } else if (issueDiff > 0) {
-        errorsMessage = '$issueCount ${pluralize('issue', issueCount)} found ($issueDiff new)';
-      } else if (issueDiff < 0) {
-        errorsMessage = '$issueCount ${pluralize('issue', issueCount)} found (${-issueDiff} fixed)';
-      } else if (issueCount != 0) {
-        errorsMessage = '$issueCount ${pluralize('issue', issueCount)} found';
-      } else {
-        errorsMessage = 'No issues found!';
-      }
-
+      final String seconds = (analysisTimer.elapsedMilliseconds / 1000.0).toStringAsFixed(2);
       final String dartDocMessage = AnalyzeBase.generateDartDocMessage(undocumentedMembers);
+      final String errorsMessage = AnalyzeBase.generateErrorsMessage(
+        issueCount: issueCount,
+        issueDiff: issueDiff,
+        files: analyzedPaths.length,
+        seconds: seconds,
+        undocumentedMembers: undocumentedMembers,
+        dartDocMessage: dartDocMessage,
+      );
 
-      final String files = '${analyzedPaths.length} ${pluralize('file', analyzedPaths.length)}';
-      final String seconds = (analysisTimer.elapsedMilliseconds / 1000.0).toStringAsFixed(2);
-      if (undocumentedMembers > 0) {
-        logger.printStatus('$errorsMessage • $dartDocMessage • analyzed $files in $seconds seconds');
-      } else {
-        logger.printStatus('$errorsMessage • analyzed $files in $seconds seconds');
-      }
+      logger.printStatus(errorsMessage);
 
       if (firstAnalysis && isBenchmarking) {
         writeBenchmark(analysisTimer, issueCount, undocumentedMembers);

packages/flutter_tools/lib/src/commands/analyze_once.dart

@@ -14,7 +14,6 @@ import '../base/file_system.dart';
 import '../base/logger.dart';
 import '../base/platform.dart';
 import '../base/terminal.dart';
-import '../base/utils.dart';
 import '../dart/analysis.dart';
 import 'analyze_base.dart';
 
@@ -67,7 +66,7 @@ class AnalyzeOnce extends AnalyzeBase {
       }
     }
 
-    if (argResults['flutter-repo'] as bool) {
+    if (isFlutterRepo) {
       // check for conflicting dependencies
       final PackageDependencyTracker dependencies = PackageDependencyTracker();
       dependencies.checkForConflictingDependencies(repoPackages, dependencies);
@@ -88,9 +87,6 @@ class AnalyzeOnce extends AnalyzeBase {
     final Completer<void> analysisCompleter = Completer<void>();
     final List<AnalysisError> errors = <AnalysisError>[];
 
-    final String sdkPath = argResults['dart-sdk'] as String ??
-      artifacts.getArtifactPath(Artifact.engineDartSdkPath);
-
     final AnalysisServer server = AnalysisServer(
       sdkPath,
       directories.toList(),
@@ -152,11 +148,8 @@ class AnalyzeOnce extends AnalyzeBase {
       timer?.stop();
     }
 
-    // count missing dartdocs
-    final int undocumentedMembers = errors.where((AnalysisError error) {
-      return error.code == 'public_member_api_docs';
-    }).length;
-    if (!(argResults['dartdocs'] as bool)) {
+    final int undocumentedMembers = AnalyzeBase.countMissingDartDocs(errors);
+    if (!isDartDocs) {
       errors.removeWhere((AnalysisError error) => error.code == 'public_member_api_docs');
     }
 
@@ -177,31 +170,28 @@ class AnalyzeOnce extends AnalyzeBase {
       logger.printStatus(error.toString(), hangingIndent: 7);
     }
 
+    final int errorCount = errors.length;
     final String seconds = (timer.elapsedMilliseconds / 1000.0).toStringAsFixed(1);
-
     final String dartDocMessage = AnalyzeBase.generateDartDocMessage(undocumentedMembers);
+    final String errorsMessage = AnalyzeBase.generateErrorsMessage(
+      issueCount: errorCount,
+      seconds: seconds,
+      undocumentedMembers: undocumentedMembers,
+      dartDocMessage: dartDocMessage,
+    );
 
-    // We consider any level of error to be an error exit (we don't report different levels).
-    if (errors.isNotEmpty) {
-      final int errorCount = errors.length;
+    if (errorCount > 0) {
       logger.printStatus('');
-      if (undocumentedMembers > 0) {
-        throwToolExit('$errorCount ${pluralize('issue', errorCount)} found. (ran in ${seconds}s; $dartDocMessage)');
-      } else {
-        throwToolExit('$errorCount ${pluralize('issue', errorCount)} found. (ran in ${seconds}s)');
-      }
+      // We consider any level of error to be an error exit (we don't report different levels).
+      throwToolExit(errorsMessage);
     }
 
-    if (server.didServerErrorOccur) {
-      throwToolExit('Server error(s) occurred. (ran in ${seconds}s)');
+    if (argResults['congratulate'] as bool) {
+      logger.printStatus(errorsMessage);
     }
 
-    if (argResults['congratulate'] as bool) {
-      if (undocumentedMembers > 0) {
-        logger.printStatus('No issues found! (ran in ${seconds}s; $dartDocMessage)');
-      } else {
-        logger.printStatus('No issues found! (ran in ${seconds}s)');
-      }
+    if (server.didServerErrorOccur) {
+      throwToolExit('Server error(s) occurred. (ran in ${seconds}s)');
     }
   }
 }

packages/flutter_tools/test/commands.shard/hermetic/analyze_test.dart

@@ -19,6 +19,30 @@ void main() {
     expect(AnalyzeBase.generateDartDocMessage(2), '2 public members lack documentation');
   });
 
+  testWithoutContext('analyze generate correct errors message', () async {
+    expect(
+      AnalyzeBase.generateErrorsMessage(
+        issueCount: 0,
+        seconds: '0.1',
+        undocumentedMembers: 1,
+        dartDocMessage: 'one public member lacks documentation',
+      ),
+      'No issues found! (ran in 0.1s; one public member lacks documentation)',
+    );
+
+    expect(
+      AnalyzeBase.generateErrorsMessage(
+        issueCount: 3,
+        issueDiff: 2,
+        files: 1,
+        seconds: '0.1',
+        undocumentedMembers: 1,
+        dartDocMessage: 'one public member lacks documentation',
+      ),
+      '3 issues found. (2 new) • analyzed 1 file (ran in 0.1s; one public member lacks documentation)',
+    );
+  });
+
   testWithoutContext('analyze inRepo', () {
     final FileSystem fileSystem = MemoryFileSystem.test();
     fileSystem.directory(_kFlutterRoot).createSync(recursive: true);
@@ -54,10 +78,10 @@ void main() {
         'offset': 362,
         'length': 72,
         'startLine': 15,
-        'startColumn': 4
+        'startColumn': 4,
       },
       'message': 'Prefer final for variable declarations if they are not reassigned.',
-      'hasFix': false
+      'hasFix': false,
     };
     expect(WrittenError.fromJson(json).toString(),
         '[info] Prefer final for variable declarations if they are not reassigned (/Users/.../lib/test.dart:15:4)');