From ec0582e6e1dcbdf6159ec3a52e052410fc8ab1c3 Mon Sep 17 00:00:00 2001 From: Niko Sams Date: Wed, 22 Nov 2023 10:05:22 +0100 Subject: [PATCH] Add ESLint rule that enforces absolute (@src/...) imports from other modules (#1303) Main motivation was to have nicer import paths for CRUD generator code It should avoid `import xxx from "../common/xxx";` and change it to `@src/common/xxx` For other examples see packages/eslint-plugin/src/rules/no-other-module-relative-import.test.ts very basic definition of "module" (a concept that doesn't really exist): - src/modulea - src/moduleb sub-modules or nested modules are not supported by this. --- .changeset/clever-jokes-chew.md | 9 +++ demo/admin/src/pages/PageContentBlock.tsx | 4 +- demo/admin/src/pages/blocks/ColumnsBlock.tsx | 3 +- demo/admin/src/pages/blocks/TwoListsBlock.tsx | 3 +- demo/api/src/config/config.ts | 2 +- .../src/footer/blocks/footer-content.block.ts | 2 +- .../blocks/footer-link-section.block.ts | 3 +- .../admin/admin-color-picker/.eslintrc.json | 5 +- packages/admin/admin-date-time/.eslintrc.json | 5 +- packages/admin/admin-icons/.eslintrc.json | 5 +- .../admin/admin-react-select/.eslintrc.json | 5 +- packages/admin/admin-rte/.eslintrc.json | 3 +- packages/admin/admin-stories/.eslintrc.json | 3 +- packages/admin/admin-theme/.eslintrc.json | 5 +- packages/admin/admin/.eslintrc.json | 3 +- packages/admin/blocks-admin/.eslintrc.json | 5 +- packages/admin/cms-admin/.eslintrc.json | 5 +- packages/api/blocks-api/.eslintrc.json | 5 +- packages/api/cms-api/.eslintrc.json | 5 +- packages/cli/.eslintrc.json | 5 +- packages/eslint-config/core-without-import.js | 2 +- packages/eslint-plugin/src/index.ts | 3 + .../no-other-module-relative-import.test.ts | 40 +++++++++++ .../rules/no-other-module-relative-import.ts | 68 +++++++++++++++++++ packages/site/cms-site/.eslintrc.json | 3 +- 25 files changed, 176 insertions(+), 25 deletions(-) create mode 100644 .changeset/clever-jokes-chew.md create mode 100644 packages/eslint-plugin/src/rules/no-other-module-relative-import.test.ts create mode 100644 packages/eslint-plugin/src/rules/no-other-module-relative-import.ts diff --git a/.changeset/clever-jokes-chew.md b/.changeset/clever-jokes-chew.md new file mode 100644 index 0000000000..188c0fe72d --- /dev/null +++ b/.changeset/clever-jokes-chew.md @@ -0,0 +1,9 @@ +--- +"@comet/eslint-plugin": minor +--- + +Add new ESLint rule to enforce absolute imports when importing from other modules + +For instance, an import `import { AThingInModuleA } from "../moduleA/AThingInModuleA"` in module `B` needs to be imported as `import { AThingInModuleA } from "@src/moduleA/AThingInModuleA"`. +The default source root `"./src"` and alias `"@src"` can be changed via the rule's `sourceRoot` and `sourceRootAlias` options. +This rule will be enforced by `@comet/eslint-config` in the next major release. \ No newline at end of file diff --git a/demo/admin/src/pages/PageContentBlock.tsx b/demo/admin/src/pages/PageContentBlock.tsx index 8145c1cb3a..f8a1714d6b 100644 --- a/demo/admin/src/pages/PageContentBlock.tsx +++ b/demo/admin/src/pages/PageContentBlock.tsx @@ -1,14 +1,14 @@ import { createBlocksBlock, SpaceBlock, YouTubeVideoBlock } from "@comet/blocks-admin"; import { AnchorBlock, DamImageBlock, DamVideoBlock } from "@comet/cms-admin"; +import { HeadlineBlock } from "@src/common/blocks/HeadlineBlock"; import { LinkListBlock } from "@src/common/blocks/LinkListBlock"; import { RichTextBlock } from "@src/common/blocks/RichTextBlock"; +import { TextImageBlock } from "@src/common/blocks/TextImageBlock"; import { userGroupAdditionalItemFields } from "@src/userGroups/userGroupAdditionalItemFields"; import { UserGroupChip } from "@src/userGroups/UserGroupChip"; import { UserGroupContextMenuItem } from "@src/userGroups/UserGroupContextMenuItem"; import * as React from "react"; -import { HeadlineBlock } from "../common/blocks/HeadlineBlock"; -import { TextImageBlock } from "../common/blocks/TextImageBlock"; import { ColumnsBlock } from "./blocks/ColumnsBlock"; import { FullWidthImageBlock } from "./blocks/FullWidthImageBlock"; import { MediaBlock } from "./blocks/MediaBlock"; diff --git a/demo/admin/src/pages/blocks/ColumnsBlock.tsx b/demo/admin/src/pages/blocks/ColumnsBlock.tsx index 748f1d1d5d..b26d82c619 100644 --- a/demo/admin/src/pages/blocks/ColumnsBlock.tsx +++ b/demo/admin/src/pages/blocks/ColumnsBlock.tsx @@ -7,11 +7,10 @@ import { SpaceBlock, } from "@comet/blocks-admin"; import { DamImageBlock } from "@comet/cms-admin"; +import { HeadlineBlock } from "@src/common/blocks/HeadlineBlock"; import { RichTextBlock } from "@src/common/blocks/RichTextBlock"; import * as React from "react"; -import { HeadlineBlock } from "../../common/blocks/HeadlineBlock"; - const ColumnsContentBlock = createBlocksBlock({ name: "ColumnsContent", supportedBlocks: { diff --git a/demo/admin/src/pages/blocks/TwoListsBlock.tsx b/demo/admin/src/pages/blocks/TwoListsBlock.tsx index e798bbdfc3..71d6aed6b0 100644 --- a/demo/admin/src/pages/blocks/TwoListsBlock.tsx +++ b/demo/admin/src/pages/blocks/TwoListsBlock.tsx @@ -1,6 +1,5 @@ import { createCompositeBlock, createListBlock } from "@comet/blocks-admin"; - -import { HeadlineBlock } from "../../common/blocks/HeadlineBlock"; +import { HeadlineBlock } from "@src/common/blocks/HeadlineBlock"; const TwoListsListBlock = createListBlock({ name: "TwoListsList", diff --git a/demo/api/src/config/config.ts b/demo/api/src/config/config.ts index ffa979bcce..5c23f246e9 100644 --- a/demo/api/src/config/config.ts +++ b/demo/api/src/config/config.ts @@ -1,7 +1,7 @@ +import cometConfig from "@src/../comet-config.json"; import { plainToClass } from "class-transformer"; import { validateSync } from "class-validator"; -import cometConfig from "../../comet-config.json"; import { EnvironmentVariables } from "./environment-variables"; // eslint-disable-next-line @typescript-eslint/explicit-function-return-type diff --git a/demo/api/src/footer/blocks/footer-content.block.ts b/demo/api/src/footer/blocks/footer-content.block.ts index 5144262d08..b45cbb7eef 100644 --- a/demo/api/src/footer/blocks/footer-content.block.ts +++ b/demo/api/src/footer/blocks/footer-content.block.ts @@ -10,9 +10,9 @@ import { ExtractBlockInput, inputToData, } from "@comet/blocks-api"; +import { LinkListBlock } from "@src/common/blocks/link-list.block"; import { IsOptional, IsString } from "class-validator"; -import { LinkListBlock } from "../../common/blocks/link-list.block"; import { FooterLinkSectionBlock } from "./footer-link-section.block"; export const FooterTopLinksBlock = createListBlock({ block: FooterLinkSectionBlock }, "FooterTopLinks"); diff --git a/demo/api/src/footer/blocks/footer-link-section.block.ts b/demo/api/src/footer/blocks/footer-link-section.block.ts index 019ca762a7..dd30c87a66 100644 --- a/demo/api/src/footer/blocks/footer-link-section.block.ts +++ b/demo/api/src/footer/blocks/footer-link-section.block.ts @@ -9,10 +9,9 @@ import { ExtractBlockInput, inputToData, } from "@comet/blocks-api"; +import { LinkListBlock } from "@src/common/blocks/link-list.block"; import { IsOptional, IsString, ValidateNested } from "class-validator"; -import { LinkListBlock } from "../../common/blocks/link-list.block"; - class FooterLinkSectionBlockData extends BlockData { @BlockField() title?: string; diff --git a/packages/admin/admin-color-picker/.eslintrc.json b/packages/admin/admin-color-picker/.eslintrc.json index 82fc8328a8..119fe64e5f 100644 --- a/packages/admin/admin-color-picker/.eslintrc.json +++ b/packages/admin/admin-color-picker/.eslintrc.json @@ -1,4 +1,7 @@ { "extends": "@comet/eslint-config/react", - "ignorePatterns": ["src/*.generated.ts", "lib/**"] + "ignorePatterns": ["src/*.generated.ts", "lib/**"], + "rules": { + "@comet/no-other-module-relative-import": "off" + } } diff --git a/packages/admin/admin-date-time/.eslintrc.json b/packages/admin/admin-date-time/.eslintrc.json index 82fc8328a8..119fe64e5f 100644 --- a/packages/admin/admin-date-time/.eslintrc.json +++ b/packages/admin/admin-date-time/.eslintrc.json @@ -1,4 +1,7 @@ { "extends": "@comet/eslint-config/react", - "ignorePatterns": ["src/*.generated.ts", "lib/**"] + "ignorePatterns": ["src/*.generated.ts", "lib/**"], + "rules": { + "@comet/no-other-module-relative-import": "off" + } } diff --git a/packages/admin/admin-icons/.eslintrc.json b/packages/admin/admin-icons/.eslintrc.json index 939cc21d5b..92a7a16c52 100644 --- a/packages/admin/admin-icons/.eslintrc.json +++ b/packages/admin/admin-icons/.eslintrc.json @@ -1,4 +1,7 @@ { "extends": "@comet/eslint-config/react", - "ignorePatterns": ["src/*.generated.ts", "src/generated/", "lib/**"] + "ignorePatterns": ["src/*.generated.ts", "src/generated/", "lib/**"], + "rules": { + "@comet/no-other-module-relative-import": "off" + } } diff --git a/packages/admin/admin-react-select/.eslintrc.json b/packages/admin/admin-react-select/.eslintrc.json index 82fc8328a8..119fe64e5f 100644 --- a/packages/admin/admin-react-select/.eslintrc.json +++ b/packages/admin/admin-react-select/.eslintrc.json @@ -1,4 +1,7 @@ { "extends": "@comet/eslint-config/react", - "ignorePatterns": ["src/*.generated.ts", "lib/**"] + "ignorePatterns": ["src/*.generated.ts", "lib/**"], + "rules": { + "@comet/no-other-module-relative-import": "off" + } } diff --git a/packages/admin/admin-rte/.eslintrc.json b/packages/admin/admin-rte/.eslintrc.json index 933e30b001..0cf3c6408f 100644 --- a/packages/admin/admin-rte/.eslintrc.json +++ b/packages/admin/admin-rte/.eslintrc.json @@ -3,6 +3,7 @@ "ignorePatterns": ["src/*.generated.ts", "lib/**"], "rules": { "@typescript-eslint/no-explicit-any": "off", - "@typescript-eslint/no-non-null-assertion": "off" + "@typescript-eslint/no-non-null-assertion": "off", + "@comet/no-other-module-relative-import": "off" } } diff --git a/packages/admin/admin-stories/.eslintrc.json b/packages/admin/admin-stories/.eslintrc.json index b762648de5..ca43e81a38 100644 --- a/packages/admin/admin-stories/.eslintrc.json +++ b/packages/admin/admin-stories/.eslintrc.json @@ -6,6 +6,7 @@ "@typescript-eslint/no-empty-function": "off", "@typescript-eslint/no-explicit-any": "off", "@typescript-eslint/no-non-null-assertion": "off", - "no-console": "off" + "no-console": "off", + "@comet/no-other-module-relative-import": "off" } } diff --git a/packages/admin/admin-theme/.eslintrc.json b/packages/admin/admin-theme/.eslintrc.json index 82fc8328a8..119fe64e5f 100644 --- a/packages/admin/admin-theme/.eslintrc.json +++ b/packages/admin/admin-theme/.eslintrc.json @@ -1,4 +1,7 @@ { "extends": "@comet/eslint-config/react", - "ignorePatterns": ["src/*.generated.ts", "lib/**"] + "ignorePatterns": ["src/*.generated.ts", "lib/**"], + "rules": { + "@comet/no-other-module-relative-import": "off" + } } diff --git a/packages/admin/admin/.eslintrc.json b/packages/admin/admin/.eslintrc.json index 933e30b001..0cf3c6408f 100644 --- a/packages/admin/admin/.eslintrc.json +++ b/packages/admin/admin/.eslintrc.json @@ -3,6 +3,7 @@ "ignorePatterns": ["src/*.generated.ts", "lib/**"], "rules": { "@typescript-eslint/no-explicit-any": "off", - "@typescript-eslint/no-non-null-assertion": "off" + "@typescript-eslint/no-non-null-assertion": "off", + "@comet/no-other-module-relative-import": "off" } } diff --git a/packages/admin/blocks-admin/.eslintrc.json b/packages/admin/blocks-admin/.eslintrc.json index 82fc8328a8..119fe64e5f 100644 --- a/packages/admin/blocks-admin/.eslintrc.json +++ b/packages/admin/blocks-admin/.eslintrc.json @@ -1,4 +1,7 @@ { "extends": "@comet/eslint-config/react", - "ignorePatterns": ["src/*.generated.ts", "lib/**"] + "ignorePatterns": ["src/*.generated.ts", "lib/**"], + "rules": { + "@comet/no-other-module-relative-import": "off" + } } diff --git a/packages/admin/cms-admin/.eslintrc.json b/packages/admin/cms-admin/.eslintrc.json index 61c2ccf6ca..500011929a 100644 --- a/packages/admin/cms-admin/.eslintrc.json +++ b/packages/admin/cms-admin/.eslintrc.json @@ -1,4 +1,7 @@ { "extends": "@comet/eslint-config/react", - "ignorePatterns": ["src/**/*.generated.ts", "lib/**"] + "ignorePatterns": ["src/**/*.generated.ts", "lib/**"], + "rules": { + "@comet/no-other-module-relative-import": "off" + } } diff --git a/packages/api/blocks-api/.eslintrc.json b/packages/api/blocks-api/.eslintrc.json index 0a02f81918..69bb612d02 100644 --- a/packages/api/blocks-api/.eslintrc.json +++ b/packages/api/blocks-api/.eslintrc.json @@ -1,4 +1,7 @@ { "extends": "@comet/eslint-config/nestjs", - "ignorePatterns": ["lib/**"] + "ignorePatterns": ["lib/**"], + "rules": { + "@comet/no-other-module-relative-import": "off" + } } diff --git a/packages/api/cms-api/.eslintrc.json b/packages/api/cms-api/.eslintrc.json index 567b04dff7..4d7ee293e2 100644 --- a/packages/api/cms-api/.eslintrc.json +++ b/packages/api/cms-api/.eslintrc.json @@ -1,4 +1,7 @@ { "extends": "@comet/eslint-config/nestjs", - "ignorePatterns": ["src/mikro-orm/migrations/**", "lib/**"] + "ignorePatterns": ["src/mikro-orm/migrations/**", "lib/**"], + "rules": { + "@comet/no-other-module-relative-import": "off" + } } diff --git a/packages/cli/.eslintrc.json b/packages/cli/.eslintrc.json index 1b25b82d8c..d9c539c2df 100644 --- a/packages/cli/.eslintrc.json +++ b/packages/cli/.eslintrc.json @@ -1,4 +1,7 @@ { "extends": "@comet/eslint-config/core", - "ignorePatterns": ["bin/", "lib/**"] + "ignorePatterns": ["bin/", "lib/**"], + "rules": { + "@comet/no-other-module-relative-import": "off" + } } diff --git a/packages/eslint-config/core-without-import.js b/packages/eslint-config/core-without-import.js index b771015a5e..0ec71210c4 100644 --- a/packages/eslint-config/core-without-import.js +++ b/packages/eslint-config/core-without-import.js @@ -9,7 +9,7 @@ module.exports = { "unused-imports/no-unused-imports": "error", "no-console": ["error", { allow: ["warn", "error"] }], "no-return-await": "error", - "json-files/sort-package-json": "error", + "json-files/sort-package-json": "error" }, overrides: [ { diff --git a/packages/eslint-plugin/src/index.ts b/packages/eslint-plugin/src/index.ts index 8c1ecd5265..1e1d676372 100644 --- a/packages/eslint-plugin/src/index.ts +++ b/packages/eslint-plugin/src/index.ts @@ -1,7 +1,10 @@ +import noOtherModuleRelativeImport from "./rules/no-other-module-relative-import"; import noPrivateSiblingImport from "./rules/no-private-sibling-import"; + const plugin = { rules: { "no-private-sibling-import": noPrivateSiblingImport, + "no-other-module-relative-import": noOtherModuleRelativeImport, }, }; export type Plugin = typeof plugin; diff --git a/packages/eslint-plugin/src/rules/no-other-module-relative-import.test.ts b/packages/eslint-plugin/src/rules/no-other-module-relative-import.test.ts new file mode 100644 index 0000000000..569a0fff2f --- /dev/null +++ b/packages/eslint-plugin/src/rules/no-other-module-relative-import.test.ts @@ -0,0 +1,40 @@ +import { RuleTester } from "eslint"; + +import noOtherModuleRelativeImport from "./no-other-module-relative-import"; + +const ruleTester = new RuleTester({ + parser: require.resolve("@typescript-eslint/parser"), +}); + +const errors = [{ message: "Avoid relative import from other module" }]; + +const options = [{ sourceRoot: "./src", sourceRootAlias: "@src" }]; + +ruleTester.run("no-other-module-relative-import", noOtherModuleRelativeImport, { + valid: [ + { + code: `import Bar from "@src/moduleb/Bar";`, + filename: `${process.cwd()}/src/modulea/Foo.ts`, + options, + }, + { code: `import Bar from "../Bar";`, filename: `${process.cwd()}/src/modulea/sub/Foo.ts`, options }, + { code: `import Bar from "xx/bar";`, filename: `${process.cwd()}/src/modulea/Foo.ts`, options }, + ], + + invalid: [ + { + code: `import Bar from "../moduleb/Bar";`, + filename: `${process.cwd()}/src/modulea/Foo.ts`, + options, + errors, + output: `import Bar from "@src/moduleb/Bar";`, + }, + { + code: `import Bar from "../../Bar";`, + filename: `${process.cwd()}/src/modulea/sub/Foo.ts`, + options, + errors, + output: `import Bar from "@src/Bar";`, + }, + ], +}); diff --git a/packages/eslint-plugin/src/rules/no-other-module-relative-import.ts b/packages/eslint-plugin/src/rules/no-other-module-relative-import.ts new file mode 100644 index 0000000000..fad31c7766 --- /dev/null +++ b/packages/eslint-plugin/src/rules/no-other-module-relative-import.ts @@ -0,0 +1,68 @@ +import { Rule } from "eslint"; +import path from "path"; + +function parentDirCount(dir: string) { + const match = dir.match(/^(\.\.\/)*/); + return match[0].length / 3; +} + +export default { + meta: { + type: "suggestion", + fixable: "code", + schema: [ + { + type: "object", + properties: { + sourceRoot: { + type: "string", + }, + sourceRootAlias: { + type: "string", + }, + }, + additionalProperties: false, + }, + ], + }, + create(context) { + return { + ImportDeclaration: function (node) { + const options = context.options[0] ?? { sourceRoot: "./src", sourceRootAlias: "@src" }; + + const importParentDirCount = parentDirCount(node.source.value.toString()); + if (!importParentDirCount) { + // import is not relative + return; + } + + const filePath = context.getPhysicalFilename ? context.getPhysicalFilename() : context.getFilename(); + if (filePath == "") return; // If the input is from stdin, this test can't fail + const sourceDir = `${context.getCwd()}/${options.sourceRoot}`; + + const fileDir = path.dirname(filePath); + + const relativeFileToSourceDir = path.relative(sourceDir, fileDir); + if (!relativeFileToSourceDir || relativeFileToSourceDir.startsWith("..")) { + // file is not in source directory + return; + } + + const fileSubdirectoriesCount = relativeFileToSourceDir.split(path.sep).length; + + // importParentDirCount is the number of ../ parts in the import path + // fileSubdirectoriesCount is the number of subdirectories in the file path relative to the source directory + if (importParentDirCount >= fileSubdirectoriesCount) { + context.report({ + node, + message: "Avoid relative import from other module", + fix: (fixer) => { + const importPathRelativeToSourceDir = path.relative(sourceDir, `${fileDir}/${node.source.value.toString()}`); + return fixer.replaceText(node.source, `"${options.sourceRootAlias}/${importPathRelativeToSourceDir}"`); + }, + }); + } + }, + }; + }, +} as Rule.RuleModule; diff --git a/packages/site/cms-site/.eslintrc.json b/packages/site/cms-site/.eslintrc.json index 54044f4ba4..c4b57d8b2e 100644 --- a/packages/site/cms-site/.eslintrc.json +++ b/packages/site/cms-site/.eslintrc.json @@ -2,6 +2,7 @@ "extends": "@comet/eslint-config/nextjs", "ignorePatterns": ["src/*.generated.ts", "lib/**"], "rules": { - "@next/next/no-html-link-for-pages": "off" // disabled because lib has no pages dir + "@next/next/no-html-link-for-pages": "off", // disabled because lib has no pages dir + "@comet/no-other-module-relative-import": "off" } }