From c77ecdafff5dc8689720eff2d176163b43241193 Mon Sep 17 00:00:00 2001 From: Ludwig Meysel Date: Fri, 18 Oct 2024 13:38:43 +0200 Subject: [PATCH 1/4] Add rule to enforce a Relation wrapper to avoid ESM dependency issues --- src/rules/enforce-relation-wrapper.test.ts | 99 +++++++++++++++ src/rules/enforce-relation-wrapper.ts | 137 +++++++++++++++++++++ 2 files changed, 236 insertions(+) create mode 100644 src/rules/enforce-relation-wrapper.test.ts create mode 100644 src/rules/enforce-relation-wrapper.ts diff --git a/src/rules/enforce-relation-wrapper.test.ts b/src/rules/enforce-relation-wrapper.test.ts new file mode 100644 index 0000000..f565bb5 --- /dev/null +++ b/src/rules/enforce-relation-wrapper.test.ts @@ -0,0 +1,99 @@ +import path from 'path'; +import * as vitest from 'vitest'; +import tsParser from '@typescript-eslint/parser'; +import { RuleTester } from '@typescript-eslint/rule-tester'; +import rule from './enforce-relation-wrapper.js'; + +RuleTester.afterAll = vitest.afterAll; +RuleTester.it = vitest.it; +RuleTester.itOnly = vitest.it.only; +RuleTester.describe = vitest.describe; + +const ruleTester = new RuleTester({ + languageOptions: { + parser: tsParser, + parserOptions: { + project: './tsconfig.json', + tsconfigRootDir: path.join(__dirname, '../../tests'), + }, + }, +}); + +ruleTester.run('enforce-relation-wrapper', rule, { + valid: [ + { + name: 'should "see" aliased imports', + code: `import { OneToMany as OTM, Relation as Foreign } from 'typeorm' + class Foo { + @OTM(() => Bar) + prop: Foreign | undefined + }`, + }, + { + name: 'should respect current code as long as all wrappings are okay', + code: `import { OneToMany, Relation } from 'typeorm' + class Foo { + @OneToMany(() => Bar) + prop: Relation | Relation + }`, + }, + ], + invalid: [ + { + name: 'should fail due to missing Relation<...>-wrapper', + code: `import { OneToMany } from 'typeorm' + class Foo { + @OneToMany(() => Bar) + prop: Bar | undefined + }`, + errors: [{ messageId: 'expectedRelation' }, { messageId: 'preferRelation' }], + output: `import { OneToMany, Relation } from 'typeorm' + class Foo { + @OneToMany(() => Bar) + prop: Relation | undefined + }`, + }, + { + name: 'should fail due to missing Relation<...>-wrapper with aliased decorator', + code: `import { OneToMany as OTM } from 'typeorm' + class Foo { + @OTM(() => Bar) + prop: Bar | undefined + }`, + errors: [{ messageId: 'expectedRelation' }, { messageId: 'preferRelation' }], + output: `import { OneToMany as OTM, Relation } from 'typeorm' + class Foo { + @OTM(() => Bar) + prop: Relation | undefined + }`, + }, + { + name: 'should fail due to missing Relation<...>-wrapper but respects wrapper alias', + code: `import { OneToMany, Relation as Reference } from 'typeorm' + class Foo { + @OneToMany(() => Bar) + prop: Bar | undefined + }`, + errors: [{ messageId: 'preferRelation' }], + output: `import { OneToMany, Relation as Reference } from 'typeorm' + class Foo { + @OneToMany(() => Bar) + prop: Reference | undefined + }`, + }, + { + name: 'should fail due to partially missing Relation<...>-wrapper', + code: `import { OneToMany } from 'typeorm' + class Foo { + @OneToMany(() => Bar) + prop: Relation | Bar | undefined + }`, + errors: [{ messageId: 'expectedRelation' }, { messageId: 'preferRelation' }], + output: `import { OneToMany, Relation } from 'typeorm' + class Foo { + @OneToMany(() => Bar) + prop: Relation | undefined + }`, + }, + ], +}); diff --git a/src/rules/enforce-relation-wrapper.ts b/src/rules/enforce-relation-wrapper.ts new file mode 100644 index 0000000..fe77ad1 --- /dev/null +++ b/src/rules/enforce-relation-wrapper.ts @@ -0,0 +1,137 @@ +import { ESLintUtils, TSESTree } from '@typescript-eslint/utils'; + +const createRule = ESLintUtils.RuleCreator((name) => name); + +const relationDecorators = ['OneToOne', 'OneToMany', 'ManyToOne', 'ManyToMany'] as const; + +const rule = createRule({ + name: 'enforce-relation-wrapper', + meta: { + type: 'suggestion', + docs: { + description: 'Avoid statically typed relations and prefer the Relation<...>-wrapper.', + }, + messages: { + preferRelation: 'Better use Relation<...>-wrapper for relation types.', + expectedRelation: + 'When importing a relation decorator, the Relation<...>-wrapper is also expected to be imported.', + }, + schema: [], + fixable: 'code', + hasSuggestions: true, + }, + defaultOptions: [], + create(context) { + const alias = { + Relation: 'Relation', + OneToMany: 'OneToMany', + ManyToOne: 'ManyToOne', + OneToOne: 'OneToOne', + ManyToMany: 'ManyToMany', + }; + let relationAliases: string[]; + + function getWrapping( + typeNode: TSESTree.TypeNode, + wrappedTypes: TSESTree.TypeNode[], + unwrappedTypes: TSESTree.TypeNode[], + ): boolean { + let fixRequired = false; + if (typeNode.type === TSESTree.AST_NODE_TYPES.TSUnionType) + for (const type of typeNode.types) { + fixRequired = getWrapping(type, wrappedTypes, unwrappedTypes) || fixRequired; + } + else if (typeNode.type === TSESTree.AST_NODE_TYPES.TSTypeReference) { + const { typeName, typeArguments } = typeNode; + if ( + typeName.type === TSESTree.AST_NODE_TYPES.Identifier && + typeName.name === alias.Relation && + typeArguments?.params.length === 1 + ) { + // intentionally skip "fixRequired" here as types are already wrapped + for (const typeArgument of typeArguments.params) + getWrapping(typeArgument, wrappedTypes, unwrappedTypes); + } else { + wrappedTypes.push(typeNode); + fixRequired = true; + } + } else unwrappedTypes.push(typeNode); + + return fixRequired; + } + + return { + ImportDeclaration(node) { + if (node.source.value !== 'typeorm') return; + let requiresImportRelation = false, + relationImported = false; + for (const specifier of node.specifiers) { + if (specifier.type !== TSESTree.AST_NODE_TYPES.ImportSpecifier) continue; + const { imported } = specifier; + if (relationDecorators.includes(imported.name as never)) { + alias[imported.name as keyof typeof alias] = specifier.local.name; + requiresImportRelation = true; + } + if (imported.name === 'Relation') { + alias.Relation = specifier.local.name; + relationImported = true; + continue; + } + } + if (requiresImportRelation && !relationImported) + context.report({ + messageId: 'expectedRelation', + node, + fix(fixer) { + const lastSpec = node.specifiers[node.specifiers.length - 1]; + return fixer.replaceText( + lastSpec, + context.sourceCode.getText(lastSpec) + ', Relation', + ); + }, + }); + }, + PropertyDefinition(node) { + if (!relationAliases) + relationAliases = [...relationDecorators.map((_) => alias[_])]; + + for (const decorator of node.decorators) { + if (decorator.expression.type !== TSESTree.AST_NODE_TYPES.CallExpression) + continue; + + const { callee } = decorator.expression; + if (callee.type !== TSESTree.AST_NODE_TYPES.Identifier) continue; + + if (relationAliases.includes(callee.name) && node.typeAnnotation) { + const wrappedTypes: TSESTree.TypeNode[] = []; + const unwrappedTypes: TSESTree.TypeNode[] = []; + + const { typeAnnotation } = node.typeAnnotation; + + if (!getWrapping(typeAnnotation, wrappedTypes, unwrappedTypes)) return; + + const relation = wrappedTypes + .map((type) => context.sourceCode.getText(type)) + .join(' | '); + + const replacement = [ + `${alias.Relation}<${relation}>`, + ...unwrappedTypes.map((type) => context.sourceCode.getText(type)), + ].join(' | '); + + // if (type.includes('Relation<')) return; + context.report({ + messageId: 'preferRelation', + node: typeAnnotation, + fix(fixer) { + return fixer.replaceTextRange(typeAnnotation.range, replacement); + }, + }); + } + } + }, + }; + }, +}); + +export default rule; From 2e257c0ce1d8cbeb3e62144984d122de5c922960 Mon Sep 17 00:00:00 2001 From: Ludwig Meysel Date: Fri, 18 Oct 2024 13:39:28 +0200 Subject: [PATCH 2/4] Document enforce-relation-wrapper rule --- README.md | 131 ++++++++++++++++++++++++++++++++++++++---------------- 1 file changed, 93 insertions(+), 38 deletions(-) diff --git a/README.md b/README.md index 1e7e013..447d3f6 100644 --- a/README.md +++ b/README.md @@ -22,9 +22,9 @@ import tseslint from 'typescript-eslint'; import typeormTypescriptRecommended from 'eslint-plugin-typeorm-typescript/recommended'; export default tseslint.config( - eslint.configs.recommended, - ...tseslint.configs.recommended, - typeormTypescriptRecommended, + eslint.configs.recommended, + ...tseslint.configs.recommended, + typeormTypescriptRecommended, ); ``` @@ -35,18 +35,17 @@ import eslint from '@eslint/js'; import tseslint from 'typescript-eslint'; import typeormTypescriptPlugin from 'eslint-plugin-typeorm-typescript'; -export default tseslint.config( - eslint.configs.recommended, - ...tseslint.configs.recommended, - { - plugins: {'typeorm-typescript': typeormTypescriptPlugin}, +export default tseslint.config(eslint.configs.recommended, ...tseslint.configs.recommended, { + plugins: { 'typeorm-typescript': typeormTypescriptPlugin }, rules: { - "typeorm-typescript/enforce-column-types": "error", - "typeorm-typescript/enforce-relation-types": "warn", - "typeorm-typescript/enforce-consistent-nullability": ["error", { "specifyNullable": "always" }] - } - } -); + 'typeorm-typescript/enforce-column-types': 'error', + 'typeorm-typescript/enforce-relation-types': 'warn', + 'typeorm-typescript/enforce-consistent-nullability': [ + 'error', + { specifyNullable: 'always' }, + ], + }, +}); ``` For more information, see an example of the [recommended configuration](./examples/recommended/). @@ -57,12 +56,12 @@ If you are still on legacy ESLint, update `.eslintrc.json` with the plugin to th ```json { - "plugins": ["typeorm-typescript"], - "rules": { - "typeorm-typescript/enforce-column-types": "error", - "typeorm-typescript/enforce-relation-types": "error", - "typeorm-typescript/enforce-consistent-nullability": "error" - } + "plugins": ["typeorm-typescript"], + "rules": { + "typeorm-typescript/enforce-column-types": "error", + "typeorm-typescript/enforce-relation-types": "error", + "typeorm-typescript/enforce-consistent-nullability": "error" + } } ``` @@ -87,9 +86,9 @@ It also handle primary columns (`number` by default), create and update columns ```json { - "rules": { - "typeorm-typescript/enforce-column-types": "error" - } + "rules": { + "typeorm-typescript/enforce-column-types": "error" + } } ``` @@ -100,11 +99,11 @@ Examples of **incorrect code** for this rule: ```ts class Entity { // Should be string - @Column({ type: "varchar" }) + @Column({ type: 'varchar' }) name: number; // Should be string | null - @Column({ type: "varchar", nullable: true }) + @Column({ type: 'varchar', nullable: true }) name: string; // Should be Date | null @@ -118,11 +117,11 @@ Examples of **correct code** for this rule: ```ts class Entity { // TypeORM data type and TypeScript type are consistent - @Column({ type: "varchar" }) + @Column({ type: 'varchar' }) name: string; // Nullability is correct - @Column({ type: "varchar", nullable: true }) + @Column({ type: 'varchar', nullable: true }) name: string | null; } ``` @@ -137,9 +136,9 @@ which is an easy mistake to make. ```json { - "rules": { - "typeorm-typescript/enforce-relation-types": "error" - } + "rules": { + "typeorm-typescript/enforce-relation-types": "error" + } } ``` @@ -207,9 +206,15 @@ that either only the non-default value is set (no parameters or `non-default`) o "rules": { // If you want to report an error for unnecessary nullables "typeorm-typescript/enforce-consistent-nullability": "error", // or - "typeorm-typescript/enforce-consistent-nullability": ["error", { "specifyNullable": "non-default" }], + "typeorm-typescript/enforce-consistent-nullability": [ + "error", + { "specifyNullable": "non-default" }, + ], // If you want to force setting nullable everywhere to avoid confusion - "typeorm-typescript/enforce-consistent-nullability": ["error", { "specifyNullable": "always" }], + "typeorm-typescript/enforce-consistent-nullability": [ + "error", + { "specifyNullable": "always" }, + ], }, } ``` @@ -223,7 +228,7 @@ With `{ "specifyNullable": "non-default" }`: ```ts class Entity { // Columns are non-nullable by default, remove it - @Column({ type: "varchar", nullable: false }) + @Column({ type: 'varchar', nullable: false }) name: number; // Relations are nullable by default, remove it @@ -238,7 +243,7 @@ With `{ "specifyNullable": "always" }`: ```ts class Entity { // Mark this to nullable false to make it clear - @Column({ type: "varchar" }) + @Column({ type: 'varchar' }) name: number; // Mark this to nullable true to make it clear @@ -255,10 +260,10 @@ With `{ "specifyNullable": "non-default" }`: ```ts class Entity { // Nullability only defined when it is different than default - @Column({ type: "varchar", nullable: true }) + @Column({ type: 'varchar', nullable: true }) middleName: number | null; - @Column({ type: "varchar" }) + @Column({ type: 'varchar' }) name: number; @OneToOne(() => Other) @@ -272,10 +277,10 @@ With `{ "specifyNullable": "always" }`: ```ts class Entity { // Nullable is set everywhere, no default behaviour is implied - @Column({ type: "varchar", nullable: true }) + @Column({ type: 'varchar', nullable: true }) middleName: number | null; - @Column({ type: "varchar", nullable: false }) + @Column({ type: 'varchar', nullable: false }) name: number; @OneToOne(() => Other, { nullable: true }) @@ -283,3 +288,53 @@ class Entity { other: Other | null; } ``` + +### typeorm-typescript/enforce-relation-wrapper + +TypeORM offers a Relation wrapper which is required when migrating to ESM, to avoid dependency issues. See also [Relations in ESM projects](https://typeorm.io/#relations-in-esm-projects). However, esp. during migration to ESM it may happen that all relations must be updated by hand, which might be very cumbersome. This rule allows to enforce the Relation<...>-wrapper and also may fix your code to do the migration for you. + +#### Configuration + +```json +{ + "rules": { + "typeorm-typescript/enforce-relation-wrapper": "error" + } +} +``` + +#### Examples + +Examples of **incorrect code** for this rule: + +```ts +class Entity { + // Should be Relation | null + @OneToOne(() => Other) + @JoinColumn() + other: Other | null; + + // Should be Relation + @OneToMany(() => Other, (other) => other.entity) + other: Other[]; + + // Should be Relation | null + @ManyToOne(() => Other) + other: Other; +} +``` + +Examples of **correct code** for this rule: + +```ts +class Entity { + // Join is correctly nullable relation .... + @OneToOne(() => Other) + @JoinColumn() + other: Relation | null; + + // *ToMany rules are an array relation + @OneToMany(() => Other, (other) => other.entity) + others: Relation; +} +``` From eaea71d40bf61e8e05d0fe6a4fb41044142063fc Mon Sep 17 00:00:00 2001 From: Ludwig Meysel Date: Fri, 18 Oct 2024 13:47:17 +0200 Subject: [PATCH 3/4] Register enforce-relation-wrapper rule --- src/index.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/index.ts b/src/index.ts index 707a29c..a669557 100644 --- a/src/index.ts +++ b/src/index.ts @@ -2,12 +2,14 @@ import { FlatConfig } from '@typescript-eslint/utils/ts-eslint'; import enforceColumnTypes from './rules/enforce-column-types.js'; import enforceConsistentNullability from './rules/enforce-consistent-nullability.js'; import enforceRelationTypes from './rules/enforce-relation-types.js'; +import enforceRelationWrapper from './rules/enforce-relation-wrapper.js'; import recommended from './recommended.js'; export const rules = { 'enforce-column-types': enforceColumnTypes, 'enforce-consistent-nullability': enforceConsistentNullability, 'enforce-relation-types': enforceRelationTypes, + 'enforce-relation-wrapper': enforceRelationWrapper, }; export const plugin: FlatConfig.Plugin = { From 6be573f2974dc68898f9cc2b4bda38a1e55da240 Mon Sep 17 00:00:00 2001 From: Ludwig Meysel Date: Fri, 18 Oct 2024 14:25:42 +0200 Subject: [PATCH 4/4] Add support for potentially Relation<...>-wrapped types --- README.md | 7 ++++ src/rules/enforce-relation-types.test.ts | 45 ++++++++++++++++++++++++ src/rules/enforce-relation-types.ts | 21 +++++++++-- src/utils/relationType.ts | 14 +++++--- 4 files changed, 81 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 447d3f6..82c86fc 100644 --- a/README.md +++ b/README.md @@ -176,6 +176,8 @@ class Entity { Examples of **correct code** for this rule: ```ts +import { Relation } from 'typeorm'; + class Entity { // Join is correctly nullable... @OneToOne(() => Other) @@ -190,6 +192,11 @@ class Entity { // *ToMany rules are an array @OneToMany(() => Other, (other) => other.entity) others: Other[]; + + // Even accepts if you wrap the relation type with the typeorm Relation<...>-wrapper + @OneToOne(() => Other, { nullable: true }) + @JoinColumn() + other: Relation | null; } ``` diff --git a/src/rules/enforce-relation-types.test.ts b/src/rules/enforce-relation-types.test.ts index 2b322d3..5fb1b4b 100644 --- a/src/rules/enforce-relation-types.test.ts +++ b/src/rules/enforce-relation-types.test.ts @@ -141,6 +141,51 @@ ruleTester.run('enforce-relation-types', enforceRelationTypes, { others: Promise; }`, }, + { + name: 'should allow lazy relations wrapped with Relation<...> wrapper array', + code: `import { Relation } from 'typeorm'; + class Entity { + @ManyToMany(() => Other) + @JoinTable() + others: Promise[]>; + }`, + }, + { + name: 'should allow lazy relations array with Relation<...> wrapper', + code: `import { Relation } from 'typeorm'; + class Entity { + @ManyToMany(() => Other) + @JoinTable() + others: Promise>; + }`, + }, + { + name: 'should allow scalar relations wrapped with Relation<...> wrapper', + code: `import { Relation } from 'typeorm'; + class Entity { + @OneToOne(() => Other) + @JoinTable() + others: Relation | null; + }`, + }, + { + name: 'should allow nullable relations wrapped with Relation<...> wrapper covering null', + code: `import { Relation } from 'typeorm'; + class Entity { + @OneToOne(() => Other, { nullable: true }) + @JoinTable() + others: Relation; + }`, + }, + { + name: 'should allow nullable relations wrapped with Relation<...> wrapper not covering null', + code: `import { Relation } from 'typeorm'; + class Entity { + @OneToOne(() => Other, { nullable: true }) + @JoinTable() + others: Relation | null; + }`, + }, ], invalid: [ { diff --git a/src/rules/enforce-relation-types.ts b/src/rules/enforce-relation-types.ts index 697227f..ce5b4e2 100644 --- a/src/rules/enforce-relation-types.ts +++ b/src/rules/enforce-relation-types.ts @@ -1,4 +1,4 @@ -import { AST_NODE_TYPES, ESLintUtils } from '@typescript-eslint/utils'; +import { AST_NODE_TYPES, ESLintUtils, TSESTree } from '@typescript-eslint/utils'; import { ReportSuggestionArray } from '@typescript-eslint/utils/ts-eslint'; import { findEitherDecoratorArguments, @@ -53,7 +53,21 @@ const enforceColumnTypes = createRule({ schema: [], }, create(context) { + let relationWrapperAlias: string | undefined; + return { + ImportDeclaration(node) { + if (node.source.value !== 'typeorm') return; + + for (const specifier of node.specifiers) { + if (specifier.type !== TSESTree.AST_NODE_TYPES.ImportSpecifier) continue; + const { imported } = specifier; + if (imported.name === 'Relation') { + relationWrapperAlias = specifier.local.name; + return; + } + } + }, PropertyDefinition(node) { const relationArguments = findEitherDecoratorArguments(node.decorators, [ 'OneToOne', @@ -89,7 +103,10 @@ const enforceColumnTypes = createRule({ return; } const { typeAnnotation } = node.typeAnnotation; - const typescriptType = convertTypeToRelationType(typeAnnotation); + const typescriptType = convertTypeToRelationType( + typeAnnotation, + relationWrapperAlias, + ); if (!isTypesEqual(typeormType, typescriptType)) { let messageId: EnforceColumnMessages = 'typescript_typeorm_relation_mismatch'; diff --git a/src/utils/relationType.ts b/src/utils/relationType.ts index 88da850..140d38c 100644 --- a/src/utils/relationType.ts +++ b/src/utils/relationType.ts @@ -10,17 +10,23 @@ interface RelationType { export type Relation = 'OneToOne' | 'OneToMany' | 'ManyToOne' | 'ManyToMany'; -export function convertTypeToRelationType(arg: TSESTree.TypeNode): RelationType { +export function convertTypeToRelationType( + arg: TSESTree.TypeNode, + relationWrapperName?: string, +): RelationType { switch (arg.type) { case AST_NODE_TYPES.TSTypeReference: { const name = arg.typeName.type === AST_NODE_TYPES.Identifier ? arg.typeName.name : ''; const param = arg.typeArguments?.params?.[0]; if (name === 'Promise' && param) { return { - ...convertTypeToRelationType(param), + ...convertTypeToRelationType(param, relationWrapperName), isLazy: true, }; } + if (name === relationWrapperName && param) { + return convertTypeToRelationType(param, relationWrapperName); + } return { name, isArray: false, @@ -29,7 +35,7 @@ export function convertTypeToRelationType(arg: TSESTree.TypeNode): RelationType }; } case AST_NODE_TYPES.TSArrayType: { - const item = convertTypeToRelationType(arg.elementType); + const item = convertTypeToRelationType(arg.elementType, relationWrapperName); return { ...item, isArray: true }; } case AST_NODE_TYPES.TSNullKeyword: { @@ -38,7 +44,7 @@ export function convertTypeToRelationType(arg: TSESTree.TypeNode): RelationType case AST_NODE_TYPES.TSUnionType: { return arg.types.reduce( (acc, currentNode) => { - const current = convertTypeToRelationType(currentNode); + const current = convertTypeToRelationType(currentNode, relationWrapperName); return { name: acc.name || current.name, isArray: acc.isArray || current.isArray,