Merge pull request #241 from jfmengels/no-extraneous-dependencies

Add `no-extraneous-dependencies` rule

Author: benmosher

CHANGELOG.md

@@ -6,6 +6,7 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel
 ## [Unreleased]
 ### Added
 - [`no-named-as-default-member`] to `warnings` canned config
+- add [`no-extraneous-dependencies`] rule
 
 ## [1.5.0] - 2016-04-18
 ### Added

README.md

@@ -48,10 +48,12 @@ This plugin intends to support linting of ES2015+ (ES6+) import/export syntax, a
 * Ensure all imports appear before other statements ([`imports-first`])
 * Report repeated import of the same module in multiple places ([`no-duplicates`])
 * Report namespace imports ([`no-namespace`])
+* Forbid the use of extraneous packages ([`no-extraneous-dependencies`])
 
 [`imports-first`]: ./docs/rules/imports-first.md
 [`no-duplicates`]: ./docs/rules/no-duplicates.md
 [`no-namespace`]: ./docs/rules/no-namespace.md
+[`no-extraneous-dependencies`]: ./docs/rules/no-extraneous-dependencies.md
 
 
 ## Installation

docs/rules/no-extraneous-dependencies.md

@@ -0,0 +1,68 @@
+# Forbid the use of extraneous packages
+
+Forbid the import of external modules that are not declared in the `package.json`'s `dependencies` or `devDependencies`.
+The closest parent `package.json` will be used. If no `package.json` is found, the rule will not lint anything.
+
+### Options
+
+This rule supports the following options:
+
+`devDependencies`: If set to `false`, then the rule will show an error when `devDependencies` are imported. Defaults to `true`.
+
+You can set the options like this:
+
+```js
+"import/no-extraneous-dependencies": ["error", {"devDependencies": false}]
+```
+
+
+## Rule Details
+
+Given the following `package.json`:
+```json
+{
+  "name": "my-project",
+  "...": "...",
+  "dependencies": {
+    "builtin-modules": "^1.1.1",
+    "lodash.cond": "^4.2.0",
+    "lodash.find": "^4.2.0",
+    "pkg-up": "^1.0.0"
+  },
+  "devDependencies": {
+    "ava": "^0.13.0",
+    "eslint": "^2.4.0",
+    "eslint-plugin-ava": "^1.3.0",
+    "xo": "^0.13.0"
+  }
+}
+```
+
+
+## Fail
+
+```js
+var _ = require('lodash');
+import _ from 'lodash';
+
+/* eslint import/no-extraneous-dependencies: ["error", {"devDependencies": false}] */
+import test from 'ava';
+var test = require('ava');
+```
+
+
+## Pass
+
+```js
+// Builtin and internal modules are fine
+var path = require('path');
+var foo = require('./foo');
+
+import test from 'ava';
+import find from 'lodash.find';
+```
+
+
+## When Not To Use It
+
+If you do not have a `package.json` file in your project.

package.json

@@ -65,11 +65,14 @@
     "eslint": "2.x"
   },
   "dependencies": {
+    "builtin-modules": "^1.1.1",
     "doctrine": "1.2.0",
     "es6-map": "^0.1.3",
     "es6-set": "^0.1.4",
     "es6-symbol": "*",
     "eslint-import-resolver-node": "^0.2.0",
-    "object-assign": "^4.0.1"
+    "lodash.cond": "^4.3.0",
+    "object-assign": "^4.0.1",
+    "pkg-up": "^1.0.0"
   }
 }

src/core/importType.js

@@ -0,0 +1,52 @@
+import cond from 'lodash.cond'
+import builtinModules from 'builtin-modules'
+import { basename, join } from 'path'
+
+import resolve from './resolve'
+
+function constant(value) {
+  return () => value
+}
+
+function isBuiltIn(name) {
+  return builtinModules.indexOf(name) !== -1
+}
+
+const externalModuleRegExp = /^\w/
+function isExternalModule(name, path) {
+  if (!externalModuleRegExp.test(name)) return false
+  return (!path || -1 < path.indexOf(join('node_modules', name)))
+}
+
+function isProjectModule(name, path) {
+  if (!externalModuleRegExp.test(name)) return false
+  return (path && -1 === path.indexOf(join('node_modules', name)))
+}
+
+function isRelativeToParent(name) {
+  return name.indexOf('../') === 0
+}
+
+const indexFiles = ['.', './', './index', './index.js']
+function isIndex(name, path) {
+  if (path) return basename(path).split('.')[0] === 'index'
+  return indexFiles.indexOf(name) !== -1
+}
+
+function isRelativeToSibling(name) {
+  return name.indexOf('./') === 0
+}
+
+const typeTest = cond([
+  [isBuiltIn, constant('builtin')],
+  [isExternalModule, constant('external')],
+  [isProjectModule, constant('project')],
+  [isRelativeToParent, constant('parent')],
+  [isIndex, constant('index')],
+  [isRelativeToSibling, constant('sibling')],
+  [constant(true), constant('unknown')],
+])
+
+export default function resolveImportType(name, context) {
+  return typeTest(name, resolve(name, context))
+}

