diff --git a/eslint-plugin-expensify/CONST.js b/eslint-plugin-expensify/CONST.js index 966d318..7ff5ce1 100644 --- a/eslint-plugin-expensify/CONST.js +++ b/eslint-plugin-expensify/CONST.js @@ -28,5 +28,6 @@ module.exports = { MUST_USE_VARIABLE_FOR_ASSIGNMENT: '{{key}} must be assigned as a variable instead of direct assignment.', NO_DEFAULT_PROPS: 'defaultProps should not be used in function components. Use default Arguments instead.', USE_PERIODS_ERROR_MESSAGES: 'Use periods at the end of error messages.', + NO_ACC_SPREAD_IN_REDUCE: 'Avoid a use of spread (`...`) operator on accumulators in reduce callback. Mutate them directly instead.', }, }; diff --git a/eslint-plugin-expensify/no-acc-spread-in-reduce.js b/eslint-plugin-expensify/no-acc-spread-in-reduce.js new file mode 100644 index 0000000..e34f650 --- /dev/null +++ b/eslint-plugin-expensify/no-acc-spread-in-reduce.js @@ -0,0 +1,48 @@ +const _ = require('lodash'); +const CONST = require('./CONST'); + +// Matches function expression as a direct descendant (argument callback) of "reduce" or "reduceRight" call +const MATCH = 'CallExpression:matches([callee.property.name="reduce"], [callee.property.name="reduceRight"]) > :matches(ArrowFunctionExpression, FunctionExpression)'; + +module.exports = { + meta: { + type: 'problem', + docs: { + description: CONST.MESSAGE.NO_ACC_SPREAD_IN_REDUCE, + category: 'Best Practices', + recommended: false, + }, + schema: [], // no options + }, + create(context) { + return { + [MATCH](node) { + // Retrieve accumulator variable + const accumulator = context.getDeclaredVariables(node)[0]; + if (!accumulator) { + return; + } + + // Check if accumulatorVariable has any references (is used in the scope) + if (!accumulator.references.length) { + return; + } + + // Check if any of the references are used in a SpreadElement + const isAccumulatorVariableUsedSpread = _.some( + accumulator.references, + reference => reference.identifier.parent.type === 'SpreadElement', + ); + if (!isAccumulatorVariableUsedSpread) { + return; + } + + // Accumulator variable is used in a SpreadElement, report it + context.report({ + node, + message: CONST.MESSAGE.NO_ACC_SPREAD_IN_REDUCE, + }); + }, + }; + }, +}; diff --git a/eslint-plugin-expensify/tests/no-acc-spread-in-reduce.test.js b/eslint-plugin-expensify/tests/no-acc-spread-in-reduce.test.js new file mode 100644 index 0000000..23767bd --- /dev/null +++ b/eslint-plugin-expensify/tests/no-acc-spread-in-reduce.test.js @@ -0,0 +1,60 @@ +const RuleTester = require('eslint').RuleTester; +const rule = require('../no-acc-spread-in-reduce'); +const CONST = require('../CONST'); + +const ruleTester = new RuleTester({ + parserOptions: { + ecmaVersion: 2018, + sourceType: 'module', + }, +}); + +const errors = [ + { + message: CONST.MESSAGE.NO_ACC_SPREAD_IN_REDUCE, + }, +]; + +ruleTester.run('no-spread-in-reduce', rule, { + valid: [ + { + code: ` + array.reduce((acc, item) => { + acc[item.key] = item.value; + return acc; + } + , {}); + `, + }, + { + code: ` + array.reduce((acc, item) => { + const spread = { ...somethingElse }; + return acc[item.key] = item.value; + }, {}); + `, + }, + ], + invalid: [ + { + code: ` + const arr = []; + arr.reduce((acc, i) => ({ ...acc, i }), {}) + `, + errors, + }, + { + code: ` + const arr = []; + arr.reduceRight((acc, i) => ({ ...acc, i }), {}) + `, + errors, + }, + { + code: ` + ([]).reduce((acc, i) => ({ ...acc, i }), {}) + `, + errors, + }, + ], +}); diff --git a/rules/expensify.js b/rules/expensify.js index fb5fccd..c484de1 100644 --- a/rules/expensify.js +++ b/rules/expensify.js @@ -15,6 +15,7 @@ module.exports = { 'rulesdir/no-call-actions-from-actions': 'error', 'rulesdir/no-api-side-effects-method': 'error', 'rulesdir/prefer-localization': 'error', + 'rulesdir/no-acc-spread-in-reduce': 'error', 'no-restricted-imports': ['error', { paths: [{ name: 'react-native',