Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[New] no-parent-barrel-import: Forbid import from parent barrel file #2779

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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'),
}),

],
});