Skip to content

Commit 87c1c2a

Browse files
committed
fix(require-returns-check): throw statements to be exempted from final check if elsewhere in function; fixes #892
1 parent 5d20bdf commit 87c1c2a

File tree

3 files changed

+87
-2
lines changed

3 files changed

+87
-2
lines changed

README.md

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17552,6 +17552,18 @@ function quux () {
1755217552
}
1755317553
}
1755417554
// Message: JSDoc @returns declaration present but return expression not available in function.
17555+
17556+
/**
17557+
* @returns {true}
17558+
*/
17559+
function quux () {
17560+
if (true) {
17561+
throw new Error('abc');
17562+
}
17563+
17564+
throw new Error('def');
17565+
}
17566+
// Message: JSDoc @returns declaration present but return expression not available in function.
1755517567
````
1755617568

1755717569
The following patterns are not considered problems:
@@ -18003,6 +18015,17 @@ const quux = function () {
1800318015
resolve(3);
1800418016
});
1800518017
};
18018+
18019+
/**
18020+
* @returns {true}
18021+
*/
18022+
function quux () {
18023+
if (true) {
18024+
return true;
18025+
}
18026+
18027+
throw new Error('Fail');
18028+
}
1800618029
````
1800718030

1800818031

src/utils/hasReturnValue.js

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,10 @@ const allBrancheshaveReturnValues = (node, promFilter) => {
8484
);
8585
}
8686

87+
case 'ThrowStatement': {
88+
return true;
89+
}
90+
8791
case 'ReturnStatement': {
8892
// void return does not count.
8993
if (node.argument === null) {
@@ -418,8 +422,9 @@ const hasNonEmptyResolverCall = (node, resolverName) => {
418422
const hasValueOrExecutorHasNonEmptyResolveValue = (node, anyPromiseAsReturn, allBranches) => {
419423
const hasReturnMethod = allBranches ?
420424
(nde, promiseFilter) => {
425+
let hasReturn;
421426
try {
422-
hasReturnValue(nde, true, promiseFilter);
427+
hasReturn = hasReturnValue(nde, true, promiseFilter);
423428
} catch (error) {
424429
// istanbul ignore else
425430
if (error.message === 'Null return') {
@@ -430,7 +435,9 @@ const hasValueOrExecutorHasNonEmptyResolveValue = (node, anyPromiseAsReturn, all
430435
throw error;
431436
}
432437

433-
return allBrancheshaveReturnValues(nde, promiseFilter);
438+
// `hasReturn` check needed since `throw` treated as valid return by
439+
// `allBrancheshaveReturnValues`
440+
return hasReturn && allBrancheshaveReturnValues(nde, promiseFilter);
434441
} :
435442
(nde, promiseFilter) => {
436443
return hasReturnValue(nde, false, promiseFilter);

test/rules/assertions/requireReturnsCheck.js

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -589,6 +589,47 @@ export default {
589589
},
590590
],
591591
},
592+
{
593+
code: `
594+
/**
595+
* @returns {true}
596+
*/
597+
function quux () {
598+
if (true) {
599+
throw new Error('abc');
600+
}
601+
602+
throw new Error('def');
603+
}
604+
`,
605+
errors: [
606+
{
607+
line: 2,
608+
message: 'JSDoc @returns declaration present but return expression not available in function.',
609+
},
610+
],
611+
},
612+
{
613+
code: `
614+
/**
615+
* @returns {true}
616+
*/
617+
function quux () {
618+
if (true) {
619+
return true;
620+
}
621+
622+
const a = () => {};
623+
}
624+
`,
625+
errors: [
626+
{
627+
line: 2,
628+
message: 'JSDoc @returns declaration present but return expression not available in function.',
629+
},
630+
],
631+
ignoreReadme: true,
632+
},
592633
],
593634
valid: [
594635
{
@@ -1260,5 +1301,19 @@ export default {
12601301
};
12611302
`,
12621303
},
1304+
{
1305+
code: `
1306+
/**
1307+
* @returns {true}
1308+
*/
1309+
function quux () {
1310+
if (true) {
1311+
return true;
1312+
}
1313+
1314+
throw new Error('Fail');
1315+
}
1316+
`,
1317+
},
12631318
],
12641319
};

0 commit comments

Comments
 (0)