Repository: lo1tuma/eslint-plugin-mocha (ts)

Issue:
handle-done-callback: wrongly detect done callback
I am using nightwatch with mocha, my test function look like this
```js
it('uses BDD to run the Google simple test', function(browser) {
  browser
    .url('http://google.com')
    .expect.element('body').to.be.present.before(1000);

  browser.setValue('input[type=text]', ['nightwatch', browser.Keys.ENTER])
    .pause(1000)
    .assert.containsText('#main', 'Night Watch');
});
```
(More detail about using mocha with nightwatch is [here](https://github.com/nightwatchjs/nightwatch-docs/blob/master/guide/running-tests/using-mocha.md))

When I check with eslint, it says 
`error  Expected "browser" callback to be handled  mocha/handle-done-callback`

I am using
```json
{
  "nightwatch": "^1.1.13",
  "mocha": "^6.2.0",
  "eslint": "^6.1.0",
  "eslint-plugin-mocha": "^6.0.0"
}
```

The checker thinks `browser` is `done` callback, but it's not.
handle-done-callback should ignore skipped tests
`handle-done-callback` should ignore skipped tests, either via an option or by default.

Currently, this type of code fails:
```js
it.skip('test name', function (done) {
  // TODO finish this test
});
```



Pull request:
New option `ignoreSkipped` for `handle-done-callback` rule
Fixes #210 .

Patch:
diff --git a/.eslintrc.json b/.eslintrc.json
index a644354..ede75d7 100644
--- a/.eslintrc.json
+++ b/.eslintrc.json
@@ -20,7 +20,8 @@
             "files": [ "test/**/*.js", "benchmarks/**/*.js" ],
             "env": { "mocha": true },
             "rules": {
-                "max-nested-callbacks": [ "error", 8 ]
+                "max-nested-callbacks": [ "error", 8 ],
+                "max-statements": [ "error", 30 ]
             }
         }
     ]
