diff --git a/CHANGELOG.md b/CHANGELOG.md index 9b4cb56088..b24ccc722d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel - [`order`]: Add support for TypeScript's "import equals"-expressions ([#1785], thanks [@manuth]) - [`import/default`]: support default export in TSExportAssignment ([#1689], thanks [@Maxim-Mazurok]) - [`no-restricted-paths`]: add custom message support ([#1802], thanks [@malykhinvi]) +- add [`no-relative-packages`] ([#966], thanks [@panrafal]) ### Fixed - [`group-exports`]: Flow type export awareness ([#1702], thanks [@ernestostifano]) @@ -814,6 +815,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 @@ -1195,3 +1197,4 @@ for info on changes for earlier releases. [@adjerbetian]: https://github.com/adjerbetian [@Maxim-Mazurok]: https://github.com/Maxim-Mazurok [@malykhinvi]: https://github.com/malykhinvi +[@panrafal]: https://github.com/panrafal diff --git a/docs/rules/no-relative-packages.md b/docs/rules/no-relative-packages.md new file mode 100644 index 0000000000..53d0fe85f3 --- /dev/null +++ b/docs/rules/no-relative-packages.md @@ -0,0 +1,66 @@ +# 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 d0a98b7cb9..89306821dd 100644 --- a/src/index.js +++ b/src/index.js @@ -10,8 +10,8 @@ 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'), 'no-cycle': require('./rules/no-cycle'), 'no-named-default': require('./rules/no-named-default'), @@ -19,7 +19,6 @@ export const rules = { 'no-named-as-default-member': require('./rules/no-named-as-default-member'), 'no-anonymous-default-export': require('./rules/no-anonymous-default-export'), 'no-unused-modules': require('./rules/no-unused-modules'), - 'no-commonjs': require('./rules/no-commonjs'), 'no-amd': require('./rules/no-amd'), 'no-duplicates': require('./rules/no-duplicates'), diff --git a/src/rules/no-relative-packages.js b/src/rules/no-relative-packages.js new file mode 100644 index 0000000000..4e069a6b90 --- /dev/null +++ b/src/rules/no-relative-packages.js @@ -0,0 +1,72 @@ +import path from 'path' +import readPkgUp from 'read-pkg-up' + +import resolve from 'eslint-module-utils/resolve' +import importType from '../core/importType' +import isStaticRequire from '../core/staticRequire' +import docsUrl from '../docsUrl' + +function findNamedPackage(filePath) { + const found = readPkgUp.sync({cwd: filePath, normalize: false}) + // console.log(found) + 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('named'), + }, + schema: [], + }, + + create: function noRelativePackages(context) { + return { + ImportDeclaration(node) { + checkImportForRelativePackage(context, node.source.value, node.source) + }, + CallExpression(node) { + if (isStaticRequire(node)) { + const [firstArgument] = node.arguments + checkImportForRelativePackage(context, firstArgument.value, firstArgument) + } + }, + } + }, +} diff --git a/tests/files/package-named/index.js b/tests/files/package-named/index.js new file mode 100644 index 0000000000..ea9b101e1c --- /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 0000000000..dbda7111f0 --- /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/index.js b/tests/files/package/index.js new file mode 100644 index 0000000000..ea9b101e1c --- /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 0000000000..ad83f1ea7f --- /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 0000000000..cb2ff98648 --- /dev/null +++ b/tests/src/rules/no-relative-packages.js @@ -0,0 +1,66 @@ +import { RuleTester } from 'eslint' +import rule from 'rules/no-relative-packages' + +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'), + }), + ], + + 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 "@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 "eslint-plugin-import/tests/files/bar" instead of "../bar"', + line: 1, + column: 17, + } ], + }), + ], +})