diff --git a/pkg/analyzer/lib/src/dart/analysis/driver.dart b/pkg/analyzer/lib/src/dart/analysis/driver.dart index 599d0c9eb6a..69040333d01 100644 --- a/pkg/analyzer/lib/src/dart/analysis/driver.dart +++ b/pkg/analyzer/lib/src/dart/analysis/driver.dart @@ -259,6 +259,7 @@ class AnalysisDriver { bool analyzed = false; for (String path in _priorityFiles) { if (_filesToAnalyze.remove(path)) { + analyzed = true; AnalysisResult result = _computeAnalysisResult(path, withUnit: true); yield result; @@ -331,7 +332,9 @@ class AnalysisDriver { */ void changeFile(String path) { _changedFiles.add(path); - _filesToAnalyze.add(path); + if (_explicitFiles.contains(path)) { + _filesToAnalyze.add(path); + } _transitionToAnalyzing(); _hasWork.notify(); } @@ -472,6 +475,7 @@ class AnalysisDriver { AnalysisContext _createAnalysisContext(_LibraryContext libraryContext) { AnalysisContextImpl analysisContext = AnalysisEngine.instance.createAnalysisContext(); + analysisContext.analysisOptions = _analysisOptions; analysisContext.sourceFactory = new SourceFactory((_sourceFactory as SourceFactoryImpl).resolvers); @@ -941,7 +945,7 @@ class _File { List bytes = driver._byteStore.get(key); if (bytes != null) { PackageBundle unlinked = new PackageBundle.fromBuffer(bytes); - driver._fileApiSignatureMap[path] = unlinked.apiSignature; + _updateApiSignature(driver, path, unlinked.apiSignature); return new _File._(driver, source, null, contentHash, unlinked, null); } } @@ -972,7 +976,7 @@ class _File { }); } unlinked = new PackageBundle.fromBuffer(bytes); - driver._fileApiSignatureMap[path] = unlinked.apiSignature; + _updateApiSignature(driver, path, unlinked.apiSignature); } // Return the full file. return new _File._(driver, source, content, contentHash, unlinked, unit); @@ -1021,6 +1025,15 @@ class _File { unit.lineInfo = lineInfo; return unit; } + + static void _updateApiSignature( + AnalysisDriver driver, String path, String newSignature) { + String oldSignature = driver._fileApiSignatureMap[path]; + if (oldSignature != null && oldSignature != newSignature) { + driver._dependencySignatureMap.clear(); + } + driver._fileApiSignatureMap[path] = newSignature; + } } /** diff --git a/pkg/analyzer/test/src/dart/analysis/driver_test.dart b/pkg/analyzer/test/src/dart/analysis/driver_test.dart index 9384ba2302f..34e20efdbb1 100644 --- a/pkg/analyzer/test/src/dart/analysis/driver_test.dart +++ b/pkg/analyzer/test/src/dart/analysis/driver_test.dart @@ -17,7 +17,6 @@ import 'package:analyzer/src/dart/analysis/driver.dart'; import 'package:analyzer/src/error/codes.dart'; import 'package:analyzer/src/generated/engine.dart' show AnalysisOptionsImpl; import 'package:analyzer/src/generated/source.dart'; -import 'package:async/async.dart'; import 'package:convert/convert.dart'; import 'package:crypto/crypto.dart'; import 'package:test/test.dart'; @@ -31,6 +30,20 @@ main() { }); } +/** + * Returns a [Future] that completes after pumping the event queue [times] + * times. By default, this should pump the event queue enough times to allow + * any code to run, as long as it's not waiting on some external event. + */ +Future pumpEventQueue([int times = 5000]) { + if (times == 0) return new Future.value(); + // We use a delayed future to allow microtask events to finish. The + // Future.value or Future() constructors use scheduleMicrotask themselves and + // would therefore not wait for microtask callbacks that are scheduled after + // invoking this method. + return new Future.delayed(Duration.ZERO, () => pumpEventQueue(times - 1)); +} + @reflectiveTest class DriverTest { static final MockSdk sdk = new MockSdk(); @@ -41,7 +54,7 @@ class DriverTest { final StringBuffer logBuffer = new StringBuffer(); AnalysisDriver driver; - StreamSplitter statusSplitter; + final _Monitor idleStatusMonitor = new _Monitor(); final List allStatuses = []; final List allResults = []; @@ -64,11 +77,163 @@ class DriverTest { }) ], null, provider), new AnalysisOptionsImpl()..strongMode = true); - statusSplitter = new StreamSplitter(driver.status); - statusSplitter.split().listen(allStatuses.add); + driver.status.lastWhere((status) { + allStatuses.add(status); + if (status.isIdle) { + idleStatusMonitor.notify(); + } + }); driver.results.listen(allResults.add); } + test_addFile_thenRemove() async { + var a = _p('/test/lib/a.dart'); + var b = _p('/test/lib/b.dart'); + provider.newFile(a, 'class A {}'); + provider.newFile(b, 'class B {}'); + driver.addFile(a); + driver.addFile(b); + + // Now remove 'a'. + driver.removeFile(a); + + await _waitForIdle(); + + // Only 'b' has been analyzed, because 'a' was removed before we started. + expect(allResults, hasLength(1)); + expect(allResults[0].path, b); + } + + test_changeFile_implicitlyAnalyzed() async { + var a = _p('/test/lib/a.dart'); + var b = _p('/test/lib/b.dart'); + provider.newFile( + a, + r''' +import 'b.dart'; +var A = B; +'''); + provider.newFile(b, 'var B = 1;'); + + driver.priorityFiles = [a]; + driver.addFile(a); + + // We have a result only for "a". + await _waitForIdle(); + expect(allResults, hasLength(1)); + { + AnalysisResult ar = allResults.firstWhere((r) => r.path == a); + expect(_getTopLevelVarType(ar.unit, 'A'), 'int'); + } + allResults.clear(); + + // Change "b" and notify. + provider.updateFile(b, 'var B = 1.2;'); + driver.changeFile(b); + + // While "b" is not analyzed explicitly, it is analyzed implicitly. + // The change causes "a" to be reanalyzed. + await _waitForIdle(); + expect(allResults, hasLength(1)); + { + AnalysisResult ar = allResults.firstWhere((r) => r.path == a); + expect(_getTopLevelVarType(ar.unit, 'A'), 'double'); + } + } + + test_changeFile_selfConsistent() async { + var a = _p('/test/lib/a.dart'); + var b = _p('/test/lib/b.dart'); + provider.newFile( + a, + r''' +import 'b.dart'; +var A1 = 1; +var A2 = B1; +'''); + provider.newFile( + b, + r''' +import 'a.dart'; +var B1 = A1; +'''); + + driver.priorityFiles = [a, b]; + driver.addFile(a); + driver.addFile(b); + await _waitForIdle(); + + // We have results for both "a" and "b". + expect(allResults, hasLength(2)); + { + AnalysisResult ar = allResults.firstWhere((r) => r.path == a); + expect(_getTopLevelVarType(ar.unit, 'A1'), 'int'); + expect(_getTopLevelVarType(ar.unit, 'A2'), 'int'); + } + { + AnalysisResult br = allResults.firstWhere((r) => r.path == b); + expect(_getTopLevelVarType(br.unit, 'B1'), 'int'); + } + + // Clear the results and update "a". + allResults.clear(); + provider.updateFile( + a, + r''' +import 'b.dart'; +var A1 = 1.2; +var A2 = B1; +'''); + driver.changeFile(a); + + // We again get results for both "a" and "b". + // The results are consistent. + await _waitForIdle(); + expect(allResults, hasLength(2)); + { + AnalysisResult ar = allResults.firstWhere((r) => r.path == a); + expect(_getTopLevelVarType(ar.unit, 'A1'), 'double'); + expect(_getTopLevelVarType(ar.unit, 'A2'), 'double'); + } + { + AnalysisResult br = allResults.firstWhere((r) => r.path == b); + expect(_getTopLevelVarType(br.unit, 'B1'), 'double'); + } + } + + test_changeFile_single() async { + _addTestFile('var V = 1;', priority: true); + + // Initial analysis. + { + await _waitForIdle(); + expect(allResults, hasLength(1)); + AnalysisResult result = allResults[0]; + expect(result.path, testFile); + expect(_getTopLevelVarType(result.unit, 'V'), 'int'); + } + + // Update the file, but don't notify the driver. + allResults.clear(); + provider.updateFile(testFile, 'var V = 1.2'); + + // No new results. + await pumpEventQueue(); + expect(allResults, isEmpty); + + // Notify the driver about the change. + driver.changeFile(testFile); + + // We get a new result. + { + await _waitForIdle(); + expect(allResults, hasLength(1)); + AnalysisResult result = allResults[0]; + expect(result.path, testFile); + expect(_getTopLevelVarType(result.unit, 'V'), 'double'); + } + } + test_getResult() async { String content = 'int f() => 42;'; _addTestFile(content, priority: true); @@ -107,6 +272,147 @@ class DriverTest { } } + test_getResult_selfConsistent() async { + var a = _p('/test/lib/a.dart'); + var b = _p('/test/lib/b.dart'); + provider.newFile( + a, + r''' +import 'b.dart'; +var A1 = 1; +var A2 = B1; +'''); + provider.newFile( + b, + r''' +import 'a.dart'; +var B1 = A1; +'''); + + driver.addFile(a); + driver.addFile(b); + await _waitForIdle(); + + { + AnalysisResult result = await driver.getResult(a); + expect(_getTopLevelVarType(result.unit, 'A1'), 'int'); + expect(_getTopLevelVarType(result.unit, 'A2'), 'int'); + } + + // Update "a" that that "A1" is now "double". + // Get result for "a". + // + // Even though we have not notified the driver about the change, + // we still get "double" for "A1", because getResult() re-read the content. + // + // We also get "double" for "A2", even though "A2" has the type from "b". + // That's because we check for "a" API signature consistency, and because + // it has changed, we invalidated the dependency cache, relinked libraries + // and recomputed types. + provider.updateFile( + a, + r''' +import 'b.dart'; +var A1 = 1.2; +var A2 = B1; +'''); + { + AnalysisResult result = await driver.getResult(a); + expect(_getTopLevelVarType(result.unit, 'A1'), 'double'); + expect(_getTopLevelVarType(result.unit, 'A2'), 'double'); + } + } + + test_getResult_thenRemove() async { + _addTestFile('main() {}', priority: true); + + Future resultFuture = driver.getResult(testFile); + driver.removeFile(testFile); + + AnalysisResult result = await resultFuture; + expect(result, isNotNull); + expect(result.path, testFile); + expect(result.unit, isNotNull); + } + + test_getResult_twoPendingFutures() async { + String content = 'main() {}'; + _addTestFile(content, priority: true); + + Future future1 = driver.getResult(testFile); + Future future2 = driver.getResult(testFile); + + // Both futures complete, with the same result. + AnalysisResult result1 = await future1; + AnalysisResult result2 = await future2; + expect(result2, same(result1)); + expect(result1.path, testFile); + expect(result1.unit, isNotNull); + } + + test_removeFile_changeFile_implicitlyAnalyzed() async { + var a = _p('/test/lib/a.dart'); + var b = _p('/test/lib/b.dart'); + provider.newFile( + a, + r''' +import 'b.dart'; +var A = B; +'''); + provider.newFile(b, 'var B = 1;'); + + driver.priorityFiles = [a, b]; + driver.addFile(a); + driver.addFile(b); + + // We have results for both "a" and "b". + await _waitForIdle(); + expect(allResults, hasLength(2)); + { + AnalysisResult ar = allResults.firstWhere((r) => r.path == a); + expect(_getTopLevelVarType(ar.unit, 'A'), 'int'); + } + { + AnalysisResult br = allResults.firstWhere((r) => r.path == b); + expect(_getTopLevelVarType(br.unit, 'B'), 'int'); + } + allResults.clear(); + + // Remove "b" and send the change notification. + provider.updateFile(b, 'var B = 1.2;'); + driver.removeFile(b); + driver.changeFile(b); + + // While "b" is not analyzed explicitly, it is analyzed implicitly. + // We don't get a result for "b". + // But the change causes "a" to be reanalyzed. + await _waitForIdle(); + expect(allResults, hasLength(1)); + { + AnalysisResult ar = allResults.firstWhere((r) => r.path == a); + expect(_getTopLevelVarType(ar.unit, 'A'), 'double'); + } + } + + test_removeFile_changeFile_notAnalyzed() async { + _addTestFile('main() {}'); + + // We have a result. + await _waitForIdle(); + expect(allResults, hasLength(1)); + expect(allResults[0].path, testFile); + allResults.clear(); + + // Remove the file and send the change notification. + // The change notification does nothing, because the file is explicitly + // or implicitly analyzed. + driver.removeFile(testFile); + driver.changeFile(testFile); + + await _waitForIdle(); + expect(allResults, isEmpty); + } + test_results_priority() async { String content = 'int f() => 42;'; _addTestFile(content, priority: true); @@ -182,13 +488,31 @@ class DriverTest { } } + VariableDeclaration _getTopLevelVar(CompilationUnit unit, String name) { + for (CompilationUnitMember declaration in unit.declarations) { + if (declaration is TopLevelVariableDeclaration) { + for (VariableDeclaration variable in declaration.variables.variables) { + if (variable.name.name == name) { + return variable; + } + } + } + } + fail('Cannot find the top-level variable $name in\n$unit'); + return null; + } + + String _getTopLevelVarType(CompilationUnit unit, String name) { + return _getTopLevelVar(unit, name).element.type.toString(); + } + /** * Return the [provider] specific path for the given Posix [path]. */ String _p(String path) => provider.convertPath(path); Future _waitForIdle() async { - await statusSplitter.split().firstWhere((status) => status.isIdle); + await idleStatusMonitor.signal; } static String _md5(String content) { @@ -196,6 +520,21 @@ class DriverTest { } } +class _Monitor { + Completer _completer = new Completer(); + + Future get signal async { + await _completer.future; + _completer = new Completer(); + } + + void notify() { + if (!_completer.isCompleted) { + _completer.complete(null); + } + } +} + class _TestByteStore implements ByteStore { final map = >{};