Fix: fix global equal() check in no-assert-equal rule (#183)

qunitjs · May 17, 2021 · 9fee369 · 9fee369
1 parent de7ca73
commit 9fee369
Show file tree

Hide file tree

Showing 2 changed files with 39 additions and 25 deletions.
diff --git a/lib/rules/no-assert-equal.js b/lib/rules/no-assert-equal.js
@@ -9,7 +9,8 @@
 //------------------------------------------------------------------------------
 
 const assert = require("assert"),
-    utils = require("../utils");
+    utils = require("../utils"),
+    { ReferenceTracker } = require("eslint-utils");
 
 //------------------------------------------------------------------------------
 // Rule Definition
@@ -39,11 +40,9 @@ module.exports = {
         // supported by QUnit).
         const testStack = [];
 
-        function isGlobalEqual(calleeNode) {
-            return calleeNode &&
-                calleeNode.type === "Identifier" &&
-                calleeNode.name === "equal";
-        }
+        // We check upfront to find all the references to global equal(),
+        // and then report them if they end up being inside test contexts.
+        const globalEqualCallNodes = new Set();
 
         function getCurrentAssertContextVariable() {
             assert(testStack.length, "Test stack should not be empty");
@@ -60,37 +59,30 @@ module.exports = {
                 calleeNode.object.name === getCurrentAssertContextVariable();
         }
 
-        function isEqual(calleeNode) {
-            return isGlobalEqual(calleeNode) ||
-                isAssertEqual(calleeNode);
-        }
-
-        function reportError(node) {
-            const assertVar = getCurrentAssertContextVariable();
-
+        function reportError(node, isGlobal) {
             context.report({
                 node: node,
-                messageId: isGlobalEqual(node.callee) ? "unexpectedGlobalEqual" : "unexpectedAssertEqual",
+                messageId: isGlobal ? "unexpectedGlobalEqual" : "unexpectedAssertEqual",
                 data: {
-                    assertVar
+                    assertVar: isGlobal ? null : getCurrentAssertContextVariable()
                 },
                 suggest: [
                     {
                         messageId: "switchToDeepEqual",
                         fix(fixer) {
-                            return fixer.replaceText(isGlobalEqual(node.callee) ? node.callee : node.callee.property, "deepEqual");
+                            return fixer.replaceText(isGlobal ? node.callee : node.callee.property, "deepEqual");
                         }
                     },
                     {
                         messageId: "switchToPropEqual",
                         fix(fixer) {
-                            return fixer.replaceText(isGlobalEqual(node.callee) ? node.callee : node.callee.property, "propEqual");
+                            return fixer.replaceText(isGlobal ? node.callee : node.callee.property, "propEqual");
                         }
                     },
                     {
                         messageId: "switchToStrictEqual",
                         fix(fixer) {
-                            return fixer.replaceText(isGlobalEqual(node.callee) ? node.callee : node.callee.property, "strictEqual");
+                            return fixer.replaceText(isGlobal ? node.callee : node.callee.property, "strictEqual");
                         }
                     }
                 ]
@@ -105,15 +97,29 @@ module.exports = {
                     testStack.push({
                         assertVar: utils.getAssertContextNameForTest(node.arguments)
                     });
-                } else if (testStack.length > 0 && isEqual(node.callee)) {
-                    reportError(node);
+                } else if (testStack.length > 0) {
+                    if (isAssertEqual(node.callee)) {
+                        reportError(node, false);
+                    } else if (globalEqualCallNodes.has(node)) {
+                        reportError(node, true);
+                    }
                 }
             },
             "CallExpression:exit": function (node) {
                 /* istanbul ignore else: correctly does nothing */
                 if (utils.isTest(node.callee) || utils.isAsyncTest(node.callee)) {
                     testStack.pop();
                 }
+            },
+            "Program": function () {
+                // Gather all calls to global `equal()`.
+
+                const tracker = new ReferenceTracker(context.getScope());
+                const traceMap = { equal: { [ReferenceTracker.CALL]: true } };
+
+                for (const { node } of tracker.iterateGlobalReferences(traceMap)) {
+                    globalEqualCallNodes.add(node);
+                }
             }
         };
     }

diff --git a/tests/lib/rules/no-assert-equal.js b/tests/lib/rules/no-assert-equal.js
@@ -32,8 +32,14 @@ ruleTester.run("no-assert-equal", rule, {
         "QUnit.test('Name', function () { deepEqual(a, b); });",
         "QUnit.test('Name', function () { propEqual(a, b); });",
 
-        // equal is not within test context
-        "equal(a, b);"
+        // global `equal` but not within test context
+        {
+            code: "equal(a, b);",
+            globals: { equal: true }
+        },
+
+        // `equal` but not the global
+        "function equal(a,b) {}; QUnit.test('Name', function () { equal(a, b); });"
     ],
 
     invalid: [
@@ -97,7 +103,8 @@ ruleTester.run("no-assert-equal", rule, {
                         output: "QUnit.test('Name', function (assert) { strictEqual(a, b); });"
                     }
                 ]
-            }]
+            }],
+            globals: { equal: true }
         },
         {
             code: "QUnit.test('Name', function () { equal(a, b); });",
@@ -117,7 +124,8 @@ ruleTester.run("no-assert-equal", rule, {
                         output: "QUnit.test('Name', function () { strictEqual(a, b); });"
                     }
                 ]
-            }]
+            }],
+            globals: { equal: true }
         }
     ]
 });