Skip to content

Commit 5fb9088

Browse files
author
Alexej Yaroshevich
committed
fixing location determination, more tests
1 parent ecefdc6 commit 5fb9088

6 files changed

Lines changed: 171 additions & 101 deletions

File tree

.jscsrc

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
"disallowImplicitTypeConversion": ["string"],
1313

1414
"excludeFiles": [
15-
"test/data/**"
15+
"test/data/**",
16+
"test/lib/rules/**"
1617
]
1718
}

lib/jsdoc-helpers.js

Lines changed: 24 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,12 @@ function jsDocParseComments (comments) {
2121
function getJsDocForNode (node) {
2222
var jsdoc = getJsDocForLine(node.loc.start.line);
2323
if (jsdoc) {
24-
jsdoc.data = parseJsDoc(jsdoc.value) || {};
25-
jsdoc.invalid = !jsdoc.data.line;
26-
jsdoc.data.tags = jsdoc.data.tags || [];
24+
var comment = Array(jsdoc.loc.start.column + 1).join(' ') + '/*' + jsdoc.value + '*/';
25+
jsdoc.data = parseJsDoc(comment);
26+
jsdoc.lines = comment.split('\n').map(function (v) {
27+
return v.substr(jsdoc.loc.start.column);
28+
});
29+
jsdoc.invalid = !jsdoc.data.hasOwnProperty('line');
2730
jsdoc.forEachTag = jsDocForEachTag;
2831
}
2932
return jsdoc;
@@ -37,7 +40,7 @@ function jsDocParseComments (comments) {
3740
}
3841

3942
function getJsDocForLine (line) {
40-
line--;
43+
line--; // todo: buggy behaviour, can't jump back over empty lines
4144
for (var i = 0, l = comments.length; i < l; i++) {
4245
var commentNode = comments[i];
4346
if (commentNode.loc.end.line === line && commentNode.type === 'Block' &&
@@ -49,10 +52,12 @@ function jsDocParseComments (comments) {
4952
}
5053

5154
function parseJsDoc (comment) {
52-
return parse('/*' + comment + '*/', {
55+
var parsed = parse(comment, {
5356
lineNumbers: true,
5457
rawValue: true
55-
})[0] || [];
58+
})[0] || {};
59+
parsed.tags = parsed.tags || [];
60+
return parsed;
5661
}
5762
}
5863

@@ -68,12 +73,20 @@ function jsDocTagValidator (validator) {
6873
});
6974
};
7075

71-
function fixErrLocation (err, node, shift) {
72-
return function (text, loc) {
73-
loc = loc || {};
74-
loc.line = loc.hasOwnProperty('line') ? loc.line : (node.loc.start.line + shift);
75-
err(text, loc);
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);
7683
};
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+
}*/
7790
}
7891
}
7992

lib/rules/validate-jsdoc.js

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -47,15 +47,16 @@ module.exports.prototype = {
4747

4848
], function (node) {
4949
node.jsDoc = jsDocs.forNode(node);
50+
var commentStart = (node.jsDoc || node).loc.start;
51+
5052
for (var j = 0, k = activeValidators.length; j < k; j += 1) {
5153
activeValidators[j].call(that, node, addError);
5254
}
5355

54-
function addError(text, loc) {
55-
loc = loc || {};
56-
loc.line = loc.hasOwnProperty('line') ? loc.line : (node.jsDoc.loc.start.line);
57-
loc.column = loc.hasOwnProperty('column') ? loc.column : 0; //node.jsDoc[i].indexOf('@');
58-
errors.add(text, loc.line, loc.column);
56+
function addError(text, relLine, relColumn) {
57+
var line = commentStart.line + (relLine || 0);
58+
var column = commentStart.column + (relColumn || 0);
59+
errors.add(text, line, column);
5960
}
6061
});
6162

test/init.js

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
var Checker = require('jscs/lib/checker');
22
var parse = require('comment-parser');
3-
var expect = require('chai').expect;
3+
var chai = require('chai');
4+
var expect = chai.expect;
45

56
global.parse = parse;
67
global.fnBody = fnBody;
@@ -69,13 +70,34 @@ function rulesChecker(opts) {
6970
if (!test.hasOwnProperty('errors') || (typeof test.errors === 'number')) {
7071
expect(checked.getErrorCount())
7172
.to.eq(test.errors || 0);
72-
} else {
73+
} else if (Array.isArray(test.errors)) {
7374
expect(errors)
7475
.to.deep.equal(test.errors);
76+
} else {
77+
expect(checked.getErrorCount())
78+
.to.not.eq(0);
79+
expect(errors[0])
80+
.to.deep.similar(test.errors);
7581
}
7682
});
7783

