Skip to content

Commit

Permalink
lib: remove env: node in eslint config for lib files
Browse files Browse the repository at this point in the history
This patch removes the redundant `require-globals` custom
eslint rule by removing `env: node` in the eslint config
and whitelist the globals that can be accessed in native
modules instead of black listing them. This makes sense
for our `lib/` files because here we are creating the
Node.js environment instead of running in a normal user
land Node.js environment.

PR-URL: #27082
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
  • Loading branch information
joyeecheung committed Apr 6, 2019
1 parent 864860e commit de23055
Show file tree
Hide file tree
Showing 12 changed files with 57 additions and 117 deletions.
3 changes: 2 additions & 1 deletion .eslintrc.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
'use strict';

/* eslint-env node */

const Module = require('module');
const path = require('path');

Expand Down Expand Up @@ -31,7 +33,6 @@ Module._findPath = (request, paths, isMain) => {
module.exports = {
root: true,
plugins: ['markdown', 'node-core'],
env: { node: true, es6: true },
parser: 'babel-eslint',
parserOptions: { sourceType: 'script' },
overrides: [
Expand Down
4 changes: 4 additions & 0 deletions benchmark/.eslintrc.yaml
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
## Benchmark-specific linter rules

env:
node: true
es6: true

rules:
comma-dangle:
- error
Expand Down
22 changes: 21 additions & 1 deletion lib/.eslintrc.yaml
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
env:
es6: true

rules:
prefer-object-spread: error
no-buffer-constructor: error
Expand All @@ -19,10 +22,11 @@ rules:
- selector: "CallExpression[callee.object.name='Error'][callee.property.name='captureStackTrace']"
message: "Please use `require('internal/errors').hideStackFrames()` instead."
# Custom rules in tools/eslint-rules
node-core/require-globals: error
node-core/lowercase-name-for-primitive: error
node-core/non-ascii-character: error
globals:
Intl: false
# Assertions
CHECK: false
CHECK_EQ: false
CHECK_GE: false
Expand All @@ -37,5 +41,21 @@ globals:
DCHECK_LE: false
DCHECK_LT: false
DCHECK_NE: false
# Parameters passed to internal modules
global: false
require: false
process: false
exports: false
module: false
internalBinding: false
primordials: false
# Globals
# TODO(joyeecheung): if possible, get these in native modules
# through `require` instead of grabbing them from the global proxy.
clearTimeout: false
setTimeout: false
clearInterval: false
setInterval: false
setImmediate: false
clearImmediate: false
console: false
23 changes: 15 additions & 8 deletions lib/buffer.js
Original file line number Diff line number Diff line change
Expand Up @@ -654,9 +654,10 @@ Buffer.prototype.equals = function equals(otherBuffer) {
return _compare(this, otherBuffer) === 0;
};

let INSPECT_MAX_BYTES = 50;
// Override how buffers are presented by util.inspect().
Buffer.prototype[customInspectSymbol] = function inspect(recurseTimes, ctx) {
const max = exports.INSPECT_MAX_BYTES;
const max = INSPECT_MAX_BYTES;
const actualMax = Math.min(max, this.length);
const remaining = this.length - max;
let str = this.hexSlice(0, actualMax).replace(/(.{2})/g, '$1 ').trim();
Expand Down Expand Up @@ -1089,19 +1090,25 @@ if (internalBinding('config').hasIntl) {
};
}

module.exports = exports = {
module.exports = {
Buffer,
SlowBuffer,
transcode,
INSPECT_MAX_BYTES: 50,

// Legacy
kMaxLength,
kStringMaxLength
};

Object.defineProperty(exports, 'constants', {
configurable: false,
enumerable: true,
value: constants
Object.defineProperties(module.exports, {
constants: {
configurable: false,
enumerable: true,
value: constants
},
INSPECT_MAX_BYTES: {
configurable: true,
enumerable: true,
get() { return INSPECT_MAX_BYTES; },
set(val) { INSPECT_MAX_BYTES = val; }
}
});
4 changes: 2 additions & 2 deletions lib/crypto.js
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ function createVerify(algorithm, options) {
return new Verify(algorithm, options);
}

module.exports = exports = {
module.exports = {
// Methods
createCipheriv,
createDecipheriv,
Expand Down Expand Up @@ -218,7 +218,7 @@ function getFipsForced() {
return 1;
}

Object.defineProperties(exports, {
Object.defineProperties(module.exports, {
createCipher: {
enumerable: false,
value: deprecate(createCipher,
Expand Down
1 change: 1 addition & 0 deletions lib/internal/bootstrap/loaders.js
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ const internalBindingWhitelist = new SafeSet([
let internalBinding;
{
const bindingObj = Object.create(null);
// eslint-disable-next-line no-global-assign
internalBinding = function internalBinding(module) {
let mod = bindingObj[module];
if (typeof mod !== 'object') {
Expand Down
2 changes: 1 addition & 1 deletion lib/internal/modules/cjs/helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ function normalizeReferrerURL(referrer) {
return new URL(referrer).href;
}

module.exports = exports = {
module.exports = {
addBuiltinLibsToObject,
builtinLibs,
makeRequireFunction,
Expand Down
6 changes: 3 additions & 3 deletions lib/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,8 @@ function timestamp() {
}

// Log is just a thin wrapper to console.log that prepends a timestamp
function log() {
console.log('%s - %s', timestamp(), exports.format.apply(exports, arguments));
function log(...args) {
console.log('%s - %s', timestamp(), format(...args));
}

/**
Expand Down Expand Up @@ -218,7 +218,7 @@ function getSystemErrorName(err) {
}

// Keep the `exports =` so that various functions can still be monkeypatched
module.exports = exports = {
module.exports = {
_errnoException: errnoException,
_exceptionWithHostPort: exceptionWithHostPort,
_extend,
Expand Down
4 changes: 4 additions & 0 deletions test/.eslintrc.yaml
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
## Test-specific linter rules

env:
node: true
es6: true

rules:
# ECMAScript 6
# http://eslint.org/docs/rules/#ecmascript-6
Expand Down
51 changes: 0 additions & 51 deletions test/parallel/test-eslint-require-buffer.js

This file was deleted.

4 changes: 4 additions & 0 deletions tools/.eslintrc.yaml
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
env:
node: true
es6: true

rules:
comma-dangle:
- error
Expand Down
50 changes: 0 additions & 50 deletions tools/eslint-rules/require-globals.js

This file was deleted.

0 comments on commit de23055

Please sign in to comment.