make the @override tag work as expected (#1215)

The `@override` tag no longer causes all other doclet properties to be ignored. Instead, the doclet will keep its own properties and inherit any missing properties from the parent class or interface.

lib/jsdoc/augment.js

@@ -6,6 +6,9 @@
  */
 
 var doop = require('jsdoc/util/doop');
+var jsdoc = {
+    doclet: require('jsdoc/doclet')
+};
 var name = require('jsdoc/name');
 
 var hasOwnProp = Object.prototype.hasOwnProperty;
@@ -87,6 +90,12 @@ function getMembers(longname, docs, scopes) {
     return members;
 }
 
+function getDocumentedLongname(longname, docs) {
+    var doclets = docs.index.documented[longname] || [];
+
+    return doclets[doclets.length - 1];
+}
+
 function addDocletProperty(doclets, propName, value) {
     for (var i = 0, l = doclets.length; i < l; i++) {
         doclets[i][propName] = value;
@@ -194,13 +203,24 @@ function explicitlyInherits(doclets) {
     return inherits;
 }
 
+function changeMemberof(longname, newMemberof) {
+    var atoms = name.shorten(longname);
+
+    atoms.memberof = newMemberof;
+
+    return name.combine(atoms);
+}
+
 // TODO: try to reduce overlap with similar methods
 function getInheritedAdditions(doclets, docs, index) {
     var additionIndexes;
     var additions = [];
+    var childDoclet;
+    var childLongname;
     var doc;
+    var parentDoclet;
+    var parentMembers;
     var parents;
-    var members;
     var member;
     var parts;
 
@@ -216,34 +236,42 @@ function getInheritedAdditions(doclets, docs, index) {
             additionIndexes = {};
 
             for (var j = 0, jj = parents.length; j < jj; j++) {
-                members = getMembers(parents[j], docs, ['instance']);
+                parentMembers = getMembers(parents[j], docs, ['instance']);
+
+                for (var k = 0, kk = parentMembers.length; k < kk; k++) {
+                    parentDoclet = parentMembers[k];
 
-                for (var k = 0, kk = members.length; k < kk; k++) {
                     // We only care about symbols that are documented.
-                    if (members[k].undocumented) {
+                    if (parentDoclet.undocumented) {
                         continue;
                     }
 
-                    member = doop(members[k]);
+                    childLongname = changeMemberof(parentDoclet.longname, doc.longname);
+                    childDoclet = getDocumentedLongname(childLongname, docs) || {};
+
+                    // We don't want to fold in properties from the child doclet if it had an
+                    // `@inheritdoc` tag.
+                    if (hasOwnProp.call(childDoclet, 'inheritdoc')) {
+                        childDoclet = {};
+                    }
+
+                    member = jsdoc.doclet.combine(childDoclet, parentDoclet);
 
                     if (!member.inherited) {
                         member.inherits = member.longname;
                     }
                     member.inherited = true;
 
-                    // TODO: this will fail on longnames like: MyClass#"quoted#Longname"
-                    // and nested instance members like: MyClass#MyOtherClass#myMethod;
-                    // switch to updateLongname()!
                     member.memberof = doc.longname;
-                    parts = member.longname.split('#');
-                    parts[0] = doc.longname;
-                    member.longname = parts.join('#');
+                    parts = name.shorten(member.longname);
+                    parts.memberof = doc.longname;
+                    member.longname = name.combine(parts);
 
                     // Indicate what the descendant is overriding. (We only care about the closest
                     // ancestor. For classes A > B > C, if B#a overrides A#a, and C#a inherits B#a,
                     // we don't want the doclet for C#a to say that it overrides A#a.)
                     if ( hasOwnProp.call(docs.index.longname, member.longname) ) {
-                        member.overrides = members[k].longname;
+                        member.overrides = parentDoclet.longname;
                     }
                     else {
                         delete member.overrides;
@@ -283,7 +311,7 @@ function getInheritedAdditions(doclets, docs, index) {
                     // update the doclets to indicate what the descendant is overriding.
                     else {
                         addDocletProperty(index.documented[member.longname], 'overrides',
-                            members[k].longname);
+                            parentDoclet.longname);
                     }
                 }
             }
@@ -395,12 +423,15 @@ function updateImplements(implDoclets, implementedLongname) {
 function getImplementedAdditions(implDoclets, allDoclets, index) {
     var additionIndexes;
     var additions = [];
+    var childDoclet;
+    var childLongname;
     var commentedDoclets = index.documented;
     var doclet;
     var implementations;
     var implExists;
     var implementationDoclet;
     var interfaceDoclets;
+    var parentDoclet;
 
     // interfaceDoclets will be undefined if the implemented symbol isn't documented
     implDoclets = implDoclets || [];
@@ -417,15 +448,26 @@ function getImplementedAdditions(implDoclets, allDoclets, index) {
                 interfaceDoclets = getMembers(implementations[j], allDoclets, ['instance']);
 
                 for (var k = 0, kk = interfaceDoclets.length; k < kk; k++) {
+                    parentDoclet = interfaceDoclets[k];
+
                     // We only care about symbols that are documented.
-                    if (interfaceDoclets[k].undocumented) {
+                    if (parentDoclet.undocumented) {
                         continue;
                     }
 
-                    implementationDoclet = doop(interfaceDoclets[k]);
+                    childLongname = changeMemberof(parentDoclet.longname, doclet.longname);
+                    childDoclet = getDocumentedLongname(childLongname, allDoclets) || {};
+
+                    // We don't want to fold in properties from the child doclet if it had an
+                    // `@inheritdoc` tag.
+                    if (hasOwnProp.call(childDoclet, 'inheritdoc')) {
+                        childDoclet = {};
+                    }
+
+                    implementationDoclet = jsdoc.doclet.combine(childDoclet, parentDoclet);
 
                     reparentDoclet(doclet, implementationDoclet);
-                    updateImplements(implementationDoclet, interfaceDoclets[k].longname);
+                    updateImplements(implementationDoclet, parentDoclet.longname);
 
                     // If there's no implementation, move along.
                     implExists = hasOwnProp.call(allDoclets.index.longname,
@@ -469,7 +511,7 @@ function getImplementedAdditions(implDoclets, allDoclets, index) {
                     // indicate what the implementation is implementing.
                     else {
                         updateImplements(commentedDoclets[implementationDoclet.longname],
-                            interfaceDoclets[k].longname);
+                            parentDoclet.longname);
                     }
                 }
             }

test/fixtures/overridetag2.js

@@ -0,0 +1,44 @@
+/**
+ * Parent interface.
+ * @interface
+ */
+function Connection() {}
+
+/**
+ * Open the connection.
+ */
+Connection.prototype.open = function() {};
+
+/**
+ * Close the connection.
+ */
+Connection.prototype.close = function() {};
+
+/**
+ * Read the specified number of bytes from the connection.
+ *
+ * @function Connection#read
+ * @param {number} bytes - The number of bytes to read.
+ * @return {Buffer} The bytes that were read.
+ */
+Connection.prototype.read = function(bytes) {};
+
+/**
+ * Child class.
+ * @class
+ * @implements Connection
+ */
+function Socket() {}
+
+/** @override */
+Socket.prototype.open = function() {};
+
+/**
+ * Close the socket.
+ * @param {string} message - A message explaining why the socket is being closed.
+ * @override
+ */
+Socket.prototype.close = function() {};
+
+/** @override */
+Socket.prototype.read = function(bytes) {};

test/specs/tags/overridetag.js

@@ -2,44 +2,74 @@
 
 describe('@override tag', function() {
     var docSet = jasmine.getDocSetFromFile('test/fixtures/overridetag.js');
+    var docSet2 = jasmine.getDocSetFromFile('test/fixtures/overridetag2.js');
 
     function ignored($) {
         return $.ignore !== true;
     }
 
-    it('should cause the symbol to be documented', function() {
-        var open = docSet.getByLongname('Socket#open');
-
-        expect(open.length).toBe(2);
-        expect(open[0].ignore).toBe(true);
-        expect(open[1].ignore).not.toBeDefined();
-        expect(open[1].description).toBe('Open the connection.');
-    });
-
-    it('should cause all other tags to be ignored', function() {
-        var close = docSet.getByLongname('Socket#close').filter(ignored)[0];
-
-        expect(close.description).toBe('Close the connection.');
-        expect(close.params).not.toBeDefined();
-    });
-
-    it('should not say that the child symbol is abstract', function() {
-        var open = docSet.getByLongname('Socket#open').filter(ignored)[0];
-        var parentOpen = docSet.getByLongname('Connection#open')[0];
-
-        expect(parentOpen.virtual).toBe(true);
-        expect(open.virtual).not.toBeDefined();
-    });
-
-    it('should work with interface members whose names are specified in the comment', function() {
-        var connectionRead = docSet.getByLongname('Connection#read').filter(ignored)[0];
-        var socketRead = docSet.getByLongname('Socket#read').filter(ignored)[0];
-
-        expect(socketRead).toBeDefined();
-        expect(socketRead.description).toBe(connectionRead.description);
-    });
-
     xit('should only be available if the Closure dictionary is enabled', function() {
         // TODO
     });
+
+    describe('classes', function() {
+        it('should cause the symbol to be documented', function() {
+            var open = docSet.getByLongname('Socket#open');
+
+            expect(open.length).toBe(2);
+            expect(open[0].ignore).toBe(true);
+            expect(open[1].ignore).not.toBeDefined();
+            expect(open[1].description).toBe('Open the connection.');
+        });
+
+        it('should use any other tags that are defined', function() {
+            var close = docSet.getByLongname('Socket#close').filter(ignored)[0];
+
+            expect(close.description).toBe('Close the socket.');
+            expect(close.params).toBeDefined();
+            expect(close.params.length).toBe(1);
+        });
+
+        it('should not say that the child symbol is abstract', function() {
+            var open = docSet.getByLongname('Socket#open').filter(ignored)[0];
+            var parentOpen = docSet.getByLongname('Connection#open')[0];
+
+            expect(parentOpen.virtual).toBe(true);
+            expect(open.virtual).not.toBeDefined();
+        });
+
+        it('should work with class members whose names are specified in the comment', function() {
+            var connectionRead = docSet.getByLongname('Connection#read').filter(ignored)[0];
+            var socketRead = docSet.getByLongname('Socket#read').filter(ignored)[0];
+
+            expect(socketRead).toBeDefined();
+            expect(socketRead.description).toBe(connectionRead.description);
+        });
+    });
+
+    describe('interfaces', function() {
+        it('should cause the symbol to be documented', function() {
+            var open = docSet2.getByLongname('Socket#open').filter(ignored);
+
+            expect(open.length).toBe(1);
+            expect(open[0].description).toBe('Open the connection.');
+        });
+
+        it('should use any other tags that are defined', function() {
+            var close = docSet2.getByLongname('Socket#close').filter(ignored)[0];
+
+            expect(close.description).toBe('Close the socket.');
+            expect(close.params).toBeDefined();
+            expect(close.params.length).toBe(1);
+        });
+
+        it('should work with interface members whose names are specified in the comment',
+            function() {
+                var connectionRead = docSet2.getByLongname('Connection#read').filter(ignored)[0];
+                var socketRead = docSet2.getByLongname('Socket#read').filter(ignored)[0];
+
+                expect(socketRead).toBeDefined();
+                expect(socketRead.description).toBe(connectionRead.description);
+            });
+    });
 });