Repository: lo1tuma/eslint-plugin-mocha
Pull request #147 (2018-02-02 15:23:57)

Adds rule "no setup in describe"
Setup for test cases in mocha should be done in `before`, `beforeEach`, or `it` blocks. Unfortunately there is nothing stopping you from doing setup directly inside a `describe` block and it's not immediately obvious setup should be inside a `before` or `beforeEach`.

This rule catches many cases where setup is accidentally done directly in a describe block and promotes the use of the hooks `before` and `beforeEach`.

This rule's code is a little more complex than I'd like because it does what it can to minimize processing time outside a describe block (`Array.length` check only).

This rule should never be activated by default because it blocks the use of dynamically generated tests.

donabrams: I've addressed everything I don't have questions about.
lo1tuma: Hi @donabrams, sorry for the late feedback. This looks almost good to me, only a rebase is missing and the removal of the `.eslintrc` changes 😉.

Patch:
diff --git a/docs/rules/README.md b/docs/rules/README.md
index 8c16752..1b1a8ad 100644
--- a/docs/rules/README.md
+++ b/docs/rules/README.md
@@ -11,6 +11,7 @@
 * [no-nested-tests](no-nested-tests.md) - disallow tests to be nested within other tests
 * [no-pending-tests](no-pending-tests.md) - disallow pending/unimplemented mocha tests
 * [no-return-and-callback](no-return-and-callback.md) - disallow returning in a test or hook function that uses a callback
+* [no-setup-in-describe](no-setup-in-describe.md) - disallow calling functions and dot operaters directly in describe blocks
 * [no-sibling-hooks](no-sibling-hooks.md) - disallow duplicate uses of a hook at the same level inside a describe
 * [no-skipped-tests](no-skipped-tests.md) - disallow skipped mocha tests (fixable)
 * [no-synchronous-tests](no-synchronous-tests.md) - disallow synchronous tests