7884
});
7985
}
8086
};
8187
}
88+
89+
chai.use(function (chai, utils) {
90+
utils.addMethod(chai.Assertion.prototype, 'similar', method);
91+
92+
function method(expected) {
93+
var obj = utils.flag(this, 'object');
94+
95+
Object.keys(obj).forEach(function (k) {
96+
if (!expected.hasOwnProperty(k)) {
97+
expected[k] = obj[k];
98+
}
99+
});
100+
101+
new chai.Assertion(obj).to.be.deep.equal(expected);
102+
}
103+
});
Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,115 @@
1+
describe('rules/validate-jsdoc @param', function () {
2+
var checker = global.checker({
3+
additionalRules: ['lib/rules/validate-jsdoc.js']
4+
});
5+
checker.rules({checkParamNames: true});
6+
7+
describe('common', function () {
8+
9+
checker.cases([
10+
/* jshint ignore:start */
11+
{
12+
it: 'should report invalid jsdoc',
13+
code: function () {
14+
var x = 1;
15+
/**
16+
* @param
17+
*/
18+
function funcName(xxx) {
19+
}
20+
},
21+
errors: 1
22+
}
23+
/* jshint ignore:end */
24+
]);
25+
26+
});
27+
28+
describe('param-names', function () {
29+
30+
checker.cases([
31+
/* jshint ignore:start */
32+
{
33+
it: 'should report error in jsdoc for function',
34+
code: function () {
35+
var x = 1;
36+
/**
37+
* @param {String} yyy
38+
*/
39+
function funcName(xxx) {
40+
}
41+
},
42+
errors: 1
43+
}, {
44+
it: 'should report error in jsdoc for method',
45+
code: function () {
46+
Cls.prototype = {
47+
/**
48+
* @param {String} yyy
49+
*/
50+
run: function(xxx) {
51+
}
52+
};
53+
},
54+
errors: 1
55+
}, {
56+
it: 'should not report valid jsdoc for method',
57+
code: function () {
58+
var x = 1;
59+
/**
60+
* @param {String} xxx
61+
*/
62+
function funcName(xxx) {
63+
}
64+
}
65+
}, {
66+
it: 'should not report valid jsdoc for function',
67+
code: function () {
68+
Cls.prototype = {
69+
/**
70+
* @param {String} xxx
71+
*/
72+
run: function(xxx) {
73+
}
74+
};
75+
}
76+
}
77+
/* jshint ignore:end */
78+
]);
79+
80+
});
81+
82+
describe('param-names location', function () {
83+
84+
checker.cases([
85+
/* jshint ignore:start */
86+
{
87+
it: 'should report error in jsdoc for function',
88+
code: function () {
89+
var x = 1;
90+
/**
91+
* @param {String} yyy
92+
*/
93+
function funcName(xxx) {
94+
}
95+
},
96+
errors: {line: 3, column: 3}
97+
}, {
98+
it: 'should report error in jsdoc for method',
99+
code: function () {
100+
Cls.prototype = {
101+
/**
102+
* @param {String} yyy
103+
*/
104+
run: function(xxx) {
105+
}
106+
};
107+
},
108+
errors: {line: 3, column: 7}
109+
}
110+
/* jshint ignore:end */
111+
]);
112+
113+
});
114+
115+
});

test/test.validate-jsdoc-basic.js

Lines changed: 0 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -10,88 +10,6 @@ describe('rules/validate-jsdoc @param', function () {
1010
checker.configure({ additionalRules: ['lib/rules/validate-jsdoc.js'] });
1111
});
1212

13-
describe('common', function () {
14-
15-
it('should report invalid jsdoc', function () {
16-
checker.configure({ jsDoc: { checkParamNames: true } });
17-
assert(
18-
checker.checkString(
19-
'var x = 1;\n' +
20-
'/**\n' +
21-
' * @param\n' +
22-
' */\n' +
23-
'function funcName(xxx) {\n' +
24-
'\n' +
25-
'}'
26-
).getErrorCount() === 1
27-
);
28-
});
29-
30-
});
31-
32-
describe('param-names', function () {
33-
34-
it('should report error in jsdoc for function', function () {
35-
checker.configure({ jsDoc: { checkParamNames: true } });
36-
assert(
37-
checker.checkString(
38-
'var x = 1;\n' +
39-
'/**\n' +
40-
' * @param {String} yyy\n' +
41-
' */\n' +
42-
'function funcName(xxx) {\n' +
43-
'\n' +
44-
'}'
45-
).getErrorCount() === 1
46-
);
47-
});
48-
it('should report error in jsdoc for method', function () {
49-
checker.configure({ jsDoc: { checkParamNames: true } });
50-
assert(
51-
checker.checkString(
52-
'Cls.prototype = {\n' +
53-
' /**\n' +
54-
' * @param {String} yyy\n' +
55-
' */\n' +
56-
' run: function(xxx) {\n' +
57-
' \n' +
58-
' }\n' +
59-
'};'
60-
).getErrorCount() === 1
61-
);
62-
});
63-
it('should not report valid jsdoc for method', function () {
64-
checker.configure({ jsDoc: { checkParamNames: true } });
65-
assert(
66-
checker.checkString(
67-
'var x = 1;\n' +
68-
'/**\n' +
69-
' * @param {String} xxx\n' +
70-
' */\n' +
71-
'function funcName(xxx) {\n' +
72-
'\n' +
73-
'}'
74-
).isEmpty()
75-
);
76-
});
77-
it('should not report valid jsdoc for function', function () {
78-
checker.configure({ jsDoc: { checkParamNames: true } });
79-
assert(
80-
checker.checkString(
81-
'Cls.prototype = {\n' +
82-
' /**\n' +
83-
' * @param {String} xxx\n' +
84-
' */\n' +
85-
' run: function(xxx) {\n' +
86-
' \n' +
87-
' }\n' +
88-
'};'
89-
).isEmpty()
90-
);
91-
});
92-
93-
});
94-
9513
describe('redudant-params', function () {
9614

9715
it('should report redundant jsdoc-param for function', function () {

0 commit comments

Comments
 (0)