diff --git a/pkg/analysis_server/lib/src/analysis_server.dart b/pkg/analysis_server/lib/src/analysis_server.dart index 34531c68402..9bac7d92535 100644 --- a/pkg/analysis_server/lib/src/analysis_server.dart +++ b/pkg/analysis_server/lib/src/analysis_server.dart @@ -589,7 +589,8 @@ class AnalysisServer { } /** - * Returns `true` if there is a subscription for the given [server] and [file]. + * Returns `true` if there is a subscription for the given [service] and + * [file]. */ bool hasAnalysisSubscription(AnalysisService service, String file) { Set files = analysisServices[service]; @@ -699,9 +700,7 @@ class AnalysisServer { */ void scheduleOperation(ServerOperation operation) { addOperation(operation); - if (!performOperationPending) { - _schedulePerformOperation(); - } + _schedulePerformOperation(); } /** @@ -929,6 +928,15 @@ class AnalysisServer { }); } + void test_flushResolvedUnit(String file) { + if (AnalysisEngine.isDartFileName(file)) { + AnalysisContextImpl context = getAnalysisContext(file); + Source source = getSource(file); + DartEntry dartEntry = context.getReadableSourceEntryOrNull(source); + dartEntry.flushAstStructures(); + } + } + /** * Implementation for `analysis.updateContent`. */ @@ -967,6 +975,7 @@ class AnalysisServer { throw new AnalysisException('Illegal change type'); } _overlayState.setContents(source, newContents); + // Update all contexts. for (InternalAnalysisContext context in folderMap.values) { if (context.handleContentsChanged( source, @@ -974,6 +983,28 @@ class AnalysisServer { newContents, true)) { schedulePerformAnalysisOperation(context); + } else { + // When the client sends any change for a source, we should resend + // subscribed notifications, even if there were no changes in the + // source contents. + // TODO(scheglov) consider checking if there are subscriptions. + if (AnalysisEngine.isDartFileName(file)) { + CompilationUnit dartUnit = + context.ensureAnyResolvedDartUnit(source); + if (dartUnit != null) { + AnalysisErrorInfo errorInfo = context.getErrors(source); + scheduleNotificationOperations( + this, + file, + errorInfo.lineInfo, + context, + null, + dartUnit, + errorInfo.errors); + } else { + schedulePerformAnalysisOperation(context); + } + } } } }); @@ -1008,7 +1039,9 @@ class AnalysisServer { * Schedules [performOperation] exection. */ void _schedulePerformOperation() { - assert(!performOperationPending); + if (performOperationPending) { + return; + } /* * TODO (danrubel) Rip out this workaround once the underlying problem * is fixed. Currently, the VM and dart:io do not deliver content diff --git a/pkg/analysis_server/lib/src/operation/operation_analysis.dart b/pkg/analysis_server/lib/src/operation/operation_analysis.dart index 07a4fdc1e0c..ec612a5568f 100644 --- a/pkg/analysis_server/lib/src/operation/operation_analysis.dart +++ b/pkg/analysis_server/lib/src/operation/operation_analysis.dart @@ -20,6 +20,68 @@ import 'package:analyzer/src/generated/html.dart'; import 'package:analyzer/src/generated/source.dart'; +/** + * Schedules sending notifications for the given [file] using the resolved + * [resolvedDartUnit]. + */ +void scheduleNotificationOperations(AnalysisServer server, String file, + LineInfo lineInfo, AnalysisContext context, CompilationUnit parsedDartUnit, + CompilationUnit resolvedDartUnit, List errors) { + // Only send notifications if the current context is the preferred + // context for the file. This avoids redundant notification messages + // being sent to the client (see dartbug.com/22210). + // TODO(paulberry): note that there is a small risk that this will cause + // notifications to be lost if the preferred context for a file changes + // while analysis is in progress (e.g. because the client sent an + // analysis.setAnalysisRoots message). + if (server.getAnalysisContext(file) != context) { + return; + } + // Dart + CompilationUnit dartUnit = + resolvedDartUnit != null ? resolvedDartUnit : parsedDartUnit; + if (resolvedDartUnit != null) { + if (server.hasAnalysisSubscription( + protocol.AnalysisService.HIGHLIGHTS, + file)) { + server.scheduleOperation( + new _DartHighlightsOperation(file, resolvedDartUnit)); + } + if (server.hasAnalysisSubscription( + protocol.AnalysisService.NAVIGATION, + file)) { + server.scheduleOperation( + new _DartNavigationOperation(file, resolvedDartUnit)); + } + if (server.hasAnalysisSubscription( + protocol.AnalysisService.OCCURRENCES, + file)) { + server.scheduleOperation( + new _DartOccurrencesOperation(file, resolvedDartUnit)); + } + if (server.hasAnalysisSubscription( + protocol.AnalysisService.OVERRIDES, + file)) { + server.scheduleOperation( + new _DartOverridesOperation(file, resolvedDartUnit)); + } + } + if (dartUnit != null) { + if (server.hasAnalysisSubscription( + protocol.AnalysisService.OUTLINE, + file)) { + server.scheduleOperation( + new _DartOutlineOperation(file, lineInfo, dartUnit)); + } + } + // errors + if (server.shouldSendErrorsNotificationFor(file)) { + server.scheduleOperation( + new _NotificationErrorsOperation(file, lineInfo, errors)); + } +} + + void sendAnalysisNotificationErrors(AnalysisServer server, String file, LineInfo lineInfo, List errors) { try { @@ -173,61 +235,17 @@ class PerformAnalysisOperation extends ServerOperation { ChangeNotice notice = notices[i]; Source source = notice.source; String file = source.fullName; - // Only send notifications if the current context is the preferred - // context for the file. This avoids redundant notification messages - // being sent to the client (see dartbug.com/22210). - // TODO(paulberry): note that there is a small risk that this will cause - // notifications to be lost if the preferred context for a file changes - // while analysis is in progress (e.g. because the client sent an - // analysis.setAnalysisRoots message). - if (server.getAnalysisContext(file) != context) { - continue; - } // Dart CompilationUnit parsedDartUnit = notice.parsedDartUnit; CompilationUnit resolvedDartUnit = notice.resolvedDartUnit; - CompilationUnit dartUnit = - resolvedDartUnit != null ? resolvedDartUnit : parsedDartUnit; - if (resolvedDartUnit != null) { - if (server.hasAnalysisSubscription( - protocol.AnalysisService.HIGHLIGHTS, - file)) { - server.addOperation( - new _DartHighlightsOperation(file, resolvedDartUnit)); - } - if (server.hasAnalysisSubscription( - protocol.AnalysisService.NAVIGATION, - file)) { - server.addOperation( - new _DartNavigationOperation(file, resolvedDartUnit)); - } - if (server.hasAnalysisSubscription( - protocol.AnalysisService.OCCURRENCES, - file)) { - server.addOperation( - new _DartOccurrencesOperation(file, resolvedDartUnit)); - } - if (server.hasAnalysisSubscription( - protocol.AnalysisService.OVERRIDES, - file)) { - server.addOperation( - new _DartOverridesOperation(file, resolvedDartUnit)); - } - } - if (dartUnit != null) { - if (server.hasAnalysisSubscription( - protocol.AnalysisService.OUTLINE, - file)) { - LineInfo lineInfo = notice.lineInfo; - server.addOperation( - new _DartOutlineOperation(file, lineInfo, dartUnit)); - } - } - // errors - if (server.shouldSendErrorsNotificationFor(file)) { - server.addOperation( - new _NotificationErrorsOperation(file, notice.lineInfo, notice.errors)); - } + scheduleNotificationOperations( + server, + file, + notice.lineInfo, + context, + parsedDartUnit, + resolvedDartUnit, + notice.errors); // done server.fileAnalyzed(notice); } diff --git a/pkg/analysis_server/test/analysis/update_content_test.dart b/pkg/analysis_server/test/analysis/update_content_test.dart index 1fbcaa3fead..f4d5bca234e 100644 --- a/pkg/analysis_server/test/analysis/update_content_test.dart +++ b/pkg/analysis_server/test/analysis/update_content_test.dart @@ -87,4 +87,43 @@ f() {} expect(filesErrors[barPath], isEmpty); }); } + + test_sendNoticesAfterNopChange() async { + createProject(); + addTestFile(''); + await server.onAnalysisComplete; + // add an overlay + server.updateContent('1', { + testFile: new AddContentOverlay('main() {} main() {}') + }); + await server.onAnalysisComplete; + // clear errors and make a no-op change + filesErrors.clear(); + server.updateContent('2', { + testFile: new ChangeContentOverlay([new SourceEdit(0, 4, 'main')]) + }); + await server.onAnalysisComplete; + // errors should have been resent + expect(filesErrors, isNotEmpty); + } + + test_sendNoticesAfterNopChange_flushedUnit() async { + createProject(); + addTestFile(''); + await server.onAnalysisComplete; + // add an overlay + server.updateContent('1', { + testFile: new AddContentOverlay('main() {} main() {}') + }); + await server.onAnalysisComplete; + // clear errors and make a no-op change + filesErrors.clear(); + server.test_flushResolvedUnit(testFile); + server.updateContent('2', { + testFile: new ChangeContentOverlay([new SourceEdit(0, 4, 'main')]) + }); + await server.onAnalysisComplete; + // errors should have been resent + expect(filesErrors, isNotEmpty); + } } diff --git a/pkg/analyzer/lib/src/generated/engine.dart b/pkg/analyzer/lib/src/generated/engine.dart index fbd1d396d42..743353abe9b 100644 --- a/pkg/analyzer/lib/src/generated/engine.dart +++ b/pkg/analyzer/lib/src/generated/engine.dart @@ -1855,6 +1855,39 @@ class AnalysisContextImpl implements InternalAnalysisContext { _pendingFutureSources.clear(); } + @override + CompilationUnit ensureAnyResolvedDartUnit(Source source) { + SourceEntry sourceEntry = _cache.get(source); + if (sourceEntry is! DartEntry) { + return null; + } + DartEntry dartEntry = sourceEntry; + // Check if there is a resolved unit. + CompilationUnit unit = dartEntry.anyResolvedCompilationUnit; + if (unit != null) { + return unit; + } + // Invalidate the flushed RESOLVED_UNIT to force it eventually. + bool shouldBeScheduled = false; + List librariesContaining = dartEntry.containingLibraries; + for (Source librarySource in librariesContaining) { + if (dartEntry.getStateInLibrary(DartEntry.RESOLVED_UNIT, librarySource) == + CacheState.FLUSHED) { + dartEntry.setStateInLibrary( + DartEntry.RESOLVED_UNIT, + librarySource, + CacheState.INVALID); + shouldBeScheduled = true; + } + } + if (shouldBeScheduled) { + _workManager.add(source, SourcePriority.UNKNOWN); + } + // We cannot provide a resolved unit right now, + // but the future analysis will. + return null; + } + @override bool exists(Source source) { if (source == null) { @@ -2184,16 +2217,12 @@ class AnalysisContextImpl implements InternalAnalysisContext { !_tryPoorMansIncrementalResolution(source, newContents)) { _sourceChanged(source); } - if (sourceEntry != null) { - sourceEntry.modificationTime = - _contentCache.getModificationStamp(source); - sourceEntry.setValue(SourceEntry.CONTENT, newContents); - } + sourceEntry.modificationTime = + _contentCache.getModificationStamp(source); + sourceEntry.setValue(SourceEntry.CONTENT, newContents); } else { - if (sourceEntry != null) { - sourceEntry.modificationTime = - _contentCache.getModificationStamp(source); - } + sourceEntry.modificationTime = + _contentCache.getModificationStamp(source); } } else if (originalContents != null) { _incrementalAnalysisCache = @@ -2201,17 +2230,15 @@ class AnalysisContextImpl implements InternalAnalysisContext { changed = newContents != originalContents; // We are removing the overlay for the file, check if the file's // contents is the same as it was in the overlay. - if (sourceEntry != null) { - try { - TimestampedData fileContents = getContents(source); - String fileContentsData = fileContents.data; - if (fileContentsData == originalContents) { - sourceEntry.modificationTime = fileContents.modificationTime; - sourceEntry.setValue(SourceEntry.CONTENT, fileContentsData); - changed = false; - } - } catch (e) { + try { + TimestampedData fileContents = getContents(source); + String fileContentsData = fileContents.data; + if (fileContentsData == originalContents) { + sourceEntry.modificationTime = fileContents.modificationTime; + sourceEntry.setValue(SourceEntry.CONTENT, fileContentsData); + changed = false; } + } catch (e) { } // If not the same content (e.g. the file is being closed without save), // then force analysis. @@ -9702,6 +9729,13 @@ abstract class InternalAnalysisContext implements AnalysisContext { */ CompilationUnit computeResolvableCompilationUnit(Source source); + /** + * Return any resolved [CompilationUnit] for the given [source] if not + * flushed, otherwise return `null` and ensures that the [CompilationUnit] + * will be eventually returned to the client from [performAnalysisTask]. + */ + CompilationUnit ensureAnyResolvedDartUnit(Source source); + /** * Return context that owns the given source. * @@ -9725,7 +9759,7 @@ abstract class InternalAnalysisContext implements AnalysisContext { * is the updated contents. If [notify] is true, a source changed event is * triggered. * - * Normally it should not be necessary for clinets to call this function, + * Normally it should not be necessary for clients to call this function, * since it will be automatically invoked in response to a call to * [applyChanges] or [setContents]. However, if this analysis context is * sharing its content cache with other contexts, then the client must diff --git a/pkg/analyzer/test/generated/engine_test.dart b/pkg/analyzer/test/generated/engine_test.dart index a30c5f63e8b..0089eb8d04e 100644 --- a/pkg/analyzer/test/generated/engine_test.dart +++ b/pkg/analyzer/test/generated/engine_test.dart @@ -6048,6 +6048,11 @@ class TestAnalysisContext implements InternalAnalysisContext { fail("Unexpected invocation of dispose"); } @override + CompilationUnit ensureAnyResolvedDartUnit(Source source) { + fail("Unexpected invocation of ensureAnyResolvedDartUnit"); + return null; + } + @override bool exists(Source source) { fail("Unexpected invocation of exists"); return false; @@ -6210,6 +6215,7 @@ class TestAnalysisContext implements InternalAnalysisContext { void setContents(Source source, String contents) { fail("Unexpected invocation of setContents"); } + @override void visitCacheItems(void callback(Source source, SourceEntry dartEntry, DataDescriptor rowDesc, CacheState state)) {