src/core/staticRequire.js

@@ -0,0 +1,8 @@
+// todo: merge with module visitor
+export default function isStaticRequire(node) {
+  return node &&
+    node.callee.type === 'Identifier' &&
+    node.callee.name === 'require' &&
+    node.arguments.length === 1 &&
+    node.arguments[0].type === 'Literal'
+}

src/index.js

@@ -13,6 +13,7 @@ export const rules = {
   'no-amd': require('./rules/no-amd'),
   'no-duplicates': require('./rules/no-duplicates'),
   'imports-first': require('./rules/imports-first'),
+  'no-extraneous-dependencies': require('./rules/no-extraneous-dependencies'),
 
   // metadata-based
   'no-deprecated': require('./rules/no-deprecated'),

src/rules/no-extraneous-dependencies.js

@@ -0,0 +1,77 @@
+import fs from 'fs'
+import pkgUp from 'pkg-up'
+import importType from '../core/importType'
+import isStaticRequire from '../core/staticRequire'
+
+function getDependencies(context) {
+  const filepath = pkgUp.sync(context.getFilename())
+  if (!filepath) {
+    return null
+  }
+
+  try {
+    const packageContent = JSON.parse(fs.readFileSync(filepath, 'utf8'))
+    return {
+      dependencies: packageContent.dependencies || {},
+      devDependencies: packageContent.devDependencies || {},
+    }
+  } catch (e) {
+    return null
+  }
+}
+
+function missingErrorMessage(packageName) {
+  return `'${packageName}' is not listed in the project's dependencies. ` +
+  `Run 'npm i -S ${packageName}' to add it`
+}
+
+function devDepErrorMessage(packageName) {
+  return `'${packageName}' is not listed in the project's dependencies, not devDependencies.`
+}
+
+function reportIfMissing(context, deps, allowDevDeps, node, name) {
+  if (importType(name, context) !== 'external') {
+    return
+  }
+  const packageName = name.split('/')[0]
+
+  if (deps.dependencies[packageName] === undefined) {
+    if (!allowDevDeps) {
+      context.report(node, devDepErrorMessage(packageName))
+    } else if (deps.devDependencies[packageName] === undefined) {
+      context.report(node, missingErrorMessage(packageName))
+    }
+  }
+}
+
+module.exports = function (context) {
+  const options = context.options[0] || {}
+  const allowDevDeps = options.devDependencies !== false
+  const deps = getDependencies(context)
+
+  if (!deps) {
+    return {}
+  }
+
+  // todo: use module visitor from module-utils core
+  return {
+    ImportDeclaration: function (node) {
+      reportIfMissing(context, deps, allowDevDeps, node, node.source.value)
+    },
+    CallExpression: function handleRequires(node) {
+      if (isStaticRequire(node)) {
+        reportIfMissing(context, deps, allowDevDeps, node, node.arguments[0].value)
+      }
+    },
+  }
+}
+
+module.exports.schema = [
+  {
+    'type': 'object',
+    'properties': {
+      'devDependencies': { 'type': 'boolean' },
+    },
+    'additionalProperties': false,
+  },
+]

tests/.eslintrc

@@ -3,5 +3,4 @@ env:
   mocha: true
 rules:
   no-unused-expressions: 0
-  quotes: [2, 'single', 'avoid-escape']
   max-len: 0

tests/files/importType/index.js

@@ -0,0 +1 @@
+/* for importType test, just needs to exist */

tests/files/package.json

@@ -1 +1,13 @@
-{ "dummy": true }
+{
+  "dummy": true,
+  "devDependencies": {
+    "eslint": "2.x"
+  },
+  "peerDependencies": {
+    "eslint": "2.x"
+  },
+  "dependencies": {
+    "lodash.cond": "^4.3.0",
+    "pkg-up": "^1.0.0"
+  }
+}

tests/src/core/importType.js

@@ -0,0 +1,60 @@
+import { expect } from 'chai'
+import * as path from 'path'
+
+import importType from 'core/importType'
+
+import { testContext } from '../utils'
+
+describe('importType(name)', function () {
+  const context = testContext()
+
+  it("should return 'builtin' for node.js modules", function() {
+    expect(importType('fs', context)).to.equal('builtin')
+    expect(importType('path', context)).to.equal('builtin')
+  })
+
+  it("should return 'external' for non-builtin modules without a relative path", function() {
+    expect(importType('lodash', context)).to.equal('external')
+    expect(importType('async', context)).to.equal('external')
+    expect(importType('chalk', context)).to.equal('external')
+    expect(importType('foo', context)).to.equal('external')
+    expect(importType('lodash.find', context)).to.equal('external')
+    expect(importType('lodash/fp', context)).to.equal('external')
+  })
+
+  it("should return 'project' for non-builtins resolved outside of node_modules", function () {
+    const pathContext = testContext({ "import/resolver": { node: { paths: [ path.join(__dirname, '..', '..', 'files') ] } } })
+    expect(importType('importType', pathContext)).to.equal('project')
+  })
+
+  it("should return 'parent' for internal modules that go through the parent", function() {
+    expect(importType('../foo', context)).to.equal('parent')
+    expect(importType('../../foo', context)).to.equal('parent')
+    expect(importType('../bar/foo', context)).to.equal('parent')
+  })
+
+  it("should return 'sibling' for internal modules that are connected to one of the siblings", function() {
+    expect(importType('./foo', context)).to.equal('sibling')
+    expect(importType('./foo/bar', context)).to.equal('sibling')
+  })
+
+  describe("should return 'index' for sibling index file when", function() {
+    it("resolves", function() {
+      expect(importType('./importType', context)).to.equal('index')
+      expect(importType('./importType/', context)).to.equal('index')
+      expect(importType('./importType/index', context)).to.equal('index')
+      expect(importType('./importType/index.js', context)).to.equal('index')
+    })
+    it("doesn't resolve", function() {
+      expect(importType('.', context)).to.equal('index')
+      expect(importType('./', context)).to.equal('index')
+      expect(importType('./index', context)).to.equal('index')
+      expect(importType('./index.js', context)).to.equal('index')
+    })
+  })
+
+  it("should return 'unknown' for any unhandled cases", function() {
+    expect(importType('/malformed', context)).to.equal('unknown')
+    expect(importType('   foo', context)).to.equal('unknown')
+  })
+})

tests/src/rules/no-extraneous-dependencies.js

@@ -0,0 +1,62 @@
+import { test } from '../utils'
+import * as path from 'path'
+
+import { RuleTester } from 'eslint'
+
+const ruleTester = new RuleTester()
+    , rule = require('rules/no-extraneous-dependencies')
+
+ruleTester.run('no-extraneous-dependencies', rule, {
+  valid: [
+    test({ code: 'import "lodash.cond"'}),
+    test({ code: 'import "pkg-up"'}),
+    test({ code: 'import foo, { bar } from "lodash.cond"'}),
+    test({ code: 'import foo, { bar } from "pkg-up"'}),
+    test({ code: 'import "eslint"'}),
+    test({ code: 'import "eslint/lib/api"'}),
+    test({ code: 'require("lodash.cond")'}),
+    test({ code: 'require("pkg-up")'}),
+    test({ code: 'var foo = require("lodash.cond")'}),
+    test({ code: 'var foo = require("pkg-up")'}),
+    test({ code: 'import "fs"'}),
+    test({ code: 'import "./foo"'}),
+
+    // 'project' type
+    test({
+      code: 'import "importType"',
+      settings: { "import/resolver": { node: { paths: [ path.join(__dirname, '../../files') ] } } },
+    }),
+  ],
+  invalid: [
+    test({
+      code: 'import "not-a-dependency"',
+      errors: [{
+        ruleId: 'no-extraneous-dependencies',
+        message: '\'not-a-dependency\' is not listed in the project\'s dependencies. Run \'npm i -S not-a-dependency\' to add it',
+      }],
+    }),
+    test({
+      code: 'import "eslint"',
+      options: [{devDependencies: false}],
+      errors: [{
+        ruleId: 'no-extraneous-dependencies',
+        message: '\'eslint\' is not listed in the project\'s dependencies, not devDependencies.',
+      }],
+    }),
+    test({
+      code: 'var foo = require("not-a-dependency");',
+      errors: [{
+        ruleId: 'no-extraneous-dependencies',
+        message: '\'not-a-dependency\' is not listed in the project\'s dependencies. Run \'npm i -S not-a-dependency\' to add it',
+      }],
+    }),
+    test({
+      code: 'var eslint = require("eslint");',
+      options: [{devDependencies: false}],
+      errors: [{
+        ruleId: 'no-extraneous-dependencies',
+        message: '\'eslint\' is not listed in the project\'s dependencies, not devDependencies.',
+      }],
+    }),
+  ],
+})