diff --git a/docs/rules/no-setup-in-describe.md b/docs/rules/no-setup-in-describe.md
new file mode 100644
index 0000000..6a26ac1
--- /dev/null
+++ b/docs/rules/no-setup-in-describe.md
@@ -0,0 +1,71 @@
+
+# Disallow setup in describe blocks (no-setup-in-describe)
+
+Setup for test cases in mocha should be done in `before`, `beforeEach`, or `it` blocks. Unfortunately there is nothing stopping you from doing setup directly inside a `describe` block.
+
+```js
+describe('something', function () {
+    const a = setup();
+    const b = a.getter;
+    it('should work', function () {
+        assert(a === b);
+    });
+});
+```
+
+Any setup directly in a `describe` is run before all tests execute. This is undesirable primarily for two reasons:
+
+1. When doing TDD in a large codebase, all setup is run for tests that don't have `only` set. This can add a substantial amount of time per iteration.
+2. If global state is altered by the setup of another describe block, your test may be affected.
+
+This rule reports all function calls and use of the dot operator (due to getters and setters) directly in describe blocks.
+
+If you're using [dynamically generated tests](https://mochajs.org/#dynamically-generating-tests), you should disable this rule.
+
+## Rule Details
+
+This rule looks for all function calls and use of the dot operator which are nested directly in a describe block.
+
+The following patterns are considered problems:
+
+```js
+describe('something', function () {
+    let a = b.c;
+    it('should work', function () {});
+});
+
+describe('something', function () {
+    const a = setup();
+    it('should work', function () {});
+});
+
+```
+
+These patterns would not be considered problems:
+
+```js
+describe('something', function () {
+    var a;
+    beforeEach(function() {
+        a = setup();
+    });
+    it('should work', function () {});
+});
+function getTest() {
+    var a = setup(),
+        b = a.someGetter;
+    describe('something', function() {
+        it ('should work', function() {});
+    });
+}
+describe('something', function () {
+    function setup() {
+        const a = setup(),
+            b = a.someGetter;
+        return { a, b };
+    }
+    it('should work', function () {
+        const { a, b } = setup();
+    });
+});
+```
diff --git a/index.js b/index.js
index 9d776b0..67452ad 100644
--- a/index.js
+++ b/index.js
@@ -18,7 +18,8 @@ module.exports = {
         'no-top-level-hooks': require('./lib/rules/no-top-level-hooks'),
         'no-identical-title': require('./lib/rules/no-identical-title'),
         'max-top-level-suites': require('./lib/rules/max-top-level-suites'),
-        'no-nested-tests': require('./lib/rules/no-nested-tests')
+        'no-nested-tests': require('./lib/rules/no-nested-tests'),
+        'no-setup-in-describe': require('./lib/rules/no-setup-in-describe')
     },
     configs: {
         recommended: {
diff --git a/lib/rules/no-setup-in-describe.js b/lib/rules/no-setup-in-describe.js
new file mode 100644
index 0000000..7b76826
--- /dev/null
+++ b/lib/rules/no-setup-in-describe.js
@@ -0,0 +1,99 @@
+'use strict';
+
+var astUtils = require('../util/ast'),
+    additionalSuiteNames = require('../util/settings').additionalSuiteNames;
+
+module.exports = function noSetupInDescribe(context) {
+    var nesting = [],
+        settings = context.settings,
+        FUNCTION = 1,
+        DESCRIBE = 2,
+        // "Pure" nodes are hooks (like `beforeEach`) or `it` calls
+        PURE = 3;
+
+    function isPureNode(node) {
+        return astUtils.isHookIdentifier(node) || astUtils.isTestCase(node);
+    }
+
+    function reportCallExpression(callExpression) {
+        var message = 'Unexpected function call in describe block.';
+
+        context.report({
+            message: message,
+            node: callExpression.callee
+        });
+    }
+
+    function reportMemberExpression(memberExpression) {
+        var message = 'Unexpected member expression in describe block. ' +
+            'Member expressions may call functions via getters.';
+
+        context.report({
+            message: message,
+            node: memberExpression
+        });
+    }
+
+    function isNestedInDescribeBlock() {
+        return nesting.length &&
+                nesting.indexOf(PURE) === -1 &&
+                nesting.lastIndexOf(FUNCTION) < nesting.lastIndexOf(DESCRIBE);
+    }
+
+    function handleCallExpressionInDescribe(node) {
+        if (isPureNode(node)) {
+            nesting.push(PURE);
+        } else if (isNestedInDescribeBlock()) {
+            reportCallExpression(node);
+        }
+    }
+
+    return {
+        CallExpression: function (node) {
+            var isDescribe = astUtils.isDescribe(node, additionalSuiteNames(settings));
+            if (isDescribe) {
+                nesting.push(DESCRIBE);
+                return;
+            }
+            // don't process anything else if the first describe hasn't been processed
+            if (!nesting.length) {
+                return;
+            }
+            handleCallExpressionInDescribe(node);
+        },
+
+        'CallExpression:exit': function (node) {
+            if (astUtils.isDescribe(node) || nesting.length && isPureNode(node)) {
+                nesting.pop();
+            }
+        },
+
+        MemberExpression: function (node) {
+            if (isNestedInDescribeBlock()) {
+                reportMemberExpression(node);
+            }
+        },
+
+        FunctionDeclaration: function () {
+            if (nesting.length) {
+                nesting.push(FUNCTION);
+            }
+        },
+        'FunctionDeclaration:exit': function () {
+            if (nesting.length) {
+                nesting.pop();
+            }
+        },
+
+        ArrowFunctionExpression: function () {
+            if (nesting.length) {
+                nesting.push(FUNCTION);
+            }
+        },
+        'ArrowFunctionExpression:exit': function () {
+            if (nesting.length) {
+                nesting.pop();
+            }
+        }
+    };
+};


Test patch:
diff --git a/test/rules/no-setup-in-describe.js b/test/rules/no-setup-in-describe.js
new file mode 100644
index 0000000..24df3f8
--- /dev/null
+++ b/test/rules/no-setup-in-describe.js
@@ -0,0 +1,119 @@
+'use strict';
+
+var RuleTester = require('eslint').RuleTester,
+    rule = require('../../lib/rules/no-setup-in-describe'),
+    ruleTester = new RuleTester(),
+    memberExpressionError = 'Unexpected member expression in describe block. ' +
+        'Member expressions may call functions via getters.';
+
+ruleTester.run('no-setup-in-describe', rule, {
+    valid: [
+        'it()',
+        'it(); it(); it()',
+        'a.b',
+        'b()',
+        'function g() { a() }',
+        { code: '() => { a.b }', parserOptions: { ecmaVersion: 6 } },
+        'it("", function () { b(); })',
+        'it("", function () { a.b; })',
+        'it("", function () { a[b]; })',
+        'it("", function () { a["b"]; })',
+        'describe("", function () { it(); })',
+        'describe("", function () { it("", function () { b(); }); })',
+        'describe("", function () { it("", function () { a.b; }); })',
+        'describe("", function () { it("", function () { a[b]; }); })',
+        'describe("", function () { it("", function () { a["b"]; }); })',
+        'describe("", function () { function a() { b(); }; it(); })',
+        'describe("", function () { function a() { b.c; }; it(); })',
+        { code: 'describe("", function () { var a = () => b(); it(); })', parserOptions: { ecmaVersion: 6 } },
+        { code: 'describe("", function () { var a = () => b.c; it(); })', parserOptions: { ecmaVersion: 6 } },
+        'describe("", function () { describe("", function () { it(); }); it(); })',
+        {
+            code: 'foo("", function () { it(); })',
+            settings: {
+                'mocha/additionalSuiteNames': [ 'foo' ]
+            }
+        }, {
+            code: 'foo("", function () { it(); })',
+            settings: {
+                mocha: {
+                   additionalSuiteNames: [ 'foo' ]
+               }
+           }
+        }, {
+            code: 'foo("", function () { it("", function () { b(); }); })',
+            settings: {
+                mocha: {
+                   additionalSuiteNames: [ 'foo' ]
+               }
+           }
+        }
+    ],
+
+    invalid: [
+        {
+            code: 'describe("", function () { a(); });',
+            errors: [ {
+                message: 'Unexpected function call in describe block.',
+                line: 1,
+                column: 28
+            } ]
+        }, {
+            code: 'foo("", function () { a(); });',
+            settings: {
+                mocha: {
+                   additionalSuiteNames: [ 'foo' ]
+               }
+            },
+            errors: [ {
+                message: 'Unexpected function call in describe block.',
+                line: 1,
+                column: 23
+            } ]
+        }, {
+            code: 'foo("", function () { a[b]; });',
+            settings: {
+                mocha: {
+                   additionalSuiteNames: [ 'foo' ]
+               }
+            },
+            errors: [ {
+                message: memberExpressionError,
+                line: 1,
+                column: 23
+            } ]
+        }, {
+            code: 'foo("", function () { a["b"]; });',
+            settings: {
+                mocha: {
+                   additionalSuiteNames: [ 'foo' ]
+               }
+            },
+            errors: [ {
+                message: memberExpressionError,
+                line: 1,
+                column: 23
+            } ]
+        },
+        {
+            code: 'describe("", function () { a.b; });',
+            errors: [ {
+                message: memberExpressionError,
+                line: 1,
+                column: 28
+            } ]
+        }, {
+            code: 'foo("", function () { a.b; });',
+            settings: {
+                mocha: {
+                   additionalSuiteNames: [ 'foo' ]
+               }
+            },
+            errors: [ {
+                message: memberExpressionError,
+                line: 1,
+                column: 23
+            } ]
+        }
+    ]
+});