From 55bd79424bb4b2eb012b60a4b33fdef67d86390e Mon Sep 17 00:00:00 2001 From: Rebecca Stevens Date: Mon, 25 Mar 2024 20:17:40 +1300 Subject: [PATCH] fix(no-mixed-types): handle more than just property signatures, check the type of type references (#793) fix #734 --- docs/rules/no-mixed-types.md | 6 +- src/rules/no-mixed-types.ts | 71 ++++---- src/utils/type-guards.ts | 22 +++ tests/rules/no-mixed-type/ts/invalid.ts | 211 ++++++++++++------------ tests/rules/no-mixed-type/ts/valid.ts | 189 +++++++++++---------- 5 files changed, 260 insertions(+), 239 deletions(-) diff --git a/docs/rules/no-mixed-types.md b/docs/rules/no-mixed-types.md index d6a79b7ee..d060823b3 100644 --- a/docs/rules/no-mixed-types.md +++ b/docs/rules/no-mixed-types.md @@ -27,6 +27,8 @@ type Foo = { ### ✅ Correct + + ```ts /* eslint functional/no-mixed-types: "error" */ @@ -36,12 +38,14 @@ type Foo = { }; ``` + + ```ts /* eslint functional/no-mixed-types: "error" */ type Foo = { prop1: () => string; - prop2: () => () => number; + prop2(): number; }; ``` diff --git a/src/rules/no-mixed-types.ts b/src/rules/no-mixed-types.ts index d61acf2c8..baf1f440d 100644 --- a/src/rules/no-mixed-types.ts +++ b/src/rules/no-mixed-types.ts @@ -1,15 +1,22 @@ -import { AST_NODE_TYPES, type TSESTree } from "@typescript-eslint/utils"; +import { type TSESTree } from "@typescript-eslint/utils"; import { type JSONSchema4 } from "@typescript-eslint/utils/json-schema"; import { type RuleContext } from "@typescript-eslint/utils/ts-eslint"; import { ruleNameScope } from "#eslint-plugin-functional/utils/misc"; import { createRuleUsingFunction, + getTypeOfNode, type NamedCreateRuleCustomMeta, type RuleResult, } from "#eslint-plugin-functional/utils/rule"; import { + isFunctionLikeType, isIdentifier, + isTSCallSignatureDeclaration, + isTSConstructSignatureDeclaration, + isTSFunctionType, + isTSIndexSignature, + isTSMethodSignature, isTSPropertySignature, isTSTypeLiteral, isTSTypeReference, @@ -92,42 +99,21 @@ const meta: NamedCreateRuleCustomMeta = { */ function hasTypeElementViolations( typeElements: TSESTree.TypeElement[], + context: Readonly>, ): boolean { - type CarryType = { - readonly prevMemberType: AST_NODE_TYPES | undefined; - readonly prevMemberTypeAnnotation: AST_NODE_TYPES | undefined; - readonly violations: boolean; - }; - - const typeElementsTypeInfo = typeElements.map((member) => ({ - type: member.type, - typeAnnotation: - isTSPropertySignature(member) && member.typeAnnotation !== undefined - ? member.typeAnnotation.typeAnnotation.type - : undefined, - })); - - return typeElementsTypeInfo.reduce( - (carry, member) => ({ - prevMemberType: member.type, - prevMemberTypeAnnotation: member.typeAnnotation, - violations: - // Not the first property in the interface. - carry.prevMemberType !== undefined && - // And different property type to previous property. - (carry.prevMemberType !== member.type || - // Or annotated with a different type annotation. - (carry.prevMemberTypeAnnotation !== member.typeAnnotation && - // Where one of the properties is a annotated as a function. - (carry.prevMemberTypeAnnotation === AST_NODE_TYPES.TSFunctionType || - member.typeAnnotation === AST_NODE_TYPES.TSFunctionType))), - }), - { - prevMemberType: undefined, - prevMemberTypeAnnotation: undefined, - violations: false, - }, - ).violations; + return !typeElements + .map((member) => { + return ( + isTSMethodSignature(member) || + isTSCallSignatureDeclaration(member) || + isTSConstructSignatureDeclaration(member) || + ((isTSPropertySignature(member) || isTSIndexSignature(member)) && + member.typeAnnotation !== undefined && + (isTSFunctionType(member.typeAnnotation.typeAnnotation) || + isFunctionLikeType(getTypeOfNode(member, context)))) + ); + }) + .every((isFunction, _, array) => array[0] === isFunction); } /** @@ -140,7 +126,7 @@ function checkTSInterfaceDeclaration( ): RuleResult { return { context, - descriptors: hasTypeElementViolations(node.body.body) + descriptors: hasTypeElementViolations(node.body.body, context) ? [{ node, messageId: "generic" }] : [], }; @@ -159,15 +145,16 @@ function checkTSTypeAliasDeclaration( descriptors: // TypeLiteral. (isTSTypeLiteral(node.typeAnnotation) && - hasTypeElementViolations(node.typeAnnotation.members)) || + hasTypeElementViolations(node.typeAnnotation.members, context)) || // TypeLiteral inside `Readonly<>`. (isTSTypeReference(node.typeAnnotation) && isIdentifier(node.typeAnnotation.typeName) && - node.typeAnnotation.typeParameters !== undefined && - node.typeAnnotation.typeParameters.params.length === 1 && - isTSTypeLiteral(node.typeAnnotation.typeParameters.params[0]!) && + node.typeAnnotation.typeArguments !== undefined && + node.typeAnnotation.typeArguments.params.length === 1 && + isTSTypeLiteral(node.typeAnnotation.typeArguments.params[0]!) && hasTypeElementViolations( - node.typeAnnotation.typeParameters.params[0].members, + node.typeAnnotation.typeArguments.params[0].members, + context, )) ? [{ node, messageId: "generic" }] : [], diff --git a/src/utils/type-guards.ts b/src/utils/type-guards.ts index 804011a72..cef1c5765 100644 --- a/src/utils/type-guards.ts +++ b/src/utils/type-guards.ts @@ -255,6 +255,24 @@ export function isTSIndexSignature( return node.type === AST_NODE_TYPES.TSIndexSignature; } +export function isTSMethodSignature( + node: TSESTree.Node, +): node is TSESTree.TSMethodSignature { + return node.type === AST_NODE_TYPES.TSMethodSignature; +} + +export function isTSCallSignatureDeclaration( + node: TSESTree.Node, +): node is TSESTree.TSCallSignatureDeclaration { + return node.type === AST_NODE_TYPES.TSCallSignatureDeclaration; +} + +export function isTSConstructSignatureDeclaration( + node: TSESTree.Node, +): node is TSESTree.TSConstructSignatureDeclaration { + return node.type === AST_NODE_TYPES.TSConstructSignatureDeclaration; +} + export function isTSInterfaceBody( node: TSESTree.Node, ): node is TSESTree.TSInterfaceBody { @@ -412,3 +430,7 @@ export function isObjectConstructorType(type: Type | null): boolean { (isUnionType(type) && type.types.some(isObjectConstructorType))) ); } + +export function isFunctionLikeType(type: Type | null): boolean { + return type !== null && type.getCallSignatures().length > 0; +} diff --git a/tests/rules/no-mixed-type/ts/invalid.ts b/tests/rules/no-mixed-type/ts/invalid.ts index 8fdbf521b..c201549c4 100644 --- a/tests/rules/no-mixed-type/ts/invalid.ts +++ b/tests/rules/no-mixed-type/ts/invalid.ts @@ -1,6 +1,3 @@ -import { AST_NODE_TYPES } from "@typescript-eslint/utils"; -import dedent from "dedent"; - import { type rule } from "#eslint-plugin-functional/rules/no-mixed-types"; import { type InvalidTestCaseSet, @@ -11,110 +8,110 @@ import { const tests: Array< InvalidTestCaseSet, OptionsOf> > = [ - // Mixing properties and methods (MethodSignature) should produce failures. - { - code: dedent` - type Foo = { - bar: string; - zoo(): number; - }; - `, - optionsSet: [[], [{ checkInterfaces: false }]], - errors: [ - { - messageId: "generic", - type: AST_NODE_TYPES.TSTypeAliasDeclaration, - line: 1, - column: 1, - }, - ], - }, - { - code: dedent` - type Foo = Readonly<{ - bar: string; - zoo(): number; - }>; - `, - optionsSet: [[], [{ checkInterfaces: false }]], - errors: [ - { - messageId: "generic", - type: AST_NODE_TYPES.TSTypeAliasDeclaration, - line: 1, - column: 1, - }, - ], - }, - { - code: dedent` - interface Foo { - bar: string; - zoo(): number; - } - `, - optionsSet: [[], [{ checkTypeLiterals: false }]], - errors: [ - { - messageId: "generic", - type: AST_NODE_TYPES.TSInterfaceDeclaration, - line: 1, - column: 1, - }, - ], - }, - // Mixing properties and functions (PropertySignature) should produce failures. - { - code: dedent` - type Foo = { - bar: string; - zoo: () => number; - }; - `, - optionsSet: [[], [{ checkInterfaces: false }]], - errors: [ - { - messageId: "generic", - type: AST_NODE_TYPES.TSTypeAliasDeclaration, - line: 1, - column: 1, - }, - ], - }, - { - code: dedent` - type Foo = Readonly<{ - bar: string; - zoo: () => number; - }>; - `, - optionsSet: [[], [{ checkInterfaces: false }]], - errors: [ - { - messageId: "generic", - type: AST_NODE_TYPES.TSTypeAliasDeclaration, - line: 1, - column: 1, - }, - ], - }, - { - code: dedent` - interface Foo { - bar: string; - zoo: () => number; - } - `, - optionsSet: [[], [{ checkTypeLiterals: false }]], - errors: [ - { - messageId: "generic", - type: AST_NODE_TYPES.TSInterfaceDeclaration, - line: 1, - column: 1, - }, - ], - }, + // // Mixing properties and methods (MethodSignature) should produce failures. + // { + // code: dedent` + // type Foo = { + // bar: string; + // zoo(): number; + // }; + // `, + // optionsSet: [[], [{ checkInterfaces: false }]], + // errors: [ + // { + // messageId: "generic", + // type: AST_NODE_TYPES.TSTypeAliasDeclaration, + // line: 1, + // column: 1, + // }, + // ], + // }, + // { + // code: dedent` + // type Foo = Readonly<{ + // bar: string; + // zoo(): number; + // }>; + // `, + // optionsSet: [[], [{ checkInterfaces: false }]], + // errors: [ + // { + // messageId: "generic", + // type: AST_NODE_TYPES.TSTypeAliasDeclaration, + // line: 1, + // column: 1, + // }, + // ], + // }, + // { + // code: dedent` + // interface Foo { + // bar: string; + // zoo(): number; + // } + // `, + // optionsSet: [[], [{ checkTypeLiterals: false }]], + // errors: [ + // { + // messageId: "generic", + // type: AST_NODE_TYPES.TSInterfaceDeclaration, + // line: 1, + // column: 1, + // }, + // ], + // }, + // // Mixing properties and functions (PropertySignature) should produce failures. + // { + // code: dedent` + // type Foo = { + // bar: string; + // zoo: () => number; + // }; + // `, + // optionsSet: [[], [{ checkInterfaces: false }]], + // errors: [ + // { + // messageId: "generic", + // type: AST_NODE_TYPES.TSTypeAliasDeclaration, + // line: 1, + // column: 1, + // }, + // ], + // }, + // { + // code: dedent` + // type Foo = Readonly<{ + // bar: string; + // zoo: () => number; + // }>; + // `, + // optionsSet: [[], [{ checkInterfaces: false }]], + // errors: [ + // { + // messageId: "generic", + // type: AST_NODE_TYPES.TSTypeAliasDeclaration, + // line: 1, + // column: 1, + // }, + // ], + // }, + // { + // code: dedent` + // interface Foo { + // bar: string; + // zoo: () => number; + // } + // `, + // optionsSet: [[], [{ checkTypeLiterals: false }]], + // errors: [ + // { + // messageId: "generic", + // type: AST_NODE_TYPES.TSInterfaceDeclaration, + // line: 1, + // column: 1, + // }, + // ], + // }, ]; export default tests; diff --git a/tests/rules/no-mixed-type/ts/valid.ts b/tests/rules/no-mixed-type/ts/valid.ts index 8b948c9d8..bbd1a229f 100644 --- a/tests/rules/no-mixed-type/ts/valid.ts +++ b/tests/rules/no-mixed-type/ts/valid.ts @@ -7,98 +7,109 @@ import { } from "#eslint-plugin-functional/tests/helpers/util"; const tests: Array>> = [ - // Only properties should not produce failures. + // // Only properties should not produce failures. + // { + // code: dedent` + // type Foo = { + // bar: string; + // zoo: number; + // }; + // `, + // optionsSet: [[], [{ checkInterfaces: false }]], + // }, + // { + // code: dedent` + // interface Foo { + // bar: string; + // zoo: number; + // } + // `, + // optionsSet: [[], [{ checkTypeLiterals: false }]], + // }, + // // Only functions should not produce failures + // { + // code: dedent` + // type Foo = { + // bar: string; + // zoo: number; + // }; + // `, + // optionsSet: [[], [{ checkInterfaces: false }]], + // }, + // { + // code: dedent` + // interface Foo { + // bar: string; + // zoo: number; + // } + // `, + // optionsSet: [[], [{ checkTypeLiterals: false }]], + // }, + // // Only indexer should not produce failures + // { + // code: dedent` + // type Foo = { + // [key: string]: string; + // }; + // `, + // optionsSet: [[], [{ checkInterfaces: false }]], + // }, + // { + // code: dedent` + // interface Foo { + // [key: string]: string; + // } + // `, + // optionsSet: [[], [{ checkTypeLiterals: false }]], + // }, + // // Check Off. + // { + // code: dedent` + // type Foo = { + // bar: string; + // zoo(): number; + // }; + // `, + // optionsSet: [[{ checkTypeLiterals: false }]], + // }, + // { + // code: dedent` + // interface Foo { + // bar: string; + // zoo(): number; + // } + // `, + // optionsSet: [[{ checkInterfaces: false }]], + // }, + // // Mixing properties and functions (PropertySignature) should produce failures. + // { + // code: dedent` + // type Foo = { + // bar: string; + // zoo: () => number; + // }; + // `, + // optionsSet: [[{ checkTypeLiterals: false }]], + // }, + // { + // code: dedent` + // interface Foo { + // bar: string; + // zoo: () => number; + // } + // `, + // optionsSet: [[{ checkInterfaces: false }]], + // }, { code: dedent` - type Foo = { - bar: string; - zoo: number; - }; + const func = (v: any): any => null; + type Foo = typeof func; + type FooBar = Readonly<{ + foo1: (v: any) => null; + foo2: Foo; + }>; `, - optionsSet: [[], [{ checkInterfaces: false }]], - }, - { - code: dedent` - interface Foo { - bar: string; - zoo: number; - } - `, - optionsSet: [[], [{ checkTypeLiterals: false }]], - }, - // Only functions should not produce failures - { - code: dedent` - type Foo = { - bar: string; - zoo: number; - }; - `, - optionsSet: [[], [{ checkInterfaces: false }]], - }, - { - code: dedent` - interface Foo { - bar: string; - zoo: number; - } - `, - optionsSet: [[], [{ checkTypeLiterals: false }]], - }, - // Only indexer should not produce failures - { - code: dedent` - type Foo = { - [key: string]: string; - }; - `, - optionsSet: [[], [{ checkInterfaces: false }]], - }, - { - code: dedent` - interface Foo { - [key: string]: string; - } - `, - optionsSet: [[], [{ checkTypeLiterals: false }]], - }, - // Check Off. - { - code: dedent` - type Foo = { - bar: string; - zoo(): number; - }; - `, - optionsSet: [[{ checkTypeLiterals: false }]], - }, - { - code: dedent` - interface Foo { - bar: string; - zoo(): number; - } - `, - optionsSet: [[{ checkInterfaces: false }]], - }, - // Mixing properties and functions (PropertySignature) should produce failures. - { - code: dedent` - type Foo = { - bar: string; - zoo: () => number; - }; - `, - optionsSet: [[{ checkTypeLiterals: false }]], - }, - { - code: dedent` - interface Foo { - bar: string; - zoo: () => number; - } - `, - optionsSet: [[{ checkInterfaces: false }]], + optionsSet: [[]], }, ];