From b28379e3983d0817bff25f1cb6f54c448ff1272f Mon Sep 17 00:00:00 2001 From: Jeff Williams Date: Sat, 15 Jul 2017 15:33:18 -0700 Subject: [PATCH] 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 | 76 +++++++++++++++++++++------ test/fixtures/overridetag2.js | 44 ++++++++++++++++ test/specs/tags/overridetag.js | 94 ++++++++++++++++++++++------------ 3 files changed, 165 insertions(+), 49 deletions(-) create mode 100644 test/fixtures/overridetag2.js diff --git a/lib/jsdoc/augment.js b/lib/jsdoc/augment.js index 2754ad3b..eea1d976 100644 --- a/lib/jsdoc/augment.js +++ b/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); } } } diff --git a/test/fixtures/overridetag2.js b/test/fixtures/overridetag2.js new file mode 100644 index 00000000..44e6ef0a --- /dev/null +++ b/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) {}; diff --git a/test/specs/tags/overridetag.js b/test/specs/tags/overridetag.js index df7bda19..b021c415 100644 --- a/test/specs/tags/overridetag.js +++ b/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); + }); + }); });