Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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>

Expand Down
33 changes: 25 additions & 8 deletions lib/src/tasks/analyze/api.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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;
Expand Down Expand Up @@ -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);
}
22 changes: 19 additions & 3 deletions lib/src/tasks/analyze/cli.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand All @@ -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) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

might be better to implement hints as an option instead of a flag with values of 'none', 'enabled', 'fatal'?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

might be better to implement hints as an option instead of a flag with values of 'none', 'enabled', 'fatal'?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm torn on this. I think I like that idea, but it could be confusing to make --hints function differently than the dartanalyzer.

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)';
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since fatal hints aren't an option for dartanalyzer (yet) I just wanted to tack this on to the output to differentiate it

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since fatal hints aren't an option for dartanalyzer (yet) I just wanted to tack this on to the output to differentiate it from non-fatal hints

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍


reporter.logGroup(title, outputStream: task.analyzerOutput);
await task.done;
return task.successful
? new CliResult.success('Analysis completed.')
Expand Down
2 changes: 2 additions & 0 deletions lib/src/tasks/analyze/config.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
23 changes: 22 additions & 1 deletion test/integration/analyze_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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 {
Expand Down