Commit 48427cb7 authored by Ian Hickson's avatar Ian Hickson Committed by GitHub

Revert "Flutter analyze watch improvements (#11143)" (#11328)

This reverts commit e13e7806.

Turns out that with this patch, we aren't actually catching all
errors. For example, `flutter analyze --flutter-repo --watch` didn't
report errors in `dev/devicelab/test/adb_test.dart`.
parent 8e388482
...@@ -39,25 +39,6 @@ abstract class AnalyzeBase { ...@@ -39,25 +39,6 @@ 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) { void writeBenchmark(Stopwatch stopwatch, int errorCount, int membersMissingDocumentation) {
final String benchmarkOut = 'analysis_benchmark.json'; final String benchmarkOut = 'analysis_benchmark.json';
final Map<String, dynamic> data = <String, dynamic>{ final Map<String, dynamic> data = <String, dynamic>{
......
...@@ -31,20 +31,15 @@ class AnalyzeContinuously extends AnalyzeBase { ...@@ -31,20 +31,15 @@ class AnalyzeContinuously extends AnalyzeBase {
Stopwatch analysisTimer; Stopwatch analysisTimer;
int lastErrorCount = 0; int lastErrorCount = 0;
Status analysisStatus; Status analysisStatus;
bool flutterRepo;
bool showDartDocIssuesIndividually;
@override @override
Future<Null> analyze() async { Future<Null> analyze() async {
List<String> directories; List<String> directories;
flutterRepo = argResults['flutter-repo'] || inRepo(null); if (argResults['dartdocs'])
showDartDocIssuesIndividually = argResults['dartdocs']; throwToolExit('The --dartdocs option is currently not supported when using --watch.');
if (showDartDocIssuesIndividually && !flutterRepo) if (argResults['flutter-repo']) {
throwToolExit('The --dartdocs option is only supported when using --flutter-repo.');
if (flutterRepo) {
final PackageDependencyTracker dependencies = new PackageDependencyTracker(); final PackageDependencyTracker dependencies = new PackageDependencyTracker();
dependencies.checkForConflictingDependencies(repoPackages, dependencies); dependencies.checkForConflictingDependencies(repoPackages, dependencies);
directories = repoPackages.map((Directory dir) => dir.path).toList(); directories = repoPackages.map((Directory dir) => dir.path).toList();
...@@ -57,7 +52,7 @@ class AnalyzeContinuously extends AnalyzeBase { ...@@ -57,7 +52,7 @@ class AnalyzeContinuously extends AnalyzeBase {
analysisTarget = fs.currentDirectory.path; analysisTarget = fs.currentDirectory.path;
} }
final AnalysisServer server = new AnalysisServer(dartSdkPath, directories, flutterRepo: flutterRepo); final AnalysisServer server = new AnalysisServer(dartSdkPath, directories);
server.onAnalyzing.listen((bool isAnalyzing) => _handleAnalysisStatus(server, isAnalyzing)); server.onAnalyzing.listen((bool isAnalyzing) => _handleAnalysisStatus(server, isAnalyzing));
server.onErrors.listen(_handleAnalysisErrors); server.onErrors.listen(_handleAnalysisErrors);
...@@ -87,52 +82,29 @@ class AnalyzeContinuously extends AnalyzeBase { ...@@ -87,52 +82,29 @@ class AnalyzeContinuously extends AnalyzeBase {
logger.printStatus(terminal.clearScreen(), newline: false); logger.printStatus(terminal.clearScreen(), newline: false);
// Remove errors for deleted files, sort, and print errors. // Remove errors for deleted files, sort, and print errors.
final List<AnalysisError> allErrors = <AnalysisError>[]; final List<AnalysisError> errors = <AnalysisError>[];
for (String path in analysisErrors.keys.toList()) { for (String path in analysisErrors.keys.toList()) {
if (fs.isFileSync(path)) { if (fs.isFileSync(path)) {
allErrors.addAll(analysisErrors[path]); errors.addAll(analysisErrors[path]);
} else { } else {
analysisErrors.remove(path); analysisErrors.remove(path);
} }
} }
// Summarize dartdoc issues rather than displaying each individually errors.sort();
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 detailErrors) { for (AnalysisError error in errors) {
printStatus(error.toString()); printStatus(error.toString());
if (error.code != null) if (error.code != null)
printTrace('error code: ${error.code}'); printTrace('error code: ${error.code}');
} }
dumpErrors(detailErrors.map<String>((AnalysisError error) => error.toLegacyString())); dumpErrors(errors.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. // Print an analysis summary.
String errorsMessage; String errorsMessage;
final int issueCount = detailErrors.length; final int issueCount = errors.length;
final int issueDiff = issueCount - lastErrorCount; final int issueDiff = issueCount - lastErrorCount;
lastErrorCount = issueCount; lastErrorCount = issueCount;
...@@ -178,11 +150,10 @@ class AnalyzeContinuously extends AnalyzeBase { ...@@ -178,11 +150,10 @@ class AnalyzeContinuously extends AnalyzeBase {
} }
class AnalysisServer { class AnalysisServer {
AnalysisServer(this.sdk, this.directories, { this.flutterRepo: false }); AnalysisServer(this.sdk, this.directories);
final String sdk; final String sdk;
final List<String> directories; final List<String> directories;
final bool flutterRepo;
Process _process; Process _process;
final StreamController<bool> _analyzingController = new StreamController<bool>.broadcast(); final StreamController<bool> _analyzingController = new StreamController<bool>.broadcast();
...@@ -198,13 +169,6 @@ class AnalysisServer { ...@@ -198,13 +169,6 @@ class AnalysisServer {
'--sdk', '--sdk',
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(' ')}'); printTrace('dart ${command.skip(1).join(' ')}');
_process = await processManager.start(command); _process = await processManager.start(command);
......
...@@ -245,6 +245,25 @@ class AnalyzeOnce extends AnalyzeBase { ...@@ -245,6 +245,25 @@ class AnalyzeOnce extends AnalyzeBase {
return dir; 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) { List<File> _collectDartFiles(Directory dir, List<File> collected) {
// Bail out in case of a .dartignore. // Bail out in case of a .dartignore.
if (fs.isFileSync(fs.path.join(dir.path, '.dartignore'))) if (fs.isFileSync(fs.path.join(dir.path, '.dartignore')))
......
...@@ -23,56 +23,52 @@ void main() { ...@@ -23,56 +23,52 @@ void main() {
tempDir = fs.systemTempDirectory.createTempSync('analysis_test'); tempDir = fs.systemTempDirectory.createTempSync('analysis_test');
}); });
Future<AnalysisServer> analyzeWithServer({ bool brokenCode: false, bool flutterRepo: false, int expectedErrorCount: 0 }) async { tearDown(() {
_createSampleProject(tempDir, brokenCode: brokenCode); tempDir?.deleteSync(recursive: true);
return server?.dispose();
});
group('analyze --watch', () {
testUsingContext('AnalysisServer success', () async {
_createSampleProject(tempDir);
await pubGet(directory: tempDir.path); await pubGet(directory: tempDir.path);
server = new AnalysisServer(dartSdkPath, <String>[tempDir.path], flutterRepo: flutterRepo); server = new AnalysisServer(dartSdkPath, <String>[tempDir.path]);
int errorCount = 0;
final Future<bool> onDone = server.onAnalyzing.where((bool analyzing) => analyzing == false).first; final Future<bool> onDone = server.onAnalyzing.where((bool analyzing) => analyzing == false).first;
final List<AnalysisError> errors = <AnalysisError>[]; server.onErrors.listen((FileAnalysisErrors errors) => errorCount += errors.errors.length);
server.onErrors.listen((FileAnalysisErrors fileErrors) {
errors.addAll(fileErrors.errors);
});
await server.start(); await server.start();
await onDone; await onDone;
expect(errors, hasLength(expectedErrorCount)); expect(errorCount, 0);
return server;
}
tearDown(() {
tempDir?.deleteSync(recursive: true);
return server?.dispose();
});
group('analyze --watch', () {
});
group('AnalysisServer', () {
testUsingContext('success', () async {
server = await analyzeWithServer();
}, overrides: <Type, Generator>{ }, overrides: <Type, Generator>{
OperatingSystemUtils: () => os OperatingSystemUtils: () => os
}); });
});
testUsingContext('errors', () async { testUsingContext('AnalysisServer errors', () async {
server = await analyzeWithServer(brokenCode: true, expectedErrorCount: 1); _createSampleProject(tempDir, brokenCode: true);
}, overrides: <Type, Generator>{
OperatingSystemUtils: () => os 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('--flutter-repo', () async { await server.start();
// When a Dart SDK containing support for the --flutter-repo startup flag await onDone;
// https://github.com/dart-lang/sdk/commit/def1ee6604c4b3385b567cb9832af0dbbaf32e0d
// is rolled into Flutter, then the expectedErrorCount should be set to 1. expect(errorCount, greaterThan(0));
server = await analyzeWithServer(flutterRepo: true, expectedErrorCount: 0);
}, overrides: <Type, Generator>{ }, overrides: <Type, Generator>{
OperatingSystemUtils: () => os OperatingSystemUtils: () => os
}); });
});
} }
void _createSampleProject(Directory directory, { bool brokenCode: false }) { void _createSampleProject(Directory directory, { bool brokenCode: false }) {
...@@ -81,13 +77,6 @@ void _createSampleProject(Directory directory, { bool brokenCode: false }) { ...@@ -81,13 +77,6 @@ void _createSampleProject(Directory directory, { bool brokenCode: false }) {
name: foo_project 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')); final File dartFile = fs.file(fs.path.join(directory.path, 'lib', 'main.dart'));
dartFile.parent.createSync(); dartFile.parent.createSync();
dartFile.writeAsStringSync(''' dartFile.writeAsStringSync('''
...@@ -95,7 +84,5 @@ void main() { ...@@ -95,7 +84,5 @@ void main() {
print('hello world'); print('hello world');
${brokenCode ? 'prints("hello world");' : ''} ${brokenCode ? 'prints("hello world");' : ''}
} }
class SomeClassWithoutDartDoc { }
'''); ''');
} }
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment