firka/flutter
firka/flutter
5
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

flutter analyze cleanup (#20490)

Browse Source 
* `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
This commit is contained in:
Ian Hickson 2018-08-20 12:51:07 -07:00 committed by GitHub
parent 2ed8a50e7e
commit ad1eaff45a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 234 additions and 134 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
analysis_options.yaml
View File

@ -7,7 +7,9 @@
# See the configuration guide for more
# https://github.com/dart-lang/sdk/tree/master/pkg/analyzer#configuring-the-analyzer
#
# There are four similar analysis options files in the flutter repos:
# There are other similar analysis options files in the flutter repos,
# which should be kept in sync with this file:
#
# - analysis_options.yaml (this file)
# - packages/flutter/lib/analysis_options_user.yaml
# - https://github.com/flutter/plugins/blob/master/analysis_options.yaml
@ -15,9 +17,6 @@
#
# This file contains the analysis options used by Flutter tools, such as IntelliJ,
# Android Studio, and the `flutter analyze` command.
#
# The flutter/plugins repo contains a copy of this file, which should be kept
# in sync with this file.
analyzer:
language:
@ -132,6 +131,7 @@ linter:
- prefer_is_not_empty
- prefer_single_quotes
- prefer_typing_uninitialized_variables
# - public_member_api_docs # enabled on a case-by-case basis; see e.g. packages/analysis_options.yaml
- recursive_getters
- slash_for_doc_comments
- sort_constructors_first

80
dev/bots/analyze-sample-code.dart
View File

@ -99,7 +99,7 @@ class Section {
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 @@ Future<Null> main() async {
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,7 +165,9 @@ Future<Null> main() async {
foundDart = true;
}
}
} else if (line == '// Examples can assume:') {
}
if (!inSampleSection) {
if (line == '// Examples can assume:') {
assert(block.isEmpty);
startLine = new Line(file.path, lineNumber + 1, 3);
inPreamble = true;
@ -174,6 +182,7 @@ Future<Null> main() async {
}
}
}
}
buffer.add('// generated code');
buffer.add('import \'dart:async\';');
buffer.add('import \'dart:convert\';');
@ -189,8 +198,6 @@ Future<Null> main() async {
}
}
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 @@ dependencies:
<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);
if (errors.first.startsWith('Running "flutter packages get" in '))
// rest is stdout
if (errors.isNotEmpty && errors.first == 'Building flutter tool...')
errors.removeAt(0);
if (errors.isNotEmpty && errors.first.startsWith('Running "flutter packages get" in '))
errors.removeAt(0);
int errorCount = 0;
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 RegExp errorPattern = new RegExp('^ +([a-z]+)$kBullet(.+)$kBullet(.+):([0-9]+):([0-9]+)$kBullet([-a-z_]+)\$', caseSensitive: false);
for (String error in errors) {
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) {

6
dev/bots/test.dart
View File

@ -535,11 +535,11 @@ Future<Null> _verifyNoTestPackageImports(String workingDirectory) async {
})
.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 @@ Future<Null> _verifyNoTestPackageImports(String workingDirectory) async {
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 @@ Future<Null> _verifyNoTestPackageImports(String workingDirectory) async {
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'));

43
dev/bots/test/analyze-sample-code-test-input/known_broken_documentation.dart Normal file
View File

@ -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!'),
/// ),
/// ```

67
dev/bots/test/analyze-sample-code_test.dart Normal file
View File

@ -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);
}

2
packages/flutter/lib/src/widgets/text.dart
View File

