Fix memory leak in incremental resolver

pkg/analyzer/test/generated/incremental_resolver_test.dart

 // Copyright (c) 2014, 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.test.generated.incremental_resolver_test;
 
 import 'package:analyzer/dart/ast/ast.dart';
 import 'package:analyzer/dart/element/element.dart';
 import 'package:analyzer/src/context/cache.dart';
 import 'package:analyzer/src/dart/ast/utilities.dart';
 import 'package:analyzer/src/dart/element/builder.dart';
 import 'package:analyzer/src/dart/element/element.dart';
 import 'package:analyzer/src/generated/engine.dart';
 import 'package:analyzer/src/generated/error.dart';
 import 'package:analyzer/src/generated/incremental_logger.dart' as logging;
 import 'package:analyzer/src/generated/incremental_resolution_validator.dart';
 import 'package:analyzer/src/generated/incremental_resolver.dart';
 import 'package:analyzer/src/generated/java_engine.dart';
 import 'package:analyzer/src/generated/parser.dart';
 import 'package:analyzer/src/generated/resolver.dart';
 import 'package:analyzer/src/generated/scanner.dart';
 import 'package:analyzer/src/generated/source_io.dart';
 import 'package:analyzer/src/generated/testing/ast_factory.dart';
 import 'package:analyzer/src/generated/testing/element_factory.dart';
+import 'package:analyzer/src/generated/utilities_collection.dart';
 import 'package:analyzer/src/task/dart.dart';
 import 'package:analyzer/task/dart.dart';
+import 'package:analyzer/task/model.dart';
 import 'package:unittest/unittest.dart';
 
 import '../reflective_tests.dart';
 import 'resolver_test.dart';
 import 'test_support.dart';
 
 main() {
   initializeTestEnvironment();
   runReflectiveTests(DeclarationMatcherTest);
   runReflectiveTests(IncrementalResolverTest);
@@ skipping 74 matching lines @@
     fullErrors.sort((a, b) => a.offset - b.offset);
   }
   int length = incrErrors.length;
   for (int i = 0; i < length; i++) {
     AnalysisError incrError = incrErrors[i];
     AnalysisError fullError = fullErrors[i];
     _assertEqualError(incrError, fullError);
   }
 }
 
