Skip to content
Permalink

Comparing changes

Choose two branches to see what’s changed or to start a new pull request. If you need to, you can also or learn more about diff comparisons.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also . Learn more about diff comparisons here.
base repository: gajus/eslint-plugin-jsdoc
Failed to load repositories. Confirm that selected base ref is valid, then try again.
Loading
base: v39.3.22
Choose a base ref
...
head repository: gajus/eslint-plugin-jsdoc
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: v39.3.23
Choose a head ref
  • 1 commit
  • 3 files changed
  • 1 contributor

Commits on Oct 23, 2022

  1. fix(require-returns-check): allow implicit-return finally if other …

    …try-catch branches returning; fixes #926
    
    Also:
    - fix(`require-returns-check`): finally-only returns to be treated as returns
    brettz9 committed Oct 23, 2022
    Copy the full SHA
    12da503 View commit details
Showing with 74 additions and 37 deletions.
  1. +23 −12 README.md
  2. +22 −5 src/utils/hasReturnValue.js
  3. +29 −20 test/rules/assertions/requireReturnsCheck.js
35 changes: 23 additions & 12 deletions README.md
Original file line number Diff line number Diff line change
@@ -17568,18 +17568,6 @@ function quux () {
}
// Message: JSDoc @returns declaration present but return expression not available in function.

/**
* @returns {true}
*/
function quux () {
try {
} catch (error) {
} finally {
return true;
}
}
// Message: JSDoc @returns declaration present but return expression not available in function.

/**
* @returns {true}
*/
@@ -18130,6 +18118,29 @@ function parseSipHeaders(logPrefix, sipMessage, headers) {
return {};
}
}

/**
* @returns {true}
*/
function quux () {
try {
} catch (error) {
} finally {
return true;
}
}

/** Returns true.
*
* @returns {boolean} true
*/
function getTrue() {
try {
return true;
} finally {
console.log('returning...');
}
}
````


27 changes: 22 additions & 5 deletions src/utils/hasReturnValue.js
Original file line number Diff line number Diff line change
@@ -162,11 +162,28 @@ const allBrancheshaveReturnValues = (node, promFilter) => {
}

case 'TryStatement': {
return allBrancheshaveReturnValues(node.block, promFilter) &&
(!node.handler ||
allBrancheshaveReturnValues(node.handler && node.handler.body, promFilter)) &&
(!node.finalizer ||
allBrancheshaveReturnValues(node.finalizer, promFilter));
// If `finally` returns, all return
return node.finalizer && allBrancheshaveReturnValues(node.finalizer, promFilter) ||
// Return in `try`/`catch` may still occur despite `finally`
allBrancheshaveReturnValues(node.block, promFilter) &&
(!node.handler ||
allBrancheshaveReturnValues(node.handler && node.handler.body, promFilter)) &&
(!node.finalizer || (() => {
try {
hasReturnValue(node.finalizer, true, promFilter);
} catch (error) {
// istanbul ignore else
if (error.message === 'Null return') {
return false;
}

// istanbul ignore next
throw error;
}

// As long as not an explicit empty return, then return true
return true;
})());
}

case 'SwitchStatement': {
49 changes: 29 additions & 20 deletions test/rules/assertions/requireReturnsCheck.js
Original file line number Diff line number Diff line change
@@ -569,26 +569,6 @@ export default {
},
],
},
{
code: `
/**
* @returns {true}
*/
function quux () {
try {
} catch (error) {
} finally {
return true;
}
}
`,
errors: [
{
line: 2,
message: 'JSDoc @returns declaration present but return expression not available in function.',
},
],
},
{
code: `
/**
@@ -1417,5 +1397,34 @@ export default {
}
`,
},
{
code: `
/**
* @returns {true}
*/
function quux () {
try {
} catch (error) {
} finally {
return true;
}
}
`,
},
{
code: `
/** Returns true.
*
* @returns {boolean} true
*/
function getTrue() {
try {
return true;
} finally {
console.log('returning...');
}
}
`,
},
],
};