Merge pull request #102 from travissanderson-wf/fatal_hints

CP-1140 Initial implementation of failing on analyzer hints

Author: jayudey-wf

README.md

@@ -237,6 +237,12 @@ All configuration options for the `analyze` task are found on the
             <td><code>true</code></td>
             <td>Show hint results.</td>
         </tr>
+        <tr>
+        	<td><code>fatalHints</code></td>
+        	<td><code>bool</code></td>
+        	<td><code>false</code></td>
+        	<td>Fail on hints (requests hints to be true).</td>
+        </tr>
     </tbody>
 </table>
 

lib/src/tasks/analyze/api.dart

@@ -25,7 +25,8 @@ import 'package:dart_dev/src/tasks/task.dart';
 AnalyzeTask analyze(
     {List<String> entryPoints: defaultEntryPoints,
     bool fatalWarnings: defaultFatalWarnings,
-    bool hints: defaultHints}) {
+    bool hints: defaultHints,
+    bool fatalHints: defaultFatalHints}) {
   var executable = 'dartanalyzer';
   var args = [];
   if (fatalWarnings) {
@@ -36,14 +37,30 @@ AnalyzeTask analyze(
   }
   args.addAll(_findFilesFromEntryPoints(entryPoints));
 
+  var postProcessCompleter = new Completer();
   TaskProcess process = new TaskProcess(executable, args);
-  AnalyzeTask task =
-      new AnalyzeTask('$executable ${args.join(' ')}', process.done);
+  AnalyzeTask task = new AnalyzeTask(
+      '$executable ${args.join(' ')}', postProcessCompleter.future);
 
-  process.stdout.listen(task._analyzerOutput.add);
-  process.stderr.listen(task._analyzerOutput.addError);
-  process.exitCode.then((code) {
-    task.successful = code <= 0;
+  var matchingLineFuture = task.analyzerOutput
+      .firstWhere((line) => line.contains('[hint] '), defaultValue: () => null);
+
+  var stdoutDone = process.stdout.listen(task._analyzerOutput.add).asFuture();
+  var stderrDone = process.stderr.listen(task._analyzerOutput.add).asFuture();
+  Future.wait([stdoutDone, stderrDone])
+      .then((_) => task._analyzerOutput.close());
+
+  matchingLineFuture.then((matchingLine) {
+    process.exitCode.then((exitCode) async {
+      if (exitCode > 0) {
+        task.successful = false;
+      } else if (fatalHints) {
+        task.successful = matchingLine == null;
+      } else {
+        task.successful = true;
+      }
+      postProcessCompleter.complete();
+    });
   });
 
   return task;
@@ -72,7 +89,7 @@ class AnalyzeTask extends Task {
   final String analyzerCommand;
   final Future done;
 
-  StreamController<String> _analyzerOutput = new StreamController();
+  StreamController<String> _analyzerOutput = new StreamController.broadcast();
   Stream<String> get analyzerOutput => _analyzerOutput.stream;
   AnalyzeTask(String this.analyzerCommand, Future this.done);
 }

lib/src/tasks/analyze/cli.dart

@@ -32,7 +32,11 @@ class AnalyzeCli extends TaskCli {
         negatable: true,
         help: 'Treat non-type warnings as fatal.')
     ..addFlag('hints',
-        defaultsTo: defaultHints, negatable: true, help: 'Show hint results.');
+        defaultsTo: defaultHints, negatable: true, help: 'Show hint results.')
+    ..addFlag('fatal-hints',
+        defaultsTo: defaultFatalHints,
+        negatable: true,
+        help: 'Treat hints as fatal.');
 
   final String command = 'analyze';
 
@@ -41,10 +45,22 @@ class AnalyzeCli extends TaskCli {
     bool fatalWarnings = TaskCli.valueOf(
         'fatal-warnings', parsedArgs, config.analyze.fatalWarnings);
     bool hints = TaskCli.valueOf('hints', parsedArgs, config.analyze.hints);
+    bool fatalHints =
+        TaskCli.valueOf('fatal-hints', parsedArgs, config.analyze.fatalHints);
+
+    if (!hints && fatalHints) {
+      return new CliResult.fail('You must enable hints to fail on hints.');
+    }
 
     AnalyzeTask task = analyze(
-        entryPoints: entryPoints, fatalWarnings: fatalWarnings, hints: hints);
-    reporter.logGroup(task.analyzerCommand, outputStream: task.analyzerOutput);
+        entryPoints: entryPoints,
+        fatalWarnings: fatalWarnings,
+        hints: hints,
+        fatalHints: fatalHints);
+    var title = task.analyzerCommand;
+    if (fatalHints) title += ' (treating hints as fatal)';
+
+    reporter.logGroup(title, outputStream: task.analyzerOutput);
     await task.done;
     return task.successful
         ? new CliResult.success('Analysis completed.')

lib/src/tasks/analyze/config.dart

@@ -19,9 +19,11 @@ import 'package:dart_dev/src/tasks/config.dart';
 const List<String> defaultEntryPoints = const ['lib/'];
 const bool defaultFatalWarnings = true;
 const bool defaultHints = true;
+const bool defaultFatalHints = false;
 
 class AnalyzeConfig extends TaskConfig {
   List<String> entryPoints = defaultEntryPoints.toList();
   bool fatalWarnings = defaultFatalWarnings;
   bool hints = defaultHints;
+  bool fatalHints = defaultFatalHints;
 }

test/integration/analyze_test.dart

@@ -26,12 +26,16 @@ const String projectWithHints = 'test/fixtures/analyze/hints';
 const String projectWithNoIssues = 'test/fixtures/analyze/no_issues';
 
 Future<Analysis> analyzeProject(String projectPath,
-    {bool fatalWarnings: true, bool hints: true}) async {
+    {bool fatalWarnings: true,
+    bool hints: true,
+    bool fatalHints: false}) async {
   await Process.run('pub', ['get'], workingDirectory: projectPath);
 
   var args = ['run', 'dart_dev', 'analyze'];
   args.add(fatalWarnings ? '--fatal-warnings' : '--no-fatal-warnings');
   args.add(hints ? '--hints' : '--no-hints');
+  args.add(fatalHints ? '--fatal-hints' : ' --no-fatal-hints');
+
   TaskProcess process =
       new TaskProcess('pub', args, workingDirectory: projectPath);
 
@@ -92,16 +96,33 @@ void main() {
     test('should report no issues found', () async {
       Analysis analysis = await analyzeProject(projectWithNoIssues);
       expect(analysis.noIssues, isTrue);
+      expect(analysis.exitCode, equals(0));
     });
 
     test('should report hints if configured to do so', () async {
       Analysis analysis = await analyzeProject(projectWithHints);
       expect(analysis.numHints, equals(2));
+      expect(analysis.exitCode, equals(0));
     });
 
     test('should not report hints if configured to ignore them', () async {
       Analysis analysis = await analyzeProject(projectWithHints, hints: false);
       expect(analysis.numHints, equals(0));
+      expect(analysis.exitCode, equals(0));
+    });
+
+    test('should report hints as fatal if configured to do so', () async {
+      Analysis analysis =
+          await analyzeProject(projectWithHints, fatalHints: true);
+      expect(analysis.numHints, equals(2));
+      expect(analysis.exitCode, greaterThan(0));
+    });
+
+    test('should not report hints as fatal if none existed', () async {
+      Analysis analysis =
+          await analyzeProject(projectWithNoIssues, fatalHints: true);
+      expect(analysis.numHints, equals(0));
+      expect(analysis.exitCode, equals(0));
     });
 
     test('should report warnings as fatal if configured to do so', () async {