From d0b09e2792b5c371f9b2c30cf55c83a85ff33d76 Mon Sep 17 00:00:00 2001 From: arensman Date: Fri, 11 Jul 2025 16:34:56 +0200 Subject: [PATCH 1/4] Updated no-assert-equal rule to also catch assert.equal in module hooks Moved isInModule into utils (was duplicated around a few places) Added before and after to recognized module hook properties Added test case --- lib/rules/no-assert-equal.js | 44 ++++++++++++++++++++++ lib/rules/no-qunit-start-in-tests.js | 18 +-------- lib/rules/no-setup-teardown.js | 16 +------- lib/rules/resolve-async.js | 18 +-------- lib/utils.js | 21 ++++++++++- tests/lib/rules/no-arrow-tests.js | 20 ++++++++++ tests/lib/rules/no-assert-equal.js | 25 ++++++++++++ tests/lib/rules/no-qunit-start-in-tests.js | 8 ++++ tests/lib/rules/no-setup-teardown.js | 10 ++++- tests/lib/rules/resolve-async.js | 8 ++++ 10 files changed, 138 insertions(+), 50 deletions(-) diff --git a/lib/rules/no-assert-equal.js b/lib/rules/no-assert-equal.js index 5b0b5101..fce16899 100644 --- a/lib/rules/no-assert-equal.js +++ b/lib/rules/no-assert-equal.js @@ -159,6 +159,41 @@ module.exports = { }); } + /** + * @param {import('eslint').Rule.Node} propertyNode + * @returns {boolean} + */ + function isModuleHookCallback(node) { + return ( + node.parent && + node.parent.type === "Property" && + utils.isModuleHookPropertyKey(node.parent.key) && + utils.isInModule(node.parent) + ); + } + + /** + * @param {import('eslint').Rule.Node} propertyNode + * @returns {boolean} + */ + function pushModuleHookAssert(node) { + if (isModuleHookCallback(node)) { + testStack.push({ + assertVar: utils.getAssertContextName(node), + }); + } + } + + /** + * @param {import('eslint').Rule.Node} propertyNode + * @returns {boolean} + */ + function popModuleHookAssert(node) { + if (isModuleHookCallback(node)) { + testStack.pop(); + } + } + return { CallExpression: function (node) { /* istanbul ignore else: correctly does nothing */ @@ -188,6 +223,15 @@ module.exports = { testStack.pop(); } }, + + FunctionDeclaration: pushModuleHookAssert, + FunctionExpression: pushModuleHookAssert, + ArrowFunctionExpression: pushModuleHookAssert, + + "FunctionDeclaration:exit": popModuleHookAssert, + "FunctionExpression:exit": popModuleHookAssert, + "ArrowFunctionExpression:exit": popModuleHookAssert, + Program: function (node) { // Gather all calls to global `equal()`. /* istanbul ignore next: deprecated code paths only followed by old eslint versions */ diff --git a/lib/rules/no-qunit-start-in-tests.js b/lib/rules/no-qunit-start-in-tests.js index 009815e0..900b403c 100644 --- a/lib/rules/no-qunit-start-in-tests.js +++ b/lib/rules/no-qunit-start-in-tests.js @@ -51,20 +51,6 @@ module.exports = { ); } - /** - * @param {import('eslint').Rule.Node} propertyNode - * @returns {boolean} - */ - function isInModule(propertyNode) { - return ( - propertyNode && - propertyNode.parent && // ObjectExpression - propertyNode.parent.parent && // CallExpression? - propertyNode.parent.parent.type === "CallExpression" && - utils.isModule(propertyNode.parent.parent.callee) - ); - } - //---------------------------------------------------------------------- // Public //---------------------------------------------------------------------- @@ -93,7 +79,7 @@ module.exports = { Property: function (node) { if ( utils.isModuleHookPropertyKey(node.key) && - isInModule(node) && + utils.isInModule(node) && node.key.type === "Identifier" ) { contextStack.push(`${node.key.name} hook`); @@ -109,7 +95,7 @@ module.exports = { "Property:exit": function (node) { if ( utils.isModuleHookPropertyKey(node.key) && - isInModule(node) + utils.isInModule(node) ) { contextStack.pop(); } diff --git a/lib/rules/no-setup-teardown.js b/lib/rules/no-setup-teardown.js index 0d0d7c98..81e71bf8 100644 --- a/lib/rules/no-setup-teardown.js +++ b/lib/rules/no-setup-teardown.js @@ -68,25 +68,11 @@ module.exports = { } } - /** - * @param {import('eslint').Rule.Node} propertyNode - * @returns {boolean} - */ - function isInModule(propertyNode) { - return ( - propertyNode && - propertyNode.parent && // ObjectExpression - propertyNode.parent.parent && // CallExpression? - propertyNode.parent.parent.type === "CallExpression" && - utils.isModule(propertyNode.parent.parent.callee) - ); - } - return { Property: function (node) { if ( utils.isModuleHookPropertyKey(node.key) && - isInModule(node) + utils.isInModule(node) ) { checkModuleHook(node); } diff --git a/lib/rules/resolve-async.js b/lib/rules/resolve-async.js index 9880cc3b..61d0e256 100644 --- a/lib/rules/resolve-async.js +++ b/lib/rules/resolve-async.js @@ -159,20 +159,6 @@ module.exports = { } } - /** - * @param {import('eslint').Rule.Node} propertyNode - * @returns {boolean} - */ - function isInModule(propertyNode) { - return ( - propertyNode && - propertyNode.parent && // ObjectExpression - propertyNode.parent.parent && // CallExpression? - propertyNode.parent.parent.type === "CallExpression" && - utils.isModule(propertyNode.parent.parent.callee) - ); - } - return { CallExpression: function (node) { const callbackVar = getAsyncCallbackVarOrNull(node.callee); @@ -213,7 +199,7 @@ module.exports = { Property: function (node) { if ( utils.isModuleHookPropertyKey(node.key) && - isInModule(node) + utils.isInModule(node) ) { asyncStateStack.push({ stopSemaphoreCount: 0, @@ -228,7 +214,7 @@ module.exports = { "Property:exit": function (node) { if ( utils.isModuleHookPropertyKey(node.key) && - isInModule(node) + utils.isInModule(node) ) { const asyncState = asyncStateStack.pop(); if (!asyncState) { diff --git a/lib/utils.js b/lib/utils.js index 232a61d4..7f05fe45 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -9,7 +9,12 @@ const assert = require("node:assert"); const SUPPORTED_TEST_IDENTIFIERS = new Set(["test", "asyncTest", "only"]); const OLD_MODULE_HOOK_IDENTIFIERS = ["setup", "teardown"]; -const NEW_MODULE_HOOK_IDENTIFIERS = ["beforeEach", "afterEach"]; +const NEW_MODULE_HOOK_IDENTIFIERS = [ + "before", + "beforeEach", + "afterEach", + "after", +]; const ALL_MODULE_HOOK_IDENTIFIERS = new Set([ ...OLD_MODULE_HOOK_IDENTIFIERS, ...NEW_MODULE_HOOK_IDENTIFIERS, @@ -216,6 +221,20 @@ exports.isModule = function (calleeNode) { return result; }; +/** + * @param {import('eslint').Rule.Node} propertyNode + * @returns {boolean} + */ +exports.isInModule = function (propertyNode) { + return ( + propertyNode && + propertyNode.parent && // ObjectExpression + propertyNode.parent.parent && // CallExpression? + propertyNode.parent.parent.type === "CallExpression" && + this.isModule(propertyNode.parent.parent.callee) + ); +}; + /** * @param {import('estree').Node} identifierNode * @returns {boolean} diff --git a/tests/lib/rules/no-arrow-tests.js b/tests/lib/rules/no-arrow-tests.js index 6ee28a52..0f656bcb 100644 --- a/tests/lib/rules/no-arrow-tests.js +++ b/tests/lib/rules/no-arrow-tests.js @@ -188,6 +188,26 @@ ruleTester.run("no-arrow-tests", rule, { }, ], }, + { + code: "QUnit.module('module', { before: () => {} });", + output: "QUnit.module('module', { before: function() {} });", + errors: [ + { + messageId: "noArrowFunction", + type: "ArrowFunctionExpression", + }, + ], + }, + { + code: "QUnit.module('module', { after: () => {} });", + output: "QUnit.module('module', { after: function() {} });", + errors: [ + { + messageId: "noArrowFunction", + type: "ArrowFunctionExpression", + }, + ], + }, { code: "module('module', { setup: () => {} });", output: "module('module', { setup: function() {} });", diff --git a/tests/lib/rules/no-assert-equal.js b/tests/lib/rules/no-assert-equal.js index 6156c388..22aef773 100644 --- a/tests/lib/rules/no-assert-equal.js +++ b/tests/lib/rules/no-assert-equal.js @@ -184,5 +184,30 @@ ruleTester.run("no-assert-equal", rule, { }, ], }, + { + // assert.equal in module hooks + code: "QUnit.module('My module', { before: function (assert) { assert.equal(1, 1); } });", + parser: require.resolve("@typescript-eslint/parser"), + errors: [ + { + messageId: "unexpectedAssertEqual", + data: { assertVar: "assert" }, + suggestions: [ + { + messageId: "switchToDeepEqual", + output: "QUnit.module('My module', { before: function (assert) { assert.deepEqual(1, 1); } });", + }, + { + messageId: "switchToPropEqual", + output: "QUnit.module('My module', { before: function (assert) { assert.propEqual(1, 1); } });", + }, + { + messageId: "switchToStrictEqual", + output: "QUnit.module('My module', { before: function (assert) { assert.strictEqual(1, 1); } });", + }, + ], + }, + ], + }, ], }); diff --git a/tests/lib/rules/no-qunit-start-in-tests.js b/tests/lib/rules/no-qunit-start-in-tests.js index 1cf24d5b..baf4e3d3 100644 --- a/tests/lib/rules/no-qunit-start-in-tests.js +++ b/tests/lib/rules/no-qunit-start-in-tests.js @@ -57,6 +57,14 @@ ruleTester.run("no-qunit-start-in-tests", rule, { code: 'QUnit.module("module", { afterEach: function() { QUnit.start(); } });', errors: [createError("afterEach hook")], }, + { + code: 'QUnit.module("module", { before: function() { QUnit.start(); } });', + errors: [createError("before hook")], + }, + { + code: 'QUnit.module("module", { after: function() { QUnit.start(); } });', + errors: [createError("after hook")], + }, { code: 'QUnit.module("module", { setup: function() { QUnit.start(); } });', errors: [createError("setup hook")], diff --git a/tests/lib/rules/no-setup-teardown.js b/tests/lib/rules/no-setup-teardown.js index ac20316b..0a42d63d 100644 --- a/tests/lib/rules/no-setup-teardown.js +++ b/tests/lib/rules/no-setup-teardown.js @@ -29,8 +29,14 @@ ruleTester.run("no-setup-teardown", rule, { // afterEach "QUnit.module('Module', { afterEach: function () {} });", - // both - "QUnit.module('Module', { beforeEach: function () {}, afterEach: function () {} });", + // before + "QUnit.module('Module', { before: function () {} });", + + // after + "QUnit.module('Module', { after: function () {} });", + + // all + "QUnit.module('Module', { before: function () {}, beforeEach: function () {}, afterEach: function () {}, after: function () {} });", // other property names are not reported "QUnit.module('Module', { foo: function () {} });", diff --git a/tests/lib/rules/resolve-async.js b/tests/lib/rules/resolve-async.js index 0768fdbe..35c43253 100644 --- a/tests/lib/rules/resolve-async.js +++ b/tests/lib/rules/resolve-async.js @@ -195,6 +195,10 @@ ruleTester.run("resolve-async", rule, { code: "QUnit.module('name', { teardown: function () { QUnit.stop(); } });", errors: [createNeedStartCallsMessage("Property")], }, + { + code: "QUnit.module('name', { before: function () { QUnit.stop(); } });", + errors: [createNeedStartCallsMessage("Property")], + }, { code: "QUnit.module('name', { beforeEach: function () { QUnit.stop(); } });", errors: [createNeedStartCallsMessage("Property")], @@ -203,6 +207,10 @@ ruleTester.run("resolve-async", rule, { code: "QUnit.module('name', { afterEach: function () { QUnit.stop(); } });", errors: [createNeedStartCallsMessage("Property")], }, + { + code: "QUnit.module('name', { after: function () { QUnit.stop(); } });", + errors: [createNeedStartCallsMessage("Property")], + }, // Multiple start() calls needed { From 8e8b956a2bfacd9e8a7a0ec0ebd6acf9982fc04c Mon Sep 17 00:00:00 2001 From: arensman Date: Fri, 11 Jul 2025 16:45:09 +0200 Subject: [PATCH 2/4] Remove parser option --- tests/lib/rules/no-assert-equal.js | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/lib/rules/no-assert-equal.js b/tests/lib/rules/no-assert-equal.js index 22aef773..a5e7e4e5 100644 --- a/tests/lib/rules/no-assert-equal.js +++ b/tests/lib/rules/no-assert-equal.js @@ -187,7 +187,6 @@ ruleTester.run("no-assert-equal", rule, { { // assert.equal in module hooks code: "QUnit.module('My module', { before: function (assert) { assert.equal(1, 1); } });", - parser: require.resolve("@typescript-eslint/parser"), errors: [ { messageId: "unexpectedAssertEqual", From 153ea983eaeb9e0d2a3463fe637e8b305562ea7e Mon Sep 17 00:00:00 2001 From: arensman Date: Mon, 14 Jul 2025 11:21:27 +0200 Subject: [PATCH 3/4] Fix type checks --- lib/rules/no-assert-equal.js | 24 ++++++++++++------------ lib/utils.js | 2 +- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/lib/rules/no-assert-equal.js b/lib/rules/no-assert-equal.js index fce16899..0aad59eb 100644 --- a/lib/rules/no-assert-equal.js +++ b/lib/rules/no-assert-equal.js @@ -163,33 +163,33 @@ module.exports = { * @param {import('eslint').Rule.Node} propertyNode * @returns {boolean} */ - function isModuleHookCallback(node) { + function isModuleHookCallback(propertyNode) { return ( - node.parent && - node.parent.type === "Property" && - utils.isModuleHookPropertyKey(node.parent.key) && - utils.isInModule(node.parent) + propertyNode.parent && + propertyNode.parent.type === "Property" && + utils.isModuleHookPropertyKey(propertyNode.parent.key) && + utils.isInModule(propertyNode.parent) ); } /** * @param {import('eslint').Rule.Node} propertyNode - * @returns {boolean} + * @returns {void} */ - function pushModuleHookAssert(node) { - if (isModuleHookCallback(node)) { + function pushModuleHookAssert(propertyNode) { + if (isModuleHookCallback(propertyNode)) { testStack.push({ - assertVar: utils.getAssertContextName(node), + assertVar: utils.getAssertContextName(propertyNode), }); } } /** * @param {import('eslint').Rule.Node} propertyNode - * @returns {boolean} + * @returns {void} */ - function popModuleHookAssert(node) { - if (isModuleHookCallback(node)) { + function popModuleHookAssert(propertyNode) { + if (isModuleHookCallback(propertyNode)) { testStack.pop(); } } diff --git a/lib/utils.js b/lib/utils.js index 7f05fe45..58a7e693 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -231,7 +231,7 @@ exports.isInModule = function (propertyNode) { propertyNode.parent && // ObjectExpression propertyNode.parent.parent && // CallExpression? propertyNode.parent.parent.type === "CallExpression" && - this.isModule(propertyNode.parent.parent.callee) + exports.isModule(propertyNode.parent.parent.callee) ); }; From c7ec4fdce9cbe646d372448cfb92064f581eda74 Mon Sep 17 00:00:00 2001 From: arensman Date: Tue, 15 Jul 2025 11:21:38 +0200 Subject: [PATCH 4/4] Remove detecting after and before --- lib/utils.js | 7 +------ tests/lib/rules/no-arrow-tests.js | 20 -------------------- tests/lib/rules/no-assert-equal.js | 8 ++++---- tests/lib/rules/no-qunit-start-in-tests.js | 8 -------- tests/lib/rules/no-setup-teardown.js | 10 ++-------- tests/lib/rules/resolve-async.js | 8 -------- 6 files changed, 7 insertions(+), 54 deletions(-) diff --git a/lib/utils.js b/lib/utils.js index 58a7e693..bdadc3af 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -9,12 +9,7 @@ const assert = require("node:assert"); const SUPPORTED_TEST_IDENTIFIERS = new Set(["test", "asyncTest", "only"]); const OLD_MODULE_HOOK_IDENTIFIERS = ["setup", "teardown"]; -const NEW_MODULE_HOOK_IDENTIFIERS = [ - "before", - "beforeEach", - "afterEach", - "after", -]; +const NEW_MODULE_HOOK_IDENTIFIERS = ["beforeEach", "afterEach"]; const ALL_MODULE_HOOK_IDENTIFIERS = new Set([ ...OLD_MODULE_HOOK_IDENTIFIERS, ...NEW_MODULE_HOOK_IDENTIFIERS, diff --git a/tests/lib/rules/no-arrow-tests.js b/tests/lib/rules/no-arrow-tests.js index 0f656bcb..6ee28a52 100644 --- a/tests/lib/rules/no-arrow-tests.js +++ b/tests/lib/rules/no-arrow-tests.js @@ -188,26 +188,6 @@ ruleTester.run("no-arrow-tests", rule, { }, ], }, - { - code: "QUnit.module('module', { before: () => {} });", - output: "QUnit.module('module', { before: function() {} });", - errors: [ - { - messageId: "noArrowFunction", - type: "ArrowFunctionExpression", - }, - ], - }, - { - code: "QUnit.module('module', { after: () => {} });", - output: "QUnit.module('module', { after: function() {} });", - errors: [ - { - messageId: "noArrowFunction", - type: "ArrowFunctionExpression", - }, - ], - }, { code: "module('module', { setup: () => {} });", output: "module('module', { setup: function() {} });", diff --git a/tests/lib/rules/no-assert-equal.js b/tests/lib/rules/no-assert-equal.js index a5e7e4e5..5f225a2a 100644 --- a/tests/lib/rules/no-assert-equal.js +++ b/tests/lib/rules/no-assert-equal.js @@ -186,7 +186,7 @@ ruleTester.run("no-assert-equal", rule, { }, { // assert.equal in module hooks - code: "QUnit.module('My module', { before: function (assert) { assert.equal(1, 1); } });", + code: "QUnit.module('My module', { beforeEach: function (assert) { assert.equal(1, 1); } });", errors: [ { messageId: "unexpectedAssertEqual", @@ -194,15 +194,15 @@ ruleTester.run("no-assert-equal", rule, { suggestions: [ { messageId: "switchToDeepEqual", - output: "QUnit.module('My module', { before: function (assert) { assert.deepEqual(1, 1); } });", + output: "QUnit.module('My module', { beforeEach: function (assert) { assert.deepEqual(1, 1); } });", }, { messageId: "switchToPropEqual", - output: "QUnit.module('My module', { before: function (assert) { assert.propEqual(1, 1); } });", + output: "QUnit.module('My module', { beforeEach: function (assert) { assert.propEqual(1, 1); } });", }, { messageId: "switchToStrictEqual", - output: "QUnit.module('My module', { before: function (assert) { assert.strictEqual(1, 1); } });", + output: "QUnit.module('My module', { beforeEach: function (assert) { assert.strictEqual(1, 1); } });", }, ], }, diff --git a/tests/lib/rules/no-qunit-start-in-tests.js b/tests/lib/rules/no-qunit-start-in-tests.js index baf4e3d3..1cf24d5b 100644 --- a/tests/lib/rules/no-qunit-start-in-tests.js +++ b/tests/lib/rules/no-qunit-start-in-tests.js @@ -57,14 +57,6 @@ ruleTester.run("no-qunit-start-in-tests", rule, { code: 'QUnit.module("module", { afterEach: function() { QUnit.start(); } });', errors: [createError("afterEach hook")], }, - { - code: 'QUnit.module("module", { before: function() { QUnit.start(); } });', - errors: [createError("before hook")], - }, - { - code: 'QUnit.module("module", { after: function() { QUnit.start(); } });', - errors: [createError("after hook")], - }, { code: 'QUnit.module("module", { setup: function() { QUnit.start(); } });', errors: [createError("setup hook")], diff --git a/tests/lib/rules/no-setup-teardown.js b/tests/lib/rules/no-setup-teardown.js index 0a42d63d..ac20316b 100644 --- a/tests/lib/rules/no-setup-teardown.js +++ b/tests/lib/rules/no-setup-teardown.js @@ -29,14 +29,8 @@ ruleTester.run("no-setup-teardown", rule, { // afterEach "QUnit.module('Module', { afterEach: function () {} });", - // before - "QUnit.module('Module', { before: function () {} });", - - // after - "QUnit.module('Module', { after: function () {} });", - - // all - "QUnit.module('Module', { before: function () {}, beforeEach: function () {}, afterEach: function () {}, after: function () {} });", + // both + "QUnit.module('Module', { beforeEach: function () {}, afterEach: function () {} });", // other property names are not reported "QUnit.module('Module', { foo: function () {} });", diff --git a/tests/lib/rules/resolve-async.js b/tests/lib/rules/resolve-async.js index 35c43253..0768fdbe 100644 --- a/tests/lib/rules/resolve-async.js +++ b/tests/lib/rules/resolve-async.js @@ -195,10 +195,6 @@ ruleTester.run("resolve-async", rule, { code: "QUnit.module('name', { teardown: function () { QUnit.stop(); } });", errors: [createNeedStartCallsMessage("Property")], }, - { - code: "QUnit.module('name', { before: function () { QUnit.stop(); } });", - errors: [createNeedStartCallsMessage("Property")], - }, { code: "QUnit.module('name', { beforeEach: function () { QUnit.stop(); } });", errors: [createNeedStartCallsMessage("Property")], @@ -207,10 +203,6 @@ ruleTester.run("resolve-async", rule, { code: "QUnit.module('name', { afterEach: function () { QUnit.stop(); } });", errors: [createNeedStartCallsMessage("Property")], }, - { - code: "QUnit.module('name', { after: function () { QUnit.stop(); } });", - errors: [createNeedStartCallsMessage("Property")], - }, // Multiple start() calls needed {