Skip to content

Commit

Permalink
Merge pull request #5747 from eslint/issue-5745
Browse files Browse the repository at this point in the history 
Fix: valid-jsdoc crash w/ Field & Array Type
  • Loading branch information
@gyandeeps
gyandeeps committed Mar 31, 2016
2 parents ed307d0 + 12902c5 commit 2bb8486
Show file tree
Hide file tree
Showing 2 changed files with 177 additions and 29 deletions.
61 changes: 32 additions & 29 deletions lib/rules/valid-jsdoc.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,13 +107,13 @@ module.exports = function(context) {
var currentType;
var expectedType;

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

expectedType = preferType[currentType];
expectedType = currentType && preferType[currentType];

return {
currentType: currentType,
Expand All @@ -123,46 +123,49 @@ module.exports = function(context) {

/**
* Check if return tag type is void or undefined
* @param {Object} tag JSDoc tag
* @param {Object} jsdocNode JSDoc node
* @param {Object} type JSDoc tag
* @returns {void}
* @private
*/
function validateTagType(tag, jsdocNode) {
if (!tag.type || !canTypeBeValidated(tag.type.type)) {
function validateType(jsdocNode, type) {
if (!type || !canTypeBeValidated(type.type)) {
return;
}

var typesToCheck = [];
var elements = [];

if (tag.type.type === "TypeApplication") { // {Array.<String>}
elements = tag.type.applications[0].type === "UnionType" ? tag.type.applications[0].elements : tag.type.applications;
typesToCheck.push(getCurrentExpectedTypes(tag.type));
} else if (tag.type.type === "RecordType") { // {{20:String}}
elements = tag.type.fields;
} else if (tag.type.type === "UnionType") { // {String|number|Test}
elements = tag.type.elements;
} else {
typesToCheck.push(getCurrentExpectedTypes(tag.type));
switch (type.type) {
case "TypeApplication": // {Array.<String>}
elements = type.applications[0].type === "UnionType" ? type.applications[0].elements : type.applications;
typesToCheck.push(getCurrentExpectedTypes(type));
break;
case "RecordType": // {{20:String}}
elements = type.fields;
break;
case "UnionType": // {String|number|Test}
case "ArrayType": // {[String, number, Test]}
elements = type.elements;
break;
case "FieldType": // Array.<{count: number, votes: number}>
typesToCheck.push(getCurrentExpectedTypes(type.value));
break;
default:
typesToCheck.push(getCurrentExpectedTypes(type));
}

elements.forEach(function(type) {
type = type.value ? type.value : type; // we have to use type.value for RecordType
if (canTypeBeValidated(type.type)) {
typesToCheck.push(getCurrentExpectedTypes(type));
}
});
elements.forEach(validateType.bind(null, jsdocNode));

typesToCheck.forEach(function(type) {
if (type.expectedType &&
type.expectedType !== type.currentType) {
typesToCheck.forEach(function(typeToCheck) {
if (typeToCheck.expectedType &&
typeToCheck.expectedType !== typeToCheck.currentType) {
context.report({
node: jsdocNode,
message: "Use '{{expectedType}}' instead of '{{currentType}}'.",
data: {
currentType: type.currentType,
expectedType: type.expectedType
currentType: typeToCheck.currentType,
expectedType: typeToCheck.expectedType
}
});
}
Expand Down Expand Up @@ -268,8 +271,8 @@ module.exports = function(context) {
}

// validate the types
if (checkPreferType) {
validateTagType(tag, jsdocNode);
if (checkPreferType && tag.type) {
validateType(jsdocNode, tag.type);
}
});

Expand Down
145 changes: 145 additions & 0 deletions tests/lib/rules/valid-jsdoc.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -415,6 +415,51 @@ ruleTester.run("valid-jsdoc", rule, {
}
}]
},
{
code:
"/**\n" +
"* Foo\n" +
"* @param {Array.<{id: number, votes: number}>} hi - desc\n" +
"* @returns {Array.<{summary: string}>} desc\n" +
"*/\n" +
"function foo(hi){}",
options: [{
preferType: {
"Number": "number",
"String": "string"
}
}]
},
{
code:
"/**\n" +
"* Foo\n" +
"* @param {Array.<[string, number]>} hi - desc\n" +
"* @returns {Array.<[string, string]>} desc\n" +
"*/\n" +
"function foo(hi){}",
options: [{
preferType: {
"Number": "number",
"String": "string"
}
}]
},
{
code:
"/**\n" +
"* Foo\n" +
"* @param {Object<string,Object<string, number>>} hi - because why not\n" +
"* @returns {Boolean} desc\n" +
"*/\n" +
"function foo(hi){}",
options: [{
preferType: {
"Number": "number",
"String": "string"
}
}]
},
{
code: "/**\n* Description\n* @param {string} a bar\n* @returns {string} desc */\nfunction foo(a = 1){}",
parserOptions: {
Expand Down Expand Up @@ -1014,6 +1059,106 @@ ruleTester.run("valid-jsdoc", rule, {
type: "Block"
}
]
},
{
code:
"/**\n" +
"* Foo\n" +
"* @param {Array.<{id: Number, votes: Number}>} hi - desc\n" +
"* @returns {Array.<{summary: String}>} desc\n" +
"*/\n" +
"function foo(hi){}",
options: [{
preferType: {
"Number": "number",
"String": "string"
}
}],
errors: [
{
message: "Use 'number' instead of 'Number'.",
type: "Block"
},
{
message: "Use 'number' instead of 'Number'.",
type: "Block"
},
{
message: "Use 'string' instead of 'String'.",
type: "Block"
}
]
},
{
code:
"/**\n" +
"* Foo\n" +
"* @param {Array.<[String, Number]>} hi - desc\n" +
"* @returns {Array.<[String, String]>} desc\n" +
"*/\n" +
"function foo(hi){}",
options: [{
preferType: {
"Number": "number",
"String": "string"
}
}],
errors: [
{
message: "Use 'string' instead of 'String'.",
type: "Block"
},
{
message: "Use 'number' instead of 'Number'.",
type: "Block"
},
{
message: "Use 'string' instead of 'String'.",
type: "Block"
},
{
message: "Use 'string' instead of 'String'.",
type: "Block"
}
]
},
{
code:
"/**\n" +
"* Foo\n" +
"* @param {object<String,object<String, Number>>} hi - because why not\n" +
"* @returns {Boolean} desc\n" +
"*/\n" +
"function foo(hi){}",
options: [{
preferType: {
"Number": "number",
"String": "string",
"object": "Object"
}
}],
errors: [
{
message: "Use 'string' instead of 'String'.",
type: "Block"
},
{
message: "Use 'string' instead of 'String'.",
type: "Block"
},
{
message: "Use 'number' instead of 'Number'.",
type: "Block"
},
{
message: "Use 'Object' instead of 'object'.",
type: "Block"
},
{
message: "Use 'Object' instead of 'object'.",
type: "Block"
}
]
}
]
});

0 comments on commit 2bb8486

Please sign in to comment.