@ -191,7 +191,7 @@ class DefaultTextStyle extends InheritedWidget {
/// const TextSpan(text: 'world', style: const TextStyle(fontWeight: FontWeight.bold)),
/// ],
/// ),
/// ),
/// )
/// ```
///
/// ## Interactivity

11
packages/flutter_tools/lib/src/commands/analyze.dart
View File

@ -20,8 +20,8 @@ class AnalyzeCommand extends FlutterCommand {
help: 'Analyze the current project, if applicable.', defaultsTo: true);
argParser.addFlag('dartdocs',
negatable: false,
help: 'List every public member that is lacking documentation '
'(only works with --flutter-repo).',
help: 'List every public member that is lacking documentation.\n'
'(The public_member_api_docs lint must be enabled in analysis_options.yaml)',
hide: !verboseHelp);
argParser.addFlag('watch',
help: 'Run analysis continuously, watching the filesystem for changes.',
@ -45,13 +45,14 @@ class AnalyzeCommand extends FlutterCommand {
// Not used by analyze --watch
argParser.addFlag('congratulate',
help: 'When analyzing the flutter repository, show output even when '
'there are no errors, warnings, hints, or lints.',
help: 'Show output even when there are no errors, warnings, hints, or lints.\n'
'Ignored if --watch is specified.',
defaultsTo: true);
argParser.addFlag('preamble',
defaultsTo: true,
help: 'When analyzing the flutter repository, display the number of '
'files that will be analyzed.');
'files that will be analyzed.\n'
'Ignored if --watch is specified.');
}
/// The working directory for testing analysis using dartanalyzer.

36
packages/flutter_tools/lib/src/commands/analyze_continuously.dart
View File

@ -36,9 +36,6 @@ class AnalyzeContinuously extends AnalyzeBase {
Future<Null> analyze() async {
List<String> directories;
if (argResults['dartdocs'])
throwToolExit('The --dartdocs option is currently not supported when using --watch.');
if (argResults['flutter-repo']) {
final PackageDependencyTracker dependencies = new PackageDependencyTracker();
dependencies.checkForConflictingDependencies(repoPackages, dependencies);
@ -96,6 +93,17 @@ 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']) {
errors.removeWhere((AnalysisError error) => error.code == 'public_member_api_docs');
issueCount -= undocumentedMembers;
}
errors.sort();
for (AnalysisError error in errors) {
@ -108,15 +116,9 @@ class AnalyzeContinuously extends AnalyzeBase {
// Print an analysis summary.
String errorsMessage;
int issueCount = errors.length;
final int issueDiff = issueCount - lastErrorCount;
lastErrorCount = issueCount;
final int undocumentedCount = errors.where((AnalysisError issue) {
return issue.code == 'public_member_api_docs';
}).length;
if (firstAnalysis)
errorsMessage = '$issueCount ${pluralize('issue', issueCount)} found';
else if (issueDiff > 0)
@ -128,15 +130,23 @@ class AnalyzeContinuously extends AnalyzeBase {
else
errorsMessage = 'no issues found';
String dartdocMessage;
if (undocumentedMembers == 1) {
dartdocMessage = 'one public member lacks documentation';
} else {
dartdocMessage = '$undocumentedMembers public members lack documentation';
}
final String files = '${analyzedPaths.length} ${pluralize('file', analyzedPaths.length)}';
final String seconds = (analysisTimer.elapsedMilliseconds / 1000.0).toStringAsFixed(2);
if (undocumentedMembers > 0) {
printStatus('$errorsMessage • $dartdocMessage • analyzed $files in $seconds seconds');
} else {
printStatus('$errorsMessage • analyzed $files in $seconds seconds');
}
if (firstAnalysis && isBenchmarking) {
// We don't want to return a failing exit code based on missing documentation.
issueCount -= undocumentedCount;
writeBenchmark(analysisTimer, issueCount, undocumentedCount);
writeBenchmark(analysisTimer, issueCount, undocumentedMembers);
server.dispose().whenComplete(() { exit(issueCount > 0 ? 1 : 0); });
}

78
packages/flutter_tools/lib/src/commands/analyze_once.dart
View File

@ -54,30 +54,18 @@ class AnalyzeOnce extends AnalyzeBase {
if (argResults['flutter-repo']) {
// check for conflicting dependencies
final PackageDependencyTracker dependencies =
new PackageDependencyTracker();
final PackageDependencyTracker dependencies = new PackageDependencyTracker();
dependencies.checkForConflictingDependencies(repoPackages, dependencies);
directories.addAll(repoRoots);
if (argResults.wasParsed('current-package') &&
argResults['current-package']) {
if (argResults.wasParsed('current-package') && argResults['current-package'])
directories.add(currentDirectory);
}
} else {
if (argResults['current-package']) {
if (argResults['current-package'])
directories.add(currentDirectory);
}
}
if (argResults['dartdocs'] && !argResults['flutter-repo']) {
throwToolExit(
'The --dartdocs option is currently only supported with --flutter-repo.');
}
if (directories.isEmpty) {
if (directories.isEmpty)
throwToolExit('Nothing to analyze.', exitCode: 0);
}
// analyze all
final Completer<Null> analysisCompleter = new Completer<Null>();
@ -96,8 +84,6 @@ class AnalyzeOnce extends AnalyzeBase {
}
});
server.onErrors.listen((FileAnalysisErrors fileErrors) {
fileErrors.errors
.removeWhere((AnalysisError error) => error.type == 'TODO');
errors.addAll(fileErrors.errors);
});
@ -123,62 +109,50 @@ class AnalyzeOnce extends AnalyzeBase {
progress?.cancel();
timer.stop();
// report dartdocs
int undocumentedMembers = 0;
if (argResults['flutter-repo']) {
undocumentedMembers = errors.where((AnalysisError error) {
// count missing dartdocs
final int undocumentedMembers = errors.where((AnalysisError error) {
return error.code == 'public_member_api_docs';
}).length;
if (!argResults['dartdocs']) {
errors.removeWhere(
(AnalysisError error) => error.code == 'public_member_api_docs');
}
}
if (!argResults['dartdocs'])
errors.removeWhere((AnalysisError error) => error.code == 'public_member_api_docs');
// emit benchmarks
if (isBenchmarking) {
if (isBenchmarking)
writeBenchmark(timer, errors.length, undocumentedMembers);
}
// report results
dumpErrors(
errors.map<String>((AnalysisError error) => error.toLegacyString()));
// --write
dumpErrors(errors.map<String>((AnalysisError error) => error.toLegacyString()));
if (errors.isNotEmpty && argResults['preamble']) {
// report errors
if (errors.isNotEmpty && argResults['preamble'])
printStatus('');
}
errors.sort();
for (AnalysisError error in errors) {
for (AnalysisError error in errors)
printStatus(error.toString());
}
final String seconds =
(timer.elapsedMilliseconds / 1000.0).toStringAsFixed(1);
final String seconds = (timer.elapsedMilliseconds / 1000.0).toStringAsFixed(1);
String dartdocMessage;
if (undocumentedMembers == 1) {
dartdocMessage = 'one public member lacks documentation';
} else {
dartdocMessage = '$undocumentedMembers public members lack documentation';
}
// 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;
printStatus('');
printStatus(
'${errors.length} ${pluralize('issue', errors.length)} found. (ran in ${seconds}s)');
if (undocumentedMembers > 0) {
throwToolExit('[lint] $undocumentedMembers public '
'${ undocumentedMembers == 1
? "member lacks"
: "members lack" } documentation');
throwToolExit('$errorCount ${pluralize('issue', errorCount)} found. (ran in ${seconds}s; $dartdocMessage)');
} else {
throwToolExit(null);
throwToolExit('$errorCount ${pluralize('issue', errorCount)} found. (ran in ${seconds}s)');
}
}
if (argResults['congratulate']) {
if (undocumentedMembers > 0) {
printStatus('No issues found! (ran in ${seconds}s; '
'$undocumentedMembers public ${ undocumentedMembers ==
1 ? "member lacks" : "members lack" } documentation)');
printStatus('No issues found! (ran in ${seconds}s; $dartdocMessage)');
} else {
printStatus('No issues found! (ran in ${seconds}s)');
}

3
packages/flutter_tools/lib/src/dart/analysis.dart
View File

@ -201,7 +201,8 @@ class AnalysisError implements Comparable<AnalysisError> {
String toString() {
return '${severity.toLowerCase().padLeft(7)} $_separator '
'$messageSentenceFragment $_separator '
'${fs.path.relative(file)}:$startLine:$startColumn';
'${fs.path.relative(file)}:$startLine:$startColumn $_separator '
'$code';
}
String toLegacyString() {

6
packages/flutter_tools/test/commands/analyze_once_test.dart
View File

@ -95,8 +95,8 @@ void main() {
'Analyzing',
'warning $analyzerSeparator The parameter \'onPressed\' is required',
'info $analyzerSeparator The method \'_incrementCounter\' isn\'t used',
'2 issues found.',
],
exitMessageContains: '2 issues found.',
toolExit: true,
);
}, timeout: allowForSlowAnalyzeTests);
@ -122,8 +122,8 @@ void main() {
'warning $analyzerSeparator The parameter \'onPressed\' is required',
'info $analyzerSeparator The method \'_incrementCounter\' isn\'t used',
'info $analyzerSeparator Only throw instances of classes extending either Exception or Error',
'3 issues found.',
],
exitMessageContains: '3 issues found.',
toolExit: true,
);
}, timeout: allowForSlowAnalyzeTests);
@ -153,8 +153,8 @@ void bar() {
arguments: <String>['analyze'],
statusTextContains: <String>[
'Analyzing',
'1 issue found.',
],
exitMessageContains: '1 issue found.',
toolExit: true,
);
} finally {