Skip to content

Commit 3df61a7

Browse files
targosMylesBorins
authored andcommitted
tools: add ESLint rule for assert.throws arguments
The second argument to "assert.throws" is usually a validation RegExp or function for the thrown error. However, the function also accepts a string and in this case it is interpreted as a message for the AssertionError and not used for validation. It is common for people to forget this and pass a validation string by mistake. This new rule checks that we never pass a string literal as a second argument to "assert.throws". Additionally, there is an option to enforce the function to be called with at least two arguments. It is currently off because we have many tests that do not comply with this rule. PR-URL: #10089 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Teddy Katz <[email protected]> Reviewed-By: Rich Trott <[email protected]>
1 parent 34f0681 commit 3df61a7

File tree

2 files changed

+60
-0
lines changed

2 files changed

+60
-0
lines changed

.eslintrc

+1
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ rules:
9090
align-function-arguments: 2
9191
align-multiline-assignment: 2
9292
assert-fail-single-argument: 2
93+
assert-throws-arguments: [2, { requireTwo: false }]
9394
new-with-error: [2, "Error", "RangeError", "TypeError", "SyntaxError", "ReferenceError"]
9495

9596
# Global scoped method and vars
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
/**
2+
* @fileoverview Check that assert.throws is never called with a string as
3+
* second argument.
4+
* @author Michaël Zasso
5+
*/
6+
'use strict';
7+
8+
//------------------------------------------------------------------------------
9+
// Rule Definition
10+
//------------------------------------------------------------------------------
11+
12+
function checkThrowsArguments(context, node) {
13+
if (node.callee.type === 'MemberExpression' &&
14+
node.callee.object.name === 'assert' &&
15+
node.callee.property.name === 'throws') {
16+
const args = node.arguments;
17+
if (args.length > 3) {
18+
context.report({
19+
message: 'Too many arguments',
20+
node: node
21+
});
22+
} else if (args.length > 1) {
23+
const error = args[1];
24+
if (error.type === 'Literal' && typeof error.value === 'string') {
25+
context.report({
26+
message: 'Unexpected string as second argument',
27+
node: error
28+
});
29+
}
30+
} else {
31+
if (context.options[0].requireTwo) {
32+
context.report({
33+
message: 'Expected at least two arguments',
34+
node: node
35+
});
36+
}
37+
}
38+
}
39+
}
40+
41+
module.exports = {
42+
meta: {
43+
schema: [
44+
{
45+
type: 'object',
46+
properties: {
47+
requireTwo: {
48+
type: 'boolean'
49+
}
50+
}
51+
}
52+
]
53+
},
54+
create: function(context) {
55+
return {
56+
CallExpression: (node) => checkThrowsArguments(context, node)
57+
};
58+
}
59+
};

0 commit comments

Comments
 (0)