Improvements to task model

pkg/analyzer/lib/src/task/model.dart

Index: pkg/analyzer/lib/src/task/model.dart
diff --git a/pkg/analyzer/lib/src/task/model.dart b/pkg/analyzer/lib/src/task/model.dart
index 03e165f569d554e69f5999b6131fbacb6be13d88..5f43ebcada5438df5ddd4316a9c8bb634b947c72 100644
--- a/pkg/analyzer/lib/src/task/model.dart
+++ b/pkg/analyzer/lib/src/task/model.dart
@@ -19,9 +19,11 @@ class ContributionPointImpl<V> extends ResultDescriptorImpl<V> implements
   final List<ResultDescriptor<V>> contributors = <ResultDescriptor<V>>[];
 
   /**
-   * Initialize a newly created contribution point to have the given [name].
+   * Initialize a newly created contribution point to have the given [name] and
+   * [defaultValue].
    */
-  ContributionPointImpl(String name) : super(name);
+  ContributionPointImpl(String name, V defaultValue)

[Paul Berry, 2015/01/26 17:31:06]

Since the "value" associated with a contribution point is always determined by
concatenating together the results of all the tasks that contribute to it, does
it even make sense for a contribution point to have a default value?  Maybe we
should remove the "defaultValue" argument and just pass "null" to super.

[Brian Wilkerson, 2015/01/26 19:47:53]

I think it doesn't. I'll make the change.

+      : super(name, defaultValue);
 
   /**
    * Record that the given analysis [result] contibutes to this result.
@@ -41,10 +43,17 @@ class ResultDescriptorImpl<V> implements ResultDescriptor<V> {
   final String name;
 
   /**
-   * Initialize a newly created analysis result to have the given [name]. If a
-   * contribution point is specified, then this result will contribute to it.
+   * Return the default value for results described by this descriptor.
    */
-  ResultDescriptorImpl(this.name, {ContributionPoint contributesTo}) {
+  final V defaultValue;

[Paul Berry, 2015/01/26 17:31:06]

This design forces all results to share the same default value.  If the values
stored in results were immutable data structures, I would be ok with this, but
since we sometimes mutate them (e.g. when annotating an AST with pointers to the
element model), it feels a little dangerous.

Would it be better to instead store a function having type () -> V, so that we
can use it to create a default value on the fly when we need it?

[Brian Wilkerson, 2015/01/26 19:47:53]

Good point! It isn't a problem at the moment because all of the results whose
values can be mutable data structures currently have a default value of 'null',
but we should definitely revisit this, given that plugins will be contributing
result descriptors.

+
+  /**
+   * Initialize a newly created analysis result to have the given [name] and
+   * [defaultValue]. If a contribution point is specified, then this result will
+   * contribute to it.
+   */
+  ResultDescriptorImpl(this.name, this.defaultValue,
+      {ContributionPoint contributesTo}) {
     if (contributesTo is ContributionPointImpl) {
       contributesTo.recordContributor(this);
     }