Skip to content

Commit a5ae34f

Browse files
committed
- require-returns-check: If a return is documented, allow @class and @interface (as with @constructor) to ignore reporting a missing return in body as constructors implicitly return the class; for testing, add getter check
- `require-returns`: Allow presence of `@interface` to avoid need for `@returns` (as `@interface implies a class`); Allow presence of ES setter to avoid need for `@returns` as setters don't return
1 parent 9f5d826 commit a5ae34f

File tree

5 files changed

+72
-16
lines changed

5 files changed

+72
-16
lines changed

src/iterateJsdoc.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,10 @@ const curryUtils = (
6060
return node.parent && node.parent.kind === 'constructor';
6161
};
6262

63+
utils.isSetter = () => {
64+
return functionNode.parent.kind === 'set';
65+
};
66+
6367
utils.getJsdocParameterNamesDeep = () => {
6468
return jsdocUtils.getJsdocParameterNamesDeep(jsdoc, utils.getPreferredTagName('param'));
6569
};

src/rules/requireReturns.js

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,12 +30,20 @@ const canSkip = (utils) => {
3030
'class',
3131
'constructor',
3232

33+
// This seems to imply a class as well
34+
'interface',
35+
3336
// While we may, in a future rule, err in the case of (regular) functions
3437
// using @implements (see https://github.com/gajus/eslint-plugin-jsdoc/issues/201 ),
3538
// this should not error for those using the tag to indicate implementation of
3639
// a particular function signature
3740
'implements'
38-
]) || utils.isConstructor();
41+
]) ||
42+
utils.isConstructor() ||
43+
44+
// Though ESLint avoided getters: https://github.com/eslint/eslint/blob/master/lib/rules/valid-jsdoc.js#L435
45+
// ... getters seem that they should, unlike setters, always return:
46+
utils.isSetter();
3947
};
4048

4149
export default iterateJsdoc(({

src/rules/requireReturnsCheck.js

Lines changed: 10 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,33 +1,28 @@
11
import iterateJsdoc from '../iterateJsdoc';
22

3-
const canSkip = (utils, node) => {
3+
const canSkip = (utils) => {
44
return utils.hasATag([
55
// An abstract function is by definition incomplete
6-
// so it is perfectly fine if the return is missing.
6+
// so it is perfectly fine if a return is documented but
7+
// not present within the function.
78
// A subclass may inherit the doc and implement the
89
// missing return.
910
'abstract',
1011
'virtual',
1112

12-
// A constructor function returns `this` by default
13-
'constructor'
14-
]) ||
15-
16-
utils.isConstructor() ||
17-
18-
// Implicit return like `() => foo` is ok
19-
utils.isArrowExpression() ||
20-
21-
// Async function always returns a `Promise`
22-
node.async;
13+
// A constructor function returns `this` by default, so may be `@returns`
14+
// tag indicating this but no explicit return
15+
'class',
16+
'constructor',
17+
'interface'
18+
]) || utils.isConstructor();
2319
};
2420

2521
export default iterateJsdoc(({
2622
report,
27-
node,
2823
utils
2924
}) => {
30-
if (canSkip(utils, node)) {
25+
if (canSkip(utils)) {
3126
return;
3227
}
3328

test/rules/assertions/requireReturns.js

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,24 @@ export default {
155155
forceRequireReturn: true
156156
}
157157
}
158+
},
159+
{
160+
code: `
161+
const language = {
162+
/**
163+
* @param {string} name
164+
*/
165+
get name() {
166+
return this._name;
167+
}
168+
}
169+
`,
170+
errors: [
171+
{
172+
line: 3,
173+
message: 'Missing JSDoc @returns declaration.'
174+
}
175+
]
158176
}
159177
],
160178
valid: [
@@ -378,6 +396,18 @@ export default {
378396
forceRequireReturn: true
379397
}
380398
}
399+
},
400+
{
401+
code: `
402+
const language = {
403+
/**
404+
* @param {string} name
405+
*/
406+
set name(name) {
407+
this._name = name;
408+
}
409+
}
410+
`
381411
}
382412
]
383413
};

test/rules/assertions/requireReturnsCheck.js

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,25 @@ export default {
7070
message: 'Found more than one @returns declaration.'
7171
}
7272
]
73+
},
74+
{
75+
code: `
76+
const language = {
77+
/**
78+
* @param {string} name
79+
* @returns {string}
80+
*/
81+
get name() {
82+
this._name = name;
83+
}
84+
}
85+
`,
86+
errors: [
87+
{
88+
line: 3,
89+
message: 'JSDoc @returns declaration present but return expression not available in function.'
90+
}
91+
]
7392
}
7493
],
7594
valid: [

0 commit comments

Comments
 (0)