Merge pull request #97 from kacper-mikolajczak/reduce-no-acc-spread

No accumulator spread in reduce methods

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.',
     },
 };

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,
+                });
+            },
+        };
+    },
+};

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,
+        },
+    ],
+});

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',