Skip to content

Commit

Permalink
checkTypes: split rule logic and add checkin for other typed tags
Browse files Browse the repository at this point in the history
  • Loading branch information
Alexej Yaroshevich committed Nov 9, 2014
1 parent d846a5b commit f74570c
Show file tree
Hide file tree
Showing 8 changed files with 206 additions and 49 deletions.
29 changes: 20 additions & 9 deletions lib/jsdoc.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@ var commentParser = require('comment-parser');
var TypeParser = require('jsdoctypeparser').Parser;
var TypeBuilder = require('jsdoctypeparser').Builder;

// wtf but it needed to stop writing warnings to stdout
// and revert exceptions functionality
TypeBuilder.ENABLE_EXCEPTIONS = true;

module.exports = {
/**
* @param {string} comment
Expand All @@ -22,14 +26,21 @@ module.exports = {
* @constructor
*/
function DocComment(commentNode) {
// var comment = Array(jsdoc.loc.start.column + 1).join(' ') + '/*' + jsdoc.value + '*/';
// normalize data
var loc = commentNode.loc;
var lines = [Array(loc.start.column + 1).join(' '), '/*', commentNode.value, '*/']
.join('').split('\n').map(function(v) {
return v.substr(loc.start.column);
});
var value = lines.join('\n');

// parse comments
var _parsed = _parseComment(commentNode.value) || {};
var _parsed = _parseComment(value) || {};

// fill up fields
this.loc = commentNode.loc;
this.value = commentNode.value;
this.lines = commentNode.lines || commentNode.value.split('\n');
this.loc = loc;
this.value = value;
this.lines = lines;
this.valid = _parsed.hasOwnProperty('line');

// doc parsed data
Expand Down Expand Up @@ -96,11 +107,11 @@ function DocType(type) {
this.value = type;

var parsed = _parseDocType(type);
var data = parsed.valid ? _simplifyType(parsed) : [];

this.optional = parsed.optional;
this.valid = parsed.valid;

var data = _simplifyType(parsed);

/**
* match type
* @param {EsprimaNode} node
Expand Down Expand Up @@ -136,8 +147,8 @@ function _parseDocType(typeString) {
node = parser.parse(typeString);
node.valid = true;
} catch (e) {
console.error(e.stack);
node = [];
node = {};
node.error = e.message;
node.valid = false;
}
return node;
Expand Down
33 changes: 13 additions & 20 deletions lib/rules/validate-jsdoc.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,17 @@ module.exports.prototype = {
]
};

// classic checker
if (_this._rulesForNodes.file) {
// call file checkers
var validators = _this._rulesForNodes.file;
if (validators) {
validators.forEach(function(v) {
v.call(_this, file, errors);
});
}
}

// iterate over scopes
Object.keys(scopes).forEach(function(scope) {

Expand Down Expand Up @@ -174,30 +185,12 @@ function patchNodesInFile(file) {

function getJsdoc() {
if (!this.hasOwnProperty('_jsdoc')) {
this._jsdoc = findNodeComment(this);
var res = findDocCommentBeforeLine(this.loc.start.line);
this._jsdoc = res ? jsdoc.createDocComment(res) : null;
}
return this._jsdoc;
}

function findNodeComment(node) {
var commentNode = findDocCommentBeforeLine(node.loc.start.line);
if (!commentNode) {
return null;
}

// normalize data
var loc = commentNode.loc;
var lines = [Array(loc.start.column + 1).join(' '), '/*', commentNode.value, '*/']
.join('').split('\n').map(function(v) {
return v.substr(loc.start.column);
});
var value = lines.join('\n');
commentNode.lines = lines;
commentNode.value = value;

return jsdoc.createDocComment(commentNode);
}

function findDocCommentBeforeLine(line) {
line--; // todo: buggy behaviour, can't jump back over empty lines
for (var i = 0, l = fileComments.length; i < l; i++) {
Expand Down
65 changes: 65 additions & 0 deletions lib/rules/validate-jsdoc/check-types.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
var jsdoc = require('../../jsdoc');

module.exports = validateTypesInTags;
module.exports.scopes = ['file'];
module.exports.options = {
checkTypes: {allowedValues: [true]}
};

var allowedTags = {
// common
typedef: true,
type: true,
param: true,
'return': true,
returns: true,
'enum': true,

// jsdoc
'var': true,
prop: true,
property: true,
arg: true,
argument: true,

// jsduck
cfg: true,

// closure
lends: true,
extends: true,
implements: true,
define: true
};

/**
* validator for types in tags
* @param {JSCS.JSFile} file
* @param {JSCS.Errors} errors
*/
function validateTypesInTags(file, errors) {
var comments = file.getComments();
comments.forEach(function(commentNode) {
if (commentNode.type !== 'Block' || commentNode.value[0] !== '*') {
return;
}

// trying to create DocComment object
var node = jsdoc.createDocComment(commentNode);
if (!node.valid) {
return;
}

node.iterateByType(Object.keys(allowedTags), function(tag) {
if (!tag.type) {
// skip untyped params
return;
}
if (!tag.type.valid) {
// throw an error if not valid
errors.add('jsDoc checkTypes: Expected valid type instead of ' + tag.value,
node.loc.start.line + tag.line, node.loc.start.column + tag.id.length + 1);
}
});
});
}
1 change: 1 addition & 0 deletions lib/rules/validate-jsdoc/index.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
var assert = require('assert');

var validatorsByName = module.exports = {
checkTypes: require('./check-types'),
param: require('./param'),
returns: require('./returns'),
checkRedundantAccess: require('./check-redundant-access'),
Expand Down
7 changes: 1 addition & 6 deletions lib/rules/validate-jsdoc/param.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,7 @@ module.exports.scopes = ['function'];
module.exports.options = {
checkParamNames: {allowedValues: [true]},
requireParamTypes: {allowedValues: [true]},
checkRedundantParams: {allowedValues: [true]},
checkTypes: {allowedValues: [true]}
checkRedundantParams: {allowedValues: [true]}
};

/**
Expand All @@ -31,10 +30,6 @@ function validateParamTag(node, tag, err) {
return err('Missing JsDoc @param type');
}

if (options.checkTypes && !tag.type.valid) {
return err('Invalid JsDoc type definition');
}

// skip if there is dot in param name (object's inner param)
if (tag.name.indexOf('.') !== -1) {
return;
Expand Down
7 changes: 0 additions & 7 deletions lib/rules/validate-jsdoc/returns.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,6 @@ module.exports.options = {
},
checkRedundantReturns: {
allowedValues: [true]
},
checkTypes: {
allowedValues: [true]
}
};

Expand All @@ -38,10 +35,6 @@ function validateReturnsTag(node, tag, err) {
err('Missing type in @returns statement');
}

if (options.checkTypes && !tag.type.valid) {
return err('Invalid type definition in @returns statement');
}

if (!options.checkRedundantReturns && !options.checkReturnTypes) {
return;
}
Expand Down
2 changes: 1 addition & 1 deletion test/lib/rules/validate-jsdoc/check-return-types.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ describe('rules/validate-jsdoc', function () {
checker.cases([
/* jshint ignore:start */
{
it: 'shouldn\'t throw',
it: 'should throw invalid type',
errors: 1,
code: function () {
/**
Expand Down
111 changes: 105 additions & 6 deletions test/lib/rules/validate-jsdoc/check-types.js
Original file line number Diff line number Diff line change
@@ -1,22 +1,121 @@
describe('rules/validate-jsdoc', function () {
describe('rules/validate-jsdoc', function() {
var checker = global.checker({
additionalRules: ['lib/rules/validate-jsdoc.js']
});

describe('check-types', function () {
describe('check-types', function() {

checker.rules({checkTypes: true});
checker.cases([
/* jshint ignore:start */
{
it: 'should neither throw nor report',
skip: 1,
code: function () {
it: 'should report invalid typedef',
errors: 1,
code: function() {
/**
* @typedef {something/invalid} name
*/
}
}, {
it: 'should not report empty valid typedef',
code: function() {
/**
* @typedef alphanum
*/
}
}, {
it: 'should not report valid typedef',
code: function() {
/**
* @typedef {(string|number)} alphanum
*/
}

}, {
it: 'should report invalid property',
errors: 1,
code: function() {
/**
* @typedef alphanum
* @property {something/invalid} invalid
*/
}
}, {
it: 'should not report valid properties',
code: function() {
/**
* @typedef {object} user
* @property {number} age
* @property {string} name
*/
}
}, {
it: 'should report member',
code: function() {
function ClsName () {
/** @member {invalid/type} */
this.point = {};
}
}
}, {
it: 'should not report member',
code: function() {
function ClsName () {
/** @member {Object} */
this.point = {};
}
}

}, {
it: 'should not report member',
code: function() {
function ClsName () {
/** @member {Object} */
this.point = {};
}
}

}, {
it: 'should not report return',
code: function() {
/**
* @return {{a: number, b: string}}
*/
function foo() {
return {};
}
}
}, {
it: 'should not report empty types in params and returns',
code: function() {
/**
* @param q
* @param w
* @return
*/
ClsName.prototype.point = function (q, w) {
}
}
}, {
it: 'should not report valid types in params and returns',
code: function() {
/**
* @param {Object} q
* @param {Number} w
* @return {String}
*/
ClsName.prototype.point = function (q, w) {
}
}
}, {
it: 'should report invalid types in params and returns',
errors: 3,
code: function() {
/**
* @param {Obj+ect} q
* @param {Num/ber} w
* @return {Str~ing}
*/
ClsName.prototype.point = function (q, w) {
}
}
}
Expand Down

0 comments on commit f74570c

Please sign in to comment.