diff --git a/lib/jsdoc/src/astbuilder.js b/lib/jsdoc/src/astbuilder.js index 504d6cdb..ed478e46 100644 --- a/lib/jsdoc/src/astbuilder.js +++ b/lib/jsdoc/src/astbuilder.js @@ -185,17 +185,16 @@ function CommentAttacher(comments, tokens) { this._tokens = tokens || []; this._tokenIndex = 0; - this._previousNodeEnd = 0; + this._previousNode = null; this._astRoot = null; - this._strayComments = []; - this._resetPendingComment() + this._resetPendingComments() ._resetCandidates(); } // TODO: docs -CommentAttacher.prototype._resetPendingComment = function() { - this._pendingComment = null; +CommentAttacher.prototype._resetPendingComments = function() { + this._pendingComments = []; this._pendingCommentRange = null; return this; @@ -221,7 +220,7 @@ CommentAttacher.prototype._nextToken = function() { // TODO: docs // find the index of the atom whose end position is closest to (but not after) the specified // position -CommentAttacher.prototype._nextIndexBefore = function(startIndex, atoms, position) { +CommentAttacher.prototype._nextIndexBefore = function(atoms, startIndex, position) { var atom; var newIndex = startIndex; @@ -244,7 +243,7 @@ CommentAttacher.prototype._nextIndexBefore = function(startIndex, atoms, positio CommentAttacher.prototype._advanceTokenIndex = function(node) { var position = node.range[0]; - this._tokenIndex = this._nextIndexBefore(this._tokenIndex, this._tokens, position); + this._tokenIndex = this._nextIndexBefore(this._tokens, this._tokenIndex, position); return this; }; @@ -252,33 +251,54 @@ 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 = this._nextIndexBefore(this._comments, 0, position); - // all comments before the node (except the last one) are considered stray comments + // all comments before the node (except the last one) are pended if (commentIndex > 0) { - this._strayComments = this._strayComments.concat( this._comments.splice(0, + this._pendingComments = this._pendingComments.concat( this._comments.splice(0, commentIndex) ); } }; +CommentAttacher.prototype._attachPendingCommentsAsLeading = function(target) { + target.leadingComments = (target.leadingComments || []).concat(this._pendingComments); +}; + +CommentAttacher.prototype._attachPendingCommentsAsTrailing = function(target) { + target.trailingComments = (target.trailingComments || []).concat(this._pendingComments); +}; + // TODO: docs -CommentAttacher.prototype._attachPendingComment = function() { +CommentAttacher.prototype._attachPendingComments = function(currentNode) { var target; - if (!this._pendingComment) { + if (!this._pendingComments.length) { return this; } + // if there are one or more candidate nodes, attach the pending comments before the last + // candidate node if (this._candidates.length > 0) { target = this._candidates[this._candidates.length - 1]; - target.leadingComments = target.leadingComments || []; - target.leadingComments.push(this._pendingComment); + this._attachPendingCommentsAsLeading(target); } + // if we don't have a previous node, attach pending comments before the AST root; this should + // mean that we haven't encountered any other nodes yet, or that the source file contains + // JSDoc comments but not code + else if (!this._previousNode) { + target = this._astRoot; + this._attachPendingCommentsAsLeading(target); + } + // otherwise, the comments must come after the current node (or the last node of the AST, if + // we've run out of nodes) else { - this._strayComments.push(this._pendingComment); + this._attachPendingCommentsAsTrailing(currentNode || this._previousNode); } - this._resetPendingComment() + // update the previous node + this._previousNode = currentNode; + + this._resetPendingComments() ._resetCandidates(); return this; @@ -329,8 +349,8 @@ CommentAttacher.prototype.visit = function(node) { // now we can check whether the current node is in the right position to accept the next comment isEligible = this._isEligible(node); - // attach the pending comment, if there is one - this._attachPendingComment(); + // attach the pending comments, if any + this._attachPendingComments(node); // okay, now that we've done all that bookkeeping, we can check whether the current node accepts // leading comments and add it to the candidate list if needed @@ -341,8 +361,8 @@ CommentAttacher.prototype.visit = function(node) { } this._candidates.push(node); - // we have a candidate node, so pend the current comment if necessary - this._pendingComment = this._pendingComment || this._comments.splice(0, 1)[0]; + // we have a candidate node, so pend the current comment + this._pendingComments.push(this._comments.splice(0, 1)[0]); } return VISITOR_CONTINUE; @@ -350,41 +370,50 @@ CommentAttacher.prototype.visit = function(node) { // TODO: docs CommentAttacher.prototype.finish = function() { - // any remaining comments are stray comments - this._strayComments = this._strayComments.concat(this._comments); + var length = this._comments.length; - // deal with the pending comment, if there is one - this._attachPendingComment(); - - // attach stray comments to the AST root - if (this._strayComments.length) { - this._astRoot.trailingComments = this._strayComments.slice(0); + // any leftover comments are pended + if (length) { + this._pendingComments = this._pendingComments.concat( this._comments.splice(0, length) ); } + + // attach the pending comments, if any + this._attachPendingComments(); }; // TODO: docs // TODO: refactor to make this extensible /** + * @private * @param {string} filename - The full path to the source file. * @param {Object} ast - An abstract syntax tree that conforms to the Mozilla Parser API. */ AstBuilder.prototype._postProcess = function(filename, ast) { var attachComments = !!ast.comments && !!ast.comments.length; - var commentAttacher = new CommentAttacher( scrubComments(ast.comments.slice(0)), ast.tokens ); - var visitor = { + var commentAttacher; + var scrubbed; + var visitor; + var walker; + + if (!attachComments) { + return; + } + + scrubbed = scrubComments(ast.comments.slice(0)); + commentAttacher = new CommentAttacher(scrubbed.slice(0), ast.tokens); + visitor = { visit: function(node) { - if (attachComments) { - attachComments = commentAttacher.visit(node); - } + return commentAttacher.visit(node); } }; + walker = new jsdoc.src.Walker(); - var walker = new jsdoc.src.Walker(); - walker.recurse(filename, ast, visitor); + walker.recurse(ast, visitor, filename); commentAttacher.finish(); - // remove the comment/token arrays; we no longer need then - ast.comments = []; + // replace the comments with the filtered comments + ast.comments = scrubbed; + // we no longer need the tokens ast.tokens = []; }; diff --git a/lib/jsdoc/src/parser.js b/lib/jsdoc/src/parser.js index aa357a12..d3d025c7 100644 --- a/lib/jsdoc/src/parser.js +++ b/lib/jsdoc/src/parser.js @@ -1,5 +1,3 @@ -/*global env, Packages */ -/*eslint no-script-url:0 */ /** * @module jsdoc/src/parser */ @@ -26,13 +24,15 @@ var util = require('util'); var hasOwnProp = Object.prototype.hasOwnProperty; var Syntax = jsdoc.src.syntax.Syntax; -// Prefix for JavaScript strings that were provided in lieu of a filename. -var SCHEMA = 'javascript:'; // TODO: docs var PARSERS = exports.PARSERS = { esprima: 'jsdoc/src/parser', rhino: 'rhino/jsdoc/src/parser' }; +/*eslint-disable no-script-url */ +// Prefix for JavaScript strings that were provided in lieu of a filename. +var SCHEMA = 'javascript:'; +/*eslint-enable no-script-url */ // TODO: docs exports.createParser = function(type) { @@ -120,7 +120,7 @@ Parser.prototype.clear = function() { * var docs = jsdocParser.parse(myFiles); */ Parser.prototype.parse = function(sourceFiles, encoding) { - encoding = encoding || env.conf.encoding || 'utf8'; + encoding = encoding || global.env.conf.encoding || 'utf8'; var filename = ''; var sourceCode = ''; @@ -233,7 +233,7 @@ Parser.prototype._parseSourceCode = function(sourceCode, sourceName) { ast = this._astBuilder.build(sourceCode, sourceName); if (ast) { - this._walker.recurse(sourceName, ast, this._visitor); + this._walkAst(ast, this._visitor, sourceName); } } @@ -241,6 +241,11 @@ Parser.prototype._parseSourceCode = function(sourceCode, sourceName) { logger.info('complete.'); }; +/** @private */ +Parser.prototype._walkAst = function(ast, visitor, sourceName) { + this._walker.recurse(ast, visitor, sourceName); +}; + // TODO: docs Parser.prototype.addDocletRef = function(e) { var node; diff --git a/lib/jsdoc/src/visitor.js b/lib/jsdoc/src/visitor.js index 80e909e0..5a72b3d2 100644 --- a/lib/jsdoc/src/visitor.js +++ b/lib/jsdoc/src/visitor.js @@ -182,13 +182,6 @@ Visitor.prototype.visit = function(node, filename) { this._visitors[i].call(this, node, this._parser, filename); } - // we also need to visit standalone comments, which are not attached to a node - if (node.type === Syntax.Program && node.comments && node.comments.length) { - for (i = 0, l = node.comments.length; i < l; i++) { - this.visitNodeComments.call(this, node.comments[i], this._parser, filename); - } - } - return true; }; @@ -206,8 +199,8 @@ function isValidJsdoc(commentSrc) { // TODO: docs function hasJsdocComments(node) { - return (node && node.leadingComments && node.leadingComments.length > 0) || - (node && node.trailingComments && node.trailingComments.length > 0); + return (node && node.leadingComments && node.leadingComments.length) || + (node && node.trailingComments && node.trailingComments.length); } // TODO: docs @@ -233,13 +226,10 @@ Visitor.prototype.visitNodeComments = function(node, parser, filename) { return true; } - comments = []; - if (node.type === BLOCK_COMMENT) { - comments.push(node); - } + comments = (node.type === BLOCK_COMMENT) ? [node] : []; if (node.leadingComments && node.leadingComments.length) { - comments = node.leadingComments.slice(0); + comments = comments.concat( node.leadingComments.slice(0) ); } if (node.trailingComments && node.trailingComments.length) { @@ -269,12 +259,6 @@ Visitor.prototype.visitNode = function(node, parser, filename) { var e = this.makeSymbolFoundEvent(node, parser, filename); - if (!node.nodeId) { - Object.defineProperty(node, 'nodeId', { - value: parser.getUniqueId() - }); - } - if (this._nodeVisitors && this._nodeVisitors.length) { for (i = 0, l = this._nodeVisitors.length; i < l; i++) { this._nodeVisitors[i].visitNode(node, e, parser, filename); diff --git a/lib/jsdoc/src/walker.js b/lib/jsdoc/src/walker.js index 2407dc89..ad1ac917 100644 --- a/lib/jsdoc/src/walker.js +++ b/lib/jsdoc/src/walker.js @@ -521,14 +521,17 @@ Walker.prototype._recurse = function(filename, ast) { }; // TODO: docs -// TODO: allow visitor.visit to prevent traversal of child nodes // TODO: skip the AST root node to be consistent with Rhino? -Walker.prototype.recurse = function(filename, ast, visitor) { +Walker.prototype.recurse = function(ast, visitor, filename) { + var shouldContinue; var state = this._recurse(filename, ast); if (visitor) { for (var i = 0, l = state.nodes.length; i < l; i++) { - visitor.visit.call(visitor, state.nodes[i], filename); + shouldContinue = visitor.visit.call(visitor, state.nodes[i], filename); + if (!shouldContinue) { + break; + } } } diff --git a/rhino/js.jar b/rhino/js.jar index 696a37d4..83b3b70f 100644 Binary files a/rhino/js.jar and b/rhino/js.jar differ diff --git a/rhino/jsdoc/src/parser.js b/rhino/jsdoc/src/parser.js index 1b1c05b2..3d26552d 100644 --- a/rhino/jsdoc/src/parser.js +++ b/rhino/jsdoc/src/parser.js @@ -36,3 +36,13 @@ Parser.prototype.addNodeVisitor = function(visitor) { Parser.prototype.getNodeVisitors = function() { return this._visitor.getRhinoNodeVisitors(); }; + +// TODO: docs +Parser.prototype._walkAst = function(ast, visitor, sourceName) { + // On Rhino, we visit the comments all at once before we walk the AST + this._visitor.visitNodeComments({ + leadingComments: ast.comments + }, this, sourceName); + + Parser.super_.prototype._walkAst.call(this, ast, visitor, sourceName); +}; diff --git a/rhino/jsdoc/src/visitor.js b/rhino/jsdoc/src/visitor.js index 0295f2df..783c17a6 100644 --- a/rhino/jsdoc/src/visitor.js +++ b/rhino/jsdoc/src/visitor.js @@ -15,6 +15,11 @@ var Visitor = exports.Visitor = function(parser) { // Rhino nodes retrieved from the org.jsdoc.AstBuilder instance this._rhinoNodes = null; + // only visit nodes, not their comments--the parser visits all the comments at once + this._visitors = [ + this.visitNode + ]; + this.addAstNodeVisitor({ visitNode: this._visitRhinoNode.bind(this) }); @@ -26,6 +31,14 @@ Visitor.prototype.addRhinoNodeVisitor = function(visitor) { this._rhinoNodeVisitors.push(visitor); }; +// TODO: docs (deprecated) +Visitor.prototype.removeRhinoNodeVisitor = function(visitor) { + var idx = this._rhinoNodeVisitors.indexOf(visitor); + if (idx !== -1) { + this._rhinoNodeVisitors.splice(idx, 1); + } +}; + // TODO: docs (deprecated) Visitor.prototype.getRhinoNodeVisitors = function() { return this._rhinoNodeVisitors; @@ -34,9 +47,8 @@ Visitor.prototype.getRhinoNodeVisitors = function() { // TODO: docs (deprecated) Visitor.prototype._visitRhinoNode = function(astNode, e, parser, filename) { var rhinoNode; - var visitors = this._rhinoNodeVisitors; - // if there are no visitors, bail out before we retrieve all the nodes + if (!visitors.length) { return; } diff --git a/test/fixtures/eventorder.js b/test/fixtures/eventorder.js new file mode 100644 index 00000000..0c9e497b --- /dev/null +++ b/test/fixtures/eventorder.js @@ -0,0 +1,32 @@ +'use strict'; + +var _ = require('underscore'); + +/** + * Socket class. + * @class + */ +function Socket() { + // ... +} + +/** + * Send a packet. + * @param {Packet} packet - The packet to send. + * @return {boolean} `true` on success, `false` on failure. + */ +Socket.prototype.send = function(packet) { + // ... +}; + +/** + * Virtual doclet for `Packet` class. + * @class Packet + */ + +/** + * Close the socket. + */ +Socket.prototype.close = function() { + // ... +}; diff --git a/test/specs/jsdoc/src/parser.js b/test/specs/jsdoc/src/parser.js index 2da1d9d0..0266d03e 100644 --- a/test/specs/jsdoc/src/parser.js +++ b/test/specs/jsdoc/src/parser.js @@ -340,6 +340,71 @@ describe('jsdoc/src/parser', function() { expect(doclet.comment).toMatch('REPLACED!'); }); }); + + describe('event order', function() { + var events = { + all: [], + jsdocCommentFound: [], + symbolFound: [] + }; + var source = fs.readFileSync(path.join(global.env.dirname, + 'test/fixtures/eventorder.js'), 'utf8'); + + function pushEvent(e) { + events.all.push(e); + events[e.event].push(e); + } + + function sourceOrderSort(atom1, atom2) { + if (atom1.range[1] < atom2.range[0]) { + return -1; + } + else if (atom1.range[0] < atom2.range[0] && atom1.range[1] === atom2.range[1]) { + return 1; + } + else { + return 0; + } + } + + // Rhino fires events in a different order + if (jasmine.jsParser === 'rhino') { + it('should fire all jsdocCommentFound events, in source order, ' + + 'then all symbolFound events, in source order', function() { + parser.on('jsdocCommentFound', pushEvent); + + parser.on('symbolFound', pushEvent); + + jsdoc.src.handlers.attachTo(parser); + parser.parse('javascript:' + source); + + // make sure jsdocCommentFound events are in the correct order + events.jsdocCommentFound.slice(0).sort(sourceOrderSort) + .forEach(function(e, i) { + expect(e).toBe(events.jsdocCommentFound[i]); + }); + // make sure symbolFound events are in the correct order + events.symbolFound.slice(0).sort(sourceOrderSort).forEach(function(e, i) { + expect(e).toBe(events.symbolFound[i]); + }); + // make sure jsdocCommentFound events are all first + events.all.slice(0, events.jsdocCommentFound.length) + .forEach(function(e, i) { + expect(e).toBe(events.all[i]); + }); + }); + } + else { + it('should fire interleaved jsdocCommentFound and symbolFound events, ' + + 'in source order', function() { + jsdoc.src.handlers.attachTo(parser); + parser.parse(source); + events.all.slice(0).sort(sourceOrderSort).forEach(function(e, i) { + expect(e).toBe(events.all[i]); + }); + }); + } + }); }); describe('addAstNodeVisitor', function() {