From 597f189eb45af457c72ffa2b4e140a21f43c1914 Mon Sep 17 00:00:00 2001 From: Konstantin Shcheglov Date: Wed, 26 Aug 2015 09:45:19 -0700 Subject: [PATCH] Extension point for WorkManagerFactory(s). I'm not quite happy that internal classes get into the extension point declaration. But I guess that because of the nature of the work WorkManager(s) are doing, we have to have significant exposure to internals. Also, onAnalysisOptionsChanged() and onSourceFactoryChanged()... these probably are better to implement as streams in InternalAnalysisContext. Thoughts? R=brianwilkerson@google.com, paulberry@google.com BUG= Review URL: https://codereview.chromium.org//1311773005 . --- pkg/analyzer/lib/plugin/task.dart | 15 ++ pkg/analyzer/lib/src/context/cache.dart | 41 ------ pkg/analyzer/lib/src/context/context.dart | 100 +++++++------ .../src/generated/incremental_resolver.dart | 2 +- .../lib/src/plugin/engine_plugin.dart | 53 +++++++ .../lib/src/task/dart_work_manager.dart | 33 ++--- pkg/analyzer/lib/src/task/driver.dart | 85 ++---------- .../lib/src/task/html_work_manager.dart | 30 ++-- pkg/analyzer/lib/task/model.dart | 131 ++++++++++++++++++ pkg/analyzer/test/generated/engine_test.dart | 2 +- .../test/src/task/dart_work_manager_test.dart | 31 +++-- .../test/src/task/html_work_manager_test.dart | 14 +- 12 files changed, 322 insertions(+), 215 deletions(-) diff --git a/pkg/analyzer/lib/plugin/task.dart b/pkg/analyzer/lib/plugin/task.dart index 8ea132fa16f..72c33fa78fc 100644 --- a/pkg/analyzer/lib/plugin/task.dart +++ b/pkg/analyzer/lib/plugin/task.dart @@ -8,6 +8,7 @@ */ library analyzer.plugin.task; +import 'package:analyzer/src/generated/engine.dart' hide WorkManager; import 'package:analyzer/src/plugin/engine_plugin.dart'; import 'package:analyzer/task/model.dart'; import 'package:plugin/plugin.dart'; @@ -19,3 +20,17 @@ import 'package:plugin/plugin.dart'; */ final String TASK_EXTENSION_POINT_ID = Plugin.join( EnginePlugin.UNIQUE_IDENTIFIER, EnginePlugin.TASK_EXTENSION_POINT); + +/** + * The identifier of the extension point that allows plugins to register new + * work managers with the analysis engine. The object used as an extension must + * be a [WorkManagerFactory]. + */ +final String WORK_MANAGER_EXTENSION_POINT_ID = Plugin.join( + EnginePlugin.UNIQUE_IDENTIFIER, + EnginePlugin.WORK_MANAGER_FACTORY_EXTENSION_POINT); + +/** + * A function that will create a new [WorkManager] for the given [context]. + */ +typedef WorkManager WorkManagerFactory(InternalAnalysisContext context); diff --git a/pkg/analyzer/lib/src/context/cache.dart b/pkg/analyzer/lib/src/context/cache.dart index c12262d5b0b..d45c72986b4 100644 --- a/pkg/analyzer/lib/src/context/cache.dart +++ b/pkg/analyzer/lib/src/context/cache.dart @@ -12,7 +12,6 @@ import 'package:analyzer/src/generated/engine.dart' import 'package:analyzer/src/generated/java_engine.dart'; import 'package:analyzer/src/generated/source.dart'; import 'package:analyzer/src/generated/utilities_collection.dart'; -import 'package:analyzer/src/generated/utilities_general.dart'; import 'package:analyzer/src/task/model.dart'; import 'package:analyzer/task/model.dart'; @@ -1116,46 +1115,6 @@ class SdkCachePartition extends CachePartition { } } -/** - * A specification of a specific result computed for a specific target. - */ -class TargetedResult { - /** - * An empty list of results. - */ - static final List EMPTY_LIST = const []; - - /** - * The target with which the result is associated. - */ - final AnalysisTarget target; - - /** - * The result associated with the target. - */ - final ResultDescriptor result; - - /** - * Initialize a new targeted result. - */ - TargetedResult(this.target, this.result); - - @override - int get hashCode { - return JenkinsSmiHash.combine(target.hashCode, result.hashCode); - } - - @override - bool operator ==(other) { - return other is TargetedResult && - other.target == target && - other.result == result; - } - - @override - String toString() => '$result for $target'; -} - /** * A cache partition that contains all targets not contained in other partitions. */ diff --git a/pkg/analyzer/lib/src/context/context.dart b/pkg/analyzer/lib/src/context/context.dart index e48a5ccefa8..d4cf8feef6d 100644 --- a/pkg/analyzer/lib/src/context/context.dart +++ b/pkg/analyzer/lib/src/context/context.dart @@ -8,6 +8,7 @@ import 'dart:async'; import 'dart:collection'; import 'package:analyzer/instrumentation/instrumentation.dart'; +import 'package:analyzer/plugin/task.dart'; import 'package:analyzer/src/cancelable_future.dart'; import 'package:analyzer/src/context/cache.dart'; import 'package:analyzer/src/generated/ast.dart'; @@ -33,8 +34,6 @@ import 'package:analyzer/src/generated/utilities_collection.dart'; import 'package:analyzer/src/task/dart.dart'; import 'package:analyzer/src/task/dart_work_manager.dart'; import 'package:analyzer/src/task/driver.dart'; -import 'package:analyzer/src/task/html.dart'; -import 'package:analyzer/src/task/html_work_manager.dart'; import 'package:analyzer/src/task/incremental_element_builder.dart'; import 'package:analyzer/src/task/manager.dart'; import 'package:analyzer/task/dart.dart'; @@ -121,16 +120,16 @@ class AnalysisContextImpl implements InternalAnalysisContext { */ TaskManager _taskManager; + /** + * A list of all [WorkManager]s used by this context. + */ + final List workManagers = []; + /** * The [DartWorkManager] instance that performs Dart specific scheduling. */ DartWorkManager dartWorkManager; - /** - * The work manager that performs HTML specific scheduling. - */ - HtmlWorkManager htmlWorkManager; - /** * The analysis driver used to perform analysis. */ @@ -219,11 +218,17 @@ class AnalysisContextImpl implements InternalAnalysisContext { _privatePartition = new UniversalCachePartition(this); _cache = createCacheFromSourceFactory(null); _taskManager = AnalysisEngine.instance.taskManager; - // TODO(scheglov) Get WorkManager(Factory)(s) from plugins. - dartWorkManager = new DartWorkManager(this); - htmlWorkManager = new HtmlWorkManager(this); - driver = new AnalysisDriver( - _taskManager, [dartWorkManager, htmlWorkManager], this); + for (WorkManagerFactory factory + in AnalysisEngine.instance.enginePlugin.workManagerFactories) { + WorkManager workManager = factory(this); + if (workManager != null) { + workManagers.add(workManager); + if (workManager is DartWorkManager) { + dartWorkManager = workManager; + } + } + } + driver = new AnalysisDriver(_taskManager, workManagers, this); _onSourcesChangedController = new StreamController.broadcast(); _implicitAnalysisEventsController = @@ -268,8 +273,9 @@ class AnalysisContextImpl implements InternalAnalysisContext { this._options.lint = options.lint; this._options.preserveComments = options.preserveComments; if (needsRecompute) { - dartWorkManager.onAnalysisOptionsChanged(); - htmlWorkManager.onAnalysisOptionsChanged(); + for (WorkManager workManager in workManagers) { + workManager.onAnalysisOptionsChanged(); + } } } @@ -287,8 +293,9 @@ class AnalysisContextImpl implements InternalAnalysisContext { _priorityOrder = sources; } } - dartWorkManager.applyPriorityTargets(_priorityOrder); - htmlWorkManager.applyPriorityTargets(_priorityOrder); + for (WorkManager workManager in workManagers) { + workManager.applyPriorityTargets(_priorityOrder); + } } @override @@ -360,7 +367,8 @@ class AnalysisContextImpl implements InternalAnalysisContext { /** * Make _pendingFutureSources available to unit tests. */ - HashMap> get pendingFutureSources_forTesting => + HashMap> get pendingFutureSources_forTesting => _pendingFutureTargets; @override @@ -389,8 +397,9 @@ class AnalysisContextImpl implements InternalAnalysisContext { factory.context = this; _sourceFactory = factory; _cache = createCacheFromSourceFactory(factory); - dartWorkManager.onSourceFactoryChanged(); - htmlWorkManager.onSourceFactoryChanged(); + for (WorkManager workManager in workManagers) { + workManager.onSourceFactoryChanged(); + } } @override @@ -531,10 +540,10 @@ class AnalysisContextImpl implements InternalAnalysisContext { for (Source source in removedSources) { _sourceRemoved(source); } - dartWorkManager.applyChange( - changeSet.addedSources, changeSet.changedSources, removedSources); - htmlWorkManager.applyChange( - changeSet.addedSources, changeSet.changedSources, removedSources); + for (WorkManager workManager in workManagers) { + workManager.applyChange( + changeSet.addedSources, changeSet.changedSources, removedSources); + } _onSourcesChangedController.add(new SourcesChangedEvent(changeSet)); } @@ -635,8 +644,8 @@ class AnalysisContextImpl implements InternalAnalysisContext { return new CancelableFuture.error(new AnalysisNotScheduledError()); } var unitTarget = new LibrarySpecificUnit(librarySource, unitSource); - return new _AnalysisFutureHelper(this).computeAsync( - unitTarget, (CacheEntry entry) { + return new _AnalysisFutureHelper(this) + .computeAsync(unitTarget, (CacheEntry entry) { CacheState state = entry.getState(RESOLVED_UNIT); if (state == CacheState.ERROR) { throw entry.exception; @@ -801,13 +810,13 @@ class AnalysisContextImpl implements InternalAnalysisContext { @override AnalysisErrorInfo getErrors(Source source) { - String name = source.shortName; - if (AnalysisEngine.isDartFileName(name) || source is DartScript) { - return dartWorkManager.getErrors(source); - } else if (AnalysisEngine.isHtmlFileName(name)) { - return htmlWorkManager.getErrors(source); + List allErrors = []; + for (WorkManager workManager in workManagers) { + List errors = workManager.getErrors(source); + allErrors.addAll(errors); } - return new AnalysisErrorInfoImpl(AnalysisError.NO_ERRORS, null); + LineInfo lineInfo = getLineInfo(source); + return new AnalysisErrorInfoImpl(allErrors, lineInfo); } @override @@ -1530,8 +1539,12 @@ class AnalysisContextImpl implements InternalAnalysisContext { * related to it. If so, add the source to the set of sources that need to be * processed. This method is intended to be used for testing purposes only. */ - void _getSourcesNeedingProcessing(Source source, CacheEntry entry, - bool isPriority, bool hintsEnabled, bool lintsEnabled, + void _getSourcesNeedingProcessing( + Source source, + CacheEntry entry, + bool isPriority, + bool hintsEnabled, + bool lintsEnabled, HashSet sources) { CacheState state = entry.getState(CONTENT); if (state == CacheState.INVALID || @@ -1763,10 +1776,10 @@ class AnalysisContextImpl implements InternalAnalysisContext { } entry.setState(CONTENT, CacheState.INVALID); } - dartWorkManager.applyChange( - Source.EMPTY_LIST, [source], Source.EMPTY_LIST); - htmlWorkManager.applyChange( - Source.EMPTY_LIST, [source], Source.EMPTY_LIST); + for (WorkManager workManager in workManagers) { + workManager.applyChange( + Source.EMPTY_LIST, [source], Source.EMPTY_LIST); + } } /** @@ -1846,11 +1859,18 @@ class AnalysisContextImpl implements InternalAnalysisContext { // do resolution Stopwatch perfCounter = new Stopwatch()..start(); PoorMansIncrementalResolver resolver = new PoorMansIncrementalResolver( - typeProvider, unitSource, null, sourceEntry, unitEntry, oldUnit, - analysisOptions.incrementalApi, analysisOptions); + typeProvider, + unitSource, + null, + sourceEntry, + unitEntry, + oldUnit, + analysisOptions.incrementalApi, + analysisOptions); bool success = resolver.resolve(newCode); AnalysisEngine.instance.instrumentationService.logPerformance( - AnalysisPerformanceKind.INCREMENTAL, perfCounter, + AnalysisPerformanceKind.INCREMENTAL, + perfCounter, 'success=$success,context_id=$_id,code_length=${newCode.length}'); if (!success) { return false; diff --git a/pkg/analyzer/lib/src/generated/incremental_resolver.dart b/pkg/analyzer/lib/src/generated/incremental_resolver.dart index 2c360e15aab..ea1e0fa9442 100644 --- a/pkg/analyzer/lib/src/generated/incremental_resolver.dart +++ b/pkg/analyzer/lib/src/generated/incremental_resolver.dart @@ -24,7 +24,7 @@ import 'package:analyzer/src/task/dart.dart' import 'package:analyzer/task/dart.dart' show DART_ERRORS, LibrarySpecificUnit, PARSED_UNIT, TOKEN_STREAM; import 'package:analyzer/task/general.dart' show CONTENT, LINE_INFO; -import 'package:analyzer/task/model.dart' show ResultDescriptor; +import 'package:analyzer/task/model.dart' show ResultDescriptor, TargetedResult; import 'ast.dart'; import 'element.dart'; diff --git a/pkg/analyzer/lib/src/plugin/engine_plugin.dart b/pkg/analyzer/lib/src/plugin/engine_plugin.dart index eb516de99ac..3b80617ace6 100644 --- a/pkg/analyzer/lib/src/plugin/engine_plugin.dart +++ b/pkg/analyzer/lib/src/plugin/engine_plugin.dart @@ -5,9 +5,13 @@ library analyzer.src.plugin.engine_plugin; import 'package:analyzer/plugin/task.dart'; +import 'package:analyzer/src/generated/engine.dart' + show InternalAnalysisContext; import 'package:analyzer/src/task/dart.dart'; +import 'package:analyzer/src/task/dart_work_manager.dart'; import 'package:analyzer/src/task/general.dart'; import 'package:analyzer/src/task/html.dart'; +import 'package:analyzer/src/task/html_work_manager.dart'; import 'package:analyzer/task/model.dart'; import 'package:plugin/plugin.dart'; @@ -22,6 +26,13 @@ class EnginePlugin implements Plugin { */ static const String TASK_EXTENSION_POINT = 'task'; + /** + * The simple identifier of the extension point that allows plugins to + * register new work manager factories with the analysis engine. + */ + static const String WORK_MANAGER_FACTORY_EXTENSION_POINT = + 'workManagerFactory'; + /** * The unique identifier of this plugin. */ @@ -33,6 +44,12 @@ class EnginePlugin implements Plugin { */ ExtensionPoint taskExtensionPoint; + /** + * The extension point that allows plugins to register new work manager + * factories with the analysis engine. + */ + ExtensionPoint workManagerFactoryExtensionPoint; + /** * Initialize a newly created plugin. */ @@ -46,14 +63,29 @@ class EnginePlugin implements Plugin { @override String get uniqueIdentifier => UNIQUE_IDENTIFIER; + /** + * Return a list containing all of the work manager factories that were + * contributed. + */ + List get workManagerFactories => + workManagerFactoryExtensionPoint.extensions; + @override void registerExtensionPoints(RegisterExtensionPoint registerExtensionPoint) { taskExtensionPoint = registerExtensionPoint(TASK_EXTENSION_POINT, _validateTaskExtension); + workManagerFactoryExtensionPoint = registerExtensionPoint( + WORK_MANAGER_FACTORY_EXTENSION_POINT, + _validateWorkManagerFactoryExtension); } @override void registerExtensions(RegisterExtension registerExtension) { + _registerTaskExtensions(registerExtension); + _registerWorkManagerFactoryExtensions(registerExtension); + } + + void _registerTaskExtensions(RegisterExtension registerExtension) { String taskId = TASK_EXTENSION_POINT_ID; // // Register general tasks. @@ -97,6 +129,15 @@ class EnginePlugin implements Plugin { registerExtension(taskId, ParseHtmlTask.DESCRIPTOR); } + void _registerWorkManagerFactoryExtensions( + RegisterExtension registerExtension) { + String taskId = WORK_MANAGER_EXTENSION_POINT_ID; + registerExtension(taskId, + (InternalAnalysisContext context) => new DartWorkManager(context)); + registerExtension(taskId, + (InternalAnalysisContext context) => new HtmlWorkManager(context)); + } + /** * Validate the given extension by throwing an [ExtensionError] if it is not a * valid domain. @@ -107,4 +148,16 @@ class EnginePlugin implements Plugin { throw new ExtensionError('Extensions to $id must be a TaskDescriptor'); } } + + /** + * Validate the given extension by throwing an [ExtensionError] if it is not a + * valid domain. + */ + void _validateWorkManagerFactoryExtension(Object extension) { + if (extension is! WorkManagerFactory) { + String id = taskExtensionPoint.uniqueIdentifier; + throw new ExtensionError( + 'Extensions to $id must be a WorkManagerFactory'); + } + } } diff --git a/pkg/analyzer/lib/src/task/dart_work_manager.dart b/pkg/analyzer/lib/src/task/dart_work_manager.dart index c02d5947da9..f813fe9df20 100644 --- a/pkg/analyzer/lib/src/task/dart_work_manager.dart +++ b/pkg/analyzer/lib/src/task/dart_work_manager.dart @@ -21,8 +21,8 @@ import 'package:analyzer/src/generated/utilities_collection.dart'; import 'package:analyzer/src/task/dart.dart'; import 'package:analyzer/src/task/driver.dart'; import 'package:analyzer/task/dart.dart'; -import 'package:analyzer/task/general.dart'; import 'package:analyzer/task/model.dart'; +import 'package:analyzer/src/task/html.dart'; /** * The manager for Dart specific analysis. @@ -113,9 +113,7 @@ class DartWorkManager implements WorkManager { priorityResultQueue.add(new TargetedResult(target, result)); } - /** - * Notifies the manager about changes in the explicit source list. - */ + @override void applyChange(List addedSources, List changedSources, List removedSources) { addedSources = addedSources.where(_isDartSource).toList(); @@ -166,18 +164,16 @@ class DartWorkManager implements WorkManager { } } - /** - * Return an [AnalysisErrorInfo] containing the list of all of the errors and - * the line info associated with the given [source]. The list of errors will - * be empty if the source is not known to the context or if there are no - * errors in the source. The errors contained in the list can be incomplete. - */ - AnalysisErrorInfo getErrors(Source source) { - if (analysisCache.getState(source, DART_ERRORS) == CacheState.VALID) { - List errors = analysisCache.getValue(source, DART_ERRORS); - LineInfo lineInfo = analysisCache.getValue(source, LINE_INFO); - return new AnalysisErrorInfoImpl(errors, lineInfo); + @override + List getErrors(Source source) { + if (!_isDartSource(source) && source is! DartScript) { + return AnalysisError.NO_ERRORS; } + // If analysis is finished, use all the errors. + if (analysisCache.getState(source, DART_ERRORS) == CacheState.VALID) { + return analysisCache.getValue(source, DART_ERRORS); + } + // If analysis is in progress, combine all known partial results. List errors = []; for (ResultDescriptor descriptor in _SOURCE_ERRORS) { errors.addAll(analysisCache.getValue(source, descriptor)); @@ -188,8 +184,7 @@ class DartWorkManager implements WorkManager { errors.addAll(analysisCache.getValue(unit, descriptor)); } } - LineInfo lineInfo = analysisCache.getValue(source, LINE_INFO); - return new AnalysisErrorInfoImpl(errors, lineInfo); + return errors; } /** @@ -315,7 +310,7 @@ class DartWorkManager implements WorkManager { } }); if (shouldSetErrors) { - AnalysisErrorInfo info = getErrors(target); + AnalysisErrorInfo info = context.getErrors(target); context.getNotice(target).setErrors(info.errors, info.lineInfo); } } @@ -329,7 +324,7 @@ class DartWorkManager implements WorkManager { } }); if (shouldSetErrors) { - AnalysisErrorInfo info = getErrors(source); + AnalysisErrorInfo info = context.getErrors(source); context.getNotice(source).setErrors(info.errors, info.lineInfo); } } diff --git a/pkg/analyzer/lib/src/task/driver.dart b/pkg/analyzer/lib/src/task/driver.dart index ff5767db903..e4210d50ec5 100644 --- a/pkg/analyzer/lib/src/task/driver.dart +++ b/pkg/analyzer/lib/src/task/driver.dart @@ -9,7 +9,7 @@ import 'dart:collection'; import 'package:analyzer/src/context/cache.dart'; import 'package:analyzer/src/generated/engine.dart' - hide AnalysisTask, AnalysisContextImpl; + hide AnalysisTask, AnalysisContextImpl, WorkManager; import 'package:analyzer/src/generated/java_engine.dart'; import 'package:analyzer/src/generated/resolver.dart'; import 'package:analyzer/src/generated/utilities_general.dart'; @@ -44,7 +44,8 @@ class AnalysisDriver { /** * The map of [ComputedResult] controllers. */ - final Map> resultComputedControllers = + final Map> resultComputedControllers = >{}; /** @@ -197,8 +198,10 @@ class AnalysisDriver { * [descriptor] is computed. */ Stream onResultComputed(ResultDescriptor descriptor) { - return resultComputedControllers.putIfAbsent(descriptor, () => - new StreamController.broadcast(sync: true)).stream; + return resultComputedControllers + .putIfAbsent(descriptor, + () => new StreamController.broadcast(sync: true)) + .stream; } /** @@ -476,8 +479,9 @@ class InfiniteTaskLoopException extends AnalysisException { * Initialize a newly created exception to represent a failed attempt to * perform the given [task] due to the given [dependencyCycle]. */ - InfiniteTaskLoopException(AnalysisTask task, this.dependencyCycle) : super( - 'Infinite loop while performing task ${task.descriptor.name} for ${task.target}'); + InfiniteTaskLoopException(AnalysisTask task, this.dependencyCycle) + : super( + 'Infinite loop while performing task ${task.descriptor.name} for ${task.target}'); } /** @@ -565,10 +569,10 @@ class WorkItem { * described by the given descriptor. */ WorkItem(this.context, this.target, this.descriptor, this.spawningResult) { - AnalysisTarget actualTarget = identical( - target, AnalysisContextTarget.request) - ? new AnalysisContextTarget(context) - : target; + AnalysisTarget actualTarget = + identical(target, AnalysisContextTarget.request) + ? new AnalysisContextTarget(context) + : target; Map inputDescriptors = descriptor.createTaskInputs(actualTarget); builder = new TopLevelTaskInputBuilder(inputDescriptors); @@ -671,42 +675,6 @@ class WorkItem { String toString() => 'Run $descriptor on $target'; } -/** - * [AnalysisDriver] uses [WorkManager]s to select results to compute. - * - * They know specific of the targets and results they care about, - * so they can request analysis results in optimal order. - */ -abstract class WorkManager { - /** - * Notifies the managers that the given set of priority [targets] was set. - */ - void applyPriorityTargets(List targets); - - /** - * Return the next [TargetedResult] that this work manager wants to be - * computed, or `null` if this manager doesn't need any new results. - */ - TargetedResult getNextResult(); - - /** - * Return the priority if the next work order this work manager want to be - * computed. The [AnalysisDriver] will perform the work order with - * the highest priority. - * - * Even if the returned value is [WorkOrderPriority.NONE], it still does not - * guarantee that [getNextResult] will return not `null`. - */ - WorkOrderPriority getNextResultPriority(); - - /** - * Notifies the manager that the given [outputs] were produced for - * the given [target]. - */ - void resultsComputed( - AnalysisTarget target, Map outputs); -} - /** * A description of the work to be done to compute a desired analysis result. * The class implements a lazy depth-first traversal of the work item's input. @@ -773,31 +741,6 @@ class WorkOrder implements Iterator { } } -/** - * The priorities of work orders returned by [WorkManager]s. - */ -enum WorkOrderPriority { - /** - * Responding to an user's action. - */ - INTERACTIVE, - - /** - * Computing information for priority sources. - */ - PRIORITY, - - /** - * A work should be done, but without any special urgency. - */ - NORMAL, - - /** - * Nothing to do. - */ - NONE -} - /** * Specilaization of [CycleAwareDependencyWalker] for use by [WorkOrder]. */ diff --git a/pkg/analyzer/lib/src/task/html_work_manager.dart b/pkg/analyzer/lib/src/task/html_work_manager.dart index d438d51929f..faedfb25f7e 100644 --- a/pkg/analyzer/lib/src/task/html_work_manager.dart +++ b/pkg/analyzer/lib/src/task/html_work_manager.dart @@ -20,7 +20,6 @@ import 'package:analyzer/src/generated/source.dart'; import 'package:analyzer/src/generated/utilities_collection.dart'; import 'package:analyzer/src/task/driver.dart'; import 'package:analyzer/src/task/html.dart'; -import 'package:analyzer/task/general.dart'; import 'package:analyzer/task/html.dart'; import 'package:analyzer/task/model.dart'; @@ -71,9 +70,7 @@ class HtmlWorkManager implements WorkManager { priorityResultQueue.add(new TargetedResult(target, result)); } - /** - * Notifies the manager about changes in the explicit source list. - */ + @override void applyChange(List addedSources, List changedSources, List removedSources) { addedSources = addedSources.where(_isHtmlSource).toList(); @@ -103,26 +100,23 @@ class HtmlWorkManager implements WorkManager { } } - /** - * Return an [AnalysisErrorInfo] containing the list of all of the errors and - * the line info associated with the given [source]. The list of errors will - * be empty if the source is not known to the context or if there are no - * errors in the source. The errors contained in the list can be incomplete. - */ - AnalysisErrorInfo getErrors(Source source) { - if (analysisCache.getState(source, HTML_ERRORS) == CacheState.VALID) { - List errors = analysisCache.getValue(source, HTML_ERRORS); - LineInfo lineInfo = analysisCache.getValue(source, LINE_INFO); - return new AnalysisErrorInfoImpl(errors, lineInfo); + @override + List getErrors(Source source) { + if (!_isHtmlSource(source)) { + return AnalysisError.NO_ERRORS; } + // If analysis is finished, use all the errors. + if (analysisCache.getState(source, HTML_ERRORS) == CacheState.VALID) { + return analysisCache.getValue(source, HTML_ERRORS); + } + // If analysis is in progress, combine all known partial results. List errors = []; errors.addAll(analysisCache.getValue(source, HTML_DOCUMENT_ERRORS)); List scripts = analysisCache.getValue(source, DART_SCRIPTS); for (DartScript script in scripts) { errors.addAll(context.getErrors(script).errors); } - LineInfo lineInfo = analysisCache.getValue(source, LINE_INFO); - return new AnalysisErrorInfoImpl(errors, lineInfo); + return errors; } @override @@ -210,7 +204,7 @@ class HtmlWorkManager implements WorkManager { } }); if (shouldSetErrors) { - AnalysisErrorInfo info = getErrors(target); + AnalysisErrorInfo info = context.getErrors(target); context.getNotice(target).setErrors(info.errors, info.lineInfo); } } diff --git a/pkg/analyzer/lib/task/model.dart b/pkg/analyzer/lib/task/model.dart index 0a457661e82..93755761dcb 100644 --- a/pkg/analyzer/lib/task/model.dart +++ b/pkg/analyzer/lib/task/model.dart @@ -7,8 +7,10 @@ library analyzer.task.model; import 'dart:collection'; import 'package:analyzer/src/generated/engine.dart' hide AnalysisTask; +import 'package:analyzer/src/generated/error.dart' show AnalysisError; import 'package:analyzer/src/generated/java_engine.dart'; import 'package:analyzer/src/generated/source.dart'; +import 'package:analyzer/src/generated/utilities_general.dart'; import 'package:analyzer/src/task/driver.dart'; import 'package:analyzer/src/task/model.dart'; @@ -389,6 +391,48 @@ abstract class ResultDescriptor { TaskInput of(AnalysisTarget target); } +/** + * A specification of the given [result] for the given [target]. + * + * Clients are not expected to subtype this class. + */ +class TargetedResult { + /** + * An empty list of results. + */ + static final List EMPTY_LIST = const []; + + /** + * The target with which the result is associated. + */ + final AnalysisTarget target; + + /** + * The result associated with the target. + */ + final ResultDescriptor result; + + /** + * Initialize a new targeted result. + */ + TargetedResult(this.target, this.result); + + @override + int get hashCode { + return JenkinsSmiHash.combine(target.hashCode, result.hashCode); + } + + @override + bool operator ==(other) { + return other is TargetedResult && + other.target == target && + other.result == result; + } + + @override + String toString() => '$result for $target'; +} + /** * A description of an [AnalysisTask]. */ @@ -514,3 +558,90 @@ abstract class TaskInputBuilder { */ bool moveNext(); } + +/** + * [WorkManager]s are used to drive analysis. + * + * They know specific of the targets and results they care about, + * so they can request analysis results in optimal order. + */ +abstract class WorkManager { + /** + * Notifies the manager about changes in the explicit source list. + */ + void applyChange(List addedSources, List changedSources, + List removedSources); + + /** + * Notifies the managers that the given set of priority [targets] was set. + */ + void applyPriorityTargets(List targets); + + /** + * Return a list of all of the errors associated with the given [source]. + * The list of errors will be empty if the source is not known to the context + * or if there are no errors in the source. The errors contained in the list + * can be incomplete. + */ + List getErrors(Source source); + + /** + * Return the next [TargetedResult] that this work manager wants to be + * computed, or `null` if this manager doesn't need any new results. + */ + TargetedResult getNextResult(); + + /** + * Return the priority if the next work order this work manager want to be + * computed. The [AnalysisDriver] will perform the work order with + * the highest priority. + * + * Even if the returned value is [WorkOrderPriority.NONE], it still does not + * guarantee that [getNextResult] will return not `null`. + */ + WorkOrderPriority getNextResultPriority(); + + /** + * Notifies the manager about analysis options changes. + */ + void onAnalysisOptionsChanged(); + + /** + * Notifies the manager about [SourceFactory] changes. + */ + void onSourceFactoryChanged(); + + /** + * Notifies the manager that the given [outputs] were produced for + * the given [target]. + */ + void resultsComputed( + AnalysisTarget target, Map outputs); +} + +/** + * The priorities of work orders returned by [WorkManager]s. + * + * New priorities may be added with time, clients need to tolerate this. + */ +enum WorkOrderPriority { + /** + * Responding to an user's action. + */ + INTERACTIVE, + + /** + * Computing information for priority sources. + */ + PRIORITY, + + /** + * A work should be done, but without any special urgency. + */ + NORMAL, + + /** + * Nothing to do. + */ + NONE +} diff --git a/pkg/analyzer/test/generated/engine_test.dart b/pkg/analyzer/test/generated/engine_test.dart index 2afaef440da..9f68c013bc2 100644 --- a/pkg/analyzer/test/generated/engine_test.dart +++ b/pkg/analyzer/test/generated/engine_test.dart @@ -31,7 +31,7 @@ import 'package:analyzer/src/generated/testing/element_factory.dart'; import 'package:analyzer/src/generated/utilities_collection.dart'; import 'package:analyzer/src/services/lint.dart'; import 'package:analyzer/src/string_source.dart'; -import 'package:analyzer/task/model.dart' hide AnalysisTask; +import 'package:analyzer/task/model.dart' hide AnalysisTask, WorkManager; import 'package:html/dom.dart' show Document; import 'package:path/path.dart' as pathos; import 'package:typed_mock/typed_mock.dart'; diff --git a/pkg/analyzer/test/src/task/dart_work_manager_test.dart b/pkg/analyzer/test/src/task/dart_work_manager_test.dart index f0a1df49757..a5b6b26b6b2 100644 --- a/pkg/analyzer/test/src/task/dart_work_manager_test.dart +++ b/pkg/analyzer/test/src/task/dart_work_manager_test.dart @@ -9,6 +9,7 @@ import 'package:analyzer/src/generated/ast.dart'; import 'package:analyzer/src/generated/engine.dart' show AnalysisErrorInfo, + AnalysisErrorInfoImpl, CacheState, ChangeNoticeImpl, InternalAnalysisContext; @@ -294,15 +295,12 @@ class DartWorkManagerTest { AnalysisError error2 = new AnalysisError(source1, 2, 0, ScannerErrorCode.MISSING_DIGIT); when(context.getLibrariesContaining(source1)).thenReturn([source2]); - LineInfo lineInfo = new LineInfo([0]); - entry1.setValue(LINE_INFO, lineInfo, []); entry1.setValue(SCAN_ERRORS, [error1], []); context .getCacheEntry(new LibrarySpecificUnit(source2, source1)) .setValue(VERIFY_ERRORS, [error2], []); - AnalysisErrorInfo errorInfo = manager.getErrors(source1); - expect(errorInfo.errors, unorderedEquals([error1, error2])); - expect(errorInfo.lineInfo, lineInfo); + List errors = manager.getErrors(source1); + expect(errors, unorderedEquals([error1, error2])); } void test_getErrors_hasFullList() { @@ -311,12 +309,9 @@ class DartWorkManagerTest { AnalysisError error2 = new AnalysisError(source1, 2, 0, ScannerErrorCode.MISSING_DIGIT); when(context.getLibrariesContaining(source1)).thenReturn([source2]); - LineInfo lineInfo = new LineInfo([0]); - entry1.setValue(LINE_INFO, lineInfo, []); entry1.setValue(DART_ERRORS, [error1, error2], []); - AnalysisErrorInfo errorInfo = manager.getErrors(source1); - expect(errorInfo.errors, unorderedEquals([error1, error2])); - expect(errorInfo.lineInfo, lineInfo); + List errors = manager.getErrors(source1); + expect(errors, unorderedEquals([error1, error2])); } void test_getLibrariesContainingPart() { @@ -530,12 +525,14 @@ class DartWorkManagerTest { } void test_resultsComputed_errors_forLibrarySpecificUnit() { + LineInfo lineInfo = new LineInfo([0]); AnalysisError error1 = new AnalysisError(source1, 1, 0, ScannerErrorCode.MISSING_DIGIT); AnalysisError error2 = new AnalysisError(source1, 2, 0, ScannerErrorCode.MISSING_DIGIT); when(context.getLibrariesContaining(source1)).thenReturn([source2]); - LineInfo lineInfo = new LineInfo([0]); + when(context.getErrors(source1)) + .thenReturn(new AnalysisErrorInfoImpl([error1, error2], lineInfo)); entry1.setValue(LINE_INFO, lineInfo, []); entry1.setValue(SCAN_ERRORS, [error1], []); AnalysisTarget unitTarget = new LibrarySpecificUnit(source2, source1); @@ -552,12 +549,14 @@ class DartWorkManagerTest { } void test_resultsComputed_errors_forSource() { + LineInfo lineInfo = new LineInfo([0]); AnalysisError error1 = new AnalysisError(source1, 1, 0, ScannerErrorCode.MISSING_DIGIT); AnalysisError error2 = new AnalysisError(source1, 2, 0, ScannerErrorCode.MISSING_DIGIT); when(context.getLibrariesContaining(source1)).thenReturn([source2]); - LineInfo lineInfo = new LineInfo([0]); + when(context.getErrors(source1)) + .thenReturn(new AnalysisErrorInfoImpl([error1, error2], lineInfo)); entry1.setValue(LINE_INFO, lineInfo, []); entry1.setValue(SCAN_ERRORS, [error1], []); entry1.setValue(PARSE_ERRORS, [error2], []); @@ -615,8 +614,10 @@ class DartWorkManagerTest { } void test_resultsComputed_parsedUnit() { - when(context.getLibrariesContaining(source1)).thenReturn([]); LineInfo lineInfo = new LineInfo([0]); + when(context.getLibrariesContaining(source1)).thenReturn([]); + when(context.getErrors(source1)) + .thenReturn(new AnalysisErrorInfoImpl([], lineInfo)); entry1.setValue(LINE_INFO, lineInfo, []); CompilationUnit unit = AstFactory.compilationUnit(); manager.resultsComputed(source1, {PARSED_UNIT: unit}); @@ -627,8 +628,10 @@ class DartWorkManagerTest { } void test_resultsComputed_resolvedUnit() { - when(context.getLibrariesContaining(source2)).thenReturn([]); LineInfo lineInfo = new LineInfo([0]); + when(context.getLibrariesContaining(source2)).thenReturn([]); + when(context.getErrors(source2)) + .thenReturn(new AnalysisErrorInfoImpl([], lineInfo)); entry2.setValue(LINE_INFO, lineInfo, []); CompilationUnit unit = AstFactory.compilationUnit(); manager.resultsComputed( diff --git a/pkg/analyzer/test/src/task/html_work_manager_test.dart b/pkg/analyzer/test/src/task/html_work_manager_test.dart index 10fd07a2414..35bc13dd525 100644 --- a/pkg/analyzer/test/src/task/html_work_manager_test.dart +++ b/pkg/analyzer/test/src/task/html_work_manager_test.dart @@ -134,18 +134,15 @@ class HtmlWorkManagerTest { new AnalysisError(source1, 1, 0, HtmlErrorCode.PARSE_ERROR, ['']); AnalysisError error2 = new AnalysisError(source1, 2, 0, HtmlErrorCode.PARSE_ERROR, ['']); - LineInfo lineInfo = new LineInfo([0]); entry1.setValue(HTML_DOCUMENT_ERRORS, [error1], []); - entry1.setValue(LINE_INFO, lineInfo, []); DartScript script = new DartScript(source1, []); entry1.setValue(DART_SCRIPTS, [script], []); CacheEntry scriptEntry = context.getCacheEntry(script); scriptEntry.setValue(DART_ERRORS, [error2], []); - AnalysisErrorInfo errorInfo = manager.getErrors(source1); - expect(errorInfo.errors, unorderedEquals([error1, error2])); - expect(errorInfo.lineInfo, lineInfo); + List errors = manager.getErrors(source1); + expect(errors, unorderedEquals([error1, error2])); } void test_getErrors_partialList() { @@ -153,13 +150,10 @@ class HtmlWorkManagerTest { new AnalysisError(source1, 1, 0, HtmlErrorCode.PARSE_ERROR, ['']); AnalysisError error2 = new AnalysisError(source1, 2, 0, HtmlErrorCode.PARSE_ERROR, ['']); - LineInfo lineInfo = new LineInfo([0]); entry1.setValue(HTML_DOCUMENT_ERRORS, [error1, error2], []); - entry1.setValue(LINE_INFO, lineInfo, []); - AnalysisErrorInfo errorInfo = manager.getErrors(source1); - expect(errorInfo.errors, unorderedEquals([error1, error2])); - expect(errorInfo.lineInfo, lineInfo); + List errors = manager.getErrors(source1); + expect(errors, unorderedEquals([error1, error2])); } void test_getNextResult_hasNormal_firstIsError() {