Improve sending/flushing of errors for files in dot folders
Ensure errors are only sent when there is an appropriate time to flush
them (plus ensure if of those places flushes them).
Fixes https://github.com/dart-lang/sdk/issues/33778.
Change-Id: Ieb4a2631c24649ba013eda93808b003b0e2ddb95
Bug: https://github.com/dart-lang/sdk/issues/33778
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/101822
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
diff --git a/pkg/analysis_server/lib/src/analysis_server.dart b/pkg/analysis_server/lib/src/analysis_server.dart
index 4eb8f59..a48bca9 100644
--- a/pkg/analysis_server/lib/src/analysis_server.dart
+++ b/pkg/analysis_server/lib/src/analysis_server.dart
@@ -576,7 +576,11 @@
/// Returns `true` if errors should be reported for [file] with the given
/// absolute path.
bool shouldSendErrorsNotificationFor(String file) {
- return contextManager.isInAnalysisRoot(file);
+ // Errors should not be reported for things that are explicitly skipped
+ // during normal analysis (for example dot folders are skipped over in
+ // _handleWatchEventImpl).
+ return contextManager.isInAnalysisRoot(file) &&
+ !contextManager.isContainedInDotFolder(file);
}
Future<void> shutdown() {
diff --git a/pkg/analysis_server/lib/src/context_manager.dart b/pkg/analysis_server/lib/src/context_manager.dart
index 5066368..fa1d36f 100644
--- a/pkg/analysis_server/lib/src/context_manager.dart
+++ b/pkg/analysis_server/lib/src/context_manager.dart
@@ -287,6 +287,12 @@
List<AnalysisDriver> getDriversInAnalysisRoot(Folder analysisRoot);
/**
+ * Determine whether the given [path], when interpreted relative to innermost
+ * context root, contains a folder whose name starts with '.'.
+ */
+ bool isContainedInDotFolder(String path);
+
+ /**
* Return `true` if the given [path] is ignored by a [ContextInfo] whose
* folder contains it.
*/
@@ -556,6 +562,15 @@
return drivers;
}
+ /**
+ * Determine whether the given [path], when interpreted relative to innermost
+ * context root, contains a folder whose name starts with '.'.
+ */
+ bool isContainedInDotFolder(String path) {
+ ContextInfo info = _getInnermostContextInfoFor(path);
+ return info != null && _isContainedInDotFolder(info.folder.path, path);
+ }
+
@override
bool isIgnored(String path) {
ContextInfo info = rootInfo;
@@ -870,7 +885,7 @@
}
/**
- * Use the given analysis [driver] to analyze the content of the
+ * Use the given analysis [driver] to analyze the content of the
* AndroidManifest file at the given [path].
*/
void _analyzeManifestFile(AnalysisDriver driver, String path) {
diff --git a/pkg/analysis_server/test/analysis/notification_errors_test.dart b/pkg/analysis_server/test/analysis/notification_errors_test.dart
index ffec72b..d344bc9 100644
--- a/pkg/analysis_server/test/analysis/notification_errors_test.dart
+++ b/pkg/analysis_server/test/analysis/notification_errors_test.dart
@@ -29,6 +29,12 @@
if (notification.event == ANALYSIS_NOTIFICATION_ERRORS) {
var decoded = new AnalysisErrorsParams.fromNotification(notification);
filesErrors[decoded.file] = decoded.errors;
+ } else if (notification.event == ANALYSIS_NOTIFICATION_FLUSH_RESULTS) {
+ var decoded =
+ new AnalysisFlushResultsParams.fromNotification(notification);
+ for (var file in decoded.files) {
+ filesErrors[file] = null;
+ }
}
}
@@ -125,6 +131,55 @@
expect(errors, isNull);
}
+ test_dotFolder_priority() async {
+ // Files inside dotFolders should not generate error notifications even
+ // if they are added to priority (priority affects only priority, not what
+ // is analyzed).
+ createProject();
+ addTestFile('');
+ String brokenFile =
+ newFile(join(projectPath, '.dart_tool/broken.dart'), content: 'err')
+ .path;
+
+ await waitForTasksFinished();
+ await pumpEventQueue(times: 5000);
+ expect(filesErrors[brokenFile], isNull);
+
+ // Add to priority files and give chance for the file to be analyzed (if
+ // it would).
+ await setPriorityFiles([brokenFile]);
+ await waitForTasksFinished();
+ await pumpEventQueue(times: 5000);
+
+ // There should still be no errors.
+ expect(filesErrors[brokenFile], isNull);
+ }
+
+ test_dotFolder_unopenedFile() async {
+ // Files inside dotFolders are not analyzed. Sending requests that cause
+ // them to be opened (such as hovers) should not result in error notifications
+ // because there is no event that would flush them and they'd remain in the
+ // editor forever.
+ createProject();
+ addTestFile('');
+ String brokenFile =
+ newFile(join(projectPath, '.dart_tool/broken.dart'), content: 'err')
+ .path;
+
+ await waitForTasksFinished();
+ await pumpEventQueue(times: 5000);
+ expect(filesErrors[brokenFile], isNull);
+
+ // Send a getHover request for the file that will cause it to be read from disk.
+ await waitResponse(
+ new AnalysisGetHoverParams(brokenFile, 0).toRequest('0'));
+ await waitForTasksFinished();
+ await pumpEventQueue(times: 5000);
+
+ // There should be no errors because the file is not being analyzed.
+ expect(filesErrors[brokenFile], isNull);
+ }
+
test_importError() async {
createProject();
@@ -191,6 +246,108 @@
expect(filesErrors[otherFile], isNull);
}
+ test_overlay_dotFolder() async {
+ // Files inside dotFolders should not generate error notifications even
+ // if they have overlays added.
+ createProject();
+ addTestFile('');
+ String brokenFile =
+ newFile(join(projectPath, '.dart_tool/broken.dart'), content: 'err')
+ .path;
+
+ await waitForTasksFinished();
+ await pumpEventQueue(times: 5000);
+ expect(filesErrors[brokenFile], isNull);
+
+ // Add and overlay and give chance for the file to be analyzed (if
+ // it would).
+ await waitResponse(
+ new AnalysisUpdateContentParams({
+ brokenFile: new AddContentOverlay('err'),
+ }).toRequest('1'),
+ );
+ await waitForTasksFinished();
+ await pumpEventQueue(times: 5000);
+
+ // There should still be no errors.
+ expect(filesErrors[brokenFile], isNull);
+ }
+
+ test_overlay_newFile() async {
+ // Overlays added for files that don't exist on disk should still generate
+ // error notifications. Removing the overlay if the file is not on disk
+ // should clear the errors.
+ createProject();
+ addTestFile('');
+ String brokenFile = convertPath(join(projectPath, 'broken.dart'));
+
+ // Add and overlay and give chance for the file to be analyzed.
+ await waitResponse(
+ new AnalysisUpdateContentParams({
+ brokenFile: new AddContentOverlay('err'),
+ }).toRequest('0'),
+ );
+ await waitForTasksFinished();
+ await pumpEventQueue(times: 5000);
+
+ // There should now be errors.
+ expect(filesErrors[brokenFile], hasLength(greaterThan(0)));
+
+ // Remove the overlay (this file no longer exists anywhere).
+ await waitResponse(
+ new AnalysisUpdateContentParams({
+ brokenFile: new RemoveContentOverlay(),
+ }).toRequest('1'),
+ );
+ await waitForTasksFinished();
+ await pumpEventQueue(times: 5000);
+
+ // Unlike other tests here, removing an overlay for a file that doesn't exist
+ // on disk doesn't flush errors, but re-analyzes the missing file, which results
+ // in an error notification of 0 errors rather than a flush.
+ expect(filesErrors[brokenFile], isEmpty);
+ }
+
+ test_overlay_newFileSavedBeforeRemoving() async {
+ // Overlays added for files that don't exist on disk should still generate
+ // error notifications. If the file is subsequently saved to disk before the
+ // overlay is removed, the errors should not be flushed when the overlay is
+ // removed.
+ createProject();
+ addTestFile('');
+ String brokenFile = convertPath(join(projectPath, 'broken.dart'));
+
+ // Add and overlay and give chance for the file to be analyzed.
+ await waitResponse(
+ new AnalysisUpdateContentParams({
+ brokenFile: new AddContentOverlay('err'),
+ }).toRequest('0'),
+ );
+ await waitForTasksFinished();
+ await pumpEventQueue(times: 5000);
+
+ // There should now be errors.
+ expect(filesErrors[brokenFile], hasLength(greaterThan(0)));
+
+ // Write the file to disk.
+ newFile(brokenFile, content: 'err');
+ await waitForTasksFinished();
+ await pumpEventQueue(times: 5000);
+
+ // Remove the overlay.
+ await waitResponse(
+ new AnalysisUpdateContentParams({
+ brokenFile: new RemoveContentOverlay(),
+ }).toRequest('1'),
+ );
+ await waitForTasksFinished();
+ await pumpEventQueue(times: 5000);
+
+ // Errors should not have been flushed since the file still exists without
+ // the overlay.
+ expect(filesErrors[brokenFile], hasLength(greaterThan(0)));
+ }
+
test_ParserError() async {
createProject();
addTestFile('library lib');
