diff --git a/.travis.yml b/.travis.yml index db060b4bd6..ea8c60a593 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,5 +1,6 @@ language: node_js node_js: + - '10' - '8' - '6' - '4' @@ -7,6 +8,7 @@ node_js: os: linux env: + - ESLINT_VERSION=5 - ESLINT_VERSION=4 - ESLINT_VERSION=3 - ESLINT_VERSION=2 @@ -14,18 +16,25 @@ env: # osx backlog is often deep, so to be polite we can just hit these highlights matrix: include: + - env: PACKAGE=resolvers/node + node_js: 10 - env: PACKAGE=resolvers/node node_js: 8 - env: PACKAGE=resolvers/node node_js: 6 - env: PACKAGE=resolvers/node node_js: 4 + - env: PACKAGE=resolvers/webpack + node_js: 10 - env: PACKAGE=resolvers/webpack node_js: 8 - env: PACKAGE=resolvers/webpack node_js: 6 - env: PACKAGE=resolvers/webpack node_js: 4 + - os: osx + env: ESLINT_VERSION=5 + node_js: 10 - os: osx env: ESLINT_VERSION=4 node_js: 8 @@ -35,6 +44,9 @@ matrix: - os: osx env: ESLINT_VERSION=2 node_js: 4 + exclude: + - node_js: '4' + env: ESLINT_VERSION=5 before_install: - 'nvm install-latest-npm' diff --git a/CHANGELOG.md b/CHANGELOG.md index 7302400637..d384d5c390 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,13 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel ## [Unreleased] +## [2.13.0] - 2018-06-24 +### Added +- Add ESLint 5 support ([#1122], thanks [@ai] and [@ljharb]) +- Add [`no-relative-parent-imports`] rule: disallow relative imports from parent directories ([#1093], thanks [@chrislloyd]) + +### Fixed +- `namespace` rule: ensure it works in eslint 5/ecmaVersion 2018 (thanks [@ljharb]) ## [2.12.0] - 2018-05-17 ### Added @@ -466,6 +473,8 @@ for info on changes for earlier releases. [`memo-parser`]: ./memo-parser/README.md +[#1122]: https://github.com/benmosher/eslint-plugin-import/pull/1122 +[#1093]: https://github.com/benmosher/eslint-plugin-import/pull/1093 [#1085]: https://github.com/benmosher/eslint-plugin-import/pull/1085 [#1068]: https://github.com/benmosher/eslint-plugin-import/pull/1068 [#1046]: https://github.com/benmosher/eslint-plugin-import/pull/1046 @@ -722,3 +731,5 @@ for info on changes for earlier releases. [@manovotny]: https://github.com/manovotny [@mattijsbliek]: https://github.com/mattijsbliek [@hulkish]: https://github.com/hulkish +[@chrislloyd]: https://github.com/chrislloyd +[@ai]: https://github.com/ai diff --git a/README.md b/README.md index 541c58296e..53b2640627 100644 --- a/README.md +++ b/README.md @@ -26,6 +26,7 @@ This plugin intends to support linting of ES2015+ (ES6+) import/export syntax, a * Forbid a module from importing itself ([`no-self-import`]) * Forbid a module from importing a module with a dependency path back to itself ([`no-cycle`]) * Prevent unnecessary path segments in import and require statements ([`no-useless-path-segments`]) +* Forbid importing modules from parent directories ([`no-relative-parent-imports`]) [`no-unresolved`]: ./docs/rules/no-unresolved.md [`named`]: ./docs/rules/named.md @@ -39,6 +40,7 @@ This plugin intends to support linting of ES2015+ (ES6+) import/export syntax, a [`no-self-import`]: ./docs/rules/no-self-import.md [`no-cycle`]: ./docs/rules/no-cycle.md [`no-useless-path-segments`]: ./docs/rules/no-useless-path-segments.md +[`no-relative-parent-imports`]: ./docs/rules/no-relative-parent-imports.md ### Helpful warnings diff --git a/appveyor.yml b/appveyor.yml index b2e2a2d31a..0176e12545 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -1,10 +1,16 @@ # Test against this version of Node.js environment: matrix: + - nodejs_version: "10" - nodejs_version: "8" - nodejs_version: "6" - nodejs_version: "4" +matrix: + fast_finish: true + allow_failures: + - nodejs_version: "4" # for eslint 5 + # platform: # - x86 # - x64 diff --git a/config/recommended.js b/config/recommended.js index a72a8b13dd..70514eed3e 100644 --- a/config/recommended.js +++ b/config/recommended.js @@ -23,7 +23,6 @@ module.exports = { // all of them) parserOptions: { sourceType: 'module', - ecmaVersion: 6, - ecmaFeatures: { experimentalObjectRestSpread: true }, + ecmaVersion: 2018, }, } diff --git a/docs/rules/no-relative-parent-imports.md b/docs/rules/no-relative-parent-imports.md new file mode 100644 index 0000000000..84913b5400 --- /dev/null +++ b/docs/rules/no-relative-parent-imports.md @@ -0,0 +1,120 @@ +# no-relative-parent-imports + +Use this rule to prevent imports to folders in relative parent paths. + +This rule is useful for enforcing tree-like folder structures instead of complex graph-like folder structures. While this restriction might be a departure from Node's default resolution style, it can lead large, complex codebases to be easier to maintain. If you've ever had debates over "where to put files" this rule is for you. + +To fix violations of this rule there are three general strategies. Given this example: + +``` +numbers +└── three.js +add.js +``` + +```js +// ./add.js +export default function (numbers) { + return numbers.reduce((sum, n) => sum + n, 0); +} + +// ./numbers/three.js +import add from '../add'; // violates import/no-relative-parent-imports + +export default function three() { + return add([1, 2]); +} +``` + +You can, + +1. Move the file to be in a sibling folder (or higher) of the dependency. + +`three.js` could be be in the same folder as `add.js`: + +``` +three.js +add.js +``` + +or since `add` doesn't have any imports, it could be in it's own directory (namespace): + +``` +math +└── add.js +three.js +``` + +2. Pass the dependency as an argument at runtime (dependency injection) + +```js +// three.js +export default function three(add) { + return add([1, 2]); +} + +// somewhere else when you use `three.js`: +import add from './add'; +import three from './numbers/three'; +console.log(three(add)); +``` + +3. Make the dependency a package so it's globally available to all files in your project: + +```js +import add from 'add'; // from https://www.npmjs.com/package/add +export default function three() { + return add([1,2]); +} +``` + +These are (respectively) static, dynamic & global solutions to graph-like dependency resolution. + +### Examples + +Given the following folder structure: + +``` +my-project +├── lib +│ ├── a.js +│ └── b.js +└── main.js +``` + +And the .eslintrc file: +``` +{ + ... + "rules": { + "import/no-relative-parent-imports": "error" + } +} +``` + +The following patterns are considered problems: + +```js +/** + * in my-project/lib/a.js + */ + +import bar from '../main'; // Import parent file using a relative path +``` + +The following patterns are NOT considered problems: + +```js +/** + * in my-project/main.js + */ + +import foo from 'foo'; // Import package using module path +import a from './lib/a'; // Import child file using relative path + +/** + * in my-project/lib/a.js + */ + +import b from './b'; // Import sibling file using relative path +``` diff --git a/package.json b/package.json index 6e0df24f29..7d39395a76 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "eslint-plugin-import", - "version": "2.12.0", + "version": "2.13.0", "description": "Import with sanity.", "engines": { "node": ">=4" @@ -52,7 +52,7 @@ "chai": "^3.5.0", "coveralls": "^3.0.0", "cross-env": "^4.0.0", - "eslint": "2.x - 4.x", + "eslint": "2.x - 5.x", "eslint-import-resolver-node": "file:./resolvers/node", "eslint-import-resolver-typescript": "^1.0.2", "eslint-import-resolver-webpack": "file:./resolvers/webpack", @@ -70,7 +70,7 @@ "typescript-eslint-parser": "^15.0.0" }, "peerDependencies": { - "eslint": "2.x - 4.x" + "eslint": "2.x - 5.x" }, "dependencies": { "contains-path": "^0.1.0", diff --git a/resolvers/webpack/CHANGELOG.md b/resolvers/webpack/CHANGELOG.md index 022d7447cf..93246bf0b7 100644 --- a/resolvers/webpack/CHANGELOG.md +++ b/resolvers/webpack/CHANGELOG.md @@ -5,6 +5,9 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel ## Unreleased +## 0.10.1 - 2018-06-24 +### Fixed +- log a useful error in a module bug arises ([#768]/[#767], thanks [@mattkrick]) ## 0.10.0 - 2018-05-17 ### Changed @@ -104,6 +107,7 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel [#1091]: https://github.com/benmosher/eslint-plugin-import/pull/1091 [#969]: https://github.com/benmosher/eslint-plugin-import/pull/969 [#968]: https://github.com/benmosher/eslint-plugin-import/pull/968 +[#768]: https://github.com/benmosher/eslint-plugin-import/pull/768 [#683]: https://github.com/benmosher/eslint-plugin-import/pull/683 [#572]: https://github.com/benmosher/eslint-plugin-import/pull/572 [#569]: https://github.com/benmosher/eslint-plugin-import/pull/569 @@ -118,6 +122,7 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel [#164]: https://github.com/benmosher/eslint-plugin-import/pull/164 [#788]: https://github.com/benmosher/eslint-plugin-import/issues/788 +[#767]: https://github.com/benmosher/eslint-plugin-import/issues/767 [#681]: https://github.com/benmosher/eslint-plugin-import/issues/681 [#435]: https://github.com/benmosher/eslint-plugin-import/issues/435 [#411]: https://github.com/benmosher/eslint-plugin-import/issues/411 @@ -141,3 +146,4 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel [@ljharb]: https://github.com/ljharb [@SkeLLLa]: https://github.com/SkeLLLa [@graingert]: https://github.com/graingert +[@mattkrick]: https://github.com/mattkrick diff --git a/resolvers/webpack/index.js b/resolvers/webpack/index.js index 1a39d92a1b..4c9abc61d8 100644 --- a/resolvers/webpack/index.js +++ b/resolvers/webpack/index.js @@ -65,7 +65,12 @@ exports.resolve = function (source, file, settings) { log('Config path resolved to:', configPath) if (configPath) { - webpackConfig = require(configPath) + try { + webpackConfig = require(configPath) + } catch(e) { + console.log('Error resolving webpackConfig', e) + throw e + } } else { log("No config path found relative to", file, "; using {}") webpackConfig = {} diff --git a/resolvers/webpack/package.json b/resolvers/webpack/package.json index e1c586019f..a36a78e474 100644 --- a/resolvers/webpack/package.json +++ b/resolvers/webpack/package.json @@ -1,6 +1,6 @@ { "name": "eslint-import-resolver-webpack", - "version": "0.10.0", + "version": "0.10.1", "description": "Resolve paths to dependencies, given a webpack.config.js. Plugin for eslint-plugin-import.", "main": "index.js", "scripts": { diff --git a/src/index.js b/src/index.js index 5b55527b26..7df67867f5 100644 --- a/src/index.js +++ b/src/index.js @@ -10,6 +10,7 @@ export const rules = { 'no-restricted-paths': require('./rules/no-restricted-paths'), 'no-internal-modules': require('./rules/no-internal-modules'), 'group-exports': require('./rules/group-exports'), + 'no-relative-parent-imports': require('./rules/no-relative-parent-imports'), 'no-self-import': require('./rules/no-self-import'), 'no-cycle': require('./rules/no-cycle'), diff --git a/src/rules/namespace.js b/src/rules/namespace.js index 71dd57db8c..93e5891594 100644 --- a/src/rules/namespace.js +++ b/src/rules/namespace.js @@ -161,7 +161,7 @@ module.exports = { if (pattern.type !== 'ObjectPattern') return for (let property of pattern.properties) { - if (property.type === 'ExperimentalRestProperty') { + if (property.type === 'ExperimentalRestProperty' || !property.key) { continue } diff --git a/src/rules/no-relative-parent-imports.js b/src/rules/no-relative-parent-imports.js new file mode 100644 index 0000000000..3153eeb784 --- /dev/null +++ b/src/rules/no-relative-parent-imports.js @@ -0,0 +1,34 @@ +import moduleVisitor, { makeOptionsSchema } from 'eslint-module-utils/moduleVisitor' +import docsUrl from '../docsUrl' +import { basename } from 'path' + +import importType from '../core/importType' + +module.exports = { + meta: { + docs: { + url: docsUrl('no-relative-parent-imports'), + }, + schema: [makeOptionsSchema()], + }, + + create: function noRelativePackages(context) { + const myPath = context.getFilename() + if (myPath === '') return {} // can't cycle-check a non-file + + function checkSourceValue(sourceNode) { + const depPath = sourceNode.value + if (importType(depPath, context) === 'parent') { + context.report({ + node: sourceNode, + message: 'Relative imports from parent directories are not allowed. ' + + `Please either pass what you're importing through at runtime ` + + `(dependency injection), move \`${basename(myPath)}\` to same ` + + `directory as \`${depPath}\` or consider making \`${depPath}\` a package.`, + }) + } + } + + return moduleVisitor(checkSourceValue, context.options[0]) + }, +} diff --git a/tests/src/rules/namespace.js b/tests/src/rules/namespace.js index 19a69a8d98..1cfee2b54d 100644 --- a/tests/src/rules/namespace.js +++ b/tests/src/rules/namespace.js @@ -96,9 +96,7 @@ const valid = [ test({ code: `import * as names from './named-exports'; const {a, b, ...rest} = names;`, parserOptions: { - ecmaFeatures: { - experimentalObjectRestSpread: true, - }, + ecmaVersion: 2018, }, }), test({ diff --git a/tests/src/rules/no-relative-parent-imports.js b/tests/src/rules/no-relative-parent-imports.js new file mode 100644 index 0000000000..6d7a2c2fae --- /dev/null +++ b/tests/src/rules/no-relative-parent-imports.js @@ -0,0 +1,73 @@ +import { RuleTester } from 'eslint' +import rule from 'rules/no-relative-parent-imports' +import { test as _test, testFilePath } from '../utils' + +const test = def => _test(Object.assign(def, { + filename: testFilePath('./internal-modules/plugins/plugin2/index.js'), + parser: 'babel-eslint', +})) + +const ruleTester = new RuleTester() + +ruleTester.run('no-relative-parent-imports', rule, { + valid: [ + test({ + code: 'import foo from "./internal.js"', + }), + test({ + code: 'import foo from "./app/index.js"', + }), + test({ + code: 'import foo from "package"', + }), + test({ + code: 'require("./internal.js")', + options: [{ commonjs: true }], + }), + test({ + code: 'require("./app/index.js")', + options: [{ commonjs: true }], + }), + test({ + code: 'require("package")', + options: [{ commonjs: true }], + }), + test({ + code: 'import("./internal.js")', + }), + test({ + code: 'import("./app/index.js")', + }), + test({ + code: 'import("package")', + }), + ], + + invalid: [ + test({ + code: 'import foo from "../plugin.js"', + errors: [ { + message: 'Relative imports from parent directories are not allowed. Please either pass what you\'re importing through at runtime (dependency injection), move `index.js` to same directory as `../plugin.js` or consider making `../plugin.js` a package.', + line: 1, + column: 17, + } ], + }), + test({ + code: 'require("../plugin.js")', + options: [{ commonjs: true }], + errors: [ { + message: 'Relative imports from parent directories are not allowed. Please either pass what you\'re importing through at runtime (dependency injection), move `index.js` to same directory as `../plugin.js` or consider making `../plugin.js` a package.', + line: 1, + column: 9, + } ], + }), + test({ + code: 'import("../plugin.js")', + errors: [ { + message: 'Relative imports from parent directories are not allowed. Please either pass what you\'re importing through at runtime (dependency injection), move `index.js` to same directory as `../plugin.js` or consider making `../plugin.js` a package.', + line: 1, + column: 8, + } ], + }), + ], +}) diff --git a/tests/src/rules/no-restricted-paths.js b/tests/src/rules/no-restricted-paths.js index 88cc8ad150..13f8472cb1 100644 --- a/tests/src/rules/no-restricted-paths.js +++ b/tests/src/rules/no-restricted-paths.js @@ -28,6 +28,23 @@ ruleTester.run('no-restricted-paths', rule, { zones: [ { target: './tests/files/restricted-paths/client', from: './tests/files/restricted-paths/other' } ], } ], }), + + + // irrelevant function calls + test({ code: 'notrequire("../server/b.js")' }), + test({ + code: 'notrequire("../server/b.js")', + filename: testFilePath('./restricted-paths/client/a.js'), + options: [ { + zones: [ { target: './tests/files/restricted-paths/client', from: './tests/files/restricted-paths/server' } ], + } ], }), + + // no config + test({ code: 'require("../server/b.js")' }), + test({ code: 'import b from "../server/b.js"' }), + + // builtin (ignore) + test({ code: 'require("os")' }) ], invalid: [ diff --git a/utils/moduleVisitor.js b/utils/moduleVisitor.js index 7bb980e45d..2e736242e6 100644 --- a/utils/moduleVisitor.js +++ b/utils/moduleVisitor.js @@ -34,6 +34,18 @@ exports.default = function visitModules(visitor, options) { checkSourceValue(node.source, node) } + // for esmodule dynamic `import()` calls + function checkImportCall(node) { + if (node.callee.type !== 'Import') return + if (node.arguments.length !== 1) return + + const modulePath = node.arguments[0] + if (modulePath.type !== 'Literal') return + if (typeof modulePath.value !== 'string') return + + checkSourceValue(modulePath, node) + } + // for CommonJS `require` calls // adapted from @mctep: http://git.io/v4rAu function checkCommon(call) { @@ -74,6 +86,7 @@ exports.default = function visitModules(visitor, options) { 'ImportDeclaration': checkSource, 'ExportNamedDeclaration': checkSource, 'ExportAllDeclaration': checkSource, + 'CallExpression': checkImportCall, }) }