Update SOURCE_KIND when a missing source file appears

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

 // Copyright (c) 2015, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 library analyzer.src.context.context;
 
 import 'dart:async';
 import 'dart:collection';
 
 import 'package:analyzer/dart/ast/ast.dart';
@@ skipping 999 matching lines @@
       Source source, String originalContents, String newContents, bool notify) {
     CacheEntry entry = _cache.get(source);
     if (entry == null) {
       return false;
     }
     bool changed = newContents != originalContents;
     if (newContents != null) {
       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);
       } else {
         entry.modificationTime = _contentCache.getModificationStamp(source);
       }
     } else if (originalContents != null) {
       // We are removing the overlay for the file, check if the file's
       // contents is the same as it was in the overlay.
       try {
@@ skipping 716 matching lines @@
     } else {
       entry.explicitlyAdded = true;
       entry.modificationTime = getModificationStamp(source);
       entry.setState(CONTENT, CacheState.INVALID);
     }
   }
 
   /**
    * 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);
         if (fileContents.data == sourceContent) {
           int time = fileContents.modificationTime;
           for (CacheEntry entry in _entriesFor(source)) {
             entry.modificationTime = time;
           }
           return;
         }
@@ skipping 28 matching lines @@
 //              print(
 //                  'dartDelta: add=${dartDelta.addedNames} remove=${dartDelta.removedNames}');
               delta = dartDelta;
               entry.setState(CONTENT, CacheState.INVALID, delta: delta);
               return;
             }
           }
         }
       }
       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) {
       workManager.applyChange(
           Source.EMPTY_LIST, <Source>[source], Source.EMPTY_LIST);
     }
   }
 
   /**
    * Record that the give [source] has been deleted.
@@ skipping 302 matching lines @@
     }
     DartSdk sdk = factory.dartSdk;
     if (sdk == null) {
       throw new IllegalArgumentException(
           "The source factory for an SDK analysis context must have a DartUriResolver");
     }
     return new AnalysisCache(
         <CachePartition>[AnalysisEngine.instance.partitionManager.forSdk(sdk)]);
   }
 }