From 387d3335cdc0a575889f01eff46809d3f9a394b7 Mon Sep 17 00:00:00 2001 From: Jeroen Engels Date: Tue, 23 Aug 2016 11:32:35 +0200 Subject: [PATCH 01/24] Add `allow` option to `no-nodejs-modules` rule (fixes #452) (#509) * Add `allow` option to `no-nodejs-modules` rule (fixes #452) * Add test case * Fix not working in Node v4 --- CHANGELOG.md | 4 ++- docs/rules/no-nodejs-modules.md | 9 +++++++ src/rules/no-nodejs-modules.js | 11 ++++++--- tests/src/rules/no-nodejs-modules.js | 37 ++++++++++++++++++++++++++++ 4 files changed, 56 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d3c399196..9c5aef7d7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ This project adheres to [Semantic Versioning](http://semver.org/). This change log adheres to standards from [Keep a CHANGELOG](http://keepachangelog.com). ## [Unreleased] - +- Added an `allow` option to [`no-nodejs-modules`] to allow exceptions ([#452], [#509]). ## [1.14.0] - 2016-08-22 ### Added @@ -290,6 +290,7 @@ for info on changes for earlier releases. [`prefer-default-export`]: ./docs/rules/prefer-default-export.md [`no-restricted-paths`]: ./docs/rules/no-restricted-paths.md +[#509]: https://github.com/benmosher/eslint-plugin-import/pull/509 [#503]: https://github.com/benmosher/eslint-plugin-import/pull/503 [#499]: https://github.com/benmosher/eslint-plugin-import/pull/499 [#461]: https://github.com/benmosher/eslint-plugin-import/pull/461 @@ -331,6 +332,7 @@ for info on changes for earlier releases. [#478]: https://github.com/benmosher/eslint-plugin-import/issues/478 [#456]: https://github.com/benmosher/eslint-plugin-import/issues/456 [#453]: https://github.com/benmosher/eslint-plugin-import/issues/453 +[#452]: https://github.com/benmosher/eslint-plugin-import/issues/452 [#441]: https://github.com/benmosher/eslint-plugin-import/issues/441 [#423]: https://github.com/benmosher/eslint-plugin-import/issues/423 [#416]: https://github.com/benmosher/eslint-plugin-import/issues/416 diff --git a/docs/rules/no-nodejs-modules.md b/docs/rules/no-nodejs-modules.md index df3c27e4e..59f1e3693 100644 --- a/docs/rules/no-nodejs-modules.md +++ b/docs/rules/no-nodejs-modules.md @@ -2,6 +2,12 @@ Forbid the use of Node.js builtin modules. Can be useful for client-side web projects that do not have access to those modules. +### Options + +This rule supports the following options: + +- `allow`: Array of names of allowed modules. Defaults to an empty array. + ## Rule Details ### Fail @@ -24,6 +30,9 @@ import foo from './foo'; var _ = require('lodash'); var foo = require('foo'); var foo = require('./foo'); + +/* eslint import/no-nodejs-modules: ["error", {"allow": ["path"]}] */ +import path from 'path'; ``` ## When Not To Use It diff --git a/src/rules/no-nodejs-modules.js b/src/rules/no-nodejs-modules.js index 7cdb94ec9..9160b9e8d 100644 --- a/src/rules/no-nodejs-modules.js +++ b/src/rules/no-nodejs-modules.js @@ -1,20 +1,23 @@ import importType from '../core/importType' import isStaticRequire from '../core/staticRequire' -function reportIfMissing(context, node, name) { - if (importType(name, context) === 'builtin') { +function reportIfMissing(context, node, allowed, name) { + if (allowed.indexOf(name) === -1 && importType(name, context) === 'builtin') { context.report(node, 'Do not import Node.js builtin module "' + name + '"') } } module.exports = function (context) { + const options = context.options[0] || {} + const allowed = options.allow || [] + return { ImportDeclaration: function handleImports(node) { - reportIfMissing(context, node, node.source.value) + reportIfMissing(context, node, allowed, node.source.value) }, CallExpression: function handleRequires(node) { if (isStaticRequire(node)) { - reportIfMissing(context, node, node.arguments[0].value) + reportIfMissing(context, node, allowed, node.arguments[0].value) } }, } diff --git a/tests/src/rules/no-nodejs-modules.js b/tests/src/rules/no-nodejs-modules.js index 826640e31..b5e55fafc 100644 --- a/tests/src/rules/no-nodejs-modules.js +++ b/tests/src/rules/no-nodejs-modules.js @@ -26,6 +26,36 @@ ruleTester.run('no-nodejs-modules', rule, { test({ code: 'var foo = require("foo")'}), test({ code: 'var foo = require("./")'}), test({ code: 'var foo = require("@scope/foo")'}), + test({ + code: 'import events from "events"', + options: [{ + allow: ['events'], + }], + }), + test({ + code: 'import path from "path"', + options: [{ + allow: ['path'], + }], + }), + test({ + code: 'var events = require("events")', + options: [{ + allow: ['events'], + }], + }), + test({ + code: 'var path = require("path")', + options: [{ + allow: ['path'], + }], + }), + test({ + code: 'import path from "path";import events from "events"', + options: [{ + allow: ['path', 'events'], + }], + }), ], invalid: [ test({ @@ -44,5 +74,12 @@ ruleTester.run('no-nodejs-modules', rule, { code: 'var fs = require("fs")', errors: [error('Do not import Node.js builtin module "fs"')], }), + test({ + code: 'import fs from "fs"', + options: [{ + allow: ['path'], + }], + errors: [error('Do not import Node.js builtin module "fs"')], + }), ], }) From ff0ef90e50ec3fae6b65c0e4fe55e516a2e0476d Mon Sep 17 00:00:00 2001 From: Greenkeeper Date: Tue, 23 Aug 2016 11:36:14 +0200 Subject: [PATCH 02/24] chore(package): update doctrine to version 1.3.0 (#511) https://greenkeeper.io/ --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index dde6e7578..787045ae1 100644 --- a/package.json +++ b/package.json @@ -73,7 +73,7 @@ "builtin-modules": "^1.1.1", "contains-path": "^0.1.0", "debug": "^2.2.0", - "doctrine": "1.2.x", + "doctrine": "1.3.0", "es6-map": "^0.1.3", "es6-set": "^0.1.4", "eslint-import-resolver-node": "^0.2.0", From 0d98253ea3bc1c5998223cab7b1a095ba943b210 Mon Sep 17 00:00:00 2001 From: Jason Quense Date: Mon, 22 Aug 2016 12:17:50 -0400 Subject: [PATCH 03/24] Fix false positive with default import having a "default" property (fixes #507, closes #508) --- CHANGELOG.md | 5 +++++ src/rules/no-named-as-default-member.js | 4 ++++ tests/files/default-export-default-property.js | 4 ++++ tests/src/rules/no-named-as-default-member.js | 1 + 4 files changed, 14 insertions(+) create mode 100644 tests/files/default-export-default-property.js diff --git a/CHANGELOG.md b/CHANGELOG.md index 9c5aef7d7..c5d2e2103 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,8 +4,12 @@ This project adheres to [Semantic Versioning](http://semver.org/). This change log adheres to standards from [Keep a CHANGELOG](http://keepachangelog.com). ## [Unreleased] +### Added - Added an `allow` option to [`no-nodejs-modules`] to allow exceptions ([#452], [#509]). +### Fixed +- [`no-named-as-default-member`] Allow default import to have a property named "default" ([#507], thanks [@jquense]) + ## [1.14.0] - 2016-08-22 ### Added - [`import/parsers` setting]: parse some dependencies (i.e. TypeScript!) with a different parser than the ESLint-configured parser. ([#503]) @@ -329,6 +333,7 @@ for info on changes for earlier releases. [#157]: https://github.com/benmosher/eslint-plugin-import/pull/157 [#314]: https://github.com/benmosher/eslint-plugin-import/pull/314 +[#507]: https://github.com/benmosher/eslint-plugin-import/issues/507 [#478]: https://github.com/benmosher/eslint-plugin-import/issues/478 [#456]: https://github.com/benmosher/eslint-plugin-import/issues/456 [#453]: https://github.com/benmosher/eslint-plugin-import/issues/453 diff --git a/src/rules/no-named-as-default-member.js b/src/rules/no-named-as-default-member.js index ba102e377..fc836a403 100644 --- a/src/rules/no-named-as-default-member.js +++ b/src/rules/no-named-as-default-member.js @@ -69,6 +69,10 @@ module.exports = function(context) { if (fileImport == null) return lookups.forEach(({propName, node}) => { + // the default import can have a "default" property + if (propName === 'default') { + return + } if (fileImport.exportMap.namespace.has(propName)) { context.report({ node, diff --git a/tests/files/default-export-default-property.js b/tests/files/default-export-default-property.js new file mode 100644 index 000000000..c453da57d --- /dev/null +++ b/tests/files/default-export-default-property.js @@ -0,0 +1,4 @@ + +export default { + default: 'baz' +} diff --git a/tests/src/rules/no-named-as-default-member.js b/tests/src/rules/no-named-as-default-member.js index 559794b30..b5e294d19 100644 --- a/tests/src/rules/no-named-as-default-member.js +++ b/tests/src/rules/no-named-as-default-member.js @@ -10,6 +10,7 @@ ruleTester.run('no-named-as-default-member', rule, { test({code: 'import bar from "./bar"; const baz = bar.baz'}), test({code: 'import {foo} from "./bar"; const baz = foo.baz;'}), test({code: 'import * as named from "./named-exports"; const a = named.a'}), + test({code: 'import foo from "./default-export-default-property"; const a = foo.default'}), ...SYNTAX_CASES, ], From 18c4831066e65ad3a9b2ca303a28eece53c3da50 Mon Sep 17 00:00:00 2001 From: Ben Mosher Date: Tue, 23 Aug 2016 08:49:50 -0400 Subject: [PATCH 04/24] added #508 link to changelog --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c5d2e2103..f0a5a615e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,7 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel - Added an `allow` option to [`no-nodejs-modules`] to allow exceptions ([#452], [#509]). ### Fixed -- [`no-named-as-default-member`] Allow default import to have a property named "default" ([#507], thanks [@jquense]) +- [`no-named-as-default-member`] Allow default import to have a property named "default" ([#507]+[#508], thanks [@jquense] for both!) ## [1.14.0] - 2016-08-22 ### Added @@ -295,6 +295,7 @@ for info on changes for earlier releases. [`no-restricted-paths`]: ./docs/rules/no-restricted-paths.md [#509]: https://github.com/benmosher/eslint-plugin-import/pull/509 +[#508]: https://github.com/benmosher/eslint-plugin-import/pull/508 [#503]: https://github.com/benmosher/eslint-plugin-import/pull/503 [#499]: https://github.com/benmosher/eslint-plugin-import/pull/499 [#461]: https://github.com/benmosher/eslint-plugin-import/pull/461 From 940544467b7d1932117619a6c51b9b1c06d81bc2 Mon Sep 17 00:00:00 2001 From: Ben Mosher Date: Thu, 25 Aug 2016 21:46:19 -0400 Subject: [PATCH 05/24] cleanup after #511 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 787045ae1..cd646efa4 100644 --- a/package.json +++ b/package.json @@ -73,7 +73,7 @@ "builtin-modules": "^1.1.1", "contains-path": "^0.1.0", "debug": "^2.2.0", - "doctrine": "1.3.0", + "doctrine": "1.3.x", "es6-map": "^0.1.3", "es6-set": "^0.1.4", "eslint-import-resolver-node": "^0.2.0", From 8da6a5ac6215cfaad51b1ec9ea5a9dd90aaa6359 Mon Sep 17 00:00:00 2001 From: Gabriele Cimato Date: Fri, 26 Aug 2016 03:51:04 -0700 Subject: [PATCH 06/24] doc: minor typos (#523) --- docs/rules/prefer-default-export.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/rules/prefer-default-export.md b/docs/rules/prefer-default-export.md index 560c98964..d3b6fbb0d 100644 --- a/docs/rules/prefer-default-export.md +++ b/docs/rules/prefer-default-export.md @@ -1,6 +1,6 @@ # prefer-default-export -When there is only a single export from a module prefer using default export over named export. +When there is only a single export from a module, prefer using default export over named export. ## Rule Details @@ -9,7 +9,7 @@ The following patterns are considered warnings: ```javascript // bad.js -// There is only a single module export and its a named export. +// There is only a single module export and it's a named export. export const foo = 'foo'; ``` @@ -28,7 +28,7 @@ export default 'bar'; ```javascript // good2.js -// There is more thank one named exports in the module. +// There is more than one named export in the module. export const foo = 'foo'; export const bar = 'bar'; ``` @@ -36,7 +36,7 @@ export const bar = 'bar'; ```javascript // good3.js -// There is more thank one named exports in the module +// There is more than one named export in the module const foo = 'foo'; const bar = 'bar'; export { foo, bar } From d9b97308534cd0f57a069da64d42c03ed7b1a707 Mon Sep 17 00:00:00 2001 From: Kenneth Powers Date: Sat, 27 Aug 2016 15:12:45 -0400 Subject: [PATCH 07/24] Allow regular expressions for rule no-extraneous-dependencies. --- src/rules/no-extraneous-dependencies.js | 21 ++++++++++---- tests/src/rules/no-extraneous-dependencies.js | 28 +++++++++++++++++++ 2 files changed, 43 insertions(+), 6 deletions(-) diff --git a/src/rules/no-extraneous-dependencies.js b/src/rules/no-extraneous-dependencies.js index 059307e04..00ef6e573 100644 --- a/src/rules/no-extraneous-dependencies.js +++ b/src/rules/no-extraneous-dependencies.js @@ -71,8 +71,17 @@ function reportIfMissing(context, deps, depsOptions, node, name) { context.report(node, missingErrorMessage(packageName)) } +function testConfig(config, filename) { + if (typeof config === 'boolean' || typeof config === 'undefined') { + return config + } + const pattern = new RegExp(config) // `config` is either a string or RegExp + return pattern.test(filename) +} + module.exports = function (context) { const options = context.options[0] || {} + const filename = context.getFilename() const deps = getDependencies(context) if (!deps) { @@ -80,9 +89,9 @@ module.exports = function (context) { } const depsOptions = { - allowDevDeps: options.devDependencies !== false, - allowOptDeps: options.optionalDependencies !== false, - allowPeerDeps: options.peerDependencies !== false, + allowDevDeps: testConfig(options.devDependencies, filename) !== false, + allowOptDeps: testConfig(options.optionalDependencies, filename) !== false, + allowPeerDeps: testConfig(options.peerDependencies, filename) !== false, } // todo: use module visitor from module-utils core @@ -102,9 +111,9 @@ module.exports.schema = [ { 'type': 'object', 'properties': { - 'devDependencies': { 'type': 'boolean' }, - 'optionalDependencies': { 'type': 'boolean' }, - 'peerDependencies': { 'type': 'boolean' }, + 'devDependencies': { 'type': ['boolean', 'string', 'object'] }, + 'optionalDependencies': { 'type': ['boolean', 'string', 'object'] }, + 'peerDependencies': { 'type': ['boolean', 'string', 'object'] }, }, 'additionalProperties': false, }, diff --git a/tests/src/rules/no-extraneous-dependencies.js b/tests/src/rules/no-extraneous-dependencies.js index f0b43b09b..d874b649e 100644 --- a/tests/src/rules/no-extraneous-dependencies.js +++ b/tests/src/rules/no-extraneous-dependencies.js @@ -35,6 +35,16 @@ ruleTester.run('no-extraneous-dependencies', rule, { code: 'import "importType"', settings: { 'import/resolver': { node: { paths: [ path.join(__dirname, '../../files') ] } } }, }), + test({ + code: 'import chai from "chai"', + options: [{devDependencies: /test|spec/}], + filename: 'glob.test.js', + }), + test({ + code: 'import chai from "chai"', + options: [{devDependencies: 'test|spec'}], + filename: 'glob.spec.js', + }), ], invalid: [ test({ @@ -89,6 +99,24 @@ ruleTester.run('no-extraneous-dependencies', rule, { message: '\'glob\' should be listed in the project\'s dependencies, not devDependencies.', }], }), + test({ + code: 'import chai from "chai"', + options: [{devDependencies: /test|spec/}], + filename: 'foo.tes.js', + errors: [{ + ruleId: 'no-extraneous-dependencies', + message: '\'chai\' should be listed in the project\'s dependencies, not devDependencies.', + }], + }), + test({ + code: 'import chai from "chai"', + options: [{devDependencies: 'test|spec'}], + filename: 'foo.tes.js', + errors: [{ + ruleId: 'no-extraneous-dependencies', + message: '\'chai\' should be listed in the project\'s dependencies, not devDependencies.', + }], + }), test({ code: 'var eslint = require("lodash.isarray")', options: [{optionalDependencies: false}], From c739798b547689769a26c57ecfa21781d2ab4389 Mon Sep 17 00:00:00 2001 From: Kenneth Powers Date: Sat, 27 Aug 2016 19:58:47 -0400 Subject: [PATCH 08/24] Add RegExp documentation for no-extraneous-dependencies. --- docs/rules/no-extraneous-dependencies.md | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/docs/rules/no-extraneous-dependencies.md b/docs/rules/no-extraneous-dependencies.md index 7f57d9706..b73bf0797 100644 --- a/docs/rules/no-extraneous-dependencies.md +++ b/docs/rules/no-extraneous-dependencies.md @@ -19,6 +19,19 @@ You can set the options like this: "import/no-extraneous-dependencies": ["error", {"devDependencies": false, "optionalDependencies": false, "peerDependencies": false}] ``` +You can also use regular expressions instead of literal booleans: + +```js +"import/no-extraneous-dependencies": ["error", {"devDependencies": "test|spec"}] +``` + +If you are using JavaScript configuration (e.g., `.eslintrc.js`), then you can use a RegExp literal instead of a string: + +```js +"import/no-extraneous-dependencies": ["error", {"devDependencies": /test|spec/}] +``` + +When using a regular expression the result of running [`test`] against the name of the file being linted is used as the boolean value. For example, the above configurations will allow the import of `devDependencies` in files whose names include `test` or `spec`. ## Rule Details @@ -86,3 +99,5 @@ import react from 'react'; ## When Not To Use It If you do not have a `package.json` file in your project. + +[`test`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/RegExp/test "RegExp.prototype.test" From 1be47202268cfc786e89fd0161ab01ab51e693ba Mon Sep 17 00:00:00 2001 From: Kenneth Powers Date: Sun, 28 Aug 2016 18:10:42 -0400 Subject: [PATCH 09/24] Update doc due to review. --- docs/rules/no-extraneous-dependencies.md | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/docs/rules/no-extraneous-dependencies.md b/docs/rules/no-extraneous-dependencies.md index b73bf0797..1b7f94bac 100644 --- a/docs/rules/no-extraneous-dependencies.md +++ b/docs/rules/no-extraneous-dependencies.md @@ -31,7 +31,7 @@ If you are using JavaScript configuration (e.g., `.eslintrc.js`), then you can u "import/no-extraneous-dependencies": ["error", {"devDependencies": /test|spec/}] ``` -When using a regular expression the result of running [`test`] against the name of the file being linted is used as the boolean value. For example, the above configurations will allow the import of `devDependencies` in files whose names include `test` or `spec`. +When using a regular expression, the setting will be activated if the name of the file being linted matches the given regular expression. For example, the above configurations will allow the import of `devDependencies` in files whose names include `test` or `spec`. ## Rule Details @@ -99,5 +99,3 @@ import react from 'react'; ## When Not To Use It If you do not have a `package.json` file in your project. - -[`test`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/RegExp/test "RegExp.prototype.test" From 8ff322d5865226af79052be15580582c23a2113a Mon Sep 17 00:00:00 2001 From: Kenneth Powers Date: Sun, 28 Aug 2016 19:25:24 -0400 Subject: [PATCH 10/24] Extend no-extraneous-dependencies to support globs and arrays. --- docs/rules/no-extraneous-dependencies.md | 18 ++++++-- package.json | 1 + src/rules/no-extraneous-dependencies.js | 25 ++++++++--- tests/src/rules/no-extraneous-dependencies.js | 41 +++++++++++++++++-- 4 files changed, 73 insertions(+), 12 deletions(-) diff --git a/docs/rules/no-extraneous-dependencies.md b/docs/rules/no-extraneous-dependencies.md index 1b7f94bac..a51ef7324 100644 --- a/docs/rules/no-extraneous-dependencies.md +++ b/docs/rules/no-extraneous-dependencies.md @@ -19,10 +19,16 @@ You can set the options like this: "import/no-extraneous-dependencies": ["error", {"devDependencies": false, "optionalDependencies": false, "peerDependencies": false}] ``` -You can also use regular expressions instead of literal booleans: +You can also use globs instead of literal booleans: ```js -"import/no-extraneous-dependencies": ["error", {"devDependencies": "test|spec"}] +"import/no-extraneous-dependencies": ["error", {"devDependencies": "*.test.js"}] +``` + +You can also use regular expressions instead of literal booleans or globs (note: the string must begin and end with `/`): + +```js +"import/no-extraneous-dependencies": ["error", {"devDependencies": "/test|spec/"}] ``` If you are using JavaScript configuration (e.g., `.eslintrc.js`), then you can use a RegExp literal instead of a string: @@ -31,7 +37,13 @@ If you are using JavaScript configuration (e.g., `.eslintrc.js`), then you can u "import/no-extraneous-dependencies": ["error", {"devDependencies": /test|spec/}] ``` -When using a regular expression, the setting will be activated if the name of the file being linted matches the given regular expression. For example, the above configurations will allow the import of `devDependencies` in files whose names include `test` or `spec`. +When using a glob or regular expression, the setting will be activated if the name of the file being linted matches the given glob or regular expression. In addition, you can use an array to specify multiple globs or regular expressions: + +```js +"import/no-extraneous-dependencies": ["error", {"devDependencies": [/test/, '/spec/', '*.test.js', '*.spec.js']}] +``` + +When using an array of globs or regular expressions, the setting will be activated if the name of the file being linted matches a single glob or regular expression in the array. ## Rule Details diff --git a/package.json b/package.json index cd646efa4..c9da183ab 100644 --- a/package.json +++ b/package.json @@ -81,6 +81,7 @@ "lodash.endswith": "^4.0.1", "lodash.find": "^4.3.0", "lodash.findindex": "^4.3.0", + "minimatch": "^3.0.3", "object-assign": "^4.0.1", "pkg-dir": "^1.0.0", "pkg-up": "^1.0.0" diff --git a/src/rules/no-extraneous-dependencies.js b/src/rules/no-extraneous-dependencies.js index 00ef6e573..daa08fa0a 100644 --- a/src/rules/no-extraneous-dependencies.js +++ b/src/rules/no-extraneous-dependencies.js @@ -1,5 +1,6 @@ import fs from 'fs' import pkgUp from 'pkg-up' +import minimatch from 'minimatch' import importType from '../core/importType' import isStaticRequire from '../core/staticRequire' @@ -72,11 +73,25 @@ function reportIfMissing(context, deps, depsOptions, node, name) { } function testConfig(config, filename) { + // Simplest configuration first if (typeof config === 'boolean' || typeof config === 'undefined') { return config } - const pattern = new RegExp(config) // `config` is either a string or RegExp - return pattern.test(filename) + // Account for the possibility of an array for multiple configuration + return [].concat(config).some(c => { + if (typeof c === 'object') { + // `c` is a literal RegExp + return c.test(filename) + } + // By this point `c` must be a string. + if (/^\/.+\/$/.test(c)) { + // `c` is a string representation of a RegExp. + const pattern = new RegExp(c.substring(1, c.length - 1)) + return pattern.test(filename) + } + // `c` must be a string representing a glob + return minimatch(filename, c) + }) } module.exports = function (context) { @@ -111,9 +126,9 @@ module.exports.schema = [ { 'type': 'object', 'properties': { - 'devDependencies': { 'type': ['boolean', 'string', 'object'] }, - 'optionalDependencies': { 'type': ['boolean', 'string', 'object'] }, - 'peerDependencies': { 'type': ['boolean', 'string', 'object'] }, + 'devDependencies': { 'type': ['boolean', 'string', 'object', 'array'] }, + 'optionalDependencies': { 'type': ['boolean', 'string', 'object', 'array'] }, + 'peerDependencies': { 'type': ['boolean', 'string', 'object', 'array'] }, }, 'additionalProperties': false, }, diff --git a/tests/src/rules/no-extraneous-dependencies.js b/tests/src/rules/no-extraneous-dependencies.js index d874b649e..7d370539d 100644 --- a/tests/src/rules/no-extraneous-dependencies.js +++ b/tests/src/rules/no-extraneous-dependencies.js @@ -38,12 +38,27 @@ ruleTester.run('no-extraneous-dependencies', rule, { test({ code: 'import chai from "chai"', options: [{devDependencies: /test|spec/}], - filename: 'glob.test.js', + filename: 'foo.test.js', }), test({ code: 'import chai from "chai"', - options: [{devDependencies: 'test|spec'}], - filename: 'glob.spec.js', + options: [{devDependencies: '/test|spec/'}], + filename: 'foo.spec.js', + }), + test({ + code: 'import chai from "chai"', + options: [{devDependencies: '*.spec.js'}], + filename: 'foo.spec.js', + }), + test({ + code: 'import chai from "chai"', + options: [{devDependencies: ['*.test.js', '*.spec.js']}], + filename: 'foo.spec.js', + }), + test({ + code: 'import chai from "chai"', + options: [{devDependencies: [/\.test\.js$/, /\.spec\.js$/]}], + filename: 'foo.spec.js', }), ], invalid: [ @@ -110,7 +125,25 @@ ruleTester.run('no-extraneous-dependencies', rule, { }), test({ code: 'import chai from "chai"', - options: [{devDependencies: 'test|spec'}], + options: [{devDependencies: '/test|spec/'}], + filename: 'foo.tes.js', + errors: [{ + ruleId: 'no-extraneous-dependencies', + message: '\'chai\' should be listed in the project\'s dependencies, not devDependencies.', + }], + }), + test({ + code: 'import chai from "chai"', + options: [{devDependencies: '*.test.js'}], + filename: 'foo.tes.js', + errors: [{ + ruleId: 'no-extraneous-dependencies', + message: '\'chai\' should be listed in the project\'s dependencies, not devDependencies.', + }], + }), + test({ + code: 'import chai from "chai"', + options: [{devDependencies: ['*.test.js', /test\.js$/, '/test/']}], filename: 'foo.tes.js', errors: [{ ruleId: 'no-extraneous-dependencies', From 8d16c0895e2580ce6c12b34066a13fca0fdee636 Mon Sep 17 00:00:00 2001 From: Kenneth Powers Date: Mon, 29 Aug 2016 19:45:20 -0400 Subject: [PATCH 11/24] Remove regular expression support from no-extraneous-dependencies. --- docs/rules/no-extraneous-dependencies.md | 18 ++-------- src/rules/no-extraneous-dependencies.js | 21 +++-------- tests/src/rules/no-extraneous-dependencies.js | 35 +------------------ 3 files changed, 8 insertions(+), 66 deletions(-) diff --git a/docs/rules/no-extraneous-dependencies.md b/docs/rules/no-extraneous-dependencies.md index a51ef7324..2a5fca86e 100644 --- a/docs/rules/no-extraneous-dependencies.md +++ b/docs/rules/no-extraneous-dependencies.md @@ -25,25 +25,13 @@ You can also use globs instead of literal booleans: "import/no-extraneous-dependencies": ["error", {"devDependencies": "*.test.js"}] ``` -You can also use regular expressions instead of literal booleans or globs (note: the string must begin and end with `/`): +When using a glob, the setting will be activated if the name of the file being linted matches the given glob. In addition, you can use an array to specify multiple globs: ```js -"import/no-extraneous-dependencies": ["error", {"devDependencies": "/test|spec/"}] +"import/no-extraneous-dependencies": ["error", {"devDependencies": ['*.test.js', '*.spec.js']}] ``` -If you are using JavaScript configuration (e.g., `.eslintrc.js`), then you can use a RegExp literal instead of a string: - -```js -"import/no-extraneous-dependencies": ["error", {"devDependencies": /test|spec/}] -``` - -When using a glob or regular expression, the setting will be activated if the name of the file being linted matches the given glob or regular expression. In addition, you can use an array to specify multiple globs or regular expressions: - -```js -"import/no-extraneous-dependencies": ["error", {"devDependencies": [/test/, '/spec/', '*.test.js', '*.spec.js']}] -``` - -When using an array of globs or regular expressions, the setting will be activated if the name of the file being linted matches a single glob or regular expression in the array. +When using an array of globs, the setting will be activated if the name of the file being linted matches a single glob in the array. ## Rule Details diff --git a/src/rules/no-extraneous-dependencies.js b/src/rules/no-extraneous-dependencies.js index daa08fa0a..e128c1060 100644 --- a/src/rules/no-extraneous-dependencies.js +++ b/src/rules/no-extraneous-dependencies.js @@ -78,20 +78,7 @@ function testConfig(config, filename) { return config } // Account for the possibility of an array for multiple configuration - return [].concat(config).some(c => { - if (typeof c === 'object') { - // `c` is a literal RegExp - return c.test(filename) - } - // By this point `c` must be a string. - if (/^\/.+\/$/.test(c)) { - // `c` is a string representation of a RegExp. - const pattern = new RegExp(c.substring(1, c.length - 1)) - return pattern.test(filename) - } - // `c` must be a string representing a glob - return minimatch(filename, c) - }) + return [].concat(config).some(c => minimatch(filename, c)) } module.exports = function (context) { @@ -126,9 +113,9 @@ module.exports.schema = [ { 'type': 'object', 'properties': { - 'devDependencies': { 'type': ['boolean', 'string', 'object', 'array'] }, - 'optionalDependencies': { 'type': ['boolean', 'string', 'object', 'array'] }, - 'peerDependencies': { 'type': ['boolean', 'string', 'object', 'array'] }, + 'devDependencies': { 'type': ['boolean', 'string', 'array'] }, + 'optionalDependencies': { 'type': ['boolean', 'string', 'array'] }, + 'peerDependencies': { 'type': ['boolean', 'string', 'array'] }, }, 'additionalProperties': false, }, diff --git a/tests/src/rules/no-extraneous-dependencies.js b/tests/src/rules/no-extraneous-dependencies.js index 7d370539d..a66a0325e 100644 --- a/tests/src/rules/no-extraneous-dependencies.js +++ b/tests/src/rules/no-extraneous-dependencies.js @@ -35,16 +35,6 @@ ruleTester.run('no-extraneous-dependencies', rule, { code: 'import "importType"', settings: { 'import/resolver': { node: { paths: [ path.join(__dirname, '../../files') ] } } }, }), - test({ - code: 'import chai from "chai"', - options: [{devDependencies: /test|spec/}], - filename: 'foo.test.js', - }), - test({ - code: 'import chai from "chai"', - options: [{devDependencies: '/test|spec/'}], - filename: 'foo.spec.js', - }), test({ code: 'import chai from "chai"', options: [{devDependencies: '*.spec.js'}], @@ -55,11 +45,6 @@ ruleTester.run('no-extraneous-dependencies', rule, { options: [{devDependencies: ['*.test.js', '*.spec.js']}], filename: 'foo.spec.js', }), - test({ - code: 'import chai from "chai"', - options: [{devDependencies: [/\.test\.js$/, /\.spec\.js$/]}], - filename: 'foo.spec.js', - }), ], invalid: [ test({ @@ -114,24 +99,6 @@ ruleTester.run('no-extraneous-dependencies', rule, { message: '\'glob\' should be listed in the project\'s dependencies, not devDependencies.', }], }), - test({ - code: 'import chai from "chai"', - options: [{devDependencies: /test|spec/}], - filename: 'foo.tes.js', - errors: [{ - ruleId: 'no-extraneous-dependencies', - message: '\'chai\' should be listed in the project\'s dependencies, not devDependencies.', - }], - }), - test({ - code: 'import chai from "chai"', - options: [{devDependencies: '/test|spec/'}], - filename: 'foo.tes.js', - errors: [{ - ruleId: 'no-extraneous-dependencies', - message: '\'chai\' should be listed in the project\'s dependencies, not devDependencies.', - }], - }), test({ code: 'import chai from "chai"', options: [{devDependencies: '*.test.js'}], @@ -143,7 +110,7 @@ ruleTester.run('no-extraneous-dependencies', rule, { }), test({ code: 'import chai from "chai"', - options: [{devDependencies: ['*.test.js', /test\.js$/, '/test/']}], + options: [{devDependencies: ['*.test.js', '*.spec.js']}], filename: 'foo.tes.js', errors: [{ ruleId: 'no-extraneous-dependencies', From 218397766711f269c6870a9ee0c9e3f4764cf351 Mon Sep 17 00:00:00 2001 From: Daniel Chang Date: Tue, 30 Aug 2016 11:24:20 -0400 Subject: [PATCH 12/24] Suggested change to readme (#535) Took a while to hunt down why my config wasn't working and figured out that I needed to add this dependency only after reading through [#238](https://github.com/benmosher/eslint-plugin-import/issues/238) and noticing [this line in a linked example](https://gist.github.com/ravasthi/abcfee465411fc45a8bc28decb9d8e5e#file-package-json-L24). I think adding this line may add some clarity and save time for several other webpack / eslint noobs (like me). --- resolvers/webpack/README.md | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/resolvers/webpack/README.md b/resolvers/webpack/README.md index f6542825e..1bd1c2e80 100644 --- a/resolvers/webpack/README.md +++ b/resolvers/webpack/README.md @@ -7,6 +7,19 @@ Webpack-literate module resolution plugin for [`eslint-plugin-import`](https://w Published separately to allow pegging to a specific version in case of breaking changes. +To use with `eslint-plugin-import`, run: + +``` +npm i eslint-import-resolver-webpack -g +``` + +or if you manage ESLint as a dev dependency: + +``` +# inside your project's working tree +npm install eslint-import-resolver-webpack --save-dev +``` + Will look for `webpack.config.js` as a sibling of the first ancestral `package.json`, or a `config` parameter may be provided with another filename/path either relative to the `package.json`, or a complete, absolute path. From 28dbed8ab6f6fefe9219c54ec87161168da518c4 Mon Sep 17 00:00:00 2001 From: Greenkeeper Date: Tue, 30 Aug 2016 17:24:46 +0200 Subject: [PATCH 13/24] chore(package): update typescript-eslint-parser to version 0.2.0 (#534) https://greenkeeper.io/ --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index cd646efa4..539d79734 100644 --- a/package.json +++ b/package.json @@ -64,7 +64,7 @@ "redux": "^3.0.4", "rimraf": "2.5.2", "typescript": "^1.8.10", - "typescript-eslint-parser": "^0.1.1" + "typescript-eslint-parser": "^0.2.0" }, "peerDependencies": { "eslint": "2.x - 3.x" From 505aea8fce2a9cc4219fdbd300beeca3173634cb Mon Sep 17 00:00:00 2001 From: Kenneth Powers Date: Tue, 30 Aug 2016 12:25:48 -0400 Subject: [PATCH 14/24] Only support arrays of globs. --- docs/rules/no-extraneous-dependencies.md | 8 +------- src/rules/no-extraneous-dependencies.js | 8 ++++---- tests/src/rules/no-extraneous-dependencies.js | 4 ++-- 3 files changed, 7 insertions(+), 13 deletions(-) diff --git a/docs/rules/no-extraneous-dependencies.md b/docs/rules/no-extraneous-dependencies.md index 2a5fca86e..a15734589 100644 --- a/docs/rules/no-extraneous-dependencies.md +++ b/docs/rules/no-extraneous-dependencies.md @@ -19,13 +19,7 @@ You can set the options like this: "import/no-extraneous-dependencies": ["error", {"devDependencies": false, "optionalDependencies": false, "peerDependencies": false}] ``` -You can also use globs instead of literal booleans: - -```js -"import/no-extraneous-dependencies": ["error", {"devDependencies": "*.test.js"}] -``` - -When using a glob, the setting will be activated if the name of the file being linted matches the given glob. In addition, you can use an array to specify multiple globs: +You can also use an array of globs instead of literal booleans: ```js "import/no-extraneous-dependencies": ["error", {"devDependencies": ['*.test.js', '*.spec.js']}] diff --git a/src/rules/no-extraneous-dependencies.js b/src/rules/no-extraneous-dependencies.js index e128c1060..0b1f3b4fd 100644 --- a/src/rules/no-extraneous-dependencies.js +++ b/src/rules/no-extraneous-dependencies.js @@ -78,7 +78,7 @@ function testConfig(config, filename) { return config } // Account for the possibility of an array for multiple configuration - return [].concat(config).some(c => minimatch(filename, c)) + return config.some(c => minimatch(filename, c)) } module.exports = function (context) { @@ -113,9 +113,9 @@ module.exports.schema = [ { 'type': 'object', 'properties': { - 'devDependencies': { 'type': ['boolean', 'string', 'array'] }, - 'optionalDependencies': { 'type': ['boolean', 'string', 'array'] }, - 'peerDependencies': { 'type': ['boolean', 'string', 'array'] }, + 'devDependencies': { 'type': ['boolean', 'array'] }, + 'optionalDependencies': { 'type': ['boolean', 'array'] }, + 'peerDependencies': { 'type': ['boolean', 'array'] }, }, 'additionalProperties': false, }, diff --git a/tests/src/rules/no-extraneous-dependencies.js b/tests/src/rules/no-extraneous-dependencies.js index a66a0325e..4d739ee71 100644 --- a/tests/src/rules/no-extraneous-dependencies.js +++ b/tests/src/rules/no-extraneous-dependencies.js @@ -37,7 +37,7 @@ ruleTester.run('no-extraneous-dependencies', rule, { }), test({ code: 'import chai from "chai"', - options: [{devDependencies: '*.spec.js'}], + options: [{devDependencies: ['*.spec.js']}], filename: 'foo.spec.js', }), test({ @@ -101,7 +101,7 @@ ruleTester.run('no-extraneous-dependencies', rule, { }), test({ code: 'import chai from "chai"', - options: [{devDependencies: '*.test.js'}], + options: [{devDependencies: ['*.test.js']}], filename: 'foo.tes.js', errors: [{ ruleId: 'no-extraneous-dependencies', From 2b9b500ca9253c53cb727f46cd3201e34dc1d10b Mon Sep 17 00:00:00 2001 From: Kenneth Powers Date: Tue, 30 Aug 2016 12:47:43 -0400 Subject: [PATCH 15/24] Update comments for no-extraneous-dependencies configuration. --- src/rules/no-extraneous-dependencies.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/rules/no-extraneous-dependencies.js b/src/rules/no-extraneous-dependencies.js index 0b1f3b4fd..65bdb27e5 100644 --- a/src/rules/no-extraneous-dependencies.js +++ b/src/rules/no-extraneous-dependencies.js @@ -73,11 +73,11 @@ function reportIfMissing(context, deps, depsOptions, node, name) { } function testConfig(config, filename) { - // Simplest configuration first + // Simplest configuration first, either a boolean or nothing. if (typeof config === 'boolean' || typeof config === 'undefined') { return config } - // Account for the possibility of an array for multiple configuration + // Array of globs. return config.some(c => minimatch(filename, c)) } From f6aa376f5055896893fbe055f8b716a18f69fff4 Mon Sep 17 00:00:00 2001 From: Jeremy Gayed Date: Sat, 13 Aug 2016 00:43:46 -0400 Subject: [PATCH 16/24] Add test --- tests/src/rules/max-dependencies.js | 78 +++++++++++++++++++++++++++++ 1 file changed, 78 insertions(+) create mode 100644 tests/src/rules/max-dependencies.js diff --git a/tests/src/rules/max-dependencies.js b/tests/src/rules/max-dependencies.js new file mode 100644 index 000000000..7377c1451 --- /dev/null +++ b/tests/src/rules/max-dependencies.js @@ -0,0 +1,78 @@ +import { test } from '../utils' + +import { RuleTester } from 'eslint' + +const ruleTester = new RuleTester() + , rule = require('rules/max-dependencies') + +ruleTester.run('max-dependencies', rule, { + valid: [ + test({ code: 'import "./foo.js"' }), + + test({ code: 'import "./foo.js"; import "./bar.js";', + options: [{ + max: 2, + }], + }), + + test({ code: 'import "./foo.js"; import "./bar.js"; const a = require("./foo.js"); const b = require("./bar.js");', + options: [{ + max: 2, + }], + }), + + test({ code: 'import {x, y, z} from "./foo"'}), + ], + invalid: [ + test({ + code: 'import { x } from \'./foo\'; import { y } from \'./foo\'; import {z} from \'./bar\';', + options: [{ + max: 1, + }], + errors: [ + 'Maximum number of dependencies (1) exceeded.', + ], + }), + + test({ + code: 'import { x } from \'./foo\'; import { y } from \'./bar\'; import { z } from \'./baz\';', + options: [{ + max: 2, + }], + errors: [ + 'Maximum number of dependencies (2) exceeded.', + ], + }), + + test({ + code: 'import { x } from \'./foo\'; require("./bar"); import { z } from \'./baz\';', + options: [{ + max: 2, + }], + errors: [ + 'Maximum number of dependencies (2) exceeded.', + ], + }), + + test({ + code: 'import { x } from \'./foo\'; import { z } from \'./foo\'; require("./bar"); const path = require("path");', + options: [{ + max: 2, + }], + errors: [ + 'Maximum number of dependencies (2) exceeded.', + ], + }), + + test({ + code: 'import type { x } from \'./foo\'; import type { y } from \'./bar\'', + parser: 'babel-eslint', + options: [{ + max: 1, + }], + errors: [ + 'Maximum number of dependencies (1) exceeded.', + ], + }), + ], +}) From 5eafd099c32a3fd151ce145a25ef16cb08fddf62 Mon Sep 17 00:00:00 2001 From: Jeremy Gayed Date: Sat, 13 Aug 2016 00:44:16 -0400 Subject: [PATCH 17/24] Add rule with docs --- docs/rules/max-dependencies.md | 44 ++++++++++++++++++++++++++++++ src/rules/max-dependencies.js | 49 ++++++++++++++++++++++++++++++++++ 2 files changed, 93 insertions(+) create mode 100644 docs/rules/max-dependencies.md create mode 100644 src/rules/max-dependencies.js diff --git a/docs/rules/max-dependencies.md b/docs/rules/max-dependencies.md new file mode 100644 index 000000000..78911f232 --- /dev/null +++ b/docs/rules/max-dependencies.md @@ -0,0 +1,44 @@ +# max-dependencies + +Forbid modules to have too many dependencies (`import` or `require` statements). + +This is a useful rule because a module with too many dependencies is code smell, and usually indicates the module is doing too much and/or should be broken up into smaller modules. + +Importing multiple named exports from a single module will only count once (e.g. `import {x, y, z} from './foo'` will only count as a single dependency). + +### Options + +This rule takes the following option: + +`max`: The maximum number of dependencies allowed. Anything over will trigger the rule. **Default is 10** if the rule is enabled and no `max` is specified. + +You can set the option like this: + +```js +"import/max-dependencies": ["error", {"max": 10}] +``` + + +## Example + +Given a max value of `{"max": 2}`: + +### Fail + +```js +import a from './a'; // 1 +const b = require('./b'); // 2 +import c from './c'; // 3 - exceeds max! +``` + +### Pass + +```js +import a from './a'; // 1 +const anotherA = require('./a'); // still 1 +import {x, y, z} from './foo'; // 2 +``` + +## When Not To Use It + +If you don't care how many dependencies a module has. diff --git a/src/rules/max-dependencies.js b/src/rules/max-dependencies.js new file mode 100644 index 000000000..396e21e54 --- /dev/null +++ b/src/rules/max-dependencies.js @@ -0,0 +1,49 @@ +import Set from 'es6-set' +import isStaticRequire from '../core/staticRequire' + +const DEFAULT_MAX = 10 + +const countDependencies = (dependencies, lastNode, context) => { + const {max} = context.options[0] || { max: DEFAULT_MAX } + + if (dependencies.size > max) { + context.report( + lastNode, + `Maximum number of dependencies (${max}) exceeded.` + ) + } +} + +module.exports = context => { + const dependencies = new Set() // keep track of dependencies + let lastNode // keep track of the last node to report on + + return { + ImportDeclaration(node) { + dependencies.add(node.source.value) + lastNode = node.source + }, + + CallExpression(node) { + if (isStaticRequire(node)) { + const [ requirePath ] = node.arguments + dependencies.add(requirePath.value) + lastNode = node + } + }, + + 'Program:exit': function () { + countDependencies(dependencies, lastNode, context) + }, + } +} + +module.exports.schema = [ + { + 'type': 'object', + 'properties': { + 'max': { 'type': 'number' }, + }, + 'additionalProperties': false, + }, +] From 90ff3b86c41eb457137be93cbfcf6a964f3bf151 Mon Sep 17 00:00:00 2001 From: Jeremy Gayed Date: Sat, 13 Aug 2016 00:44:33 -0400 Subject: [PATCH 18/24] Add to src index and main README --- README.md | 4 +++- src/index.js | 1 + 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index b396daf3b..249ba36ee 100644 --- a/README.md +++ b/README.md @@ -52,6 +52,7 @@ This plugin intends to support linting of ES2015+ (ES6+) import/export syntax, a [`no-amd`]: ./docs/rules/no-amd.md [`no-nodejs-modules`]: ./docs/rules/no-nodejs-modules.md + **Style guide:** * Ensure all imports appear before other statements ([`imports-first`]) @@ -61,6 +62,7 @@ This plugin intends to support linting of ES2015+ (ES6+) import/export syntax, a * Enforce a convention in module import order ([`order`]) * Enforce a newline after import statements ([`newline-after-import`]) * Prefer a default export if module exports a single name ([`prefer-default-export`]) +* Limit the maximum number of dependencies a module can have. ([`max-dependencies`]) [`imports-first`]: ./docs/rules/imports-first.md [`no-duplicates`]: ./docs/rules/no-duplicates.md @@ -69,7 +71,7 @@ This plugin intends to support linting of ES2015+ (ES6+) import/export syntax, a [`order`]: ./docs/rules/order.md [`newline-after-import`]: ./docs/rules/newline-after-import.md [`prefer-default-export`]: ./docs/rules/prefer-default-export.md - +[`max-dependencies`]: ./docs/rules/max-dependencies.md ## Installation diff --git a/src/index.js b/src/index.js index 52d5668c5..16d23da7e 100644 --- a/src/index.js +++ b/src/index.js @@ -16,6 +16,7 @@ export const rules = { 'no-amd': require('./rules/no-amd'), 'no-duplicates': require('./rules/no-duplicates'), 'imports-first': require('./rules/imports-first'), + 'max-dependencies': require('./rules/max-dependencies'), 'no-extraneous-dependencies': require('./rules/no-extraneous-dependencies'), 'no-nodejs-modules': require('./rules/no-nodejs-modules'), 'order': require('./rules/order'), From 27c7675e39d324960d557fd27fde3bed800c2178 Mon Sep 17 00:00:00 2001 From: Jeremy Gayed Date: Sun, 21 Aug 2016 23:22:53 -0400 Subject: [PATCH 19/24] Update changelog --- CHANGELOG.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f0a5a615e..8d0349328 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,10 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel ### Fixed - [`namespace`] exception for get property from `namespace` import, which are re-export from commonjs module ([#499] fixes [#416], thanks [@wKich]) +## [1.14.0] - 2016-08-21 +### Added +- [`max-dependencies`] for specifying the maximum number of dependencies (both `import` and `require`) a module can have. (see [#489], thanks [@tizmagik]) + ## [1.13.0] - 2016-08-11 ### Added - `allowComputed` option for [`namespace`] rule. If set to `true`, won't report @@ -293,11 +297,13 @@ for info on changes for earlier releases. [`no-mutable-exports`]: ./docs/rules/no-mutable-exports.md [`prefer-default-export`]: ./docs/rules/prefer-default-export.md [`no-restricted-paths`]: ./docs/rules/no-restricted-paths.md +[`max-dependencies`]: ./docs/rules/max-dependencies.md [#509]: https://github.com/benmosher/eslint-plugin-import/pull/509 [#508]: https://github.com/benmosher/eslint-plugin-import/pull/508 [#503]: https://github.com/benmosher/eslint-plugin-import/pull/503 [#499]: https://github.com/benmosher/eslint-plugin-import/pull/499 +[#489]: https://github.com/benmosher/eslint-plugin-import/pull/489 [#461]: https://github.com/benmosher/eslint-plugin-import/pull/461 [#444]: https://github.com/benmosher/eslint-plugin-import/pull/444 [#428]: https://github.com/benmosher/eslint-plugin-import/pull/428 @@ -430,3 +436,4 @@ for info on changes for earlier releases. [@zloirock]: https://github.com/zloirock [@rhys-vdw]: https://github.com/rhys-vdw [@wKich]: https://github.com/wKich +[@tizmagik]: https://github.com/tizmagik From 159034c59fe260d190d045eb0b3f1ffc13b80c26 Mon Sep 17 00:00:00 2001 From: Jeremy Gayed Date: Wed, 31 Aug 2016 16:34:04 -0400 Subject: [PATCH 20/24] a code smell --- docs/rules/max-dependencies.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/rules/max-dependencies.md b/docs/rules/max-dependencies.md index 78911f232..f4aa2a57a 100644 --- a/docs/rules/max-dependencies.md +++ b/docs/rules/max-dependencies.md @@ -2,7 +2,7 @@ Forbid modules to have too many dependencies (`import` or `require` statements). -This is a useful rule because a module with too many dependencies is code smell, and usually indicates the module is doing too much and/or should be broken up into smaller modules. +This is a useful rule because a module with too many dependencies is a code smell, and usually indicates the module is doing too much and/or should be broken up into smaller modules. Importing multiple named exports from a single module will only count once (e.g. `import {x, y, z} from './foo'` will only count as a single dependency). From 79f3f83651f9a6ca45da00b27d201107d3f6c95e Mon Sep 17 00:00:00 2001 From: Jeroen Engels Date: Thu, 1 Sep 2016 12:30:49 +0200 Subject: [PATCH 21/24] Add `no-absolute-path` rule (fixes #530) (#538) * Add `no-absolute-path` rule (fixes #530) * fix typo --- CHANGELOG.md | 4 ++ README.md | 2 + docs/rules/no-absolute-path.md | 27 +++++++++ src/core/importType.js | 5 ++ src/index.js | 1 + src/rules/no-absolute-path.js | 21 +++++++ tests/src/core/importType.js | 11 +++- tests/src/rules/no-absolute-path.js | 86 +++++++++++++++++++++++++++++ 8 files changed, 155 insertions(+), 2 deletions(-) create mode 100644 docs/rules/no-absolute-path.md create mode 100644 src/rules/no-absolute-path.js create mode 100644 tests/src/rules/no-absolute-path.js diff --git a/CHANGELOG.md b/CHANGELOG.md index f0a5a615e..af31210f9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel ## [Unreleased] ### Added - Added an `allow` option to [`no-nodejs-modules`] to allow exceptions ([#452], [#509]). +- Added [`no-absolute-path`] rule ([#530], [#538]) ### Fixed - [`no-named-as-default-member`] Allow default import to have a property named "default" ([#507]+[#508], thanks [@jquense] for both!) @@ -293,7 +294,9 @@ for info on changes for earlier releases. [`no-mutable-exports`]: ./docs/rules/no-mutable-exports.md [`prefer-default-export`]: ./docs/rules/prefer-default-export.md [`no-restricted-paths`]: ./docs/rules/no-restricted-paths.md +[`no-absolute-path`]: ./docs/rules/no-absolute-path.md +[#538]: https://github.com/benmosher/eslint-plugin-import/pull/538 [#509]: https://github.com/benmosher/eslint-plugin-import/pull/509 [#508]: https://github.com/benmosher/eslint-plugin-import/pull/508 [#503]: https://github.com/benmosher/eslint-plugin-import/pull/503 @@ -334,6 +337,7 @@ for info on changes for earlier releases. [#157]: https://github.com/benmosher/eslint-plugin-import/pull/157 [#314]: https://github.com/benmosher/eslint-plugin-import/pull/314 +[#530]: https://github.com/benmosher/eslint-plugin-import/issues/530 [#507]: https://github.com/benmosher/eslint-plugin-import/issues/507 [#478]: https://github.com/benmosher/eslint-plugin-import/issues/478 [#456]: https://github.com/benmosher/eslint-plugin-import/issues/456 diff --git a/README.md b/README.md index b396daf3b..fa746a991 100644 --- a/README.md +++ b/README.md @@ -19,12 +19,14 @@ This plugin intends to support linting of ES2015+ (ES6+) import/export syntax, a * Ensure a default export is present, given a default import. ([`default`]) * Ensure imported namespaces contain dereferenced properties as they are dereferenced. ([`namespace`]) * Restrict which files can be imported in a given folder ([`no-restricted-paths`]) +* Forbid import of modules using absolute paths ([`no-absolute-path`]) [`no-unresolved`]: ./docs/rules/no-unresolved.md [`named`]: ./docs/rules/named.md [`default`]: ./docs/rules/default.md [`namespace`]: ./docs/rules/namespace.md [`no-restricted-paths`]: ./docs/rules/no-restricted-paths.md +[`no-absolute-path`]: ./docs/rules/no-absolute-path.md **Helpful warnings:** diff --git a/docs/rules/no-absolute-path.md b/docs/rules/no-absolute-path.md new file mode 100644 index 000000000..9a4be74f8 --- /dev/null +++ b/docs/rules/no-absolute-path.md @@ -0,0 +1,27 @@ +# Forbid import of modules using absolute paths + +Node.js allows the import of modules using an absolute path such as `/home/xyz/file.js`. That is a bad practice as it ties the code using it to your computer, and therefore makes it unusable in packages distributed on `npm` for instance. + +## Rule Details + +### Fail + +```js +import f from '/foo'; +import f from '/some/path'; + +var f = require('/foo'); +var f = require('/some/path'); +``` + +### Pass + +```js +import _ from 'lodash'; +import foo from 'foo'; +import foo from './foo'; + +var _ = require('lodash'); +var foo = require('foo'); +var foo = require('./foo'); +``` diff --git a/src/core/importType.js b/src/core/importType.js index 02edc0234..5236b1d2a 100644 --- a/src/core/importType.js +++ b/src/core/importType.js @@ -8,6 +8,10 @@ function constant(value) { return () => value } +function isAbsolute(name) { + return name.indexOf('/') === 0 +} + export function isBuiltIn(name, settings) { const extras = (settings && settings['import/core-modules']) || [] return builtinModules.indexOf(name) !== -1 || extras.indexOf(name) > -1 @@ -46,6 +50,7 @@ function isRelativeToSibling(name) { } const typeTest = cond([ + [isAbsolute, constant('absolute')], [isBuiltIn, constant('builtin')], [isExternalModule, constant('external')], [isScoped, constant('external')], diff --git a/src/index.js b/src/index.js index 52d5668c5..6d2a5d22c 100644 --- a/src/index.js +++ b/src/index.js @@ -17,6 +17,7 @@ export const rules = { 'no-duplicates': require('./rules/no-duplicates'), 'imports-first': require('./rules/imports-first'), 'no-extraneous-dependencies': require('./rules/no-extraneous-dependencies'), + 'no-absolute-path': require('./rules/no-absolute-path'), 'no-nodejs-modules': require('./rules/no-nodejs-modules'), 'order': require('./rules/order'), 'newline-after-import': require('./rules/newline-after-import'), diff --git a/src/rules/no-absolute-path.js b/src/rules/no-absolute-path.js new file mode 100644 index 000000000..08c8b80fe --- /dev/null +++ b/src/rules/no-absolute-path.js @@ -0,0 +1,21 @@ +import importType from '../core/importType' +import isStaticRequire from '../core/staticRequire' + +function reportIfMissing(context, node, name) { + if (importType(name, context) === 'absolute') { + context.report(node, 'Do not import modules using an absolute path') + } +} + +module.exports = function (context) { + return { + ImportDeclaration: function handleImports(node) { + reportIfMissing(context, node, node.source.value) + }, + CallExpression: function handleRequires(node) { + if (isStaticRequire(node)) { + reportIfMissing(context, node, node.arguments[0].value) + } + }, + } +} diff --git a/tests/src/core/importType.js b/tests/src/core/importType.js index 28a8d7f4e..5b63910af 100644 --- a/tests/src/core/importType.js +++ b/tests/src/core/importType.js @@ -8,6 +8,12 @@ import { testContext } from '../utils' describe('importType(name)', function () { const context = testContext() + it("should return 'absolute' for paths starting with a /", function() { + expect(importType('/', context)).to.equal('absolute') + expect(importType('/path', context)).to.equal('absolute') + expect(importType('/some/path', context)).to.equal('absolute') + }) + it("should return 'builtin' for node.js modules", function() { expect(importType('fs', context)).to.equal('builtin') expect(importType('path', context)).to.equal('builtin') @@ -47,7 +53,7 @@ describe('importType(name)', function () { expect(importType('./importType/index.js', context)).to.equal('sibling') }) - describe("should return 'index' for sibling index file", function() { + it("should return 'index' for sibling index file", function() { expect(importType('.', context)).to.equal('index') expect(importType('./', context)).to.equal('index') expect(importType('./index', context)).to.equal('index') @@ -55,7 +61,8 @@ describe('importType(name)', function () { }) it("should return 'unknown' for any unhandled cases", function() { - expect(importType('/malformed', context)).to.equal('unknown') + expect(importType('@malformed', context)).to.equal('unknown') + expect(importType(' /malformed', context)).to.equal('unknown') expect(importType(' foo', context)).to.equal('unknown') }) diff --git a/tests/src/rules/no-absolute-path.js b/tests/src/rules/no-absolute-path.js new file mode 100644 index 000000000..88951672b --- /dev/null +++ b/tests/src/rules/no-absolute-path.js @@ -0,0 +1,86 @@ +import { test } from '../utils' + +import { RuleTester } from 'eslint' + +const ruleTester = new RuleTester() + , rule = require('rules/no-absolute-path') + +const error = { + ruleId: 'no-absolute-path', + message: 'Do not import modules using an absolute path', +} + +ruleTester.run('no-absolute-path', rule, { + valid: [ + test({ code: 'import _ from "lodash"'}), + test({ code: 'import find from "lodash.find"'}), + test({ code: 'import foo from "./foo"'}), + test({ code: 'import foo from "../foo"'}), + test({ code: 'import foo from "foo"'}), + test({ code: 'import foo from "./"'}), + test({ code: 'import foo from "@scope/foo"'}), + test({ code: 'var _ = require("lodash")'}), + test({ code: 'var find = require("lodash.find")'}), + test({ code: 'var foo = require("./foo")'}), + test({ code: 'var foo = require("../foo")'}), + test({ code: 'var foo = require("foo")'}), + test({ code: 'var foo = require("./")'}), + test({ code: 'var foo = require("@scope/foo")'}), + test({ + code: 'import events from "events"', + options: [{ + allow: ['events'], + }], + }), + test({ + code: 'import path from "path"', + options: [{ + allow: ['path'], + }], + }), + test({ + code: 'var events = require("events")', + options: [{ + allow: ['events'], + }], + }), + test({ + code: 'var path = require("path")', + options: [{ + allow: ['path'], + }], + }), + test({ + code: 'import path from "path";import events from "events"', + options: [{ + allow: ['path', 'events'], + }], + }), + ], + invalid: [ + test({ + code: 'import f from "/foo"', + errors: [error], + }), + test({ + code: 'import f from "/foo/path"', + errors: [error], + }), + test({ + code: 'import f from "/some/path"', + errors: [error], + }), + test({ + code: 'var f = require("/foo")', + errors: [error], + }), + test({ + code: 'var f = require("/foo/path")', + errors: [error], + }), + test({ + code: 'var f = require("/some/path")', + errors: [error], + }), + ], +}) From bd99bdf4ed928d224e9bedcaa6f024b227c5e720 Mon Sep 17 00:00:00 2001 From: Greenkeeper Date: Tue, 6 Sep 2016 10:31:38 +0200 Subject: [PATCH 22/24] chore(package): update babel-plugin-istanbul to version 2.0.1 (#539) https://greenkeeper.io/ --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 0f14eb4be..98162f160 100644 --- a/package.json +++ b/package.json @@ -45,7 +45,7 @@ "homepage": "https://github.com/benmosher/eslint-plugin-import", "devDependencies": { "babel-eslint": "next", - "babel-plugin-istanbul": "^1.0.3", + "babel-plugin-istanbul": "^2.0.1", "babel-preset-es2015": "^6.6.0", "babel-preset-es2015-loose": "^7.0.0", "babel-register": "6.9.0", From eb1a060af772e72d4e6c1004860f37ce74d06040 Mon Sep 17 00:00:00 2001 From: Ben Mosher Date: Mon, 12 Sep 2016 04:31:02 -0400 Subject: [PATCH 23/24] v1.15.0 --- CHANGELOG.md | 6 +++++- package.json | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a0a6b54b3..c81cd896b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,9 @@ This project adheres to [Semantic Versioning](http://semver.org/). This change log adheres to standards from [Keep a CHANGELOG](http://keepachangelog.com). ## [Unreleased] + + +## [1.15.0] - 2016-09-12 ### Added - Added an `allow` option to [`no-nodejs-modules`] to allow exceptions ([#452], [#509]). - Added [`no-absolute-path`] rule ([#530], [#538]) @@ -382,7 +385,8 @@ for info on changes for earlier releases. [#119]: https://github.com/benmosher/eslint-plugin-import/issues/119 [#89]: https://github.com/benmosher/eslint-plugin-import/issues/89 -[Unreleased]: https://github.com/benmosher/eslint-plugin-import/compare/v1.14.0...HEAD +[Unreleased]: https://github.com/benmosher/eslint-plugin-import/compare/v1.15.0...HEAD +[1.15.0]: https://github.com/benmosher/eslint-plugin-import/compare/v1.14.0...v1.15.0 [1.14.0]: https://github.com/benmosher/eslint-plugin-import/compare/v1.13.0...v1.14.0 [1.13.0]: https://github.com/benmosher/eslint-plugin-import/compare/v1.12.0...v1.13.0 [1.12.0]: https://github.com/benmosher/eslint-plugin-import/compare/v1.11.1...v1.12.0 diff --git a/package.json b/package.json index 98162f160..c31672dea 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "eslint-plugin-import", - "version": "1.14.0", + "version": "1.15.0", "description": "Import with sanity.", "main": "lib/index.js", "directories": { From d03577c344115e0f66b19bf1b05c8e7bb6d51fe3 Mon Sep 17 00:00:00 2001 From: Ben Mosher Date: Mon, 12 Sep 2016 16:39:26 -0400 Subject: [PATCH 24/24] need to get my act together. fortunately, @jfmengels is on it --- CHANGELOG.md | 3 --- 1 file changed, 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c81cd896b..3522dda4c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -299,11 +299,8 @@ for info on changes for earlier releases. [`no-mutable-exports`]: ./docs/rules/no-mutable-exports.md [`prefer-default-export`]: ./docs/rules/prefer-default-export.md [`no-restricted-paths`]: ./docs/rules/no-restricted-paths.md -<<<<<<< HEAD [`no-absolute-path`]: ./docs/rules/no-absolute-path.md -======= [`max-dependencies`]: ./docs/rules/max-dependencies.md ->>>>>>> tizmagik/feature/max-dependencies [#538]: https://github.com/benmosher/eslint-plugin-import/pull/538 [#527]: https://github.com/benmosher/eslint-plugin-import/pull/527