No expressions in assertions option (#15)

* - Enhancement: Add `settings.mocha-cleanup.skipSkipped` for default behavior across rules - npm: Create separate script for linting * - Testing: Add nyc for coverage - npm: Add .nyc_output and .ncurc.js to ignore * - Testing: Improve coverage - Fix(`no-expressions-in-assertions`): Swap `isDefined` vs. `isUndefined` expectation * - Enhancement (`no-expressions-in-assertions`): Add `replacementsOnly` option which only reports when replacements are possible (or if there are empty arguments) and not for regular cases where there are no suggested replacements.
onechiporenko · Feb 28, 2020 · 9adea3d · 9adea3d
1 parent f31d66b
commit 9adea3d
Show file tree

Hide file tree

Showing 35 changed files with 1,243 additions and 58 deletions.
diff --git a/.eslintrc.json b/.eslintrc.json
@@ -3,6 +3,10 @@
   "rules": {
     "quotes": [2, "double"]
   },
+  "overrides": [{
+    "files": "tests/lib/utils/**",
+    "env": {"mocha": true}
+  }],
   "env": {
     "browser": true,
     "node": true

diff --git a/.npmignore b/.npmignore
@@ -1 +1,3 @@
 tests
+.nyc_output
+.ncurc.js
diff --git a/README.md b/README.md
@@ -23,29 +23,29 @@ $ npm install eslint-plugin-mocha-cleanup --save-dev
 
 ## Supported Rules
 
-Almost each rule (unless otherwise indicated) may be customized to ignore skipped tests/suites (`describe.skip`, `it.skip`, `xspecify` etc) with adding `skipSkipped: true` to the rule-options.
+Almost each rule (unless otherwise indicated) may be customized to ignore skipped tests/suites (`describe.skip`, `it.skip`, `xspecify`, etc.) by adding `skipSkipped: true` to the rule options. One can also set `"settings": {"mocha-cleanup": {"skipSkipped": true}}` to have skipping for all applicable rules by default.
 
-* `asserts-limit` Rule to disallow use more than allowed number of assertions. Tests without any assertions are also disallowed. Rule may be customized with setting maximum number of allowed asserts (Defaults to 3):
+* `asserts-limit` Rule to disallow use of more than an allowed number of assertions. Tests without any assertions are also disallowed. Rule may be customized with setting the maximum number of allowed asserts (Defaults to 3):
 
 ```json
 "rules": {
     "mocha-cleanup/asserts-limit": [2, {"assertsLimit": 3}]
 }
 ```
 
-This rule ignores tests with `done`-callback and 0 assertions. Set option `ignoreZeroAssertionsIfDoneExists` to `false` to allow such behavior:
+This rule ignores tests with a `done`-callback and 0 assertions. Set the option `ignoreZeroAssertionsIfDoneExists` to `false` to allow such behavior:
 
 ```json
 "rules": {
     "mocha-cleanup/asserts-limit": [2, {"ignoreZeroAssertionsIfDoneExists": false}]
 }
 ```
 
-* `disallow-stub-spy-restore-in-it` Rule to disallow `stub/spy/restore` in the tests (should be in the hooks)
+* `disallow-stub-spy-restore-in-it` Rule to disallow `stub/spy/restore` in tests (they should instead be in hooks)
 
-* `no-empty-title` Rule to disallow empty title in the suites and tests
+* `no-empty-title` Rule to disallow empty titles in suites and tests
 
-* `no-same-titles` Rule to disallow same titles for tests inside one suite or in the whole file. It depends on `scope` value - may be `file` or `suite`. Default - `suite`
+* `no-same-titles` Rule to disallow identical titles for tests inside of one suite or within the whole file. It depends on the `scope` value - may be `file` or `suite`. Defaults to `suite`
 
 ```json
 "rules": {
@@ -79,9 +79,9 @@ This rule ignores tests with `done`-callback and 0 assertions. Set option `ignor
 
 * `invalid-assertions` Rule to check `expect` and `should` assertions for completeness. It detects assertions that end with "chainable" words or even raw calls for `expect` and `should`
 
-* `no-expressions-in-assertions` Rule to detect expressions in the `expect` and `assert` assertions
+* `no-expressions-in-assertions` Rule to detect expressions in `expect` and `assert` assertions
 
-* `disallowed-usage` Rule to disallow usage some functions, methods or properties in the tests and hooks
+* `disallowed-usage` Rule to disallow usage of some functions, methods or properties in the tests and hooks
 
 ```json
 "rules": {
@@ -95,9 +95,9 @@ This rule ignores tests with `done`-callback and 0 assertions. Set option `ignor
 }
 ```
 
-* `disallow-stub-window` Rule to disallow stubbing some `window`-methods. **IMPORTANT** This rule doesn't have `skipSkipped` option
+* `disallow-stub-window` Rule to disallow stubbing some `window`-methods. **IMPORTANT** This rule doesn't have a `skipSkipped` option
 
-* `no-outside-declaration` Rule to disallow variables declaration outside tests and hooks
+* `no-outside-declaration` Rule to disallow variable declarations outside tests and hooks
 
 ## Usage
 

diff --git a/lib/rules/asserts-limit.js b/lib/rules/asserts-limit.js
@@ -7,6 +7,7 @@
 "use strict";
 
 var n = require("../utils/node.js");
+var isSkipSkipped = require("../utils/options.js").isSkipSkipped;
 
 var hasOwnProperty = Object.prototype.hasOwnProperty;
 
@@ -20,7 +21,7 @@ module.exports = {
     var assertsCounter = 0;
     var options = context.options[0] || {};
     var assertsLimit = options.assertsLimit >= 1 ? options.assertsLimit : 3;
-    var skipSkipped = hasOwnProperty.call(options, "skipSkipped") ? options.skipSkipped : false;
+    var skipSkipped = isSkipSkipped(options, context);
     var ignoreZeroAssertionsIfDoneExists = hasOwnProperty.call(options, "ignoreZeroAssertionsIfDoneExists") ? options.ignoreZeroAssertionsIfDoneExists : true;
     var nodeSkipped;
     var doneExists;

diff --git a/lib/rules/complexity-it.js b/lib/rules/complexity-it.js
@@ -8,6 +8,7 @@
 "use strict";
 
 var n = require("../utils/node.js");
+var isSkipSkipped = require("../utils/options.js").isSkipSkipped;
 var hasOwnProperty = Object.prototype.hasOwnProperty;
 
 //------------------------------------------------------------------------------
@@ -21,7 +22,7 @@ module.exports = {
     var currentComplexityCount = 0;
     var options = context.options[0] || {};
     var maxAllowedComplexity = hasOwnProperty.call(options, "maxAllowedComplexity") ? options.maxAllowedComplexity : 40;
-    var skipSkipped = hasOwnProperty.call(options, "skipSkipped") ? options.skipSkipped : false;
+    var skipSkipped = isSkipSkipped(options, context);
     var nodeSkipped;
 
     function increaseComplexity() {

diff --git a/lib/rules/disallow-stub-spy-restore-in-it.js b/lib/rules/disallow-stub-spy-restore-in-it.js
@@ -8,7 +8,7 @@
 
 var obj = require("../utils/obj.js");
 var n = require("../utils/node.js");
-var hasOwnProperty = Object.prototype.hasOwnProperty;
+var isSkipSkipped = require("../utils/options.js").isSkipSkipped;
 //------------------------------------------------------------------------------
 // Rule Definition
 //------------------------------------------------------------------------------
@@ -19,7 +19,7 @@ module.exports = {
     var disallowed = ["stub", "spy", "restore"];
     var insideTest = false;
     var options = context.options[0] || {};
-    var skipSkipped = hasOwnProperty.call(options, "skipSkipped") ? options.skipSkipped : false;
+    var skipSkipped = isSkipSkipped(options, context);
     var nodeSkipped = false;
     var caller = "";
 
@@ -47,13 +47,11 @@ module.exports = {
       "FunctionExpression:exit": fExit,
       "ArrowFunctionExpression:exit": fExit,
 
-      "CallExpression": function (node) {
+      "CallExpression[callee]": function (node) {
         var callee = obj.get(node, "callee");
-        if (callee) {
-          var _c = n.cleanCaller(n.getCaller(callee)).split(".").pop();
-          if (insideTest && disallowed.indexOf(_c) !== -1 && !nodeSkipped) {
-            context.report(node, "`{{name}}` is not allowed to use inside `{{caller}}`.", {name: _c, caller: caller});
-          }
+        var _c = n.cleanCaller(n.getCaller(callee)).split(".").pop();
+        if (insideTest && disallowed.indexOf(_c) !== -1 && !nodeSkipped) {
+          context.report(node, "`{{name}}` is not allowed to use inside `{{caller}}`.", {name: _c, caller: caller});
         }
       }
 

diff --git a/lib/rules/disallowed-usage.js b/lib/rules/disallowed-usage.js
@@ -8,6 +8,7 @@
 
 var n = require("../utils/node.js");
 var obj = require("../utils/obj.js");
+var isSkipSkipped = require("../utils/options.js").isSkipSkipped;
 var hasOwnProperty = Object.prototype.hasOwnProperty;
 //------------------------------------------------------------------------------
 // Rule Definition
@@ -49,8 +50,11 @@ module.exports = {
 
     var insideTest = false;
     var insideHook = false;
-    var options = context.options[0] || {};
-    var skipSkipped = hasOwnProperty.call(options, "skipSkipped") ? options.skipSkipped : false;
+    var options = context.options[0];
+    if (!options) {
+      return {};
+    }
+    var skipSkipped = isSkipSkipped(options, context);
     var disallowedMethodsInTests = hasOwnProperty.call(options, "test") ? prepareCallers(options.test) : [];
     var disallowedPropertiesInTests = hasOwnProperty.call(options, "test") ? prepareProperties(options.test) : [];
     var disallowedMethodsInHooks = hasOwnProperty.call(options, "hook") ? prepareCallers(options.hook) : [];
@@ -85,10 +89,12 @@ module.exports = {
       "ArrowFunctionExpression:exit": fExit,
       "CallExpression": function (node) {
         var parent = obj.get(node, "parent");
+        /* istanbul ignore if */
         if (!parent) {
           return;
         }
         var caller = n.cleanCaller(n.getCaller(node));
+        /* istanbul ignore if */
         if (!caller) {
           return;
         }
@@ -101,6 +107,7 @@ module.exports = {
       },
       "MemberExpression": function (node) {
         var parent = obj.get(node, "parent");
+        /* istanbul ignore if */
         if (!parent) {
           return;
         }

diff --git a/lib/rules/invalid-assertions.js b/lib/rules/invalid-assertions.js
@@ -7,7 +7,7 @@
 "use strict";
 
 var n = require("../utils/node.js");
-var hasOwnProperty = Object.prototype.hasOwnProperty;
+var isSkipSkipped = require("../utils/options.js").isSkipSkipped;
 //------------------------------------------------------------------------------
 // Rule Definition
 //------------------------------------------------------------------------------
@@ -17,7 +17,7 @@ module.exports = {
     var m = "Invalid assertion usage.";
     var insideIt = false;
     var options = context.options[0] || {};
-    var skipSkipped = hasOwnProperty.call(options, "skipSkipped") ? options.skipSkipped : false;
+    var skipSkipped = isSkipSkipped(options, context);
     var nodeSkipped;
 
     function check(node) {
@@ -26,10 +26,13 @@ module.exports = {
       }
       if (n.isAssertion(node)) {
         var parentExpression = n.getParentExpression(node);
+        /* istanbul ignore if */
         if (!parentExpression) {
           return;
         }
-        var caller = n.getCaller(parentExpression.expression) || "";
+        var caller = n.getCaller(parentExpression.expression) ||
+          /* istanbul ignore next */
+          "";
         if (["expect", "chai.expect"].indexOf(caller) !== -1) {
           return context.report(node, m);
         }

diff --git a/lib/rules/no-assertions-in-loop.js b/lib/rules/no-assertions-in-loop.js
@@ -7,6 +7,7 @@
 "use strict";
 
 var n = require("../utils/node.js");
+var isSkipSkipped = require("../utils/options.js").isSkipSkipped;
 var hasOwnProperty = Object.prototype.hasOwnProperty;
 //------------------------------------------------------------------------------
 // Rule Definition
@@ -17,11 +18,12 @@ module.exports = {
     var loopStatements = ["WhileStatement", "DoWhileStatement", "ForStatement", "ForInStatement", "ForOfStatement"];
     var insideIt = false;
     var options = context.options[0] || {};
-    var skipSkipped = hasOwnProperty.call(options, "skipSkipped") ? options.skipSkipped : false;
+    var skipSkipped = isSkipSkipped(options, context);
     var extraMemberExpression = hasOwnProperty.call(options, "extraMemberExpression") ? options.extraMemberExpression : [];
     var nodeSkipped;
 
     function detectParentLoopInTest(node) {
+      /* istanbul ignore if */
       if (!node) {
         return false;
       }

diff --git a/lib/rules/no-assertions-outside-it.js b/lib/rules/no-assertions-outside-it.js
@@ -9,7 +9,7 @@
 var n = require("../utils/node.js");
 var m = require("../utils/mocha-specs.js");
 var obj = require("../utils/obj.js");
-var hasOwnProperty = Object.prototype.hasOwnProperty;
+var isSkipSkipped = require("../utils/options.js").isSkipSkipped;
 //------------------------------------------------------------------------------
 // Rule Definition
 //------------------------------------------------------------------------------
@@ -19,7 +19,7 @@ module.exports = {
 
     var insideIt = false;
     var options = context.options[0] || {};
-    var skipSkipped = hasOwnProperty.call(options, "skipSkipped") ? options.skipSkipped : false;
+    var skipSkipped = isSkipSkipped(options, context);
 
     var message = "Assertion outside tests is not allowed.";
 

diff --git a/lib/rules/no-empty-body.js b/lib/rules/no-empty-body.js
@@ -9,7 +9,7 @@
 var n = require("../utils/node.js");
 var obj = require("../utils/obj.js");
 var mocha = require("../utils/mocha-specs.js");
-var hasOwnProperty = Object.prototype.hasOwnProperty;
+var isSkipSkipped = require("../utils/options.js").isSkipSkipped;
 var toCheck = mocha.all.concat(mocha.hooks);
 
 //------------------------------------------------------------------------------
@@ -19,7 +19,7 @@ var toCheck = mocha.all.concat(mocha.hooks);
 module.exports = {
   create: function (context) {
     var options = context.options[0] || {};
-    var skipSkipped = hasOwnProperty.call(options, "skipSkipped") ? options.skipSkipped : false;
+    var skipSkipped = isSkipSkipped(options, context);
 
     function fEnter (node) {
       if (skipSkipped && n.tryDetectSkipInParent(node)) {

diff --git a/lib/rules/no-empty-title.js b/lib/rules/no-empty-title.js
@@ -9,7 +9,7 @@
 var obj = require("../utils/obj.js");
 var n = require("../utils/node.js");
 var mocha = require("../utils/mocha-specs.js");
-var hasOwnProperty = Object.prototype.hasOwnProperty;
+var isSkipSkipped = require("../utils/options.js").isSkipSkipped;
 //------------------------------------------------------------------------------
 // Rule Definition
 //------------------------------------------------------------------------------
@@ -18,7 +18,7 @@ module.exports = {
   create: function (context) {
 
     var options = context.options[0] || {};
-    var skipSkipped = hasOwnProperty.call(options, "skipSkipped") ? options.skipSkipped : false;
+    var skipSkipped = isSkipSkipped(options, context);
 
     return {
       "CallExpression": function (node) {

diff --git a/lib/rules/no-eql-primitives.js b/lib/rules/no-eql-primitives.js
@@ -8,7 +8,7 @@
 
 var n = require("../utils/node.js");
 var obj = require("../utils/obj.js");
-var hasOwnProperty = Object.prototype.hasOwnProperty;
+var isSkipSkipped = require("../utils/options.js").isSkipSkipped;
 //------------------------------------------------------------------------------
 // Rule Definition
 //------------------------------------------------------------------------------
@@ -17,7 +17,7 @@ module.exports = {
   create: function (context) {
 
     var options = context.options[0] || {};
-    var skipSkipped = hasOwnProperty.call(options, "skipSkipped") ? options.skipSkipped : false;
+    var skipSkipped = isSkipSkipped(options, context);
     var strictCheck = ["assert.deepEqual", "assert.notDeepEqual"];
     var notStrictCheck = [".eql", ".deep.equal"];
     var insideTest = false;
@@ -47,6 +47,7 @@ module.exports = {
       "MemberExpression": function (node) {
         if (insideTest && !(skipSkipped && n.tryDetectSkipInParent(node))) {
           var caller = n.getCaller(node);
+          /* istanbul ignore if */
           if (!caller) {
             return;
           }

diff --git a/lib/rules/no-expressions-in-assertions.js b/lib/rules/no-expressions-in-assertions.js
@@ -8,7 +8,7 @@
 
 var n = require("../utils/node.js");
 var obj = require("../utils/obj.js");
-var hasOwnProperty = Object.prototype.hasOwnProperty;
+var isSkipSkipped = require("../utils/options.js").isSkipSkipped;
 //------------------------------------------------------------------------------
 // Rule Definition
 //------------------------------------------------------------------------------
@@ -17,8 +17,9 @@ module.exports = {
   create: function (context) {
     var insideIt = false;
     var options = context.options[0] || {};
-    var skipSkipped = hasOwnProperty.call(options, "skipSkipped") ? options.skipSkipped : false;
+    var skipSkipped = isSkipSkipped(options, context);
     var nodeSkipped;
+    var replacementsOnly = options.replacementsOnly;
 
     var defaultMessage = "Expression should not be used here.";
     var detailedMessage = "`{{shouldUse}}` should be used.";
@@ -75,7 +76,9 @@ module.exports = {
         }
         return context.report(node, detailedMessage, {shouldUse: shouldUse});
       }
-      return context.report(node, defaultMessage);
+      if (!replacementsOnly) {
+        return context.report(node, defaultMessage);
+      }
     }
 
     /**
@@ -136,7 +139,9 @@ module.exports = {
         }
         return context.report(node, detailedMessage, {shouldUse: shouldUse});
       }
-      return context.report(node, defaultMessage);
+      if (!replacementsOnly) {
+        return context.report(node, defaultMessage);
+      }
     }
 
     /**
@@ -165,7 +170,7 @@ module.exports = {
      * @returns {boolean}
      */
     function checkForUndefinedAssert(binaryExpression, op, node) {
-      var shouldUse = op[0] === "!" ? ".isUndefined" : ".isDefined";
+      var shouldUse = op[0] === "!" ? ".isDefined" : ".isUndefined";
       if (obj.get(binaryExpression, "left.name") === "undefined" || obj.get(binaryExpression, "right.name") === "undefined") {
         context.report(node, detailedMessage, {shouldUse: shouldUse});
         return true;
@@ -190,7 +195,7 @@ module.exports = {
     }
 
     function dontReport(arg) {
-      return arg.operator === "typeof";
+      return replacementsOnly || arg.operator === "typeof";
     }
 
     return {
@@ -259,6 +264,9 @@ module.exports = {
         properties: {
           skipSkipped: {
             type: "boolean"
+          },
+          replacementsOnly: {
+            type: "boolean"
           }
         },
         additionalProperties: false