Web components based app to view dart docs. Still has rough edges.

client/web/ast.dart

 library ast;

[Siggi Cherem (dart-lang), 2012/12/19 19:47:33]

+ copyright
+ library level doc

[Jacob, 2013/01/02 19:54:58]

Done.

 
 import 'package:web_ui/safe_html.dart';
 import 'markdown.dart' as md;
 
 /**
  * Top level data model for the app.
  * Mapping from String ids to [LibraryElement] objects describing all currently
  * loaded libraries. All code must be written to work properly if more libraries
  * are loaded incrementally.
  */
 Map<String, LibraryElement> libraries = <LibraryElement>{};
 
 List<String> LIBRARY_KINDS = ['variable', 'property', 'method', 'class', 'exception', 'typedef'];

[Siggi Cherem (dart-lang), 2012/12/19 19:47:33]

style nit: 80 col (here and the line below)

[Siggi Cherem (dart-lang), 2012/12/19 19:47:33]

nit: rename to LIBRARY_ITEMS? for a moment I thought this was going to be kinds
of libraries, rather than kinds of items in a library that you'll display.

or LIBRARY_ELEMENTS - to match the Element terminology you use below

[Jacob, 2013/01/02 19:54:58]

Changed to LIBRARY_CHILD_KINDS
these are the KIND names of library children that we should display in the UI.
Let me know if you want a different name.
On 2012/12/19 19:47:33, Siggi Cherem (dart-lang) wrote:

[Jacob, 2013/01/02 19:54:58]

renamed to LIBRARY_ITEMS

 List<String> CLASS_KINDS = ['constructor', 'variable', 'property', 'method', 'operator'];

[Siggi Cherem (dart-lang), 2012/12/19 19:47:33]

maybe rename as CLASS_ITEMS? or CLASS_ELEMENTS?

[Jacob, 2013/01/02 19:54:58]

