Skip to content

Commit

Permalink
Update: use doctrine range information in valid-jsdoc (#9831)
Browse files Browse the repository at this point in the history
  • Loading branch information
@not-an-aardvark
not-an-aardvark committed Jan 14, 2018
1 parent 133336e commit 1d61930
Show file tree
Hide file tree
Showing 3 changed files with 401 additions and 86 deletions.
117 changes: 89 additions & 28 deletions lib/rules/valid-jsdoc.js
View file
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
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

0 comments on commit 1d61930

Please sign in to comment.