Skip to content

Commit 01157b1

Browse files
arensmanarensman
arensman
and
arensman
authored
fix: Flag assert.equal in module hooks in no-assert-equal rule (#570)
* 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 * Remove parser option * Fix type checks * Remove detecting after and before --------- Co-authored-by: arensman <[email protected]>
1 parent 55c2a8e commit 01157b1

File tree

6 files changed

+87
-47
lines changed

6 files changed

+87
-47
lines changed

‎lib/rules/no-assert-equal.js

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,41 @@ module.exports = {
159159
});
160160
}
161161

162+
/**
163+
* @param {import('eslint').Rule.Node} propertyNode
164+
* @returns {boolean}
165+
*/
166+
function isModuleHookCallback(propertyNode) {
167+
return (
168+
propertyNode.parent &&
169+
propertyNode.parent.type === "Property" &&
170+
utils.isModuleHookPropertyKey(propertyNode.parent.key) &&
171+
utils.isInModule(propertyNode.parent)
172+
);
173+
}
174+
175+
/**
176+
* @param {import('eslint').Rule.Node} propertyNode
177+
* @returns {void}
178+
*/
179+
function pushModuleHookAssert(propertyNode) {
180+
if (isModuleHookCallback(propertyNode)) {
181+
testStack.push({
182+
assertVar: utils.getAssertContextName(propertyNode),
183+
});
184+
}
185+
}
186+
187+
/**
188+
* @param {import('eslint').Rule.Node} propertyNode
189+
* @returns {void}
190+
*/
191+
function popModuleHookAssert(propertyNode) {
192+
if (isModuleHookCallback(propertyNode)) {
193+
testStack.pop();
194+
}
195+
}
196+
162197
return {
163198
CallExpression: function (node) {
164199
/* istanbul ignore else: correctly does nothing */
@@ -188,6 +223,15 @@ module.exports = {
188223
testStack.pop();
189224
}
190225
},
226+
227+
FunctionDeclaration: pushModuleHookAssert,
228+
FunctionExpression: pushModuleHookAssert,
229+
ArrowFunctionExpression: pushModuleHookAssert,
230+
231+
"FunctionDeclaration:exit": popModuleHookAssert,
232+
"FunctionExpression:exit": popModuleHookAssert,
233+
"ArrowFunctionExpression:exit": popModuleHookAssert,
234+
191235
Program: function (node) {
192236
// Gather all calls to global `equal()`.
193237
/* istanbul ignore next: deprecated code paths only followed by old eslint versions */

‎lib/rules/no-qunit-start-in-tests.js

Lines changed: 2 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -51,20 +51,6 @@ module.exports = {
5151
);
5252
}
5353

54-
/**
55-
* @param {import('eslint').Rule.Node} propertyNode
56-
* @returns {boolean}
57-
*/
58-
function isInModule(propertyNode) {
59-
return (
60-
propertyNode &&
61-
propertyNode.parent && // ObjectExpression
62-
propertyNode.parent.parent && // CallExpression?
63-
propertyNode.parent.parent.type === "CallExpression" &&
64-
utils.isModule(propertyNode.parent.parent.callee)
65-
);
66-
}
67-
6854
//----------------------------------------------------------------------
6955
// Public
7056
//----------------------------------------------------------------------
@@ -93,7 +79,7 @@ module.exports = {
9379
Property: function (node) {
9480
if (
9581
utils.isModuleHookPropertyKey(node.key) &&
96-
isInModule(node) &&
82+
utils.isInModule(node) &&
9783
node.key.type === "Identifier"
9884
) {
9985
contextStack.push(`${node.key.name} hook`);
@@ -109,7 +95,7 @@ module.exports = {
10995
"Property:exit": function (node) {
11096
if (
11197
utils.isModuleHookPropertyKey(node.key) &&
112-
isInModule(node)
98+
utils.isInModule(node)
11399
) {
114100
contextStack.pop();
115101
}

‎lib/rules/no-setup-teardown.js

Lines changed: 1 addition & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -68,25 +68,11 @@ module.exports = {
6868
}
6969
}
7070

71-
/**
72-
* @param {import('eslint').Rule.Node} propertyNode
73-
* @returns {boolean}
74-
*/
75-
function isInModule(propertyNode) {
76-
return (
77-
propertyNode &&
78-
propertyNode.parent && // ObjectExpression
79-
propertyNode.parent.parent && // CallExpression?
80-
propertyNode.parent.parent.type === "CallExpression" &&
81-
utils.isModule(propertyNode.parent.parent.callee)
82-
);
83-
}
84-
8571
return {
8672
Property: function (node) {
8773
if (
8874
utils.isModuleHookPropertyKey(node.key) &&
89-
isInModule(node)
75+
utils.isInModule(node)
9076
) {
9177
checkModuleHook(node);
9278
}

‎lib/rules/resolve-async.js

Lines changed: 2 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -159,20 +159,6 @@ module.exports = {
159159
}
160160
}
161161

162-
/**
163-
* @param {import('eslint').Rule.Node} propertyNode
164-
* @returns {boolean}
165-
*/
166-
function isInModule(propertyNode) {
167-
return (
168-
propertyNode &&
169-
propertyNode.parent && // ObjectExpression
170-
propertyNode.parent.parent && // CallExpression?
171-
propertyNode.parent.parent.type === "CallExpression" &&
172-
utils.isModule(propertyNode.parent.parent.callee)
173-
);
174-
}
175-
176162
return {
177163
CallExpression: function (node) {
178164
const callbackVar = getAsyncCallbackVarOrNull(node.callee);
@@ -213,7 +199,7 @@ module.exports = {
213199
Property: function (node) {
214200
if (
215201
utils.isModuleHookPropertyKey(node.key) &&
216-
isInModule(node)
202+
utils.isInModule(node)
217203
) {
218204
asyncStateStack.push({
219205
stopSemaphoreCount: 0,
@@ -228,7 +214,7 @@ module.exports = {
228214
"Property:exit": function (node) {
229215
if (
230216
utils.isModuleHookPropertyKey(node.key) &&
231-
isInModule(node)
217+
utils.isInModule(node)
232218
) {
233219
const asyncState = asyncStateStack.pop();
234220
if (!asyncState) {

‎lib/utils.js

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -216,6 +216,20 @@ exports.isModule = function (calleeNode) {
216216
return result;
217217
};
218218

219+
/**
220+
* @param {import('eslint').Rule.Node} propertyNode
221+
* @returns {boolean}
222+
*/
223+
exports.isInModule = function (propertyNode) {
224+
return (
225+
propertyNode &&
226+
propertyNode.parent && // ObjectExpression
227+
propertyNode.parent.parent && // CallExpression?
228+
propertyNode.parent.parent.type === "CallExpression" &&
229+
exports.isModule(propertyNode.parent.parent.callee)
230+
);
231+
};
232+
219233
/**
220234
* @param {import('estree').Node} identifierNode
221235
* @returns {boolean}

‎tests/lib/rules/no-assert-equal.js

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -184,5 +184,29 @@ ruleTester.run("no-assert-equal", rule, {
184184
},
185185
],
186186
},
187+
{
188+
// assert.equal in module hooks
189+
code: "QUnit.module('My module', { beforeEach: function (assert) { assert.equal(1, 1); } });",
190+
errors: [
191+
{
192+
messageId: "unexpectedAssertEqual",
193+
data: { assertVar: "assert" },
194+
suggestions: [
195+
{
196+
messageId: "switchToDeepEqual",
197+
output: "QUnit.module('My module', { beforeEach: function (assert) { assert.deepEqual(1, 1); } });",
198+
},
199+
{
200+
messageId: "switchToPropEqual",
201+
output: "QUnit.module('My module', { beforeEach: function (assert) { assert.propEqual(1, 1); } });",
202+
},
203+
{
204+
messageId: "switchToStrictEqual",
205+
output: "QUnit.module('My module', { beforeEach: function (assert) { assert.strictEqual(1, 1); } });",
206+
},
207+
],
208+
},
209+
],
210+
},
187211
],
188212
});

0 commit comments

Comments
 (0)