Skip to content

Commit

Permalink
tools: add ESLint rule for assert.throws arguments
Browse files Browse the repository at this point in the history
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]>
  • Loading branch information
targos authored and Fishrock123 committed Dec 5, 2016
1 parent 6087e36 commit 64854f6
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 0 deletions.
1 change: 1 addition & 0 deletions .eslintrc
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ rules:
align-function-arguments: 2
align-multiline-assignment: 2
assert-fail-single-argument: 2
assert-throws-arguments: [2, { requireTwo: false }]
new-with-error: [2, Error, RangeError, TypeError, SyntaxError, ReferenceError]
no-useless-regex-char-class-escape: [2, { override: ['[', ']'] }]

Expand Down
59 changes: 59 additions & 0 deletions tools/eslint-rules/assert-throws-arguments.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
/**
* @fileoverview Check that assert.throws is never called with a string as
* second argument.
* @author Michaël Zasso
*/
'use strict';

//------------------------------------------------------------------------------
// Rule Definition
//------------------------------------------------------------------------------

function checkThrowsArguments(context, node) {
if (node.callee.type === 'MemberExpression' &&
node.callee.object.name === 'assert' &&
node.callee.property.name === 'throws') {
const args = node.arguments;
if (args.length > 3) {
context.report({
message: 'Too many arguments',
node: node
});
} else if (args.length > 1) {
const error = args[1];
if (error.type === 'Literal' && typeof error.value === 'string') {
context.report({
message: 'Unexpected string as second argument',
node: error
});
}
} else {
if (context.options[0].requireTwo) {
context.report({
message: 'Expected at least two arguments',
node: node
});
}
}
}
}

module.exports = {
meta: {
schema: [
{
type: 'object',
properties: {
requireTwo: {
type: 'boolean'
}
}
}
]
},
create: function(context) {
return {
CallExpression: (node) => checkThrowsArguments(context, node)
};
}
};

0 comments on commit 64854f6

Please sign in to comment.