no-extraneous-dependencies: add support/option for optionalDependencies (fixes import-js#266)

Author: jfmengels

CHANGELOG.md

@@ -6,6 +6,7 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel
 ## [Unreleased]
 ### Added
 - [`newline-after-import`], new rule. ([#245], thanks [@singles])
+- Added an `optionalDependencies` option to [`no-extraneous-dependencies`] to allow/forbid optional dependencies ([#266], thanks [@jfmengels]).
 
 ## resolvers/webpack/0.2.4 - 2016-04-29
 ### Changed
@@ -210,6 +211,7 @@ for info on changes for earlier releases.
 [#286]: https://github.com/benmosher/eslint-plugin-import/issues/286
 [#281]: https://github.com/benmosher/eslint-plugin-import/issues/281
 [#272]: https://github.com/benmosher/eslint-plugin-import/issues/272
+[#266]: https://github.com/benmosher/eslint-plugin-import/issues/266
 [#216]: https://github.com/benmosher/eslint-plugin-import/issues/216
 [#214]: https://github.com/benmosher/eslint-plugin-import/issues/214
 [#210]: https://github.com/benmosher/eslint-plugin-import/issues/210

docs/rules/no-extraneous-dependencies.md

@@ -8,11 +8,12 @@ The closest parent `package.json` will be used. If no `package.json` is found, t
 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`.
+`optionalDependencies`: If set to `false`, then the rule will show an error when `optionalDependencies` are imported. Defaults to `true`.
 
 You can set the options like this:
 
 ```js
-"import/no-extraneous-dependencies": ["error", {"devDependencies": false}]
+"import/no-extraneous-dependencies": ["error", {"devDependencies": false, "optionalDependencies": false}]
 ```
 
 
@@ -34,6 +35,9 @@ Given the following `package.json`:
     "eslint": "^2.4.0",
     "eslint-plugin-ava": "^1.3.0",
     "xo": "^0.13.0"
+  },
+  "optionalDependencies": {
+    "lodash.isarray": "^4.0.0"
   }
 }
 ```
@@ -48,6 +52,10 @@ import _ from 'lodash';
 /* eslint import/no-extraneous-dependencies: ["error", {"devDependencies": false}] */
 import test from 'ava';
 var test = require('ava');
+
+/* eslint import/no-extraneous-dependencies: ["error", {"optionalDependencies": false}] */
+import isArray from 'lodash.isarray';
+var isArray = require('lodash.isarray');
 ```
 
 
@@ -60,6 +68,7 @@ var foo = require('./foo');
 
 import test from 'ava';
 import find from 'lodash.find';
+import find from 'lodash.isarray';
 ```
 
 

src/rules/no-extraneous-dependencies.js

@@ -14,39 +14,61 @@ function getDependencies(context) {
     return {
       dependencies: packageContent.dependencies || {},
       devDependencies: packageContent.devDependencies || {},
+      optionalDependencies: packageContent.optionalDependencies || {},
     }
   } 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`
+  return `'${packageName}' should be 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.`
+  return `'${packageName}' should be listed in the project's dependencies, not devDependencies.`
 }
 
-function reportIfMissing(context, deps, allowDevDeps, node, name) {
+function optDepErrorMessage(packageName) {
+  return `'${packageName}' should be listed in the project's dependencies, ` +
+    `not optionalDependencies.`
+}
+
+function reportIfMissing(context, deps, allowDevDeps, allowOptDeps, 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))
-    }
+  const isInDeps = deps.dependencies[packageName] !== undefined
+  const isInDevDeps = deps.devDependencies[packageName] !== undefined
+  const isInOptDeps = deps.optionalDependencies[packageName] !== undefined
+
+  if (isInDeps ||
+    (allowDevDeps && isInDevDeps) ||
+    (allowOptDeps && isInOptDeps)
+  ) {
+    return
+  }
+
+  if (isInDevDeps && !allowDevDeps) {
+    context.report(node, devDepErrorMessage(packageName))
+    return
+  }
+
+  if (isInOptDeps && !allowOptDeps) {
+    context.report(node, optDepErrorMessage(packageName))
+    return
   }
+
+  context.report(node, missingErrorMessage(packageName))
 }
 
 module.exports = function (context) {
   const options = context.options[0] || {}
   const allowDevDeps = options.devDependencies !== false
+  const allowOptDeps = options.optionalDependencies !== false
   const deps = getDependencies(context)
 
   if (!deps) {
@@ -56,11 +78,11 @@ module.exports = function (context) {
   // todo: use module visitor from module-utils core
   return {
     ImportDeclaration: function (node) {
-      reportIfMissing(context, deps, allowDevDeps, node, node.source.value)
+      reportIfMissing(context, deps, allowDevDeps, allowOptDeps, node, node.source.value)
     },
     CallExpression: function handleRequires(node) {
       if (isStaticRequire(node)) {
-        reportIfMissing(context, deps, allowDevDeps, node, node.arguments[0].value)
+        reportIfMissing(context, deps, allowDevDeps, allowOptDeps, node, node.arguments[0].value)
       }
     },
   }
@@ -71,6 +93,7 @@ module.exports.schema = [
     'type': 'object',
     'properties': {
       'devDependencies': { 'type': 'boolean' },
+      'optionalDependencies': { 'type': 'boolean' },
     },
     'additionalProperties': false,
   },

tests/files/package.json

@@ -9,5 +9,8 @@
   "dependencies": {
     "lodash.cond": "^4.3.0",
     "pkg-up": "^1.0.0"
+  },
+  "optionalDependencies": {
+    "lodash.isarray": "^4.0.0"
   }
 }

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

@@ -20,6 +20,7 @@ ruleTester.run('no-extraneous-dependencies', rule, {
     test({ code: 'var foo = require("pkg-up")'}),
     test({ code: 'import "fs"'}),
     test({ code: 'import "./foo"'}),
+    test({ code: 'import "lodash.isarray"'}),
 
     // 'project' type
     test({
@@ -32,30 +33,46 @@ ruleTester.run('no-extraneous-dependencies', rule, {
       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',
+        message: '\'not-a-dependency\' should be 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.',
+        message: '\'eslint\' should be listed in the project\'s dependencies, not devDependencies.',
       }],
     }),
     test({
-      code: 'var foo = require("not-a-dependency");',
+      code: 'import "lodash.isarray"',
+      options: [{optionalDependencies: false}],
       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',
+        message: '\'lodash.isarray\' should be listed in the project\'s dependencies, not optionalDependencies.',
       }],
     }),
     test({
-      code: 'var eslint = require("eslint");',
+      code: 'var foo = require("not-a-dependency")',
+      errors: [{
+        ruleId: 'no-extraneous-dependencies',
+        message: '\'not-a-dependency\' should be 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.',
+        message: '\'eslint\' should be listed in the project\'s dependencies, not devDependencies.',
+      }],
+    }),
+    test({
+      code: 'var eslint = require("lodash.isarray")',
+      options: [{optionalDependencies: false}],
+      errors: [{
+        ruleId: 'no-extraneous-dependencies',
+        message: '\'lodash.isarray\' should be listed in the project\'s dependencies, not optionalDependencies.',
       }],
     }),
   ],