Skip to content

Commit 48427cb

Browse files
authored
Revert "Flutter analyze watch improvements (flutter#11143)" (flutter#11328)
This reverts commit e13e780. 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`.
1 parent 8e38848 commit 48427cb

File tree

4 files changed

+62
-111
lines changed

4 files changed

+62
-111
lines changed

packages/flutter_tools/lib/src/commands/analyze_base.dart

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -39,25 +39,6 @@ abstract class AnalyzeBase {
3939
}
4040
}
4141

42-
List<String> flutterRootComponents;
43-
bool isFlutterLibrary(String filename) {
44-
flutterRootComponents ??= fs.path.normalize(fs.path.absolute(Cache.flutterRoot)).split(fs.path.separator);
45-
final List<String> filenameComponents = fs.path.normalize(fs.path.absolute(filename)).split(fs.path.separator);
46-
if (filenameComponents.length < flutterRootComponents.length + 4) // the 4: 'packages', package_name, 'lib', file_name
47-
return false;
48-
for (int index = 0; index < flutterRootComponents.length; index += 1) {
49-
if (flutterRootComponents[index] != filenameComponents[index])
50-
return false;
51-
}
52-
if (filenameComponents[flutterRootComponents.length] != 'packages')
53-
return false;
54-
if (filenameComponents[flutterRootComponents.length + 1] == 'flutter_tools')
55-
return false;
56-
if (filenameComponents[flutterRootComponents.length + 2] != 'lib')
57-
return false;
58-
return true;
59-
}
60-
6142
void writeBenchmark(Stopwatch stopwatch, int errorCount, int membersMissingDocumentation) {
6243
final String benchmarkOut = 'analysis_benchmark.json';
6344
final Map<String, dynamic> data = <String, dynamic>{

packages/flutter_tools/lib/src/commands/analyze_continuously.dart

Lines changed: 11 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -31,20 +31,15 @@ class AnalyzeContinuously extends AnalyzeBase {
3131
Stopwatch analysisTimer;
3232
int lastErrorCount = 0;
3333
Status analysisStatus;
34-
bool flutterRepo;
35-
bool showDartDocIssuesIndividually;
3634

3735
@override
3836
Future<Null> analyze() async {
3937
List<String> directories;
4038

41-
flutterRepo = argResults['flutter-repo'] || inRepo(null);
42-
showDartDocIssuesIndividually = argResults['dartdocs'];
39+
if (argResults['dartdocs'])
40+
throwToolExit('The --dartdocs option is currently not supported when using --watch.');
4341

44-
if (showDartDocIssuesIndividually && !flutterRepo)
45-
throwToolExit('The --dartdocs option is only supported when using --flutter-repo.');
46-
47-
if (flutterRepo) {
42+
if (argResults['flutter-repo']) {
4843
final PackageDependencyTracker dependencies = new PackageDependencyTracker();
4944
dependencies.checkForConflictingDependencies(repoPackages, dependencies);
5045
directories = repoPackages.map((Directory dir) => dir.path).toList();
@@ -57,7 +52,7 @@ class AnalyzeContinuously extends AnalyzeBase {
5752
analysisTarget = fs.currentDirectory.path;
5853
}
5954

60-
final AnalysisServer server = new AnalysisServer(dartSdkPath, directories, flutterRepo: flutterRepo);
55+
final AnalysisServer server = new AnalysisServer(dartSdkPath, directories);
6156
server.onAnalyzing.listen((bool isAnalyzing) => _handleAnalysisStatus(server, isAnalyzing));
6257
server.onErrors.listen(_handleAnalysisErrors);
6358

@@ -87,52 +82,29 @@ class AnalyzeContinuously extends AnalyzeBase {
8782
logger.printStatus(terminal.clearScreen(), newline: false);
8883

8984
// Remove errors for deleted files, sort, and print errors.
90-
final List<AnalysisError> allErrors = <AnalysisError>[];
85+
final List<AnalysisError> errors = <AnalysisError>[];
9186
for (String path in analysisErrors.keys.toList()) {
9287
if (fs.isFileSync(path)) {
93-
allErrors.addAll(analysisErrors[path]);
88+
errors.addAll(analysisErrors[path]);
9489
} else {
9590
analysisErrors.remove(path);
9691
}
9792
}
9893

99-
// Summarize dartdoc issues rather than displaying each individually
100-
int membersMissingDocumentation = 0;
101-
List<AnalysisError> detailErrors;
102-
if (flutterRepo && !showDartDocIssuesIndividually) {
103-
detailErrors = allErrors.where((AnalysisError error) {
104-
if (error.code == 'public_member_api_docs') {
105-
// https://github.com/dart-lang/linter/issues/208
106-
if (isFlutterLibrary(error.file))
107-
membersMissingDocumentation += 1;
108-
return true;
109-
}
110-
return false;
111-
}).toList();
112-
} else {
113-
detailErrors = allErrors;
114-
}
115-
116-
detailErrors.sort();
94+
errors.sort();
11795

118-
for (AnalysisError error in detailErrors) {
96+
for (AnalysisError error in errors) {
11997
printStatus(error.toString());
12098
if (error.code != null)
12199
printTrace('error code: ${error.code}');
122100
}
123101

124-
dumpErrors(detailErrors.map<String>((AnalysisError error) => error.toLegacyString()));
125-
126-
if (membersMissingDocumentation != 0) {
127-
printStatus(membersMissingDocumentation == 1
128-
? '1 public member lacks documentation'
129-
: '$membersMissingDocumentation public members lack documentation');
130-
}
102+
dumpErrors(errors.map<String>((AnalysisError error) => error.toLegacyString()));
131103

132104
// Print an analysis summary.
133105
String errorsMessage;
134106

135-
final int issueCount = detailErrors.length;
107+
final int issueCount = errors.length;
136108
final int issueDiff = issueCount - lastErrorCount;
137109
lastErrorCount = issueCount;
138110

@@ -178,11 +150,10 @@ class AnalyzeContinuously extends AnalyzeBase {
178150
}
179151

180152
class AnalysisServer {
181-
AnalysisServer(this.sdk, this.directories, { this.flutterRepo: false });
153+
AnalysisServer(this.sdk, this.directories);
182154

183155
final String sdk;
184156
final List<String> directories;
185-
final bool flutterRepo;
186157

187158
Process _process;
188159
final StreamController<bool> _analyzingController = new StreamController<bool>.broadcast();
@@ -198,13 +169,6 @@ class AnalysisServer {
198169
'--sdk',
199170
sdk,
200171
];
201-
// Let the analysis server know that the flutter repository is being analyzed
202-
// so that it can enable the public_member_api_docs lint even though
203-
// the analysis_options file does not have that lint enabled.
204-
// It is not enabled in the analysis_option file
205-
// because doing so causes too much noise in the IDE.
206-
if (flutterRepo)
207-
command.add('--flutter-repo');
208172

209173
printTrace('dart ${command.skip(1).join(' ')}');
210174
_process = await processManager.start(command);

packages/flutter_tools/lib/src/commands/analyze_once.dart

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -245,6 +245,25 @@ class AnalyzeOnce extends AnalyzeBase {
245245
return dir;
246246
}
247247

248+
List<String> flutterRootComponents;
249+
bool isFlutterLibrary(String filename) {
250+
flutterRootComponents ??= fs.path.normalize(fs.path.absolute(Cache.flutterRoot)).split(fs.path.separator);
251+
final List<String> filenameComponents = fs.path.normalize(fs.path.absolute(filename)).split(fs.path.separator);
252+
if (filenameComponents.length < flutterRootComponents.length + 4) // the 4: 'packages', package_name, 'lib', file_name
253+
return false;
254+
for (int index = 0; index < flutterRootComponents.length; index += 1) {
255+
if (flutterRootComponents[index] != filenameComponents[index])
256+
return false;
257+
}
258+
if (filenameComponents[flutterRootComponents.length] != 'packages')
259+
return false;
260+
if (filenameComponents[flutterRootComponents.length + 1] == 'flutter_tools')
261+
return false;
262+
if (filenameComponents[flutterRootComponents.length + 2] != 'lib')
263+
return false;
264+
return true;
265+
}
266+
248267
List<File> _collectDartFiles(Directory dir, List<File> collected) {
249268
// Bail out in case of a .dartignore.
250269
if (fs.isFileSync(fs.path.join(dir.path, '.dartignore')))

packages/flutter_tools/test/commands/analyze_continuously_test.dart

Lines changed: 32 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -23,55 +23,51 @@ void main() {
2323
tempDir = fs.systemTempDirectory.createTempSync('analysis_test');
2424
});
2525

26-
Future<AnalysisServer> analyzeWithServer({ bool brokenCode: false, bool flutterRepo: false, int expectedErrorCount: 0 }) async {
27-
_createSampleProject(tempDir, brokenCode: brokenCode);
28-
29-
await pubGet(directory: tempDir.path);
30-
31-
server = new AnalysisServer(dartSdkPath, <String>[tempDir.path], flutterRepo: flutterRepo);
32-
33-
final Future<bool> onDone = server.onAnalyzing.where((bool analyzing) => analyzing == false).first;
34-
final List<AnalysisError> errors = <AnalysisError>[];
35-
server.onErrors.listen((FileAnalysisErrors fileErrors) {
36-
errors.addAll(fileErrors.errors);
37-
});
38-
39-
await server.start();
40-
await onDone;
41-
42-
expect(errors, hasLength(expectedErrorCount));
43-
return server;
44-
}
45-
4626
tearDown(() {
4727
tempDir?.deleteSync(recursive: true);
4828
return server?.dispose();
4929
});
5030

5131
group('analyze --watch', () {
52-
});
32+
testUsingContext('AnalysisServer success', () async {
33+
_createSampleProject(tempDir);
5334

54-
group('AnalysisServer', () {
55-
testUsingContext('success', () async {
56-
server = await analyzeWithServer();
57-
}, overrides: <Type, Generator>{
58-
OperatingSystemUtils: () => os
59-
});
35+
await pubGet(directory: tempDir.path);
36+
37+
server = new AnalysisServer(dartSdkPath, <String>[tempDir.path]);
38+
39+
int errorCount = 0;
40+
final Future<bool> onDone = server.onAnalyzing.where((bool analyzing) => analyzing == false).first;
41+
server.onErrors.listen((FileAnalysisErrors errors) => errorCount += errors.errors.length);
6042

61-
testUsingContext('errors', () async {
62-
server = await analyzeWithServer(brokenCode: true, expectedErrorCount: 1);
43+
await server.start();
44+
await onDone;
45+
46+
expect(errorCount, 0);
6347
}, overrides: <Type, Generator>{
6448
OperatingSystemUtils: () => os
6549
});
50+
});
6651

67-
testUsingContext('--flutter-repo', () async {
68-
// When a Dart SDK containing support for the --flutter-repo startup flag
69-
// https://github.com/dart-lang/sdk/commit/def1ee6604c4b3385b567cb9832af0dbbaf32e0d
70-
// is rolled into Flutter, then the expectedErrorCount should be set to 1.
71-
server = await analyzeWithServer(flutterRepo: true, expectedErrorCount: 0);
72-
}, overrides: <Type, Generator>{
73-
OperatingSystemUtils: () => os
52+
testUsingContext('AnalysisServer errors', () async {
53+
_createSampleProject(tempDir, brokenCode: true);
54+
55+
await pubGet(directory: tempDir.path);
56+
57+
server = new AnalysisServer(dartSdkPath, <String>[tempDir.path]);
58+
59+
int errorCount = 0;
60+
final Future<bool> onDone = server.onAnalyzing.where((bool analyzing) => analyzing == false).first;
61+
server.onErrors.listen((FileAnalysisErrors errors) {
62+
errorCount += errors.errors.length;
7463
});
64+
65+
await server.start();
66+
await onDone;
67+
68+
expect(errorCount, greaterThan(0));
69+
}, overrides: <Type, Generator>{
70+
OperatingSystemUtils: () => os
7571
});
7672
}
7773

@@ -81,21 +77,12 @@ void _createSampleProject(Directory directory, { bool brokenCode: false }) {
8177
name: foo_project
8278
''');
8379

84-
final File analysisOptionsFile = fs.file(fs.path.join(directory.path, 'analysis_options.yaml'));
85-
analysisOptionsFile.writeAsStringSync('''
86-
linter:
87-
rules:
88-
- hash_and_equals
89-
''');
90-
9180
final File dartFile = fs.file(fs.path.join(directory.path, 'lib', 'main.dart'));
9281
dartFile.parent.createSync();
9382
dartFile.writeAsStringSync('''
9483
void main() {
9584
print('hello world');
9685
${brokenCode ? 'prints("hello world");' : ''}
9786
}
98-
99-
class SomeClassWithoutDartDoc { }
10087
''');
10188
}

0 commit comments

Comments
 (0)