Fix: Allow for third parameter in no-hooks-from-ancestor-modules

Fixes #230.
platinumazure · May 15, 2022 · e0297c3 · e0297c3
1 parent f3878c1
commit e0297c3
Show file tree

Hide file tree

Showing 3 changed files with 50 additions and 2 deletions.
diff --git a/docs/rules/no-hooks-from-ancestor-modules.md b/docs/rules/no-hooks-from-ancestor-modules.md
@@ -33,6 +33,12 @@ The following patterns are not warnings:
 QUnit.module("example module", function(hooks) {
     hooks.beforeEach(function() {});
 });
+
+QUnit.module("outer module", function() {
+    QUnit.module("inner module", function(hooks) {
+        hooks.beforeEach(function() {});
+    });
+});
 ```
 
 ## When Not To Use It

diff --git a/lib/rules/no-hooks-from-ancestor-modules.js b/lib/rules/no-hooks-from-ancestor-modules.js
@@ -60,6 +60,12 @@ module.exports = {
                 isInModuleCallbackBody(node);
         }
 
+        function getCallbackArg(args) {
+            // Callback can be either args[1] or args[2]
+            // https://api.qunitjs.com/QUnit/module/
+            return args.slice(1, 3).find(arg => arg.type === "FunctionExpression");
+        }
+
         function getHooksIdentifierFromParams(params) {
             // In TypeScript, `this` can be passed as the first function parameter to add a type to it,
             // and we want to ignore that parameter since we're looking for the `hooks` variable.
@@ -78,8 +84,8 @@ module.exports = {
                         description: node.arguments[0].value
                     };
 
-                    const callback = node.arguments[1];
-                    const hooksParam = callback && callback.type === "FunctionExpression" ? getHooksIdentifierFromParams(callback.params) : null;
+                    const callback = getCallbackArg(node.arguments);
+                    const hooksParam = callback ? getHooksIdentifierFromParams(callback.params) : null;
                     moduleStackInfo.hookIdentifierName = hooksParam ? hooksParam.name : null;
                     moduleStack.push(moduleStackInfo);
                 } else if (isHookInvocation(node)) {

diff --git a/tests/lib/rules/no-hooks-from-ancestor-modules.js b/tests/lib/rules/no-hooks-from-ancestor-modules.js
@@ -48,6 +48,12 @@ ruleTester.run("no-hooks-from-ancestor-modules", rule, {
         QUnit.module("module", function(hooks) { hooks.afterEach(function() {}); });
         `,
         `
+        QUnit.module("module", {}, function(hooks) { hooks.beforeEach(function() {}); });
+        `,
+        `
+        QUnit.module("module", makeOptions(), function(hooks) { hooks.beforeEach(function() {}); });
+        `,
+        `
         QUnit.module("module-a", function() {
             QUnit.module("module-b", function(hooks) {
                 hooks.beforeEach(function() {});
@@ -140,6 +146,36 @@ ruleTester.run("no-hooks-from-ancestor-modules", rule, {
                 })
             ]
         },
+        {
+            code: `
+                QUnit.module("module-a", {}, function (hooks) {
+                    QUnit.module("module-b", {}, function () {
+                        hooks.beforeEach(function () {});
+                    });
+                });
+            `,
+            errors: [
+                createError({
+                    invokedMethodName: "beforeEach",
+                    usedHooksIdentifierName: "hooks"
+                })
+            ]
+        },
+        {
+            code: `
+                QUnit.module("module-a", makeOptions(), function (hooks) {
+                    QUnit.module("module-b", makeOptions(), function () {
+                        hooks.beforeEach(function () {});
+                    });
+                });
+            `,
+            errors: [
+                createError({
+                    invokedMethodName: "beforeEach",
+                    usedHooksIdentifierName: "hooks"
+                })
+            ]
+        },
         {
             code: `
                 QUnit.module("first", function (firstHooks) {