Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Update: use doctrine range information in valid-jsdoc #9831

Merged
merged 2 commits into from
Jan 14, 2018
Merged

Update: use doctrine range information in valid-jsdoc #9831

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2dba01f
Update: use doctrine range information in valid-jsdoc
not-an-aardvark Jan 8, 2018
06fd4de
Also autofix `"prefer"` corrections
not-an-aardvark Jan 8, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
117 changes: 89 additions & 28 deletions lib/rules/valid-jsdoc.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,9 @@ module.exports = {
},
additionalProperties: false
}
]
],

fixable: "code"
},

create(context) {
Expand Down Expand Up @@ -145,23 +147,35 @@ module.exports = {
/**
* Extract the current and expected type based on the input type object
* @param {Object} type JSDoc tag
* @returns {Object} current and expected type object
* @returns {{currentType: Doctrine.Type, expectedTypeName: string}} The current type annotation and
* the expected name of the annotation
* @private
*/
function getCurrentExpectedTypes(type) {
let currentType;

if (type.name) {
currentType = type.name;
currentType = type;
} else if (type.expression) {
currentType = type.expression.name;
currentType = type.expression;
}

const expectedType = currentType && preferType[currentType];

return {
currentType,
expectedType
expectedTypeName: currentType && preferType[currentType.name]
};
}

/**
* Gets the location of a JSDoc node in a file
* @param {Token} jsdocComment The comment that this node is parsed from
* @param {{range: number[]}} parsedJsdocNode A tag or other node which was parsed from this comment
* @returns {{start: SourceLocation, end: SourceLocation}} The 0-based source location for the tag
*/
function getAbsoluteRange(jsdocComment, parsedJsdocNode) {
return {
start: sourceCode.getLocFromIndex(jsdocComment.range[0] + 2 + parsedJsdocNode.range[0]),
end: sourceCode.getLocFromIndex(jsdocComment.range[0] + 2 + parsedJsdocNode.range[1])
};
}

Expand Down Expand Up @@ -204,14 +218,21 @@ module.exports = {
elements.forEach(validateType.bind(null, jsdocNode));

typesToCheck.forEach(typeToCheck => {
if (typeToCheck.expectedType &&
typeToCheck.expectedType !== typeToCheck.currentType) {
if (typeToCheck.expectedTypeName &&
typeToCheck.expectedTypeName !== typeToCheck.currentType.name) {
context.report({
node: jsdocNode,
message: "Use '{{expectedType}}' instead of '{{currentType}}'.",
message: "Use '{{expectedTypeName}}' instead of '{{currentTypeName}}'.",
loc: getAbsoluteRange(jsdocNode, typeToCheck.currentType),
data: {
currentType: typeToCheck.currentType,
expectedType: typeToCheck.expectedType
currentTypeName: typeToCheck.currentType.name,
expectedTypeName: typeToCheck.expectedTypeName
},
fix(fixer) {
return fixer.replaceTextRange(
typeToCheck.currentType.range.map(indexInComment => jsdocNode.range[0] + 2 + indexInComment),
typeToCheck.expectedTypeName
);
}
});
}
Expand All @@ -227,8 +248,8 @@ module.exports = {
function checkJSDoc(node) {
const jsdocNode = sourceCode.getJSDocComment(node),
functionData = fns.pop(),
params = Object.create(null),
paramsTags = [];
paramTagsByName = Object.create(null),
paramTags = [];
let hasReturns = false,
returnsTag,
hasConstructor = false,
Expand All @@ -244,7 +265,8 @@ module.exports = {
jsdoc = doctrine.parse(jsdocNode.value, {
strict: true,
unwrap: true,
sloppy: true
sloppy: true,
range: true
});
} catch (ex) {

Expand All @@ -264,7 +286,7 @@ module.exports = {
case "param":
case "arg":
case "argument":
paramsTags.push(tag);
paramTags.push(tag);
break;

case "return":
Expand Down Expand Up @@ -297,7 +319,29 @@ module.exports = {

// check tag preferences
if (prefer.hasOwnProperty(tag.title) && tag.title !== prefer[tag.title]) {
context.report({ node: jsdocNode, message: "Use @{{name}} instead.", data: { name: prefer[tag.title] } });
const entireTagRange = getAbsoluteRange(jsdocNode, tag);

context.report({
node: jsdocNode,
message: "Use @{{name}} instead.",
loc: {
start: entireTagRange.start,
end: {
line: entireTagRange.start.line,
column: entireTagRange.start.column + `@${tag.title}`.length
}
},
data: { name: prefer[tag.title] },
fix(fixer) {
return fixer.replaceTextRange(
[
jsdocNode.range[0] + tag.range[0] + 3,
jsdocNode.range[0] + tag.range[0] + tag.title.length + 3
],
prefer[tag.title]
);
}
});
}

// validate the types
Expand All @@ -306,17 +350,32 @@ module.exports = {
}
});

paramsTags.forEach(param => {
paramTags.forEach(param => {
if (!param.type) {
context.report({ node: jsdocNode, message: "Missing JSDoc parameter type for '{{name}}'.", data: { name: param.name } });
context.report({
node: jsdocNode,
message: "Missing JSDoc parameter type for '{{name}}'.",
loc: getAbsoluteRange(jsdocNode, param),
data: { name: param.name }
});
}
if (!param.description && requireParamDescription) {
context.report({ node: jsdocNode, message: "Missing JSDoc parameter description for '{{name}}'.", data: { name: param.name } });
context.report({
node: jsdocNode,
message: "Missing JSDoc parameter description for '{{name}}'.",
loc: getAbsoluteRange(jsdocNode, param),
data: { name: param.name }
});
}
if (params[param.name]) {
context.report({ node: jsdocNode, message: "Duplicate JSDoc parameter '{{name}}'.", data: { name: param.name } });
if (paramTagsByName[param.name]) {
context.report({
node: jsdocNode,
message: "Duplicate JSDoc parameter '{{name}}'.",
loc: getAbsoluteRange(jsdocNode, param),
data: { name: param.name }
});
} else if (param.name.indexOf(".") === -1) {
params[param.name] = 1;
paramTagsByName[param.name] = param;
}
});

Expand All @@ -325,6 +384,7 @@ module.exports = {
context.report({
node: jsdocNode,
message: "Unexpected @{{title}} tag; function has no return statement.",
loc: getAbsoluteRange(jsdocNode, returnsTag),
data: {
title: returnsTag.title
}
Expand Down Expand Up @@ -356,10 +416,10 @@ module.exports = {
}

// check the parameters
const jsdocParams = Object.keys(params);
const jsdocParamNames = Object.keys(paramTagsByName);

if (node.params) {
node.params.forEach((param, i) => {
node.params.forEach((param, paramsIndex) => {
if (param.type === "AssignmentPattern") {
param = param.left;
}
Expand All @@ -368,16 +428,17 @@ module.exports = {

// TODO(nzakas): Figure out logical things to do with destructured, default, rest params
if (param.type === "Identifier") {
if (jsdocParams[i] && (name !== jsdocParams[i])) {
if (jsdocParamNames[paramsIndex] && (name !== jsdocParamNames[paramsIndex])) {
context.report({
node: jsdocNode,
message: "Expected JSDoc for '{{name}}' but found '{{jsdocName}}'.",
loc: getAbsoluteRange(jsdocNode, paramTagsByName[jsdocParamNames[paramsIndex]]),
data: {
name,
jsdocName: jsdocParams[i]
jsdocName: jsdocParamNames[paramsIndex]
}
});
} else if (!params[name] && !isOverride) {
} else if (!paramTagsByName[name] && !isOverride) {
context.report({
node: jsdocNode,
message: "Missing JSDoc for parameter '{{name}}'.",
Expand Down
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
"concat-stream": "^1.6.0",
"cross-spawn": "^5.1.0",
"debug": "^3.1.0",
"doctrine": "^2.0.2",
"doctrine": "^2.1.0",
"eslint-scope": "^3.7.1",
"eslint-visitor-keys": "^1.0.0",
"espree": "^3.5.2",
Expand Down