diff --git a/pkg/analysis_server/test/lsp/server_abstract.dart b/pkg/analysis_server/test/lsp/server_abstract.dart index b4d14d7c603..26cd6166f24 100644 --- a/pkg/analysis_server/test/lsp/server_abstract.dart +++ b/pkg/analysis_server/test/lsp/server_abstract.dart @@ -1467,7 +1467,7 @@ mixin LspAnalysisServerTestMixin implements ClientCapabilitiesHelperMixin { text: content)), ); await sendNotificationToServer(notification); - await pumpEventQueue(); + await pumpEventQueue(times: 128); } int positionCompare(Position p1, Position p2) { diff --git a/pkg/analyzer/lib/src/dart/analysis/driver.dart b/pkg/analyzer/lib/src/dart/analysis/driver.dart index fcf5d4e5a4a..3e16e13dc32 100644 --- a/pkg/analyzer/lib/src/dart/analysis/driver.dart +++ b/pkg/analyzer/lib/src/dart/analysis/driver.dart @@ -222,7 +222,7 @@ class AnalysisDriver implements AnalysisDriverGeneric { final bool enableIndex; /// The current analysis session. - late AnalysisSessionImpl _currentSession; + late final AnalysisSessionImpl _currentSession = AnalysisSessionImpl(this); /// The current library context, consistent with the [_currentSession]. /// @@ -299,7 +299,6 @@ class AnalysisDriver implements AnalysisDriverGeneric { _sourceFactory = sourceFactory, _externalSummaries = externalSummaries, testingData = retainDataForTesting ? TestingData() : null { - _createNewSession(); _onResults = _resultController.stream.asBroadcastStream(); _testView = AnalysisDriverTestView(this); _createFileTracker(); @@ -461,14 +460,10 @@ class AnalysisDriver implements AnalysisDriverGeneric { return; } if (file_paths.isDart(resourceProvider.pathContext, path)) { + _priorityResults.clear(); + _removePotentiallyAffectedLibraries(path); _fileTracker.addFile(path); - // If the file is known, it has already been read, even if it did not - // exist. Now we are notified that the file exists, so we need to - // re-read it and make sure that we invalidate signature of the files - // that reference it. - if (_fsState.knownFilePaths.contains(path)) { - _changeFile(path); - } + _scheduler.notify(this); } } @@ -490,7 +485,15 @@ class AnalysisDriver implements AnalysisDriverGeneric { /// [changeFile] invocation. void changeFile(String path) { _throwIfNotAbsolutePath(path); - _changeFile(path); + if (!_fsState.hasUri(path)) { + return; + } + if (file_paths.isDart(resourceProvider.pathContext, path)) { + _priorityResults.clear(); + _removePotentiallyAffectedLibraries(path); + _fileTracker.changeFile(path); + _scheduler.notify(this); + } } /// Clear the library context and any related data structures. Mostly we do @@ -500,6 +503,7 @@ class AnalysisDriver implements AnalysisDriverGeneric { /// periodically. @visibleForTesting void clearLibraryContext() { + _libraryContext?.invalidAllLibraries(); _libraryContext = null; _currentSession.clearHierarchies(); } @@ -528,9 +532,15 @@ class AnalysisDriver implements AnalysisDriverGeneric { if (sourceFactory != null) { _sourceFactory = sourceFactory; } + + _priorityResults.clear(); + clearLibraryContext(); + Iterable addedFiles = _fileTracker.addedFiles; _createFileTracker(); _fileTracker.addFiles(addedFiles); + + _scheduler.notify(this); } /// Return a [Future] that completes when discovery of all files that are @@ -1197,17 +1207,25 @@ class AnalysisDriver implements AnalysisDriverGeneric { /// but does not guarantee this. void removeFile(String path) { _throwIfNotAbsolutePath(path); - _fileTracker.removeFile(path); - clearLibraryContext(); - _priorityResults.clear(); + if (!_fsState.hasUri(path)) { + return; + } + if (file_paths.isDart(resourceProvider.pathContext, path)) { + _priorityResults.clear(); + _removePotentiallyAffectedLibraries(path); + _fileTracker.removeFile(path); + _scheduler.notify(this); + } } /// Reset URI resolution, read again all files, build files graph, and ensure /// that for all added files new results are reported. void resetUriResolution() { + _priorityResults.clear(); + clearLibraryContext(); _fsState.resetUriResolution(); _fileTracker.scheduleAllAddedFiles(); - _changeHook(); + _scheduler.notify(this); } void _addDeclaredVariablesToSignature(ApiSignature buffer) { @@ -1221,22 +1239,6 @@ class AnalysisDriver implements AnalysisDriverGeneric { } } - /// Implementation for [changeFile]. - void _changeFile(String path) { - _fileTracker.changeFile(path); - clearLibraryContext(); - _priorityResults.clear(); - } - - /// Handles a notification from the [FileTracker] that there has been a change - /// of state. - void _changeHook() { - _createNewSession(); - clearLibraryContext(); - _priorityResults.clear(); - _scheduler.notify(this); - } - /// There was an exception during a file analysis, we don't know why. /// But it might have been caused by an inconsistency of files state, and /// the library context state. Reset the library context, and hope that @@ -1483,7 +1485,7 @@ class AnalysisDriver implements AnalysisDriverGeneric { externalSummaries: _externalSummaries, fileContentCache: _fileContentCache, ); - _fileTracker = FileTracker(_logger, _fsState, _changeHook); + _fileTracker = FileTracker(_logger, _fsState); } /// Return the context in which the [library] should be analyzed. @@ -1516,11 +1518,6 @@ class AnalysisDriver implements AnalysisDriverGeneric { return libraryContext; } - /// Create a new analysis session, so invalidating the current one. - void _createNewSession() { - _currentSession = AnalysisSessionImpl(this); - } - /// If this has not been done yet, schedule discovery of all files that are /// potentially available, so that they are included in [knownFiles]. void _discoverAvailableFiles() { @@ -1682,6 +1679,18 @@ class AnalysisDriver implements AnalysisDriverGeneric { 'missing', errorsResult, AnalysisDriverUnitIndexBuilder()); } + void _removePotentiallyAffectedLibraries(String path) { + _logger.run('Invalidate affected by $path.', () { + _logger.writeln('Work in $name'); + var affected = {}; + _fsState.collectAffected(path, affected); + _logger.writeln('Remove ${affected.length} libraries.'); + _libraryContext?.elementFactory.removeLibraries( + affected.map((e) => e.uriStr).toSet(), + ); + }); + } + void _reportException(String path, Object exception, StackTrace stackTrace) { String? contextKey; if (exception is _ExceptionState) { @@ -2053,6 +2062,9 @@ class AnalysisDriverTestView { FileTracker get fileTracker => driver._fileTracker; + /// TODO(scheglov) Rename this, and [libraryContext]. + LibraryContext? get libraryContext2 => driver._libraryContext; + Map get priorityResults { return driver._priorityResults; } diff --git a/pkg/analyzer/lib/src/dart/analysis/file_state.dart b/pkg/analyzer/lib/src/dart/analysis/file_state.dart index e3387ae6f9b..eff19a91e65 100644 --- a/pkg/analyzer/lib/src/dart/analysis/file_state.dart +++ b/pkg/analyzer/lib/src/dart/analysis/file_state.dart @@ -781,6 +781,26 @@ class FileSystemState { @visibleForTesting FileSystemStateTestView get test => _testView; + /// Collected files that transitively reference a file with the [path]. + /// These files are potentially affected by the change. + void collectAffected(String path, Set affected) { + final knownFiles = this.knownFiles.toList(); + + collectAffected(FileState file) { + if (affected.add(file)) { + for (var other in knownFiles) { + if (other.directReferencedFiles.contains(file)) { + collectAffected(other); + } + } + } + } + + for (var file in _pathToFiles[path] ?? []) { + collectAffected(file); + } + } + FeatureSet contextFeatureSet( String path, Uri uri, diff --git a/pkg/analyzer/lib/src/dart/analysis/file_tracker.dart b/pkg/analyzer/lib/src/dart/analysis/file_tracker.dart index 0494153a84c..45ea739a7de 100644 --- a/pkg/analyzer/lib/src/dart/analysis/file_tracker.dart +++ b/pkg/analyzer/lib/src/dart/analysis/file_tracker.dart @@ -16,10 +16,6 @@ import 'package:analyzer/src/dart/analysis/performance_logger.dart'; /// /// Provides methods for updating the file system state in response to changes. class FileTracker { - /// Callback invoked whenever a change occurs that may require the client to - /// perform analysis. - final void Function() _changeHook; - /// The logger to write performed operations and performance to. final PerformanceLog _logger; @@ -49,7 +45,7 @@ class FileTracker { /// have any special relation with changed files. var _pendingFiles = {}; - FileTracker(this._logger, this._fsState, this._changeHook); + FileTracker(this._logger, this._fsState); /// Returns the path to exactly one that needs analysis. Throws a /// [StateError] if no files need analysis. @@ -100,27 +96,24 @@ class FileTracker { /// Adds the given [path] to the set of "added files". void addFile(String path) { - _fsState.markFileForReading(path); addedFiles.add(path); - _pendingFiles.add(path); - _changeHook(); + changeFile(path); } /// Adds the given [paths] to the set of "added files". void addFiles(Iterable paths) { addedFiles.addAll(paths); _pendingFiles.addAll(paths); - _changeHook(); } /// Adds the given [path] to the set of "changed files". void changeFile(String path) { + _fsState.markFileForReading(path); _changedFiles.add(path); + if (addedFiles.contains(path)) { _pendingChangedFiles.add(path); } - _fsState.markFileForReading(path); - _changeHook(); } /// Removes the given [path] from the set of "pending files". @@ -165,7 +158,6 @@ class FileTracker { // files seems extreme. _fsState.removeFile(path); _pendingFiles.addAll(addedFiles); - _changeHook(); } /// Schedule all added files for analysis. diff --git a/pkg/analyzer/lib/src/dart/analysis/library_context.dart b/pkg/analyzer/lib/src/dart/analysis/library_context.dart index f80a74492b6..cab5839750b 100644 --- a/pkg/analyzer/lib/src/dart/analysis/library_context.dart +++ b/pkg/analyzer/lib/src/dart/analysis/library_context.dart @@ -89,6 +89,11 @@ class LibraryContext { return elementFactory.libraryOfUri2('$uri'); } + /// We are about to discard this context, mark all libraries invalid. + void invalidAllLibraries() { + elementFactory.invalidateAllLibraries(); + } + /// Load data required to access elements of the given [targetLibrary]. void load2(FileState targetLibrary) { timerLoad2.start(); diff --git a/pkg/analyzer/lib/src/dart/element/element.dart b/pkg/analyzer/lib/src/dart/element/element.dart index 766ace46203..994614b3262 100644 --- a/pkg/analyzer/lib/src/dart/element/element.dart +++ b/pkg/analyzer/lib/src/dart/element/element.dart @@ -3723,6 +3723,12 @@ class LibraryElementImpl extends _ExistingElementImpl @override final AnalysisSession session; + /// If `true`, then this library is valid in the session. + /// + /// A library becomes invalid when one of its files, or one of its + /// dependencies, changes. + bool isValid = true; + /// The language version for the library. LibraryLanguageVersion? _languageVersion; diff --git a/pkg/analyzer/lib/src/summary2/linked_element_factory.dart b/pkg/analyzer/lib/src/summary2/linked_element_factory.dart index e83e4928f55..456548ea9d5 100644 --- a/pkg/analyzer/lib/src/summary2/linked_element_factory.dart +++ b/pkg/analyzer/lib/src/summary2/linked_element_factory.dart @@ -164,6 +164,13 @@ class LinkedElementFactory { return libraryReaders[uriStr] != null; } + /// We are about to discard this factory, mark all libraries invalid. + void invalidateAllLibraries() { + for (var libraryReference in rootReference.children) { + _invalidateLibrary(libraryReference); + } + } + LibraryElementImpl? libraryOfUri(String uriStr) { var reference = rootReference.getChild(uriStr); return elementOfReference(reference) as LibraryElementImpl?; @@ -189,7 +196,8 @@ class LinkedElementFactory { for (var uriStr in uriStrSet) { _exportsOfLibrary.remove(uriStr); libraryReaders.remove(uriStr); - rootReference.removeChild(uriStr); + var libraryReference = rootReference.removeChild(uriStr); + _invalidateLibrary(libraryReference); } analysisSession.classHierarchy.removeOfLibraries(uriStrSet); @@ -239,4 +247,11 @@ class LinkedElementFactory { libraryElement.createLoadLibraryFunction(); } + + void _invalidateLibrary(Reference? libraryReference) { + var libraryElement = libraryReference?.element; + if (libraryElement is LibraryElementImpl) { + libraryElement.isValid = false; + } + } } diff --git a/pkg/analyzer/lib/src/summary2/reference.dart b/pkg/analyzer/lib/src/summary2/reference.dart index e73b634fc87..8c305bde616 100644 --- a/pkg/analyzer/lib/src/summary2/reference.dart +++ b/pkg/analyzer/lib/src/summary2/reference.dart @@ -65,8 +65,8 @@ class Reference { return map[name] ??= Reference._(this, name); } - void removeChild(String name) { - _children?.remove(name); + Reference? removeChild(String name) { + return _children?.remove(name); } @override diff --git a/pkg/analyzer/test/src/dart/analysis/driver_test.dart b/pkg/analyzer/test/src/dart/analysis/driver_test.dart index 29a974ac1ae..f2b9cde619f 100644 --- a/pkg/analyzer/test/src/dart/analysis/driver_test.dart +++ b/pkg/analyzer/test/src/dart/analysis/driver_test.dart @@ -727,6 +727,128 @@ var A = B; expect(driver.fsState.knownFilePaths, isNot(contains(b))); } + test_changeFile_potentiallyAffected_imported() async { + newFile('/test/lib/a.dart', content: ''); + var b = newFile('/test/lib/b.dart', content: ''' +import 'a.dart'; +'''); + newFile('/test/lib/c.dart', content: ''' +import 'b.dart'; +'''); + newFile('/test/lib/d.dart', content: ''' +import 'c.dart'; +'''); + newFile('/test/lib/e.dart', content: ''); + + Future getLibrary(String shortName) async { + var uriStr = 'package:test/$shortName'; + var result = await driver.getLibraryByUriValid(uriStr); + return result.element as LibraryElementImpl; + } + + var a_element = await getLibrary('a.dart'); + var b_element = await getLibrary('b.dart'); + var c_element = await getLibrary('c.dart'); + var d_element = await getLibrary('d.dart'); + var e_element = await getLibrary('e.dart'); + + // We have all libraries loaded after analysis. + driver.assertLoadedLibraryUriSet( + included: [ + 'package:test/a.dart', + 'package:test/b.dart', + 'package:test/c.dart', + 'package:test/d.dart', + 'package:test/e.dart', + ], + ); + + // All libraries are valid. + expect(a_element.isValid, true); + expect(b_element.isValid, true); + expect(c_element.isValid, true); + expect(d_element.isValid, true); + expect(e_element.isValid, true); + + // Change `b.dart`, also removes `c.dart` and `d.dart` that import it. + // But `a.dart` and `d.dart` is not affected. + driver.changeFile(b.path); + driver.assertLoadedLibraryUriSet( + excluded: [ + 'package:test/b.dart', + 'package:test/c.dart', + 'package:test/d.dart', + ], + included: [ + 'package:test/a.dart', + 'package:test/e.dart', + ], + ); + + // Only `a.dart` and `e.dart` are still valid. + expect(a_element.isValid, true); + expect(b_element.isValid, false); + expect(c_element.isValid, false); + expect(d_element.isValid, false); + expect(e_element.isValid, true); + } + + test_changeFile_potentiallyAffected_part() async { + var a = newFile('/test/lib/a.dart', content: ''' +part of 'b.dart'; +'''); + newFile('/test/lib/b.dart', content: ''' +part 'a.dart'; +'''); + newFile('/test/lib/c.dart', content: ''' +import 'b.dart'; +'''); + newFile('/test/lib/d.dart', content: ''); + + Future getLibrary(String shortName) async { + var uriStr = 'package:test/$shortName'; + var result = await driver.getLibraryByUriValid(uriStr); + return result.element as LibraryElementImpl; + } + + var b_element = await getLibrary('b.dart'); + var c_element = await getLibrary('c.dart'); + var d_element = await getLibrary('d.dart'); + + // We have all libraries loaded after analysis. + driver.assertLoadedLibraryUriSet( + included: [ + 'package:test/b.dart', + 'package:test/c.dart', + 'package:test/d.dart', + ], + ); + + // All libraries are valid. + expect(b_element.isValid, true); + expect(c_element.isValid, true); + expect(d_element.isValid, true); + + // Change `a.dart`, remove `b.dart` that part it. + // Removes `c.dart` that imports `b.dart`. + // But `d.dart` is not affected. + driver.changeFile(a.path); + driver.assertLoadedLibraryUriSet( + excluded: [ + 'package:test/b.dart', + 'package:test/c.dart', + ], + included: [ + 'package:test/d.dart', + ], + ); + + // Only `d.dart` is still valid. + expect(b_element.isValid, false); + expect(c_element.isValid, false); + expect(d_element.isValid, true); + } + test_changeFile_selfConsistent() async { var a = convertPath('/test/lib/a.dart'); var b = convertPath('/test/lib/b.dart'); @@ -987,7 +1109,7 @@ const x = 1; } test_currentSession() async { - var a = convertPath('/a.dart'); + var a = convertPath('/test/lib/a.dart'); newFile(a, content: 'var V = 1;'); await driver.getResultValid(a); @@ -1002,8 +1124,9 @@ const x = 1; var session2 = driver.currentSession; expect(session2, isNotNull); - // We get a new session. - expect(session2, isNot(session1)); + // We don't discard the session anymore. + // So, the session is always the same. + expect(session2, same(session1)); } test_discoverAvailableFiles_packages() async { @@ -3349,7 +3472,32 @@ extension on AnalysisDriver { return getFileSync2(path) as FileResult; } + Set get loadedLibraryUriSet { + var elementFactory = this.test.libraryContext2!.elementFactory; + var libraryReferences = elementFactory.rootReference.children; + return libraryReferences.map((e) => e.name).toSet(); + } + + void assertLoadedLibraryUriSet({ + Iterable? included, + Iterable? excluded, + }) { + var uriSet = loadedLibraryUriSet; + if (included != null) { + expect(uriSet, containsAll(included)); + } + if (excluded != null) { + for (var excludedUri in excluded) { + expect(uriSet, isNot(contains(excludedUri))); + } + } + } + Future getResultValid(String path) async { return await getResult2(path) as ResolvedUnitResult; } + + Future getLibraryByUriValid(String uriStr) async { + return await getLibraryByUri2(uriStr) as LibraryElementResult; + } }