From 9a0c28a9257fd73380dbdfa6366b0a9d6db22e66 Mon Sep 17 00:00:00 2001 From: StyleShit Date: Mon, 12 Feb 2024 22:43:06 +0200 Subject: [PATCH 01/36] feat(eslint-plugin): [require-types-exports] add new rule Closes #7670 --- .../src/rules/require-types-exports.ts | 288 ++++ .../tests/rules/require-types-exports.test.ts | 1217 +++++++++++++++++ 2 files changed, 1505 insertions(+) create mode 100644 packages/eslint-plugin/src/rules/require-types-exports.ts create mode 100644 packages/eslint-plugin/tests/rules/require-types-exports.test.ts diff --git a/packages/eslint-plugin/src/rules/require-types-exports.ts b/packages/eslint-plugin/src/rules/require-types-exports.ts new file mode 100644 index 00000000000..a972eb70746 --- /dev/null +++ b/packages/eslint-plugin/src/rules/require-types-exports.ts @@ -0,0 +1,288 @@ +import type { TSESTree } from '@typescript-eslint/utils'; +import { AST_NODE_TYPES } from '@typescript-eslint/utils'; + +import { createRule } from '../util'; + +type MessageIds = 'requireTypeExport'; + +export default createRule<[], MessageIds>({ + name: 'require-types-exports', + meta: { + type: 'suggestion', + docs: { + recommended: 'strict', + description: + 'Require exporting types that are used in exported functions declarations', + }, + messages: { + requireTypeExport: 'Expected type "{{ name }}" to be exported', + }, + schema: [], + }, + defaultOptions: [], + create(context) { + const exportedTypes = new Set(); + const reported = new Set(); + + function visitExportedFunctionDeclaration( + node: TSESTree.ExportNamedDeclaration & { + declaration: TSESTree.FunctionDeclaration | TSESTree.TSDeclareFunction; + }, + ): void { + checkFunctionParamsTypes(node.declaration); + checkFunctionReturnType(node.declaration); + } + + function visitExportedVariableDeclaration( + node: TSESTree.ExportNamedDeclaration & { + declaration: TSESTree.VariableDeclaration; + }, + ): void { + node.declaration.declarations.forEach(declaration => { + if (declaration.init?.type === AST_NODE_TYPES.ArrowFunctionExpression) { + checkFunctionParamsTypes(declaration.init); + checkFunctionReturnType(declaration.init); + } + }); + } + + function checkFunctionParamsTypes( + node: + | TSESTree.FunctionDeclaration + | TSESTree.TSDeclareFunction + | TSESTree.ArrowFunctionExpression, + ): void { + node.params.forEach(param => { + getParamTypesNodes(param).forEach(paramTypeNode => { + const name = getTypeName(paramTypeNode); + + if (!name) { + // TODO: Report on the whole function? + return; + } + + const isExported = exportedTypes.has(name); + const isReported = reported.has(name); + + if (isExported || isReported) { + return; + } + + context.report({ + node: paramTypeNode, + messageId: 'requireTypeExport', + data: { + name, + }, + }); + + reported.add(name); + }); + }); + } + + function checkFunctionReturnType( + node: + | TSESTree.FunctionDeclaration + | TSESTree.TSDeclareFunction + | TSESTree.ArrowFunctionExpression, + ): void { + const returnTypeNode = node.returnType; + + if (!returnTypeNode) { + return; + } + + getReturnTypesNodes(returnTypeNode).forEach(returnTypeNode => { + const name = getTypeName(returnTypeNode); + + if (!name) { + return; + } + + const isExported = exportedTypes.has(name); + const isReported = reported.has(name); + + if (isExported || isReported) { + return; + } + + context.report({ + node: returnTypeNode, + messageId: 'requireTypeExport', + data: { + name, + }, + }); + + reported.add(name); + }); + } + + function getParamTypesNodes( + param: TSESTree.Parameter, + ): TSESTree.TSTypeReference[] { + // Single type + if ( + param.type === AST_NODE_TYPES.Identifier && + param.typeAnnotation?.typeAnnotation.type === + AST_NODE_TYPES.TSTypeReference + ) { + return [param.typeAnnotation.typeAnnotation]; + } + + // Union or intersection + if ( + param.type === AST_NODE_TYPES.Identifier && + (param.typeAnnotation?.typeAnnotation.type === + AST_NODE_TYPES.TSUnionType || + param.typeAnnotation?.typeAnnotation.type === + AST_NODE_TYPES.TSIntersectionType) + ) { + return param.typeAnnotation.typeAnnotation.types.filter( + type => type.type === AST_NODE_TYPES.TSTypeReference, + ) as TSESTree.TSTypeReference[]; + } + + // Tuple + if ( + param.type === AST_NODE_TYPES.ArrayPattern && + param.typeAnnotation?.typeAnnotation.type === AST_NODE_TYPES.TSTupleType + ) { + return param.typeAnnotation.typeAnnotation.elementTypes.filter( + type => type.type === AST_NODE_TYPES.TSTypeReference, + ) as TSESTree.TSTypeReference[]; + } + + // Inline object + if ( + param.type === AST_NODE_TYPES.ObjectPattern && + param.typeAnnotation?.typeAnnotation.type === + AST_NODE_TYPES.TSTypeLiteral + ) { + return param.typeAnnotation.typeAnnotation.members.reduce< + TSESTree.TSTypeReference[] + >((acc, member) => { + if ( + member.type === AST_NODE_TYPES.TSPropertySignature && + member.typeAnnotation?.typeAnnotation.type === + AST_NODE_TYPES.TSTypeReference + ) { + acc.push(member.typeAnnotation.typeAnnotation); + } + + return acc; + }, []); + } + + // Rest params + if ( + param.type === AST_NODE_TYPES.RestElement && + param.typeAnnotation?.typeAnnotation.type === + AST_NODE_TYPES.TSArrayType && + param.typeAnnotation.typeAnnotation.elementType.type === + AST_NODE_TYPES.TSTypeReference + ) { + return [param.typeAnnotation.typeAnnotation.elementType]; + } + + // Default value assignment + if ( + param.type === AST_NODE_TYPES.AssignmentPattern && + param.left.typeAnnotation?.typeAnnotation.type === + AST_NODE_TYPES.TSTypeReference + ) { + return [param.left.typeAnnotation.typeAnnotation]; + } + + return []; + } + + function getReturnTypesNodes( + typeAnnotation: TSESTree.TSTypeAnnotation, + ): TSESTree.TSTypeReference[] { + // Single type + if ( + typeAnnotation.typeAnnotation.type === AST_NODE_TYPES.TSTypeReference + ) { + return [typeAnnotation.typeAnnotation]; + } + + // Union or intersection + if ( + typeAnnotation.typeAnnotation.type === AST_NODE_TYPES.TSUnionType || + typeAnnotation.typeAnnotation.type === AST_NODE_TYPES.TSIntersectionType + ) { + return typeAnnotation.typeAnnotation.types.filter( + type => type.type === AST_NODE_TYPES.TSTypeReference, + ) as TSESTree.TSTypeReference[]; + } + + // Tuple + if (typeAnnotation.typeAnnotation.type === AST_NODE_TYPES.TSTupleType) { + return typeAnnotation.typeAnnotation.elementTypes.filter( + type => type.type === AST_NODE_TYPES.TSTypeReference, + ) as TSESTree.TSTypeReference[]; + } + + // Inline object + if (typeAnnotation.typeAnnotation.type === AST_NODE_TYPES.TSTypeLiteral) { + return typeAnnotation.typeAnnotation.members.reduce< + TSESTree.TSTypeReference[] + >((acc, member) => { + if ( + member.type === AST_NODE_TYPES.TSPropertySignature && + member.typeAnnotation?.typeAnnotation.type === + AST_NODE_TYPES.TSTypeReference + ) { + acc.push(member.typeAnnotation.typeAnnotation); + } + + return acc; + }, []); + } + + return []; + } + + function collectExportedTypes(node: TSESTree.Program): void { + node.body.forEach(statement => { + if (statement.type !== AST_NODE_TYPES.ExportNamedDeclaration) { + return; + } + + const { declaration } = statement; + + if ( + declaration?.type === AST_NODE_TYPES.TSTypeAliasDeclaration || + declaration?.type === AST_NODE_TYPES.TSInterfaceDeclaration + ) { + exportedTypes.add(declaration.id.name); + + return; + } + }); + } + + function getTypeName(typeReference: TSESTree.TSTypeReference): string { + if (typeReference.typeName.type === AST_NODE_TYPES.Identifier) { + return typeReference.typeName.name; + } + + return ''; + } + + return { + Program: collectExportedTypes, + + 'ExportNamedDeclaration[declaration.type="FunctionDeclaration"]': + visitExportedFunctionDeclaration, + + 'ExportNamedDeclaration[declaration.type="TSDeclareFunction"]': + visitExportedFunctionDeclaration, + + 'ExportNamedDeclaration[declaration.type="VariableDeclaration"]': + visitExportedVariableDeclaration, + }; + }, +}); diff --git a/packages/eslint-plugin/tests/rules/require-types-exports.test.ts b/packages/eslint-plugin/tests/rules/require-types-exports.test.ts new file mode 100644 index 00000000000..f8d0437e27f --- /dev/null +++ b/packages/eslint-plugin/tests/rules/require-types-exports.test.ts @@ -0,0 +1,1217 @@ +import { RuleTester } from '@typescript-eslint/rule-tester'; + +import rule from '../../src/rules/require-types-exports'; +import { getFixturesRootDir } from '../RuleTester'; + +const rootPath = getFixturesRootDir(); + +const ruleTester = new RuleTester({ + parser: '@typescript-eslint/parser', + parserOptions: { + tsconfigRootDir: rootPath, + project: './tsconfig.json', + }, +}); + +ruleTester.run('require-types-exports', rule, { + valid: [ + 'export function f(): void {}', + 'export const f = (): void => {};', + + 'export function f(a: number): void {}', + 'export const f = (a: number): void => {};', + + 'export function f(a: any): void {}', + 'export const f = (a: any): void => {};', + + 'export function f(a: null): void {}', + 'export const f = (a: null): void => {};', + + 'export function f(a: string | number): void {}', + 'export const f = (a: string | number): void => {};', + + 'export function f(a?: string | number): void {}', + 'export const f = (a?: string | number): void => {};', + + 'export function f(a: number): string {}', + 'export const f = (a: number): string => {};', + + 'export function f(...args: any[]): void {}', + 'export const f = (...args: any[]): void => {};', + + 'export function f(...args: unknown[]): void {}', + 'export const f = (...args: unknown[]): void => {};', + + ` + type A = number; + function f(a: A): A { + return a; + } + `, + + ` + type A = number; + const f = (a: A): A => a; + `, + + ` + type A = number; + type B = string; + function f(a: A | B): any { + return a; + } + `, + + ` + type A = number; + type B = string; + const f = (a: A | B): any => a; + `, + + ` + type A = number; + declare function f(a: A): void; + `, + + ` + type A = number; + function f({ a }: { a: A }): A {} + `, + + ` + type A = number; + const f = ({ a }: { a: A }): A => {}; + `, + + ` + type A = number; + type B = string; + function f([a, b]: [A, B]): void {} + `, + + ` + type A = number; + type B = string; + const f = ([a, b]: [A, B]): void => {}; + `, + + ` + type A = number; + function f(a: A): void {} + `, + + ` + type A = number; + const f = (a: A): void => {}; + `, + + ` + interface A { + a: number; + } + + function f(a: A): A { + return a; + } + `, + + ` + interface A { + a: number; + } + + const f = (a: A): A => a; + `, + + ` + export type A = number; + export function f(a: A): void {} + `, + + ` + export type A = number; + export const f = (a: A): void => {}; + `, + + ` + export type A = number; + export type B = string; + export function f(a: A | B): void {} + `, + + ` + export type A = number; + export type B = string; + export const f = (a: A | B): void => {}; + `, + + ` + export type A = number; + export type B = string; + export function f(a: A & B): void {} + `, + + ` + export type A = number; + export type B = string; + export const f = (a: A & B): void => {}; + `, + + ` + export type A = number; + export function f(...args: A[]): void {} + `, + + ` + export type A = number; + export const f = (...args: A[]): void => {}; + `, + + ` + export type A = number; + export type B = string; + export function f(args: { a: A, b: B, c: number }): void {} + `, + + ` + export type A = number; + export type B = string; + export const f = (args: { a: A, b: B, c: number }): void => {}; + `, + + ` + export type A = number; + export type B = string; + export function f(args: [A, B]): void {} + `, + + ` + export type A = number; + export type B = string; + export const f = (args: [A, B]): void => {}; + `, + + ` + export type A = number; + export function f(a: A = 1): void {} + `, + + ` + export type A = number; + export const f = (a: A = 1): void => {}; + `, + + ` + export type A = number; + export function f(): A {} + `, + + ` + export type A = number; + export const f = (): A => {}; + `, + + ` + export type A = number; + export type B = string; + export function f(): A | B {} + `, + + ` + export type A = number; + export type B = string; + export const f = (): A | B => {}; + `, + + ` + export type A = number; + export type B = string; + export function f(): A & B {} + `, + + ` + export type A = number; + export type B = string; + export const f = (): A & B => {}; + `, + + ` + export type A = number; + export type B = string; + export function f(): [A, B] {} + `, + + ` + export type A = number; + export type B = string; + export const f = (): [A, B] => {}; + `, + + ` + export type A = number; + export type B = string; + export function f(): { a: A, b: B } {} + `, + + ` + export type A = number; + export type B = string; + export const f = (): { a: A, b: B } => {}; + `, + ], + + invalid: [ + { + code: ` + type Arg = number; + + export function f(a: Arg): void {} + `, + errors: [ + { + messageId: 'requireTypeExport', + line: 4, + column: 30, + endColumn: 33, + data: { + name: 'Arg', + }, + }, + ], + }, + + { + code: ` + type Arg = number; + + export const f = (a: Arg): void => {}; + `, + errors: [ + { + messageId: 'requireTypeExport', + line: 4, + column: 30, + endColumn: 33, + data: { + name: 'Arg', + }, + }, + ], + }, + + { + code: ` + type Arg = number; + + export function f(a: Arg, b: Arg): void {} + `, + errors: [ + { + messageId: 'requireTypeExport', + line: 4, + column: 30, + endColumn: 33, + data: { + name: 'Arg', + }, + }, + ], + }, + + { + code: ` + type Arg = number; + + export const f = (a: Arg, b: Arg): void => {}; + `, + errors: [ + { + messageId: 'requireTypeExport', + line: 4, + column: 30, + endColumn: 33, + data: { + name: 'Arg', + }, + }, + ], + }, + + { + code: ` + type Arg1 = number; + type Arg2 = string; + + export function f(a: Arg1, b: Arg2): void {} + `, + errors: [ + { + messageId: 'requireTypeExport', + line: 5, + column: 30, + endColumn: 34, + data: { + name: 'Arg1', + }, + }, + { + messageId: 'requireTypeExport', + line: 5, + column: 39, + endColumn: 43, + data: { + name: 'Arg2', + }, + }, + ], + }, + + { + code: ` + type Arg1 = number; + type Arg2 = string; + + export const f = (a: Arg1, b: Arg2): void => {}; + `, + errors: [ + { + messageId: 'requireTypeExport', + line: 5, + column: 30, + endColumn: 34, + data: { + name: 'Arg1', + }, + }, + { + messageId: 'requireTypeExport', + line: 5, + column: 39, + endColumn: 43, + data: { + name: 'Arg2', + }, + }, + ], + }, + + { + code: ` + type Arg1 = number; + + interface Arg2 { + a: string; + } + + export function f(a: Arg1, b: Arg2): void {} + `, + errors: [ + { + messageId: 'requireTypeExport', + line: 8, + column: 30, + endColumn: 34, + data: { + name: 'Arg1', + }, + }, + { + messageId: 'requireTypeExport', + line: 8, + column: 39, + endColumn: 43, + data: { + name: 'Arg2', + }, + }, + ], + }, + + { + code: ` + type Arg1 = number; + + interface Arg2 { + a: string; + } + + export const f = (a: Arg1, b: Arg2): void => {}; + `, + errors: [ + { + messageId: 'requireTypeExport', + line: 8, + column: 30, + endColumn: 34, + data: { + name: 'Arg1', + }, + }, + { + messageId: 'requireTypeExport', + line: 8, + column: 39, + endColumn: 43, + data: { + name: 'Arg2', + }, + }, + ], + }, + + { + code: ` + type Arg1 = number; + type Arg2 = string; + + export function f(a: Arg1 | Arg2): void {} + `, + errors: [ + { + messageId: 'requireTypeExport', + line: 5, + column: 30, + endColumn: 34, + data: { + name: 'Arg1', + }, + }, + { + messageId: 'requireTypeExport', + line: 5, + column: 37, + endColumn: 41, + data: { + name: 'Arg2', + }, + }, + ], + }, + + { + code: ` + type Arg1 = number; + type Arg2 = string; + + export const f = (a: Arg1 | Arg2): void => {}; + `, + errors: [ + { + messageId: 'requireTypeExport', + line: 5, + column: 30, + endColumn: 34, + data: { + name: 'Arg1', + }, + }, + { + messageId: 'requireTypeExport', + line: 5, + column: 37, + endColumn: 41, + data: { + name: 'Arg2', + }, + }, + ], + }, + + { + code: ` + type Arg1 = number; + type Arg2 = string; + + export function f(a: Arg1 & Arg2): void {} + `, + errors: [ + { + messageId: 'requireTypeExport', + line: 5, + column: 30, + endColumn: 34, + data: { + name: 'Arg1', + }, + }, + { + messageId: 'requireTypeExport', + line: 5, + column: 37, + endColumn: 41, + data: { + name: 'Arg2', + }, + }, + ], + }, + + { + code: ` + type Arg1 = number; + type Arg2 = string; + + export const f = (a: Arg1 & Arg2): void => {}; + `, + errors: [ + { + messageId: 'requireTypeExport', + line: 5, + column: 30, + endColumn: 34, + data: { + name: 'Arg1', + }, + }, + { + messageId: 'requireTypeExport', + line: 5, + column: 37, + endColumn: 41, + data: { + name: 'Arg2', + }, + }, + ], + }, + + { + code: ` + type Arg1 = number; + type Arg2 = string; + + export function f([a, b]: [Arg1, Arg2, number]): void {} + `, + errors: [ + { + messageId: 'requireTypeExport', + line: 5, + column: 36, + endColumn: 40, + data: { + name: 'Arg1', + }, + }, + { + messageId: 'requireTypeExport', + line: 5, + column: 42, + endColumn: 46, + data: { + name: 'Arg2', + }, + }, + ], + }, + + { + code: ` + type Arg1 = number; + type Arg2 = string; + type Arg3 = boolean; + + export function f([a, b]: [Arg1, Arg2, number], c: Arg3): void {} + `, + errors: [ + { + messageId: 'requireTypeExport', + line: 6, + column: 36, + endColumn: 40, + data: { + name: 'Arg1', + }, + }, + { + messageId: 'requireTypeExport', + line: 6, + column: 42, + endColumn: 46, + data: { + name: 'Arg2', + }, + }, + { + messageId: 'requireTypeExport', + line: 6, + column: 60, + endColumn: 64, + data: { + name: 'Arg3', + }, + }, + ], + }, + + { + code: ` + type Arg1 = number; + type Arg2 = string; + + export const f = ([a, b]: [Arg1, Arg2, number]): void => {}; + `, + errors: [ + { + messageId: 'requireTypeExport', + line: 5, + column: 36, + endColumn: 40, + data: { + name: 'Arg1', + }, + }, + { + messageId: 'requireTypeExport', + line: 5, + column: 42, + endColumn: 46, + data: { + name: 'Arg2', + }, + }, + ], + }, + + { + code: ` + type Arg1 = number; + type Arg2 = string; + + export function f({ a, b }: { a: Arg1; b: Arg2; c: number }): void {} + `, + errors: [ + { + messageId: 'requireTypeExport', + line: 5, + column: 42, + endColumn: 46, + data: { + name: 'Arg1', + }, + }, + { + messageId: 'requireTypeExport', + line: 5, + column: 51, + endColumn: 55, + data: { + name: 'Arg2', + }, + }, + ], + }, + + { + code: ` + type Arg1 = number; + type Arg2 = string; + + export const f = ({ a, b }: { a: Arg1; b: Arg2; c: number }): void => {}; + `, + errors: [ + { + messageId: 'requireTypeExport', + line: 5, + column: 42, + endColumn: 46, + data: { + name: 'Arg1', + }, + }, + { + messageId: 'requireTypeExport', + line: 5, + column: 51, + endColumn: 55, + data: { + name: 'Arg2', + }, + }, + ], + }, + + { + code: ` + type Arg = number; + + export function f(...args: Arg[]): void {} + `, + errors: [ + { + messageId: 'requireTypeExport', + line: 4, + column: 36, + endColumn: 39, + data: { + name: 'Arg', + }, + }, + ], + }, + + { + code: ` + type Arg = number; + + export const f = (...args: Arg[]): void => {}; + `, + errors: [ + { + messageId: 'requireTypeExport', + line: 4, + column: 36, + endColumn: 39, + data: { + name: 'Arg', + }, + }, + ], + }, + + { + code: ` + type Arg = number; + + export function f(a: Arg = 1): void {} + `, + errors: [ + { + messageId: 'requireTypeExport', + line: 4, + column: 30, + endColumn: 33, + data: { + name: 'Arg', + }, + }, + ], + }, + + { + code: ` + type Arg = number; + + export const f = (a: Arg = 1): void => {}; + `, + errors: [ + { + messageId: 'requireTypeExport', + line: 4, + column: 30, + endColumn: 33, + data: { + name: 'Arg', + }, + }, + ], + }, + + // TODO: Find a resaonable way to handle this case + { + code: ` + type Arg = number; + + export function f(a: T): void {} + `, + errors: [ + { + messageId: 'requireTypeExport', + line: 4, + column: 37, + endColumn: 39, + data: { + name: 'Arg', + }, + }, + ], + }, + + { + code: ` + type Arg = number; + + export const f = (a: T): void => {}; + `, + errors: [ + { + messageId: 'requireTypeExport', + line: 4, + column: 37, + endColumn: 39, + data: { + name: 'Arg', + }, + }, + ], + }, + + { + code: ` + type Ret = string; + + export function f(): Ret {} + `, + errors: [ + { + messageId: 'requireTypeExport', + line: 4, + column: 30, + endColumn: 33, + data: { + name: 'Ret', + }, + }, + ], + }, + + { + code: ` + type Ret = string; + + export const f = (): Ret => {}; + `, + errors: [ + { + messageId: 'requireTypeExport', + line: 4, + column: 30, + endColumn: 33, + data: { + name: 'Ret', + }, + }, + ], + }, + + { + code: ` + type Ret1 = string; + type Ret2 = number; + + export function f(): Ret1 | Ret2 {} + `, + errors: [ + { + messageId: 'requireTypeExport', + line: 5, + column: 30, + endColumn: 34, + data: { + name: 'Ret1', + }, + }, + { + messageId: 'requireTypeExport', + line: 5, + column: 37, + endColumn: 41, + data: { + name: 'Ret2', + }, + }, + ], + }, + + { + code: ` + type Ret1 = string; + type Ret2 = number; + + export const f = (): Ret1 | Ret2 => {}; + `, + errors: [ + { + messageId: 'requireTypeExport', + line: 5, + column: 30, + endColumn: 34, + data: { + name: 'Ret1', + }, + }, + { + messageId: 'requireTypeExport', + line: 5, + column: 37, + endColumn: 41, + data: { + name: 'Ret2', + }, + }, + ], + }, + + { + code: ` + type Ret1 = string; + type Ret2 = number; + + export function f(): Ret1 & Ret2 {} + `, + errors: [ + { + messageId: 'requireTypeExport', + line: 5, + column: 30, + endColumn: 34, + data: { + name: 'Ret1', + }, + }, + { + messageId: 'requireTypeExport', + line: 5, + column: 37, + endColumn: 41, + data: { + name: 'Ret2', + }, + }, + ], + }, + + { + code: ` + type Ret1 = string; + type Ret2 = number; + + export const f = (): Ret1 & Ret2 => {}; + `, + errors: [ + { + messageId: 'requireTypeExport', + line: 5, + column: 30, + endColumn: 34, + data: { + name: 'Ret1', + }, + }, + { + messageId: 'requireTypeExport', + line: 5, + column: 37, + endColumn: 41, + data: { + name: 'Ret2', + }, + }, + ], + }, + + { + code: ` + type Ret1 = string; + type Ret2 = number; + + export function f(): [Ret1, Ret2, number, Ret1] {} + `, + errors: [ + { + messageId: 'requireTypeExport', + line: 5, + column: 31, + endColumn: 35, + data: { + name: 'Ret1', + }, + }, + { + messageId: 'requireTypeExport', + line: 5, + column: 37, + endColumn: 41, + data: { + name: 'Ret2', + }, + }, + ], + }, + + { + code: ` + type Ret1 = string; + type Ret2 = number; + + export const f = (): [Ret1, Ret2, number, Ret1] => {}; + `, + errors: [ + { + messageId: 'requireTypeExport', + line: 5, + column: 31, + endColumn: 35, + data: { + name: 'Ret1', + }, + }, + { + messageId: 'requireTypeExport', + line: 5, + column: 37, + endColumn: 41, + data: { + name: 'Ret2', + }, + }, + ], + }, + + { + code: ` + type Ret1 = string; + type Ret2 = number; + + export function f(): { a: Ret1; b: Ret2; c: number; d: Ret1 } {} + `, + errors: [ + { + messageId: 'requireTypeExport', + line: 5, + column: 35, + endColumn: 39, + data: { + name: 'Ret1', + }, + }, + { + messageId: 'requireTypeExport', + line: 5, + column: 44, + endColumn: 48, + data: { + name: 'Ret2', + }, + }, + ], + }, + + { + code: ` + type Ret1 = string; + type Ret2 = number; + + export const f = (): { a: Ret1; b: Ret2; c: number; d: Ret1 } => {}; + `, + errors: [ + { + messageId: 'requireTypeExport', + line: 5, + column: 35, + endColumn: 39, + data: { + name: 'Ret1', + }, + }, + { + messageId: 'requireTypeExport', + line: 5, + column: 44, + endColumn: 48, + data: { + name: 'Ret2', + }, + }, + ], + }, + + { + code: ` + type Arg = number; + + export declare function f(a: Arg): void; + `, + errors: [ + { + messageId: 'requireTypeExport', + line: 4, + column: 38, + endColumn: 41, + data: { + name: 'Arg', + }, + }, + ], + }, + + { + code: ` + type Arg = number; + + export declare function f(a: Arg): Arg; + `, + errors: [ + { + messageId: 'requireTypeExport', + line: 4, + column: 38, + endColumn: 41, + data: { + name: 'Arg', + }, + }, + ], + }, + + { + code: ` + type Arg1 = number; + type Arg2 = string; + + export declare function f(a: Arg1): true; + export declare function f(a: Arg2): false; + export declare function f(a: Arg1 | Arg2): boolean; + `, + errors: [ + { + messageId: 'requireTypeExport', + line: 5, + column: 38, + endColumn: 42, + data: { + name: 'Arg1', + }, + }, + { + messageId: 'requireTypeExport', + line: 6, + column: 38, + endColumn: 42, + data: { + name: 'Arg2', + }, + }, + ], + }, + + { + code: ` + type Arg1 = number; + type Arg2 = string; + + export const f1 = (a: Arg1): void => {}, + f2 = (a: Arg2): void => {}; + `, + errors: [ + { + messageId: 'requireTypeExport', + line: 5, + column: 31, + endColumn: 35, + data: { + name: 'Arg1', + }, + }, + { + messageId: 'requireTypeExport', + line: 6, + column: 20, + endColumn: 24, + data: { + name: 'Arg2', + }, + }, + ], + }, + ], +}); From 7778868a552bfa6a692c6f2a00d6440a957853ec Mon Sep 17 00:00:00 2001 From: StyleShit Date: Mon, 12 Feb 2024 22:53:35 +0200 Subject: [PATCH 02/36] wip --- packages/eslint-plugin/src/rules/require-types-exports.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/eslint-plugin/src/rules/require-types-exports.ts b/packages/eslint-plugin/src/rules/require-types-exports.ts index a972eb70746..19df3d44c79 100644 --- a/packages/eslint-plugin/src/rules/require-types-exports.ts +++ b/packages/eslint-plugin/src/rules/require-types-exports.ts @@ -57,7 +57,7 @@ export default createRule<[], MessageIds>({ const name = getTypeName(paramTypeNode); if (!name) { - // TODO: Report on the whole function? + // TODO: Report on the whole function? Is this case even possible? return; } @@ -93,10 +93,11 @@ export default createRule<[], MessageIds>({ return; } - getReturnTypesNodes(returnTypeNode).forEach(returnTypeNode => { + getReturnTypeTypesNodes(returnTypeNode).forEach(returnTypeNode => { const name = getTypeName(returnTypeNode); if (!name) { + // TODO: Report on the whole function? Is this case even possi return; } @@ -198,7 +199,7 @@ export default createRule<[], MessageIds>({ return []; } - function getReturnTypesNodes( + function getReturnTypeTypesNodes( typeAnnotation: TSESTree.TSTypeAnnotation, ): TSESTree.TSTypeReference[] { // Single type From 12fce5b71b7de1cdab294cc48647b399b4ff7464 Mon Sep 17 00:00:00 2001 From: StyleShit Date: Mon, 12 Feb 2024 22:55:53 +0200 Subject: [PATCH 03/36] wip --- .../src/rules/require-types-exports.ts | 38 +++++++++---------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/packages/eslint-plugin/src/rules/require-types-exports.ts b/packages/eslint-plugin/src/rules/require-types-exports.ts index 19df3d44c79..78eb9e1eedc 100644 --- a/packages/eslint-plugin/src/rules/require-types-exports.ts +++ b/packages/eslint-plugin/src/rules/require-types-exports.ts @@ -24,6 +24,25 @@ export default createRule<[], MessageIds>({ const exportedTypes = new Set(); const reported = new Set(); + function collectExportedTypes(program: TSESTree.Program): void { + program.body.forEach(statement => { + if (statement.type !== AST_NODE_TYPES.ExportNamedDeclaration) { + return; + } + + const { declaration } = statement; + + if ( + declaration?.type === AST_NODE_TYPES.TSTypeAliasDeclaration || + declaration?.type === AST_NODE_TYPES.TSInterfaceDeclaration + ) { + exportedTypes.add(declaration.id.name); + + return; + } + }); + } + function visitExportedFunctionDeclaration( node: TSESTree.ExportNamedDeclaration & { declaration: TSESTree.FunctionDeclaration | TSESTree.TSDeclareFunction; @@ -246,25 +265,6 @@ export default createRule<[], MessageIds>({ return []; } - function collectExportedTypes(node: TSESTree.Program): void { - node.body.forEach(statement => { - if (statement.type !== AST_NODE_TYPES.ExportNamedDeclaration) { - return; - } - - const { declaration } = statement; - - if ( - declaration?.type === AST_NODE_TYPES.TSTypeAliasDeclaration || - declaration?.type === AST_NODE_TYPES.TSInterfaceDeclaration - ) { - exportedTypes.add(declaration.id.name); - - return; - } - }); - } - function getTypeName(typeReference: TSESTree.TSTypeReference): string { if (typeReference.typeName.type === AST_NODE_TYPES.Identifier) { return typeReference.typeName.name; From d62f86c020f208567c6417b02bd941cabcce5e74 Mon Sep 17 00:00:00 2001 From: StyleShit Date: Tue, 13 Feb 2024 18:55:43 +0200 Subject: [PATCH 04/36] lint --- packages/eslint-plugin/src/rules/require-types-exports.ts | 2 +- .../tests/rules/require-types-exports.test.ts | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/eslint-plugin/src/rules/require-types-exports.ts b/packages/eslint-plugin/src/rules/require-types-exports.ts index 78eb9e1eedc..0c0d6ec7026 100644 --- a/packages/eslint-plugin/src/rules/require-types-exports.ts +++ b/packages/eslint-plugin/src/rules/require-types-exports.ts @@ -116,7 +116,7 @@ export default createRule<[], MessageIds>({ const name = getTypeName(returnTypeNode); if (!name) { - // TODO: Report on the whole function? Is this case even possi + // TODO: Report on the whole function? Is this case even possible? return; } diff --git a/packages/eslint-plugin/tests/rules/require-types-exports.test.ts b/packages/eslint-plugin/tests/rules/require-types-exports.test.ts index f8d0437e27f..51f1bcfc82b 100644 --- a/packages/eslint-plugin/tests/rules/require-types-exports.test.ts +++ b/packages/eslint-plugin/tests/rules/require-types-exports.test.ts @@ -170,13 +170,13 @@ ruleTester.run('require-types-exports', rule, { ` export type A = number; export type B = string; - export function f(args: { a: A, b: B, c: number }): void {} + export function f(args: { a: A; b: B; c: number }): void {} `, ` export type A = number; export type B = string; - export const f = (args: { a: A, b: B, c: number }): void => {}; + export const f = (args: { a: A; b: B; c: number }): void => {}; `, ` @@ -250,13 +250,13 @@ ruleTester.run('require-types-exports', rule, { ` export type A = number; export type B = string; - export function f(): { a: A, b: B } {} + export function f(): { a: A; b: B } {} `, ` export type A = number; export type B = string; - export const f = (): { a: A, b: B } => {}; + export const f = (): { a: A; b: B } => {}; `, ], From 0ebebd2919345c86b2244b31069b49583ffb7933 Mon Sep 17 00:00:00 2001 From: StyleShit Date: Tue, 13 Feb 2024 19:11:33 +0200 Subject: [PATCH 05/36] wip --- packages/eslint-plugin/src/configs/all.ts | 1 + packages/eslint-plugin/src/configs/disable-type-checked.ts | 1 + packages/eslint-plugin/src/configs/strict-type-checked.ts | 1 + packages/eslint-plugin/src/rules/index.ts | 2 ++ packages/eslint-plugin/src/rules/require-types-exports.ts | 1 + 5 files changed, 6 insertions(+) diff --git a/packages/eslint-plugin/src/configs/all.ts b/packages/eslint-plugin/src/configs/all.ts index b1890165c7a..f29feca16e3 100644 --- a/packages/eslint-plugin/src/configs/all.ts +++ b/packages/eslint-plugin/src/configs/all.ts @@ -142,6 +142,7 @@ export = { '@typescript-eslint/require-array-sort-compare': 'error', 'require-await': 'off', '@typescript-eslint/require-await': 'error', + '@typescript-eslint/require-types-exports': 'error', '@typescript-eslint/restrict-plus-operands': 'error', '@typescript-eslint/restrict-template-expressions': 'error', 'no-return-await': 'off', diff --git a/packages/eslint-plugin/src/configs/disable-type-checked.ts b/packages/eslint-plugin/src/configs/disable-type-checked.ts index 09a5c07fd3e..a0641c0c49f 100644 --- a/packages/eslint-plugin/src/configs/disable-type-checked.ts +++ b/packages/eslint-plugin/src/configs/disable-type-checked.ts @@ -55,6 +55,7 @@ export = { '@typescript-eslint/promise-function-async': 'off', '@typescript-eslint/require-array-sort-compare': 'off', '@typescript-eslint/require-await': 'off', + '@typescript-eslint/require-types-exports': 'off', '@typescript-eslint/restrict-plus-operands': 'off', '@typescript-eslint/restrict-template-expressions': 'off', '@typescript-eslint/return-await': 'off', diff --git a/packages/eslint-plugin/src/configs/strict-type-checked.ts b/packages/eslint-plugin/src/configs/strict-type-checked.ts index 5666c64035d..7bcf3f5920c 100644 --- a/packages/eslint-plugin/src/configs/strict-type-checked.ts +++ b/packages/eslint-plugin/src/configs/strict-type-checked.ts @@ -71,6 +71,7 @@ export = { '@typescript-eslint/prefer-ts-expect-error': 'error', 'require-await': 'off', '@typescript-eslint/require-await': 'error', + '@typescript-eslint/require-types-exports': 'error', '@typescript-eslint/restrict-plus-operands': 'error', '@typescript-eslint/restrict-template-expressions': 'error', '@typescript-eslint/triple-slash-reference': 'error', diff --git a/packages/eslint-plugin/src/rules/index.ts b/packages/eslint-plugin/src/rules/index.ts index e497019debe..5dc2b88b71c 100644 --- a/packages/eslint-plugin/src/rules/index.ts +++ b/packages/eslint-plugin/src/rules/index.ts @@ -125,6 +125,7 @@ import promiseFunctionAsync from './promise-function-async'; import quotes from './quotes'; import requireArraySortCompare from './require-array-sort-compare'; import requireAwait from './require-await'; +import requireTypesExports from './require-types-exports'; import restrictPlusOperands from './restrict-plus-operands'; import restrictTemplateExpressions from './restrict-template-expressions'; import returnAwait from './return-await'; @@ -267,6 +268,7 @@ export default { quotes: quotes, 'require-array-sort-compare': requireArraySortCompare, 'require-await': requireAwait, + 'require-types-exports': requireTypesExports, 'restrict-plus-operands': restrictPlusOperands, 'restrict-template-expressions': restrictTemplateExpressions, 'return-await': returnAwait, diff --git a/packages/eslint-plugin/src/rules/require-types-exports.ts b/packages/eslint-plugin/src/rules/require-types-exports.ts index 0c0d6ec7026..32f178dd95a 100644 --- a/packages/eslint-plugin/src/rules/require-types-exports.ts +++ b/packages/eslint-plugin/src/rules/require-types-exports.ts @@ -11,6 +11,7 @@ export default createRule<[], MessageIds>({ type: 'suggestion', docs: { recommended: 'strict', + requiresTypeChecking: true, description: 'Require exporting types that are used in exported functions declarations', }, From bfee79153753777b133c5b0fe583783b60016fe4 Mon Sep 17 00:00:00 2001 From: StyleShit Date: Tue, 13 Feb 2024 19:17:25 +0200 Subject: [PATCH 06/36] spelling... --- .../eslint-plugin/tests/rules/require-types-exports.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/eslint-plugin/tests/rules/require-types-exports.test.ts b/packages/eslint-plugin/tests/rules/require-types-exports.test.ts index 51f1bcfc82b..d78a12b004d 100644 --- a/packages/eslint-plugin/tests/rules/require-types-exports.test.ts +++ b/packages/eslint-plugin/tests/rules/require-types-exports.test.ts @@ -806,7 +806,7 @@ ruleTester.run('require-types-exports', rule, { ], }, - // TODO: Find a resaonable way to handle this case + // TODO: Find a reasonable way to handle this case { code: ` type Arg = number; From b309b51e1ae3246441e31e77724a5046f850f2d7 Mon Sep 17 00:00:00 2001 From: StyleShit Date: Tue, 13 Feb 2024 20:24:39 +0200 Subject: [PATCH 07/36] wip --- .../docs/rules/require-types-exports.md | 75 ++++++++++ .../src/rules/require-types-exports.ts | 59 +++++++- .../tests/rules/require-types-exports.test.ts | 129 ++++++++++++++++++ 3 files changed, 258 insertions(+), 5 deletions(-) create mode 100644 packages/eslint-plugin/docs/rules/require-types-exports.md diff --git a/packages/eslint-plugin/docs/rules/require-types-exports.md b/packages/eslint-plugin/docs/rules/require-types-exports.md new file mode 100644 index 00000000000..767050deeba --- /dev/null +++ b/packages/eslint-plugin/docs/rules/require-types-exports.md @@ -0,0 +1,75 @@ +--- +description: 'Require exporting types that are used in exported functions declarations.' +--- + +> 🛑 This file is source code, not the primary documentation location! 🛑 +> +> See **https://typescript-eslint.io/rules/require-types-exports** for documentation. + +When exporting functions from a module, it is recommended to export also all the +types that are used in the function declarations. This is useful for consumers of +the module, as it allows them to use the types in their own code without having to +use things like [`Parameters`](https://www.typescriptlang.org/docs/handbook/utility-types.html#parameterstype) +or [`ReturnType`](https://www.typescriptlang.org/docs/handbook/utility-types.html#returntypetype) to extract the types from the function. + +## Examples + + + +### ❌ Incorrect + +```ts +type Arg = string; +type Result = number; + +export function strLength(arg: Arg): Result { + return arg.length; +} + +interface Fruit { + name: string; + color: string; +} + +export const getFruitName = (fruit: Fruit) => fruit.name; + +enum Color { + Red = 'red', + Green = 'green', + Blue = 'blue', +} + +export declare function getRandomColor(): Color; +``` + +### ✅ Correct + +```ts +export type Arg = string; +export type Result = number; + +export function strLength(arg: Arg): Result { + return arg.length; +} + +export interface Fruit { + name: string; + color: string; +} + +export const getFruitName = (fruit: Fruit) => fruit.name; + +export enum Color { + Red = 'red', + Green = 'green', + Blue = 'blue', +} + +export declare function getRandomColor(): Color; +``` + + + +## When Not To Use It + +When you don't want to enforce exporting types that are used in exported functions declarations. diff --git a/packages/eslint-plugin/src/rules/require-types-exports.ts b/packages/eslint-plugin/src/rules/require-types-exports.ts index 32f178dd95a..9a65af9ba39 100644 --- a/packages/eslint-plugin/src/rules/require-types-exports.ts +++ b/packages/eslint-plugin/src/rules/require-types-exports.ts @@ -1,7 +1,8 @@ -import type { TSESTree } from '@typescript-eslint/utils'; +import { TSESTree } from '@typescript-eslint/utils'; import { AST_NODE_TYPES } from '@typescript-eslint/utils'; -import { createRule } from '../util'; +import { createRule, getParserServices } from '../util'; +import { DefinitionType } from '@typescript-eslint/scope-manager'; type MessageIds = 'requireTypeExport'; @@ -45,7 +46,10 @@ export default createRule<[], MessageIds>({ } function visitExportedFunctionDeclaration( - node: TSESTree.ExportNamedDeclaration & { + node: ( + | TSESTree.ExportNamedDeclaration + | TSESTree.DefaultExportDeclarations + ) & { declaration: TSESTree.FunctionDeclaration | TSESTree.TSDeclareFunction; }, ): void { @@ -66,11 +70,46 @@ export default createRule<[], MessageIds>({ }); } + function visitDefaultExportedArrowFunction( + node: TSESTree.ExportDefaultDeclaration & { + declaration: TSESTree.ArrowFunctionExpression; + }, + ): void { + checkFunctionParamsTypes(node.declaration); + checkFunctionReturnType(node.declaration); + } + + function visitDefaultExportedIdentifier( + node: TSESTree.DefaultExportDeclarations & { + declaration: TSESTree.Identifier; + }, + ) { + const scope = context.sourceCode.getScope(node); + const variable = scope.set.get(node.declaration.name); + + if (!variable) { + return; + } + + for (const definition of variable.defs) { + if ( + definition.type === DefinitionType.Variable && + (definition.node.init?.type === + AST_NODE_TYPES.ArrowFunctionExpression || + definition.node.init?.type === AST_NODE_TYPES.FunctionExpression) + ) { + checkFunctionParamsTypes(definition.node.init); + checkFunctionReturnType(definition.node.init); + } + } + } + function checkFunctionParamsTypes( node: | TSESTree.FunctionDeclaration | TSESTree.TSDeclareFunction - | TSESTree.ArrowFunctionExpression, + | TSESTree.ArrowFunctionExpression + | TSESTree.FunctionExpression, ): void { node.params.forEach(param => { getParamTypesNodes(param).forEach(paramTypeNode => { @@ -105,7 +144,8 @@ export default createRule<[], MessageIds>({ node: | TSESTree.FunctionDeclaration | TSESTree.TSDeclareFunction - | TSESTree.ArrowFunctionExpression, + | TSESTree.ArrowFunctionExpression + | TSESTree.FunctionExpression, ): void { const returnTypeNode = node.returnType; @@ -285,6 +325,15 @@ export default createRule<[], MessageIds>({ 'ExportNamedDeclaration[declaration.type="VariableDeclaration"]': visitExportedVariableDeclaration, + + 'ExportDefaultDeclaration[declaration.type="FunctionDeclaration"]': + visitExportedFunctionDeclaration, + + 'ExportDefaultDeclaration[declaration.type="ArrowFunctionExpression"]': + visitDefaultExportedArrowFunction, + + 'ExportDefaultDeclaration[declaration.type="Identifier"]': + visitDefaultExportedIdentifier, }; }, }); diff --git a/packages/eslint-plugin/tests/rules/require-types-exports.test.ts b/packages/eslint-plugin/tests/rules/require-types-exports.test.ts index d78a12b004d..5832d5739c4 100644 --- a/packages/eslint-plugin/tests/rules/require-types-exports.test.ts +++ b/packages/eslint-plugin/tests/rules/require-types-exports.test.ts @@ -42,6 +42,9 @@ ruleTester.run('require-types-exports', rule, { 'export function f(...args: unknown[]): void {}', 'export const f = (...args: unknown[]): void => {};', + 'export default function f(): void {}', + 'export default (): void => {};', + ` type A = number; function f(a: A): A { @@ -299,6 +302,44 @@ ruleTester.run('require-types-exports', rule, { ], }, + { + code: ` + type Arg = number; + + export default function(a: Arg): void {} + `, + errors: [ + { + messageId: 'requireTypeExport', + line: 4, + column: 36, + endColumn: 39, + data: { + name: 'Arg', + }, + }, + ], + }, + + { + code: ` + type Arg = number; + + export default (a: Arg): void => {} + `, + errors: [ + { + messageId: 'requireTypeExport', + line: 4, + column: 28, + endColumn: 31, + data: { + name: 'Arg', + }, + }, + ], + }, + { code: ` type Arg = number; @@ -806,6 +847,52 @@ ruleTester.run('require-types-exports', rule, { ], }, + { + code: ` + enum Fruit { + Apple, + Banana, + Cherry, + } + + export function f(a: Fruit): void {} + `, + errors: [ + { + messageId: 'requireTypeExport', + line: 8, + column: 30, + endColumn: 35, + data: { + name: 'Fruit', + }, + }, + ], + }, + + { + code: ` + enum Fruit { + Apple, + Banana, + Cherry, + } + + export const f = (a: Fruit): void => {} + `, + errors: [ + { + messageId: 'requireTypeExport', + line: 8, + column: 30, + endColumn: 35, + data: { + name: 'Fruit', + }, + }, + ], + }, + // TODO: Find a reasonable way to handle this case { code: ` @@ -1115,6 +1202,48 @@ ruleTester.run('require-types-exports', rule, { ], }, + { + code: ` + type Arg = number; + + const a = (a: Arg): void => {}; + + export default a; + `, + errors: [ + { + messageId: 'requireTypeExport', + line: 4, + column: 23, + endColumn: 26, + data: { + name: 'Arg', + }, + }, + ], + }, + + { + code: ` + type Arg = number; + + const a = function (a: Arg): void {}; + + export default a; + `, + errors: [ + { + messageId: 'requireTypeExport', + line: 4, + column: 32, + endColumn: 35, + data: { + name: 'Arg', + }, + }, + ], + }, + { code: ` type Arg = number; From 6aa6446ed501c801bce0e5af7d161f8a750279c8 Mon Sep 17 00:00:00 2001 From: StyleShit Date: Tue, 13 Feb 2024 20:29:52 +0200 Subject: [PATCH 08/36] wip --- packages/eslint-plugin/src/rules/require-types-exports.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/eslint-plugin/src/rules/require-types-exports.ts b/packages/eslint-plugin/src/rules/require-types-exports.ts index 9a65af9ba39..3614f728828 100644 --- a/packages/eslint-plugin/src/rules/require-types-exports.ts +++ b/packages/eslint-plugin/src/rules/require-types-exports.ts @@ -1,7 +1,7 @@ import { TSESTree } from '@typescript-eslint/utils'; import { AST_NODE_TYPES } from '@typescript-eslint/utils'; -import { createRule, getParserServices } from '../util'; +import { createRule } from '../util'; import { DefinitionType } from '@typescript-eslint/scope-manager'; type MessageIds = 'requireTypeExport'; From 892c368fdb3787ec561660a8fd05448be1e75620 Mon Sep 17 00:00:00 2001 From: StyleShit Date: Tue, 13 Feb 2024 21:08:05 +0200 Subject: [PATCH 09/36] wip --- .../src/rules/require-types-exports.ts | 146 +++++++---- .../tests/rules/require-types-exports.test.ts | 245 +++++++++++++++++- .../require-types-exports.shot | 14 + 3 files changed, 347 insertions(+), 58 deletions(-) create mode 100644 packages/eslint-plugin/tests/schema-snapshots/require-types-exports.shot diff --git a/packages/eslint-plugin/src/rules/require-types-exports.ts b/packages/eslint-plugin/src/rules/require-types-exports.ts index 3614f728828..ffeafecf9ae 100644 --- a/packages/eslint-plugin/src/rules/require-types-exports.ts +++ b/packages/eslint-plugin/src/rules/require-types-exports.ts @@ -6,6 +6,12 @@ import { DefinitionType } from '@typescript-eslint/scope-manager'; type MessageIds = 'requireTypeExport'; +type FunctionNode = + | TSESTree.FunctionDeclaration + | TSESTree.TSDeclareFunction + | TSESTree.ArrowFunctionExpression + | TSESTree.FunctionExpression; + export default createRule<[], MessageIds>({ name: 'require-types-exports', meta: { @@ -104,16 +110,53 @@ export default createRule<[], MessageIds>({ } } - function checkFunctionParamsTypes( - node: - | TSESTree.FunctionDeclaration - | TSESTree.TSDeclareFunction - | TSESTree.ArrowFunctionExpression - | TSESTree.FunctionExpression, - ): void { + function checkFunctionParamsTypes(node: FunctionNode): void { node.params.forEach(param => { - getParamTypesNodes(param).forEach(paramTypeNode => { - const name = getTypeName(paramTypeNode); + getParamTypesNodes(param) + .flatMap(paramTypeNode => { + return convertGenericTypeToTypeReference(node, paramTypeNode); + }) + .forEach(paramTypeNode => { + const name = getTypeName(paramTypeNode); + + if (!name) { + // TODO: Report on the whole function? Is this case even possible? + return; + } + + const isExported = exportedTypes.has(name); + const isReported = reported.has(name); + + if (isExported || isReported) { + return; + } + + context.report({ + node: paramTypeNode, + messageId: 'requireTypeExport', + data: { + name, + }, + }); + + reported.add(name); + }); + }); + } + + function checkFunctionReturnType(node: FunctionNode): void { + const returnTypeNode = node.returnType; + + if (!returnTypeNode) { + return; + } + + getReturnTypeTypesNodes(returnTypeNode) + .flatMap(paramTypeNode => { + return convertGenericTypeToTypeReference(node, paramTypeNode); + }) + .forEach(returnTypeNode => { + const name = getTypeName(returnTypeNode); if (!name) { // TODO: Report on the whole function? Is this case even possible? @@ -128,7 +171,7 @@ export default createRule<[], MessageIds>({ } context.report({ - node: paramTypeNode, + node: returnTypeNode, messageId: 'requireTypeExport', data: { name, @@ -137,47 +180,6 @@ export default createRule<[], MessageIds>({ reported.add(name); }); - }); - } - - function checkFunctionReturnType( - node: - | TSESTree.FunctionDeclaration - | TSESTree.TSDeclareFunction - | TSESTree.ArrowFunctionExpression - | TSESTree.FunctionExpression, - ): void { - const returnTypeNode = node.returnType; - - if (!returnTypeNode) { - return; - } - - getReturnTypeTypesNodes(returnTypeNode).forEach(returnTypeNode => { - const name = getTypeName(returnTypeNode); - - if (!name) { - // TODO: Report on the whole function? Is this case even possible? - return; - } - - const isExported = exportedTypes.has(name); - const isReported = reported.has(name); - - if (isExported || isReported) { - return; - } - - context.report({ - node: returnTypeNode, - messageId: 'requireTypeExport', - data: { - name, - }, - }); - - reported.add(name); - }); } function getParamTypesNodes( @@ -306,6 +308,48 @@ export default createRule<[], MessageIds>({ return []; } + function convertGenericTypeToTypeReference( + functionNode: FunctionNode, + typeNode: TSESTree.TSTypeReference, + ): TSESTree.TSTypeReference | TSESTree.TSTypeReference[] { + const typeName = getTypeName(typeNode); + + if (!typeName) { + return typeNode; + } + + const scope = context.sourceCode.getScope(functionNode); + const variable = scope.set.get(typeName); + + if (!variable || !variable.isTypeVariable) { + return typeNode; + } + + for (const definition of variable.defs) { + if ( + definition.type === DefinitionType.Type && + definition.node.type === AST_NODE_TYPES.TSTypeParameter && + definition.node.constraint + ) { + switch (definition.node.constraint.type) { + case AST_NODE_TYPES.TSTypeReference: + return definition.node.constraint; + + case AST_NODE_TYPES.TSUnionType: + case AST_NODE_TYPES.TSIntersectionType: + return definition.node.constraint.types.filter( + type => type.type === AST_NODE_TYPES.TSTypeReference, + ) as TSESTree.TSTypeReference[]; + + default: + continue; + } + } + } + + return typeNode; + } + function getTypeName(typeReference: TSESTree.TSTypeReference): string { if (typeReference.typeName.type === AST_NODE_TYPES.Identifier) { return typeReference.typeName.name; diff --git a/packages/eslint-plugin/tests/rules/require-types-exports.test.ts b/packages/eslint-plugin/tests/rules/require-types-exports.test.ts index 5832d5739c4..6bdc65c1dda 100644 --- a/packages/eslint-plugin/tests/rules/require-types-exports.test.ts +++ b/packages/eslint-plugin/tests/rules/require-types-exports.test.ts @@ -893,7 +893,6 @@ ruleTester.run('require-types-exports', rule, { ], }, - // TODO: Find a reasonable way to handle this case { code: ` type Arg = number; @@ -905,7 +904,7 @@ ruleTester.run('require-types-exports', rule, { messageId: 'requireTypeExport', line: 4, column: 37, - endColumn: 39, + endColumn: 40, data: { name: 'Arg', }, @@ -915,18 +914,115 @@ ruleTester.run('require-types-exports', rule, { { code: ` - type Arg = number; + type Arg1 = number; + type Arg2 = string; - export const f = (a: T): void => {}; + export function f(a: T): void {} `, errors: [ { messageId: 'requireTypeExport', - line: 4, + line: 5, column: 37, - endColumn: 39, + endColumn: 41, data: { - name: 'Arg', + name: 'Arg1', + }, + }, + { + messageId: 'requireTypeExport', + line: 5, + column: 44, + endColumn: 48, + data: { + name: 'Arg2', + }, + }, + ], + }, + + { + code: ` + type Arg1 = number; + type Arg2 = string; + + export function f(a: T): void {} + `, + errors: [ + { + messageId: 'requireTypeExport', + line: 5, + column: 37, + endColumn: 41, + data: { + name: 'Arg1', + }, + }, + { + messageId: 'requireTypeExport', + line: 5, + column: 44, + endColumn: 48, + data: { + name: 'Arg2', + }, + }, + ], + }, + + { + code: ` + type Arg1 = number; + type Arg2 = string; + + export function f(a: T): void {} + `, + errors: [ + { + messageId: 'requireTypeExport', + line: 5, + column: 37, + endColumn: 41, + data: { + name: 'Arg1', + }, + }, + { + messageId: 'requireTypeExport', + line: 5, + column: 44, + endColumn: 48, + data: { + name: 'Arg2', + }, + }, + ], + }, + + { + code: ` + type Arg1 = number; + type Arg2 = string; + + export function f(a: T): void {} + `, + errors: [ + { + messageId: 'requireTypeExport', + line: 5, + column: 37, + endColumn: 41, + data: { + name: 'Arg1', + }, + }, + { + messageId: 'requireTypeExport', + line: 5, + column: 44, + endColumn: 48, + data: { + name: 'Arg2', }, }, ], @@ -1202,6 +1298,141 @@ ruleTester.run('require-types-exports', rule, { ], }, + { + code: ` + type Ret = string; + + export function f(): T {} + `, + errors: [ + { + messageId: 'requireTypeExport', + line: 4, + column: 37, + endColumn: 40, + data: { + name: 'Ret', + }, + }, + ], + }, + + { + code: ` + type Ret1 = string; + type Ret2 = number; + + export function f(): T {} + `, + errors: [ + { + messageId: 'requireTypeExport', + line: 5, + column: 37, + endColumn: 41, + data: { + name: 'Ret1', + }, + }, + { + messageId: 'requireTypeExport', + line: 5, + column: 44, + endColumn: 48, + data: { + name: 'Ret2', + }, + }, + ], + }, + + { + code: ` + type Ret1 = string; + type Ret2 = number; + + export function f(): T {} + `, + errors: [ + { + messageId: 'requireTypeExport', + line: 5, + column: 37, + endColumn: 41, + data: { + name: 'Ret1', + }, + }, + { + messageId: 'requireTypeExport', + line: 5, + column: 44, + endColumn: 48, + data: { + name: 'Ret2', + }, + }, + ], + }, + + { + code: ` + type Ret1 = string; + type Ret2 = number; + + export function f(): T {} + `, + errors: [ + { + messageId: 'requireTypeExport', + line: 5, + column: 37, + endColumn: 41, + data: { + name: 'Ret1', + }, + }, + { + messageId: 'requireTypeExport', + line: 5, + column: 44, + endColumn: 48, + data: { + name: 'Ret2', + }, + }, + ], + }, + + { + code: ` + type Ret1 = string; + type Ret2 = number; + + export function f(): T {} + `, + errors: [ + { + messageId: 'requireTypeExport', + line: 5, + column: 37, + endColumn: 41, + data: { + name: 'Ret1', + }, + }, + { + messageId: 'requireTypeExport', + line: 5, + column: 44, + endColumn: 48, + data: { + name: 'Ret2', + }, + }, + ], + }, + { code: ` type Arg = number; diff --git a/packages/eslint-plugin/tests/schema-snapshots/require-types-exports.shot b/packages/eslint-plugin/tests/schema-snapshots/require-types-exports.shot new file mode 100644 index 00000000000..2f0fbf6d8df --- /dev/null +++ b/packages/eslint-plugin/tests/schema-snapshots/require-types-exports.shot @@ -0,0 +1,14 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`Rule schemas should be convertible to TS types for documentation purposes require-types-exports 1`] = ` +" +# SCHEMA: + +[] + + +# TYPES: + +/** No options declared */ +type Options = [];" +`; From 0e8e58fc52567ec571b014598a345d6814e31e71 Mon Sep 17 00:00:00 2001 From: StyleShit Date: Tue, 13 Feb 2024 21:15:26 +0200 Subject: [PATCH 10/36] tuple generic --- .../src/rules/require-types-exports.ts | 9 +++++ .../tests/rules/require-types-exports.test.ts | 38 +++++++++++++++++++ 2 files changed, 47 insertions(+) diff --git a/packages/eslint-plugin/src/rules/require-types-exports.ts b/packages/eslint-plugin/src/rules/require-types-exports.ts index ffeafecf9ae..b4b8eb5f3b6 100644 --- a/packages/eslint-plugin/src/rules/require-types-exports.ts +++ b/packages/eslint-plugin/src/rules/require-types-exports.ts @@ -332,15 +332,24 @@ export default createRule<[], MessageIds>({ definition.node.constraint ) { switch (definition.node.constraint.type) { + // T extends SomeType case AST_NODE_TYPES.TSTypeReference: return definition.node.constraint; + // T extends SomeType | AnotherType + // T extends SomeType & AnotherType case AST_NODE_TYPES.TSUnionType: case AST_NODE_TYPES.TSIntersectionType: return definition.node.constraint.types.filter( type => type.type === AST_NODE_TYPES.TSTypeReference, ) as TSESTree.TSTypeReference[]; + // T extends [SomeType, AnotherType] + case AST_NODE_TYPES.TSTupleType: + return definition.node.constraint.elementTypes.filter( + type => type.type === AST_NODE_TYPES.TSTypeReference, + ) as TSESTree.TSTypeReference[]; + default: continue; } diff --git a/packages/eslint-plugin/tests/rules/require-types-exports.test.ts b/packages/eslint-plugin/tests/rules/require-types-exports.test.ts index 6bdc65c1dda..a90aa473964 100644 --- a/packages/eslint-plugin/tests/rules/require-types-exports.test.ts +++ b/packages/eslint-plugin/tests/rules/require-types-exports.test.ts @@ -1028,6 +1028,25 @@ ruleTester.run('require-types-exports', rule, { ], }, + { + code: ` + type Arg = string; + + export function f(a: T): void {} + `, + errors: [ + { + messageId: 'requireTypeExport', + line: 4, + column: 38, + endColumn: 41, + data: { + name: 'Arg', + }, + }, + ], + }, + { code: ` type Ret = string; @@ -1433,6 +1452,25 @@ ruleTester.run('require-types-exports', rule, { ], }, + { + code: ` + type Ret = string; + + export function f(): T {} + `, + errors: [ + { + messageId: 'requireTypeExport', + line: 4, + column: 38, + endColumn: 41, + data: { + name: 'Ret', + }, + }, + ], + }, + { code: ` type Arg = number; From f4018a8f43603800f3113a4fceacd3c83c11f136 Mon Sep 17 00:00:00 2001 From: StyleShit Date: Tue, 13 Feb 2024 21:37:17 +0200 Subject: [PATCH 11/36] wip --- .../src/rules/require-types-exports.ts | 16 ++++++++++++++ .../tests/rules/require-types-exports.test.ts | 21 ++++++++++++++++++- 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/packages/eslint-plugin/src/rules/require-types-exports.ts b/packages/eslint-plugin/src/rules/require-types-exports.ts index b4b8eb5f3b6..4e208e813d7 100644 --- a/packages/eslint-plugin/src/rules/require-types-exports.ts +++ b/packages/eslint-plugin/src/rules/require-types-exports.ts @@ -350,6 +350,22 @@ export default createRule<[], MessageIds>({ type => type.type === AST_NODE_TYPES.TSTypeReference, ) as TSESTree.TSTypeReference[]; + // T extends { some: SomeType, another: AnotherType } + case AST_NODE_TYPES.TSTypeLiteral: + return definition.node.constraint.members.reduce< + TSESTree.TSTypeReference[] + >((acc, member) => { + if ( + member.type === AST_NODE_TYPES.TSPropertySignature && + member.typeAnnotation?.typeAnnotation.type === + AST_NODE_TYPES.TSTypeReference + ) { + acc.push(member.typeAnnotation.typeAnnotation); + } + + return acc; + }, []); + default: continue; } diff --git a/packages/eslint-plugin/tests/rules/require-types-exports.test.ts b/packages/eslint-plugin/tests/rules/require-types-exports.test.ts index a90aa473964..0a8833f44c0 100644 --- a/packages/eslint-plugin/tests/rules/require-types-exports.test.ts +++ b/packages/eslint-plugin/tests/rules/require-types-exports.test.ts @@ -1004,7 +1004,7 @@ ruleTester.run('require-types-exports', rule, { type Arg1 = number; type Arg2 = string; - export function f(a: T): void {} + export const f = (a: T): void => {} `, errors: [ { @@ -1047,6 +1047,25 @@ ruleTester.run('require-types-exports', rule, { ], }, + { + code: ` + type Arg = string; + + export function f(a: T): void {} + `, + errors: [ + { + messageId: 'requireTypeExport', + line: 4, + column: 42, + endColumn: 45, + data: { + name: 'Arg', + }, + }, + ], + }, + { code: ` type Ret = string; From 89a8344b63b03d0dd2689266637af5f419e27d58 Mon Sep 17 00:00:00 2001 From: StyleShit Date: Tue, 13 Feb 2024 22:11:48 +0200 Subject: [PATCH 12/36] wip --- packages/eslint-plugin/src/rules/require-types-exports.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/eslint-plugin/src/rules/require-types-exports.ts b/packages/eslint-plugin/src/rules/require-types-exports.ts index 4e208e813d7..281b204556c 100644 --- a/packages/eslint-plugin/src/rules/require-types-exports.ts +++ b/packages/eslint-plugin/src/rules/require-types-exports.ts @@ -321,7 +321,7 @@ export default createRule<[], MessageIds>({ const scope = context.sourceCode.getScope(functionNode); const variable = scope.set.get(typeName); - if (!variable || !variable.isTypeVariable) { + if (!variable?.isTypeVariable) { return typeNode; } From b2138e36213a51de1f750390ef4394a2364d3cfd Mon Sep 17 00:00:00 2001 From: StyleShit Date: Thu, 15 Feb 2024 16:01:13 +0200 Subject: [PATCH 13/36] wip --- .../src/rules/require-types-exports.ts | 51 +++++++++---------- .../tests/rules/require-types-exports.test.ts | 36 ++++++++++--- 2 files changed, 53 insertions(+), 34 deletions(-) diff --git a/packages/eslint-plugin/src/rules/require-types-exports.ts b/packages/eslint-plugin/src/rules/require-types-exports.ts index 281b204556c..0f55054e3b0 100644 --- a/packages/eslint-plugin/src/rules/require-types-exports.ts +++ b/packages/eslint-plugin/src/rules/require-types-exports.ts @@ -1,8 +1,8 @@ -import { TSESTree } from '@typescript-eslint/utils'; +import { DefinitionType } from '@typescript-eslint/scope-manager'; +import type { TSESTree } from '@typescript-eslint/utils'; import { AST_NODE_TYPES } from '@typescript-eslint/utils'; import { createRule } from '../util'; -import { DefinitionType } from '@typescript-eslint/scope-manager'; type MessageIds = 'requireTypeExport'; @@ -29,26 +29,17 @@ export default createRule<[], MessageIds>({ }, defaultOptions: [], create(context) { - const exportedTypes = new Set(); - const reported = new Set(); - - function collectExportedTypes(program: TSESTree.Program): void { - program.body.forEach(statement => { - if (statement.type !== AST_NODE_TYPES.ExportNamedDeclaration) { - return; - } + const externalizedTypes = new Set(); + const reportedTypes = new Set(); - const { declaration } = statement; - - if ( - declaration?.type === AST_NODE_TYPES.TSTypeAliasDeclaration || - declaration?.type === AST_NODE_TYPES.TSInterfaceDeclaration - ) { - exportedTypes.add(declaration.id.name); + function collectImportedTypes(node: TSESTree.ImportSpecifier): void { + externalizedTypes.add(node.local.name); + } - return; - } - }); + function collectExportedTypes( + node: TSESTree.TSTypeAliasDeclaration | TSESTree.TSInterfaceDeclaration, + ): void { + externalizedTypes.add(node.id.name); } function visitExportedFunctionDeclaration( @@ -89,7 +80,7 @@ export default createRule<[], MessageIds>({ node: TSESTree.DefaultExportDeclarations & { declaration: TSESTree.Identifier; }, - ) { + ): void { const scope = context.sourceCode.getScope(node); const variable = scope.set.get(node.declaration.name); @@ -124,8 +115,8 @@ export default createRule<[], MessageIds>({ return; } - const isExported = exportedTypes.has(name); - const isReported = reported.has(name); + const isExported = externalizedTypes.has(name); + const isReported = reportedTypes.has(name); if (isExported || isReported) { return; @@ -139,7 +130,7 @@ export default createRule<[], MessageIds>({ }, }); - reported.add(name); + reportedTypes.add(name); }); }); } @@ -163,8 +154,8 @@ export default createRule<[], MessageIds>({ return; } - const isExported = exportedTypes.has(name); - const isReported = reported.has(name); + const isExported = externalizedTypes.has(name); + const isReported = reportedTypes.has(name); if (isExported || isReported) { return; @@ -178,7 +169,7 @@ export default createRule<[], MessageIds>({ }, }); - reported.add(name); + reportedTypes.add(name); }); } @@ -384,7 +375,11 @@ export default createRule<[], MessageIds>({ } return { - Program: collectExportedTypes, + 'ImportDeclaration[importKind="type"] ImportSpecifier, ImportSpecifier[importKind="type"]': + collectImportedTypes, + + 'ExportNamedDeclaration TSTypeAliasDeclaration, ExportNamedDeclaration TSInterfaceDeclaration': + collectExportedTypes, 'ExportNamedDeclaration[declaration.type="FunctionDeclaration"]': visitExportedFunctionDeclaration, diff --git a/packages/eslint-plugin/tests/rules/require-types-exports.test.ts b/packages/eslint-plugin/tests/rules/require-types-exports.test.ts index 0a8833f44c0..c83b3b89341 100644 --- a/packages/eslint-plugin/tests/rules/require-types-exports.test.ts +++ b/packages/eslint-plugin/tests/rules/require-types-exports.test.ts @@ -261,6 +261,30 @@ ruleTester.run('require-types-exports', rule, { export type B = string; export const f = (): { a: A; b: B } => {}; `, + + ` + import { testFunction, type Arg } from './module'; + + export function f(a: Arg): void {} + `, + + ` + import type { Arg } from './types'; + + export function f(a: Arg): void {} + `, + + ` + import type { ImportedArg as Arg } from './types'; + + export function f(a: Arg): void {} + `, + + ` + import type { Arg } from './types'; + + export function f(a: T): void {} + `, ], invalid: [ @@ -306,14 +330,14 @@ ruleTester.run('require-types-exports', rule, { code: ` type Arg = number; - export default function(a: Arg): void {} + export default function (a: Arg): void {} `, errors: [ { messageId: 'requireTypeExport', line: 4, - column: 36, - endColumn: 39, + column: 37, + endColumn: 40, data: { name: 'Arg', }, @@ -325,7 +349,7 @@ ruleTester.run('require-types-exports', rule, { code: ` type Arg = number; - export default (a: Arg): void => {} + export default (a: Arg): void => {}; `, errors: [ { @@ -878,7 +902,7 @@ ruleTester.run('require-types-exports', rule, { Cherry, } - export const f = (a: Fruit): void => {} + export const f = (a: Fruit): void => {}; `, errors: [ { @@ -1004,7 +1028,7 @@ ruleTester.run('require-types-exports', rule, { type Arg1 = number; type Arg2 = string; - export const f = (a: T): void => {} + export const f = (a: T): void => {}; `, errors: [ { From 1161db04d32ea6039c4646b096d3817dbe59de02 Mon Sep 17 00:00:00 2001 From: StyleShit Date: Fri, 16 Feb 2024 14:15:10 +0200 Subject: [PATCH 14/36] wip --- packages/eslint-plugin/src/rules/require-types-exports.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/eslint-plugin/src/rules/require-types-exports.ts b/packages/eslint-plugin/src/rules/require-types-exports.ts index 0f55054e3b0..681ee9d1675 100644 --- a/packages/eslint-plugin/src/rules/require-types-exports.ts +++ b/packages/eslint-plugin/src/rules/require-types-exports.ts @@ -115,10 +115,10 @@ export default createRule<[], MessageIds>({ return; } - const isExported = externalizedTypes.has(name); + const isExternalized = externalizedTypes.has(name); const isReported = reportedTypes.has(name); - if (isExported || isReported) { + if (isExternalized || isReported) { return; } @@ -154,10 +154,10 @@ export default createRule<[], MessageIds>({ return; } - const isExported = externalizedTypes.has(name); + const isExternalized = externalizedTypes.has(name); const isReported = reportedTypes.has(name); - if (isExported || isReported) { + if (isExternalized || isReported) { return; } From d9875b32873e8cbaf87749fc35a37c1a915d99de Mon Sep 17 00:00:00 2001 From: StyleShit Date: Fri, 16 Feb 2024 14:22:47 +0200 Subject: [PATCH 15/36] refactor --- .../src/rules/require-types-exports.ts | 98 ++++++++++--------- 1 file changed, 50 insertions(+), 48 deletions(-) diff --git a/packages/eslint-plugin/src/rules/require-types-exports.ts b/packages/eslint-plugin/src/rules/require-types-exports.ts index 681ee9d1675..a71f9c09ca0 100644 --- a/packages/eslint-plugin/src/rules/require-types-exports.ts +++ b/packages/eslint-plugin/src/rules/require-types-exports.ts @@ -102,55 +102,16 @@ export default createRule<[], MessageIds>({ } function checkFunctionParamsTypes(node: FunctionNode): void { - node.params.forEach(param => { - getParamTypesNodes(param) - .flatMap(paramTypeNode => { - return convertGenericTypeToTypeReference(node, paramTypeNode); - }) - .forEach(paramTypeNode => { - const name = getTypeName(paramTypeNode); - - if (!name) { - // TODO: Report on the whole function? Is this case even possible? - return; - } - - const isExternalized = externalizedTypes.has(name); - const isReported = reportedTypes.has(name); - - if (isExternalized || isReported) { - return; - } - - context.report({ - node: paramTypeNode, - messageId: 'requireTypeExport', - data: { - name, - }, - }); - - reportedTypes.add(name); - }); - }); - } - - function checkFunctionReturnType(node: FunctionNode): void { - const returnTypeNode = node.returnType; - - if (!returnTypeNode) { - return; - } + for (const param of node.params) { + const typeNodes = getParamTypesNodes(param).flatMap(typeNode => { + return convertGenericTypeToTypeReferences(node, typeNode); + }); - getReturnTypeTypesNodes(returnTypeNode) - .flatMap(paramTypeNode => { - return convertGenericTypeToTypeReference(node, paramTypeNode); - }) - .forEach(returnTypeNode => { - const name = getTypeName(returnTypeNode); + for (const typeNode of typeNodes) { + const name = getTypeName(typeNode); if (!name) { - // TODO: Report on the whole function? Is this case even possible? + // TODO: Report the whole function? Is this case even possible? return; } @@ -162,7 +123,7 @@ export default createRule<[], MessageIds>({ } context.report({ - node: returnTypeNode, + node: typeNode, messageId: 'requireTypeExport', data: { name, @@ -170,7 +131,48 @@ export default createRule<[], MessageIds>({ }); reportedTypes.add(name); + } + } + } + + function checkFunctionReturnType(node: FunctionNode): void { + const { returnType } = node; + + if (!returnType) { + return; + } + + const typeNodes = getReturnTypeTypesNodes(returnType).flatMap( + typeNode => { + return convertGenericTypeToTypeReferences(node, typeNode); + }, + ); + + for (const typeNode of typeNodes) { + const name = getTypeName(typeNode); + + if (!name) { + // TODO: Report the whole function? Is this case even possible? + return; + } + + const isExternalized = externalizedTypes.has(name); + const isReported = reportedTypes.has(name); + + if (isExternalized || isReported) { + return; + } + + context.report({ + node: typeNode, + messageId: 'requireTypeExport', + data: { + name, + }, }); + + reportedTypes.add(name); + } } function getParamTypesNodes( @@ -299,7 +301,7 @@ export default createRule<[], MessageIds>({ return []; } - function convertGenericTypeToTypeReference( + function convertGenericTypeToTypeReferences( functionNode: FunctionNode, typeNode: TSESTree.TSTypeReference, ): TSESTree.TSTypeReference | TSESTree.TSTypeReference[] { From 6338202219bc9f38172b33a45ca8624e9dcffaa6 Mon Sep 17 00:00:00 2001 From: StyleShit Date: Fri, 16 Feb 2024 14:42:09 +0200 Subject: [PATCH 16/36] make it shorter & more readable --- .../src/rules/require-types-exports.ts | 180 ++++++++---------- 1 file changed, 79 insertions(+), 101 deletions(-) diff --git a/packages/eslint-plugin/src/rules/require-types-exports.ts b/packages/eslint-plugin/src/rules/require-types-exports.ts index a71f9c09ca0..66347fb9556 100644 --- a/packages/eslint-plugin/src/rules/require-types-exports.ts +++ b/packages/eslint-plugin/src/rules/require-types-exports.ts @@ -59,12 +59,12 @@ export default createRule<[], MessageIds>({ declaration: TSESTree.VariableDeclaration; }, ): void { - node.declaration.declarations.forEach(declaration => { + for (const declaration of node.declaration.declarations) { if (declaration.init?.type === AST_NODE_TYPES.ArrowFunctionExpression) { checkFunctionParamsTypes(declaration.init); checkFunctionReturnType(declaration.init); } - }); + } } function visitDefaultExportedArrowFunction( @@ -88,49 +88,26 @@ export default createRule<[], MessageIds>({ return; } - for (const definition of variable.defs) { + for (const def of variable.defs) { if ( - definition.type === DefinitionType.Variable && - (definition.node.init?.type === - AST_NODE_TYPES.ArrowFunctionExpression || - definition.node.init?.type === AST_NODE_TYPES.FunctionExpression) + def.type === DefinitionType.Variable && + (def.node.init?.type === AST_NODE_TYPES.ArrowFunctionExpression || + def.node.init?.type === AST_NODE_TYPES.FunctionExpression) ) { - checkFunctionParamsTypes(definition.node.init); - checkFunctionReturnType(definition.node.init); + checkFunctionParamsTypes(def.node.init); + checkFunctionReturnType(def.node.init); } } } function checkFunctionParamsTypes(node: FunctionNode): void { for (const param of node.params) { - const typeNodes = getParamTypesNodes(param).flatMap(typeNode => { + const typeNodes = getParamTypeNodes(param).flatMap(typeNode => { return convertGenericTypeToTypeReferences(node, typeNode); }); for (const typeNode of typeNodes) { - const name = getTypeName(typeNode); - - if (!name) { - // TODO: Report the whole function? Is this case even possible? - return; - } - - const isExternalized = externalizedTypes.has(name); - const isReported = reportedTypes.has(name); - - if (isExternalized || isReported) { - return; - } - - context.report({ - node: typeNode, - messageId: 'requireTypeExport', - data: { - name, - }, - }); - - reportedTypes.add(name); + checkTypeNode(typeNode); } } } @@ -142,40 +119,42 @@ export default createRule<[], MessageIds>({ return; } - const typeNodes = getReturnTypeTypesNodes(returnType).flatMap( - typeNode => { - return convertGenericTypeToTypeReferences(node, typeNode); - }, - ); + const typeNodes = getReturnTypeTypeNodes(returnType).flatMap(typeNode => { + return convertGenericTypeToTypeReferences(node, typeNode); + }); for (const typeNode of typeNodes) { - const name = getTypeName(typeNode); - - if (!name) { - // TODO: Report the whole function? Is this case even possible? - return; - } + checkTypeNode(typeNode); + } + } - const isExternalized = externalizedTypes.has(name); - const isReported = reportedTypes.has(name); + function checkTypeNode(node: TSESTree.TSTypeReference): void { + const name = getTypeName(node); - if (isExternalized || isReported) { - return; - } + if (!name) { + // TODO: Report the whole function? Is this case even possible? + return; + } - context.report({ - node: typeNode, - messageId: 'requireTypeExport', - data: { - name, - }, - }); + const isExternalized = externalizedTypes.has(name); + const isReported = reportedTypes.has(name); - reportedTypes.add(name); + if (isExternalized || isReported) { + return; } + + context.report({ + node: node, + messageId: 'requireTypeExport', + data: { + name, + }, + }); + + reportedTypes.add(name); } - function getParamTypesNodes( + function getParamTypeNodes( param: TSESTree.Parameter, ): TSESTree.TSTypeReference[] { // Single type @@ -254,7 +233,7 @@ export default createRule<[], MessageIds>({ return []; } - function getReturnTypeTypesNodes( + function getReturnTypeTypeNodes( typeAnnotation: TSESTree.TSTypeAnnotation, ): TSESTree.TSTypeReference[] { // Single type @@ -318,50 +297,49 @@ export default createRule<[], MessageIds>({ return typeNode; } - for (const definition of variable.defs) { + for (const def of variable.defs) { if ( - definition.type === DefinitionType.Type && - definition.node.type === AST_NODE_TYPES.TSTypeParameter && - definition.node.constraint + def.type !== DefinitionType.Type || + def.node.type !== AST_NODE_TYPES.TSTypeParameter || + !def.node.constraint ) { - switch (definition.node.constraint.type) { - // T extends SomeType - case AST_NODE_TYPES.TSTypeReference: - return definition.node.constraint; - - // T extends SomeType | AnotherType - // T extends SomeType & AnotherType - case AST_NODE_TYPES.TSUnionType: - case AST_NODE_TYPES.TSIntersectionType: - return definition.node.constraint.types.filter( - type => type.type === AST_NODE_TYPES.TSTypeReference, - ) as TSESTree.TSTypeReference[]; - - // T extends [SomeType, AnotherType] - case AST_NODE_TYPES.TSTupleType: - return definition.node.constraint.elementTypes.filter( - type => type.type === AST_NODE_TYPES.TSTypeReference, - ) as TSESTree.TSTypeReference[]; - - // T extends { some: SomeType, another: AnotherType } - case AST_NODE_TYPES.TSTypeLiteral: - return definition.node.constraint.members.reduce< - TSESTree.TSTypeReference[] - >((acc, member) => { - if ( - member.type === AST_NODE_TYPES.TSPropertySignature && - member.typeAnnotation?.typeAnnotation.type === - AST_NODE_TYPES.TSTypeReference - ) { - acc.push(member.typeAnnotation.typeAnnotation); - } - - return acc; - }, []); - - default: - continue; - } + continue; + } + + switch (def.node.constraint.type) { + // T extends SomeType + case AST_NODE_TYPES.TSTypeReference: + return def.node.constraint; + + // T extends SomeType | AnotherType + // T extends SomeType & AnotherType + case AST_NODE_TYPES.TSUnionType: + case AST_NODE_TYPES.TSIntersectionType: + return def.node.constraint.types.filter( + type => type.type === AST_NODE_TYPES.TSTypeReference, + ) as TSESTree.TSTypeReference[]; + + // T extends [SomeType, AnotherType] + case AST_NODE_TYPES.TSTupleType: + return def.node.constraint.elementTypes.filter( + type => type.type === AST_NODE_TYPES.TSTypeReference, + ) as TSESTree.TSTypeReference[]; + + // T extends { some: SomeType, another: AnotherType } + case AST_NODE_TYPES.TSTypeLiteral: + return def.node.constraint.members.reduce< + TSESTree.TSTypeReference[] + >((acc, member) => { + if ( + member.type === AST_NODE_TYPES.TSPropertySignature && + member.typeAnnotation?.typeAnnotation.type === + AST_NODE_TYPES.TSTypeReference + ) { + acc.push(member.typeAnnotation.typeAnnotation); + } + + return acc; + }, []); } } From 1812e37f6501844fc72fc1d4ae006300464a8e34 Mon Sep 17 00:00:00 2001 From: StyleShit Date: Wed, 24 Apr 2024 12:21:57 +0300 Subject: [PATCH 17/36] fix nested types in functions --- .../src/rules/require-types-exports.ts | 240 +----------------- .../tests/rules/require-types-exports.test.ts | 48 ++++ 2 files changed, 62 insertions(+), 226 deletions(-) diff --git a/packages/eslint-plugin/src/rules/require-types-exports.ts b/packages/eslint-plugin/src/rules/require-types-exports.ts index 66347fb9556..b99ba0ea710 100644 --- a/packages/eslint-plugin/src/rules/require-types-exports.ts +++ b/packages/eslint-plugin/src/rules/require-types-exports.ts @@ -50,8 +50,7 @@ export default createRule<[], MessageIds>({ declaration: TSESTree.FunctionDeclaration | TSESTree.TSDeclareFunction; }, ): void { - checkFunctionParamsTypes(node.declaration); - checkFunctionReturnType(node.declaration); + checkFunctionTypes(node.declaration); } function visitExportedVariableDeclaration( @@ -61,8 +60,7 @@ export default createRule<[], MessageIds>({ ): void { for (const declaration of node.declaration.declarations) { if (declaration.init?.type === AST_NODE_TYPES.ArrowFunctionExpression) { - checkFunctionParamsTypes(declaration.init); - checkFunctionReturnType(declaration.init); + checkFunctionTypes(declaration.init); } } } @@ -72,8 +70,7 @@ export default createRule<[], MessageIds>({ declaration: TSESTree.ArrowFunctionExpression; }, ): void { - checkFunctionParamsTypes(node.declaration); - checkFunctionReturnType(node.declaration); + checkFunctionTypes(node.declaration); } function visitDefaultExportedIdentifier( @@ -94,38 +91,21 @@ export default createRule<[], MessageIds>({ (def.node.init?.type === AST_NODE_TYPES.ArrowFunctionExpression || def.node.init?.type === AST_NODE_TYPES.FunctionExpression) ) { - checkFunctionParamsTypes(def.node.init); - checkFunctionReturnType(def.node.init); + checkFunctionTypes(def.node.init); } } } - function checkFunctionParamsTypes(node: FunctionNode): void { - for (const param of node.params) { - const typeNodes = getParamTypeNodes(param).flatMap(typeNode => { - return convertGenericTypeToTypeReferences(node, typeNode); - }); - - for (const typeNode of typeNodes) { - checkTypeNode(typeNode); - } - } - } - - function checkFunctionReturnType(node: FunctionNode): void { - const { returnType } = node; - - if (!returnType) { - return; - } - - const typeNodes = getReturnTypeTypeNodes(returnType).flatMap(typeNode => { - return convertGenericTypeToTypeReferences(node, typeNode); - }); + function checkFunctionTypes(node: FunctionNode): void { + const scope = context.sourceCode.getScope(node); - for (const typeNode of typeNodes) { - checkTypeNode(typeNode); - } + scope.through + .map(ref => ref.identifier.parent) + .filter( + (node): node is TSESTree.TSTypeReference => + node.type === AST_NODE_TYPES.TSTypeReference, + ) + .forEach(checkTypeNode); } function checkTypeNode(node: TSESTree.TSTypeReference): void { @@ -154,198 +134,6 @@ export default createRule<[], MessageIds>({ reportedTypes.add(name); } - function getParamTypeNodes( - param: TSESTree.Parameter, - ): TSESTree.TSTypeReference[] { - // Single type - if ( - param.type === AST_NODE_TYPES.Identifier && - param.typeAnnotation?.typeAnnotation.type === - AST_NODE_TYPES.TSTypeReference - ) { - return [param.typeAnnotation.typeAnnotation]; - } - - // Union or intersection - if ( - param.type === AST_NODE_TYPES.Identifier && - (param.typeAnnotation?.typeAnnotation.type === - AST_NODE_TYPES.TSUnionType || - param.typeAnnotation?.typeAnnotation.type === - AST_NODE_TYPES.TSIntersectionType) - ) { - return param.typeAnnotation.typeAnnotation.types.filter( - type => type.type === AST_NODE_TYPES.TSTypeReference, - ) as TSESTree.TSTypeReference[]; - } - - // Tuple - if ( - param.type === AST_NODE_TYPES.ArrayPattern && - param.typeAnnotation?.typeAnnotation.type === AST_NODE_TYPES.TSTupleType - ) { - return param.typeAnnotation.typeAnnotation.elementTypes.filter( - type => type.type === AST_NODE_TYPES.TSTypeReference, - ) as TSESTree.TSTypeReference[]; - } - - // Inline object - if ( - param.type === AST_NODE_TYPES.ObjectPattern && - param.typeAnnotation?.typeAnnotation.type === - AST_NODE_TYPES.TSTypeLiteral - ) { - return param.typeAnnotation.typeAnnotation.members.reduce< - TSESTree.TSTypeReference[] - >((acc, member) => { - if ( - member.type === AST_NODE_TYPES.TSPropertySignature && - member.typeAnnotation?.typeAnnotation.type === - AST_NODE_TYPES.TSTypeReference - ) { - acc.push(member.typeAnnotation.typeAnnotation); - } - - return acc; - }, []); - } - - // Rest params - if ( - param.type === AST_NODE_TYPES.RestElement && - param.typeAnnotation?.typeAnnotation.type === - AST_NODE_TYPES.TSArrayType && - param.typeAnnotation.typeAnnotation.elementType.type === - AST_NODE_TYPES.TSTypeReference - ) { - return [param.typeAnnotation.typeAnnotation.elementType]; - } - - // Default value assignment - if ( - param.type === AST_NODE_TYPES.AssignmentPattern && - param.left.typeAnnotation?.typeAnnotation.type === - AST_NODE_TYPES.TSTypeReference - ) { - return [param.left.typeAnnotation.typeAnnotation]; - } - - return []; - } - - function getReturnTypeTypeNodes( - typeAnnotation: TSESTree.TSTypeAnnotation, - ): TSESTree.TSTypeReference[] { - // Single type - if ( - typeAnnotation.typeAnnotation.type === AST_NODE_TYPES.TSTypeReference - ) { - return [typeAnnotation.typeAnnotation]; - } - - // Union or intersection - if ( - typeAnnotation.typeAnnotation.type === AST_NODE_TYPES.TSUnionType || - typeAnnotation.typeAnnotation.type === AST_NODE_TYPES.TSIntersectionType - ) { - return typeAnnotation.typeAnnotation.types.filter( - type => type.type === AST_NODE_TYPES.TSTypeReference, - ) as TSESTree.TSTypeReference[]; - } - - // Tuple - if (typeAnnotation.typeAnnotation.type === AST_NODE_TYPES.TSTupleType) { - return typeAnnotation.typeAnnotation.elementTypes.filter( - type => type.type === AST_NODE_TYPES.TSTypeReference, - ) as TSESTree.TSTypeReference[]; - } - - // Inline object - if (typeAnnotation.typeAnnotation.type === AST_NODE_TYPES.TSTypeLiteral) { - return typeAnnotation.typeAnnotation.members.reduce< - TSESTree.TSTypeReference[] - >((acc, member) => { - if ( - member.type === AST_NODE_TYPES.TSPropertySignature && - member.typeAnnotation?.typeAnnotation.type === - AST_NODE_TYPES.TSTypeReference - ) { - acc.push(member.typeAnnotation.typeAnnotation); - } - - return acc; - }, []); - } - - return []; - } - - function convertGenericTypeToTypeReferences( - functionNode: FunctionNode, - typeNode: TSESTree.TSTypeReference, - ): TSESTree.TSTypeReference | TSESTree.TSTypeReference[] { - const typeName = getTypeName(typeNode); - - if (!typeName) { - return typeNode; - } - - const scope = context.sourceCode.getScope(functionNode); - const variable = scope.set.get(typeName); - - if (!variable?.isTypeVariable) { - return typeNode; - } - - for (const def of variable.defs) { - if ( - def.type !== DefinitionType.Type || - def.node.type !== AST_NODE_TYPES.TSTypeParameter || - !def.node.constraint - ) { - continue; - } - - switch (def.node.constraint.type) { - // T extends SomeType - case AST_NODE_TYPES.TSTypeReference: - return def.node.constraint; - - // T extends SomeType | AnotherType - // T extends SomeType & AnotherType - case AST_NODE_TYPES.TSUnionType: - case AST_NODE_TYPES.TSIntersectionType: - return def.node.constraint.types.filter( - type => type.type === AST_NODE_TYPES.TSTypeReference, - ) as TSESTree.TSTypeReference[]; - - // T extends [SomeType, AnotherType] - case AST_NODE_TYPES.TSTupleType: - return def.node.constraint.elementTypes.filter( - type => type.type === AST_NODE_TYPES.TSTypeReference, - ) as TSESTree.TSTypeReference[]; - - // T extends { some: SomeType, another: AnotherType } - case AST_NODE_TYPES.TSTypeLiteral: - return def.node.constraint.members.reduce< - TSESTree.TSTypeReference[] - >((acc, member) => { - if ( - member.type === AST_NODE_TYPES.TSPropertySignature && - member.typeAnnotation?.typeAnnotation.type === - AST_NODE_TYPES.TSTypeReference - ) { - acc.push(member.typeAnnotation.typeAnnotation); - } - - return acc; - }, []); - } - } - - return typeNode; - } - function getTypeName(typeReference: TSESTree.TSTypeReference): string { if (typeReference.typeName.type === AST_NODE_TYPES.Identifier) { return typeReference.typeName.name; @@ -355,7 +143,7 @@ export default createRule<[], MessageIds>({ } return { - 'ImportDeclaration[importKind="type"] ImportSpecifier, ImportSpecifier[importKind="type"]': + 'ImportDeclaration ImportSpecifier, ImportSpecifier': collectImportedTypes, 'ExportNamedDeclaration TSTypeAliasDeclaration, ExportNamedDeclaration TSInterfaceDeclaration': diff --git a/packages/eslint-plugin/tests/rules/require-types-exports.test.ts b/packages/eslint-plugin/tests/rules/require-types-exports.test.ts index c83b3b89341..6d05d62a244 100644 --- a/packages/eslint-plugin/tests/rules/require-types-exports.test.ts +++ b/packages/eslint-plugin/tests/rules/require-types-exports.test.ts @@ -268,6 +268,12 @@ ruleTester.run('require-types-exports', rule, { export function f(a: Arg): void {} `, + ` + import { Arg } from './types'; + + export function f(a: Arg): void {} + `, + ` import type { Arg } from './types'; @@ -1594,6 +1600,48 @@ ruleTester.run('require-types-exports', rule, { ], }, + { + code: ` + type Arg1 = number; + type Arg2 = boolean; + type Ret = string; + + export declare function f( + a: { b: { c: Arg1 | number | { d: T } } }, + e: Arg1, + ): { a: { b: T | Ret } }; + `, + errors: [ + { + messageId: 'requireTypeExport', + line: 6, + column: 45, + endColumn: 49, + data: { + name: 'Arg2', + }, + }, + { + messageId: 'requireTypeExport', + line: 7, + column: 24, + endColumn: 28, + data: { + name: 'Arg1', + }, + }, + { + messageId: 'requireTypeExport', + line: 9, + column: 26, + endColumn: 29, + data: { + name: 'Ret', + }, + }, + ], + }, + { code: ` type Arg1 = number; From 4bee779c4082e68cad2713f5e85663e95c5d62b2 Mon Sep 17 00:00:00 2001 From: StyleShit Date: Wed, 24 Apr 2024 12:29:15 +0300 Subject: [PATCH 18/36] fix docs --- .../docs/rules/require-types-exports.md | 13 +++++++------ .../src/rules/require-types-exports.ts | 3 +-- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/eslint-plugin/docs/rules/require-types-exports.md b/packages/eslint-plugin/docs/rules/require-types-exports.md index 767050deeba..9343536e6d7 100644 --- a/packages/eslint-plugin/docs/rules/require-types-exports.md +++ b/packages/eslint-plugin/docs/rules/require-types-exports.md @@ -1,16 +1,17 @@ --- -description: 'Require exporting types that are used in exported functions declarations.' +description: 'Require exporting types that are used in exported entities.' --- > 🛑 This file is source code, not the primary documentation location! 🛑 > > See **https://typescript-eslint.io/rules/require-types-exports** for documentation. -When exporting functions from a module, it is recommended to export also all the -types that are used in the function declarations. This is useful for consumers of -the module, as it allows them to use the types in their own code without having to -use things like [`Parameters`](https://www.typescriptlang.org/docs/handbook/utility-types.html#parameterstype) -or [`ReturnType`](https://www.typescriptlang.org/docs/handbook/utility-types.html#returntypetype) to extract the types from the function. +When exporting entities from a module, it is recommended to export also all the +types that are used in their declarations. This is useful for consumers of the +module, as it allows them to use the types in their own code without having to +use utility types like [`Parameters`](https://www.typescriptlang.org/docs/handbook/utility-types.html#parameterstype) +or [`ReturnType`](https://www.typescriptlang.org/docs/handbook/utility-types.html#returntypetype) +in order to extract the types from your code. ## Examples diff --git a/packages/eslint-plugin/src/rules/require-types-exports.ts b/packages/eslint-plugin/src/rules/require-types-exports.ts index b99ba0ea710..e5cde3687c4 100644 --- a/packages/eslint-plugin/src/rules/require-types-exports.ts +++ b/packages/eslint-plugin/src/rules/require-types-exports.ts @@ -19,8 +19,7 @@ export default createRule<[], MessageIds>({ docs: { recommended: 'strict', requiresTypeChecking: true, - description: - 'Require exporting types that are used in exported functions declarations', + description: 'Require exporting types that are used in exported entities', }, messages: { requireTypeExport: 'Expected type "{{ name }}" to be exported', From 26e7be7153ebb95c8f69040a9132f82472192ed2 Mon Sep 17 00:00:00 2001 From: StyleShit Date: Wed, 24 Apr 2024 12:47:42 +0300 Subject: [PATCH 19/36] add inferred return type test case --- .../tests/rules/require-types-exports.test.ts | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/packages/eslint-plugin/tests/rules/require-types-exports.test.ts b/packages/eslint-plugin/tests/rules/require-types-exports.test.ts index 6d05d62a244..63d36fd3fd1 100644 --- a/packages/eslint-plugin/tests/rules/require-types-exports.test.ts +++ b/packages/eslint-plugin/tests/rules/require-types-exports.test.ts @@ -291,6 +291,18 @@ ruleTester.run('require-types-exports', rule, { export function f(a: T): void {} `, + + ` + export type R = number; + + export function f() { + const value: { num: R } = { + num: 1, + }; + + return value; + } + `, ], invalid: [ @@ -1600,6 +1612,31 @@ ruleTester.run('require-types-exports', rule, { ], }, + { + code: ` + type R = number; + + export function f() { + const value: { num: R } = { + num: 1, + }; + + return value; + } + `, + errors: [ + { + messageId: 'requireTypeExport', + line: 5, + column: 31, + endColumn: 32, + data: { + name: 'R', + }, + }, + ], + }, + { code: ` type Arg1 = number; From e57985a6fdbd710c8deffd8ad34b52054e3b8422 Mon Sep 17 00:00:00 2001 From: StyleShit Date: Wed, 24 Apr 2024 16:21:43 +0300 Subject: [PATCH 20/36] stupidly check for variable types --- .../src/rules/require-types-exports.ts | 68 +++++++++++++++++++ .../tests/rules/require-types-exports.test.ts | 58 ++++++++++++++++ 2 files changed, 126 insertions(+) diff --git a/packages/eslint-plugin/src/rules/require-types-exports.ts b/packages/eslint-plugin/src/rules/require-types-exports.ts index e5cde3687c4..3a803a25f8a 100644 --- a/packages/eslint-plugin/src/rules/require-types-exports.ts +++ b/packages/eslint-plugin/src/rules/require-types-exports.ts @@ -1,3 +1,4 @@ +import type { Reference } from '@typescript-eslint/scope-manager'; import { DefinitionType } from '@typescript-eslint/scope-manager'; import type { TSESTree } from '@typescript-eslint/utils'; import { AST_NODE_TYPES } from '@typescript-eslint/utils'; @@ -12,6 +13,12 @@ type FunctionNode = | TSESTree.ArrowFunctionExpression | TSESTree.FunctionExpression; +type TypeReference = Reference & { + identifier: { + parent: TSESTree.TSTypeReference; + }; +}; + export default createRule<[], MessageIds>({ name: 'require-types-exports', meta: { @@ -28,9 +35,23 @@ export default createRule<[], MessageIds>({ }, defaultOptions: [], create(context) { + const typeReferences = new Set(); const externalizedTypes = new Set(); const reportedTypes = new Set(); + function collectTypeReferences(node: TSESTree.Program): void { + const scope = context.sourceCode.getScope(node); + + scope.references.forEach(r => { + if ( + r.resolved?.isTypeVariable && + r.identifier.parent.type === AST_NODE_TYPES.TSTypeReference + ) { + typeReferences.add(r as TypeReference); + } + }); + } + function collectImportedTypes(node: TSESTree.ImportSpecifier): void { externalizedTypes.add(node.local.name); } @@ -60,6 +81,8 @@ export default createRule<[], MessageIds>({ for (const declaration of node.declaration.declarations) { if (declaration.init?.type === AST_NODE_TYPES.ArrowFunctionExpression) { checkFunctionTypes(declaration.init); + } else { + checkVariableTypes(declaration); } } } @@ -107,6 +130,21 @@ export default createRule<[], MessageIds>({ .forEach(checkTypeNode); } + function checkVariableTypes( + node: TSESTree.LetOrConstOrVarDeclarator, + ): void { + if (node.id.type !== AST_NODE_TYPES.Identifier) { + return; + } + + typeReferences.forEach(r => { + // TODO: Probably not the best way to do it... + if (isLocationOverlapping(r.identifier.loc, node.loc)) { + checkTypeNode(r.identifier.parent); + } + }); + } + function checkTypeNode(node: TSESTree.TSTypeReference): void { const name = getTypeName(node); @@ -141,7 +179,37 @@ export default createRule<[], MessageIds>({ return ''; } + function isLocationOverlapping( + location: TSESTree.Node['loc'], + container: TSESTree.Node['loc'], + ): boolean { + if ( + location.start.line < container.start.line || + location.end.line > container.end.line + ) { + return false; + } + + if ( + location.start.line === container.start.line && + location.start.column < container.start.column + ) { + return false; + } + + if ( + location.end.line === container.end.line && + location.end.column > container.end.column + ) { + return false; + } + + return true; + } + return { + Program: collectTypeReferences, + 'ImportDeclaration ImportSpecifier, ImportSpecifier': collectImportedTypes, diff --git a/packages/eslint-plugin/tests/rules/require-types-exports.test.ts b/packages/eslint-plugin/tests/rules/require-types-exports.test.ts index 63d36fd3fd1..39f66ebf235 100644 --- a/packages/eslint-plugin/tests/rules/require-types-exports.test.ts +++ b/packages/eslint-plugin/tests/rules/require-types-exports.test.ts @@ -303,6 +303,24 @@ ruleTester.run('require-types-exports', rule, { return value; } `, + + ` + import type { A } from './types'; + + export type T1 = number; + + export interface T2 { + key: number; + } + + export const value: { a: { b: { c: T1 } } } | [string, T2 | A] = { + a: { + b: { + c: 1, + }, + }, + }; + `, ], invalid: [ @@ -1739,5 +1757,45 @@ ruleTester.run('require-types-exports', rule, { }, ], }, + + { + code: ` + import type { A } from './types'; + + type T1 = number; + + interface T2 { + key: number; + } + + export const value: { a: { b: { c: T1 } } } | [string, T2 | A] = { + a: { + b: { + c: 1, + }, + }, + }; + `, + errors: [ + { + messageId: 'requireTypeExport', + line: 10, + column: 44, + endColumn: 46, + data: { + name: 'T1', + }, + }, + { + messageId: 'requireTypeExport', + line: 10, + column: 64, + endColumn: 66, + data: { + name: 'T2', + }, + }, + ], + }, ], }); From cbb784c09a1aef1864d76c2cce72d2a0140d4fb6 Mon Sep 17 00:00:00 2001 From: StyleShit Date: Wed, 24 Apr 2024 18:09:19 +0300 Subject: [PATCH 21/36] support default exported variable --- .../src/rules/require-types-exports.ts | 15 ++--- .../tests/rules/require-types-exports.test.ts | 62 +++++++++++++++++++ 2 files changed, 70 insertions(+), 7 deletions(-) diff --git a/packages/eslint-plugin/src/rules/require-types-exports.ts b/packages/eslint-plugin/src/rules/require-types-exports.ts index 3a803a25f8a..62b64785dfb 100644 --- a/packages/eslint-plugin/src/rules/require-types-exports.ts +++ b/packages/eslint-plugin/src/rules/require-types-exports.ts @@ -108,12 +108,17 @@ export default createRule<[], MessageIds>({ } for (const def of variable.defs) { + if (def.type !== DefinitionType.Variable || !def.node.init) { + continue; + } + if ( - def.type === DefinitionType.Variable && - (def.node.init?.type === AST_NODE_TYPES.ArrowFunctionExpression || - def.node.init?.type === AST_NODE_TYPES.FunctionExpression) + def.node.init.type === AST_NODE_TYPES.ArrowFunctionExpression || + def.node.init.type === AST_NODE_TYPES.FunctionExpression ) { checkFunctionTypes(def.node.init); + } else { + checkVariableTypes(def.node); } } } @@ -133,10 +138,6 @@ export default createRule<[], MessageIds>({ function checkVariableTypes( node: TSESTree.LetOrConstOrVarDeclarator, ): void { - if (node.id.type !== AST_NODE_TYPES.Identifier) { - return; - } - typeReferences.forEach(r => { // TODO: Probably not the best way to do it... if (isLocationOverlapping(r.identifier.loc, node.loc)) { diff --git a/packages/eslint-plugin/tests/rules/require-types-exports.test.ts b/packages/eslint-plugin/tests/rules/require-types-exports.test.ts index 39f66ebf235..857cc404b71 100644 --- a/packages/eslint-plugin/tests/rules/require-types-exports.test.ts +++ b/packages/eslint-plugin/tests/rules/require-types-exports.test.ts @@ -321,6 +321,26 @@ ruleTester.run('require-types-exports', rule, { }, }; `, + + ` + import type { A } from './types'; + + export type T1 = number; + + export interface T2 { + key: number; + } + + const value: { a: { b: { c: T1 } } } | [string, T2 | A] = { + a: { + b: { + c: 1, + }, + }, + }; + + export default value; + `, ], invalid: [ @@ -1797,5 +1817,47 @@ ruleTester.run('require-types-exports', rule, { }, ], }, + + { + code: ` + import type { A } from './types'; + + type T1 = number; + + interface T2 { + key: number; + } + + const value: { a: { b: { c: T1 } } } | [string, T2 | A] = { + a: { + b: { + c: 1, + }, + }, + }; + + export default value; + `, + errors: [ + { + messageId: 'requireTypeExport', + line: 10, + column: 37, + endColumn: 39, + data: { + name: 'T1', + }, + }, + { + messageId: 'requireTypeExport', + line: 10, + column: 57, + endColumn: 59, + data: { + name: 'T2', + }, + }, + ], + }, ], }); From 6fb274af938403379ede6305350c5827ddedc879 Mon Sep 17 00:00:00 2001 From: StyleShit Date: Sun, 19 May 2024 21:43:16 +0300 Subject: [PATCH 22/36] update docs --- ...-types-exports.md => require-types-exports.mdx} | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) rename packages/eslint-plugin/docs/rules/{require-types-exports.md => require-types-exports.mdx} (90%) diff --git a/packages/eslint-plugin/docs/rules/require-types-exports.md b/packages/eslint-plugin/docs/rules/require-types-exports.mdx similarity index 90% rename from packages/eslint-plugin/docs/rules/require-types-exports.md rename to packages/eslint-plugin/docs/rules/require-types-exports.mdx index 9343536e6d7..baa939fc211 100644 --- a/packages/eslint-plugin/docs/rules/require-types-exports.md +++ b/packages/eslint-plugin/docs/rules/require-types-exports.mdx @@ -2,6 +2,9 @@ description: 'Require exporting types that are used in exported entities.' --- +import Tabs from '@theme/Tabs'; +import TabItem from '@theme/TabItem'; + > 🛑 This file is source code, not the primary documentation location! 🛑 > > See **https://typescript-eslint.io/rules/require-types-exports** for documentation. @@ -15,9 +18,8 @@ in order to extract the types from your code. ## Examples - - -### ❌ Incorrect + + ```ts type Arg = string; @@ -43,7 +45,8 @@ enum Color { export declare function getRandomColor(): Color; ``` -### ✅ Correct + + ```ts export type Arg = string; @@ -69,7 +72,8 @@ export enum Color { export declare function getRandomColor(): Color; ``` - + + ## When Not To Use It From 4672fe17f5137e35d597d874e420cd39e7b424f3 Mon Sep 17 00:00:00 2001 From: StyleShit Date: Sun, 19 May 2024 21:56:38 +0300 Subject: [PATCH 23/36] wip --- .../src/rules/require-types-exports.ts | 9 ++- .../require-types-exports.shot | 59 +++++++++++++++++++ .../tests/rules/require-types-exports.test.ts | 10 ++++ 3 files changed, 75 insertions(+), 3 deletions(-) create mode 100644 packages/eslint-plugin/tests/docs-eslint-output-snapshots/require-types-exports.shot diff --git a/packages/eslint-plugin/src/rules/require-types-exports.ts b/packages/eslint-plugin/src/rules/require-types-exports.ts index 62b64785dfb..bb764665623 100644 --- a/packages/eslint-plugin/src/rules/require-types-exports.ts +++ b/packages/eslint-plugin/src/rules/require-types-exports.ts @@ -24,9 +24,9 @@ export default createRule<[], MessageIds>({ meta: { type: 'suggestion', docs: { + description: 'Require exporting types that are used in exported entities', recommended: 'strict', requiresTypeChecking: true, - description: 'Require exporting types that are used in exported entities', }, messages: { requireTypeExport: 'Expected type "{{ name }}" to be exported', @@ -57,7 +57,10 @@ export default createRule<[], MessageIds>({ } function collectExportedTypes( - node: TSESTree.TSTypeAliasDeclaration | TSESTree.TSInterfaceDeclaration, + node: + | TSESTree.TSTypeAliasDeclaration + | TSESTree.TSInterfaceDeclaration + | TSESTree.TSEnumDeclaration, ): void { externalizedTypes.add(node.id.name); } @@ -214,7 +217,7 @@ export default createRule<[], MessageIds>({ 'ImportDeclaration ImportSpecifier, ImportSpecifier': collectImportedTypes, - 'ExportNamedDeclaration TSTypeAliasDeclaration, ExportNamedDeclaration TSInterfaceDeclaration': + 'ExportNamedDeclaration TSTypeAliasDeclaration, ExportNamedDeclaration TSInterfaceDeclaration, ExportNamedDeclaration TSEnumDeclaration': collectExportedTypes, 'ExportNamedDeclaration[declaration.type="FunctionDeclaration"]': diff --git a/packages/eslint-plugin/tests/docs-eslint-output-snapshots/require-types-exports.shot b/packages/eslint-plugin/tests/docs-eslint-output-snapshots/require-types-exports.shot new file mode 100644 index 00000000000..f515f012d36 --- /dev/null +++ b/packages/eslint-plugin/tests/docs-eslint-output-snapshots/require-types-exports.shot @@ -0,0 +1,59 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`Validating rule docs require-types-exports.mdx code examples ESLint output 1`] = ` +"Incorrect + +type Arg = string; +type Result = number; + +export function strLength(arg: Arg): Result { + ~~~ Expected type "Arg" to be exported + ~~~~~~ Expected type "Result" to be exported + return arg.length; +} + +interface Fruit { + name: string; + color: string; +} + +export const getFruitName = (fruit: Fruit) => fruit.name; + ~~~~~ Expected type "Fruit" to be exported + +enum Color { + Red = 'red', + Green = 'green', + Blue = 'blue', +} + +export declare function getRandomColor(): Color; + ~~~~~ Expected type "Color" to be exported +" +`; + +exports[`Validating rule docs require-types-exports.mdx code examples ESLint output 2`] = ` +"Correct + +export type Arg = string; +export type Result = number; + +export function strLength(arg: Arg): Result { + return arg.length; +} + +export interface Fruit { + name: string; + color: string; +} + +export const getFruitName = (fruit: Fruit) => fruit.name; + +export enum Color { + Red = 'red', + Green = 'green', + Blue = 'blue', +} + +export declare function getRandomColor(): Color; +" +`; diff --git a/packages/eslint-plugin/tests/rules/require-types-exports.test.ts b/packages/eslint-plugin/tests/rules/require-types-exports.test.ts index 857cc404b71..b2e8c2a3579 100644 --- a/packages/eslint-plugin/tests/rules/require-types-exports.test.ts +++ b/packages/eslint-plugin/tests/rules/require-types-exports.test.ts @@ -341,6 +341,16 @@ ruleTester.run('require-types-exports', rule, { export default value; `, + + ` + export enum Fruit { + Apple, + Banana, + Cherry, + } + + export function f(a: Fruit): void {} + `, ], invalid: [ From c79b5cbaa0d25d4164708eca04a61082b4785312 Mon Sep 17 00:00:00 2001 From: StyleShit Date: Sun, 19 May 2024 22:09:44 +0300 Subject: [PATCH 24/36] wip --- packages/eslint-plugin/src/configs/strict-type-checked-only.ts | 1 + packages/typescript-eslint/src/configs/all.ts | 1 + packages/typescript-eslint/src/configs/disable-type-checked.ts | 1 + .../typescript-eslint/src/configs/strict-type-checked-only.ts | 1 + packages/typescript-eslint/src/configs/strict-type-checked.ts | 1 + 5 files changed, 5 insertions(+) diff --git a/packages/eslint-plugin/src/configs/strict-type-checked-only.ts b/packages/eslint-plugin/src/configs/strict-type-checked-only.ts index 12709933dfb..d5b62fb4fb7 100644 --- a/packages/eslint-plugin/src/configs/strict-type-checked-only.ts +++ b/packages/eslint-plugin/src/configs/strict-type-checked-only.ts @@ -43,6 +43,7 @@ export = { '@typescript-eslint/prefer-return-this-type': 'error', 'require-await': 'off', '@typescript-eslint/require-await': 'error', + '@typescript-eslint/require-types-exports': 'error', '@typescript-eslint/restrict-plus-operands': [ 'error', { diff --git a/packages/typescript-eslint/src/configs/all.ts b/packages/typescript-eslint/src/configs/all.ts index efe0d16fceb..0579a6d7d89 100644 --- a/packages/typescript-eslint/src/configs/all.ts +++ b/packages/typescript-eslint/src/configs/all.ts @@ -153,6 +153,7 @@ export default ( '@typescript-eslint/require-array-sort-compare': 'error', 'require-await': 'off', '@typescript-eslint/require-await': 'error', + '@typescript-eslint/require-types-exports': 'error', '@typescript-eslint/restrict-plus-operands': 'error', '@typescript-eslint/restrict-template-expressions': 'error', 'no-return-await': 'off', diff --git a/packages/typescript-eslint/src/configs/disable-type-checked.ts b/packages/typescript-eslint/src/configs/disable-type-checked.ts index 8d2f29220ab..2c789c0df71 100644 --- a/packages/typescript-eslint/src/configs/disable-type-checked.ts +++ b/packages/typescript-eslint/src/configs/disable-type-checked.ts @@ -60,6 +60,7 @@ export default ( '@typescript-eslint/promise-function-async': 'off', '@typescript-eslint/require-array-sort-compare': 'off', '@typescript-eslint/require-await': 'off', + '@typescript-eslint/require-types-exports': 'off', '@typescript-eslint/restrict-plus-operands': 'off', '@typescript-eslint/restrict-template-expressions': 'off', '@typescript-eslint/return-await': 'off', diff --git a/packages/typescript-eslint/src/configs/strict-type-checked-only.ts b/packages/typescript-eslint/src/configs/strict-type-checked-only.ts index f17b5280ca4..765371534b6 100644 --- a/packages/typescript-eslint/src/configs/strict-type-checked-only.ts +++ b/packages/typescript-eslint/src/configs/strict-type-checked-only.ts @@ -52,6 +52,7 @@ export default ( '@typescript-eslint/prefer-return-this-type': 'error', 'require-await': 'off', '@typescript-eslint/require-await': 'error', + '@typescript-eslint/require-types-exports': 'error', '@typescript-eslint/restrict-plus-operands': [ 'error', { diff --git a/packages/typescript-eslint/src/configs/strict-type-checked.ts b/packages/typescript-eslint/src/configs/strict-type-checked.ts index ad62ee749e2..ddb7afd1db9 100644 --- a/packages/typescript-eslint/src/configs/strict-type-checked.ts +++ b/packages/typescript-eslint/src/configs/strict-type-checked.ts @@ -83,6 +83,7 @@ export default ( '@typescript-eslint/prefer-ts-expect-error': 'error', 'require-await': 'off', '@typescript-eslint/require-await': 'error', + '@typescript-eslint/require-types-exports': 'error', '@typescript-eslint/restrict-plus-operands': [ 'error', { From 279055a127fbfd01054d863f08dcf49cee4d5e34 Mon Sep 17 00:00:00 2001 From: StyleShit Date: Sun, 19 May 2024 22:10:47 +0300 Subject: [PATCH 25/36] wip --- packages/eslint-plugin/src/rules/require-types-exports.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/eslint-plugin/src/rules/require-types-exports.ts b/packages/eslint-plugin/src/rules/require-types-exports.ts index bb764665623..7b7d3924d30 100644 --- a/packages/eslint-plugin/src/rules/require-types-exports.ts +++ b/packages/eslint-plugin/src/rules/require-types-exports.ts @@ -26,7 +26,6 @@ export default createRule<[], MessageIds>({ docs: { description: 'Require exporting types that are used in exported entities', recommended: 'strict', - requiresTypeChecking: true, }, messages: { requireTypeExport: 'Expected type "{{ name }}" to be exported', From 2f81933fb23c0c6d937db7b1ebe9c39fc6690d56 Mon Sep 17 00:00:00 2001 From: StyleShit Date: Sun, 2 Jun 2024 22:14:30 +0300 Subject: [PATCH 26/36] improve types --- packages/eslint-plugin/src/rules/require-types-exports.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/eslint-plugin/src/rules/require-types-exports.ts b/packages/eslint-plugin/src/rules/require-types-exports.ts index 7b7d3924d30..9236274ca62 100644 --- a/packages/eslint-plugin/src/rules/require-types-exports.ts +++ b/packages/eslint-plugin/src/rules/require-types-exports.ts @@ -14,6 +14,7 @@ type FunctionNode = | TSESTree.FunctionExpression; type TypeReference = Reference & { + isTypeReference: true; identifier: { parent: TSESTree.TSTypeReference; }; @@ -44,7 +45,9 @@ export default createRule<[], MessageIds>({ scope.references.forEach(r => { if ( r.resolved?.isTypeVariable && - r.identifier.parent.type === AST_NODE_TYPES.TSTypeReference + r.identifier.type === AST_NODE_TYPES.Identifier && + r.identifier.parent.type === AST_NODE_TYPES.TSTypeReference && + r.isTypeReference ) { typeReferences.add(r as TypeReference); } From 0f788d2e38afb2c3f6f704c7ac0df8d693e6548e Mon Sep 17 00:00:00 2001 From: StyleShit Date: Sun, 2 Jun 2024 22:15:18 +0300 Subject: [PATCH 27/36] improve type reference search --- .../src/rules/require-types-exports.ts | 48 ++++++---------- .../tests/rules/require-types-exports.test.ts | 57 +++++++++++++++++++ 2 files changed, 75 insertions(+), 30 deletions(-) diff --git a/packages/eslint-plugin/src/rules/require-types-exports.ts b/packages/eslint-plugin/src/rules/require-types-exports.ts index 9236274ca62..abdad788019 100644 --- a/packages/eslint-plugin/src/rules/require-types-exports.ts +++ b/packages/eslint-plugin/src/rules/require-types-exports.ts @@ -144,13 +144,29 @@ export default createRule<[], MessageIds>({ node: TSESTree.LetOrConstOrVarDeclarator, ): void { typeReferences.forEach(r => { - // TODO: Probably not the best way to do it... - if (isLocationOverlapping(r.identifier.loc, node.loc)) { + if (isAncestorNode(node, r.identifier.parent)) { checkTypeNode(r.identifier.parent); } }); } + function isAncestorNode( + ancestor: TSESTree.Node, + node: TSESTree.Node, + ): boolean { + let parent = node.parent; + + while (parent) { + if (parent === ancestor) { + return true; + } + + parent = parent.parent; + } + + return false; + } + function checkTypeNode(node: TSESTree.TSTypeReference): void { const name = getTypeName(node); @@ -185,34 +201,6 @@ export default createRule<[], MessageIds>({ return ''; } - function isLocationOverlapping( - location: TSESTree.Node['loc'], - container: TSESTree.Node['loc'], - ): boolean { - if ( - location.start.line < container.start.line || - location.end.line > container.end.line - ) { - return false; - } - - if ( - location.start.line === container.start.line && - location.start.column < container.start.column - ) { - return false; - } - - if ( - location.end.line === container.end.line && - location.end.column > container.end.column - ) { - return false; - } - - return true; - } - return { Program: collectTypeReferences, diff --git a/packages/eslint-plugin/tests/rules/require-types-exports.test.ts b/packages/eslint-plugin/tests/rules/require-types-exports.test.ts index b2e8c2a3579..ec41d6d2e2a 100644 --- a/packages/eslint-plugin/tests/rules/require-types-exports.test.ts +++ b/packages/eslint-plugin/tests/rules/require-types-exports.test.ts @@ -1869,5 +1869,62 @@ ruleTester.run('require-types-exports', rule, { }, ], }, + + { + code: ` + type T1 = number; + + interface T2 { + key: number; + } + + type T3 = boolean; + + export const value: + | { + a: T1; + b: { + c: T2; + }; + } + | T3[] = { + a: 1, + b: { + c: { + key: 1, + }, + }, + }; + `, + errors: [ + { + messageId: 'requireTypeExport', + line: 12, + column: 18, + endColumn: 20, + data: { + name: 'T1', + }, + }, + { + messageId: 'requireTypeExport', + line: 14, + column: 20, + endColumn: 22, + data: { + name: 'T2', + }, + }, + { + messageId: 'requireTypeExport', + line: 17, + column: 13, + endColumn: 15, + data: { + name: 'T3', + }, + }, + ], + }, ], }); From 6cec0f56a80fdc10e57d240ac6cfd517efdec4ce Mon Sep 17 00:00:00 2001 From: StyleShit Date: Sun, 2 Jun 2024 22:42:13 +0300 Subject: [PATCH 28/36] don't report types from default library --- .../src/rules/require-types-exports.ts | 15 ++++++++++++++- .../tests/rules/require-types-exports.test.ts | 12 ++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/packages/eslint-plugin/src/rules/require-types-exports.ts b/packages/eslint-plugin/src/rules/require-types-exports.ts index abdad788019..a5af6ede8b5 100644 --- a/packages/eslint-plugin/src/rules/require-types-exports.ts +++ b/packages/eslint-plugin/src/rules/require-types-exports.ts @@ -3,7 +3,11 @@ import { DefinitionType } from '@typescript-eslint/scope-manager'; import type { TSESTree } from '@typescript-eslint/utils'; import { AST_NODE_TYPES } from '@typescript-eslint/utils'; -import { createRule } from '../util'; +import { + createRule, + getParserServices, + isSymbolFromDefaultLibrary, +} from '../util'; type MessageIds = 'requireTypeExport'; @@ -35,6 +39,8 @@ export default createRule<[], MessageIds>({ }, defaultOptions: [], create(context) { + const services = getParserServices(context); + const typeReferences = new Set(); const externalizedTypes = new Set(); const reportedTypes = new Set(); @@ -182,6 +188,13 @@ export default createRule<[], MessageIds>({ return; } + const tsNode = services.esTreeNodeToTSNodeMap.get(node); + const type = services.program.getTypeChecker().getTypeAtLocation(tsNode); + + if (isSymbolFromDefaultLibrary(services.program, type.getSymbol())) { + return; + } + context.report({ node: node, messageId: 'requireTypeExport', diff --git a/packages/eslint-plugin/tests/rules/require-types-exports.test.ts b/packages/eslint-plugin/tests/rules/require-types-exports.test.ts index ec41d6d2e2a..c02324e22ab 100644 --- a/packages/eslint-plugin/tests/rules/require-types-exports.test.ts +++ b/packages/eslint-plugin/tests/rules/require-types-exports.test.ts @@ -351,6 +351,18 @@ ruleTester.run('require-types-exports', rule, { export function f(a: Fruit): void {} `, + + ` + export function f(arg: Record>) { + return arg; + } + `, + + ` + export function f>>(arg: T) { + return arg; + } + `, ], invalid: [ From 497957a7f9df3a96285ea5a5c09231738a58cee6 Mon Sep 17 00:00:00 2001 From: StyleShit Date: Sun, 2 Jun 2024 23:21:47 +0300 Subject: [PATCH 29/36] getTypeName --- .../src/rules/require-types-exports.ts | 22 +++++++---- .../tests/rules/require-types-exports.test.ts | 39 +++++++++++++++++++ 2 files changed, 53 insertions(+), 8 deletions(-) diff --git a/packages/eslint-plugin/src/rules/require-types-exports.ts b/packages/eslint-plugin/src/rules/require-types-exports.ts index a5af6ede8b5..00a68f732ef 100644 --- a/packages/eslint-plugin/src/rules/require-types-exports.ts +++ b/packages/eslint-plugin/src/rules/require-types-exports.ts @@ -174,10 +174,11 @@ export default createRule<[], MessageIds>({ } function checkTypeNode(node: TSESTree.TSTypeReference): void { - const name = getTypeName(node); + const name = getTypeName(node.typeName); - if (!name) { - // TODO: Report the whole function? Is this case even possible? + // Using `this` type is allowed since it's necessarily exported + // if it's used in an exported entity. + if (name === 'this') { return; } @@ -206,12 +207,17 @@ export default createRule<[], MessageIds>({ reportedTypes.add(name); } - function getTypeName(typeReference: TSESTree.TSTypeReference): string { - if (typeReference.typeName.type === AST_NODE_TYPES.Identifier) { - return typeReference.typeName.name; - } + function getTypeName(typeName: TSESTree.EntityName): string { + switch (typeName.type) { + case AST_NODE_TYPES.Identifier: + return typeName.name; + + case AST_NODE_TYPES.TSQualifiedName: + return getTypeName(typeName.left) + '.' + typeName.right.name; - return ''; + case AST_NODE_TYPES.ThisExpression: + return 'this'; + } } return { diff --git a/packages/eslint-plugin/tests/rules/require-types-exports.test.ts b/packages/eslint-plugin/tests/rules/require-types-exports.test.ts index c02324e22ab..d7b597a4d9d 100644 --- a/packages/eslint-plugin/tests/rules/require-types-exports.test.ts +++ b/packages/eslint-plugin/tests/rules/require-types-exports.test.ts @@ -363,6 +363,22 @@ ruleTester.run('require-types-exports', rule, { return arg; } `, + + ` + export class Wrapper { + work(other: this) {} + } + `, + + ` + export namespace A { + export type B = number; + } + + export function a(arg: A.B) { + return arg; + } + `, ], invalid: [ @@ -1800,6 +1816,29 @@ ruleTester.run('require-types-exports', rule, { ], }, + { + code: ` + export namespace A { + type B = number; + } + + export function a(arg: A.B) { + return arg; + } + `, + errors: [ + { + messageId: 'requireTypeExport', + line: 6, + column: 32, + endColumn: 35, + data: { + name: 'A.B', + }, + }, + ], + }, + { code: ` import type { A } from './types'; From 700ff85cf3c74fd2cc3547328ea3e4af14d085e1 Mon Sep 17 00:00:00 2001 From: StyleShit Date: Tue, 4 Jun 2024 21:07:27 +0300 Subject: [PATCH 30/36] move utils out of the closure --- .../src/rules/require-types-exports.ts | 57 +++++++++---------- 1 file changed, 27 insertions(+), 30 deletions(-) diff --git a/packages/eslint-plugin/src/rules/require-types-exports.ts b/packages/eslint-plugin/src/rules/require-types-exports.ts index 00a68f732ef..02b2a224cd7 100644 --- a/packages/eslint-plugin/src/rules/require-types-exports.ts +++ b/packages/eslint-plugin/src/rules/require-types-exports.ts @@ -156,23 +156,6 @@ export default createRule<[], MessageIds>({ }); } - function isAncestorNode( - ancestor: TSESTree.Node, - node: TSESTree.Node, - ): boolean { - let parent = node.parent; - - while (parent) { - if (parent === ancestor) { - return true; - } - - parent = parent.parent; - } - - return false; - } - function checkTypeNode(node: TSESTree.TSTypeReference): void { const name = getTypeName(node.typeName); @@ -207,19 +190,6 @@ export default createRule<[], MessageIds>({ reportedTypes.add(name); } - function getTypeName(typeName: TSESTree.EntityName): string { - switch (typeName.type) { - case AST_NODE_TYPES.Identifier: - return typeName.name; - - case AST_NODE_TYPES.TSQualifiedName: - return getTypeName(typeName.left) + '.' + typeName.right.name; - - case AST_NODE_TYPES.ThisExpression: - return 'this'; - } - } - return { Program: collectTypeReferences, @@ -249,3 +219,30 @@ export default createRule<[], MessageIds>({ }; }, }); + +function getTypeName(typeName: TSESTree.EntityName): string { + switch (typeName.type) { + case AST_NODE_TYPES.Identifier: + return typeName.name; + + case AST_NODE_TYPES.TSQualifiedName: + return getTypeName(typeName.left) + '.' + typeName.right.name; + + case AST_NODE_TYPES.ThisExpression: + return 'this'; + } +} + +function isAncestorNode(ancestor: TSESTree.Node, node: TSESTree.Node): boolean { + let parent = node.parent; + + while (parent) { + if (parent === ancestor) { + return true; + } + + parent = parent.parent; + } + + return false; +} From 9a155b3058d37bc76fd41afb4c7e8d97aab471f9 Mon Sep 17 00:00:00 2001 From: StyleShit Date: Wed, 5 Jun 2024 21:32:33 +0300 Subject: [PATCH 31/36] support namespaced types --- .../src/rules/require-types-exports.ts | 51 +++++++++++++++---- .../tests/rules/require-types-exports.test.ts | 20 +++++--- 2 files changed, 52 insertions(+), 19 deletions(-) diff --git a/packages/eslint-plugin/src/rules/require-types-exports.ts b/packages/eslint-plugin/src/rules/require-types-exports.ts index 02b2a224cd7..fa5565d4533 100644 --- a/packages/eslint-plugin/src/rules/require-types-exports.ts +++ b/packages/eslint-plugin/src/rules/require-types-exports.ts @@ -137,13 +137,13 @@ export default createRule<[], MessageIds>({ function checkFunctionTypes(node: FunctionNode): void { const scope = context.sourceCode.getScope(node); - scope.through - .map(ref => ref.identifier.parent) - .filter( - (node): node is TSESTree.TSTypeReference => - node.type === AST_NODE_TYPES.TSTypeReference, - ) - .forEach(checkTypeNode); + scope.through.forEach(ref => { + const typeRef = findClosestTypeReference(ref.identifier, node); + + if (typeRef) { + checkTypeNode(typeRef); + } + }); } function checkVariableTypes( @@ -157,7 +157,7 @@ export default createRule<[], MessageIds>({ } function checkTypeNode(node: TSESTree.TSTypeReference): void { - const name = getTypeName(node.typeName); + let name = getTypeName(node.typeName); // Using `this` type is allowed since it's necessarily exported // if it's used in an exported entity. @@ -165,6 +165,12 @@ export default createRule<[], MessageIds>({ return; } + // Namespaced types are not exported directly, so we check the + // leftmost part of the name. + if (Array.isArray(name)) { + name = name[0]; + } + const isExternalized = externalizedTypes.has(name); const isReported = reportedTypes.has(name); @@ -196,7 +202,13 @@ export default createRule<[], MessageIds>({ 'ImportDeclaration ImportSpecifier, ImportSpecifier': collectImportedTypes, - 'ExportNamedDeclaration TSTypeAliasDeclaration, ExportNamedDeclaration TSInterfaceDeclaration, ExportNamedDeclaration TSEnumDeclaration': + 'Program > ExportNamedDeclaration > TSTypeAliasDeclaration': + collectExportedTypes, + 'Program > ExportNamedDeclaration > TSInterfaceDeclaration': + collectExportedTypes, + 'Program > ExportNamedDeclaration > TSEnumDeclaration': + collectExportedTypes, + 'Program > ExportNamedDeclaration > TSModuleDeclaration': collectExportedTypes, 'ExportNamedDeclaration[declaration.type="FunctionDeclaration"]': @@ -220,19 +232,36 @@ export default createRule<[], MessageIds>({ }, }); -function getTypeName(typeName: TSESTree.EntityName): string { +function getTypeName(typeName: TSESTree.EntityName): string | string[] { switch (typeName.type) { case AST_NODE_TYPES.Identifier: return typeName.name; case AST_NODE_TYPES.TSQualifiedName: - return getTypeName(typeName.left) + '.' + typeName.right.name; + return [...(getTypeName(typeName.left) || []), typeName.right.name]; case AST_NODE_TYPES.ThisExpression: return 'this'; } } +function findClosestTypeReference( + startNode: TSESTree.Node, + endNode: TSESTree.Node, +): TSESTree.TSTypeReference | null { + let parent = startNode.parent; + + while (parent && parent !== endNode) { + if (parent.type === AST_NODE_TYPES.TSTypeReference) { + return parent; + } + + parent = parent.parent; + } + + return null; +} + function isAncestorNode(ancestor: TSESTree.Node, node: TSESTree.Node): boolean { let parent = node.parent; diff --git a/packages/eslint-plugin/tests/rules/require-types-exports.test.ts b/packages/eslint-plugin/tests/rules/require-types-exports.test.ts index d7b597a4d9d..6a3008c3f29 100644 --- a/packages/eslint-plugin/tests/rules/require-types-exports.test.ts +++ b/packages/eslint-plugin/tests/rules/require-types-exports.test.ts @@ -372,10 +372,12 @@ ruleTester.run('require-types-exports', rule, { ` export namespace A { - export type B = number; + export namespace B { + export type C = number; + } } - export function a(arg: A.B) { + export function a(arg: A.B.C) { return arg; } `, @@ -1818,22 +1820,24 @@ ruleTester.run('require-types-exports', rule, { { code: ` - export namespace A { - type B = number; + namespace A { + export namespace B { + export type C = number; + } } - export function a(arg: A.B) { + export function a(arg: A.B.C) { return arg; } `, errors: [ { messageId: 'requireTypeExport', - line: 6, + line: 8, column: 32, - endColumn: 35, + endColumn: 37, data: { - name: 'A.B', + name: 'A', }, }, ], From b65f9c474efadcbd304ca0757c60dedd5e249879 Mon Sep 17 00:00:00 2001 From: StyleShit Date: Wed, 5 Jun 2024 21:54:23 +0300 Subject: [PATCH 32/36] fix namespaced imports --- .../src/rules/require-types-exports.ts | 14 ++++++++++---- .../tests/rules/require-types-exports.test.ts | 16 ++++++++++++++++ 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/packages/eslint-plugin/src/rules/require-types-exports.ts b/packages/eslint-plugin/src/rules/require-types-exports.ts index fa5565d4533..ae28c9d0371 100644 --- a/packages/eslint-plugin/src/rules/require-types-exports.ts +++ b/packages/eslint-plugin/src/rules/require-types-exports.ts @@ -199,8 +199,9 @@ export default createRule<[], MessageIds>({ return { Program: collectTypeReferences, - 'ImportDeclaration ImportSpecifier, ImportSpecifier': - collectImportedTypes, + 'ImportDeclaration ImportSpecifier': collectImportedTypes, + 'ImportDeclaration ImportNamespaceSpecifier': collectImportedTypes, + 'ImportDeclaration ImportDefaultSpecifier': collectImportedTypes, 'Program > ExportNamedDeclaration > TSTypeAliasDeclaration': collectExportedTypes, @@ -237,8 +238,13 @@ function getTypeName(typeName: TSESTree.EntityName): string | string[] { case AST_NODE_TYPES.Identifier: return typeName.name; - case AST_NODE_TYPES.TSQualifiedName: - return [...(getTypeName(typeName.left) || []), typeName.right.name]; + case AST_NODE_TYPES.TSQualifiedName: { + let left = getTypeName(typeName.left); + + left = Array.isArray(left) ? left : [left]; + + return [...left, typeName.right.name]; + } case AST_NODE_TYPES.ThisExpression: return 'this'; diff --git a/packages/eslint-plugin/tests/rules/require-types-exports.test.ts b/packages/eslint-plugin/tests/rules/require-types-exports.test.ts index 6a3008c3f29..86da0ca52e9 100644 --- a/packages/eslint-plugin/tests/rules/require-types-exports.test.ts +++ b/packages/eslint-plugin/tests/rules/require-types-exports.test.ts @@ -381,6 +381,22 @@ ruleTester.run('require-types-exports', rule, { return arg; } `, + + ` + import * as ts from 'typescript'; + + export function a(arg: ts.Type) { + return arg; + } + `, + + ` + import ts from 'typescript'; + + export function a(arg: ts.Type) { + return arg; + } + `, ], invalid: [ From 078e24afcc6d8695367738fd163dfc8e20916605 Mon Sep 17 00:00:00 2001 From: StyleShit Date: Wed, 5 Jun 2024 22:16:57 +0300 Subject: [PATCH 33/36] WIP --- packages/eslint-plugin/src/rules/require-types-exports.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/eslint-plugin/src/rules/require-types-exports.ts b/packages/eslint-plugin/src/rules/require-types-exports.ts index ae28c9d0371..a812151985e 100644 --- a/packages/eslint-plugin/src/rules/require-types-exports.ts +++ b/packages/eslint-plugin/src/rules/require-types-exports.ts @@ -60,7 +60,12 @@ export default createRule<[], MessageIds>({ }); } - function collectImportedTypes(node: TSESTree.ImportSpecifier): void { + function collectImportedTypes( + node: + | TSESTree.ImportSpecifier + | TSESTree.ImportNamespaceSpecifier + | TSESTree.ImportDefaultSpecifier, + ): void { externalizedTypes.add(node.local.name); } From ed23162a443c55cebcae9c716fd20e04973f1f98 Mon Sep 17 00:00:00 2001 From: StyleShit Date: Wed, 5 Jun 2024 22:45:56 +0300 Subject: [PATCH 34/36] wip --- .../src/rules/require-types-exports.ts | 27 +++++++------------ 1 file changed, 9 insertions(+), 18 deletions(-) diff --git a/packages/eslint-plugin/src/rules/require-types-exports.ts b/packages/eslint-plugin/src/rules/require-types-exports.ts index a812151985e..a3def00e15e 100644 --- a/packages/eslint-plugin/src/rules/require-types-exports.ts +++ b/packages/eslint-plugin/src/rules/require-types-exports.ts @@ -154,15 +154,15 @@ export default createRule<[], MessageIds>({ function checkVariableTypes( node: TSESTree.LetOrConstOrVarDeclarator, ): void { - typeReferences.forEach(r => { - if (isAncestorNode(node, r.identifier.parent)) { - checkTypeNode(r.identifier.parent); + typeReferences.forEach(ref => { + if (isAncestorNode(node, ref.identifier.parent)) { + checkTypeNode(ref.identifier.parent); } }); } function checkTypeNode(node: TSESTree.TSTypeReference): void { - let name = getTypeName(node.typeName); + const name = getTypeName(node.typeName); // Using `this` type is allowed since it's necessarily exported // if it's used in an exported entity. @@ -170,12 +170,6 @@ export default createRule<[], MessageIds>({ return; } - // Namespaced types are not exported directly, so we check the - // leftmost part of the name. - if (Array.isArray(name)) { - name = name[0]; - } - const isExternalized = externalizedTypes.has(name); const isReported = reportedTypes.has(name); @@ -238,18 +232,15 @@ export default createRule<[], MessageIds>({ }, }); -function getTypeName(typeName: TSESTree.EntityName): string | string[] { +function getTypeName(typeName: TSESTree.EntityName): string { switch (typeName.type) { case AST_NODE_TYPES.Identifier: return typeName.name; - case AST_NODE_TYPES.TSQualifiedName: { - let left = getTypeName(typeName.left); - - left = Array.isArray(left) ? left : [left]; - - return [...left, typeName.right.name]; - } + case AST_NODE_TYPES.TSQualifiedName: + // Namespaced types are not exported directly, so we check the + // leftmost part of the name. + return getTypeName(typeName.left); case AST_NODE_TYPES.ThisExpression: return 'this'; From ac224eb6107ba236204298b4a6e325bb9b61c7e5 Mon Sep 17 00:00:00 2001 From: StyleShit Date: Wed, 5 Jun 2024 22:46:07 +0300 Subject: [PATCH 35/36] fix propertykey tests --- packages/eslint-plugin/src/rules/require-types-exports.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/eslint-plugin/src/rules/require-types-exports.ts b/packages/eslint-plugin/src/rules/require-types-exports.ts index a3def00e15e..420ca23f811 100644 --- a/packages/eslint-plugin/src/rules/require-types-exports.ts +++ b/packages/eslint-plugin/src/rules/require-types-exports.ts @@ -179,8 +179,9 @@ export default createRule<[], MessageIds>({ const tsNode = services.esTreeNodeToTSNodeMap.get(node); const type = services.program.getTypeChecker().getTypeAtLocation(tsNode); + const symbol = type.aliasSymbol ?? type.getSymbol(); - if (isSymbolFromDefaultLibrary(services.program, type.getSymbol())) { + if (isSymbolFromDefaultLibrary(services.program, symbol)) { return; } From 417cc91ea799917a3c398805ecf16eef018cd765 Mon Sep 17 00:00:00 2001 From: StyleShit Date: Wed, 5 Jun 2024 22:55:34 +0300 Subject: [PATCH 36/36] ReturnType test --- .../eslint-plugin/tests/rules/require-types-exports.test.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/eslint-plugin/tests/rules/require-types-exports.test.ts b/packages/eslint-plugin/tests/rules/require-types-exports.test.ts index 86da0ca52e9..e58a6c55bc0 100644 --- a/packages/eslint-plugin/tests/rules/require-types-exports.test.ts +++ b/packages/eslint-plugin/tests/rules/require-types-exports.test.ts @@ -364,6 +364,12 @@ ruleTester.run('require-types-exports', rule, { } `, + ` + export function f string>>(arg: T) { + return arg; + } + `, + ` export class Wrapper { work(other: this) {}