Skip to content

Commit

Permalink
fix(require-returns, require-returns-check): properly handle catch
Browse files Browse the repository at this point in the history
refactor: Don't need undefined tag checks in `hasDefinedTypeReturnTag`
testing(jsdocUtils): `getFunctionParameterNames` and `hasDefinedTypeReturnTag` for full coverage
testing(require-returns-check): Complete coverage (try/switch/for/if/else branches)
  • Loading branch information
brettz9 committed Jun 21, 2019
1 parent 5129f67 commit ea8b0f7
Show file tree
Hide file tree
Showing 3 changed files with 168 additions and 4 deletions.
14 changes: 10 additions & 4 deletions src/jsdocUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,7 @@ const lookupTable = {
return node.type === 'ReturnStatement';
},
check (node) {
/* istanbul ignore next */
if (!lookupTable.ReturnStatement.is(node)) {
return false;
}
Expand All @@ -280,6 +281,7 @@ const lookupTable = {
return node.type === 'IfStatement';
},
check (node) {
/* istanbul ignore next */
if (!lookupTable.IfStatement.is(node)) {
return false;
}
Expand Down Expand Up @@ -324,6 +326,7 @@ const lookupTable = {
return node.type === 'TryStatement';
},
check (node) {
/* istanbul ignore next */
if (!lookupTable.TryStatement.is(node)) {
return false;
}
Expand All @@ -332,12 +335,11 @@ const lookupTable = {
return true;
}

if (node.handler && node.handler.block) {
if (lookupTable['@default'].check(node)) {
if (node.handler && node.handler.body) {
if (lookupTable['@default'].check(node.handler.body)) {
return true;
}
}

if (lookupTable.BlockStatement.check(node.finalizer)) {
return true;
}
Expand All @@ -351,10 +353,12 @@ const lookupTable = {
},
check (node, context) {
// E.g. the catch block statement is optional.
/* istanbul ignore next */
if (typeof node === 'undefined' || node === null) {
return false;
}

/* istanbul ignore next */
if (!lookupTable.BlockStatement.is(node)) {
return false;
}
Expand Down Expand Up @@ -411,10 +415,12 @@ const lookupTable = {
}

// Everything else cannot return anything.
/* istanbul ignore next */
if (RETURNFREE_STATEMENTS.includes(node.type)) {
return false;
}

/* istanbul ignore next */
// If we end up here, we stumbled upon an unknown element.
// Most likely it is enough to add it to the blacklist.
//
Expand Down Expand Up @@ -444,7 +450,7 @@ const hasReturnValue = (node, context, ignoreAsync) => {
return lookupTable[item].check(node, context, ignoreAsync);
}
}

/* istanbul ignore next */
throw new Error('Unknown element ' + node.type);
};

Expand Down
20 changes: 20 additions & 0 deletions test/jsdocUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,4 +45,24 @@ describe('jsdocUtils', () => {
});
});
});
describe('getFunctionParameterNames()', () => {
context('Unhandled param type', () => {
it('should throw with an unknown param type', () => {
expect(() => {
jsdocUtils.getFunctionParameterNames({params: [
{
type: 'AssignmentPattern'
}
]});
}).to.throw('Unsupported function signature format.');
});
});
});
describe('hasDefinedTypeReturnTag()', () => {
context('Missing tag', () => {
it('should return `false` with a missing tag', () => {
expect(jsdocUtils.hasDefinedTypeReturnTag(null)).to.equal(false);
});
});
});
});
138 changes: 138 additions & 0 deletions test/rules/assertions/requireReturnsCheck.js
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,144 @@ export default {
return;
}
`
},
{
code: `
/**
* @returns {true}
*/
function quux () {
try {
return true;
} catch (err) {
}
return;
}
`
},
{
code: `
/**
* @returns {true}
*/
function quux () {
try {
} finally {
return true;
}
return;
}
`
},
{
code: `
/**
* @returns {true}
*/
function quux () {
try {
return;
} catch (err) {
}
return true;
}
`
},
{
code: `
/**
* @returns {true}
*/
function quux () {
try {
something();
} catch (err) {
return true;
}
return;
}
`
},
{
code: `
/**
* @returns {true}
*/
function quux () {
switch (true) {
case 'abc':
return true;
}
return;
}
`
},
{
code: `
/**
* @returns {true}
*/
function quux () {
switch (true) {
case 'abc':
return;
}
return true;
}
`
},
{
code: `
/**
* @returns {true}
*/
function quux () {
for (const i of abc) {
return true;
}
return;
}
`
},
{
code: `
/**
* @returns {true}
*/
function quux () {
if (true) {
return;
}
return true;
}
`
},
{
code: `
/**
* @returns {true}
*/
function quux () {
if (true) {
return true;
}
}
`
},
{
code: `
/**
* @returns {true}
*/
function quux () {
if (true) {
return;
} else {
return true;
}
return;
}
`
}
]
};

0 comments on commit ea8b0f7

Please sign in to comment.