Flutter analyze watch improvements (flutter#11143)

* flutter analyze --watch auto detect if in flutter repo
* move isFlutterLibrary from AnalyzeOnce into AnalyzeBase for use by AnalyzeContinuously
* pass --flutter-repo to analysis server when analyzing the flutter repository
* enhance flutter analyze --watch to summarize public members lacking documentation

Author: Dan Rubel

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

@@ -39,6 +39,25 @@ abstract class AnalyzeBase {
     }
   }
 
+  List<String> flutterRootComponents;
+  bool isFlutterLibrary(String filename) {
+    flutterRootComponents ??= fs.path.normalize(fs.path.absolute(Cache.flutterRoot)).split(fs.path.separator);
+    final List<String> filenameComponents = fs.path.normalize(fs.path.absolute(filename)).split(fs.path.separator);
+    if (filenameComponents.length < flutterRootComponents.length + 4) // the 4: 'packages', package_name, 'lib', file_name
+      return false;
+    for (int index = 0; index < flutterRootComponents.length; index += 1) {
+      if (flutterRootComponents[index] != filenameComponents[index])
+        return false;
+    }
+    if (filenameComponents[flutterRootComponents.length] != 'packages')
+      return false;
+    if (filenameComponents[flutterRootComponents.length + 1] == 'flutter_tools')
+      return false;
+    if (filenameComponents[flutterRootComponents.length + 2] != 'lib')
+      return false;
+    return true;
+  }
+
   void writeBenchmark(Stopwatch stopwatch, int errorCount, int membersMissingDocumentation) {
     final String benchmarkOut = 'analysis_benchmark.json';
     final Map<String, dynamic> data = <String, dynamic>{

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

@@ -31,15 +31,20 @@ class AnalyzeContinuously extends AnalyzeBase {
   Stopwatch analysisTimer;
   int lastErrorCount = 0;
   Status analysisStatus;
+  bool flutterRepo;
+  bool showDartDocIssuesIndividually;
 
   @override
   Future<Null> analyze() async {
     List<String> directories;
 
-    if (argResults['dartdocs'])
-      throwToolExit('The --dartdocs option is currently not supported when using --watch.');
+    flutterRepo = argResults['flutter-repo'] || inRepo(null);
+    showDartDocIssuesIndividually = argResults['dartdocs'];
 
-    if (argResults['flutter-repo']) {
+    if (showDartDocIssuesIndividually && !flutterRepo)
+      throwToolExit('The --dartdocs option is only supported when using --flutter-repo.');
+
+    if (flutterRepo) {
       final PackageDependencyTracker dependencies = new PackageDependencyTracker();
       dependencies.checkForConflictingDependencies(repoPackages, dependencies);
       directories = repoPackages.map((Directory dir) => dir.path).toList();
@@ -52,7 +57,7 @@ class AnalyzeContinuously extends AnalyzeBase {
       analysisTarget = fs.currentDirectory.path;
     }
 
-    final AnalysisServer server = new AnalysisServer(dartSdkPath, directories);
+    final AnalysisServer server = new AnalysisServer(dartSdkPath, directories, flutterRepo: flutterRepo);
     server.onAnalyzing.listen((bool isAnalyzing) => _handleAnalysisStatus(server, isAnalyzing));
     server.onErrors.listen(_handleAnalysisErrors);
 
@@ -82,29 +87,52 @@ class AnalyzeContinuously extends AnalyzeBase {
       logger.printStatus(terminal.clearScreen(), newline: false);
 
       // Remove errors for deleted files, sort, and print errors.
-      final List<AnalysisError> errors = <AnalysisError>[];
+      final List<AnalysisError> allErrors = <AnalysisError>[];
       for (String path in analysisErrors.keys.toList()) {
         if (fs.isFileSync(path)) {
-          errors.addAll(analysisErrors[path]);
+          allErrors.addAll(analysisErrors[path]);
         } else {
           analysisErrors.remove(path);
         }
       }
 
-      errors.sort();
+      // Summarize dartdoc issues rather than displaying each individually
+      int membersMissingDocumentation = 0;
+      List<AnalysisError> detailErrors;
+      if (flutterRepo && !showDartDocIssuesIndividually) {
+        detailErrors = allErrors.where((AnalysisError error) {
+          if (error.code == 'public_member_api_docs') {
+            // https://github.com/dart-lang/linter/issues/208
+            if (isFlutterLibrary(error.file))
+              membersMissingDocumentation += 1;
+            return true;
+          }
+          return false;
+        }).toList();
+      } else {
+        detailErrors = allErrors;
+      }
+
+      detailErrors.sort();
 
-      for (AnalysisError error in errors) {
+      for (AnalysisError error in detailErrors) {
         printStatus(error.toString());
         if (error.code != null)
           printTrace('error code: ${error.code}');
       }
 
-      dumpErrors(errors.map<String>((AnalysisError error) => error.toLegacyString()));
+      dumpErrors(detailErrors.map<String>((AnalysisError error) => error.toLegacyString()));
+
+      if (membersMissingDocumentation != 0) {
+        printStatus(membersMissingDocumentation == 1
+          ? '1 public member lacks documentation'
+          : '$membersMissingDocumentation public members lack documentation');
+      }
 
       // Print an analysis summary.
       String errorsMessage;
 
-      final int issueCount = errors.length;
+      final int issueCount = detailErrors.length;
       final int issueDiff = issueCount - lastErrorCount;
       lastErrorCount = issueCount;
 
@@ -150,10 +178,11 @@ class AnalyzeContinuously extends AnalyzeBase {
 }
 
 class AnalysisServer {
-  AnalysisServer(this.sdk, this.directories);
+  AnalysisServer(this.sdk, this.directories, { this.flutterRepo: false });
 
   final String sdk;
   final List<String> directories;
+  final bool flutterRepo;
 
   Process _process;
   final StreamController<bool> _analyzingController = new StreamController<bool>.broadcast();
@@ -169,6 +198,13 @@ class AnalysisServer {
       '--sdk',
       sdk,
     ];
+    // Let the analysis server know that the flutter repository is being analyzed
+    // so that it can enable the public_member_api_docs lint even though
+    // the analysis_options file does not have that lint enabled.
+    // It is not enabled in the analysis_option file
+    // because doing so causes too much noise in the IDE.
+    if (flutterRepo)
+      command.add('--flutter-repo');
 
     printTrace('dart ${command.skip(1).join(' ')}');
     _process = await processManager.start(command);

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

@@ -245,25 +245,6 @@ class AnalyzeOnce extends AnalyzeBase {
     return dir;
   }
 
-  List<String> flutterRootComponents;
-  bool isFlutterLibrary(String filename) {
-    flutterRootComponents ??= fs.path.normalize(fs.path.absolute(Cache.flutterRoot)).split(fs.path.separator);
-    final List<String> filenameComponents = fs.path.normalize(fs.path.absolute(filename)).split(fs.path.separator);
-    if (filenameComponents.length < flutterRootComponents.length + 4) // the 4: 'packages', package_name, 'lib', file_name
-      return false;
-    for (int index = 0; index < flutterRootComponents.length; index += 1) {
-      if (flutterRootComponents[index] != filenameComponents[index])
-        return false;
-    }
-    if (filenameComponents[flutterRootComponents.length] != 'packages')
-      return false;
-    if (filenameComponents[flutterRootComponents.length + 1] == 'flutter_tools')
-      return false;
-    if (filenameComponents[flutterRootComponents.length + 2] != 'lib')
-      return false;
-    return true;
-  }
-
   List<File> _collectDartFiles(Directory dir, List<File> collected) {
     // Bail out in case of a .dartignore.
     if (fs.isFileSync(fs.path.join(dir.path, '.dartignore')))

packages/flutter_tools/test/commands/analyze_continuously_test.dart

@@ -23,51 +23,55 @@ void main() {
     tempDir = fs.systemTempDirectory.createTempSync('analysis_test');
   });
 
+  Future<AnalysisServer> analyzeWithServer({ bool brokenCode: false, bool flutterRepo: false, int expectedErrorCount: 0 }) async {
+    _createSampleProject(tempDir, brokenCode: brokenCode);
+
+    await pubGet(directory: tempDir.path);
+
+    server = new AnalysisServer(dartSdkPath, <String>[tempDir.path], flutterRepo: flutterRepo);
+
+    final Future<bool> onDone = server.onAnalyzing.where((bool analyzing) => analyzing == false).first;
+    final List<AnalysisError> errors = <AnalysisError>[];
+    server.onErrors.listen((FileAnalysisErrors fileErrors) {
+      errors.addAll(fileErrors.errors);
+    });
+
+    await server.start();
+    await onDone;
+
+    expect(errors, hasLength(expectedErrorCount));
+    return server;
+  }
+
   tearDown(() {
     tempDir?.deleteSync(recursive: true);
     return server?.dispose();
   });
 
   group('analyze --watch', () {
-    testUsingContext('AnalysisServer success', () async {
-      _createSampleProject(tempDir);
-
-      await pubGet(directory: tempDir.path);
-
-      server = new AnalysisServer(dartSdkPath, <String>[tempDir.path]);
-
-      int errorCount = 0;
-      final Future<bool> onDone = server.onAnalyzing.where((bool analyzing) => analyzing == false).first;
-      server.onErrors.listen((FileAnalysisErrors errors) => errorCount += errors.errors.length);
-
-      await server.start();
-      await onDone;
+  });
 
-      expect(errorCount, 0);
+  group('AnalysisServer', () {
+    testUsingContext('success', () async {
+      server = await analyzeWithServer();
     }, overrides: <Type, Generator>{
       OperatingSystemUtils: () => os
     });
-  });
 
-  testUsingContext('AnalysisServer errors', () async {
-    _createSampleProject(tempDir, brokenCode: true);
-
-    await pubGet(directory: tempDir.path);
-
-    server = new AnalysisServer(dartSdkPath, <String>[tempDir.path]);
-
-    int errorCount = 0;
-    final Future<bool> onDone = server.onAnalyzing.where((bool analyzing) => analyzing == false).first;
-    server.onErrors.listen((FileAnalysisErrors errors) {
-      errorCount += errors.errors.length;
+    testUsingContext('errors', () async {
+      server = await analyzeWithServer(brokenCode: true, expectedErrorCount: 1);
+    }, overrides: <Type, Generator>{
+      OperatingSystemUtils: () => os
     });
 
-    await server.start();
-    await onDone;
-
-    expect(errorCount, greaterThan(0));
-  }, overrides: <Type, Generator>{
-    OperatingSystemUtils: () => os
+    testUsingContext('--flutter-repo', () async {
+      // When a Dart SDK containing support for the --flutter-repo startup flag
+      // https://github.com/dart-lang/sdk/commit/def1ee6604c4b3385b567cb9832af0dbbaf32e0d
+      // is rolled into Flutter, then the expectedErrorCount should be set to 1.
+      server = await analyzeWithServer(flutterRepo: true, expectedErrorCount: 0);
+    }, overrides: <Type, Generator>{
+      OperatingSystemUtils: () => os
+    });
   });
 }
 
@@ -77,12 +81,21 @@ void _createSampleProject(Directory directory, { bool brokenCode: false }) {
 name: foo_project
 ''');
 
+  final File analysisOptionsFile = fs.file(fs.path.join(directory.path, 'analysis_options.yaml'));
+  analysisOptionsFile.writeAsStringSync('''
+linter:
+  rules:
+    - hash_and_equals
+''');
+
   final File dartFile = fs.file(fs.path.join(directory.path, 'lib', 'main.dart'));
   dartFile.parent.createSync();
   dartFile.writeAsStringSync('''
 void main() {
   print('hello world');
   ${brokenCode ? 'prints("hello world");' : ''}
 }
+
+class SomeClassWithoutDartDoc { }
 ''');
 }