Skip to content

Commit f74570c

Browse files
author
Alexej Yaroshevich
committed
checkTypes: split rule logic and add checkin for other typed tags
1 parent d846a5b commit f74570c

8 files changed

Lines changed: 206 additions & 49 deletions

File tree

lib/jsdoc.js

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,10 @@ var commentParser = require('comment-parser');
22
var TypeParser = require('jsdoctypeparser').Parser;
33
var TypeBuilder = require('jsdoctypeparser').Builder;
44

5+
// wtf but it needed to stop writing warnings to stdout
6+
// and revert exceptions functionality
7+
TypeBuilder.ENABLE_EXCEPTIONS = true;
8+
59
module.exports = {
610
/**
711
* @param {string} comment
@@ -22,14 +26,21 @@ module.exports = {
2226
* @constructor
2327
*/
2428
function DocComment(commentNode) {
25-
// var comment = Array(jsdoc.loc.start.column + 1).join(' ') + '/*' + jsdoc.value + '*/';
29+
// normalize data
30+
var loc = commentNode.loc;
31+
var lines = [Array(loc.start.column + 1).join(' '), '/*', commentNode.value, '*/']
32+
.join('').split('\n').map(function(v) {
33+
return v.substr(loc.start.column);
34+
});
35+
var value = lines.join('\n');
36+
2637
// parse comments
27-
var _parsed = _parseComment(commentNode.value) || {};
38+
var _parsed = _parseComment(value) || {};
2839

2940
// fill up fields
30-
this.loc = commentNode.loc;
31-
this.value = commentNode.value;
32-
this.lines = commentNode.lines || commentNode.value.split('\n');
41+
this.loc = loc;
42+
this.value = value;
43+
this.lines = lines;
3344
this.valid = _parsed.hasOwnProperty('line');
3445

3546
// doc parsed data
@@ -96,11 +107,11 @@ function DocType(type) {
96107
this.value = type;
97108

98109
var parsed = _parseDocType(type);
110+
var data = parsed.valid ? _simplifyType(parsed) : [];
111+
99112
this.optional = parsed.optional;
100113
this.valid = parsed.valid;
101114

102-
var data = _simplifyType(parsed);
103-
104115
/**
105116
* match type
106117
* @param {EsprimaNode} node
@@ -136,8 +147,8 @@ function _parseDocType(typeString) {
136147
node = parser.parse(typeString);
137148
node.valid = true;
138149
} catch (e) {
139-
console.error(e.stack);
140-
node = [];
150+
node = {};
151+
node.error = e.message;
141152
node.valid = false;
142153
}
143154
return node;

lib/rules/validate-jsdoc.js

Lines changed: 13 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,17 @@ module.exports.prototype = {
6363
]
6464
};
6565

66+
// classic checker
67+
if (_this._rulesForNodes.file) {
68+
// call file checkers
69+
var validators = _this._rulesForNodes.file;
70+
if (validators) {
71+
validators.forEach(function(v) {
72+
v.call(_this, file, errors);
73+
});
74+
}
75+
}
76+
6677
// iterate over scopes
6778
Object.keys(scopes).forEach(function(scope) {
6879

@@ -174,30 +185,12 @@ function patchNodesInFile(file) {
174185

175186
function getJsdoc() {
176187
if (!this.hasOwnProperty('_jsdoc')) {
177-
this._jsdoc = findNodeComment(this);
188+
var res = findDocCommentBeforeLine(this.loc.start.line);
189+
this._jsdoc = res ? jsdoc.createDocComment(res) : null;
178190
}
179191
return this._jsdoc;
180192
}
181193

182-
function findNodeComment(node) {
183-
var commentNode = findDocCommentBeforeLine(node.loc.start.line);
184-
if (!commentNode) {
185-
return null;
186-
}
187-
188-
// normalize data
189-
var loc = commentNode.loc;
190-
var lines = [Array(loc.start.column + 1).join(' '), '/*', commentNode.value, '*/']
191-
.join('').split('\n').map(function(v) {
192-
return v.substr(loc.start.column);
193-
});
194-
var value = lines.join('\n');
195-
commentNode.lines = lines;
196-
commentNode.value = value;
197-
198-
return jsdoc.createDocComment(commentNode);
199-
}
200-
201194
function findDocCommentBeforeLine(line) {
202195
line--; // todo: buggy behaviour, can't jump back over empty lines
203196
for (var i = 0, l = fileComments.length; i < l; i++) {
Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
var jsdoc = require('../../jsdoc');
2+
3+
module.exports = validateTypesInTags;
4+
module.exports.scopes = ['file'];
5+
module.exports.options = {
6+
checkTypes: {allowedValues: [true]}
7+
};
8+
9+
var allowedTags = {
10+
// common
11+
typedef: true,
12+
type: true,
13+
param: true,
14+
'return': true,
15+
returns: true,
16+
'enum': true,
17+
18+
// jsdoc
19+
'var': true,
20+
prop: true,
21+
property: true,
22+
arg: true,
23+
argument: true,
24+
25+
// jsduck
26+
cfg: true,
27+
28+
// closure
29+
lends: true,
30+
extends: true,
31+
implements: true,
32+
define: true
33+
};
34+
35+
/**
36+
* validator for types in tags
37+
* @param {JSCS.JSFile} file
38+
* @param {JSCS.Errors} errors
39+
*/
40+
function validateTypesInTags(file, errors) {
41+
var comments = file.getComments();
42+
comments.forEach(function(commentNode) {
43+
if (commentNode.type !== 'Block' || commentNode.value[0] !== '*') {
44+
return;
45+
}
46+
47+
// trying to create DocComment object
48+
var node = jsdoc.createDocComment(commentNode);
49+
if (!node.valid) {
50+
return;
51+
}
52+
53+
node.iterateByType(Object.keys(allowedTags), function(tag) {
54+
if (!tag.type) {
55+
// skip untyped params
56+
return;
57+
}
58+
if (!tag.type.valid) {
59+
// throw an error if not valid
60+
errors.add('jsDoc checkTypes: Expected valid type instead of ' + tag.value,
61+
node.loc.start.line + tag.line, node.loc.start.column + tag.id.length + 1);
62+
}
63+
});
64+
});
65+
}

lib/rules/validate-jsdoc/index.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
var assert = require('assert');
22

33
var validatorsByName = module.exports = {
4+
checkTypes: require('./check-types'),
45
param: require('./param'),
56
returns: require('./returns'),
67
checkRedundantAccess: require('./check-redundant-access'),

lib/rules/validate-jsdoc/param.js

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,7 @@ module.exports.scopes = ['function'];
55
module.exports.options = {
66
checkParamNames: {allowedValues: [true]},
77
requireParamTypes: {allowedValues: [true]},
8-
checkRedundantParams: {allowedValues: [true]},
9-
checkTypes: {allowedValues: [true]}
8+
checkRedundantParams: {allowedValues: [true]}
109
};
1110

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

34-
if (options.checkTypes && !tag.type.valid) {
35-
return err('Invalid JsDoc type definition');
36-
}
37-
3833
// skip if there is dot in param name (object's inner param)
3934
if (tag.name.indexOf('.') !== -1) {
4035
return;

lib/rules/validate-jsdoc/returns.js

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,6 @@ module.exports.options = {
1313
},
1414
checkRedundantReturns: {
1515
allowedValues: [true]
16-
},
17-
checkTypes: {
18-
allowedValues: [true]
1916
}
2017
};
2118

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

41-
if (options.checkTypes && !tag.type.valid) {
42-
return err('Invalid type definition in @returns statement');
43-
}
44-
4538
if (!options.checkRedundantReturns && !options.checkReturnTypes) {
4639
return;
4740
}

test/lib/rules/validate-jsdoc/check-return-types.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ describe('rules/validate-jsdoc', function () {
99
checker.cases([
1010
/* jshint ignore:start */
1111
{
12-
it: 'shouldn\'t throw',
12+
it: 'should throw invalid type',
1313
errors: 1,
1414
code: function () {
1515
/**

test/lib/rules/validate-jsdoc/check-types.js

Lines changed: 105 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,121 @@
1-
describe('rules/validate-jsdoc', function () {
1+
describe('rules/validate-jsdoc', function() {
22
var checker = global.checker({
33
additionalRules: ['lib/rules/validate-jsdoc.js']
44
});
55

6-
describe('check-types', function () {
6+
describe('check-types', function() {
77

88
checker.rules({checkTypes: true});
99
checker.cases([
1010
/* jshint ignore:start */
1111
{
12-
it: 'should neither throw nor report',
13-
skip: 1,
14-
code: function () {
12+
it: 'should report invalid typedef',
13+
errors: 1,
14+
code: function() {
15+
/**
16+
* @typedef {something/invalid} name
17+
*/
18+
}
19+
}, {
20+
it: 'should not report empty valid typedef',
21+
code: function() {
22+
/**
23+
* @typedef alphanum
24+
*/
25+
}
26+
}, {
27+
it: 'should not report valid typedef',
28+
code: function() {
29+
/**
30+
* @typedef {(string|number)} alphanum
31+
*/
32+
}
33+
34+
}, {
35+
it: 'should report invalid property',
36+
errors: 1,
37+
code: function() {
38+
/**
39+
* @typedef alphanum
40+
* @property {something/invalid} invalid
41+
*/
42+
}
43+
}, {
44+
it: 'should not report valid properties',
45+
code: function() {
46+
/**
47+
* @typedef {object} user
48+
* @property {number} age
49+
* @property {string} name
50+
*/
51+
}
52+
}, {
53+
it: 'should report member',
54+
code: function() {
55+
function ClsName () {
56+
/** @member {invalid/type} */
57+
this.point = {};
58+
}
59+
}
60+
}, {
61+
it: 'should not report member',
62+
code: function() {
63+
function ClsName () {
64+
/** @member {Object} */
65+
this.point = {};
66+
}
67+
}
68+
69+
}, {
70+
it: 'should not report member',
71+
code: function() {
72+
function ClsName () {
73+
/** @member {Object} */
74+
this.point = {};
75+
}
76+
}
77+
78+
}, {
79+
it: 'should not report return',
80+
code: function() {
1581
/**
1682
* @return {{a: number, b: string}}
1783
*/
1884
function foo() {
19-
return {};
85+
}
86+
}
87+
}, {
88+
it: 'should not report empty types in params and returns',
89+
code: function() {
90+
/**
91+
* @param q
92+
* @param w
93+
* @return
94+
*/
95+
ClsName.prototype.point = function (q, w) {
96+
}
97+
}
98+
}, {
99+
it: 'should not report valid types in params and returns',
100+
code: function() {
101+
/**
102+
* @param {Object} q
103+
* @param {Number} w
104+
* @return {String}
105+
*/
106+
ClsName.prototype.point = function (q, w) {
107+
}
108+
}
109+
}, {
110+
it: 'should report invalid types in params and returns',
111+
errors: 3,
112+
code: function() {
113+
/**
114+
* @param {Obj+ect} q
115+
* @param {Num/ber} w
116+
* @return {Str~ing}
117+
*/
118+
ClsName.prototype.point = function (q, w) {
20119
}
21120
}
22121
}

0 commit comments

Comments
 (0)