Skip to content

Commit d360309

Browse files
author
Alexej Yaroshevich
committed
rules api refactoring
- split rules to 2 types: node checker, and tag checker - filter rules by tags in iterator - filter rules by scope (now it's only function scope) - patch rule files (set scope everywhere)
1 parent 165ed20 commit d360309

10 files changed

Lines changed: 202 additions & 64 deletions

File tree

‎lib/jsdoc-helpers.js‎

Lines changed: 0 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ var parse = require('comment-parser');
33

44
module.exports = {
55
parseComments : jsDocParseComments,
6-
tagValidator : jsDocTagValidator,
76

87
parse : jsDocParseType,
98
match : jsDocMatchType
@@ -61,35 +60,6 @@ function jsDocParseComments (comments) {
6160
}
6261
}
6362

64-
function jsDocTagValidator (validator) {
65-
return function(node, err) {
66-
var _this = this;
67-
if (!node.jsDoc) {
68-
return;
69-
}
70-
node.jsDoc.forEachTag(function(tag, i) {
71-
// call line validator
72-
validator.call(_this, node, tag, fixErrLocation(err, node, i));
73-
});
74-
};
75-
76-
function fixErrLocation (err, node, tagN) {
77-
return function(text, line, column) {
78-
var tag = node.jsDoc.data.tags[tagN];
79-
line = line || tag.line;
80-
// buggy. multiline comment will resolved to 0
81-
column = column || node.jsDoc.lines[tag.line].indexOf(tag.value);
82-
err(text, line, column);
83-
};
84-
/*function addError(text, loc) {
85-
loc = loc || {};
86-
loc.line = loc.hasOwnProperty('line') ? loc.line : (node.jsDoc.tag.loc.line);
87-
loc.column = loc.hasOwnProperty('column') ? loc.column : -1; // node.jsDoc.data.tags[i].indexOf('@');
88-
errors.add(text, loc.line, loc.column);
89-
}*/
90-
}
91-
}
92-
9363
/**
9464
* Parses jsDoc string
9565
* @param {String} typeString

‎lib/rules/validate-jsdoc.js‎

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

33
var jsDocHelpers = require('../jsdoc-helpers');
4+
var esprimaHelpers = require('../esprima-helpers');
45
var validators = require('./validate-jsdoc/index');
56

67
module.exports = function() {};
@@ -11,62 +12,141 @@ module.exports.prototype = {
1112
configure: function(options) {
1213
assert(typeof options === 'object', 'jsDoc option requires object value');
1314

15+
// rules structured by scopes-tags for jsdoc-tags
16+
var rulesForTags = this._rulesForTags = {};
17+
// rules structured by scopes for nodes
18+
var rulesForNodes = this._rulesForNodes = {};
19+
1420
this._options = options;
1521
this._optionsList = Object.keys(options);
16-
this._validators = validators.load(this._optionsList);
1722

23+
// load validators
24+
this._validators = validators.load(this._optionsList);
1825
assert(this._validators.length, 'jsDoc was not configured properly');
1926

27+
// registering validators
2028
this._validators.forEach(function(v) {
21-
if (v.configure) {
22-
v.configure.call(this, options);
23-
}
29+
// check options
2430
if (v.options) {
2531
validators.checkOptions(v, options);
2632
}
27-
}.bind(this));
33+
// index rules by tags and scopes
34+
(v.scopes || ['']).forEach(function(scope) {
35+
if (!v.tags) {
36+
assert(v.length === 2, 'jsDoc rules: Wrong arity in ' + v._name + ' validator');
37+
rulesForNodes[scope] = rulesForNodes[scope] || [];
38+
rulesForNodes[scope].push(v);
39+
return;
40+
}
41+
assert(v.length === 3, 'jsDoc rules: Wrong arity in ' + v._name + ' validator');
42+
rulesForTags[scope] = rulesForTags[scope] || {};
43+
v.tags.forEach(function(tag) {
44+
rulesForTags[scope][tag] = rulesForTags[scope][tag] || [];
45+
rulesForTags[scope][tag].push(v);
46+
});
47+
});
48+
}, this);
2849
},
2950

3051
getOptionName: function() {
3152
return 'jsDoc';
3253
},
3354

3455
check: function(file, errors) {
35-
var activeValidators = this._validators;
36-
37-
// skip if there is no validators
38-
if (!activeValidators.length) {
39-
return;
40-
}
41-
4256
var jsDocs = jsDocHelpers.parseComments(file.getComments());
4357
var _this = this;
4458

45-
file.iterateNodesByType([
46-
'FunctionDeclaration',
47-
'FunctionExpression'
59+
var scopes = {
60+
'function': [
61+
'FunctionDeclaration',
62+
'FunctionExpression'
63+
]
64+
};
65+
66+
Object.keys(scopes).forEach(function(scope) {
67+
// skip unused scopes
68+
if (!_this._rulesForNodes[scope] && !_this._rulesForTags[scope]) {
69+
return;
70+
}
4871

49-
], function(node) {
50-
node.jsDoc = jsDocs.forNode(node);
51-
var commentStart = (node.jsDoc || node).loc.start;
72+
// traverse ast tree and search scope node types
73+
file.iterateNodesByType(scopes[scope], function(node) {
74+
// init
75+
node.jsDoc = node.jsDoc || jsDocs.forNode(node);
76+
var commentStart = (node.jsDoc || node).loc.start;
77+
var commentStartLine = commentStart.line;
78+
var validators;
79+
80+
// call node checkers
81+
validators = _this._rulesForNodes[scope];
82+
if (validators) {
83+
for (var j = 0, k = validators.length; j < k; j += 1) {
84+
validators[j].call(_this, node, addError);
85+
}
86+
}
5287

53-
for (var j = 0, k = activeValidators.length; j < k; j += 1) {
54-
activeValidators[j].call(_this, node, addError);
55-
}
88+
validators = _this._rulesForTags[scope];
89+
if (!node.jsDoc || !validators) {
90+
return;
91+
}
92+
93+
// call rule checkers
94+
node.jsDoc.forEachTag(function(tag, i) {
95+
if (!validators[tag.tag]) {
96+
return;
97+
}
98+
// call tag validator
99+
commentStart.line = commentStartLine + i;
100+
validators[tag.tag].forEach(function(v) {
101+
v.call(_this, node, tag, fixErrLocation(addError, tag));
102+
});
103+
});
104+
105+
function addError(text, relLine, relColumn) {
106+
var line;
107+
var column;
108+
if (typeof relLine === 'object') {
109+
line = relLine.line;
110+
column = relLine.column;
111+
} else {
112+
line = commentStart.line + (relLine || 0);
113+
column = commentStart.column + (relColumn || 0);
114+
}
115+
errors.add(text, line, column);
116+
}
117+
118+
function fixErrLocation (err, tag) {
119+
return function(text, line, column) {
120+
line = line || tag.line;
121+
// probably buggy. multiline comment could resolved to 0
122+
column = column || node.jsDoc.lines[tag.line].indexOf(tag.value);
123+
err(text, line, column);
124+
};
125+
}
126+
});
127+
128+
});
129+
130+
},
131+
132+
/**
133+
* caching scope search
134+
*/
135+
_getReturnStatementsForNode: function(node) {
136+
if (node.jsDoc.returnStatements) {
137+
return node.jsDoc.returnStatements;
138+
}
56139

57-
function addError(text, relLine, relColumn) {
58-
var line;
59-
var column;
60-
if (typeof relLine === 'object') {
61-
line = relLine.line;
62-
column = relLine.column;
63-
} else {
64-
line = commentStart.line + (relLine || 0);
65-
column = commentStart.column + (relColumn || 0);
140+
var statements = [];
141+
esprimaHelpers.treeIterator.iterate(node, function(n/*, parentNode, parentCollection*/) {
142+
if (n && n.type === 'ReturnStatement' && n.argument) {
143+
if (node === esprimaHelpers.closestScopeNode(n)) {
144+
statements.push(n.argument);
66145
}
67-
errors.add(text, line, column);
68146
}
69147
});
70148

149+
node.jsDoc.returnStatements = statements;
150+
return statements;
71151
}
72152
};

‎lib/rules/validate-jsdoc/check-redundant-access.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
module.exports = checkRedundantAccess;
2+
module.exports.scopes = ['function'];
23

34
module.exports.options = {
45
checkRedundantAccess: {allowedValues: [true]}

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

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
module.exports = checkReturnTypes;
2+
module.exports.tags = ['return', 'returns'];
3+
module.exports.scopes = ['function'];
4+
module.exports.options = {
5+
checkReturnTypes: {allowedValues: [true]}
6+
};
7+
8+
/**
9+
* checking returns types
10+
* @param {(FunctionDeclaration|FunctionExpression)} node
11+
* @param {DocTag} tag
12+
* @param {Function} err
13+
*/
14+
function checkReturnTypes(node, tag, err) {
15+
/* var option = this._options.checkReturnTypes;
16+
if (!node.jsdoc) {
17+
return;
18+
}
19+
20+
console.log(node.jsdoc);
21+
22+
if (node.jsdoc) {
23+
24+
}*/
25+
if (node) { err(); }
26+
return tag;
27+
}

‎lib/rules/validate-jsdoc/enforce-existence.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
module.exports = enforceExistence;
2-
2+
module.exports.scopes = ['function'];
33
module.exports.options = {
44
enforceExistence: {allowedValues: [true]}
55
};

‎lib/rules/validate-jsdoc/leading-underscore-access.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
module.exports = validateLeadingUnderscoresAccess;
2+
module.exports.scopes = ['function'];
23

34
module.exports.options = {
45
leadingUnderscoreAccess: {

‎lib/rules/validate-jsdoc/param.js‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11

22
var jsDocHelpers = require('../../jsdoc-helpers');
33

4-
module.exports = jsDocHelpers.tagValidator(validateParamLine);
4+
module.exports = validateParamLine;
5+
module.exports.tags = ['param'];
6+
module.exports.scopes = ['function'];
57

68
module.exports.options = {
79
checkParamNames: {

‎lib/rules/validate-jsdoc/returns.js‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,9 @@
22
var jsDocHelpers = require('../../jsdoc-helpers');
33
var esprimaHelpers = require('../../esprima-helpers');
44

5-
module.exports = jsDocHelpers.tagValidator(validateReturnsTag);
5+
module.exports = validateReturnsTag; //jsDocHelpers.tagValidator(validateReturnsTag);
6+
module.exports.tags = ['return', 'returns'];
7+
module.exports.scopes = ['function'];
68

79
module.exports.options = {
810
checkReturnTypes: {

‎test/lib/rules/validate-jsdoc.js‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
describe('rules/validate-jsdoc', function () {
2+
var checker = global.checker({
3+
additionalRules: ['lib/rules/validate-jsdoc.js']
4+
});
5+
6+
describe('basic checks', function () {
7+
8+
checker.rules({checkReturnTypes: true});
9+
checker.cases([
10+
/* jshint ignore:start */
11+
{
12+
it: 'shouldn\'t throw',
13+
errors: 1,
14+
code: function () {
15+
/**
16+
* @return {Foo}
17+
*/
18+
function getFoo() {
19+
return new Bar();
20+
}
21+
}
22+
}
23+
/* jshint ignore:end */
24+
]);
25+
26+
});
27+
});

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

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
describe('rules/validate-jsdoc', function () {
2+
var checker = global.checker({
3+
additionalRules: ['lib/rules/validate-jsdoc.js']
4+
});
5+
6+
describe('check-types', function () {
7+
8+
checker.rules({checkTypes: true});
9+
checker.cases([
10+
/* jshint ignore:start */
11+
{
12+
it: 'should neither throw nor report',
13+
skip: 1,
14+
code: function () {
15+
/**
16+
* @return {{a: number, b: string}}
17+
*/
18+
function foo() {
19+
return {};
20+
}
21+
}
22+
}
23+
/* jshint ignore:end */
24+
]);
25+
26+
});
27+
28+
});

0 commit comments

Comments
 (0)