fix lends tag inside nested function calls (#565)

plus some minor test cleanup
This commit is contained in:
Jeff Williams 2014-02-23 20:16:16 -08:00
parent ec93145550
commit 4e3e4a0151
3 changed files with 69 additions and 16 deletions

View File

@ -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();
}

15
test/fixtures/lends5.js vendored Normal file
View File

@ -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);

View File

@ -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();
});
});
});