Members and comments inherited in dartdoc.

pkg/dartdoc/lib/dartdoc.dart

Index: pkg/dartdoc/lib/dartdoc.dart
diff --git a/pkg/dartdoc/lib/dartdoc.dart b/pkg/dartdoc/lib/dartdoc.dart
index 2eff4bebb5ff43ff7baed52570a6b3bacbc1ed3d..0a5f93d41522278168aef68c099b13327bbd9972 100644
--- a/pkg/dartdoc/lib/dartdoc.dart
+++ b/pkg/dartdoc/lib/dartdoc.dart
@@ -575,7 +575,7 @@ class Dartdoc {
       if (type.isPrivate) continue;
 
       var typeInfo = {};
-      typeInfo[NAME] = type.simpleName;
+      typeInfo[NAME] = type.displayName;
       if (type.isClass) {
         typeInfo[KIND] = CLASS;
       } else if (type.isInterface) {
@@ -592,7 +592,7 @@ class Dartdoc {
       if (!type.declaration.typeVariables.isEmpty()) {
         final typeVariables = [];
         for (final typeVariable in type.declaration.typeVariables) {
-          typeVariables.add(typeVariable.simpleName);
+          typeVariables.add(typeVariable.displayName);
         }
         typeInfo[ARGS] = Strings.join(typeVariables, ', ');
       }
@@ -612,32 +612,20 @@ class Dartdoc {
 
       var memberInfo = {};
       if (member.isField) {
-        memberInfo[NAME] = member.simpleName;
         memberInfo[KIND] = FIELD;
       } else {
         MethodMirror method = member;
         if (method.isConstructor) {
-          if (method.constructorName != '') {
-            memberInfo[NAME] = '${method.simpleName}.${method.constructorName}';
-            memberInfo[KIND] = CONSTRUCTOR;
-          } else {
-            memberInfo[NAME] = member.simpleName;
-            memberInfo[KIND] = CONSTRUCTOR;
-          }
-        } else if (method.isOperator) {
-          memberInfo[NAME] = '${method.simpleName} ${method.operatorName}';
-          memberInfo[KIND] = METHOD;
+          memberInfo[KIND] = CONSTRUCTOR;
         } else if (method.isSetter) {
-          memberInfo[NAME] = member.simpleName;
           memberInfo[KIND] = SETTER;
         } else if (method.isGetter) {
-          memberInfo[NAME] = member.simpleName;
           memberInfo[KIND] = GETTER;
         } else {
-          memberInfo[NAME] = member.simpleName;
           memberInfo[KIND] = METHOD;
         }
       }
+      memberInfo[NAME] = member.displayName;
       var anchor = memberAnchor(member);
       if (anchor != memberInfo[NAME]) {
         memberInfo[LINK_NAME] = anchor;
@@ -732,7 +720,7 @@ class Dartdoc {
     // Look for a comment for the entire library.
     final comment = getLibraryComment(library);
     if (comment != null) {
-      writeln('<div class="doc">$comment</div>');
+      writeln('<div class="doc">${comment.html}</div>');
     }
 
     // Document the top-level members.
@@ -778,6 +766,7 @@ class Dartdoc {
   void docTypes(List types, String header) {
     if (types.length == 0) return;
 
+    writeln('<div>');
     writeln('<h3>$header</h3>');
 
     for (final type in types) {
@@ -790,6 +779,7 @@ class Dartdoc {
           </div>
           ''');
     }
+    writeln('</div>');
   }
 
   void docType(InterfaceMirror type) {
@@ -823,10 +813,12 @@ class Dartdoc {
           $kind
         </h2>
         ''');
+    writeln('<span class="show-inherited">Hide inherited</span>');

[Lasse Reichstein Nielsen, 2012/10/04 07:41:19]

Use a button! This is clearly a clickable thing with a non-navigating behavior,
so it should be a button. 
Also, give it an id instead of a class. It should be unique in the document.

[Johnni Winther, 2012/10/04 12:46:21]

Done.

 
-    docCode(type.location, getTypeComment(type));
+    docCode(type, type.location, getTypeComment(type));
     docInheritance(type);
     docTypedef(type);
+
     docConstructors(type);
     docMembers(type);
 
@@ -921,7 +913,7 @@ class Dartdoc {
       }
 
       listTypes(subtypes, 'Subclasses');
-      listTypes(type.interfaces.getValues(), 'Implements');
+      listTypes(type.interfaces, 'Implements');
     } else {
       // Show the default class.
       if (type.defaultType != null) {
@@ -929,7 +921,7 @@ class Dartdoc {
       }
 
       // List extended interfaces.
-      listTypes(type.interfaces.getValues(), 'Extends');
+      listTypes(type.interfaces, 'Extends');
 
       // List subinterfaces and implementing classes.
       final subinterfaces = [];
@@ -968,7 +960,7 @@ class Dartdoc {
               title="Permalink to ${type.simpleName}">#</a>''');
     writeln('</h4>');
 
-    docCode(type.location, null, showCode: true);
+    docCode(type, type.location, null, showCode: true);
 
     writeln('</div>');
   }
@@ -983,6 +975,7 @@ class Dartdoc {
     }
 
     if (constructors.length > 0) {
+      writeln('<div>');
       writeln('<h3>Constructors</h3>');
       constructors.sort((x, y) => x.simpleName.toUpperCase().compareTo(
                                   y.simpleName.toUpperCase()));
@@ -990,6 +983,7 @@ class Dartdoc {
       for (final constructor in constructors) {
         docMethod(type, constructor);
       }
+      writeln('</div>');
     }
   }
 
@@ -997,50 +991,77 @@ class Dartdoc {
     // Collect the different kinds of members.
     final staticMethods = [];
     final staticFields = [];
+    final memberMap = new Map<String,MemberMirror>();
     final instanceMethods = [];
     final instanceFields = [];
 
-    for (MemberMirror member in orderByName(host.declaredMembers.getValues())) {
-      if (member.isPrivate) continue;
-
-      final methods = member.isStatic ? staticMethods : instanceMethods;
-      final fields = member.isStatic ? staticFields : instanceFields;
+    host.declaredMembers.forEach((_, MemberMirror member) {

[Lasse Reichstein Nielsen, 2012/10/04 07:41:19]

Add a host.forEachDeclaredMember method instead of acting directly on the map.
When you even ignore the key, it's obvious that the Map is an internal storage
representation that needn't be externally visible.

[Johnni Winther, 2012/10/04 12:46:21]

[declaredMembers] is also used for lookup where the key is used. This is
actually the specific case because we want to go through all members.

+      if (member.isPrivate) return;
+      if (member.isStatic) {
+        if (member.isMethod) {
+          staticMethods.add(member);
+        } else  if (member.isField) {
+          staticFields.add(member);
+        }
+      }
+    });
 
-      if (member.isMethod) {
-        methods.add(member);
-      } else if (member.isField) {
-        fields.add(member);
+    if (host is InterfaceMirror) {
+      var iterable = new HierarchyIterable(host, includeType: true);
+      for (InterfaceMirror type in iterable) {
+        type.declaredMembers.forEach((_, MemberMirror member) {
+          if (member.isPrivate) return;
+          if (!member.isStatic) {
+            memberMap.putIfAbsent(member.simpleName, () => member);
+          }
+        });
       }
     }
 
-    if (staticMethods.length > 0) {
-      final title = host is LibraryMirror ? 'Functions' : 'Static Methods';
-      writeln('<h3>$title</h3>');
-      for (final method in orderByName(staticMethods)) {
-        docMethod(host, method);
+    memberMap.forEach((_, MemberMirror member) {
+      if (member.isMethod) {
+        instanceMethods.add(member);
+      } else  if (member.isField) {
+        instanceFields.add(member);
       }
-    }
+    });
 
     if (staticFields.length > 0) {
       final title = host is LibraryMirror ? 'Variables' : 'Static Fields';
+      writeln('<div>');
       writeln('<h3>$title</h3>');
       for (final field in orderByName(staticFields)) {
         docField(host, field);
       }
+      writeln('</div>');
     }
 
-    if (instanceMethods.length > 0) {
-      writeln('<h3>Methods</h3>');
-      for (final method in orderByName(instanceMethods)) {
+    if (staticMethods.length > 0) {
+      final title = host is LibraryMirror ? 'Functions' : 'Static Methods';
+      writeln('<div>');

[Lasse Reichstein Nielsen, 2012/10/04 07:41:19]

Why wrap it in a div if it doesn't even have a class?
Seems like pure overhead.

[Johnni Winther, 2012/10/04 12:46:21]

Not quite sure. It was requested in dartbug.com/1156 and has something to do
with user search.

+      writeln('<h3>$title</h3>');
+      for (final method in orderByName(staticMethods)) {
         docMethod(host, method);
       }
+      writeln('</div>');
     }
 
     if (instanceFields.length > 0) {
+      writeln('<div>');
       writeln('<h3>Fields</h3>');
       for (final field in orderByName(instanceFields)) {
         docField(host, field);
       }
+      writeln('</div>');
+    }
+
+    if (instanceMethods.length > 0) {
+      writeln('<div>');
+      writeln('<h3>Methods</h3>');
+      for (final method in orderByName(instanceMethods)) {
+        docMethod(host, method);
+      }
+      writeln('</div>');
     }
   }
 
@@ -1053,8 +1074,10 @@ class Dartdoc {
     _currentMember = method;
 
     bool showCode = includeSource && !method.isAbstract;
+    bool inherited = host != method.surroundingDeclaration;
 
-    writeln('<div class="method"><h4 id="${memberAnchor(method)}">');
+    writeln('<div class="method${inherited ? ' inherited': ''}">'
+            '<h4 id="${memberAnchor(method)}">');
 
     if (showCode) {
       writeln('<span class="show-code">Code</span>');
@@ -1070,11 +1093,11 @@ class Dartdoc {
       write('abstract ');
     }
 
-    if (method.constructorName == null) {
+    if (!method.isConstructor) {
       annotateType(host, method.returnType);
     }
 
-    var name = method.simpleName;
+    var name = method.displayName;
     // Translate specially-named methods: getters, setters, operators.
     if (method.isGetter) {
       // Getter.
@@ -1082,18 +1105,10 @@ class Dartdoc {
     } else if (method.isSetter) {
       // Setter.
       name = 'set $name';
-    } else if (method.isOperator) {
-      name = 'operator ${method.operatorName}';
     }
 
     write('<strong>$name</strong>');
 
-    // Named constructors.
-    if (method.constructorName != null && method.constructorName != '') {
-      write('.');
-      write(method.constructorName);
-    }
-
     docParamList(host, method.parameters);
 
     var prefix = host is LibraryMirror ? '' : '${typeName(host)}.';
@@ -1101,7 +1116,13 @@ class Dartdoc {
               title="Permalink to $prefix$name">#</a>''');
     writeln('</h4>');
 
-    docCode(method.location, getMethodComment(method), showCode: showCode);
+    if (inherited) {
+      write('<div class="inherited-from">inherited from ');
+      annotateType(host, method.surroundingDeclaration);
+      write('</div>');
+    }
+
+    docCode(host, method.location, getMemberComment(method), showCode: showCode);
 
     writeln('</div>');
   }
@@ -1111,7 +1132,10 @@ class Dartdoc {
     _totalMembers++;
     _currentMember = field;
 
-    writeln('<div class="field"><h4 id="${memberAnchor(field)}">');
+    bool inherited = host != field.surroundingDeclaration;
+
+    writeln('<div class="field${inherited ? ' inherited' : ''}">'
+            '<h4 id="${memberAnchor(field)}">');
 
     if (includeSource) {
       writeln('<span class="show-code">Code</span>');
@@ -1133,7 +1157,14 @@ class Dartdoc {
         </h4>
         ''');
 
-    docCode(field.location, getFieldComment(field), showCode: true);
+    if (inherited) {
+      write('<div class="inherited-from">inherited from ');
+      annotateType(host, field.surroundingDeclaration);
+      write('</div>');
+    }
+
+    docCode(host, field.location, getMemberComment(field), showCode: true);
+
     writeln('</div>');
   }
 
@@ -1169,10 +1200,20 @@ class Dartdoc {
    * Documents the code contained within [span] with [comment]. If [showCode]
    * is `true` (and [includeSource] is set), also includes the source code.
    */
-  void docCode(Location location, String comment, [bool showCode = false]) {
+  void docCode(ObjectMirror host, Location location, DocComment comment,
+               [bool showCode = false]) {
     writeln('<div class="doc">');
     if (comment != null) {
-      writeln(comment);
+      if (comment.inheritedFrom !== null) {
+        writeln('<div class="inherited">');
+        writeln(comment.html);
+        write('<div class="docs-inherited-from">docs inherited from ');
+        annotateType(host, comment.inheritedFrom);
+        write('</div>');
+        writeln('</div>');
+      } else {
+        writeln(comment.html);
+      }
     }
 
     if (includeSource && showCode) {
@@ -1184,40 +1225,56 @@ class Dartdoc {
     writeln('</div>');
   }
 
+  DocComment createDocComment(String text, [InterfaceMirror inheritedFrom]) =>
+      new DocComment(text, inheritedFrom);
+
 
   /** Get the doc comment associated with the given library. */
-  String getLibraryComment(LibraryMirror library) {
+  DocComment getLibraryComment(LibraryMirror library) {
     // Look for a comment for the entire library.
     final comment = _comments.findLibrary(library.location.source);
-    if (comment != null) {
-      return md.markdownToHtml(comment);
-    }
-    return null;
+    if (comment == null) return null;
+    return createDocComment(comment);
   }
 
   /** Get the doc comment associated with the given type. */
-  String getTypeComment(TypeMirror type) {
+  DocComment getTypeComment(TypeMirror type) {
     String comment = _comments.find(type.location);
     if (comment == null) return null;
-    return commentToHtml(comment);
+    return createDocComment(comment);
   }
 
-  /** Get the doc comment associated with the given method. */
-  String getMethodComment(MethodMirror method) {
-    String comment = _comments.find(method.location);
-    if (comment == null) return null;
-    return commentToHtml(comment);
-  }
-
-  /** Get the doc comment associated with the given field. */
-  String getFieldComment(FieldMirror field) {
-    String comment = _comments.find(field.location);
+  /**
+   * Get the doc comment associated with the given member.
+   *
+   * If no comment was found on the member, the hierarchy is traversed to find
+   * an inherited comment, favouring comments inherited from classes over
+   * comments inherited from interfaces.
+   */
+  DocComment getMemberComment(MemberMirror member) {
+    String comment = _comments.find(member.location);
+    InterfaceMirror inheritedFrom = null;
+    if (comment == null) {
+      if (member.surroundingDeclaration is InterfaceMirror) {
+        var iterable =
+            new HierarchyIterable(member.surroundingDeclaration,
+                                  includeType: false);
+        for (InterfaceMirror type in iterable) {
+          var inheritedMember = type.declaredMembers[member.simpleName];
+          if (inheritedMember is MemberMirror) {
+            comment = _comments.find(inheritedMember.location);
+            if (comment != null) {
+              inheritedFrom = type;
+              break;
+            }
+          }
+        }
+      }
+    }
     if (comment == null) return null;
-    return commentToHtml(comment);
+    return createDocComment(comment, inheritedFrom);
   }
 
-  String commentToHtml(String comment) => md.markdownToHtml(comment);
-
   /**
    * Converts [fullPath] which is understood to be a full path from the root of
    * the generated docs to one relative to the current file.
@@ -1266,23 +1323,7 @@ class Dartdoc {
 
   /** Gets the anchor id for the document for [member]. */
   String memberAnchor(MemberMirror member) {
-    if (member.isField) {
-      return member.simpleName;
-    }
-    MethodMirror method = member;
-    if (method.isConstructor) {
-      if (method.constructorName == '') {
-        return method.simpleName;
-      } else {
-        return '${method.simpleName}.${method.constructorName}';
-      }
-    } else if (method.isOperator) {
-      return '${method.simpleName} ${method.operatorName}';
-    } else if (method.isSetter) {
-      return '${method.simpleName}=';
-    } else {
-      return method.simpleName;
-    }
+    return member.simpleName;
   }
 
   /**
@@ -1476,7 +1517,7 @@ class Dartdoc {
 
     // See if it's another member of the current type.
     if (currentType != null) {
-      final foundMember = findMirror(currentType.declaredMembers, name);
+      final foundMember = currentType.declaredMembers[name];
       if (foundMember != null) {
         return makeLink(memberUrl(foundMember));
       }
@@ -1490,11 +1531,13 @@ class Dartdoc {
         final match =
             new RegExp(r'new ([\w$]+)(?:\.([\w$]+))?').firstMatch(name);
         if (match == null) return;
-        InterfaceMirror foundtype = findMirror(currentLibrary.types, match[1]);
+        String typeName = match[1];
+        InterfaceMirror foundtype = currentLibrary.types[typeName];
         if (foundtype == null) return;
+        String constructorName =
+            match[2] == null ? typeName : '$typeName.${match[2]}';

[Lasse Reichstein Nielsen, 2012/10/04 07:41:19]

I recommend parentheses around any non-trivial condition in a condition
expression.

[Johnni Winther, 2012/10/04 12:46:21]

Done.

         final constructor =
-            findMirror(foundtype.constructors,
-                            match[2] == null ? '' : match[2]);
+            foundtype.constructors[constructorName];
         if (constructor == null) return;
         return makeLink(memberUrl(constructor));
       })();
@@ -1504,21 +1547,21 @@ class Dartdoc {
       final foreignMemberLink = (() {
         final match = new RegExp(r'([\w$]+)\.([\w$]+)').firstMatch(name);
         if (match == null) return;
-        InterfaceMirror foundtype = findMirror(currentLibrary.types, match[1]);
+        InterfaceMirror foundtype = currentLibrary.types[match[1]];
         if (foundtype == null) return;
-        MemberMirror foundMember = findMirror(foundtype.declaredMembers, match[2]);
+        MemberMirror foundMember = foundtype.declaredMembers[match[2]];
         if (foundMember == null) return;
         return makeLink(memberUrl(foundMember));
       })();
       if (foreignMemberLink != null) return foreignMemberLink;
 
-      InterfaceMirror foundType = findMirror(currentLibrary.types, name);
+      InterfaceMirror foundType = currentLibrary.types[name];
       if (foundType != null) {
         return makeLink(typeUrl(foundType));
       }
 
       // See if it's a top-level member in the current library.
-      MemberMirror foundMember = findMirror(currentLibrary.declaredMembers, name);
+      MemberMirror foundMember = currentLibrary.declaredMembers[name];
       if (foundMember != null) {
         return makeLink(memberUrl(foundMember));
       }
@@ -1582,3 +1625,20 @@ class InternalError {
   const InternalError(this.message);
   String toString() => "InternalError: '$message'";
 }
+
+class DocComment {
+  final String text;
+
+  /**
+   * Non-null if the comment is inherited from another declaration.
+   */
+  final InterfaceMirror inheritedFrom;
+
+  DocComment(this.text, [this.inheritedFrom = null]) {
+    assert(text != null && !text.trim().isEmpty());
+  }
+
+  String get html => md.markdownToHtml(text);

[Lasse Reichstein Nielsen, 2012/10/04 07:41:19]

Why isn't the "inherited from" HTML added by this class, instead of doing it in
several places above?

If there are different versions of the comment, you could have different getters
for the different cases, but localized in the DocComment class.

[Johnni Winther, 2012/10/04 12:46:21]

The functionality might fit in the DocComment class if it had more information.
We want better diagnostics in parsing markdown, so it might all be put here in a
later CL.

+
+  String toString() => text;
+}