From 4337f0f7719e6acf05a15651a356ddceb262088e Mon Sep 17 00:00:00 2001 From: Rafal Lindemann Date: Thu, 2 Nov 2017 22:11:14 +0100 Subject: [PATCH] New rule: `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. --- docs/rules/no-relative-packages.md | 66 +++++++++++++++++++++++ src/index.js | 1 + src/rules/no-relative-packages.js | 69 +++++++++++++++++++++++++ tests/files/package-named/index.js | 1 + tests/files/package-named/package.json | 5 ++ tests/files/package/index.js | 1 + tests/files/package/package.json | 4 ++ tests/src/rules/no-relative-packages.js | 66 +++++++++++++++++++++++ 8 files changed, 213 insertions(+) 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/index.js create mode 100644 tests/files/package/package.json create mode 100644 tests/src/rules/no-relative-packages.js 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 a92489eef4..c98e058f84 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-named-default': require('./rules/no-named-default'), 'no-named-as-default': require('./rules/no-named-as-default'), diff --git a/src/rules/no-relative-packages.js b/src/rules/no-relative-packages.js new file mode 100644 index 0000000000..d5802c3168 --- /dev/null +++ b/src/rules/no-relative-packages.js @@ -0,0 +1,69 @@ +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' + +module.exports = { + meta: { + docs: {}, + }, + + create: function noRelativePackages(context) { + + 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(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}"`, + }) + } + } + + return { + ImportDeclaration(node) { + checkImportForRelativePackage(node.source.value, node.source) + }, + CallExpression(node) { + if (isStaticRequire(node)) { + const [ firstArgument ] = node.arguments + checkImportForRelativePackage(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..c3180520c6 --- /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, + } ], + }), + ], +})