diff --git a/docs/rules/handle-done-callback.md b/docs/rules/handle-done-callback.md
index 07dd174..5306a6c 100644
--- a/docs/rules/handle-done-callback.md
+++ b/docs/rules/handle-done-callback.md
@@ -59,6 +59,19 @@ before(function (done) {
     });
 });
 ```
+## Options
+
+This rule supports the following options:
+
+* `ignoreSkipped`: When set to `true` skipped test cases won’t be checked. Defaults to `false`.
+
+```json
+{
+    "rules": {
+        "mocha/handle-done-callback": ["error", {"ignoreSkipped": true}]
+    }
+}
+```
 
 ## When Not To Use It
 
diff --git a/lib/rules/handle-done-callback.js b/lib/rules/handle-done-callback.js
index 7f8ed99..49b6d3e 100644
--- a/lib/rules/handle-done-callback.js
+++ b/lib/rules/handle-done-callback.js
@@ -8,9 +8,25 @@ module.exports = {
         type: 'problem',
         docs: {
             description: 'Enforces handling of callbacks for async tests'
-        }
+        },
+        schema: [
+            {
+                type: 'object',
+                properties: {
+                    ignoreSkipped: {
+                        type: 'boolean',
+                        default: false
+                    }
+                },
+                additionalProperties: false
+            }
+        ]
     },
     create(context) {
+        const [ config = {} ] = context.options;
+        const { ignoreSkipped = false } = config;
+        const modifiersToCheck = ignoreSkipped ? [ 'only' ] : [ 'only', 'skip' ];
+
         function isAsyncFunction(functionExpression) {
             return functionExpression.params.length === 1;
         }
@@ -43,7 +59,7 @@ module.exports = {
         }
 
         function check(node) {
-            if (astUtils.hasParentMochaFunctionCall(node) && isAsyncFunction(node)) {
+            if (astUtils.hasParentMochaFunctionCall(node, { modifiers: modifiersToCheck }) && isAsyncFunction(node)) {
                 checkAsyncMochaFunction(node);
             }
         }
diff --git a/lib/util/ast.js b/lib/util/ast.js
index 85db2c1..b36f3ad 100644
--- a/lib/util/ast.js
+++ b/lib/util/ast.js
@@ -6,25 +6,16 @@ const isNil = require('ramda/src/isNil');
 const propEq = require('ramda/src/propEq');
 const pathEq = require('ramda/src/pathEq');
 const find = require('ramda/src/find');
+const { getTestCaseNames, getSuiteNames } = require('./names');
 
 const isDefined = complement(isNil);
 const isCallExpression = both(isDefined, propEq('type', 'CallExpression'));
 
-const describeAliases = [
-    'describe', 'xdescribe', 'describe.only', 'describe.skip',
-    'context', 'xcontext', 'context.only', 'context.skip',
-    'suite', 'xsuite', 'suite.only', 'suite.skip'
-];
 const hooks = [
     'before', 'after', 'beforeEach', 'afterEach', 'beforeAll', 'afterAll',
     'setup', 'teardown', 'suiteSetup', 'suiteTeardown'
 ];
 const suiteConfig = [ 'timeout', 'slow', 'retries' ];
-const testCaseNames = [
-    'it', 'it.only', 'it.skip', 'xit',
-    'test', 'test.only', 'test.skip',
-    'specify', 'specify.only', 'specify.skip', 'xspecify'
-];
 
 function getPropertyName(property) {
     return property.name || property.value;
@@ -38,8 +29,9 @@ function getNodeName(node) {
 }
 
 function isDescribe(node, additionalSuiteNames = []) {
-    return isCallExpression(node) &&
-      describeAliases.concat(additionalSuiteNames).indexOf(getNodeName(node.callee)) > -1;
+    const describeAliases = getSuiteNames({ modifiers: [ 'skip', 'only' ], additionalSuiteNames });
+
+    return isCallExpression(node) && describeAliases.indexOf(getNodeName(node.callee)) > -1;
 }
 
 function isHookIdentifier(node) {
@@ -52,7 +44,10 @@ function isHookCall(node) {
     return isCallExpression(node) && isHookIdentifier(node.callee);
 }
 
-function isTestCase(node) {
+function isTestCase(node, options = {}) {
+    const { modifiers = [ 'skip', 'only' ] } = options;
+
+    const testCaseNames = getTestCaseNames({ modifiers });
     return isCallExpression(node) && testCaseNames.indexOf(getNodeName(node.callee)) > -1;
 }
 
@@ -100,8 +95,8 @@ function isMochaFunctionCall(node, scope) {
     return isTestCase(node) || isDescribe(node) || isHookCall(node);
 }
 
-function hasParentMochaFunctionCall(functionExpression) {
-    return isTestCase(functionExpression.parent) || isHookCall(functionExpression.parent);
+function hasParentMochaFunctionCall(functionExpression, options) {
+    return isTestCase(functionExpression.parent, options) || isHookCall(functionExpression.parent);
 }
 
 function isExplicitUndefined(node) {
diff --git a/lib/util/names.js b/lib/util/names.js
new file mode 100644
index 0000000..1b41f27
--- /dev/null
+++ b/lib/util/names.js
@@ -0,0 +1,77 @@
+'use strict';
+
+const chain = require('ramda/src/chain');
+
+const suiteNames = [
+    'describe',
+    'context',
+    'suite'
+];
+
+const suiteModifiers = {
+    skip: [
+        'describe.skip',
+        'context.skip',
+        'suite.skip',
+        'xdescribe',
+        'xcontext',
+        'xsuite'
+    ],
+    only: [
+        'describe.only',
+        'context.only',
+        'suite.only'
+    ]
+};
+
+const testCaseNames = [
+    'it',
+    'test',
+    'specify'
+];
+
+const testCaseModifiers = {
+    skip: [
+        'it.skip',
+        'test.skip',
+        'specify.skip',
+        'xit',
+        'xspecify'
+    ],
+    only: [
+        'it.only',
+        'test.only',
+        'specify.only'
+    ]
+};
+
+function getTestCaseNames(options = {}) {
+    const { modifiers = [], baseNames = true } = options;
+    const names = baseNames ? testCaseNames : [];
+
+    return names.concat(chain((modifierName) => {
+        if (testCaseModifiers[modifierName]) {
+            return testCaseModifiers[modifierName];
+        }
+
+        return [];
+    }, modifiers));
+}
+
+function getSuiteNames(options = {}) {
+    const { modifiers = [], baseNames = true, additionalSuiteNames = [] } = options;
+    const names = baseNames ? suiteNames.concat(additionalSuiteNames) : [];
+
+    return names.concat(chain((modifierName) => {
+        if (suiteModifiers[modifierName]) {
+            return suiteModifiers[modifierName];
+        }
+
+        return [];
+    }, modifiers));
+}
+
+module.exports = {
+    getTestCaseNames,
+    getSuiteNames
+};


Test patch:
diff --git a/test/rules/handle-done-callback.js b/test/rules/handle-done-callback.js
index fcee12f..6f235ad 100644
--- a/test/rules/handle-done-callback.js
+++ b/test/rules/handle-done-callback.js
@@ -31,6 +31,10 @@ ruleTester.run('handle-done-callback', rules['handle-done-callback'], {
         {
             code: 'it("", (done) => { done(); });',
             parserOptions: { ecmaVersion: 6 }
+        },
+        {
+            code: 'it.skip("", function (done) { });',
+            options: [ { ignoreSkipped: true } ]
         }
     ],
 
@@ -39,6 +43,14 @@ ruleTester.run('handle-done-callback', rules['handle-done-callback'], {
             code: 'it("", function (done) { });',
             errors: [ { message: 'Expected "done" callback to be handled.', column: 18, line: 1 } ]
         },
+        {
+            code: 'it.skip("", function (done) { });',
+            errors: [ { message: 'Expected "done" callback to be handled.', column: 23, line: 1 } ]
+        },
+        {
+            code: 'xit("", function (done) { });',
+            errors: [ { message: 'Expected "done" callback to be handled.', column: 19, line: 1 } ]
+        },
         {
             code: 'it("", function (done) { callback(); });',
             errors: [ { message: 'Expected "done" callback to be handled.', column: 18, line: 1 } ]
diff --git a/test/util/namesSpec.js b/test/util/namesSpec.js
new file mode 100644
index 0000000..545c01c
--- /dev/null
+++ b/test/util/namesSpec.js
@@ -0,0 +1,257 @@
+'use strict';
+
+const { expect } = require('chai');
+const { getTestCaseNames, getSuiteNames } = require('../../lib/util/names');
+
+describe('mocha names', () => {
+    describe('test case names', () => {
+        it('returns the list of basic test case names when no options are provided', () => {
+            const testCaseNames = getTestCaseNames();
+
+            expect(testCaseNames).to.deep.equal([
+                'it',
+                'test',
+                'specify'
+            ]);
+        });
+
+        it('returns an empty list when no modifiers and no base names are wanted', () => {
+            const testCaseNames = getTestCaseNames({ baseNames: false });
+
+            expect(testCaseNames).to.deep.equal([]);
+        });
+
+        it('always returns a new array', () => {
+            const testCaseNames1 = getTestCaseNames({ baseNames: false });
+            const testCaseNames2 = getTestCaseNames({ baseNames: false });
+
+            expect(testCaseNames1).to.deep.equal(testCaseNames2);
+            expect(testCaseNames1).to.not.equal(testCaseNames2);
+        });
+
+        it('ignores invalid modifiers', () => {
+            const testCaseNames = getTestCaseNames({ modifiers: [ 'foo' ], baseNames: false });
+
+            expect(testCaseNames).to.deep.equal([]);
+        });
+
+        it('returns the list of test case names with and without "skip" modifiers applied', () => {
+            const testCaseNames = getTestCaseNames({ modifiers: [ 'skip' ] });
+
+            expect(testCaseNames).to.deep.equal([
+                'it',
+                'test',
+                'specify',
+                'it.skip',
+                'test.skip',
+                'specify.skip',
+                'xit',
+                'xspecify'
+            ]);
+        });
+
+        it('returns the list of test case names only with "skip" modifiers applied', () => {
+            const testCaseNames = getTestCaseNames({ modifiers: [ 'skip' ], baseNames: false });
+
+            expect(testCaseNames).to.deep.equal([
+                'it.skip',
+                'test.skip',
+                'specify.skip',
+                'xit',
+                'xspecify'
+            ]);
+        });
+
+        it('returns the list of test case names with and without "only" modifiers applied', () => {
+            const testCaseNames = getTestCaseNames({ modifiers: [ 'only' ] });
+
+            expect(testCaseNames).to.deep.equal([
+                'it',
+                'test',
+                'specify',
+                'it.only',
+                'test.only',
+                'specify.only'
+            ]);
+        });
+
+        it('returns the list of test case names only with "only" modifiers applied', () => {
+            const testCaseNames = getTestCaseNames({ modifiers: [ 'only' ], baseNames: false });
+
+            expect(testCaseNames).to.deep.equal([
+                'it.only',
+                'test.only',
+                'specify.only'
+            ]);
+        });
+
+        it('returns the list of all test case names', () => {
+            const testCaseNames = getTestCaseNames({ modifiers: [ 'skip', 'only' ] });
+
+            expect(testCaseNames).to.deep.equal([
+                'it',
+                'test',
+                'specify',
+                'it.skip',
+                'test.skip',
+                'specify.skip',
+                'xit',
+                'xspecify',
+                'it.only',
+                'test.only',
+                'specify.only'
+            ]);
+        });
+
+        it('returns the list of test case names only with modifiers applied', () => {
+            const testCaseNames = getTestCaseNames({ modifiers: [ 'skip', 'only' ], baseNames: false });
+
+            expect(testCaseNames).to.deep.equal([
+                'it.skip',
+                'test.skip',
+                'specify.skip',
+                'xit',
+                'xspecify',
+                'it.only',
+                'test.only',
+                'specify.only'
+            ]);
+        });
+    });
+
+    describe('suite names', () => {
+        it('returns the list of basic suite names when no options are provided', () => {
+            const suiteNames = getSuiteNames();
+
+            expect(suiteNames).to.deep.equal([
+                'describe',
+                'context',
+                'suite'
+            ]);
+        });
+
+        it('returns an empty list when no modifiers and no base names are wanted', () => {
+            const suiteNames = getSuiteNames({ baseNames: false });
+
+            expect(suiteNames).to.deep.equal([]);
+        });
+
+        it('always returns a new array', () => {
+            const suiteNames1 = getSuiteNames({ baseNames: false });
+            const suiteNames2 = getSuiteNames({ baseNames: false });
+
+            expect(suiteNames1).to.deep.equal(suiteNames2);
+            expect(suiteNames1).to.not.equal(suiteNames2);
+        });
+
+        it('ignores invalid modifiers', () => {
+            const suiteNames = getSuiteNames({ modifiers: [ 'foo' ], baseNames: false });
+
+            expect(suiteNames).to.deep.equal([]);
+        });
+
+        it('returns the list of suite names with and without "skip" modifiers applied', () => {
+            const suiteNames = getSuiteNames({ modifiers: [ 'skip' ] });
+
+            expect(suiteNames).to.deep.equal([
+                'describe',
+                'context',
+                'suite',
+                'describe.skip',
+                'context.skip',
+                'suite.skip',
+                'xdescribe',
+                'xcontext',
+                'xsuite'
+            ]);
+        });
+
+        it('returns the list of suite names only with "skip" modifiers applied', () => {
+            const suiteNames = getSuiteNames({ modifiers: [ 'skip' ], baseNames: false });
+
+            expect(suiteNames).to.deep.equal([
+                'describe.skip',
+                'context.skip',
+                'suite.skip',
+                'xdescribe',
+                'xcontext',
+                'xsuite'
+            ]);
+        });
+
+        it('returns the list of suite names with and without "only" modifiers applied', () => {
+            const suiteNames = getSuiteNames({ modifiers: [ 'only' ] });
+
+            expect(suiteNames).to.deep.equal([
+                'describe',
+                'context',
+                'suite',
+                'describe.only',
+                'context.only',
+                'suite.only'
+            ]);
+        });
+
+        it('returns the list of suite names only with "only" modifiers applied', () => {
+            const suiteNames = getSuiteNames({ modifiers: [ 'only' ], baseNames: false });
+
+            expect(suiteNames).to.deep.equal([
+                'describe.only',
+                'context.only',
+                'suite.only'
+            ]);
+        });
+
+        it('returns the list of all suite names', () => {
+            const suiteNames = getSuiteNames({ modifiers: [ 'skip', 'only' ] });
+
+            expect(suiteNames).to.deep.equal([
+                'describe',
+                'context',
+                'suite',
+                'describe.skip',
+                'context.skip',
+                'suite.skip',
+                'xdescribe',
+                'xcontext',
+                'xsuite',
+                'describe.only',
+                'context.only',
+                'suite.only'
+            ]);
+        });
+
+        it('returns the list of suite names names only with modifiers applied', () => {
+            const suiteNames = getSuiteNames({ modifiers: [ 'skip', 'only' ], baseNames: false });
+
+            expect(suiteNames).to.deep.equal([
+                'describe.skip',
+                'context.skip',
+                'suite.skip',
+                'xdescribe',
+                'xcontext',
+                'xsuite',
+                'describe.only',
+                'context.only',
+                'suite.only'
+            ]);
+        });
+
+        it('returns the additional suite names', () => {
+            const suiteNames = getSuiteNames({ additionalSuiteNames: [ 'myCustomDescribe' ] });
+
+            expect(suiteNames).to.deep.equal([
+                'describe',
+                'context',
+                'suite',
+                'myCustomDescribe'
+            ]);
+        });
+
+        it('doesn’t return the additional suite names when base names shouldn’t be included', () => {
+            const suiteNames = getSuiteNames({ additionalSuiteNames: [ 'myCustomDescribe' ], baseNames: false });
+
+            expect(suiteNames).to.deep.equal([]);
+        });
+    });
+});