From 6f5c52cec32ba94558ea31faa7dc0b6a7c7af982 Mon Sep 17 00:00:00 2001 From: Rafal Lindemann Date: Thu, 2 Nov 2017 22:11:14 +0100 Subject: [PATCH] [New]: add `no-relative-packages` Use this rule to prevent importing packages through relative paths. It's useful in Yarn/Lerna workspaces, were it's possible to import a sibling package using `../package` relative path, while direct `package` is the correct one. Co-authored-by: Rafal Lindemann Co-authored-by: Tom Payne Co-authored-by: Jordan Harband --- CHANGELOG.md | 38 +++++++----- docs/rules/no-relative-packages.md | 66 +++++++++++++++++++++ src/index.js | 1 + src/rules/no-relative-packages.js | 61 +++++++++++++++++++ tests/files/package-named/index.js | 1 + tests/files/package-named/package.json | 5 ++ tests/files/package-scoped/index.js | 1 + tests/files/package-scoped/package.json | 5 ++ tests/files/package/index.js | 1 + tests/files/package/package.json | 4 ++ tests/src/rules/no-relative-packages.js | 79 +++++++++++++++++++++++++ 11 files changed, 246 insertions(+), 16 deletions(-) create mode 100644 docs/rules/no-relative-packages.md create mode 100644 src/rules/no-relative-packages.js create mode 100644 tests/files/package-named/index.js create mode 100644 tests/files/package-named/package.json create mode 100644 tests/files/package-scoped/index.js create mode 100644 tests/files/package-scoped/package.json create mode 100644 tests/files/package/index.js create mode 100644 tests/files/package/package.json create mode 100644 tests/src/rules/no-relative-packages.js diff --git a/CHANGELOG.md b/CHANGELOG.md index ae9c0dfd0..486c0248a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel ### Added - [`no-commonjs`]: Also detect require calls with expressionless template literals: ``` require(`x`) ``` ([#1958], thanks [@FloEdelmann]) - [`no-internal-modules`]: Add `forbid` option ([#1846], thanks [@guillaumewuip]) +- add [`no-relative-packages`] ([#1860], [#966], thanks [@tapayne88] [@panrafal]) ### Fixed - [`export`]/TypeScript: properly detect export specifiers as children of a TS module block ([#1889], thanks [@andreubotella]) @@ -739,6 +740,7 @@ for info on changes for earlier releases. [`no-named-export`]: ./docs/rules/no-named-export.md [`no-namespace`]: ./docs/rules/no-namespace.md [`no-nodejs-modules`]: ./docs/rules/no-nodejs-modules.md +[`no-relative-packages`]: ./docs/rules/no-relative-packages.md [`no-relative-parent-imports`]: ./docs/rules/no-relative-parent-imports.md [`no-restricted-paths`]: ./docs/rules/no-restricted-paths.md [`no-self-import`]: ./docs/rules/no-self-import.md @@ -754,23 +756,19 @@ for info on changes for earlier releases. [`memo-parser`]: ./memo-parser/README.md [#1974]: https://github.com/benmosher/eslint-plugin-import/pull/1974 -[#1944]: https://github.com/benmosher/eslint-plugin-import/pull/1944 -[#1924]: https://github.com/benmosher/eslint-plugin-import/issues/1924 -[#1965]: https://github.com/benmosher/eslint-plugin-import/issues/1965 [#1958]: https://github.com/benmosher/eslint-plugin-import/pull/1958 [#1948]: https://github.com/benmosher/eslint-plugin-import/pull/1948 [#1947]: https://github.com/benmosher/eslint-plugin-import/pull/1947 +[#1944]: https://github.com/benmosher/eslint-plugin-import/pull/1944 [#1940]: https://github.com/benmosher/eslint-plugin-import/pull/1940 [#1897]: https://github.com/benmosher/eslint-plugin-import/pull/1897 [#1889]: https://github.com/benmosher/eslint-plugin-import/pull/1889 [#1878]: https://github.com/benmosher/eslint-plugin-import/pull/1878 -[#1854]: https://github.com/benmosher/eslint-plugin-import/issues/1854 +[#1860]: https://github.com/benmosher/eslint-plugin-import/pull/1860 [#1848]: https://github.com/benmosher/eslint-plugin-import/pull/1848 [#1846]: https://github.com/benmosher/eslint-plugin-import/pull/1846 -[#1841]: https://github.com/benmosher/eslint-plugin-import/issues/1841 [#1836]: https://github.com/benmosher/eslint-plugin-import/pull/1836 [#1835]: https://github.com/benmosher/eslint-plugin-import/pull/1835 -[#1834]: https://github.com/benmosher/eslint-plugin-import/issues/1834 [#1833]: https://github.com/benmosher/eslint-plugin-import/pull/1833 [#1831]: https://github.com/benmosher/eslint-plugin-import/pull/1831 [#1830]: https://github.com/benmosher/eslint-plugin-import/pull/1830 @@ -780,7 +778,6 @@ for info on changes for earlier releases. [#1820]: https://github.com/benmosher/eslint-plugin-import/pull/1820 [#1819]: https://github.com/benmosher/eslint-plugin-import/pull/1819 [#1802]: https://github.com/benmosher/eslint-plugin-import/pull/1802 -[#1801]: https://github.com/benmosher/eslint-plugin-import/issues/1801 [#1788]: https://github.com/benmosher/eslint-plugin-import/pull/1788 [#1786]: https://github.com/benmosher/eslint-plugin-import/pull/1786 [#1785]: https://github.com/benmosher/eslint-plugin-import/pull/1785 @@ -794,10 +791,7 @@ for info on changes for earlier releases. [#1735]: https://github.com/benmosher/eslint-plugin-import/pull/1735 [#1726]: https://github.com/benmosher/eslint-plugin-import/pull/1726 [#1724]: https://github.com/benmosher/eslint-plugin-import/pull/1724 -[#1722]: https://github.com/benmosher/eslint-plugin-import/issues/1722 [#1719]: https://github.com/benmosher/eslint-plugin-import/pull/1719 -[#1704]: https://github.com/benmosher/eslint-plugin-import/issues/1704 -[#1702]: https://github.com/benmosher/eslint-plugin-import/issues/1702 [#1696]: https://github.com/benmosher/eslint-plugin-import/pull/1696 [#1691]: https://github.com/benmosher/eslint-plugin-import/pull/1691 [#1690]: https://github.com/benmosher/eslint-plugin-import/pull/1690 @@ -808,17 +802,12 @@ for info on changes for earlier releases. [#1664]: https://github.com/benmosher/eslint-plugin-import/pull/1664 [#1658]: https://github.com/benmosher/eslint-plugin-import/pull/1658 [#1651]: https://github.com/benmosher/eslint-plugin-import/pull/1651 -[#1635]: https://github.com/benmosher/eslint-plugin-import/issues/1635 -[#1631]: https://github.com/benmosher/eslint-plugin-import/issues/1631 [#1626]: https://github.com/benmosher/eslint-plugin-import/pull/1626 [#1620]: https://github.com/benmosher/eslint-plugin-import/pull/1620 [#1619]: https://github.com/benmosher/eslint-plugin-import/pull/1619 -[#1616]: https://github.com/benmosher/eslint-plugin-import/issues/1616 -[#1613]: https://github.com/benmosher/eslint-plugin-import/issues/1613 [#1612]: https://github.com/benmosher/eslint-plugin-import/pull/1612 [#1611]: https://github.com/benmosher/eslint-plugin-import/pull/1611 [#1605]: https://github.com/benmosher/eslint-plugin-import/pull/1605 -[#1589]: https://github.com/benmosher/eslint-plugin-import/issues/1589 [#1586]: https://github.com/benmosher/eslint-plugin-import/pull/1586 [#1572]: https://github.com/benmosher/eslint-plugin-import/pull/1572 [#1569]: https://github.com/benmosher/eslint-plugin-import/pull/1569 @@ -909,6 +898,7 @@ for info on changes for earlier releases. [#1068]: https://github.com/benmosher/eslint-plugin-import/pull/1068 [#1049]: https://github.com/benmosher/eslint-plugin-import/pull/1049 [#1046]: https://github.com/benmosher/eslint-plugin-import/pull/1046 +[#966]: https://github.com/benmosher/eslint-plugin-import/pull/966 [#944]: https://github.com/benmosher/eslint-plugin-import/pull/944 [#912]: https://github.com/benmosher/eslint-plugin-import/pull/912 [#908]: https://github.com/benmosher/eslint-plugin-import/pull/908 @@ -985,10 +975,24 @@ for info on changes for earlier releases. [#211]: https://github.com/benmosher/eslint-plugin-import/pull/211 [#164]: https://github.com/benmosher/eslint-plugin-import/pull/164 [#157]: https://github.com/benmosher/eslint-plugin-import/pull/157 +[#1965]: https://github.com/benmosher/eslint-plugin-import/issues/1965 +[#1924]: https://github.com/benmosher/eslint-plugin-import/issues/1924 +[#1854]: https://github.com/benmosher/eslint-plugin-import/issues/1854 +[#1841]: https://github.com/benmosher/eslint-plugin-import/issues/1841 +[#1834]: https://github.com/benmosher/eslint-plugin-import/issues/1834 [#1814]: https://github.com/benmosher/eslint-plugin-import/issues/1814 [#1811]: https://github.com/benmosher/eslint-plugin-import/issues/1811 [#1808]: https://github.com/benmosher/eslint-plugin-import/issues/1808 [#1805]: https://github.com/benmosher/eslint-plugin-import/issues/1805 +[#1801]: https://github.com/benmosher/eslint-plugin-import/issues/1801 +[#1722]: https://github.com/benmosher/eslint-plugin-import/issues/1722 +[#1704]: https://github.com/benmosher/eslint-plugin-import/issues/1704 +[#1702]: https://github.com/benmosher/eslint-plugin-import/issues/1702 +[#1635]: https://github.com/benmosher/eslint-plugin-import/issues/1635 +[#1631]: https://github.com/benmosher/eslint-plugin-import/issues/1631 +[#1616]: https://github.com/benmosher/eslint-plugin-import/issues/1616 +[#1613]: https://github.com/benmosher/eslint-plugin-import/issues/1613 +[#1589]: https://github.com/benmosher/eslint-plugin-import/issues/1589 [#1565]: https://github.com/benmosher/eslint-plugin-import/issues/1565 [#1366]: https://github.com/benmosher/eslint-plugin-import/issues/1366 [#1334]: https://github.com/benmosher/eslint-plugin-import/issues/1334 @@ -1324,4 +1328,6 @@ for info on changes for earlier releases. [@paztis]: https://github.com/paztis [@FloEdelmann]: https://github.com/FloEdelmann [@bicstone]: https://github.com/bicstone -[@guillaumewuip]: https://github.com/guillaumewuip \ No newline at end of file +[@guillaumewuip]: https://github.com/guillaumewuip +[@tapayne88]: https://github.com/tapayne88 +[@panrafal]: https://github.com/panrafal \ No newline at end of file diff --git a/docs/rules/no-relative-packages.md b/docs/rules/no-relative-packages.md new file mode 100644 index 000000000..d5a068493 --- /dev/null +++ b/docs/rules/no-relative-packages.md @@ -0,0 +1,66 @@ +# import/no-relative-packages + +Use this rule to prevent importing packages through relative paths. + +It's useful in Yarn/Lerna workspaces, were it's possible to import a sibling +package using `../package` relative path, while direct `package` is the correct one. + + +### Examples + +Given the following folder structure: + +``` +my-project +├── packages +│ ├── foo +│ │ ├── index.js +│ │ └── package.json +│ └── bar +│ ├── index.js +│ └── package.json +└── entry.js +``` + +And the .eslintrc file: +``` +{ + ... + "rules": { + "import/no-relative-packages": "error" + } +} +``` + +The following patterns are considered problems: + +```js +/** + * in my-project/packages/foo.js + */ + +import bar from '../bar'; // Import sibling package using relative path +import entry from '../../entry.js'; // Import from parent package using relative path + +/** + * in my-project/entry.js + */ + +import bar from './packages/bar'; // Import child package using relative path +``` + +The following patterns are NOT considered problems: + +```js +/** + * in my-project/packages/foo.js + */ + +import bar from 'bar'; // Import sibling package using package name + +/** + * in my-project/entry.js + */ + +import bar from 'bar'; // Import sibling package using package name +``` diff --git a/src/index.js b/src/index.js index dbddba8c0..17002337e 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-packages': require('./rules/no-relative-packages'), 'no-relative-parent-imports': require('./rules/no-relative-parent-imports'), 'no-self-import': require('./rules/no-self-import'), diff --git a/src/rules/no-relative-packages.js b/src/rules/no-relative-packages.js new file mode 100644 index 000000000..a654c0839 --- /dev/null +++ b/src/rules/no-relative-packages.js @@ -0,0 +1,61 @@ +import path from 'path'; +import readPkgUp from 'read-pkg-up'; + +import resolve from 'eslint-module-utils/resolve'; +import moduleVisitor, { makeOptionsSchema } from 'eslint-module-utils/moduleVisitor'; +import importType from '../core/importType'; +import docsUrl from '../docsUrl'; + +function findNamedPackage(filePath) { + const found = readPkgUp.sync({ cwd: filePath, normalize: false }); + if (found.pkg && !found.pkg.name) { + return findNamedPackage(path.join(found.path, '../..')); + } + return found; +} + +function checkImportForRelativePackage(context, importPath, node) { + const potentialViolationTypes = ['parent', 'index', 'sibling']; + if (potentialViolationTypes.indexOf(importType(importPath, context)) === -1) { + return; + } + + const resolvedImport = resolve(importPath, context); + const resolvedContext = context.getFilename(); + + if (!resolvedImport || !resolvedContext) { + return; + } + + const importPkg = findNamedPackage(resolvedImport); + const contextPkg = findNamedPackage(resolvedContext); + + if (importPkg.pkg && contextPkg.pkg && importPkg.pkg.name !== contextPkg.pkg.name) { + const importBaseName = path.basename(importPath); + const importRoot = path.dirname(importPkg.path); + const properPath = path.relative(importRoot, resolvedImport); + const properImport = path.join( + importPkg.pkg.name, + path.dirname(properPath), + importBaseName === path.basename(importRoot) ? '' : importBaseName + ); + context.report({ + node, + message: `Relative import from another package is not allowed. Use \`${properImport}\` instead of \`${importPath}\``, + }); + } +} + +module.exports = { + meta: { + type: 'suggestion', + docs: { + url: docsUrl('no-relative-packages'), + }, + schema: [makeOptionsSchema()], + }, + + create(context) { + return moduleVisitor((source) => checkImportForRelativePackage(context, source.value, source), context.options[0]); + }, +}; diff --git a/tests/files/package-named/index.js b/tests/files/package-named/index.js new file mode 100644 index 000000000..ea9b101e1 --- /dev/null +++ b/tests/files/package-named/index.js @@ -0,0 +1 @@ +export default function () {} diff --git a/tests/files/package-named/package.json b/tests/files/package-named/package.json new file mode 100644 index 000000000..dbda7111f --- /dev/null +++ b/tests/files/package-named/package.json @@ -0,0 +1,5 @@ +{ + "name": "package-named", + "description": "Standard, named package", + "main": "index.js" +} \ No newline at end of file diff --git a/tests/files/package-scoped/index.js b/tests/files/package-scoped/index.js new file mode 100644 index 000000000..ea9b101e1 --- /dev/null +++ b/tests/files/package-scoped/index.js @@ -0,0 +1 @@ +export default function () {} diff --git a/tests/files/package-scoped/package.json b/tests/files/package-scoped/package.json new file mode 100644 index 000000000..a2d81cbae --- /dev/null +++ b/tests/files/package-scoped/package.json @@ -0,0 +1,5 @@ +{ + "name": "@scope/package-named", + "description": "Scoped, named package", + "main": "index.js" +} diff --git a/tests/files/package/index.js b/tests/files/package/index.js new file mode 100644 index 000000000..ea9b101e1 --- /dev/null +++ b/tests/files/package/index.js @@ -0,0 +1 @@ +export default function () {} diff --git a/tests/files/package/package.json b/tests/files/package/package.json new file mode 100644 index 000000000..ad83f1ea7 --- /dev/null +++ b/tests/files/package/package.json @@ -0,0 +1,4 @@ +{ + "description": "Unnamed package for reaching through main field - rxjs style", + "main": "index.js" +} \ No newline at end of file diff --git a/tests/src/rules/no-relative-packages.js b/tests/src/rules/no-relative-packages.js new file mode 100644 index 000000000..1a706387c --- /dev/null +++ b/tests/src/rules/no-relative-packages.js @@ -0,0 +1,79 @@ +import { RuleTester } from 'eslint'; +import rule from 'rules/no-relative-packages'; +import { normalize } from 'path'; + +import { test, testFilePath } from '../utils'; + +const ruleTester = new RuleTester(); + +ruleTester.run('no-relative-packages', rule, { + valid: [ + test({ + code: 'import foo from "./index.js"', + filename: testFilePath('./package/index.js'), + }), + test({ + code: 'import bar from "../bar"', + filename: testFilePath('./package/index.js'), + }), + test({ + code: 'import {foo} from "a"', + filename: testFilePath('./package-named/index.js'), + }), + test({ + code: 'const bar = require("../bar.js")', + filename: testFilePath('./package/index.js'), + }), + test({ + code: 'const bar = require("../not/a/file/path.js")', + filename: testFilePath('./package/index.js'), + }), + test({ + code: 'import "package"', + filename: testFilePath('./package/index.js'), + }), + test({ + code: 'require("../bar.js")', + filename: testFilePath('./package/index.js'), + }), + ], + + invalid: [ + test({ + code: 'import foo from "./package-named"', + filename: testFilePath('./bar.js'), + errors: [ { + message: 'Relative import from another package is not allowed. Use `package-named` instead of `./package-named`', + line: 1, + column: 17, + } ], + }), + test({ + code: 'import foo from "../package-named"', + filename: testFilePath('./package/index.js'), + errors: [ { + message: 'Relative import from another package is not allowed. Use `package-named` instead of `../package-named`', + line: 1, + column: 17, + } ], + }), + test({ + code: 'import foo from "../package-scoped"', + filename: testFilePath('./package/index.js'), + errors: [ { + message: `Relative import from another package is not allowed. Use \`${normalize('@scope/package-named')}\` instead of \`../package-scoped\``, + line: 1, + column: 17, + } ], + }), + test({ + code: 'import bar from "../bar"', + filename: testFilePath('./package-named/index.js'), + errors: [ { + message: `Relative import from another package is not allowed. Use \`${normalize('eslint-plugin-import/tests/files/bar')}\` instead of \`../bar\``, + line: 1, + column: 17, + } ], + }), + ], +});