Skip to content

Commit

Permalink
feat: add no-parent-barrel-import rule
Browse files Browse the repository at this point in the history
  • Loading branch information
jonioni committed May 8, 2023
1 parent 328064a commit 9a811b4
Show file tree
Hide file tree
Showing 10 changed files with 190 additions and 0 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange

## [Unreleased]

### Added

- Add [`no-parent-barrel-import`] rule: forbids a module from importing from parent barrel file

### Fixed
- [`no-duplicates`]: remove duplicate identifiers in duplicate imports ([#2577], thanks [@joe-matsec])
- [`consistent-type-specifier-style`]: fix accidental removal of comma in certain cases ([#2754], thanks [@bradzacher])
Expand Down Expand Up @@ -1057,6 +1061,7 @@ for info on changes for earlier releases.
[`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-parent-barrel-import`]: ./docs/rules/no-parent-barrel-import.md
[`no-self-import`]: ./docs/rules/no-self-import.md
[`no-unassigned-import`]: ./docs/rules/no-unassigned-import.md
[`no-unresolved`]: ./docs/rules/no-unresolved.md
Expand Down
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ This plugin intends to support linting of ES2015+ (ES6+) import/export syntax, a
| [no-cycle](docs/rules/no-cycle.md) | Forbid a module from importing a module with a dependency path back to itself. | | | | | | |
| [no-dynamic-require](docs/rules/no-dynamic-require.md) | Forbid `require()` calls with expressions. | | | | | | |
| [no-internal-modules](docs/rules/no-internal-modules.md) | Forbid importing the submodules of other modules. | | | | | | |
| [no-parent-barrel-import](docs/rules/no-parent-barrel-import.md) | Forbid a module from importing from parent barrel file. | | | | | | |
| [no-relative-packages](docs/rules/no-relative-packages.md) | Forbid importing packages through relative paths. | | | | 🔧 | | |
| [no-relative-parent-imports](docs/rules/no-relative-parent-imports.md) | Forbid importing modules from parent directories. | | | | | | |
| [no-restricted-paths](docs/rules/no-restricted-paths.md) | Enforce which files can be imported in a given folder. | | | | | | |
Expand Down
63 changes: 63 additions & 0 deletions docs/rules/no-parent-barrel-import.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
# import/no-parent-barrel-import

<!-- end auto-generated rule header -->

Forbid a module from importing from parent barrel file, as it often leads to runtime error `Cannot read ... of undefined`.

It resolves the missing circular import check from [`no-self-import`], while being computationally cheap (see [`no-cycle`]).

## Rule Details

### Fail

```js
// @foo/index.ts
export * from "./bar";

// @foo/bar/index.ts
export * from "./baz";
export * from "./qux";

// @foo/bar/baz.ts (cannot read property `X` of undefined)
import { T } from '../..'; // relative
import { T } from '..'; // relative
import { T } from '@foo'; // absolute
import { T } from '@foo/bar'; // absolute

export const X = T.X;

// @foo/bar/qux.ts
export enum T {
X = "..."
}
```

### Pass

```js
// @foo/index.ts
export * from "./bar";

// @foo/bar/index.ts
export * from "./baz";
export * from "./qux";

// @foo/bar/baz.ts (relative import for code in `@foo`)
import { T } from "./baz"; // relative
import { T } from "@foo/bar/qux"; // absolute

export const X = T.X;

// @foo/bar/qux.ts
export enum T {
X = "..."
}
```

## Further Reading

- [Related Discussion](https://github.com/import-js/eslint-plugin-import/pull/2318#issuecomment-1027807460)
- Rule to detect that module imports itself: [`no-self-import`], [`no-cycle`]

[`no-self-import`]: ./no-self-import.md
[`no-cycle`]: ./no-cycle.md
1 change: 1 addition & 0 deletions src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ export const rules = {
'consistent-type-specifier-style': require('./rules/consistent-type-specifier-style'),

'no-self-import': require('./rules/no-self-import'),
'no-parent-barrel-import': require('./rules/no-parent-barrel-import'),
'no-cycle': require('./rules/no-cycle'),
'no-named-default': require('./rules/no-named-default'),
'no-named-as-default': require('./rules/no-named-as-default'),
Expand Down
45 changes: 45 additions & 0 deletions src/rules/no-parent-barrel-import.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
/**
* @fileOverview Forbids a module from importing from parent barrel file
* @author jonioni
*/

const { parse } = require('path');
const resolve = require('eslint-module-utils/resolve').default;
const moduleVisitor = require('eslint-module-utils/moduleVisitor').default;
const docsUrl = require('../docsUrl').default;

function isImportingFromParentBarrel(context, node, requireName) {
let filePath;
if (context.getPhysicalFilename) {
filePath = context.getPhysicalFilename();
} else if (context.getFilename) {
filePath = context.getFilename();
}
const importPath = resolve(requireName, context);
console.info(`File Path: ${filePath} and ${importPath}`);
const importDetails = parse(importPath);
if (importDetails.name === 'index' && filePath.startsWith(importDetails.dir)) {
context.report({
node,
message: 'Module imports from parent barrel file.',
});
}
}

module.exports = {
meta: {
type: 'problem',
docs: {
category: 'Static analysis',
description: 'Forbid a module from importing from parent barrel file.',
recommended: true,
url: docsUrl('no-parent-barrel-import'),
},
schema: [],
},
create(context) {
return isImportingFromParentBarrel((source, node) => {
moduleVisitor(context, node, source.value);
});
},
};
1 change: 1 addition & 0 deletions tests/files/no-parent-barrel-import/foo/bar.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
// Used in `no-parent-barrel-import` tests
1 change: 1 addition & 0 deletions tests/files/no-parent-barrel-import/foo/baz.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
// Used in `no-parent-barrel-import` tests
1 change: 1 addition & 0 deletions tests/files/no-parent-barrel-import/foo/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
// Used in `no-parent-barrel-import` tests
2 changes: 2 additions & 0 deletions tests/files/no-parent-barrel-import/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
// Used in `no-parent-barrel-import` tests

70 changes: 70 additions & 0 deletions tests/src/rules/no-parent-barrel-import.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
import { test, testFilePath } from '../utils';

import { RuleTester } from 'eslint';

const ruleTester = new RuleTester();
const rule = require('rules/no-parent-barrel-import');

const error = {
message: 'Module imports from parent barrel file.',
};

ruleTester.run('no-parent-barrel-import', rule, {
valid: [
test({
code: 'import _ from "lodash"',
filename: testFilePath('./no-parent-barrel-import/index.js'),
}),
test({
code: 'import baz from "./baz"',
filename: testFilePath('./no-parent-barrel-import/foo/bar.js'),
}),
test({
code: 'import bar from "./bar"',
filename: testFilePath('./no-parent-barrel-import/foo/baz.js'),
}),
],
invalid: [
test({
code: 'import baz from "."',
errors: [error],
filename: testFilePath('./no-parent-barrel-import/foo/bar.js'),
}),
test({
code: 'import baz from "../.."',
errors: [error],
filename: testFilePath('./no-parent-barrel-import/foo/bar.js'),
}),
test({
code: 'var foo = require("./index.js")',
errors: [error],
filename: testFilePath('./no-parent-barrel-import/foo/bar.js'),
}),
test({
code: 'var foo = require(".")',
errors: [error],
filename: testFilePath('./no-parent-barrel-import/foo/baz.js'),
}),
test({
code: 'var foo = require("..")',
errors: [error],
filename: testFilePath('./no-parent-barrel-import/foo/bar.js'),
}),
test({
code: 'var foo = require("././././")',
errors: [error],
filename: testFilePath('./no-parent-barrel-import/foo/index.js'),
}),
test({
code: 'var root = require("../../..")',
errors: [error],
filename: testFilePath('./no-parent-barrel-import/foo/index.js'),
}),
test({
code: 'var root = require("..")',
errors: [error],
filename: testFilePath('./no-parent-barrel-import/index.js'),
}),

],
});

0 comments on commit 9a811b4

Please sign in to comment.