Skip to content

Commit

Permalink
New: no-hide-core-modules (fixes #66)
Browse files Browse the repository at this point in the history
  • Loading branch information
mysticatea committed Feb 23, 2017
1 parent 16973f9 commit c289f18
Show file tree
Hide file tree
Showing 11 changed files with 369 additions and 4 deletions.
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ Additional ESLint's rules for Node.js
|:------:|:--------:|:-----------------------------------------------------------------|:------------|
| | | [exports-style](docs/rules/exports-style.md) | Enforce either `module.exports` or `exports`.
| :star: | | [no-deprecated-api](docs/rules/no-deprecated-api.md) | Disallow deprecated API.
| | | [no-hide-core-modules](docs/rules/no-hide-core-modules.md) | Disallow third-party modules which are hiding core modules.
| | | [no-missing-import](docs/rules/no-missing-import.md) | Disallow `import` declarations for files that don't exist. :warning:
| :star: | | [no-missing-require](docs/rules/no-missing-require.md) | Disallow `require()`s for files that don't exist.
| :star: | | [no-unpublished-bin](docs/rules/no-unpublished-bin.md) | Disallow `bin` files that npm ignores.
Expand Down
58 changes: 58 additions & 0 deletions docs/rules/no-hide-core-modules.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
# Disallow third-party modules which are hiding core modules (node/no-hide-core-modules)

If you have dependencies which have the same name as core modules, your module would use the third-party modules instead of core modules.
Especially, if you depends on such modules indirectly and npm flattens dependencies, you can depend on such third-party modules before as you know it.
This might cause unintentional behaviors.

This rule warns `require()` expressions and `import` declarations if those import a third-party module which has the same name as core modules.

## Rule Details

:-1: Examples of **incorrect** code for this rule:

```js
/*eslint node/no-hide-core-modules: "error"*/

const util = require("util") // ERROR if `util` module exists in node_modules directory.
const path = require("path") // ERROR if `path` module exists in node_modules directory.
// ...
```

:+1: Examples of **correct** code for this rule:

```js
/*eslint node/no-hide-core-modules: "error"*/

const util = require("util") // OK if this is the core module 'util' surely.
const path = require("path") // OK if this is the core module 'path' surely.
```

## Options

```json
{
"node/no-hide-core-modules": ["error", {
"allow": [],
"ignoreDirectDependencies": false,
"ignoreIndirectDependencies": false,
}]
}
```

### allow

If you are sure that your module depends on the third-party module which has the same name as a core module, you can allow it by `allow` option.
E.g. `{"allow": ["util", "path"]}`.
Default is en empty array.

### ignoreDirectDependencies

If `ignoreDirectDependencies: true`, if the third-party module which has the same name as a core module exists in your `package.json`, this rule ignores it.

This option would allow all explicit dependencies which are hiding core modules.

### ignoreIndirectDependencies

If `ignoreIndirectDependencies: true`, if the third-party module which has the same name as a core module does not exist in your `package.json`, this rule ignores it.

This option would allow all implicit dependencies which are hiding core modules.
125 changes: 125 additions & 0 deletions lib/rules/no-hide-core-modules.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
/**
* @author Toru Nagashima
* @copyright 2016 Toru Nagashima. All rights reserved.
* See LICENSE file in root directory for full license.
*/
"use strict"

//------------------------------------------------------------------------------
// Requirements
//------------------------------------------------------------------------------

const path = require("path")
const resolve = require("resolve")
const getPackageJson = require("../util/get-package-json")
const getRequireTargets = require("../util/get-require-targets")
const getImportExportTargets = require("../util/get-import-export-targets")

//------------------------------------------------------------------------------
// Helpers
//------------------------------------------------------------------------------

const CORE_MODULES = new Set([
"assert", "buffer", "child_process", "cluster", "console", "constants",
"crypto", "dgram", "dns", /* "domain", */ "events", "fs", "http", "https",
"module", "net", "os", "path", /* "punycode", */ "querystring", "readline",
"repl", "stream", "string_decoder", "timers", "tls", "tty", "url", "util",
"vm", "zlib",
])
const BACK_SLASH = /\\/g

/**
* Creates AST event handlers for no-hide-core-modules.
*
* @param {RuleContext} context - The rule context.
* @returns {object} AST event handlers.
*/
function create(context) {
if (context.getFilename() === "<input>") {
return {}
}
const filePath = path.resolve(context.getFilename())
const dirPath = path.dirname(filePath)
const packageJson = getPackageJson(filePath)
const deps = new Set([].concat(
Object.keys((packageJson && packageJson.dependencies) || {}),
Object.keys((packageJson && packageJson.devDependencies) || {})
))
const options = context.options[0] || {}
const allow = options.allow || []
const ignoreDirectDependencies = Boolean(options.ignoreDirectDependencies)
const ignoreIndirectDependencies = Boolean(options.ignoreIndirectDependencies)

return {
"Program:exit"(node) {
const targets =
[].concat(
getRequireTargets(context, true),
getImportExportTargets(context, node, true)
)
.filter(t => CORE_MODULES.has(t.moduleName))

for (const target of targets) {
const name = target.moduleName
const allowed =
allow.indexOf(name) !== -1 ||
(ignoreDirectDependencies && deps.has(name)) ||
(ignoreIndirectDependencies && !deps.has(name))

if (allowed) {
continue
}

const resolved = resolve.sync(name, {basedir: dirPath})
const isCore = resolved === name

if (isCore) {
continue
}

context.report({
node: target.node,
loc: target.node.loc,
message: "Unexpected import of third-party module '{{name}}'.",
data: {
name: path
.relative(dirPath, resolved)
.replace(BACK_SLASH, "/"),
},
})
}
},
}
}

//------------------------------------------------------------------------------
// Rule Definition
//------------------------------------------------------------------------------

module.exports = {
create,
meta: {
docs: {
description: "disallow third-party modules which are hiding core modules",
category: "Possible Errors",
recommended: false,
},
fixable: false,
schema: [
{
type: "object",
properties: {
allow: {
type: "array",
items: {enum: Array.from(CORE_MODULES)},
additionalItems: false,
uniqueItems: true,
},
ignoreDirectDependencies: {type: "boolean"},
ignoreIndirectDependencies: {type: "boolean"},
},
additionalProperties: false,
},
],
},
}
5 changes: 3 additions & 2 deletions lib/util/get-import-export-targets.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,10 @@ const MODULE_TYPE = /^(?:Import|Export(?:Named|Default|All))Declaration$/
*
* @param {RuleContext} context - The rule context.
* @param {ASTNode} programNode - The node of Program.
* @param {boolean} includeCore - The flag to include core modules.
* @returns {ImportTarget[]} A list of found target's information.
*/
module.exports = function getImportExportTargets(context, programNode) {
module.exports = function getImportExportTargets(context, programNode, includeCore) {
const retv = []
const basedir = path.dirname(path.resolve(context.getFilename()))
const extensions = getTryExtensions(context)
Expand All @@ -48,7 +49,7 @@ module.exports = function getImportExportTargets(context, programNode) {
// Gets the target module.
const node = statement.source
const name = node && stripImportPathParams(node.value)
if (name && !resolve.isCore(name)) {
if (name && (includeCore || !resolve.isCore(name))) {
retv.push(new ImportTarget(node, name, basedir, extensions))
}
}
Expand Down
5 changes: 3 additions & 2 deletions lib/util/get-require-targets.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,10 @@ function getReferencesOfRequire(scope) {
* Core modules of Node.js (e.g. `fs`, `http`) are excluded.
*
* @param {RuleContext} context - The rule context.
* @param {boolean} includeCore - The flag to include core modules.
* @returns {ImportTarget[]} A list of found target's information.
*/
module.exports = function getRequireTargets(context) {
module.exports = function getRequireTargets(context, includeCore) {
const retv = []
const basedir = path.dirname(path.resolve(context.getFilename()))
const references = getReferencesOfRequire(context.getScope())
Expand All @@ -75,7 +76,7 @@ module.exports = function getRequireTargets(context) {
const targetNode = node.parent.arguments[0]
const rawName = getValueIfString(targetNode)
const name = rawName && stripImportPathParams(rawName)
if (name && !resolve.isCore(name)) {
if (name && (includeCore || !resolve.isCore(name))) {
retv.push(new ImportTarget(targetNode, name, basedir, extensions))
}
}
Expand Down
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"dependencies": {
}
}
Empty file.
Empty file.
5 changes: 5 additions & 0 deletions tests/fixtures/no-hide-core-modules/thirdparty/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"dependencies": {
"util": "0.0.0"
}
}
Loading

0 comments on commit c289f18

Please sign in to comment.