Skip to content

Commit

Permalink
test: make common.js mandatory via linting rule
Browse files Browse the repository at this point in the history
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: #3157
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
  • Loading branch information
Trott committed Oct 6, 2015
1 parent c78091d commit 3de353b
Show file tree
Hide file tree
Showing 6 changed files with 110 additions and 2 deletions.
2 changes: 2 additions & 0 deletions test/.eslintrc
Original file line number Diff line number Diff line change
Expand Up @@ -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
1 change: 1 addition & 0 deletions test/common.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/* eslint-disable required-modules */
'use strict';
var path = require('path');
var fs = require('fs');
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-domain-crypto.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/* eslint-disable strict */
/* eslint-disable strict, required-modules */
try {
var crypto = require('crypto');
} catch (e) {
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-regression-object-prototype.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/* eslint-disable required-modules */
'use strict';
//console.log('puts before');

Object.prototype.xadsadsdasasdxx = function() {
};
Expand Down
1 change: 1 addition & 0 deletions test/parallel/test-repl-autolibs.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/* eslint-disable required-modules */
'use strict';
var assert = require('assert');
var util = require('util');
Expand Down
104 changes: 104 additions & 0 deletions tools/eslint-rules/required-modules.js
Original file line number Diff line number Diff line change
@@ -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
};

0 comments on commit 3de353b

Please sign in to comment.