Update SOURCE_KIND when a missing source file appears

pkg/analyzer/lib/src/context/context.dart

Index: pkg/analyzer/lib/src/context/context.dart
diff --git a/pkg/analyzer/lib/src/context/context.dart b/pkg/analyzer/lib/src/context/context.dart
index cc1d41e3f9acc5b3b378dab14d5c1d5dfd9d2e08..f0859241f425e71ad3aa474534e46c2da7e9cc28 100644
--- a/pkg/analyzer/lib/src/context/context.dart
+++ b/pkg/analyzer/lib/src/context/context.dart
@@ -1017,7 +1017,9 @@ class AnalysisContextImpl implements InternalAnalysisContext {
       if (changed) {
         if (!analysisOptions.incremental ||
             !_tryPoorMansIncrementalResolution(source, newContents)) {
-          _sourceChanged(source);
+          // Equals check won't work because newContents is already in cache.
+          // Disable it since we know the file changed.

[Brian Wilkerson, 2016/02/26 15:07:32]

Why is the new content in the cache?

I'm guessing that _tryPoorMansIncrementalResolution must do that as a
side-effect, but it seems like there shouldn't be any side-effect if it returns
false. If that's the case, then we should fix _tryPoorMansIncrementalResolution,
which would negate the need for the extra argument to _sourceChanged.

[skybrian, 2016/02/26 18:09:01]

Within the analyzer, the only caller of handleContentsChanged is this method:

  void _contentsChanged(Source source, String contents, bool notify) {
    String originalContents = _contentCache.setContents(source, contents);
    handleContentsChanged(source, originalContents, contents, notify);
  }

The analysis server also calls handleContentsChanged directly. (See below.)

In either case, the content cache has already been updated.

+          _sourceChanged(source, skipIfEqual: false);
         }
         entry.modificationTime = _contentCache.getModificationStamp(source);
         entry.setValue(CONTENT, newContents, TargetedResult.EMPTY_LIST);
@@ -1754,17 +1756,20 @@ class AnalysisContextImpl implements InternalAnalysisContext {
   /**
    * Invalidate the [source] that was changed and any sources that referenced
    * the source before it existed.
+   *
+   * Note: source may be considered "changed" if it was previously missing,
+   * but pointed to by an import or export directive.
    */
-  void _sourceChanged(Source source) {
+  void _sourceChanged(Source source, {skipIfEqual: true}) {
     CacheEntry entry = _cache.get(source);
-    // If the source is removed, we don't care about it.
+    // If the source has no cache entry, there is nothing to invalidate.
     if (entry == null) {
       return;
     }
     // Check whether the content of the source is the same as it was the last
     // time.
     String sourceContent = entry.getValue(CONTENT);
-    if (sourceContent != null) {
+    if (sourceContent != null && skipIfEqual) {
       entry.setState(CONTENT, CacheState.FLUSHED);
       try {
         TimestampedData<String> fileContents = getContents(source);
@@ -1813,6 +1818,9 @@ class AnalysisContextImpl implements InternalAnalysisContext {
         }
       }
       entry.setState(CONTENT, CacheState.INVALID);
+      // Ensure that the SOURCE_KIND is recalculated when a missing
+      // source file is created.
+      entry.setState(MODIFICATION_TIME, CacheState.INVALID);

[Brian Wilkerson, 2016/02/26 15:07:32]

If we're trying to ensure that SOURCE_KIND is recalculated, shouldn't we set its
state to INVALID?

[skybrian, 2016/02/26 18:09:01]

I thought it would be less brittle this way. Since MODIFICATION_TIME has
changed, any derived data that depends on it (including SOURCE_KIND but perhaps
other things) should be rebuilt.

[Brian Wilkerson, 2016/02/26 18:35:57]

It probably doesn't matter here, but in general I'd opt for invalidating as
little as necessary in order to improve performance. But then it might also be
the case that the modification time needs to be invalidated anyway.

In either case, it's worth being explicit, because then if the dependency
structure between results changes this code won't break. So I'd at least add

  entry.setState(SOURCE_KIND, CacheState.INVALID);

even if we don't stop setting the state of the modification time.

     }
     driver.reset();
     for (WorkManager workManager in workManagers) {