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

Issue:
pretest fails on fresh clone of repo
When cloning this repo, running `npm install` and then `npm test`, the pretest script fails with 7 errors.

```
❯ npm test

> eslint-plugin-mocha@4.11.0 pretest /Users/macklinu/dev/node/eslint-plugin-mocha
> eslint .


/Users/macklinu/dev/node/eslint-plugin-mocha/lib/rules/no-mocha-arrows.js
  81:25  error  Method 'CallExpression' has a complexity of 6  complexity

/Users/macklinu/dev/node/eslint-plugin-mocha/lib/rules/no-nested-tests.js
  21:25  error  Method 'CallExpression' has a complexity of 5  complexity

/Users/macklinu/dev/node/eslint-plugin-mocha/lib/rules/no-return-and-callback.js
  48:1  error  Function 'isAllowedReturnStatement' has a complexity of 7  complexity

/Users/macklinu/dev/node/eslint-plugin-mocha/lib/rules/no-sibling-hooks.js
  25:25  error  Method 'CallExpression' has a complexity of 5  complexity

/Users/macklinu/dev/node/eslint-plugin-mocha/lib/rules/no-skipped-tests.js
  69:25  error  Method 'CallExpression' has a complexity of 5  complexity

/Users/macklinu/dev/node/eslint-plugin-mocha/lib/rules/valid-suite-description.js
  15:25  error  Method 'CallExpression' has a complexity of 8  complexity

/Users/macklinu/dev/node/eslint-plugin-mocha/lib/rules/valid-test-description.js
  15:25  error  Method 'CallExpression' has a complexity of 8  complexity

✖ 7 problems (7 errors, 0 warnings)

npm ERR! Test failed.  See above for more details.
```

Are these errors that should be fixed, or is this an ESLint rule that should be modified? I'd like to contribute a PR (both for this and other features), but not being able to run the `npm test` script locally is a blocker. Suggestions for next steps would be helpful - would be glad to open a PR. Thanks!

```
eslint-plugin-mocha git/master
❯ node --version
v8.9.4

eslint-plugin-mocha git/master
❯ npm --version
5.6.0
```

Pull request:
Fix complexity problems
This refactors some rules and ast utils to reduce the cyclomatic complexity.
Fixes #148.

Patch:
diff --git a/lib/rules/handle-done-callback.js b/lib/rules/handle-done-callback.js
index 5f49d02..6b71bcb 100644
--- a/lib/rules/handle-done-callback.js
+++ b/lib/rules/handle-done-callback.js
@@ -1,38 +1,11 @@
 'use strict';
 
