Make source be a target

pkg/analyzer/lib/src/generated/source.dart

 // 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.
 
 // This code was auto-generated, is not intended to be edited, and is subject to
 // significant change. Please see the README file for more information.
 
 library engine.source;
 
 import "dart:math" as math;
 import 'dart:collection';
 
 import 'package:analyzer/file_system/file_system.dart';
 import 'package:analyzer/source/package_map_resolver.dart';
+import 'package:analyzer/task/model.dart';
 
 import 'engine.dart';
 import 'java_core.dart';
 import 'java_engine.dart';
 import 'sdk.dart' show DartSdk;
 
 /**
  * A function that is used to handle [ContentCache] entries.
  */
 typedef void ContentCacheVisitor(Source source, int stamp, String contents);
 
 /**
+ * An abstract base class for implementations of [Source].
+ */
+abstract class AbstractSource implements Source {

[Paul Berry, 2015/01/29 18:37:55]

I'm confused as to why this class is needed, since the preexisting Source class
is already abstract.  Couldn't we just do one of the following?

1. Add the getter "Source get source => this" to the Source object
2. Modify the new task manager infrastructure so that instead of calling
".source" on a source target, it just uses it directly?

Also, I'm confused about the @override annotation on the "source" getter--I
don't see any "source" getter in the Source class to be overridden.

[Brian Wilkerson, 2015/01/30 16:39:18]

I think my intention would have been a lot more clear in Java where there's a
distinction between classes and interfaces. (Perhaps that shows that I need to
adjust my thinking even more when designing for Dart). It would also have been
more clear if there were a distinction between public API and internal
implementation details. So, while it's clear in my head, I'm not surprised, in
retrospect, that it's confusing in this CL.

I consider Source to be a public interface; something that should be implemented
but not extended. AbstractSource is an internal abstract class; something
designed to be extended (in order to get a default implementation of 'source')
but not implemented.
Only if we also either (a) changed all subtypes of Source to extend Source
rather than implement it, or (b) duplicated the implementation of the getter in
every subtype. The latter would result in duplicated code. The former would blur
the line between interfaces and classes, which I'm not convinced is good from an
API design perspective.
It isn't the task manager that needs this getter. The task manager and analysis
driver should (I think) only need to deal with targets and shouldn't need to
know anything about sources. It's (a subset of) the individual tasks that need
this getter.

In my mind, all sources are targets, but not all targets are sources (an
individual method will be a target; while it has a source it isn't itself a
source). Therefore, we need some way, within a task, to convert a target into a
source.
The getter being overridden is in AnalysisTarget.

+  @override
+  Source get source => this;
+}
+
+/**
  * Instances of class `ContentCache` hold content used to override the default content of a
  * [Source].
  */
 class ContentCache {
   /**
    * A table mapping sources to the contents of those sources. This is used to override the default
    * contents of a source.
    */
   HashMap<Source, String> _contentMap = new HashMap<Source, String>();
 
@@ skipping 265 matching lines @@
 }
 
 class LocalSourcePredicate_TRUE implements LocalSourcePredicate {
   @override
   bool isLocal(Source source) => true;
 }
 
 /**
  * An implementation of an non-existing [Source].
  */
-class NonExistingSource implements Source {
+class NonExistingSource extends AbstractSource {
   final String _name;
 
   final UriKind uriKind;
 
   NonExistingSource(this._name, this.uriKind);
 
   @override
   TimestampedData<String> get contents {
     throw new UnsupportedOperationException("${_name}does not exist.");
   }
@@ skipping 52 matching lines @@
  * Because of these assumptions, most implementations will not maintain any state but will delegate
  * to an authoritative system of record in order to implement this API. For example, a source that
  * represents files on disk would typically query the file system to determine the state of the
  * file.
  *
  * If the instances that implement this API are the system of record, then they will typically be
  * unique. In that case, sources that are created that represent non-existent files must also be
  * retained so that if those files are created at a later date the long-lived sources representing
  * those files will know that they now exist.
  */
-abstract class Source {
+abstract class Source implements AnalysisTarget {
   /**
    * An empty list of sources.
    */
   static const List<Source> EMPTY_ARRAY = const <Source>[];
 
   /**
    * Get the contents and timestamp of this source.
    *
    * Clients should consider using the the method [AnalysisContext.getContents]
    * because contexts can have local overrides of the content of a source that the source is not
@@ skipping 584 matching lines @@
 
   /**
    * Return an absolute URI that represents the given source, or `null` if a valid URI cannot
    * be computed.
    *
    * @param source the source to get URI for
    * @return the absolute URI representing the given source
    */
   Uri restoreAbsolute(Source source) => null;
 }