From 70c9e4337e032d4d9334725cb280d5674293b764 Mon Sep 17 00:00:00 2001
From: Rich Trott <rtrott@gmail.com>
Date: Thu, 1 Oct 2015 21:38:16 -0700
Subject: [PATCH] test: make common.js mandatory via linting rule

test/common.js contains code that detects global variable leaks.

This eslint rule checks that a module named `common` is loaded. It is
only applicable to files in the test directory. Tests that intentionally
leak variables can opt out with an eslint-disable comment.

PR-URL: https://github.com/nodejs/node/pull/3157
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
---
 test/.eslintrc                                |   2 +
 test/common.js                                |   1 +
 test/parallel/test-domain-crypto.js           |   2 +-
 .../test-regression-object-prototype.js       |   2 +-
 test/parallel/test-repl-autolibs.js           |   1 +
 tools/eslint-rules/required-modules.js        | 104 ++++++++++++++++++
 6 files changed, 110 insertions(+), 2 deletions(-)
 create mode 100644 tools/eslint-rules/required-modules.js

diff --git a/test/.eslintrc b/test/.eslintrc
index 2a8f61cb35f6be..5d77a660f88762 100644
--- a/test/.eslintrc
+++ b/test/.eslintrc
@@ -5,6 +5,8 @@ rules:
   no-undef: 0
   ## allow global Buffer usage
   require-buffer: 0
+  ## common module is mandatory in tests
+  required-modules: [2, "common"]
 
 globals:
   gc: false
diff --git a/test/common.js b/test/common.js
index 3ed7ce31d8c17b..c1c488c4ead15a 100644
--- a/test/common.js
+++ b/test/common.js
@@ -1,3 +1,4 @@
+/* eslint-disable required-modules */
 'use strict';
 var path = require('path');
 var fs = require('fs');
diff --git a/test/parallel/test-domain-crypto.js b/test/parallel/test-domain-crypto.js
index 00de1aee272b7b..e76e8d08791c87 100644
--- a/test/parallel/test-domain-crypto.js
+++ b/test/parallel/test-domain-crypto.js
@@ -1,4 +1,4 @@
-/* eslint-disable strict */
+/* eslint-disable strict, required-modules */
 try {
   var crypto = require('crypto');
 } catch (e) {
diff --git a/test/parallel/test-regression-object-prototype.js b/test/parallel/test-regression-object-prototype.js
index 040e718948a581..b1411bf813687a 100644
--- a/test/parallel/test-regression-object-prototype.js
+++ b/test/parallel/test-regression-object-prototype.js
@@ -1,5 +1,5 @@
+/* eslint-disable required-modules */
 'use strict';
-//console.log('puts before');
 
 Object.prototype.xadsadsdasasdxx = function() {
 };
diff --git a/test/parallel/test-repl-autolibs.js b/test/parallel/test-repl-autolibs.js
index 4103a19243aec5..e37f2d036ece4d 100644
--- a/test/parallel/test-repl-autolibs.js
+++ b/test/parallel/test-repl-autolibs.js
@@ -1,3 +1,4 @@
+/* eslint-disable required-modules */
 'use strict';
 var assert = require('assert');
 var util = require('util');
diff --git a/tools/eslint-rules/required-modules.js b/tools/eslint-rules/required-modules.js
new file mode 100644
index 00000000000000..647726bf9a2424
--- /dev/null
+++ b/tools/eslint-rules/required-modules.js
@@ -0,0 +1,104 @@
+/**
+ * @fileoverview Require usage of specified node modules.
+ * @author Rich Trott
+ */
+'use strict';
+
+var path = require('path');
+
+//------------------------------------------------------------------------------
+// Rule Definition
+//------------------------------------------------------------------------------
+
+module.exports = function(context) {
+  // trim required module names
+  var requiredModules = context.options;
+
+  var foundModules = [];
+
+  // if no modules are required we don't need to check the CallExpressions
+  if (requiredModules.length === 0) {
+    return {};
+  }
+
+  /**
+   * Function to check if a node is a string literal.
+   * @param {ASTNode} node The node to check.
+   * @returns {boolean} If the node is a string literal.
+   */
+  function isString(node) {
+    return node && node.type === 'Literal' && typeof node.value === 'string';
+  }
+
+  /**
+   * Function to check if a node is a require call.
+   * @param {ASTNode} node The node to check.
+   * @returns {boolean} If the node is a require call.
+   */
+  function isRequireCall(node) {
+    return node.callee.type === 'Identifier' && node.callee.name === 'require';
+  }
+
+  /**
+   * Function to check if a node has an argument that is a required module and
+   * return its name.
+   * @param {ASTNode} node The node to check
+   * @returns {undefined|String} required module name or undefined
+   */
+  function getRequiredModuleName(node) {
+    var moduleName;
+
+    // node has arguments and first argument is string
+    if (node.arguments.length && isString(node.arguments[0])) {
+      var argValue = path.basename(node.arguments[0].value.trim());
+
+      // check if value is in required modules array
+      if (requiredModules.indexOf(argValue) !== -1) {
+        moduleName = argValue;
+      }
+    }
+
+    return moduleName;
+  }
+
+  return {
+    'CallExpression': function(node) {
+      if (isRequireCall(node)) {
+        var requiredModuleName = getRequiredModuleName(node);
+
+        if (requiredModuleName) {
+          foundModules.push(requiredModuleName);
+        }
+      }
+    },
+    'Program:exit': function(node) {
+      if (foundModules.length < requiredModules.length) {
+        var missingModules = requiredModules.filter(
+          function(module) {
+            return foundModules.indexOf(module === -1);
+          }
+          );
+        missingModules.forEach(function(moduleName) {
+          context.report(
+            node,
+            'Mandatory module "{{moduleName}}" must be loaded.',
+            { moduleName: moduleName }
+            );
+        });
+      }
+    }
+  };
+};
+
+module.exports.schema = {
+  'type': 'array',
+  'items': [
+  {
+    'enum': [0, 1, 2]
+  }
+  ],
+  'additionalItems': {
+    'type': 'string'
+  },
+  'uniqueItems': true
+};