From 9a811b41518288ad0c5dee29547f1b50e7093878 Mon Sep 17 00:00:00 2001 From: jonioni Date: Mon, 8 May 2023 05:47:22 -0400 Subject: [PATCH] feat: add no-parent-barrel-import rule --- CHANGELOG.md | 5 ++ README.md | 1 + docs/rules/no-parent-barrel-import.md | 63 +++++++++++++++++ src/index.js | 1 + src/rules/no-parent-barrel-import.js | 45 ++++++++++++ .../files/no-parent-barrel-import/foo/bar.js | 1 + .../files/no-parent-barrel-import/foo/baz.js | 1 + .../no-parent-barrel-import/foo/index.js | 1 + tests/files/no-parent-barrel-import/index.js | 2 + tests/src/rules/no-parent-barrel-import.js | 70 +++++++++++++++++++ 10 files changed, 190 insertions(+) create mode 100644 docs/rules/no-parent-barrel-import.md create mode 100644 src/rules/no-parent-barrel-import.js create mode 100644 tests/files/no-parent-barrel-import/foo/bar.js create mode 100644 tests/files/no-parent-barrel-import/foo/baz.js create mode 100644 tests/files/no-parent-barrel-import/foo/index.js create mode 100644 tests/files/no-parent-barrel-import/index.js create mode 100644 tests/src/rules/no-parent-barrel-import.js diff --git a/CHANGELOG.md b/CHANGELOG.md index 8a8ec3669..fcc9b2090 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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]) @@ -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 diff --git a/README.md b/README.md index ca99b8faf..18d47d6e3 100644 --- a/README.md +++ b/README.md @@ -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. | | | | | | | diff --git a/docs/rules/no-parent-barrel-import.md b/docs/rules/no-parent-barrel-import.md new file mode 100644 index 000000000..643132d92 --- /dev/null +++ b/docs/rules/no-parent-barrel-import.md @@ -0,0 +1,63 @@ +# import/no-parent-barrel-import + + + +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 diff --git a/src/index.js b/src/index.js index feafba900..27a23c483 100644 --- a/src/index.js +++ b/src/index.js @@ -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'), diff --git a/src/rules/no-parent-barrel-import.js b/src/rules/no-parent-barrel-import.js new file mode 100644 index 000000000..5a640f619 --- /dev/null +++ b/src/rules/no-parent-barrel-import.js @@ -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); + }); + }, +}; diff --git a/tests/files/no-parent-barrel-import/foo/bar.js b/tests/files/no-parent-barrel-import/foo/bar.js new file mode 100644 index 000000000..b4d754a96 --- /dev/null +++ b/tests/files/no-parent-barrel-import/foo/bar.js @@ -0,0 +1 @@ +// Used in `no-parent-barrel-import` tests diff --git a/tests/files/no-parent-barrel-import/foo/baz.js b/tests/files/no-parent-barrel-import/foo/baz.js new file mode 100644 index 000000000..b4d754a96 --- /dev/null +++ b/tests/files/no-parent-barrel-import/foo/baz.js @@ -0,0 +1 @@ +// Used in `no-parent-barrel-import` tests diff --git a/tests/files/no-parent-barrel-import/foo/index.js b/tests/files/no-parent-barrel-import/foo/index.js new file mode 100644 index 000000000..b4d754a96 --- /dev/null +++ b/tests/files/no-parent-barrel-import/foo/index.js @@ -0,0 +1 @@ +// Used in `no-parent-barrel-import` tests diff --git a/tests/files/no-parent-barrel-import/index.js b/tests/files/no-parent-barrel-import/index.js new file mode 100644 index 000000000..2a8abf44e --- /dev/null +++ b/tests/files/no-parent-barrel-import/index.js @@ -0,0 +1,2 @@ +// Used in `no-parent-barrel-import` tests + diff --git a/tests/src/rules/no-parent-barrel-import.js b/tests/src/rules/no-parent-barrel-import.js new file mode 100644 index 000000000..fa8032d58 --- /dev/null +++ b/tests/src/rules/no-parent-barrel-import.js @@ -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'), + }), + + ], +});