`flutter analyze` cleanup (#20490)
* `flutter analyze` cleanup
* Make `--dartdocs` work in all modes.
* Make `analyze-sample-code.dart` more resilient.
* Add a test for `analyze-sample-code.dart`.
* Minor cleanup in related code and files.
* Apply review comments
* Fix tests
diff --git a/dev/bots/analyze-sample-code.dart b/dev/bots/analyze-sample-code.dart
index ecc1491..4507ebb 100644
--- a/dev/bots/analyze-sample-code.dart
+++ b/dev/bots/analyze-sample-code.dart
@@ -99,7 +99,7 @@
const String kDartDocPrefix = '///';
const String kDartDocPrefixWithSpace = '$kDartDocPrefix ';
-Future<Null> main() async {
+Future<Null> main(List<String> arguments) async {
final Directory tempDir = Directory.systemTemp.createTempSync('flutter_analyze_sample_code.');
int exitCode = 1;
bool keepMain = false;
@@ -107,7 +107,13 @@
try {
final File mainDart = new File(path.join(tempDir.path, 'main.dart'));
final File pubSpec = new File(path.join(tempDir.path, 'pubspec.yaml'));
- final Directory flutterPackage = new Directory(path.join(_flutterRoot, 'packages', 'flutter', 'lib'));
+ Directory flutterPackage;
+ if (arguments.length == 1) {
+ // Used for testing.
+ flutterPackage = new Directory(arguments.single);
+ } else {
+ flutterPackage = new Directory(path.join(_flutterRoot, 'packages', 'flutter', 'lib'));
+ }
final List<Section> sections = <Section>[];
int sampleCodeSections = 0;
for (FileSystemEntity file in flutterPackage.listSync(recursive: true, followLinks: false)) {
@@ -159,17 +165,20 @@
foundDart = true;
}
}
- } else if (line == '// Examples can assume:') {
- assert(block.isEmpty);
- startLine = new Line(file.path, lineNumber + 1, 3);
- inPreamble = true;
- } else if (trimmedLine == '/// ## Sample code' ||
- trimmedLine.startsWith('/// ## Sample code:') ||
- trimmedLine == '/// ### Sample code' ||
- trimmedLine.startsWith('/// ### Sample code:')) {
- inSampleSection = true;
- foundDart = false;
- sampleCodeSections += 1;
+ }
+ if (!inSampleSection) {
+ if (line == '// Examples can assume:') {
+ assert(block.isEmpty);
+ startLine = new Line(file.path, lineNumber + 1, 3);
+ inPreamble = true;
+ } else if (trimmedLine == '/// ## Sample code' ||
+ trimmedLine.startsWith('/// ## Sample code:') ||
+ trimmedLine == '/// ### Sample code' ||
+ trimmedLine.startsWith('/// ### Sample code:')) {
+ inSampleSection = true;
+ foundDart = false;
+ sampleCodeSections += 1;
+ }
}
}
}
@@ -189,8 +198,6 @@
}
}
buffer.add('');
- buffer.add('// ignore_for_file: unused_element');
- buffer.add('');
final List<Line> lines = new List<Line>.filled(buffer.length, null, growable: true);
for (Section section in sections) {
buffer.addAll(section.strings);
@@ -212,50 +219,47 @@
<String>['analyze', '--no-preamble', '--no-congratulate', mainDart.parent.path],
workingDirectory: tempDir.path,
);
- stderr.addStream(process.stderr);
- final List<String> errors = await process.stdout.transform<String>(utf8.decoder).transform<String>(const LineSplitter()).toList();
- if (errors.first == 'Building flutter tool...')
+ final List<String> errors = <String>[];
+ errors.addAll(await process.stderr.transform<String>(utf8.decoder).transform<String>(const LineSplitter()).toList());
+ errors.add(null);
+ errors.addAll(await process.stdout.transform<String>(utf8.decoder).transform<String>(const LineSplitter()).toList());
+ // top is stderr
+ if (errors.isNotEmpty && (errors.first.contains(' issues found. (ran in ') || errors.first.contains(' issue found. (ran in '))) {
+ errors.removeAt(0); // the "23 issues found" message goes onto stderr, which is concatenated first
+ if (errors.isNotEmpty && errors.last.isEmpty)
+ errors.removeLast(); // if there's an "issues found" message, we put a blank line on stdout before it
+ }
+ // null separates stderr from stdout
+ if (errors.first != null)
+ throw 'cannot analyze dartdocs; unexpected error output: $errors';
+ errors.removeAt(0);
+ // rest is stdout
+ if (errors.isNotEmpty && errors.first == 'Building flutter tool...')
errors.removeAt(0);
- if (errors.first.startsWith('Running "flutter packages get" in '))
+ if (errors.isNotEmpty && errors.first.startsWith('Running "flutter packages get" in '))
errors.removeAt(0);
int errorCount = 0;
+ final String kBullet = Platform.isWindows ? ' - ' : ' • ';
+ final RegExp errorPattern = new RegExp('^ +([a-z]+)$kBullet(.+)$kBullet(.+):([0-9]+):([0-9]+)$kBullet([-a-z_]+)\$', caseSensitive: false);
for (String error in errors) {
- final String kBullet = Platform.isWindows ? ' - ' : ' • ';
- const String kColon = ':';
- final RegExp atRegExp = new RegExp(r' at .*main.dart:');
- final int start = error.indexOf(kBullet);
- final int end = error.indexOf(atRegExp);
- if (start >= 0 && end >= 0) {
- final String message = error.substring(start + kBullet.length, end);
- final String atMatch = atRegExp.firstMatch(error)[0];
- final int colon2 = error.indexOf(kColon, end + atMatch.length);
- if (colon2 < 0) {
+ final Match parts = errorPattern.matchAsPrefix(error);
+ if (parts != null) {
+ final String message = parts[2];
+ final String file = parts[3];
+ final String line = parts[4];
+ final String column = parts[5];
+ final String errorCode = parts[6];
+ final int lineNumber = int.parse(line, radix: 10);
+ final int columnNumber = int.parse(column, radix: 10);
+ if (file != 'main.dart') {
keepMain = true;
- throw 'failed to parse error message: $error';
- }
- final String line = error.substring(end + atMatch.length, colon2);
- final int bullet2 = error.indexOf(kBullet, colon2);
- if (bullet2 < 0) {
- keepMain = true;
- throw 'failed to parse error message: $error';
- }
- final String column = error.substring(colon2 + kColon.length, bullet2);
-
- final int lineNumber = int.tryParse(line, radix: 10);
-
- final int columnNumber = int.tryParse(column, radix: 10);
- if (lineNumber == null) {
- throw 'failed to parse error message: $error';
- }
- if (columnNumber == null) {
- throw 'failed to parse error message: $error';
+ throw 'cannot analyze dartdocs; analysis errors exist in $file: $error';
}
if (lineNumber < 1 || lineNumber > lines.length) {
keepMain = true;
throw 'failed to parse error message (read line number as $lineNumber; total number of lines is ${lines.length}): $error';
}
final Line actualLine = lines[lineNumber - 1];
- final String errorCode = error.substring(bullet2 + kBullet.length);
if (errorCode == 'unused_element') {
// We don't really care if sample code isn't used!
} else if (actualLine == null) {
diff --git a/dev/bots/test.dart b/dev/bots/test.dart
index 5a2a4a7..5492210 100644
--- a/dev/bots/test.dart
+++ b/dev/bots/test.dart
@@ -535,11 +535,11 @@
})
.map<String>((FileSystemEntity entity) {
final File file = entity;
- final String data = file.readAsStringSync();
final String name = path.relative(file.path, from: workingDirectory);
if (name.startsWith('bin/cache') ||
name == 'dev/bots/test.dart')
return null;
+ final String data = file.readAsStringSync();
if (data.contains("import 'package:test/test.dart'")) {
if (data.contains("// Defines a 'package:test' shim.")) {
shims.add(' $name');
@@ -578,7 +578,7 @@
if (count == 1)
return null;
}
- return ' $name: uses \'package:test\' directly.';
+ return ' $name: uses \'package:test\' directly';
}
})
.where((String line) => line != null)
@@ -590,7 +590,7 @@
print('$red━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━$reset');
final String s1 = errors.length == 1 ? 's' : '';
final String s2 = errors.length == 1 ? '' : 's';
- print('${bold}The following file$s2 depend$s1 on \'package:test\' directly:$reset');
+ print('${bold}The following file$s2 use$s1 \'package:test\' incorrectly:$reset');
print(errors.join('\n'));
print('Rather than depending on \'package:test\' directly, use one of the shims:');
print(shims.join('\n'));
diff --git a/dev/bots/test/analyze-sample-code-test-input/known_broken_documentation.dart b/dev/bots/test/analyze-sample-code-test-input/known_broken_documentation.dart
new file mode 100644
index 0000000..bce048c
--- /dev/null
+++ b/dev/bots/test/analyze-sample-code-test-input/known_broken_documentation.dart
@@ -0,0 +1,43 @@
+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// This file is used by ../analyze-sample-code_test.dart, which depends on the
+// precise contents (including especially the comments) of this file.
+
+// Examples can assume:
+// bool _visible = true;
+
+/// A blabla that blabla its blabla blabla blabla.
+///
+/// Bla blabla blabla its blabla into an blabla blabla and then blabla the
+/// blabla back into the blabla blabla blabla.
+///
+/// Bla blabla of blabla blabla than 0.0 and 1.0, this blabla is blabla blabla
+/// blabla it blabla pirates blabla the blabla into of blabla blabla. Bla the
+/// blabla 0.0, the penzance blabla is blabla not blabla at all. Bla the blabla
+/// 1.0, the blabla is blabla blabla blabla an blabla blabla.
+///
+/// ### Sample code
+///
+/// Bla blabla blabla some [Text] when the `_blabla` blabla blabla is true, and
+/// blabla it when it is blabla:
+///
+/// ```dart
+/// new Opacity(
+/// opacity: _visible ? 1.0 : 0.0,
+/// child: const Text('Poor wandering ones!'),
+/// )
+/// ```
+///
+/// ## Sample code
+///
+/// Bla blabla blabla some [Text] when the `_blabla` blabla blabla is true, and
+/// blabla finale blabla:
+///
+/// ```dart
+/// new Opacity(
+/// opacity: _visible ? 1.0 : 0.0,
+/// child: const Text('Poor wandering ones!'),
+/// ),
+/// ```
diff --git a/dev/bots/test/analyze-sample-code_test.dart b/dev/bots/test/analyze-sample-code_test.dart
new file mode 100644
index 0000000..0774eea
--- /dev/null
+++ b/dev/bots/test/analyze-sample-code_test.dart
@@ -0,0 +1,67 @@
+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+import 'dart:convert';
+import 'dart:io';
+
+import 'common.dart';
+
+void main() {
+ test('analyze-sample-code', () async {
+ final Process process = await Process.start(
+ '../../bin/cache/dart-sdk/bin/dart',
+ <String>['analyze-sample-code.dart', 'test/analyze-sample-code-test-input'],
+ );
+ final List<String> stdout = await process.stdout.transform(utf8.decoder).transform(const LineSplitter()).toList();
+ final List<String> stderr = await process.stderr.transform(utf8.decoder).transform(const LineSplitter()).toList();
+ final Match line = new RegExp(r'^(.+)/main\.dart:[0-9]+:[0-9]+: .+$').matchAsPrefix(stdout[1]);
+ expect(line, isNot(isNull));
+ final String directory = line.group(1);
+ new Directory(directory).deleteSync(recursive: true);
+ expect(await process.exitCode, 1);
+ expect(stderr, isEmpty);
+ expect(stdout, <String>[
+ 'Found 2 sample code sections.',
+ "$directory/main.dart:1:8: Unused import: 'dart:async'",
+ "$directory/main.dart:2:8: Unused import: 'dart:convert'",
+ "$directory/main.dart:3:8: Unused import: 'dart:math'",
+ "$directory/main.dart:4:8: Unused import: 'dart:typed_data'",
+ "$directory/main.dart:5:8: Unused import: 'dart:ui'",
+ "$directory/main.dart:6:8: Unused import: 'package:flutter_test/flutter_test.dart'",
+ "$directory/main.dart:9:8: Target of URI doesn't exist: 'package:flutter/known_broken_documentation.dart'",
+ "test/analyze-sample-code-test-input/known_broken_documentation.dart:27:9: Undefined class 'Opacity' (undefined_class)",
+ "test/analyze-sample-code-test-input/known_broken_documentation.dart:29:20: Undefined class 'Text' (undefined_class)",
+ "test/analyze-sample-code-test-input/known_broken_documentation.dart:39:9: Undefined class 'Opacity' (undefined_class)",
+ "test/analyze-sample-code-test-input/known_broken_documentation.dart:41:20: Undefined class 'Text' (undefined_class)",
+ 'test/analyze-sample-code-test-input/known_broken_documentation.dart:42:5: unexpected comma at end of sample code',
+ 'Kept $directory because it had errors (see above).',
+ '-------8<-------',
+ ' 1: // generated code',
+ " 2: import 'dart:async';",
+ " 3: import 'dart:convert';",
+ " 4: import 'dart:math' as math;",
+ " 5: import 'dart:typed_data';",
+ " 6: import 'dart:ui' as ui;",
+ " 7: import 'package:flutter_test/flutter_test.dart';",
+ ' 8: ',
+ ' 9: // test/analyze-sample-code-test-input/known_broken_documentation.dart',
+ " 10: import 'package:flutter/known_broken_documentation.dart';",
+ ' 11: ',
+ ' 12: bool _visible = true;',
+ ' 13: dynamic expression1 = ',
+ ' 14: new Opacity(',
+ ' 15: opacity: _visible ? 1.0 : 0.0,',
+ " 16: child: const Text('Poor wandering ones!'),",
+ ' 17: )',
+ ' 18: ;',
+ ' 19: dynamic expression2 = ',
+ ' 20: new Opacity(',
+ ' 21: opacity: _visible ? 1.0 : 0.0,',
+ " 22: child: const Text('Poor wandering ones!'),",
+ ' 23: ),',
+ ' 24: ;',
+ '-------8<-------',
+ ]);
+ }, skip: !Platform.isLinux);
+}