-var R = require('ramda');
+var R = require('ramda'),
+    astUtils = require('../util/ast');
 
 module.exports = function (context) {
-    var possibleAsyncFunctionNames = [
-        'it',
-        'it.only',
-        'test',
-        'test.only',
-        'specify',
-        'specify.only',
-        'before',
-        'after',
-        'beforeEach',
-        'afterEach'
-    ];
-
-    function getCalleeName(callee) {
-        if (callee.type === 'MemberExpression') {
-             return callee.object.name + '.' + callee.property.name;
-        }
-
-        return callee.name;
-    }
-
     function hasParentMochaFunctionCall(functionExpression) {
-        var name;
-
-        if (functionExpression.parent && functionExpression.parent.type === 'CallExpression') {
-            name = getCalleeName(functionExpression.parent.callee);
-            return possibleAsyncFunctionNames.indexOf(name) > -1;
-        }
-
-        return false;
+        return astUtils.isTestCase(functionExpression.parent) || astUtils.isHookCall(functionExpression.parent);
     }
 
     function isAsyncFunction(functionExpression) {
diff --git a/lib/rules/no-mocha-arrows.js b/lib/rules/no-mocha-arrows.js
index 3f401c2..8a9ab92 100644
--- a/lib/rules/no-mocha-arrows.js
+++ b/lib/rules/no-mocha-arrows.js
@@ -6,41 +6,9 @@
  */
 
 var R = require('ramda'),
-    mochaFunctionNames = [
-        'describe',
-        'describe.skip',
-        'xdescribe',
-        'suite',
-        'context',
-        'xcontext',
-        'specify',
-        'xspecify',
-        'it',
-        'it.only',
-        'xit',
-        'test',
-        'test.only',
-        'specify',
-        'specify.only',
-        'before',
-        'after',
-        'beforeEach',
-        'afterEach'
-    ];
+    astUtils = require('../util/ast');
 
 module.exports = function (context) {
-    function getCalleeName(callee) {
-        if (callee.type === 'MemberExpression') {
-             return callee.object.name + '.' + callee.property.name;
-        }
-
-        return callee.name;
-    }
-
-    function isLikelyMochaGlobal(scope, name) {
-        return !R.find(R.propEq('name', name), scope.variables);
-    }
-
     function fixArrowFunction(fixer, fn) {
         var sourceCode = context.getSourceCode(),
             paramsLeftParen = sourceCode.getFirstToken(fn),
@@ -79,21 +47,19 @@ module.exports = function (context) {
 
     return {
         CallExpression: function (node) {
-            var name = getCalleeName(node.callee),
+            var name = astUtils.getNodeName(node.callee),
                 fnArg;
 
-            if (name && mochaFunctionNames.indexOf(name) > -1) {
+            if (astUtils.isMochaFunctionCall(node, context.getScope())) {
                 fnArg = node.arguments.slice(-1)[0];
                 if (fnArg && fnArg.type === 'ArrowFunctionExpression') {
-                    if (isLikelyMochaGlobal(context.getScope(), name)) {
-                        context.report({
-                            node: node,
-                            message: 'Do not pass arrow functions to ' + name + '()',
-                            fix: function (fixer) {
-                                return fixArrowFunction(fixer, fnArg);
-                            }
-                        });
-                    }
+                    context.report({
+                        node: node,
+                        message: 'Do not pass arrow functions to ' + name + '()',
+                        fix: function (fixer) {
+                            return fixArrowFunction(fixer, fnArg);
+                        }
+                    });
                 }
             }
         }
diff --git a/lib/rules/no-return-and-callback.js b/lib/rules/no-return-and-callback.js
index a3c193c..0d4958e 100644
--- a/lib/rules/no-return-and-callback.js
+++ b/lib/rules/no-return-and-callback.js
@@ -1,37 +1,11 @@
 'use strict';
 
 var R = require('ramda'),
-    findReturnStatement = R.find(R.propEq('type', 'ReturnStatement')),
-    possibleAsyncFunctionNames = [
-        'it',
-        'it.only',
-        'test',
-        'test.only',
-        'specify',
-        'specify.only',
-        'before',
-        'after',
-        'beforeEach',
-        'afterEach'
-    ];
-
-function getCalleeName(callee) {
-    if (callee.type === 'MemberExpression') {
-        return callee.object.name + '.' + callee.property.name;
-    }
-
-    return callee.name;
-}
+    astUtils = require('../util/ast'),
+    findReturnStatement = R.find(R.propEq('type', 'ReturnStatement'));
 
 function hasParentMochaFunctionCall(functionExpression) {
-    var name;
-
-    if (functionExpression.parent && functionExpression.parent.type === 'CallExpression') {
-        name = getCalleeName(functionExpression.parent.callee);
-        return possibleAsyncFunctionNames.indexOf(name) > -1;
-    }
-
-    return false;
+    return astUtils.isTestCase(functionExpression.parent) || astUtils.isHookCall(functionExpression.parent);
 }
 
 function reportIfShortArrowFunction(context, node) {
@@ -45,17 +19,31 @@ function reportIfShortArrowFunction(context, node) {
     return false;
 }
 
+function isExplicitUndefined(node) {
+    return node && node.type === 'Identifier' && node.name === 'undefined';
+}
+
+function isReturnOfUndefined(node) {
+    var argument = node.argument,
+        isImplicitUndefined = argument === null;
+
+    return isImplicitUndefined || isExplicitUndefined(argument);
+}
+
+function isFunctionCallWithName(node, name) {
+    return node.type === 'CallExpression' &&
+        node.callee.type === 'Identifier' &&
+        node.callee.name === name;
+}
+
 function isAllowedReturnStatement(node, doneName) {
     var argument = node.argument;
-    if (argument === null || argument.type === 'Literal') {
-        return true;
-    }
-    if (argument.type === 'Identifier' && argument.name === 'undefined') {
+
+    if (isReturnOfUndefined(node) || argument.type === 'Literal') {
         return true;
     }
-    return argument.type === 'CallExpression' &&
-        argument.callee.type === 'Identifier' &&
-        argument.callee.name === doneName;
+
+    return isFunctionCallWithName(argument, doneName);
 }
 
 function reportIfFunctionWithBlock(context, node, doneName) {
diff --git a/lib/rules/no-sibling-hooks.js b/lib/rules/no-sibling-hooks.js
index 700d82f..b2a65c4 100644
--- a/lib/rules/no-sibling-hooks.js
+++ b/lib/rules/no-sibling-hooks.js
@@ -23,7 +23,8 @@ module.exports = function (context) {
         },
 
         CallExpression: function (node) {
-            var name = node.callee && node.callee.name;
+            var name = astUtil.getNodeName(node.callee);
+
             if (astUtil.isDescribe(node, additionalSuiteNames(settings))) {
               isUsed.push(newDescribeLayer(node));
               return;
diff --git a/lib/rules/valid-suite-description.js b/lib/rules/valid-suite-description.js
index f7d2c94..b633a29 100644
--- a/lib/rules/valid-suite-description.js
+++ b/lib/rules/valid-suite-description.js
@@ -5,20 +5,37 @@
  * @author Alexander Afanasyev
  */
 
-var defaultSuiteNames = [ 'describe', 'context', 'suite' ];
+var defaultSuiteNames = [ 'describe', 'context', 'suite' ],
+    astUtils = require('../util/ast');
 
 module.exports = function (context) {
     var pattern = new RegExp(context.options[0]),
         suiteNames = context.options[1] ? context.options[1] : defaultSuiteNames;
 
+    function isSuite(node) {
+        return node.callee && node.callee.name && suiteNames.indexOf(node.callee.name) > -1;
+    }
+
+    function hasValidSuiteDescription(mochaCallExpression) {
+        var args = mochaCallExpression.arguments,
+            description = args[0];
+
+        return astUtils.isStringLiteral(description) && pattern.test(description.value);
+    }
+
+    function hasValidOrNoSuiteDescription(mochaCallExpression) {
+        var args = mochaCallExpression.arguments,
+            hasNoSuiteDescription = args.length === 0;
+
+        return hasNoSuiteDescription || hasValidSuiteDescription(mochaCallExpression);
+    }
+
     return {
         CallExpression: function (node) {
-            var callee = node.callee,
-                args;
+            var callee = node.callee;
 
-            if (callee && callee.name && suiteNames.indexOf(callee.name) > -1) {
-                args = node.arguments;
-                if (args && args[0] && typeof args[0].value === 'string' && !pattern.test(args[0].value)) {
+            if (isSuite(node)) {
+                if (!hasValidOrNoSuiteDescription(node)) {
                     context.report(node, 'Invalid "' + callee.name + '()" description found.');
                 }
             }
diff --git a/lib/util/ast.js b/lib/util/ast.js
index 176adf3..dfb8d13 100644
--- a/lib/util/ast.js
+++ b/lib/util/ast.js
@@ -1,14 +1,15 @@
-/* eslint-env node*/
-
 'use strict';
 
-var describeAliases = [ 'describe', 'xdescribe', 'describe.only', 'describe.skip',
+var R = require('ramda'),
+    isDefined = R.complement(R.isNil),
+    isCallExpression = R.both(isDefined, R.propEq('type', 'CallExpression')),
+    describeAliases = [ 'describe', 'xdescribe', 'describe.only', 'describe.skip',
                         'context', 'xcontext', 'context.only', 'context.skip',
                         'suite', 'xsuite', 'suite.only', 'suite.skip' ],
     hooks = [ 'before', 'after', 'beforeEach', 'afterEach' ],
-    testCaseNames = [ 'it', 'it.only', 'it.skip',
+    testCaseNames = [ 'it', 'it.only', 'it.skip', 'xit',
                       'test', 'test.only', 'test.skip',
-                      'specify', 'specify.only', 'specify.skip' ];
+                      'specify', 'specify.only', 'specify.skip', 'xspecify' ];
 
 function getPropertyName(property) {
     return property.name || property.value;
@@ -22,8 +23,7 @@ function getNodeName(node) {
 }
 
 function isDescribe(node, additionalSuiteNames) {
-  return node
-      && node.type === 'CallExpression'
+  return isCallExpression(node)
       && describeAliases.concat(additionalSuiteNames).indexOf(getNodeName(node.callee)) > -1;
 }
 
@@ -33,10 +33,42 @@ function isHookIdentifier(node) {
       && hooks.indexOf(node.name) !== -1;
 }
 
+function isHookCall(node) {
+    return isCallExpression(node) && isHookIdentifier(node.callee);
+}
+
 function isTestCase(node) {
-    return node
-        && node.type === 'CallExpression'
-        && testCaseNames.indexOf(getNodeName(node.callee)) > -1;
+    return isCallExpression(node) && testCaseNames.indexOf(getNodeName(node.callee)) > -1;
+}
+
+function findReference(scope, node) {
+    var hasSameRangeAsNode = R.pathEq([ 'identifier', 'range' ], node.range);
+
+    return R.find(hasSameRangeAsNode, scope.references);
+}
+
+function isShadowed(scope, identifier) {
+    var reference = findReference(scope, identifier);
+
+    return reference && reference.resolved && reference.resolved.defs.length > 0;
+}
+
+function isCallToShadowedReference(node, scope) {
+    var identifier = node.callee.type === 'MemberExpression' ? node.callee.object : node.callee;
+
+    return isShadowed(scope, identifier);
+}
+
+function isMochaFunctionCall(node, scope) {
+    if (isCallToShadowedReference(node, scope)) {
+        return false;
+    }
+
+    return isTestCase(node) || isDescribe(node) || isHookCall(node);
+}
+
+function isStringLiteral(node) {
+    return node && node.type === 'Literal' && typeof node.value === 'string';
 }
 
 module.exports = {
@@ -44,5 +76,8 @@ module.exports = {
   isHookIdentifier: isHookIdentifier,
   isTestCase: isTestCase,
   getPropertyName: getPropertyName,
-  getNodeName: getNodeName
+  getNodeName: getNodeName,
+  isMochaFunctionCall: isMochaFunctionCall,
+  isHookCall: isHookCall,
+  isStringLiteral: isStringLiteral
 };


Test patch:
diff --git a/lib/rules/no-global-tests.js b/lib/rules/no-global-tests.js
index cd7ab33..17a853d 100644
--- a/lib/rules/no-global-tests.js
+++ b/lib/rules/no-global-tests.js
@@ -1,30 +1,8 @@
 'use strict';
 
-module.exports = function (context) {
-    var testFunctionNames = [
-            'it',
-            'it.only',
-            'it.skip',
-            'test',
-            'test.only',
-            'test.skip',
-            'specify',
-            'specify.only',
-            'specify.skip'
-        ];
-
-    function getFnName(callee) {
-        if (callee.type === 'MemberExpression') {
-            if (callee.computed) {
-                return callee.object.name + '.' + callee.property.value;
-            }
-
-            return callee.object.name + '.' + callee.property.name;
-        }
-
-        return callee.name;
-    }
+var astUtils = require('../util/ast');
 
+module.exports = function (context) {
     function isGlobalScope(scope) {
         return scope.type === 'global' || scope.type === 'module';
     }
@@ -32,10 +10,9 @@ module.exports = function (context) {
     return {
         CallExpression: function (node) {
             var callee = node.callee,
-                fnName = getFnName(callee),
                 scope = context.getScope();
 
-            if (testFunctionNames.indexOf(fnName) !== -1 && isGlobalScope(scope)) {
+            if (astUtils.isTestCase(node) && isGlobalScope(scope)) {
                 context.report(callee, 'Unexpected global mocha test.');
             }
         }
diff --git a/lib/rules/no-nested-tests.js b/lib/rules/no-nested-tests.js
index d7fe8b7..c3bac0e 100644
--- a/lib/rules/no-nested-tests.js
+++ b/lib/rules/no-nested-tests.js
@@ -17,12 +17,19 @@ module.exports = function noNestedTests(context) {
         });
     }
 
+    function isNestedTest(isTestCase, isDescribe) {
+        var isNested = testNestingLevel > 0,
+            isTest = isTestCase || isDescribe;
+
+        return isNested && isTest;
+    }
+
     return {
         CallExpression: function (node) {
             var isTestCase = astUtils.isTestCase(node),
                 isDescribe = astUtils.isDescribe(node, additionalSuiteNames(settings));
 
-            if (testNestingLevel > 0 && (isTestCase || isDescribe)) {
+            if (isNestedTest(isTestCase, isDescribe)) {
                 report(node, isTestCase);
             }
 
diff --git a/lib/rules/no-pending-tests.js b/lib/rules/no-pending-tests.js
index 8df0db7..fb7dde1 100644
--- a/lib/rules/no-pending-tests.js
+++ b/lib/rules/no-pending-tests.js
@@ -1,19 +1,10 @@
 'use strict';
 
-module.exports = function (context) {
-    var mochaTestFunctionNames = [
-        'it',
-        'test',
-        'specify'
-    ];
-
-    function isMochaTest(callee) {
-        return callee.type === 'Identifier' &&
-            mochaTestFunctionNames.indexOf(callee.name) !== -1;
-    }
+var astUtils = require('../util/ast');
 
+module.exports = function (context) {
     function isPendingMochaTest(node) {
-        return isMochaTest(node.callee) &&
+        return astUtils.isTestCase(node) &&
             node.arguments.length === 1 &&
             node.arguments[0].type === 'Literal';
     }
diff --git a/lib/rules/no-skipped-tests.js b/lib/rules/no-skipped-tests.js
index 0438902..6432eee 100644
--- a/lib/rules/no-skipped-tests.js
+++ b/lib/rules/no-skipped-tests.js
@@ -69,13 +69,13 @@ module.exports = function (context) {
         CallExpression: function (node) {
             var callee = node.callee;
 
-            if (callee && isCallToMochasSkipFunction(callee)) {
+            if (isCallToMochasSkipFunction(callee)) {
                 context.report({
                     node: callee.property,
                     message: 'Unexpected skipped mocha test.',
                     fix: createSkipAutofixFunction(callee)
                 });
-            } else if (callee && isCallToMochaXFunction(callee)) {
+            } else if (isCallToMochaXFunction(callee)) {
                 context.report({
                     node: callee,
                     message: 'Unexpected skipped mocha test.',
diff --git a/lib/rules/valid-test-description.js b/lib/rules/valid-test-description.js
index 1021bdc..9024fff 100644
--- a/lib/rules/valid-test-description.js
+++ b/lib/rules/valid-test-description.js
@@ -5,20 +5,37 @@
  * @author Alexander Afanasyev
  */
 
-var defaultTestNames = [ 'it', 'test', 'specify' ];
+var defaultTestNames = [ 'it', 'test', 'specify' ],
+    astUtils = require('../util/ast');
 
 module.exports = function (context) {
     var pattern = context.options[0] ? new RegExp(context.options[0]) : /^should/,
         testNames = context.options[1] ? context.options[1] : defaultTestNames;
 
+    function isTest(node) {
+        return node.callee && node.callee.name && testNames.indexOf(node.callee.name) > -1;
+    }
+
+    function hasValidTestDescription(mochaCallExpression) {
+        var args = mochaCallExpression.arguments,
+            testDescriptionArgument = args[0];
+
+        return astUtils.isStringLiteral(testDescriptionArgument) && pattern.test(testDescriptionArgument.value);
+    }
+
+    function hasValidOrNoTestDescription(mochaCallExpression) {
+        var args = mochaCallExpression.arguments,
+            hasNoTestDescription = args.length === 0;
+
+        return hasNoTestDescription || hasValidTestDescription(mochaCallExpression);
+    }
+
     return {
         CallExpression: function (node) {
-            var callee = node.callee,
-                args;
+            var callee = node.callee;
 
-            if (callee && callee.name && testNames.indexOf(callee.name) > -1) {
-                args = node.arguments;
-                if (args && args[0] && typeof args[0].value === 'string' && !pattern.test(args[0].value)) {
+            if (isTest(node)) {
+                if (!hasValidOrNoTestDescription(node)) {
                     context.report(node, 'Invalid "' + callee.name + '()" description found.');
                 }
             }
diff --git a/test/rules/no-mocha-arrows.js b/test/rules/no-mocha-arrows.js
index 7ca807e..fe84109 100644
--- a/test/rules/no-mocha-arrows.js
+++ b/test/rules/no-mocha-arrows.js
@@ -14,12 +14,15 @@ ruleTester.run('no-mocha-arrows', rules['no-mocha-arrows'], {
         'it()',
         'it(function() { assert(something, false); })',
         'it("should be false", function() { assert(something, false); })',
-         // In this example, `it` is not a global.
-        'function it () {}; it(() => { console.log("okay") })',
         'it.only()',
         'it(function(done) { assert(something, false); done(); })',
         'it(function*() { assert(something, false) })',
-        'it(async function () { assert(something, false) })'
+        'it(async function () { assert(something, false) })',
+
+         // In those examples, `it` is not a global.
+        'function it () {}; it(() => { console.log("okay") })',
+        'function it () {}; it.only(() => { console.log("okay") })',
+        'function it () {}; it(() => {}); it(() => {});'
     ],
 
     invalid: [