Check for workspaces consistency after analysis contexts creation.

Bug: https://github.com/dart-lang/linter/issues/2694
Change-Id: I13cae115e48ae794e0751b6099f864ca9d202cdc
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/203821
Reviewed-by: Brian Wilkerson <[email protected]>
Commit-Queue: Konstantin Shcheglov <[email protected]>

Author: scheglov

pkg/analysis_server/lib/src/context_manager.dart

@@ -175,6 +175,10 @@ class ContextManagerImpl implements ContextManager {
   final Map<Folder, AnalysisDriver> driverMap =
       HashMap<Folder, AnalysisDriver>();
 
+  /// The timer that is started after creating analysis contexts, to check
+  /// for in-between changes to configuration files.
+  Timer? _collectionConsistencyCheckTimer;
+
   /// Stream subscription we are using to watch each analysis root directory for
   /// changes.
   final Map<Folder, StreamSubscription<WatchEvent>> changeSubscriptions =
@@ -461,6 +465,7 @@ class ContextManagerImpl implements ContextManager {
     }
 
     callbacks.afterContextsCreated();
+    _scheduleCollectionConsistencyCheck(collection);
   }
 
   /// Clean up and destroy the context associated with the given folder.
@@ -482,6 +487,7 @@ class ContextManagerImpl implements ContextManager {
   void _destroyAnalysisContexts() {
     var collection = _collection;
     if (collection != null) {
+      _collectionConsistencyCheckTimer?.cancel();
       for (var analysisContext in collection.contexts) {
         _destroyAnalysisContext(analysisContext);
       }
@@ -632,6 +638,28 @@ class ContextManagerImpl implements ContextManager {
         existingExcludedSet.containsAll(excludedPaths);
   }
 
+  /// We create analysis contexts, and then start watching the file system
+  /// for modifications to Dart files, and to configuration files, e.g.
+  /// `pubspec.yaml` file Pub workspaces.
+  ///
+  /// So, it is possible that one of these files will be changed between the
+  /// moment when we read it, and the moment when we started watching for
+  /// changes. Using `package:watcher` before creating analysis contexts
+  /// was still not reliable enough.
+  ///
+  /// To work around this we check after a short timeout, and hope that
+  /// any subsequent changes will be noticed by `package:watcher`.
+  void _scheduleCollectionConsistencyCheck(
+    AnalysisContextCollectionImpl collection,
+  ) {
+    _collectionConsistencyCheckTimer = Timer(Duration(seconds: 1), () {
+      _collectionConsistencyCheckTimer = null;
+      if (!collection.areWorkspacesConsistent) {
+        _createAnalysisContexts();
+      }
+    });
+  }
+
   /// Starts watching for the `bazel-bin` and `blaze-bin` symlinks.
   ///
   /// This is important since these symlinks might not be present when the

pkg/analyzer/lib/src/dart/analysis/analysis_context_collection.dart

@@ -79,6 +79,20 @@ class AnalysisContextCollectionImpl implements AnalysisContextCollection {
     }
   }
 
+  /// Return `true` if the read state of configuration files is consistent
+  /// with their current state on the file system. We use this as a work
+  /// around an issue with watching for file system changes.
+  bool get areWorkspacesConsistent {
+    for (var analysisContext in contexts) {
+      var contextRoot = analysisContext.contextRoot;
+      var workspace = contextRoot.workspace;
+      if (!workspace.isConsistentWithFileSystem) {
+        return false;
+      }
+    }
+    return true;
+  }
+
   @override
   DriverBasedAnalysisContext contextFor(String path) {
     _throwIfNotAbsoluteNormalizedPath(path);

pkg/analyzer/lib/src/workspace/package_build.dart

@@ -145,6 +145,9 @@ class PackageBuildWorkspace extends Workspace implements PubWorkspace {
   /// package:build does it.
   static const String _pubspecName = 'pubspec.yaml';
 
+  /// The associated pubspec file.
+  final File _pubspecFile;
+
   /// The content of the `pubspec.yaml` file.
   /// We read it once, so that all usages return consistent results.
   final String? _pubspecContent;
@@ -185,10 +188,16 @@ class PackageBuildWorkspace extends Workspace implements PubWorkspace {
     this.generatedRootPath,
     this.generatedThisPath,
     File pubspecFile,
-  ) : _pubspecContent = _fileContentOrNull(pubspecFile) {
+  )   : _pubspecFile = pubspecFile,
+        _pubspecContent = _fileContentOrNull(pubspecFile) {
     _theOnlyPackage = PackageBuildWorkspacePackage(root, this);
   }
 
+  @override
+  bool get isConsistentWithFileSystem {
+    return _fileContentOrNull(_pubspecFile) == _pubspecContent;
+  }
+
   @override
   UriResolver get packageUriResolver => PackageBuildPackageUriResolver(
       this, PackageMapUriResolver(provider, packageMap));

pkg/analyzer/lib/src/workspace/pub.dart

@@ -21,6 +21,9 @@ class PubWorkspace extends SimpleWorkspace {
   /// Each Pub workspace is itself one package.
   late final PubWorkspacePackage _theOnlyPackage;
 
+  /// The associated pubspec file.
+  final File _pubspecFile;
+
   /// The content of the `pubspec.yaml` file.
   /// We read it once, so that all usages return consistent results.
   final String? _pubspecContent;
@@ -30,11 +33,17 @@ class PubWorkspace extends SimpleWorkspace {
     Map<String, List<Folder>> packageMap,
     String root,
     File pubspecFile,
-  )   : _pubspecContent = _fileContentOrNull(pubspecFile),
+  )   : _pubspecFile = pubspecFile,
+        _pubspecContent = _fileContentOrNull(pubspecFile),
         super(provider, packageMap, root) {
     _theOnlyPackage = PubWorkspacePackage(root, this);
   }
 
+  @override
+  bool get isConsistentWithFileSystem {
+    return _fileContentOrNull(_pubspecFile) == _pubspecContent;
+  }
+
   @internal
   @override
   void contributeToResolutionSalt(ApiSignature buffer) {

pkg/analyzer/lib/src/workspace/workspace.dart

@@ -17,6 +17,11 @@ abstract class Workspace {
   /// Return true iff this [Workspace] is a [BazelWorkspace].
   bool get isBazel => false;
 
+  /// Return `true` if the read state of configuration files is consistent
+  /// with their current state on the file system.
+  @internal
+  bool get isConsistentWithFileSystem => true;
+
   /// The [UriResolver] that can resolve `package` URIs.
   UriResolver get packageUriResolver;
 

pkg/analyzer/test/src/workspace/package_build_test.dart

@@ -640,6 +640,17 @@ class PackageBuildWorkspaceTest with ResourceProviderMixin {
         workspace.findFile(convertPath('/workspace/web/file.dart')), webFile);
   }
 
+  void test_isConsistentWithFileSystem() {
+    newFolder('/workspace/.dart_tool/build/generated/project/bin');
+    newPubspecYamlFile('/workspace', 'name: project');
+    PackageBuildWorkspace workspace =
+        _createWorkspace('/workspace', ['project']);
+    expect(workspace.isConsistentWithFileSystem, isTrue);
+
+    newPubspecYamlFile('/workspace', 'name: my2');
+    expect(workspace.isConsistentWithFileSystem, isFalse);
+  }
+
   PackageBuildWorkspace _createWorkspace(
       String root, List<String> packageNames) {
     return PackageBuildWorkspace.find(

pkg/analyzer/test/src/workspace/pub_test.dart

@@ -111,4 +111,14 @@ class PubWorkspaceTest with ResourceProviderMixin {
         resourceProvider, {}, convertPath('/workspace/lib/lib1.dart'));
     expect(workspace, isNull);
   }
+
+  void test_isConsistentWithFileSystem() {
+    newPubspecYamlFile('/workspace', 'name: my');
+    var workspace =
+        PubWorkspace.find(resourceProvider, {}, convertPath('/workspace'))!;
+    expect(workspace.isConsistentWithFileSystem, isTrue);
+
+    newPubspecYamlFile('/workspace', 'name: my2');
+    expect(workspace.isConsistentWithFileSystem, isFalse);
+  }
 }