Skip to content

Commit 70c9e43

Browse files
Trottjasnell
authored andcommitted
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: #3157 Reviewed-By: Rod Vagg <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
1 parent b717956 commit 70c9e43

6 files changed

+110
-2
lines changed

test/.eslintrc

+2
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ rules:
55
no-undef: 0
66
## allow global Buffer usage
77
require-buffer: 0
8+
## common module is mandatory in tests
9+
required-modules: [2, "common"]
810

911
globals:
1012
gc: false

test/common.js

+1
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
/* eslint-disable required-modules */
12
'use strict';
23
var path = require('path');
34
var fs = require('fs');

test/parallel/test-domain-crypto.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
/* eslint-disable strict */
1+
/* eslint-disable strict, required-modules */
22
try {
33
var crypto = require('crypto');
44
} catch (e) {

test/parallel/test-regression-object-prototype.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1+
/* eslint-disable required-modules */
12
'use strict';
2-
//console.log('puts before');
33

44
Object.prototype.xadsadsdasasdxx = function() {
55
};

test/parallel/test-repl-autolibs.js

+1
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
/* eslint-disable required-modules */
12
'use strict';
23
var assert = require('assert');
34
var util = require('util');
+104
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,104 @@
1+
/**
2+
* @fileoverview Require usage of specified node modules.
3+
* @author Rich Trott
4+
*/
5+
'use strict';
6+
7+
var path = require('path');
8+
9+
//------------------------------------------------------------------------------
10+
// Rule Definition
11+
//------------------------------------------------------------------------------
12+
13+
module.exports = function(context) {
14+
// trim required module names
15+
var requiredModules = context.options;
16+
17+
var foundModules = [];
18+
19+
// if no modules are required we don't need to check the CallExpressions
20+
if (requiredModules.length === 0) {
21+
return {};
22+
}
23+
24+
/**
25+
* Function to check if a node is a string literal.
26+
* @param {ASTNode} node The node to check.
27+
* @returns {boolean} If the node is a string literal.
28+
*/
29+
function isString(node) {
30+
return node && node.type === 'Literal' && typeof node.value === 'string';
31+
}
32+
33+
/**
34+
* Function to check if a node is a require call.
35+
* @param {ASTNode} node The node to check.
36+
* @returns {boolean} If the node is a require call.
37+
*/
38+
function isRequireCall(node) {
39+
return node.callee.type === 'Identifier' && node.callee.name === 'require';
40+
}
41+
42+
/**
43+
* Function to check if a node has an argument that is a required module and
44+
* return its name.
45+
* @param {ASTNode} node The node to check
46+
* @returns {undefined|String} required module name or undefined
47+
*/
48+
function getRequiredModuleName(node) {
49+
var moduleName;
50+
51+
// node has arguments and first argument is string
52+
if (node.arguments.length && isString(node.arguments[0])) {
53+
var argValue = path.basename(node.arguments[0].value.trim());
54+
55+
// check if value is in required modules array
56+
if (requiredModules.indexOf(argValue) !== -1) {
57+
moduleName = argValue;
58+
}
59+
}
60+
61+
return moduleName;
62+
}
63+
64+
return {
65+
'CallExpression': function(node) {
66+
if (isRequireCall(node)) {
67+
var requiredModuleName = getRequiredModuleName(node);
68+
69+
if (requiredModuleName) {
70+
foundModules.push(requiredModuleName);
71+
}
72+
}
73+
},
74+
'Program:exit': function(node) {
75+
if (foundModules.length < requiredModules.length) {
76+
var missingModules = requiredModules.filter(
77+
function(module) {
78+
return foundModules.indexOf(module === -1);
79+
}
80+
);
81+
missingModules.forEach(function(moduleName) {
82+
context.report(
83+
node,
84+
'Mandatory module "{{moduleName}}" must be loaded.',
85+
{ moduleName: moduleName }
86+
);
87+
});
88+
}
89+
}
90+
};
91+
};
92+
93+
module.exports.schema = {
94+
'type': 'array',
95+
'items': [
96+
{
97+
'enum': [0, 1, 2]
98+
}
99+
],
100+
'additionalItems': {
101+
'type': 'string'
102+
},
103+
'uniqueItems': true
104+
};

0 commit comments

Comments
 (0)