Skip to content

Commit

Permalink
Update: Deprecate sourceCode.getComments() (fixes #8408) (#8434)
Browse files Browse the repository at this point in the history
* Update: Deprecate sourceCode.getComments() (fixes #8408)

* Refactor common code into separate method

* Update JSDoc comments

* One more JSDoc comment fix for consistency
  • Loading branch information
kaicataldo committed Apr 20, 2017
1 parent ac39e3b commit 735d02d
Show file tree
Hide file tree
Showing 17 changed files with 227 additions and 39 deletions.
16 changes: 13 additions & 3 deletions docs/developer-guide/working-with-rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,9 @@ Once you have an instance of `SourceCode`, you can use the methods on it to work

* `getText(node)` - returns the source code for the given node. Omit `node` to get the whole source.
* `getAllComments()` - returns an array of all comments in the source.
* `getComments(node)` - returns the leading and trailing comments arrays for the given node.
* `getCommentsBefore(nodeOrToken)` - returns an array of comment tokens that occur directly before the given node or token.
* `getCommentsAfter(nodeOrToken)` - returns an array of comment tokens that occur directly after the given node or token.
* `getCommentsInside(node)` - returns an array of all comment tokens inside a given node.
* `getJSDocComment(node)` - returns the JSDoc comment for a given node or `null` if there is none.
* `isSpaceBetweenTokens(first, second)` - returns true if there is a whitespace character between the two tokens.
* `getFirstToken(node, skipOptions)` - returns the first token representing the given node.
Expand Down Expand Up @@ -307,6 +309,14 @@ There are also some properties you can access:

You should use a `SourceCode` object whenever you need to get more information about the code being linted.

#### Deprecated

Please note that the following methods have been deprecated and will be removed in a future version of ESLint:

* `getComments()` - replaced by `getCommentsBefore()`, `getCommentsAfter()`, and `getCommentsInside()`
* `getTokenOrCommentBefore()` - replaced by `getTokenBefore()` with the `{ includeComments: true }` option
* `getTokenOrCommentAfter()` - replaced by `getTokenAfter()` with the `{ includeComments: true }` option

### Options Schemas

Rules may export a `schema` property, which is a [JSON schema](http://json-schema.org/) format description of a rule's options which will be used by ESLint to validate configuration options and prevent invalid or unexpected inputs before they are passed to the rule in `context.options`.
Expand Down Expand Up @@ -370,9 +380,9 @@ While comments are not technically part of the AST, ESLint provides a few ways f

This method returns an array of all the comments found in the program. This is useful for rules that need to check all comments regardless of location.

#### sourceCode.getComments(node)
#### sourceCode.getCommentsBefore(), sourceCode.getCommentsAfter(), and sourceCode.getCommentsInside()

This method returns comments for a specific node in the form of an object containing arrays of "leading" (occurring before the given node) and "trailing" (occurring after the given node) comment tokens. This is useful for rules that need to check comments around a given node.
These methods return an array of comments that appear directly before, directly after, and inside nodes, respectively. They are useful for rules that need to check comments in relation to a given node or token.

Keep in mind that the results of this method are calculated on demand.

Expand Down
2 changes: 1 addition & 1 deletion lib/ast-utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ function hasJSDocThisTag(node, sourceCode) {
// because callbacks don't have its JSDoc comment.
// e.g.
// sinon.test(/* @this sinon.Sandbox */function() { this.spy(); });
return sourceCode.getComments(node).leading.some(comment => thisTagPattern.test(comment.value));
return sourceCode.getCommentsBefore(node).some(comment => thisTagPattern.test(comment.value));
}

/**
Expand Down
3 changes: 3 additions & 0 deletions lib/eslint.js
Original file line number Diff line number Diff line change
Expand Up @@ -1030,6 +1030,9 @@ module.exports = (function() {
getAllComments: "getAllComments",
getNodeByRangeIndex: "getNodeByRangeIndex",
getComments: "getComments",
getCommentsBefore: "getCommentsBefore",
getCommentsAfter: "getCommentsAfter",
getCommentsInside: "getCommentsInside",
getJSDocComment: "getJSDocComment",
getFirstToken: "getFirstToken",
getFirstTokens: "getFirstTokens",
Expand Down
2 changes: 1 addition & 1 deletion lib/rules/curly.js
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,7 @@ module.exports = {
}
} else if (multiOrNest) {
if (hasBlock && body.body.length === 1 && isOneLiner(body.body[0])) {
const leadingComments = sourceCode.getComments(body.body[0]).leading;
const leadingComments = sourceCode.getCommentsBefore(body.body[0]);

expected = leadingComments.length > 0;
} else if (!isOneLiner(body)) {
Expand Down
2 changes: 1 addition & 1 deletion lib/rules/default-case.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ module.exports = {
let comment;

const lastCase = last(node.cases);
const comments = sourceCode.getComments(lastCase).trailing;
const comments = sourceCode.getCommentsAfter(lastCase);

if (comments.length) {
comment = last(comments);
Expand Down
2 changes: 1 addition & 1 deletion lib/rules/indent.js
Original file line number Diff line number Diff line change
Expand Up @@ -1037,7 +1037,7 @@ module.exports = {
* This is necessary because sourceCode.getTokenBefore does not handle a comment as an argument correctly.
*/
const precedingTokens = sourceCode.ast.comments.reduce((commentMap, comment) => {
const tokenOrCommentBefore = sourceCode.getTokenOrCommentBefore(comment);
const tokenOrCommentBefore = sourceCode.getTokenBefore(comment, { includeComments: true });

return commentMap.set(comment, commentMap.has(tokenOrCommentBefore) ? commentMap.get(tokenOrCommentBefore) : tokenOrCommentBefore);
}, new WeakMap());
Expand Down
2 changes: 1 addition & 1 deletion lib/rules/key-spacing.js
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,7 @@ module.exports = {
// Check that the first comment is adjacent to the end of the group, the
// last comment is adjacent to the candidate property, and that successive
// comments are adjacent to each other.
const leadingComments = sourceCode.getComments(candidate).leading;
const leadingComments = sourceCode.getCommentsBefore(candidate);

if (
leadingComments.length &&
Expand Down
2 changes: 1 addition & 1 deletion lib/rules/lines-around-directive.js
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ module.exports = {
}

const firstDirective = directives[0];
const leadingComments = sourceCode.getComments(firstDirective).leading;
const leadingComments = sourceCode.getCommentsBefore(firstDirective);

// Only check before the first directive if it is preceded by a comment or if it is at the top of
// the file and expectLineBefore is set to "never". This is to not force a newline at the top of
Expand Down
4 changes: 2 additions & 2 deletions lib/rules/newline-before-return.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ module.exports = {
* @private
*/
function calcCommentLines(node, lineNumTokenBefore) {
const comments = sourceCode.getComments(node).leading;
const comments = sourceCode.getCommentsBefore(node);
let numLinesComments = 0;

if (!comments.length) {
Expand Down Expand Up @@ -153,7 +153,7 @@ module.exports = {
* @private
*/
function canFix(node) {
const leadingComments = sourceCode.getComments(node).leading;
const leadingComments = sourceCode.getCommentsBefore(node);
const lastLeadingComment = leadingComments[leadingComments.length - 1];
const tokenBefore = sourceCode.getTokenBefore(node);

Expand Down
2 changes: 1 addition & 1 deletion lib/rules/no-empty.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ module.exports = {
}

// any other block is only allowed to be empty, if it contains a comment
if (sourceCode.getComments(node).trailing.length > 0) {
if (sourceCode.getCommentsInside(node).length > 0) {
return;
}

Expand Down
2 changes: 1 addition & 1 deletion lib/rules/no-fallthrough.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ const DEFAULT_FALLTHROUGH_COMMENT = /falls?\s?through/i;
*/
function hasFallthroughComment(node, context, fallthroughCommentPattern) {
const sourceCode = context.getSourceCode();
const comment = lodash.last(sourceCode.getComments(node).leading);
const comment = lodash.last(sourceCode.getCommentsBefore(node));

return Boolean(comment && fallthroughCommentPattern.test(comment.value));
}
Expand Down
2 changes: 1 addition & 1 deletion lib/rules/sort-imports.js
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ module.exports = {
message: "Member '{{memberName}}' of the import declaration should be sorted alphabetically.",
data: { memberName: importSpecifiers[firstUnsortedIndex].local.name },
fix(fixer) {
if (importSpecifiers.some(specifier => sourceCode.getComments(specifier).leading.length || sourceCode.getComments(specifier).trailing.length)) {
if (importSpecifiers.some(specifier => sourceCode.getCommentsBefore(specifier).length || sourceCode.getCommentsAfter(specifier).length)) {

// If there are comments in the ImportSpecifier list, don't rearrange the specifiers.
return null;
Expand Down
2 changes: 1 addition & 1 deletion lib/rules/template-tag-spacing.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ module.exports = {
loc: tagToken.loc.start,
message: "Unexpected space between template tag and template literal.",
fix(fixer) {
const comments = sourceCode.getComments(node.quasi).leading;
const comments = sourceCode.getCommentsBefore(node.quasi);

// Don't fix anything if there's a single line comment after the template tag
if (comments.some(comment => comment.type === "Line")) {
Expand Down
69 changes: 69 additions & 0 deletions lib/token-store/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ const assert = require("assert");
const cursors = require("./cursors");
const ForwardTokenCursor = require("./forward-token-cursor");
const PaddedTokenCursor = require("./padded-token-cursor");
const astUtils = require("../ast-utils");

//------------------------------------------------------------------------------
// Helpers
Expand Down Expand Up @@ -172,6 +173,24 @@ function createCursorWithPadding(tokens, comments, indexMap, startLoc, endLoc, b
return createCursorWithCount(cursors.forward, tokens, comments, indexMap, startLoc, endLoc, beforeCount);
}

/**
* Gets comment tokens that are adjacent to the current cursor position.
* @param {Cursor} cursor - A cursor instance.
* @returns {Array} An array of comment tokens adjacent to the current cursor position.
* @private
*/
function getAdjacentCommentTokensFromCursor(cursor) {
const tokens = [];
let currentToken = cursor.getOneToken();

while (currentToken && astUtils.isCommentToken(currentToken)) {
tokens.push(currentToken);
currentToken = cursor.getOneToken();
}

return tokens;
}

//------------------------------------------------------------------------------
// Exports
//------------------------------------------------------------------------------
Expand Down Expand Up @@ -540,4 +559,54 @@ module.exports = class TokenStore {
padding
).getAllTokens();
}

/**
* Gets all comment tokens directly before the given node or token.
* @param {ASTNode|token} nodeOrToken The AST node or token to check for adjacent comment tokens.
* @returns {Array} An array of comments in occurrence order.
*/
getCommentsBefore(nodeOrToken) {
const cursor = createCursorWithCount(
cursors.backward,
this[TOKENS],
this[COMMENTS],
this[INDEX_MAP],
-1,
nodeOrToken.range[0],
{ includeComments: true }
);

return getAdjacentCommentTokensFromCursor(cursor).reverse();
}

/**
* Gets all comment tokens directly after the given node or token.
* @param {ASTNode|token} nodeOrToken The AST node or token to check for adjacent comment tokens.
* @returns {Array} An array of comments in occurrence order.
*/
getCommentsAfter(nodeOrToken) {
const cursor = createCursorWithCount(
cursors.forward,
this[TOKENS],
this[COMMENTS],
this[INDEX_MAP],
nodeOrToken.range[1],
-1,
{ includeComments: true }
);

return getAdjacentCommentTokensFromCursor(cursor);
}

/**
* Gets all comment tokens inside the given node.
* @param {ASTNode} node The AST node to get the comments for.
* @returns {Array} An array of comments in occurrence order.
*/
getCommentsInside(node) {
return this.getTokens(node, {
includeComments: true,
filter: astUtils.isCommentToken
});
}
};
26 changes: 14 additions & 12 deletions lib/util/source-code.js
Original file line number Diff line number Diff line change
Expand Up @@ -173,8 +173,8 @@ class SourceCode extends TokenStore {
}
this.lines.push(this.text.slice(this.lineStartIndices[this.lineStartIndices.length - 1]));

// Store for comments found using getComments().
this._commentStore = new WeakMap();
// Cache for comments found using getComments().
this._commentCache = new WeakMap();

// don't allow modification of this object
Object.freeze(this);
Expand Down Expand Up @@ -229,8 +229,8 @@ class SourceCode extends TokenStore {
* @public
*/
getComments(node) {
if (this._commentStore.has(node)) {
return this._commentStore.get(node);
if (this._commentCache.has(node)) {
return this._commentCache.get(node);
}

const comments = {
Expand Down Expand Up @@ -273,10 +273,12 @@ class SourceCode extends TokenStore {
if (node.parent && (currentToken.start < node.parent.start)) {
break;
}
comments.leading.unshift(currentToken);
comments.leading.push(currentToken);
currentToken = this.getTokenBefore(currentToken, { includeComments: true });
}

comments.leading.reverse();

currentToken = this.getTokenAfter(node, { includeComments: true });

while (currentToken && astUtils.isCommentToken(currentToken)) {
Expand All @@ -288,7 +290,7 @@ class SourceCode extends TokenStore {
}
}

this._commentStore.set(node, comments);
this._commentCache.set(node, comments);
return comments;
}

Expand All @@ -301,23 +303,23 @@ class SourceCode extends TokenStore {
*/
getJSDocComment(node) {
let parent = node.parent;
const leadingComments = this.getComments(node).leading;
const leadingComments = this.getCommentsBefore(node);

switch (node.type) {
case "ClassDeclaration":
case "FunctionDeclaration":
if (looksLikeExport(parent)) {
return findJSDocComment(this.getComments(parent).leading, parent.loc.start.line);
return findJSDocComment(this.getCommentsBefore(parent), parent.loc.start.line);
}
return findJSDocComment(leadingComments, node.loc.start.line);

case "ClassExpression":
return findJSDocComment(this.getComments(parent.parent).leading, parent.parent.loc.start.line);
return findJSDocComment(this.getCommentsBefore(parent.parent), parent.parent.loc.start.line);

case "ArrowFunctionExpression":
case "FunctionExpression":
if (parent.type !== "CallExpression" && parent.type !== "NewExpression") {
let parentLeadingComments = this.getComments(parent).leading;
let parentLeadingComments = this.getCommentsBefore(parent);

while (!parentLeadingComments.length && !/Function/.test(parent.type) && parent.type !== "MethodDefinition" && parent.type !== "Property") {
parent = parent.parent;
Expand All @@ -326,10 +328,10 @@ class SourceCode extends TokenStore {
break;
}

parentLeadingComments = this.getComments(parent).leading;
parentLeadingComments = this.getCommentsBefore(parent);
}

return parent && (parent.type !== "FunctionDeclaration") ? findJSDocComment(parentLeadingComments, parent.loc.start.line) : null;
return parent && parent.type !== "FunctionDeclaration" && parent.type !== "Program" ? findJSDocComment(parentLeadingComments, parent.loc.start.line) : null;
} else if (leadingComments.length) {
return findJSDocComment(leadingComments, node.loc.start.line);
}
Expand Down
16 changes: 4 additions & 12 deletions tests/lib/eslint.js
Original file line number Diff line number Diff line change
Expand Up @@ -2620,19 +2620,11 @@ describe("eslint", () => {

eslint.reset();

eslint.on("Program", node => {
assert.equal(node.comments.length, 1);
assert.equal(node.comments[0].type, "Shebang");

let comments = eslint.getComments(node);

assert.equal(comments.leading.length, 0);
assert.equal(comments.trailing.length, 0);
eslint.on("Program", () => {
const comments = eslint.getAllComments();

comments = eslint.getComments(node.body[0]);
assert.equal(comments.leading.length, 1);
assert.equal(comments.trailing.length, 0);
assert.equal(comments.leading[0].type, "Shebang");
assert.equal(comments.length, 1);
assert.equal(comments[0].type, "Shebang");
});
eslint.verify(code, config, "foo.js", true);
});
Expand Down
Loading

0 comments on commit 735d02d

Please sign in to comment.