renamed to CLASS_ITEMS.

 // TODO(jacobr): add package kinds?
 
 // TODO(jacobr): i18n
 /**
  * Pretty names for the various kinds displayed.
  */
 final KIND_TITLES = {'property': 'Properties',

[Siggi Cherem (dart-lang), 2012/12/19 19:47:33]

style nit: feel free to ignore, but I might prefer moving the hole list down one
line:

final Kind_TITLES = {
    'property': 'Properties',
    ...,
};

[Jacob, 2013/01/02 19:54:58]

Done.

                      'variable': 'Variables',
                      'method': 'Functions',
                      'constructor': 'Constructors',
                      'class': 'Classes',
                      'operator': 'Operators',
                      'typedef': 'Typedefs',
                      'exception': 'Exceptions'
 };
 
 /**
  * Block of elements to render summary documentation for that all share the

[Siggi Cherem (dart-lang), 2012/12/19 19:47:33]

for that all => for all that

[Jacob, 2013/01/02 19:54:58]

Done.

  * same kind.
  *
  * For example, all properties, all functions, or all constructors.
  */
 class ElementBlock {
   String kind;
   List<Element> elements;
 
   ElementBlock(this.kind, this.elements);
 
   String get kindTitle => KIND_TITLES[kind];
 }
 
 Reference jsonDeserializeReference(Map json) {

[Siggi Cherem (dart-lang), 2012/12/19 19:47:33]

nit: consider reordering the code here to put the classes up front, and the json
deserialization at the bottom of the file?

[Jacob, 2013/01/02 19:54:58]

Agreed. Moved to the bottom of the file.

   return json != null ? new Reference(json) : null;
 }
 
 /**
- * Deserializes JSON into [Element] or [Reference] objects.
+ * Deserializes JSON into an [Element] object.
  */
 Element jsonDeserialize(Map json, Element parent) {
   if (json == null) return null;
-  if (!json.containsKey('kind')) {
+  var kind = json['kind'];
+  if (kind == null) {
     throw "Unable to deserialize $json";
   }
 
-  switch (json['kind']) {
+  switch (kind) {
     case 'class':
       return new ClassElement(json, parent);
     case 'typedef':
       return new TypedefElement(json, parent);
     case 'typeparam':
       return new TypeParameterElement(json, parent);
     case 'library':
       return new LibraryElement(json, parent);
     case 'method':
       return new MethodElement(json, parent);
     case 'property':
       return new PropertyElement(json, parent);
     case 'constructor':
       return new ConstructorElement(json, parent);
     case 'variable':
       return new VariableElement(json, parent);
     case 'param':
       return new ParameterElement(json, parent);
     default:
       return new Element(json, parent);
   }
 }
 
-List<Element> jsonDeserializeArray(List json, Element parent) {
+List<Element> _jsonDeserializeArray(List json, Element parent) {
   var ret = <Element>[];
   if (json != null) {

[Siggi Cherem (dart-lang), 2012/12/19 19:47:33]

nit: consider switchting this to

if (json == null) return ret;

John and I are becoming big fans of this style :)

[Jacob, 2013/01/02 19:54:58]

Done.

     for (Map elementJson in json) {
       ret.add(jsonDeserialize(elementJson, parent));
     }
   }
   return ret;
 }
 
-List<Reference> jsonDeserializeReferenceArray(List json) {
+List<Reference> _jsonDeserializeReferenceArray(List json) {
   var ret = <Reference>[];
   if (json != null) {

[Siggi Cherem (dart-lang), 2012/12/19 19:47:33]

same here

[Jacob, 2013/01/02 19:54:58]

Done.

     for (Map referenceJson in json) {
       ret.add(new Reference(referenceJson));
     }
   }
   return ret;
 }
 
 /**
  * Reference to an [Element].
  */
 class Reference {
   final String refId;
   final String name;
+  final List<Reference> arguments;
+
   Reference(Map json) :
     name = json['name'],
-    refId = json['refId'];
+    refId = json['refId'],
+    arguments = _jsonDeserializeReferenceArray(json['arguments']);
+
+  /**
+   * Short description appropriate for displaying in a tree control or other
+   * situtation where a short description is required.
+   */
+  String get shortDescription {
+    if (arguments.isEmpty) {
+      return name;
+    } else {
+      var params = Strings.join(
+          arguments.map((param) => param.shortDescription), ', ');
+      return '$name<$params>';
+    }
+  }
 }
 
 /**
  * Lookup a library based on the [libraryId].
- *
- * If the library cannot be found, a stub dummy [Library] will be returned.
  */
 LibraryElement lookupLibrary(String libraryId) {
-  var library = libraries[libraryId];
-  if (library == null) {
-    library = new LibraryElement.stub(libraryId, null);
-  }
-  return library;
+  return libraries[libraryId];
 }
 
 /**
  * Resolve the [Element] matching the [referenceId].
  *
  * If the Element cannot be found, a stub dummy [Element] will be returned.
  */
 Element lookupReferenceId(String referenceId) {
   var parts = referenceId.split(new RegExp('/'));
   Element current = lookupLibrary(parts.first);
   var result = <Element>[current];

[Siggi Cherem (dart-lang), 2012/12/19 19:47:33]

'result' seems unused?

[Jacob, 2013/01/02 19:54:58]

yep that was obsolete. fixed.

-  for (var i = 1; i < parts.length; i++) {
+  for (var i = 1; i < parts.length && current != null; i++) {
     var id = parts[i];
     var next = null;
     for (var child in current.children) {
       if (child.id == id) {
         next = child;

[Siggi Cherem (dart-lang), 2012/12/19 19:47:33]

you could directly do:
 current = child;

and remove lines 151 and 158.

[Jacob, 2013/01/02 19:54:58]

The trouble is that isn't quite right.  You need to know when there is no
matching element.

On 2012/12/19 19:47:33, Siggi Cherem (dart-lang) wrote:

         break;
       }
     }
-    if (next == null) {
-      next = new Element.stub(id, current);
-    }
     current = next;
   }
   return current;
 }
 
 /**
  * Invoke [callback] on every [Element] in the ast.
  */
 _traverseWorld(void callback(Element)) {
   for (var library in libraries.values) {
     library.traverse(callback);
   }
 }
 
-// TODO(jacobr): remove this method when templates handle safe HTML containing
+// TODO(jacobr): remove this method when templates handle [SafeHTML] containing
+// multiple top level nodes correct.
 SafeHtml _markdownToSafeHtml(String text) {
   // We currently have to insert an extra span for now because of
   // https://github.com/dart-lang/dart-web-components/issues/212

[Siggi Cherem (dart-lang), 2012/12/19 19:47:33]

dart-web-components ==> web-ui

[Jacob, 2013/01/02 19:54:58]

Done.

   return new SafeHtml.unsafe(text != null && !text.isEmpty ?
         '<span>${md.markdownToHtml(text)}</span>' : '<span><span>');
 }
 
 /**
  * Specifies the order elements should appear in the UI.
  */
 int elementUiOrder(Element a, Element b) {

[Siggi Cherem (dart-lang), 2012/12/19 19:47:33]

should Element implement Comparable and this function become 'compareTo' in
Element?

[Jacob, 2013/01/02 19:54:58]

Done.

   if (a.isPrivate != b.isPrivate) {
     return a.isPrivate == true ? 1 : -1;
   }
   return a.name.compareTo(b.name);
 }
 
 /**
  * Base class for all elements in the AST.
  */
 class Element {
   final Element parent;
   /** Human readable type name for the node. */

[Siggi Cherem (dart-lang), 2012/12/19 19:47:33]

nit: add empty line above comment, here and in every field below

   final String rawKind;
   /** Human readable name for the element. */
   final String name;
   /** Id for the node that is unique within its parent's children. */
   final String id;
   /** Raw text of the comment associated with the Element if any. */
   final String comment;
   /** Whether the node is private. */
   final bool isPrivate;
 
   final String _uri;

[Siggi Cherem (dart-lang), 2012/12/19 19:47:33]

nit: move these private members down with the other private ones? One idea is to
put them next to the public properties that wrap them. For instance, put '_uri'
next to 'uri', and '_line' next to 'line'.
(same with _commentHtml, _typeParameters, etc)

[Jacob, 2013/01/02 19:54:58]

Done.

   final String _line;
 
   /** Children of the node. */
   List<Element> children;
 
   /** Whether the [Element] is currently being loaded. */
   final bool loading;
 
   String _refId;
 
   Map _members;
   SafeHtml _commentHtml;
   List<Element> _references;
 
+  List<Element> _typeParameters;
+
   Element(Map json, this.parent) :
     name = json['name'],
     rawKind = json['kind'],
     id = json['id'],
     comment = json['comment'],
     isPrivate = json['isPrivate'],
     _uri = json['uri'],
     _line = json['line'],
     loading = false {
-    children = jsonDeserializeArray(json['children'], this);
+    children = _jsonDeserializeArray(json['children'], this);
   }
 
   /**
    * Returns a kind name that make sense for the UI rather than the AST
    * kinds.  For example, setters are considered properties instead of
-   * methods.
+   * methods in the UI but not the AST.
    */
   String get uiKind => kind;
 
+  /**
+   * Longer possibly multiple word description of the [kind.

[Siggi Cherem (dart-lang), 2012/12/19 19:47:33]

[kind. => [kind].

[Jacob, 2013/01/02 19:54:58]

Done.

+   */
+  String get kindDescription => uiKind;
+
   /** Invoke [callback] on this [Element] and all descendants. */
   void traverse(void callback(Element)) {
     callback(this);
     for (var child in children) {
       callback(child);
     }
   }
 
   /**
-   * Uri containing the definition of the element.
+   * Uri containing the source code for the definition of the element.
    */
   String get uri {
     Element current = this;
     while (current != null) {
       if (current._uri != null) return current._uri;
       current = current.parent;
     }
     return null;
   }

[Siggi Cherem (dart-lang), 2012/12/19 19:47:33]

maybe use a recursive pattern here instead:

String get uri =>
  _uri != null ? _uri : (parent != null ? parent.uri : null);

[Jacob, 2013/01/02 19:54:58]

Done.

 
   /**
-   * Line in the original source file that starts the definition of the element.
+   * Line in the original source file that begins the definition of the element.
    */
   String get line {
     Element current = this;
     while (current != null) {
       if (current._line != null) return current._line;
       current = current.parent;
     }
     return null;
   }

[Siggi Cherem (dart-lang), 2012/12/19 19:47:33]

similary - use recursive pattern for line too.

[Jacob, 2013/01/02 19:54:58]

Done.

 
-  Element.stub(this.id, this.parent) :
-    name = '???', // TODO(jacobr): remove/add
-    _uri = null,
-    _line = null,
-    comment = null,
-    rawKind = null,
-    children = <Element>[],
-    isPrivate = null,
-    loading = true;
-
   /**
    *  Globally unique identifier for this element.
    */
   String get refId {
     if (_refId == null) {
        if (parent == null) {
          _refId = id;
        } else {
          _refId = '${parent.refId}/$id';
        }

[Siggi Cherem (dart-lang), 2012/12/19 19:47:33]

style nit: feel free to ignore, maybe compact the code a bit the ternary form:
_refId = (parent == null) ? id : '${parent.refId}/$id';

[Jacob, 2013/01/02 19:54:58]

Done.

     }
     return _refId;
   }
 
   /**
    * Whether this [Element] references the specified [referenceId].
    */
   bool hasReference(String referenceId) {
     for (var child in children) {

[Siggi Cherem (dart-lang), 2012/12/19 19:47:33]

use List.some?

bool hasReference(String referenceId) =>
  children.some((c) => c.hasReference(referenceId));

       if (child.hasReference(referenceId)) {
         return true;
       }
     }
     return false;
   }
 
   /** Returns all [Element]s that reference this [Element]. */
   List<Element> get references {
     if (_references == null) {
       _references = <Element>[];
-      // TODO(jacobr): change to filterWorld and tweak meaning.
       _traverseWorld((element) {
         if (element.hasReference(refId)) {
           _references.add(element);
         }
       });
     }
     return _references;
   }
 
   // TODO(jacobr): write without recursion.

[Siggi Cherem (dart-lang), 2012/12/19 19:47:33]

why :)? 

[Jacob, 2013/01/02 19:54:58]

performance :)
for this case the paths are pretty deep.  For the other couple of cases, you
early terminate most of the time so recursion doesn't hit performance much.
Anyway removed the premature optimization TODO.
removed comment...

   /**
    * Path from this [Element] to the root of the tree starting at the root.

[Siggi Cherem (dart-lang), 2012/12/19 19:47:33]

maybe rephrase as follows? 
Path from the root of the tree to this [Element]

[Jacob, 2013/01/02 19:54:58]

Done.

    */
   List<Element> get path {
     if (parent == null) {
       return <Element>[this];
     } else {
       return parent.path..add(this);
     }
   }
 
+  List<Element> get typeParameters {
+    if (_typeParameters == null) {
+      _typeParameters = _filterByKind('typeparam');
+    }
+    return _typeParameters;
+  }
+
   /**
    * [SafeHtml] for the comment associated with this [Element] generated from
    * the markdow comment associated with the element.
    */
   SafeHtml get commentHtml {
     if (_commentHtml == null) {
       _commentHtml = _markdownToSafeHtml(comment);
     }
     return _commentHtml;
   }
 
   /**
    * Short description appropriate for displaying in a tree control or other
    * situtation where a short description is required.
    */
-  String get shortDescription => name;
+  String get shortDescription {
+    if (typeParameters.isEmpty) {
+      return name;
+    } else {
+      var params = Strings.join(
+          typeParameters.map((param) => param.shortDescription),
+          ', ');
+      return '$name<$params>';
+    }
+  }
 
   /** Possibly normalized representation of the node kind. */

[Siggi Cherem (dart-lang), 2012/12/19 19:47:33]

strange to see 'possibly' here, maybe explain we can expect it to be normalized?

[Jacob, 2013/01/02 19:54:58]

rewrote comment.

   String get kind => rawKind;
 
   /**
    * Generate blocks of Elements for each kind in the list of [desiredKinds].
    *
    * This is helpful when rendering UI that segments members into blocks.
    * Uses the kind types that make sense for the UI rather than the AST
    * kinds.  For example, setters are considered properties instead of methods.
    */
   List<ElementBlock> _createElementBlocks(List<String> desiredKinds) {
@@ skipping 12 matching lines @@
       }
     }
     return blocks;
   }
 
   List<Element> _filterByKind(String kind) =>
       children.filter((child) => child.kind == kind);
 
   Map<String, Element> _mapForKind(String kind) {
     Map ret = {};
     if (children != null) {

[Siggi Cherem (dart-lang), 2012/12/19 19:47:33]

nit: if(children == null) return ret;

[Jacob, 2013/01/02 19:54:58]

Done.

       for (var child in children) {
         if (child.kind == kind) {
           ret[child.id] = child;
         }
       }
     }
     return ret;
   }
 
   Map<String, Element> _mapForKinds(Map<String, Element> kinds) {

[Siggi Cherem (dart-lang), 2012/12/19 19:47:33]

change argument type to Set<String> ?

[Jacob, 2013/01/02 19:54:58]

This method isn't really needed. Removed.

     Map ret = {};
     if (children != null) {
       for (var child in children) {
         if (kinds.containsKey(child.kind)) {
           ret[child.id] = child;
         }
       }
     }
     return ret;
   }
@@ skipping 11 matching lines @@
  * [Element] describing a Dart library.
  *
  * Adds convenience helpers for quickly accessing data about libraries.
  */
 class LibraryElement extends Element {
   Map<String, ClassElement> _classes;
   List<ClassElement> _sortedClasses;
   List<ElementBlock> _childBlocks;
 
   LibraryElement(json, Element parent) : super(json, parent);
-  LibraryElement.stub(String id, Element parent) : super.stub(id, parent);
 
   /** Returns all classes defined by the library. */
   Map<String, ClassElement> get classes {
     if (_classes == null) {
       _classes = _mapForKind('class');
     }
     return _classes;
   }
 
   /**
    * Returns all classes defined by the library sorted name and whether they
    * are private.
    */
   List<ClassElement> get sortedClasses {
     if (_sortedClasses == null) {
       _sortedClasses = []..addAll(classes.values)..sort(elementUiOrder);
     }
     return _sortedClasses;
   }
 
   /**
    * Returns all blocks of elements that should be rendered by UI summarizing
    * the Library.
    */
   List<ElementBlock> get childBlocks {
-    if (_childBlocks == null) _childBlocks = _createElementBlocks(LIBRARY_KINDS);
+    if (_childBlocks == null) {
+      _childBlocks = _createElementBlocks(LIBRARY_KINDS);
+    }
     return _childBlocks;
   }
 }
 
 /**
  * [Element] describing a Dart class.
  */
 class ClassElement extends Element {
   /** Members of the class grouped into logical blocks. */
   List<ElementBlock> _childBlocks;
   /** Interfaces the class implements. */

[Siggi Cherem (dart-lang), 2012/12/19 19:47:33]

nit: + empty line above comment here and below

[Jacob, 2013/01/02 19:54:58]

Done.

   final List<Reference> interfaces;
   /** Superclass of this class. */
   final Reference superclass;
 
   List<ClassElement> _superclasses;
   List<ClassElement> _subclasses;
+  final bool isAbstract;
 
   ClassElement(Map json, Element parent)
     : super(json, parent),
-      interfaces = jsonDeserializeReferenceArray(json['interfaces']),
-      superclass = jsonDeserializeReference(json['superclass']);
-
-  ClassElement.stub(String id, Element parent)
-    : super.stub(id, parent),
-      interfaces = [],
-      superclass = null;
+      interfaces = _jsonDeserializeReferenceArray(json['interfaces']),
+      superclass = jsonDeserializeReference(json['superclass']),
+      isAbstract = json['isAbstract'];
 
   /** Returns all superclasses of this class. */
   List<ClassElement> get superclasses {
     if (_superclasses == null) {
       _superclasses = <ClassElement>[];
       addSuperclasses(clazz) {

[Siggi Cherem (dart-lang), 2012/12/19 19:47:33]

nit: consider adding here the type parameter and removing it from line 498?

[Jacob, 2013/01/02 19:54:58]

i don't follow.

[Siggi Cherem (dart-lang), 2013/01/02 21:40:55]

In line 498 (now line 402) you seem to have a downcast, but it doesn't seem
necessary. You can simply do:

var superclassElement = loopupReferenceId(...);

I was initially thinking that if you wanted additional type information, we
could put the type annotation in the argument of this function, like this:

addSuperclasses(ClassElement clazz) {
  ...
}

[Jacob, 2013/01/03 00:09:33]

just removed the ClassElement and changed it to a var. I can't remember why I
wanted a type there.

         if (clazz.superclass != null) {
           ClassElement superclassElement =
               lookupReferenceId(clazz.superclass.refId);

[Siggi Cherem (dart-lang), 2012/12/19 19:47:33]

one thing I was wondering is whether Reference could hold a pointer to the
resolved Element once it's resolved.

The idea is that when deserializing you could use a map to associate
referenceids to Reference objects, so there is a single Reference object with a
particular refid.

Once you have a Reference object, you only would need to do a lookupReferenceId
once on it, and you would be done.

[Jacob, 2013/01/02 19:54:58]

I don't think the times when references are resolved are performance
bottlenecks.  
In addition, keep in mind that it is very important for this to not occur at
deserialization time as all the libraries where the references are from may not
be available until those libs are later resolved.  The next big data model CL
change for this code is to only load library definitions as required.
On 2012/12/19 19:47:33, Siggi Cherem (dart-lang) wrote:

[Siggi Cherem (dart-lang), 2013/01/02 21:40:55]

We can wait and see later how this would work after the next model changes. My
suggestion was not so much about performance though, but about abstraction. It
might be easier to chat in person about it, but I think it would be a nice model
to work with if we reference objects were unique. You would still allow
reference objects with unresolved elements (when a library is not loaded, for
example), but then the reference objects will be easier to work with than using
only the string reference ids.

[Jacob, 2013/01/03 00:09:33]

Lets chat about this offline.
On 2013/01/02 21:40:55, Siggi Cherem (dart-lang) wrote:

           addSuperclasses(superclassElement);
           _superclasses.add(superclassElement);
         }
       }
       addSuperclasses(this);
     }
     return _superclasses;
   }
 
+  String get kindDescription =>
+      isAbstract  == true ? 'abstract $uiKind' : uiKind;
+
   /**
    * Returns classes that directly extend or implement this class.
    */
   List<ClassElement> get subclasses {
     if (_subclasses == null) {
       _subclasses = <ClassElement>[];
       for (var library in libraries.values) {
         for (ClassElement candidateClass in library.sortedClasses) {
           if (candidateClass.implementsOrExtends(refId)) {
             _subclasses.add(candidateClass);
@@ skipping 18 matching lines @@
   /**
    * Returns blocks of elements clustered by kind ordered in the desired
    * order for describing a class definition.
    */
   List<ElementBlock> get childBlocks {
     if (_childBlocks == null) _childBlocks = _createElementBlocks(CLASS_KINDS);
     return _childBlocks;
   }
 }
 
+/**
+ * Element describing a typedef.
+ */
 class TypedefElement extends Element {
   final Reference returnType;
-  List<ParameterElement> _parameters;
+  List<Element> _parameters;
 
   TypedefElement(Map json, Element parent) : super(json, parent),
       returnType = jsonDeserializeReference(json['returnType']);
 
   /**
    * Returns a list of the parameters of the typedef.
    */
-  List<ParameterElement> get parameters {
+  List<Element> get parameters {
     if (_parameters == null) {
       _parameters = _filterByKind('param');
     }
     return _parameters;
   }
 }
 
 /**
  * [Element] describing a method which may be a regular method, a setter, or an
  * operator.
  */
 abstract class MethodElementBase extends Element {

[Siggi Cherem (dart-lang), 2012/12/19 19:47:33]

consider renaming this class? some ideas:
* ClassMemberElement
* CallableElement
* MethodLikeElement

[Jacob, 2013/01/03 00:09:33]

Went with MethodLikeElement.
ClassMemberElement is inaccurate due to top level methods.

On 2012/12/19 19:47:33, Siggi Cherem (dart-lang) wrote:

 
   final bool isOperator;
   final bool isStatic;
   final bool isSetter;
 
   MethodElementBase(Map json, Element parent)
     : super(json, parent),
       isOperator = json['isOperator'] == true,
       isStatic = json['isStatic'] == true,
       isSetter = json['isSetter'] == true;
 
   bool hasReference(String referenceId) {
     if (super.hasReference(referenceId)) return true;
     return returnType != null && returnType.refId == referenceId;
   }
 
   String get uiKind => isSetter ? 'property' : kind;
 
   /**
    * Returns a plain text short description of the method suitable for rendering
    * in a tree control or other case where a short method description is
    * required.
    */
   String get shortDescription {
     if (isSetter == true) {

[Siggi Cherem (dart-lang), 2012/12/19 19:47:33]

this should not be null by this point, you could simply do if (isSetter)

Also - why not move this condition under PropertyElement (since it's not
possible to have this in operators and normal methods?)

[Jacob, 2013/01/02 19:54:58]

sadly in the AST, setters aren't property elements. only getters are property
elements. fixed the == true ugliness.
On 2012/12/19 19:47:33, Siggi Cherem (dart-lang) wrote:

       var sb = new StringBuffer('${name.substring(0, name.length-1)}');

[Siggi Cherem (dart-lang), 2012/12/19 19:47:33]

nit: spaces around '-' (name.length - 1)

[Jacob, 2013/01/02 19:54:58]

Done.

       if (!parameters.isEmpty && parameters.first != null
           && parameters.first.type != null) {
         sb..add(' ')..add(parameters.first.type.name);

[Siggi Cherem (dart-lang), 2012/12/19 19:47:33]

what you have seems fine, but it makes me ask some questions about cascades and
style:
- since sb already return themselves, the cascades are not needed. Do we prefer
them in this kind of scenario?
- since sb is a variable, do we prefer cascades or doing:
  sb.add(' ');
  sb.add(...);
  ?

[Jacob, 2013/01/02 19:54:58]

my take is StringBuffer should just return void from these methods in the future
if we believe cascades are the way to do chaining.

On 2012/12/19 19:47:33, Siggi Cherem (dart-lang) wrote:

       }
       return sb.toString();
     }
     return '$name(${Strings.join(parameters.map(
         (arg) => arg.type != null ? arg.type.name : ''), ', ')})';
   }
 
   Reference get returnType;
-  List<ParameterElement> _parameters;
+  List<Element> _parameters;
 
   /**
    * Returns a list of the parameters of the Method.
    */
-  List<ParameterElement> get parameters {
+  List<Element> get parameters {
     if (_parameters == null) {
       _parameters = _filterByKind('param');
     }
     return _parameters;
   }
 
   // For UI purposes we want to treat operators as their own kind.
   String get kind => isOperator ? 'operator' : rawKind;
 }
 
 /**
  * Element describing a parameter.
  */
 class ParameterElement extends Element {
   /** Type of the parameter. */
   final Reference type;
   /** Whether the parameter is optional. */
   final bool isOptional;
 
   ParameterElement(Map json, Element parent) :
       super(json, parent),
       type = jsonDeserializeReference(json['ref']),
       isOptional = json['isOptional'];
 
   bool hasReference(String referenceId) {
     if (super.hasReference(referenceId)) return true;
     return type != null && type.refId == referenceId;

[Siggi Cherem (dart-lang), 2012/12/19 19:47:33]

consider swapping the order of the conditions?
seems like the second kind of check is probably more efficient than
Element.hasReference, so maybe this will be faster:

bool hasReference(...) {
  if (type != null && ...) return true;
  return super.hasReference(...);
}

[Jacob, 2013/01/02 19:54:58]

super.hasReference is cheap. 
In any case, both conditions will typically have to be checked as 99.9% of the
time, hasReference will return false.

On 2012/12/19 19:47:33, Siggi Cherem (dart-lang) wrote:

   }
 }
 
 /**
  * Element describing a generic type parameter.
  */
 class TypeParameterElement extends Element {
   /** Upper bound for the parameter. */
   Reference upperBound;
 
   TypeParameterElement(Map json, Element parent) :
     super(json, parent),
     upperBound = jsonDeserializeReference(json['upperBound']);
 
+  String get shortDescription {
+    if (upperBound == null) {
+      return name;
+    } else {
+      return '$name extends ${upperBound.shortDescription}';

[Siggi Cherem (dart-lang), 2012/12/19 19:47:33]

mmm. I wonder if we want to simply make shortDecription also toString, so here
you'd do:

String toString() =>  upperBound == null ? name : '$name extends $upperBound';

[Jacob, 2013/01/02 19:54:58]

I like to be specific about this as there are many reasonable toString
definitions for this class.

+    }
+  }
+
   bool hasReference(String referenceId) {
     if (super.hasReference(referenceId)) return true;
     return upperBound != null && upperBound.refId == referenceId;
   }
 }
 
+/**
+ * Element describing a method.
+ */
 class MethodElement extends MethodElementBase {
 
   final Reference returnType;
 
   MethodElement(Map json, Element parent) : super(json, parent),
       returnType = jsonDeserializeReference(json['returnType']);
 }
 
+/**
+ * Element describing a property getter.
+ */
 class PropertyElement extends MethodElementBase {
   final Reference returnType;
 
   String get shortDescription => name;
 
   PropertyElement(Map json, Element parent) : super(json, parent),
     returnType = jsonDeserializeReference(json['ref']);
 }
 
+/**
+ * Element describing a variable.
+ */
 class VariableElement extends MethodElementBase {
   final Reference returnType;
   /** Whether this variable is final. */
   final bool isFinal;
 
   String get shortDescription => name;
 
   VariableElement(Map json, Element parent) : super(json, parent),
     returnType = jsonDeserializeReference(json['ref']),
     isFinal = json['isFinal'];
 }
 
+/**
+ * Element describing a constructor.
+ */
 class ConstructorElement extends MethodElementBase {
   ConstructorElement(json, Element parent) : super(json, parent);
 
   Reference get returnType => null;
 }