+void _checkCacheEntries(AnalysisCache cache) {
+  Set seen = new Set();
+  MapIterator<AnalysisTarget, CacheEntry> it = cache.iterator();
+  while (it.moveNext()) {
+    AnalysisTarget key = it.key;
+    if (cache.get(key) == null) {

[Brian Wilkerson, 2016/02/02 22:51:14]

Would the test

  cache.get(key) != it.value

be better? It probably doesn't matter for any of the tests that are using this,
but if we add a test that makes a change then reverts it, I think we could end
up getting a non-null but still wrong value. Not positive, though.

[scheglov, 2016/02/02 23:04:25]

`it.value` works mostly as `cache.get(key)`.
So, in case of a damaged Element location both will return `null` (almost
always).
So, we might want to have both.

[skybrian, 2016/02/02 23:27:02]

Yes, the old value is (usually) no longer accessible with the mutated key,
because the hash has changed and we're looking in a different slot which has no
entries that compare equal.

I think the only way to retrieve the old value would be to create a key that
compares equal with the mutated key, but returns the old hashCode. But checking
for nulls and duplicate keys seems like enough.

+      fail("cache corrupted: value of $key changed to null");
+    }
+    if (seen.contains(key)) {

[Brian Wilkerson, 2016/02/02 22:51:14]

nit: just use "!seen.add(key)" here and remove line 134

[skybrian, 2016/02/02 23:27:02]

Done.

+      fail("cache corrupted: $key appears more than once");
+    }
+    seen.add(key);
+  }
+}
+
 @reflectiveTest
 class DeclarationMatcherTest extends ResolverTestCase {
   void setUp() {
     super.setUp();
     test_resolveApiChanges = true;
   }
 
   void test_false_class_annotation_accessor_edit() {
     _assertDoesNotMatch(
         r'''
@@ skipping 2983 matching lines @@
   void test_classMemberAccessor_body() {
     _resolveUnit(r'''
 class A {
   int get test {
     return 1 + 2;
   }
 }''');
     _resolve(_editString('+', '*'), _isFunctionBody);
   }
 
+  void test_computeConstants_offsetChanged() {
+    _resolveUnit(r'''
+int f() => 0;
+main() {
+  const x1 = f();
+  const x2 = f();
+  const x3 = f();
+  const x4 = f();
+  const x5 = f();
+  print(x1 + x2 + x3 + x4 + x5 + 1);
+}
+''');
+    _resolve(_editString('x1', ' x1'), _isFunctionBody);
+  }
+
   void test_constructor_body() {
     _resolveUnit(r'''
 class A {
   int f;
   A(int a, int b) {
     f = a + b;
   }
 }''');
     _resolve(_editString('+', '*'), _isFunctionBody);
   }
@@ skipping 285 matching lines @@
    * Then resolves the new code from scratch and validates that results of
    * the incremental resolution and non-incremental resolutions are the same.
    */
   void _resolve(_Edit edit, Predicate<AstNode> predicate) {
     int offset = edit.offset;
     // parse "newCode"
     String newCode = code.substring(0, offset) +
         edit.replacement +
         code.substring(offset + edit.length);
     CompilationUnit newUnit = _parseUnit(newCode);
+    AnalysisCache cache = analysisContext2.analysisCache;
+    _checkCacheEntries(cache);
+
     // replace the node
     AstNode oldNode = _findNodeAt(unit, offset, predicate);
     AstNode newNode = _findNodeAt(newUnit, offset, predicate);
     {
       bool success = NodeReplacer.replace(oldNode, newNode);
       expect(success, isTrue);
     }
     // update tokens
     {
       int delta = edit.replacement.length - edit.length;
       _shiftTokens(unit.beginToken, offset, delta);
     }
     // do incremental resolution
     int updateOffset = edit.offset;
     int updateEndOld = updateOffset + edit.length;
     int updateOldNew = updateOffset + edit.replacement.length;
     IncrementalResolver resolver;
     LibrarySpecificUnit lsu = new LibrarySpecificUnit(source, source);
-    AnalysisCache cache = analysisContext2.analysisCache;
-    resolver = new IncrementalResolver(cache.get(source), cache.get(lsu),
+    resolver = new IncrementalResolver(cache, cache.get(source), cache.get(lsu),
         unit.element, updateOffset, updateEndOld, updateOldNew);
     bool success = resolver.resolve(newNode);
     expect(success, isTrue);
+    _checkCacheEntries(cache);
+
     List<AnalysisError> newErrors = analysisContext.computeErrors(source);
     // resolve "newCode" from scratch
     CompilationUnit fullNewUnit;
     {
       source = addSource(newCode);
       _runTasks();
       LibraryElement library = resolve2(source);
       fullNewUnit = resolveCompilationUnit(source, library);
     }
+    _checkCacheEntries(cache);
+
     try {
       assertSameResolution(unit, fullNewUnit);
     } on IncrementalResolutionMismatch catch (mismatch) {
       fail(mismatch.message);
     }
     // errors
     List<AnalysisError> newFullErrors =
         analysisContext.getErrors(source).errors;
     _assertEqualErrors(newErrors, newFullErrors);
     // prepare for the next cycle
     code = newCode;
   }
 
   void _resolveUnit(String code) {
     this.code = code;
     source = addSource(code);
     library = resolve2(source);
     unit = resolveCompilationUnit(source, library);
     _runTasks();
+    _checkCacheEntries(analysisContext2.analysisCache);
   }
 
   void _runTasks() {
     AnalysisResult result = analysisContext.performAnalysisTask();
     while (result.changeNotices != null) {
       result = analysisContext.performAnalysisTask();
     }
   }
 
   static AstNode _findNodeAt(
@@ skipping 1370 matching lines @@
       // Validate that "incremental" and "full" units have the same resolution.
       try {
         assertSameResolution(newUnit, fullNewUnit, validateTypes: true);
       } on IncrementalResolutionMismatch catch (mismatch) {
         fail(mismatch.message);
       }
       List<AnalysisError> newFullErrors =
           analysisContext.getErrors(source).errors;
       _assertEqualErrors(newErrors, newFullErrors);
     }
+    _checkCacheEntries(analysisContext2.analysisCache);
   }
 
   static void _assertEqualToken(Token incrToken, Token fullToken) {
 //    print('[${incrToken.offset}] |$incrToken| vs. [${fullToken.offset}] |$fullToken|');
     expect(incrToken.type, fullToken.type);
     expect(incrToken.offset, fullToken.offset);
     expect(incrToken.length, fullToken.length);
     expect(incrToken.lexeme, fullToken.lexeme);
   }
 
@@ skipping 256 matching lines @@
   @override
   void logException(Object exception, [Object stackTrace]) {
     hasError = true;
   }
 
   @override
   logging.LoggingTimer startTimer() {
     return new logging.LoggingTimer(this);
   }
 }