Update: Refactor no-conditional-require rule

* Replace stack-based conditional tracking with `ancestor` function
* Improve code-coverage
* Add `docs` meta

Author: Casey Visco

lib/rules/no-conditional-require.js

@@ -1,72 +1,50 @@
 /**
- * @fileoverview Disallow use of conditional `require` calls
+ * @fileoverview Rule to disallow use of conditional `require` calls
  * @author Casey Visco
  */
 
 "use strict";
 
-const util = require("../util");
+const isRequireCall = require("../util").isRequireCall;
+const ancestor = require("../utils/ast").ancestor;
+
+// -----------------------------------------------------------------------------
+// Helpers
+// -----------------------------------------------------------------------------
+
+/**
+ * Test if supplied `node` represents a conditional—either an `if` statement
+ * or a ternary expression.
+ * @private
+ * @param {ASTNode} node - node to test
+ * @returns {Boolean} true if node represents a conditional
+ */
+function isConditional(node) {
+    return node.type === "IfStatement" ||
+           node.type === "ConditionalExpression";
+}
+
+// -----------------------------------------------------------------------------
+// Rule Definition
+// -----------------------------------------------------------------------------
+
+const ERROR_MSG = "Conditional `require` calls are not allowed.";
 
 module.exports = {
     meta: {
-        docs: {},
+        docs: {
+            description: "Disallow use of conditional `require` calls",
+            category: "Stylistic Choices",
+            recommended: true
+        },
         schema: []
     },
 
     create: function (context) {
-        const MESSAGE = "Conditional `require` calls are not allowed.";
-        const condStack = [];
-
-        /**
-         * Determine if we are currently inside of a conditional.
-         *
-         * @private
-         * @returns {Boolean} true if inside at least one conditional
-         */
-        function isInsideConditional() {
-            return condStack.length > 0;
-        }
-
-        /**
-         * Add provided conditional `node` to the stack.
-         *
-         * @private
-         * @param {ASTNode} node - ConditionalExpression or IfStatement node
-         * @returns {void}
-         */
-        function pushConditional(node) {
-            condStack.push(node);
-        }
-
-        /**
-         * Remove provided conditional `node` from the stack if it is the last in
-         * the list.
-         *
-         * @private
-         * @param {ASTNode} node - ConditionalExpression or IfStatement node
-         * @returns {void}
-         */
-        function popConditional(node) {
-            const len = condStack.length;
-
-            if (len && condStack[len - 1] === node) {
-                condStack.pop();
-            }
-        }
-
         return {
-
-            // Standard "If" block
-            "IfStatement": pushConditional,
-            "IfStatement:exit": popConditional,
-
-            // Ternary Expression (?:)
-            "ConditionalExpression": pushConditional,
-            "ConditionalExpression:exit": popConditional,
-
             "CallExpression": function (node) {
-                if (util.isRequireCall(node) && isInsideConditional()) {
-                    context.report(node, MESSAGE);
+                if (isRequireCall(node) && ancestor(isConditional, node)) {
+                    context.report(node, ERROR_MSG);
                 }
             }
         };

lib/utils/ast.js

@@ -1,5 +1,5 @@
 /**
- * @fileoverview Predicates for testing AST Nodes
+ * @fileoverview Helpers for testing AST Nodes
  * @author Casey Visco
  */
 
@@ -129,6 +129,22 @@ function hasCallback(node) {
            node.arguments.some(isFunctionExpr);
 }
 
+/**
+ * Traverse parent nodes until one is found that satisfies `predicate` is found.
+ * @param {Function} predicate - predicate to test each ancestor against
+ * @param {ASTNode} node - child node to begin search at
+ * @returns {Boolean} true if an ancestor satisfies `predicate`
+ */
+function ancestor(predicate, node) {
+    while ((node = node.parent)) {
+        if (predicate(node)) {
+            return true;
+        }
+    }
+
+    return false;
+}
+
 module.exports = {
     isIdentifier,
     isLiteral,
@@ -140,5 +156,6 @@ module.exports = {
     isExprStatement,
     isStringLiteralArray,
     hasParams,
-    hasCallback
+    hasCallback,
+    ancestor
 };

tests/lib/utils/ast.js

@@ -419,3 +419,31 @@ describe("ast.hasCallback", function () {
     });
 
 });
+
+describe("ast.ancestor", function () {
+    const sampleNode = {
+        parent: {
+            parent: {
+                parent: {
+                    type: "bar"
+                },
+                type: "foo"
+            }
+        }
+    };
+
+    it("should return `true` if an ancestor satisfies the predicate", function () {
+        const actual = ast.ancestor((node) => node.type === "foo", sampleNode);
+        const expected = true;
+
+        assert.equal(actual, expected);
+    });
+
+    it("should return `false` if no ancestor satisfies the predicate", function () {
+        const actual = ast.ancestor((node) => node.type === "baz", sampleNode);
+        const expected = false;
+
+        assert.equal(actual, expected);
+    });
+
+});