From 4e3e4a01515f13b99ecbd2dcb2759a32d6547904 Mon Sep 17 00:00:00 2001 From: Jeff Williams Date: Sun, 23 Feb 2014 20:16:16 -0800 Subject: [PATCH] fix lends tag inside nested function calls (#565) plus some minor test cleanup --- lib/jsdoc/src/astbuilder.js | 35 ++++++++++++++++++++++++------- test/fixtures/lends5.js | 15 +++++++++++++ test/specs/documentation/lends.js | 35 ++++++++++++++++++++++++------- 3 files changed, 69 insertions(+), 16 deletions(-) create mode 100644 test/fixtures/lends5.js diff --git a/lib/jsdoc/src/astbuilder.js b/lib/jsdoc/src/astbuilder.js index 93b76d7a..a32587ba 100644 --- a/lib/jsdoc/src/astbuilder.js +++ b/lib/jsdoc/src/astbuilder.js @@ -30,10 +30,10 @@ var acceptsLeadingComments = exports.acceptsLeadingComments = (function() { })(); // TODO: docs -var searchChildNodes = (function() { +var searchDescendants = (function() { var search = {}; var shouldSearch = [ - // TODO: add to this as needed + Syntax.CallExpression ]; for (var i = 0, l = shouldSearch.length; i < l; i++) { @@ -155,6 +155,8 @@ function CommentAttacher(comments, tokens) { this._previousNodeEnd = 0; this._astRoot = null; this._strayComments = []; + + this._parentRange = []; this._resetPendingComment() ._resetCandidates(); @@ -223,8 +225,16 @@ CommentAttacher.prototype._advanceTokenIndex = function(node) { // TODO: docs CommentAttacher.prototype._fastForwardComments = function(node) { - var position = node.range[0]; - var commentIndex = this._nextIndexBefore(0, this._comments, position); + var commentIndex; + var position; + + // don't fast-forward if there might be a sibling that can receive the comment + if ( this._parentRange.length && isWithin(node.range, this._parentRange) ) { + return; + } + + position = node.range[0]; + commentIndex = this._nextIndexBefore(0, this._comments, position); // all comments before the node (except the last one) are considered stray comments if (commentIndex > 0) { @@ -282,12 +292,11 @@ CommentAttacher.prototype._isEligible = function(node) { // TODO: docs CommentAttacher.prototype._shouldSkipNode = function(node) { - return !isWithin(node.range, this._pendingCommentRange) && !searchChildNodes[node.type]; + return !isWithin(node.range, this._pendingCommentRange) && !this._parentRange.length; }; // TODO: docs -// TODO: this is overdesigned. should be able to ditch searchChildNodes. how often do we get -// multiple candidate nodes? +// TODO: do we ever get multiple candidate nodes? CommentAttacher.prototype.visit = function(node) { var isEligible; @@ -299,6 +308,16 @@ CommentAttacher.prototype.visit = function(node) { // set the AST root if necessary this._astRoot = this._astRoot || node; + // clear the parent range if we've moved past it + if ( this._parentRange.length && !isWithin(node.range, this._parentRange) ) { + this._parentRange = []; + } + + // if we need to search this node's descendants, set the range in which to search + if (searchDescendants[node.type] && !this._parentRange.length) { + this._parentRange = node.range.slice(0); + } + // move to the next token, and fast-forward past comments that can no longer be attached this._advanceTokenIndex(node); this._fastForwardComments(node); @@ -308,7 +327,7 @@ CommentAttacher.prototype.visit = function(node) { // if we already had a pending comment, and the current node is in the wrong place to be a // comment target, try attaching the comment if ( (this._pendingComment && !isWithin(node.range, this._pendingCommentRange)) || - !searchChildNodes[node.type] ) { + !searchDescendants[node.type] ) { this._attachPendingComment(); } diff --git a/test/fixtures/lends5.js b/test/fixtures/lends5.js new file mode 100644 index 00000000..846cd17a --- /dev/null +++ b/test/fixtures/lends5.js @@ -0,0 +1,15 @@ +(function() { + /** + * @class Person + */ + function Person(name) {} + + Person.prototype = Object.create(null, /** @lends Person.prototype */ { + /** Speak a message. */ + say: function(message) { + return this.name + " says: " + message; + } + }); + + this.Person = Person; +}).call(this); diff --git a/test/specs/documentation/lends.js b/test/specs/documentation/lends.js index 74904ad4..9f0c4293 100644 --- a/test/specs/documentation/lends.js +++ b/test/specs/documentation/lends.js @@ -51,18 +51,37 @@ describe("lends", function() { describe("case that uses @lends within a closure", function() { var docSet = jasmine.getDocSetFromFile('test/fixtures/lends4.js'); - var klass = docSet.getByLongname('Person'); - var name = docSet.getByLongname('Person#name'); + var person = docSet.getByLongname('Person'); + var say = docSet.getByLongname('Person#say'); it("The class constructor should be documented with the name of the lendee", function() { - expect(klass.length).toBe(1); - expect(klass[0].name).toBe('Person'); - expect(klass[0].kind).toBe('class'); - expect(klass[0].scope).toBe('global'); + expect(person.length).toBe(1); + expect(person[0].name).toBe('Person'); + expect(person[0].kind).toBe('class'); }); - it("A class member should be documented as a member of the lendee", function() { - expect(name.length).toBe(1); + it("A class' instance method should be documented as a member of the lendee", function() { + expect(say.length).toBe(1); + }); + }); + + describe("case that uses @lends within nested function calls", function() { + var docSet = jasmine.getDocSetFromFile('test/fixtures/lends5.js'); + var person = docSet.getByLongname('Person').filter(function(d) { + return !d.undocumented; + })[0]; + var say = docSet.getByLongname('Person#say').filter(function(d) { + return !d.undocumented; + })[0]; + + it("The class constructor should be documented with the name of the lendee", function() { + expect(person).toBeDefined(); + expect(person.name).toBe('Person'); + expect(person.kind).toBe('class'); + }); + + it("A class' instance method should be documented as a member of the lendee", function() { + expect(say).toBeDefined(); }); }); });