From 6826e267b952a1cebdf8c4d7523c27986e8d1bf4 Mon Sep 17 00:00:00 2001 From: Konstantin Shcheglov Date: Mon, 31 Oct 2016 09:54:31 -0700 Subject: [PATCH] Integration of the new analysis driver, behind a flag. The flag is --enable-new-analysis-driver, disabled by default. WIP, only error notifications for now. R=brianwilkerson@google.com, paulberry@google.com BUG= Review URL: https://codereview.chromium.org/2465923002 . --- .../lib/src/analysis_server.dart | 154 +++++++++++++++++- .../lib/src/computer/new_notifications.dart | 21 +++ .../lib/src/context_manager.dart | 65 ++++++-- .../lib/src/domain_completion.dart | 4 + .../lib/src/edit/edit_domain.dart | 12 +- .../lib/src/server/driver.dart | 11 ++ .../lib/src/single_context_manager.dart | 9 + .../test/context_manager_test.dart | 10 +- .../lib/src/dart/analysis/driver.dart | 16 ++ 9 files changed, 279 insertions(+), 23 deletions(-) create mode 100644 pkg/analysis_server/lib/src/computer/new_notifications.dart diff --git a/pkg/analysis_server/lib/src/analysis_server.dart b/pkg/analysis_server/lib/src/analysis_server.dart index fdef53a210d..b0396660bf8 100644 --- a/pkg/analysis_server/lib/src/analysis_server.dart +++ b/pkg/analysis_server/lib/src/analysis_server.dart @@ -14,6 +14,7 @@ import 'package:analysis_server/plugin/protocol/protocol.dart' hide AnalysisOptions, Element; import 'package:analysis_server/src/analysis_logger.dart'; import 'package:analysis_server/src/channel/channel.dart'; +import 'package:analysis_server/src/computer/new_notifications.dart'; import 'package:analysis_server/src/context_manager.dart'; import 'package:analysis_server/src/operation/operation.dart'; import 'package:analysis_server/src/operation/operation_analysis.dart'; @@ -32,6 +33,9 @@ import 'package:analyzer/instrumentation/instrumentation.dart'; import 'package:analyzer/plugin/resolver_provider.dart'; import 'package:analyzer/source/pub_package_map_provider.dart'; import 'package:analyzer/src/context/builder.dart'; +import 'package:analyzer/src/dart/analysis/byte_store.dart'; +import 'package:analyzer/src/dart/analysis/driver.dart' as nd; +import 'package:analyzer/src/dart/analysis/file_byte_store.dart'; import 'package:analyzer/src/dart/ast/utilities.dart'; import 'package:analyzer/src/generated/engine.dart'; import 'package:analyzer/src/generated/sdk.dart'; @@ -307,6 +311,8 @@ class AnalysisServer { */ PubSummaryManager pubSummaryManager; + ByteStore byteStore; + /** * Initialize a newly created server to receive requests from and send * responses to the given [channel]. @@ -341,6 +347,10 @@ class AnalysisServer { options.finerGrainedInvalidation; defaultContextOptions.generateImplicitErrors = false; operationQueue = new ServerOperationQueue(); + byteStore = new MemoryCachingByteStore( + new FileByteStore( + resourceProvider.getStateLocation('.analysis-driver')), + 1024); if (useSingleContextManager) { contextManager = new SingleContextManager(resourceProvider, sdkManager, packageResolverProvider, analyzedFilesGlobs, defaultContextOptions); @@ -352,7 +362,8 @@ class AnalysisServer { packageMapProvider, analyzedFilesGlobs, instrumentationService, - defaultContextOptions); + defaultContextOptions, + options.enableNewAnalysisDriver); } this.fileResolverProvider = fileResolverProvider; this.packageResolverProvider = packageResolverProvider; @@ -411,6 +422,11 @@ class AnalysisServer { return _analyzedFilesGlobs; } + /** + * A table mapping [Folder]s to the [AnalysisDriver]s associated with them. + */ + Map get driverMap => contextManager.driverMap; + /** * Return a table mapping [Folder]s to the [AnalysisContext]s associated with * them. @@ -1086,6 +1102,10 @@ class AnalysisServer { */ void setAnalysisSubscriptions( Map> subscriptions) { + if (options.enableNewAnalysisDriver) { + // TODO(scheglov) implement for the new analysis driver + return; + } // send notifications for already analyzed sources subscriptions.forEach((service, Set newFiles) { Set oldFiles = analysisServices[service]; @@ -1175,6 +1195,12 @@ class AnalysisServer { * Set the priority files to the given [files]. */ void setPriorityFiles(String requestId, List files) { + if (options.enableNewAnalysisDriver) { + driverMap.values.forEach((driver) { + driver.priorityFiles = files; + }); + return; + } // Note: when a file is a priority file, that information needs to be // propagated to all contexts that analyze the file, so that all contexts // will be able to do incremental resolution of the file. See @@ -1294,6 +1320,45 @@ class AnalysisServer { * Implementation for `analysis.updateContent`. */ void updateContent(String id, Map changes) { + if (options.enableNewAnalysisDriver) { + changes.forEach((file, change) { + Source source = resourceProvider.getFile(file).createSource(); + // Prepare the new contents. + String oldContents = overlayState.getContents(source); + String newContents; + if (change is AddContentOverlay) { + newContents = change.content; + } else if (change is ChangeContentOverlay) { + if (oldContents == null) { + // The client may only send a ChangeContentOverlay if there is + // already an existing overlay for the source. + throw new RequestFailure(new Response(id, + error: new RequestError(RequestErrorCode.INVALID_OVERLAY_CHANGE, + 'Invalid overlay change'))); + } + try { + newContents = SourceEdit.applySequence(oldContents, change.edits); + } on RangeError { + throw new RequestFailure(new Response(id, + error: new RequestError(RequestErrorCode.INVALID_OVERLAY_CHANGE, + 'Invalid overlay change'))); + } + } else if (change is RemoveContentOverlay) { + newContents = null; + } else { + // Protocol parsing should have ensured that we never get here. + throw new AnalysisException('Illegal change type'); + } + + overlayState.setContents(source, newContents); + + driverMap.values.forEach((driver) { + driver.changeFile(file); + }); + // TODO(scheglov) implement other cases + }); + return; + } changes.forEach((file, change) { ContextSourcePair contextSource = getContextSourcePair(file); Source source = contextSource.source; @@ -1406,6 +1471,10 @@ class AnalysisServer { * existing analysis context. */ void updateOptions(List optionUpdaters) { + if (options.enableNewAnalysisDriver) { + // TODO(scheglov) implement for the new analysis driver + return; + } // // Update existing contexts. // @@ -1549,6 +1618,7 @@ class AnalysisServer { class AnalysisServerOptions { bool enableIncrementalResolutionApi = false; bool enableIncrementalResolutionValidation = false; + bool enableNewAnalysisDriver = false; bool enablePubSummaryManager = false; bool finerGrainedInvalidation = false; bool noErrorNotification = false; @@ -1597,6 +1667,56 @@ class ServerContextManagerCallbacks extends ContextManagerCallbacks { ServerContextManagerCallbacks(this.analysisServer, this.resourceProvider); + @override + nd.AnalysisDriver addAnalysisDriver(Folder folder, AnalysisOptions options) { + SourceFactory sourceFactory; + AnalysisOptions analysisOptions; + { + ContextBuilder builder = createContextBuilder(folder, options); + AnalysisContext context = builder.buildContext(folder.path); + sourceFactory = context.sourceFactory; + analysisOptions = context.analysisOptions; + context.dispose(); + } + nd.AnalysisDriver analysisDriver = new nd.AnalysisDriver( + new nd.PerformanceLog(io.stdout), + resourceProvider, + analysisServer.byteStore, + analysisServer.overlayState, + sourceFactory, + analysisOptions); + analysisDriver.name = folder.shortName; + analysisDriver.status.listen((status) { + // TODO(scheglov) send server status + }); + analysisDriver.results.listen((result) { + new_sendErrorNotification(analysisServer, result); + // TODO(scheglov) Implement more notifications. + // HIGHLIGHTS + // IMPLEMENTED + // NAVIGATION + // OVERRIDES + // OCCURRENCES (not used in IDEA) + // OUTLINE (not used in IDEA) +// { +// var unit = result.unit; +// if (unit != null) { +// print('[results][${analysisDriver.name}] ${result.path}'); +// sendAnalysisNotificationHighlights(analysisServer, result.path, unit); +// { +// NavigationCollectorImpl collector = +// computeSimpleDartNavigation(unit); +// var params = new protocol.AnalysisNavigationParams(result.path, +// collector.regions, collector.targets, collector.files); +// analysisServer.sendNotification(params.toNotification()); +// } +// } +// } + }); + analysisServer.driverMap[folder] = analysisDriver; + return analysisDriver; + } + @override AnalysisContext addContext(Folder folder, AnalysisOptions options) { ContextBuilder builder = createContextBuilder(folder, options); @@ -1612,15 +1732,31 @@ class ServerContextManagerCallbacks extends ContextManagerCallbacks { @override void applyChangesToContext(Folder contextFolder, ChangeSet changeSet) { - AnalysisContext context = analysisServer.folderMap[contextFolder]; - if (context != null) { - context.applyChanges(changeSet); - analysisServer.schedulePerformAnalysisOperation(context); - List flushedFiles = new List(); - for (Source source in changeSet.removedSources) { - flushedFiles.add(source.fullName); + if (analysisServer.options.enableNewAnalysisDriver) { + nd.AnalysisDriver analysisDriver = + analysisServer.driverMap[contextFolder]; + if (analysisDriver != null) { + changeSet.addedSources.forEach((source) { + analysisDriver.addFile(source.fullName); + }); + changeSet.changedSources.forEach((source) { + analysisDriver.changeFile(source.fullName); + }); + changeSet.removedSources.forEach((source) { + analysisDriver.removeFile(source.fullName); + }); + } + } else { + AnalysisContext context = analysisServer.folderMap[contextFolder]; + if (context != null) { + context.applyChanges(changeSet); + analysisServer.schedulePerformAnalysisOperation(context); + List flushedFiles = new List(); + for (Source source in changeSet.removedSources) { + flushedFiles.add(source.fullName); + } + sendAnalysisNotificationFlushResults(analysisServer, flushedFiles); } - sendAnalysisNotificationFlushResults(analysisServer, flushedFiles); } } diff --git a/pkg/analysis_server/lib/src/computer/new_notifications.dart b/pkg/analysis_server/lib/src/computer/new_notifications.dart new file mode 100644 index 00000000000..f1c6cee40f0 --- /dev/null +++ b/pkg/analysis_server/lib/src/computer/new_notifications.dart @@ -0,0 +1,21 @@ +// 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. + +import 'package:analysis_server/plugin/protocol/protocol.dart' + show AnalysisError; +import 'package:analysis_server/src/analysis_server.dart' show AnalysisServer; +import 'package:analysis_server/src/protocol_server.dart' as protocol; +import 'package:analyzer/src/dart/analysis/driver.dart'; +import 'package:analyzer/src/generated/source.dart'; + +void new_sendErrorNotification( + AnalysisServer analysisServer, AnalysisResult result) { + List serverErrors = []; + for (var error in result.errors) { + serverErrors + .add(protocol.newAnalysisError_fromEngine(new LineInfo([0]), error)); + } + var params = new protocol.AnalysisErrorsParams(result.path, serverErrors); + analysisServer.sendNotification(params.toNotification()); +} diff --git a/pkg/analysis_server/lib/src/context_manager.dart b/pkg/analysis_server/lib/src/context_manager.dart index 66977c472fc..59376dde8d8 100644 --- a/pkg/analysis_server/lib/src/context_manager.dart +++ b/pkg/analysis_server/lib/src/context_manager.dart @@ -24,6 +24,7 @@ import 'package:analyzer/source/pub_package_map_provider.dart'; import 'package:analyzer/source/sdk_ext.dart'; import 'package:analyzer/src/context/builder.dart'; import 'package:analyzer/src/context/context.dart' as context; +import 'package:analyzer/src/dart/analysis/driver.dart'; import 'package:analyzer/src/dart/sdk/sdk.dart'; import 'package:analyzer/src/generated/engine.dart'; import 'package:analyzer/src/generated/java_io.dart'; @@ -89,6 +90,11 @@ class ContextInfo { */ Set _dependencies = new Set(); + /** + * The analysis driver that was created for the [folder]. + */ + AnalysisDriver analysisDriver; + /** * The analysis context that was created for the [folder]. */ @@ -245,6 +251,11 @@ abstract class ContextManager { */ void set callbacks(ContextManagerCallbacks value); + /** + * A table mapping [Folder]s to the [AnalysisDriver]s associated with them. + */ + Map get driverMap; + /** * Return the list of excluded paths (folders and files) most recently passed * to [setRoots]. @@ -320,6 +331,12 @@ abstract class ContextManager { * modified. */ abstract class ContextManagerCallbacks { + /** + * Create and return a new analysis driver rooted at the given [folder], with + * the given analysis [options]. + */ + AnalysisDriver addAnalysisDriver(Folder folder, AnalysisOptions options); + /** * Create and return a new analysis context rooted at the given [folder], with * the given analysis [options]. @@ -479,6 +496,8 @@ class ContextManagerImpl implements ContextManager { */ final InstrumentationService _instrumentationService; + final bool enableNewAnalysisDriver; + @override ContextManagerCallbacks callbacks; @@ -488,11 +507,14 @@ class ContextManagerImpl implements ContextManager { */ final ContextInfo rootInfo = new ContextInfo._root(); + @override + final Map driverMap = + new HashMap(); + /** * A table mapping [Folder]s to the [AnalysisContext]s associated with them. */ - @override - final Map folderMap = + final Map _folderMap = new HashMap(); /** @@ -509,7 +531,8 @@ class ContextManagerImpl implements ContextManager { this._packageMapProvider, this.analyzedFilesGlobs, this._instrumentationService, - this.defaultContextOptions) { + this.defaultContextOptions, + this.enableNewAnalysisDriver) { absolutePathContext = resourceProvider.absolutePathContext; pathContext = resourceProvider.pathContext; } @@ -517,6 +540,14 @@ class ContextManagerImpl implements ContextManager { @override Iterable get analysisContexts => folderMap.values; + Map get folderMap { + if (enableNewAnalysisDriver) { + throw new StateError('Should not be used with the new analysis driver'); + } else { + return _folderMap; + } + } + @override List contextsInAnalysisRoot(Folder analysisRoot) { List contexts = []; @@ -1033,9 +1064,13 @@ class ContextManagerImpl implements ContextManager { applyToAnalysisOptions(options, optionMap); info.setDependencies(dependencies); - info.context = callbacks.addContext(folder, options); - folderMap[folder] = info.context; - info.context.name = folder.path; + if (enableNewAnalysisDriver) { + info.analysisDriver = callbacks.addAnalysisDriver(folder, options); + } else { + info.context = callbacks.addContext(folder, options); + _folderMap[folder] = info.context; + info.context.name = folder.path; + } // Look for pubspec-specified analysis configuration. File pubspec; @@ -1052,13 +1087,21 @@ class ContextManagerImpl implements ContextManager { } if (pubspec != null) { File pubSource = resourceProvider.getFile(pubspec.path); - setConfiguration( - info.context, - new AnalysisConfiguration.fromPubspec( - pubSource, resourceProvider, disposition.packages)); + if (enableNewAnalysisDriver) { + // TODO(scheglov) implement for the new analysis driver + } else { + setConfiguration( + info.context, + new AnalysisConfiguration.fromPubspec( + pubSource, resourceProvider, disposition.packages)); + } } - processOptionsForContext(info, optionMap); + if (enableNewAnalysisDriver) { + // TODO(scheglov) implement for the new analysis driver + } else { + processOptionsForContext(info, optionMap); + } return info; } diff --git a/pkg/analysis_server/lib/src/domain_completion.dart b/pkg/analysis_server/lib/src/domain_completion.dart index f9ea745ddb2..4c2221a6ff0 100644 --- a/pkg/analysis_server/lib/src/domain_completion.dart +++ b/pkg/analysis_server/lib/src/domain_completion.dart @@ -102,6 +102,10 @@ class CompletionDomainHandler implements RequestHandler { @override Response handleRequest(Request request) { + if (server.options.enableNewAnalysisDriver) { + // TODO(scheglov) implement for the new analysis driver + return new CompletionGetSuggestionsResult('0').toResponse(request.id); + } if (server.searchEngine == null) { return new Response.noIndexGenerated(request); } diff --git a/pkg/analysis_server/lib/src/edit/edit_domain.dart b/pkg/analysis_server/lib/src/edit/edit_domain.dart index 44e9ea7a206..c0323a442e2 100644 --- a/pkg/analysis_server/lib/src/edit/edit_domain.dart +++ b/pkg/analysis_server/lib/src/edit/edit_domain.dart @@ -135,6 +135,10 @@ class EditDomainHandler implements RequestHandler { } Future getAssists(Request request) async { + if (server.options.enableNewAnalysisDriver) { + // TODO(scheglov) implement for the new analysis driver + return; + } EditGetAssistsParams params = new EditGetAssistsParams.fromRequest(request); ContextSourcePair pair = server.getContextSourcePair(params.file); engine.AnalysisContext context = pair.context; @@ -152,7 +156,11 @@ class EditDomainHandler implements RequestHandler { server.sendResponse(response); } - getFixes(Request request) async { + Future getFixes(Request request) async { + if (server.options.enableNewAnalysisDriver) { + // TODO(scheglov) implement for the new analysis driver + return; + } var params = new EditGetFixesParams.fromRequest(request); String file = params.file; int offset = params.offset; @@ -184,7 +192,7 @@ class EditDomainHandler implements RequestHandler { } } // respond - return server.sendResponse( + server.sendResponse( new EditGetFixesResult(errorFixesList).toResponse(request.id)); } diff --git a/pkg/analysis_server/lib/src/server/driver.dart b/pkg/analysis_server/lib/src/server/driver.dart index 3095df81c36..df6df7bc05d 100644 --- a/pkg/analysis_server/lib/src/server/driver.dart +++ b/pkg/analysis_server/lib/src/server/driver.dart @@ -242,6 +242,11 @@ class Driver implements ServerStarter { static const String INCREMENTAL_RESOLUTION_VALIDATION = "incremental-resolution-validation"; + /** + * The name of the option used to enable using pub summary manager. + */ + static const String ENABLE_NEW_ANALYSIS_DRIVER = 'enable-new-analysis-driver'; + /** * The name of the option used to enable using pub summary manager. */ @@ -383,6 +388,8 @@ class Driver implements ServerStarter { results[ENABLE_INCREMENTAL_RESOLUTION_API]; analysisServerOptions.enableIncrementalResolutionValidation = results[INCREMENTAL_RESOLUTION_VALIDATION]; + analysisServerOptions.enableNewAnalysisDriver = + results[ENABLE_NEW_ANALYSIS_DRIVER]; analysisServerOptions.enablePubSummaryManager = results[ENABLE_PUB_SUMMARY_MANAGER]; analysisServerOptions.finerGrainedInvalidation = @@ -532,6 +539,10 @@ class Driver implements ServerStarter { help: "enable validation of incremental resolution results (slow)", defaultsTo: false, negatable: false); + parser.addFlag(ENABLE_NEW_ANALYSIS_DRIVER, + help: "enable using new analysis driver", + defaultsTo: false, + negatable: false); parser.addFlag(ENABLE_PUB_SUMMARY_MANAGER, help: "enable using summaries for pub cache packages", defaultsTo: false, diff --git a/pkg/analysis_server/lib/src/single_context_manager.dart b/pkg/analysis_server/lib/src/single_context_manager.dart index 984bc7305cb..fa99c96ac20 100644 --- a/pkg/analysis_server/lib/src/single_context_manager.dart +++ b/pkg/analysis_server/lib/src/single_context_manager.dart @@ -11,6 +11,7 @@ import 'dart:math' as math; import 'package:analysis_server/src/context_manager.dart'; import 'package:analyzer/file_system/file_system.dart'; import 'package:analyzer/plugin/resolver_provider.dart'; +import 'package:analyzer/src/dart/analysis/driver.dart'; import 'package:analyzer/src/generated/engine.dart'; import 'package:analyzer/src/generated/sdk.dart'; import 'package:analyzer/src/generated/source.dart'; @@ -84,6 +85,11 @@ class SingleContextManager implements ContextManager { @override ContextManagerCallbacks callbacks; + /** + * The analysis driver which analyses everything. + */ + AnalysisDriver analysisDriver; + /** * The context in which everything is being analyzed. */ @@ -116,6 +122,9 @@ class SingleContextManager implements ContextManager { Iterable get analysisContexts => context == null ? [] : [context]; + @override + Map get driverMap => {contextFolder: analysisDriver}; + @override Map get folderMap => {contextFolder: context}; diff --git a/pkg/analysis_server/test/context_manager_test.dart b/pkg/analysis_server/test/context_manager_test.dart index 888c2ddd52e..ce0bf7dbb75 100644 --- a/pkg/analysis_server/test/context_manager_test.dart +++ b/pkg/analysis_server/test/context_manager_test.dart @@ -13,6 +13,7 @@ import 'package:analyzer/file_system/memory_file_system.dart'; import 'package:analyzer/instrumentation/instrumentation.dart'; import 'package:analyzer/source/error_processor.dart'; import 'package:analyzer/src/context/builder.dart'; +import 'package:analyzer/src/dart/analysis/driver.dart'; import 'package:analyzer/src/error/codes.dart'; import 'package:analyzer/src/generated/engine.dart'; import 'package:analyzer/src/generated/sdk.dart'; @@ -1801,7 +1802,8 @@ abstract class ContextManagerTest { packageMapProvider, analysisFilesGlobs, InstrumentationService.NULL_SERVICE, - new AnalysisOptionsImpl()); + new AnalysisOptionsImpl(), + false); callbacks = new TestContextManagerCallbacks(resourceProvider); manager.callbacks = callbacks; resourceProvider.newFolder(projPath); @@ -2654,6 +2656,12 @@ class TestContextManagerCallbacks extends ContextManagerCallbacks { */ Iterable get currentContextPaths => currentContextTimestamps.keys; + @override + AnalysisDriver addAnalysisDriver(Folder folder, AnalysisOptions options) { + // TODO: implement addAnalysisDriver + throw new UnimplementedError(); + } + @override AnalysisContext addContext(Folder folder, AnalysisOptions options) { String path = folder.path; diff --git a/pkg/analyzer/lib/src/dart/analysis/driver.dart b/pkg/analyzer/lib/src/dart/analysis/driver.dart index 6677376eb61..d7dc7c6969b 100644 --- a/pkg/analyzer/lib/src/dart/analysis/driver.dart +++ b/pkg/analyzer/lib/src/dart/analysis/driver.dart @@ -70,6 +70,7 @@ import 'package:crypto/crypto.dart'; * TODO(scheglov) Clean up the list of implicitly analyzed files. */ class AnalysisDriver { + String name; final PerformanceLog _logger; /** @@ -224,6 +225,7 @@ class AnalysisDriver { try { PerformanceLogSection analysisSection = null; while (true) { + await pumpEventQueue(100); await _hasWork.signal; if (analysisSection == null) { @@ -707,6 +709,20 @@ class AnalysisDriver { set.remove(element); return element; } + + /** + * 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)); + } } /**