Skip to content

Commit

Permalink
Merge pull request #4276 from eslint/issue4268
Browse files Browse the repository at this point in the history 
Update: Add support for class in `valid-jsdoc` rule (fixes #4279)
  • Loading branch information
@nzakas
nzakas committed Oct 28, 2015
2 parents 1c7518a + f460ea2 commit 1e90444
Show file tree
Hide file tree
Showing 4 changed files with 267 additions and 18 deletions.
53 changes: 35 additions & 18 deletions lib/rules/valid-jsdoc.js
View file
Expand Up @@ -33,6 +33,16 @@ module.exports = function(context) {
// Using a stack to store if a function returns or not (handling nested functions)
var fns = [];

/**
* Check if node type is a Class
* @param {ASTNode} node node to check.
* @returns {boolean} True is its a class
* @private
*/
function isTypeClass(node) {
return node.type === "ClassExpression" || node.type === "ClassDeclaration";
}

/**
* When parsing a new function, store it in our function stack.
* @param {ASTNode} node A function node to check.
Expand All @@ -41,7 +51,8 @@ module.exports = function(context) {
*/
function startFunction(node) {
fns.push({
returnPresent: (node.type === "ArrowFunctionExpression" && node.body.type !== "BlockStatement")
returnPresent: (node.type === "ArrowFunctionExpression" && node.body.type !== "BlockStatement") ||
isTypeClass(node)
});
}

Expand Down Expand Up @@ -165,7 +176,7 @@ module.exports = function(context) {
});

// check for functions missing @returns
if (!isOverride && !hasReturns && !hasConstructor && node.parent.kind !== "get") {
if (!isOverride && !hasReturns && !hasConstructor && node.parent.kind !== "get" && !isTypeClass(node)) {
if (requireReturn || functionData.returnPresent) {
context.report(jsdocNode, "Missing JSDoc @" + (prefer.returns || "returns") + " for function.");
}
Expand All @@ -174,23 +185,25 @@ module.exports = function(context) {
// check the parameters
var jsdocParams = Object.keys(params);

node.params.forEach(function(param, i) {
var name = param.name;

// TODO(nzakas): Figure out logical things to do with destructured, default, rest params
if (param.type === "Identifier") {
if (jsdocParams[i] && (name !== jsdocParams[i])) {
context.report(jsdocNode, "Expected JSDoc for '{{name}}' but found '{{jsdocName}}'.", {
name: name,
jsdocName: jsdocParams[i]
});
} else if (!params[name] && !isOverride) {
context.report(jsdocNode, "Missing JSDoc for parameter '{{name}}'.", {
name: name
});
if (node.params) {
node.params.forEach(function(param, i) {
var name = param.name;

// TODO(nzakas): Figure out logical things to do with destructured, default, rest params
if (param.type === "Identifier") {
if (jsdocParams[i] && (name !== jsdocParams[i])) {
context.report(jsdocNode, "Expected JSDoc for '{{name}}' but found '{{jsdocName}}'.", {
name: name,
jsdocName: jsdocParams[i]
});
} else if (!params[name] && !isOverride) {
context.report(jsdocNode, "Missing JSDoc for parameter '{{name}}'.", {
name: name
});
}
}
}
});
});
}

if (options.matchDescription) {
var regex = new RegExp(options.matchDescription);
Expand All @@ -212,9 +225,13 @@ module.exports = function(context) {
"ArrowFunctionExpression": startFunction,
"FunctionExpression": startFunction,
"FunctionDeclaration": startFunction,
"ClassExpression": startFunction,
"ClassDeclaration": startFunction,
"ArrowFunctionExpression:exit": checkJSDoc,
"FunctionExpression:exit": checkJSDoc,
"FunctionDeclaration:exit": checkJSDoc,
"ClassExpression:exit": checkJSDoc,
"ClassDeclaration:exit": checkJSDoc,
"ReturnStatement": addReturn
};

Expand Down
6 changes: 6 additions & 0 deletions lib/util/source-code.js
View file
Expand Up @@ -211,6 +211,12 @@ SourceCode.prototype = {
}
break;

case "ClassDeclaration":
return findJSDocComment(node.leadingComments, line);

case "ClassExpression":
return findJSDocComment(parent.parent.leadingComments, line);

case "ArrowFunctionExpression":
case "FunctionExpression":

Expand Down
170 changes: 170 additions & 0 deletions tests/lib/rules/valid-jsdoc.js
View file
Expand Up @@ -121,6 +121,75 @@ ruleTester.run("valid-jsdoc", rule, {
{
code: "/** Foo \n@return {void} Foo\n */\nfunction foo(){}",
options: [{ prefer: { "return": "return" }}]
},

// classes
{
code:
"/**\n" +
" * Description for A.\n" +
" */\n" +
"class A {\n" +
" /**\n" +
" * Description for constructor.\n" +
" * @param {object[]} xs - xs\n" +
" */\n" +
" constructor(xs) {\n" +
" this.a = xs;" +
" }\n" +
"}",
options: [{requireReturn: false}],
ecmaFeatures: {
classes: true
}
},
{
code:
"/**\n" +
" * Description for A.\n" +
" */\n" +
"class A {\n" +
" /**\n" +
" * Description for constructor.\n" +
" * @param {object[]} xs - xs\n" +
" * @returns {void}\n" +
" */\n" +
" constructor(xs) {\n" +
" this.a = xs;" +
" }\n" +
"}",
options: [{requireReturn: true}],
ecmaFeatures: {
classes: true
}
},
{
code:
"/**\n" +
" * Description for A.\n" +
" */\n" +
"class A {\n" +
" /**\n" +
" * Description for constructor.\n" +
" * @param {object[]} xs - xs\n" +
" * @returns {void}\n" +
" */\n" +
" constructor(xs) {\n" +
" this.a = xs;" +
" }\n" +
" /**\n" +
" * Description for method.\n" +
" * @param {object[]} xs - xs\n" +
" * @returns {void}\n" +
" */\n" +
" print(xs) {\n" +
" this.a = xs;" +
" }\n" +
"}",
options: [],
ecmaFeatures: {
classes: true
}
}
],

Expand Down Expand Up @@ -359,6 +428,107 @@ ruleTester.run("valid-jsdoc", rule, {
message: "JSDoc description does not satisfy the regex pattern.",
type: "Block"
}]
},
// classes
{
code:
"/**\n" +
" * Description for A\n" +
" */\n" +
"class A {\n" +
" /**\n" +
" * Description for constructor\n" +
" * @param {object[]} xs - xs\n" +
" */\n" +
" constructor(xs) {\n" +
" this.a = xs;" +
" }\n" +
"}",
options: [{
requireReturn: false,
"matchDescription": "^[A-Z][A-Za-z0-9\\s]*[.]$"
}],
errors: [
{
message: "JSDoc description does not satisfy the regex pattern.",
type: "Block"
},
{
message: "JSDoc description does not satisfy the regex pattern.",
type: "Block"
}
],
ecmaFeatures: {
classes: true
}
},
{
code:
"/**\n" +
" * Description for a\n" +
" */\n" +
"var A = class {\n" +
" /**\n" +
" * Description for constructor.\n" +
" * @param {object[]} xs - xs\n" +
" */\n" +
" constructor(xs) {\n" +
" this.a = xs;" +
" }\n" +
"};",
options: [{
requireReturn: true,
"matchDescription": "^[A-Z][A-Za-z0-9\\s]*[.]$"
}],
errors: [
{
message: "JSDoc description does not satisfy the regex pattern.",
type: "Block"
},
{
message: "Missing JSDoc @returns for function.",
type: "Block"
}
],
ecmaFeatures: {
classes: true
}
},
{
code:
"/**\n" +
" * Description for A.\n" +
" */\n" +
"class A {\n" +
" /**\n" +
" * Description for constructor.\n" +
" * @param {object[]} xs - xs\n" +
" * @returns {void}\n" +
" */\n" +
" constructor(xs) {\n" +
" this.a = xs;" +
" }\n" +
" /**\n" +
" * Description for method.\n" +
" */\n" +
" print(xs) {\n" +
" this.a = xs;" +
" }\n" +
"}",
options: [],
errors: [
{
message: "Missing JSDoc @returns for function.",
type: "Block"
},
{
message: "Missing JSDoc for parameter 'xs'.",
type: "Block"
}
],
ecmaFeatures: {
classes: true
}
}
]
});
56 changes: 56 additions & 0 deletions tests/lib/util/source-code.js
View file
Expand Up @@ -585,6 +585,62 @@ describe("SourceCode", function() {
assert.isTrue(spy.calledTwice, "Event handler should be called.");
});

it("should get JSDoc comment for node when the node is a ClassExpression", function() {

var code = [
"/** Merges two objects together.*/",
"var A = class {",
"};"
].join("\n");

/**
* Check jsdoc presence
* @param {ASTNode} node not to check
* @returns {void}
* @private
*/
function assertJSDoc(node) {
var sourceCode = eslint.getSourceCode();
var jsdoc = sourceCode.getJSDocComment(node);
assert.equal(jsdoc.type, "Block");
assert.equal(jsdoc.value, "* Merges two objects together.");
}

var spy = sandbox.spy(assertJSDoc);

eslint.on("ClassExpression", spy);
eslint.verify(code, { rules: {}, ecmaFeatures: {classes: true}}, filename, true);
assert.isTrue(spy.calledOnce, "Event handler should be called.");
});

it("should get JSDoc comment for node when the node is a ClassDeclaration", function() {

var code = [
"/** Merges two objects together.*/",
"class A {",
"};"
].join("\n");

/**
* Check jsdoc presence
* @param {ASTNode} node not to check
* @returns {void}
* @private
*/
function assertJSDoc(node) {
var sourceCode = eslint.getSourceCode();
var jsdoc = sourceCode.getJSDocComment(node);
assert.equal(jsdoc.type, "Block");
assert.equal(jsdoc.value, "* Merges two objects together.");
}

var spy = sandbox.spy(assertJSDoc);

eslint.on("ClassDeclaration", spy);
eslint.verify(code, { rules: {}, ecmaFeatures: {classes: true}}, filename, true);
assert.isTrue(spy.calledOnce, "Event handler should be called.");
});

});

describe("getComments()", function() {
Expand Down

0 comments on commit 1e90444

Please sign in to comment.