Skip to content

Commit

Permalink
[New]: add no-relative-packages
Browse files Browse the repository at this point in the history
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 <rl@stamina.pl>
Co-authored-by: Tom Payne <tom@tompayne.dev>
Co-authored-by: Jordan Harband <ljharb@gmail.com>
  • Loading branch information
3 people committed Nov 2, 2017
1 parent 319d0ca commit 6f5c52c
Show file tree
Hide file tree
Showing 11 changed files with 246 additions and 16 deletions.
38 changes: 22 additions & 16 deletions CHANGELOG.md
Expand Up @@ -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])
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
[@guillaumewuip]: https://github.com/guillaumewuip
[@tapayne88]: https://github.com/tapayne88
[@panrafal]: https://github.com/panrafal
66 changes: 66 additions & 0 deletions 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
```
1 change: 1 addition & 0 deletions src/index.js
Expand Up @@ -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'),
Expand Down
61 changes: 61 additions & 0 deletions 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]);
},
};
1 change: 1 addition & 0 deletions tests/files/package-named/index.js
@@ -0,0 +1 @@
export default function () {}
5 changes: 5 additions & 0 deletions tests/files/package-named/package.json
@@ -0,0 +1,5 @@
{
"name": "package-named",
"description": "Standard, named package",
"main": "index.js"
}
1 change: 1 addition & 0 deletions tests/files/package-scoped/index.js
@@ -0,0 +1 @@
export default function () {}
5 changes: 5 additions & 0 deletions tests/files/package-scoped/package.json
@@ -0,0 +1,5 @@
{
"name": "@scope/package-named",
"description": "Scoped, named package",
"main": "index.js"
}
1 change: 1 addition & 0 deletions tests/files/package/index.js
@@ -0,0 +1 @@
export default function () {}
4 changes: 4 additions & 0 deletions tests/files/package/package.json
@@ -0,0 +1,4 @@
{
"description": "Unnamed package for reaching through main field - rxjs style",
"main": "index.js"
}
79 changes: 79 additions & 0 deletions 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,
} ],
}),
],
});

0 comments on commit 6f5c52c

Please sign in to comment.