diff --git a/__tests__/no-nesting.js b/__tests__/no-nesting.js index 9a4e26af..1943efa9 100644 --- a/__tests__/no-nesting.js +++ b/__tests__/no-nesting.js @@ -45,6 +45,17 @@ ruleTester.run('no-nesting', rule, { 'doThing().then(function() { return Promise.resolve(4) })', 'doThing().then(() => Promise.resolve(4))', 'doThing().then(() => Promise.all([a]))', + + // references vars in closure + `doThing() + .then(a => getB(a) + .then(b => getC(a, b)) + )`, + `doThing() + .then(a => { + const c = a * 2; + return getB(c).then(b => getC(c, b)) + })`, ], invalid: [ @@ -80,5 +91,24 @@ ruleTester.run('no-nesting', rule, { code: 'doThing().then(() => b.catch())', errors: [{ message: errorMessage }], }, + // references vars in closure + { + code: ` + doThing() + .then(a => getB(a) + .then(b => getC(b)) + )`, + errors: [{ message: errorMessage, line: 4 }], + }, + { + code: ` + doThing() + .then(a => getB(a) + .then(b => getC(a, b) + .then(c => getD(a, c)) + ) + )`, + errors: [{ message: errorMessage, line: 5 }], + }, ], }) diff --git a/rules/lib/has-promise-callback.js b/rules/lib/has-promise-callback.js index 8cf07c01..7b6b71ca 100644 --- a/rules/lib/has-promise-callback.js +++ b/rules/lib/has-promise-callback.js @@ -6,6 +6,26 @@ 'use strict' +/** + * @typedef {import('estree').SimpleCallExpression} CallExpression + * @typedef {import('estree').MemberExpression} MemberExpression + * @typedef {import('estree').Identifier} Identifier + * + * @typedef {object} NameIsThenOrCatch + * @property {'then' | 'catch'} name + * + * @typedef {object} PropertyIsThenOrCatch + * @property {Identifier & NameIsThenOrCatch} property + * + * @typedef {object} CalleeIsPromiseCallback + * @property {MemberExpression & PropertyIsThenOrCatch} callee + * + * @typedef {CallExpression & CalleeIsPromiseCallback} HasPromiseCallback + */ +/** + * @param {import('estree').Node} node + * @returns {node is HasPromiseCallback} + */ function hasPromiseCallback(node) { // istanbul ignore if -- only being called within `CallExpression` if (node.type !== 'CallExpression') return diff --git a/rules/no-nesting.js b/rules/no-nesting.js index 7e780253..a06ec219 100644 --- a/rules/no-nesting.js +++ b/rules/no-nesting.js @@ -18,12 +18,100 @@ module.exports = { schema: [], }, create(context) { + /** + * Array of callback function scopes. + * Scopes are in order closest to the current node. + * @type {import('eslint').Scope.Scope[]} + */ + const callbackScopes = [] + + /** + * @param {import('eslint').Scope.Scope} scope + * @returns {Iterable} + */ + function* iterateDefinedReferences(scope) { + for (const variable of scope.variables) { + for (const reference of variable.references) { + yield reference + } + } + } + return { + ':function'(node) { + if (isInsidePromise(node)) { + callbackScopes.unshift(context.getScope()) + } + }, + ':function:exit'(node) { + if (isInsidePromise(node)) { + callbackScopes.shift() + } + }, CallExpression(node) { if (!hasPromiseCallback(node)) return - if (context.getAncestors().some(isInsidePromise)) { - context.report({ node, message: 'Avoid nesting promises.' }) + if (!callbackScopes.length) { + // The node is not in the callback function. + return } + + // Checks if the argument callback uses variables defined in the closest callback function scope. + // + // e.g. + // ``` + // doThing() + // .then(a => getB(a) + // .then(b => getC(a, b)) + // ) + // ``` + // + // In the above case, Since the variables it references are undef, + // we cannot refactor the nesting like following: + // ``` + // doThing() + // .then(a => getB(a)) + // .then(b => getC(a, b)) + // ``` + // + // However, `getD` can be refactored in the following: + // ``` + // doThing() + // .then(a => getB(a) + // .then(b => getC(a, b) + // .then(c => getD(a, c)) + // ) + // ) + // ``` + // ↓ + // ``` + // doThing() + // .then(a => getB(a) + // .then(b => getC(a, b)) + // .then(c => getD(a, c)) + // ) + // ``` + // This is why we only check the closest callback function scope. + // + const closestCallbackScope = callbackScopes[0] + for (const reference of iterateDefinedReferences( + closestCallbackScope + )) { + if ( + node.arguments.some( + (arg) => + arg.range[0] <= reference.identifier.range[0] && + reference.identifier.range[1] <= arg.range[1] + ) + ) { + // Argument callbacks refer to variables defined in the callback function. + return + } + } + + context.report({ + node: node.callee.property, + message: 'Avoid nesting promises.', + }) }, } },