From 9c85d0f396da79b1279f0ac31d2ac993e9cee178 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Micha=C3=ABl=20Zasso?= <targos@protonmail.com>
Date: Sat, 26 Nov 2016 11:31:10 +0100
Subject: [PATCH] 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: https://github.com/nodejs/node/pull/10089
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Teddy Katz <teddy.katz@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
---
 .eslintrc                                     |  1 +
 tools/eslint-rules/assert-throws-arguments.js | 59 +++++++++++++++++++
 2 files changed, 60 insertions(+)
 create mode 100644 tools/eslint-rules/assert-throws-arguments.js

diff --git a/.eslintrc b/.eslintrc
index fdfdaf0833..e966348f5f 100644
--- a/.eslintrc
+++ b/.eslintrc
@@ -120,6 +120,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]
 
 # Global scoped method and vars
diff --git a/tools/eslint-rules/assert-throws-arguments.js b/tools/eslint-rules/assert-throws-arguments.js
new file mode 100644
index 0000000000..434a0ca455
--- /dev/null
+++ b/tools/eslint-rules/assert-throws-arguments.js
@@ -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)
+    };
+  }
+};