diff --git a/packages/eslint-plugin/docs/rules/related-getter-setter-pairs.mdx b/packages/eslint-plugin/docs/rules/related-getter-setter-pairs.mdx new file mode 100644 index 000000000000..d709faf5498e --- /dev/null +++ b/packages/eslint-plugin/docs/rules/related-getter-setter-pairs.mdx @@ -0,0 +1,61 @@ +--- +description: 'Enforce that `get()` types should be assignable to their equivalent `set()` type.' +--- + +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/related-getter-setter-pairs** for documentation. + +TypeScript allows defining different types for a `get` parameter and its corresponding `set` return. +Prior to TypeScript 4.3, the types had to be identical. +From TypeScript 4.3 to 5.0, the `get` type had to be a subtype of the `set` type. +As of TypeScript 5.1, the types may be completely unrelated as long as there is an explicit type annotation. + +Defining drastically different types for a `get` and `set` pair can be confusing. +It means that assigning a property to itself would not work: + +```ts +// Assumes box.value's get() return is assignable to its set() parameter +box.value = box.value; +``` + +This rule reports cases where a `get()` and `set()` have the same name, but the `get()`'s type is not assignable to the `set()`'s. + +## Examples + + + + +```ts +interface Box { + get value(): string; + set value(newValue: number); +} +``` + + + + +```ts +interface Box { + get value(): string; + set value(newValue: string); +} +``` + + + + +## When Not To Use It + +If your project needs to model unusual relationships between data, such as older DOM types, this rule may not be useful for you. +You might consider using [ESLint disable comments](https://eslint.org/docs/latest/use/configure/rules#using-configuration-comments-1) for those specific situations instead of completely disabling this rule. + +## Further Reading + +- [MDN documentation on `get`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Functions/get) +- [MDN documentation on `set`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Functions/set) +- [TypeScript 5.1 Release Notes > Unrelated Types for Getters and Setters](https://www.typescriptlang.org/docs/handbook/release-notes/typescript-5-1.html#unrelated-types-for-getters-and-setters) diff --git a/packages/eslint-plugin/src/configs/all.ts b/packages/eslint-plugin/src/configs/all.ts index 107f369260ec..1496d3af0f72 100644 --- a/packages/eslint-plugin/src/configs/all.ts +++ b/packages/eslint-plugin/src/configs/all.ts @@ -142,6 +142,7 @@ export = { '@typescript-eslint/prefer-return-this-type': 'error', '@typescript-eslint/prefer-string-starts-ends-with': 'error', '@typescript-eslint/promise-function-async': 'error', + '@typescript-eslint/related-getter-setter-pairs': 'error', '@typescript-eslint/require-array-sort-compare': 'error', 'require-await': 'off', '@typescript-eslint/require-await': 'error', diff --git a/packages/eslint-plugin/src/configs/disable-type-checked.ts b/packages/eslint-plugin/src/configs/disable-type-checked.ts index 7cf867b382f2..bbbafc868982 100644 --- a/packages/eslint-plugin/src/configs/disable-type-checked.ts +++ b/packages/eslint-plugin/src/configs/disable-type-checked.ts @@ -56,6 +56,7 @@ export = { '@typescript-eslint/prefer-return-this-type': 'off', '@typescript-eslint/prefer-string-starts-ends-with': 'off', '@typescript-eslint/promise-function-async': 'off', + '@typescript-eslint/related-getter-setter-pairs': 'off', '@typescript-eslint/require-array-sort-compare': 'off', '@typescript-eslint/require-await': 'off', '@typescript-eslint/restrict-plus-operands': 'off', 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 8bfba2276c9d..c235c02e7b81 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-promise-reject-errors': 'error', '@typescript-eslint/prefer-reduce-type-parameter': 'error', '@typescript-eslint/prefer-return-this-type': 'error', + '@typescript-eslint/related-getter-setter-pairs': 'error', 'require-await': 'off', '@typescript-eslint/require-await': 'error', '@typescript-eslint/restrict-plus-operands': [ diff --git a/packages/eslint-plugin/src/configs/strict-type-checked.ts b/packages/eslint-plugin/src/configs/strict-type-checked.ts index 4a5c7adfc93e..b00f5eb0fc14 100644 --- a/packages/eslint-plugin/src/configs/strict-type-checked.ts +++ b/packages/eslint-plugin/src/configs/strict-type-checked.ts @@ -76,6 +76,7 @@ export = { '@typescript-eslint/prefer-promise-reject-errors': 'error', '@typescript-eslint/prefer-reduce-type-parameter': 'error', '@typescript-eslint/prefer-return-this-type': 'error', + '@typescript-eslint/related-getter-setter-pairs': 'error', 'require-await': 'off', '@typescript-eslint/require-await': 'error', '@typescript-eslint/restrict-plus-operands': [ @@ -93,10 +94,10 @@ export = { { allowAny: false, allowBoolean: false, + allowNever: false, allowNullish: false, allowNumber: false, allowRegExp: false, - allowNever: false, }, ], 'no-return-await': 'off', diff --git a/packages/eslint-plugin/src/rules/index.ts b/packages/eslint-plugin/src/rules/index.ts index 72c62f3e0122..776e593ec680 100644 --- a/packages/eslint-plugin/src/rules/index.ts +++ b/packages/eslint-plugin/src/rules/index.ts @@ -114,6 +114,7 @@ import preferReturnThisType from './prefer-return-this-type'; import preferStringStartsEndsWith from './prefer-string-starts-ends-with'; import preferTsExpectError from './prefer-ts-expect-error'; import promiseFunctionAsync from './promise-function-async'; +import relatedGetterSetterPairs from './related-getter-setter-pairs'; import requireArraySortCompare from './require-array-sort-compare'; import requireAwait from './require-await'; import restrictPlusOperands from './restrict-plus-operands'; @@ -244,6 +245,7 @@ const rules = { 'prefer-string-starts-ends-with': preferStringStartsEndsWith, 'prefer-ts-expect-error': preferTsExpectError, 'promise-function-async': promiseFunctionAsync, + 'related-getter-setter-pairs': relatedGetterSetterPairs, 'require-array-sort-compare': requireArraySortCompare, 'require-await': requireAwait, 'restrict-plus-operands': restrictPlusOperands, diff --git a/packages/eslint-plugin/src/rules/related-getter-setter-pairs.ts b/packages/eslint-plugin/src/rules/related-getter-setter-pairs.ts new file mode 100644 index 000000000000..89019b1d0a03 --- /dev/null +++ b/packages/eslint-plugin/src/rules/related-getter-setter-pairs.ts @@ -0,0 +1,113 @@ +import type { TSESTree } from '@typescript-eslint/utils'; + +import { AST_NODE_TYPES } from '@typescript-eslint/utils'; + +import { createRule, getNameFromMember, getParserServices } from '../util'; + +type Method = TSESTree.MethodDefinition | TSESTree.TSMethodSignature; + +type GetMethod = { + kind: 'get'; + returnType: TSESTree.TSTypeAnnotation; +} & Method; + +type GetMethodRaw = { + returnType: TSESTree.TSTypeAnnotation | undefined; +} & GetMethod; + +type SetMethod = { kind: 'set'; params: [TSESTree.Node] } & Method; + +interface MethodPair { + get?: GetMethod; + set?: SetMethod; +} + +export default createRule({ + name: 'related-getter-setter-pairs', + meta: { + type: 'problem', + docs: { + description: + 'Enforce that `get()` types should be assignable to their equivalent `set()` type', + recommended: 'strict', + requiresTypeChecking: true, + }, + messages: { + mismatch: + '`get()` type should be assignable to its equivalent `set()` type.', + }, + schema: [], + }, + defaultOptions: [], + create(context) { + const services = getParserServices(context); + const checker = services.program.getTypeChecker(); + const methodPairsStack: Map[] = []; + + function addPropertyNode( + member: GetMethod | SetMethod, + inner: TSESTree.Node, + kind: 'get' | 'set', + ): void { + const methodPairs = methodPairsStack[methodPairsStack.length - 1]; + const { name } = getNameFromMember(member, context.sourceCode); + + methodPairs.set(name, { + ...methodPairs.get(name), + [kind]: inner, + }); + } + + return { + ':matches(ClassBody, TSInterfaceBody, TSTypeLiteral):exit'(): void { + const methodPairs = methodPairsStack[methodPairsStack.length - 1]; + + for (const pair of methodPairs.values()) { + if (!pair.get || !pair.set) { + continue; + } + + const getter = pair.get; + + const getType = services.getTypeAtLocation(getter); + const setType = services.getTypeAtLocation(pair.set.params[0]); + + if (!checker.isTypeAssignableTo(getType, setType)) { + context.report({ + node: getter.returnType.typeAnnotation, + messageId: 'mismatch', + }); + } + } + + methodPairsStack.pop(); + }, + ':matches(MethodDefinition, TSMethodSignature)[kind=get]'( + node: GetMethodRaw, + ): void { + const getter = getMethodFromNode(node); + + if (getter.returnType) { + addPropertyNode(node, getter, 'get'); + } + }, + ':matches(MethodDefinition, TSMethodSignature)[kind=set]'( + node: SetMethod, + ): void { + const setter = getMethodFromNode(node); + + if (setter.params.length === 1) { + addPropertyNode(node, setter, 'set'); + } + }, + + 'ClassBody, TSInterfaceBody, TSTypeLiteral'(): void { + methodPairsStack.push(new Map()); + }, + }; + }, +}); + +function getMethodFromNode(node: GetMethodRaw | SetMethod) { + return node.type === AST_NODE_TYPES.TSMethodSignature ? node : node.value; +} diff --git a/packages/eslint-plugin/tests/docs-eslint-output-snapshots/related-getter-setter-pairs.shot b/packages/eslint-plugin/tests/docs-eslint-output-snapshots/related-getter-setter-pairs.shot new file mode 100644 index 000000000000..f4d43a5d3fac --- /dev/null +++ b/packages/eslint-plugin/tests/docs-eslint-output-snapshots/related-getter-setter-pairs.shot @@ -0,0 +1,22 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`Validating rule docs related-getter-setter-pairs.mdx code examples ESLint output 1`] = ` +"Incorrect + +interface Box { + get value(): string; + ~~~~~~ \`get()\` type should be assignable to its equivalent \`set()\` type. + set value(newValue: number); +} +" +`; + +exports[`Validating rule docs related-getter-setter-pairs.mdx code examples ESLint output 2`] = ` +"Correct + +interface Box { + get value(): string; + set value(newValue: string); +} +" +`; diff --git a/packages/eslint-plugin/tests/rules/related-getter-setter-pairs.test.ts b/packages/eslint-plugin/tests/rules/related-getter-setter-pairs.test.ts new file mode 100644 index 000000000000..ec7e22385052 --- /dev/null +++ b/packages/eslint-plugin/tests/rules/related-getter-setter-pairs.test.ts @@ -0,0 +1,253 @@ +import { RuleTester } from '@typescript-eslint/rule-tester'; + +import rule from '../../src/rules/related-getter-setter-pairs'; +import { getFixturesRootDir } from '../RuleTester'; + +const rootDir = getFixturesRootDir(); +const ruleTester = new RuleTester({ + languageOptions: { + parserOptions: { + project: './tsconfig.json', + tsconfigRootDir: rootDir, + }, + }, +}); + +ruleTester.run('related-getter-setter-pairs', rule, { + valid: [ + ` +interface Example { + get value(): string; + set value(newValue: string); +} + `, + ` +interface Example { + get value(): string | undefined; + set value(); +} + `, + ` +interface Example { + get value(): string | undefined; + set value(newValue: string, invalid: string); +} + `, + ` +interface Example { + get value(): string; + set value(newValue: string | undefined); +} + `, + ` +interface Example { + get value(): number; +} + `, + ` +interface Example { + get value(): number; + set value(); +} + `, + ` +interface Example { + set value(newValue: string); +} + `, + ` +interface Example { + set value(); +} + `, + ` +type Example = { + get value(); +}; + `, + ` +type Example = { + set value(); +}; + `, + ` +class Example { + get value() { + return ''; + } +} + `, + ` +class Example { + get value() { + return ''; + } + set value() {} +} + `, + ` +class Example { + get value() { + return ''; + } + set value(param) {} +} + `, + ` +class Example { + get value() { + return ''; + } + set value(param: number) {} +} + `, + ` +class Example { + set value() {} +} + `, + ` +type Example = { + get value(): number; + set value(newValue: number); +}; + `, + ], + + invalid: [ + { + code: ` +interface Example { + get value(): string | undefined; + set value(newValue: string); +} + `, + errors: [ + { + column: 16, + endColumn: 34, + endLine: 3, + line: 3, + messageId: 'mismatch', + }, + ], + }, + { + code: ` +interface Example { + get value(): number; + set value(newValue: string); +} + `, + errors: [ + { + column: 16, + endColumn: 22, + endLine: 3, + line: 3, + messageId: 'mismatch', + }, + ], + }, + { + code: ` +type Example = { + get value(): number; + set value(newValue: string); +}; + `, + errors: [ + { + column: 16, + endColumn: 22, + endLine: 3, + line: 3, + messageId: 'mismatch', + }, + ], + }, + { + code: ` +class Example { + get value(): boolean { + return true; + } + set value(newValue: string) {} +} + `, + errors: [ + { + column: 16, + endColumn: 23, + endLine: 3, + line: 3, + messageId: 'mismatch', + }, + ], + }, + { + code: ` +type GetType = { a: string; b: string }; + +declare class Foo { + get a(): GetType; + + set a(x: { c: string }); +} + `, + errors: [ + { + column: 12, + endColumn: 19, + endLine: 5, + line: 5, + messageId: 'mismatch', + }, + ], + }, + { + code: ` +type GetType = { a: string; b: string }; + +type SetTypeUnused = { c: string }; + +declare class Foo { + get a(): GetType; + + set a(x: { c: string }); +} + `, + errors: [ + { + column: 12, + endColumn: 19, + endLine: 7, + line: 7, + messageId: 'mismatch', + }, + ], + }, + { + code: ` +type GetType = { a: string; b: string }; + +type SetType = { c: string }; + +declare class Foo { + get a(): GetType; + + set a(x: SetType); +} + `, + errors: [ + { + column: 12, + endColumn: 19, + endLine: 7, + line: 7, + messageId: 'mismatch', + }, + ], + }, + ], +}); diff --git a/packages/eslint-plugin/tests/schema-snapshots/related-getter-setter-pairs.shot b/packages/eslint-plugin/tests/schema-snapshots/related-getter-setter-pairs.shot new file mode 100644 index 000000000000..2284af742d87 --- /dev/null +++ b/packages/eslint-plugin/tests/schema-snapshots/related-getter-setter-pairs.shot @@ -0,0 +1,14 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`Rule schemas should be convertible to TS types for documentation purposes related-getter-setter-pairs 1`] = ` +" +# SCHEMA: + +[] + + +# TYPES: + +/** No options declared */ +type Options = [];" +`; diff --git a/packages/typescript-eslint/src/configs/all.ts b/packages/typescript-eslint/src/configs/all.ts index 1c177f1943bf..2f5a581dca4c 100644 --- a/packages/typescript-eslint/src/configs/all.ts +++ b/packages/typescript-eslint/src/configs/all.ts @@ -156,6 +156,7 @@ export default ( '@typescript-eslint/prefer-return-this-type': 'error', '@typescript-eslint/prefer-string-starts-ends-with': 'error', '@typescript-eslint/promise-function-async': 'error', + '@typescript-eslint/related-getter-setter-pairs': 'error', '@typescript-eslint/require-array-sort-compare': 'error', 'require-await': 'off', '@typescript-eslint/require-await': 'error', diff --git a/packages/typescript-eslint/src/configs/disable-type-checked.ts b/packages/typescript-eslint/src/configs/disable-type-checked.ts index b4c2afd20ec8..94b694a0fc28 100644 --- a/packages/typescript-eslint/src/configs/disable-type-checked.ts +++ b/packages/typescript-eslint/src/configs/disable-type-checked.ts @@ -63,6 +63,7 @@ export default ( '@typescript-eslint/prefer-return-this-type': 'off', '@typescript-eslint/prefer-string-starts-ends-with': 'off', '@typescript-eslint/promise-function-async': 'off', + '@typescript-eslint/related-getter-setter-pairs': 'off', '@typescript-eslint/require-array-sort-compare': 'off', '@typescript-eslint/require-await': 'off', '@typescript-eslint/restrict-plus-operands': '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 15a4d035ae3c..c9efc88da6c3 100644 --- a/packages/typescript-eslint/src/configs/strict-type-checked-only.ts +++ b/packages/typescript-eslint/src/configs/strict-type-checked-only.ts @@ -56,6 +56,7 @@ export default ( '@typescript-eslint/prefer-promise-reject-errors': 'error', '@typescript-eslint/prefer-reduce-type-parameter': 'error', '@typescript-eslint/prefer-return-this-type': 'error', + '@typescript-eslint/related-getter-setter-pairs': 'error', 'require-await': 'off', '@typescript-eslint/require-await': 'error', '@typescript-eslint/restrict-plus-operands': [ diff --git a/packages/typescript-eslint/src/configs/strict-type-checked.ts b/packages/typescript-eslint/src/configs/strict-type-checked.ts index 99e4b05d303b..4f354baf7e40 100644 --- a/packages/typescript-eslint/src/configs/strict-type-checked.ts +++ b/packages/typescript-eslint/src/configs/strict-type-checked.ts @@ -89,6 +89,7 @@ export default ( '@typescript-eslint/prefer-promise-reject-errors': 'error', '@typescript-eslint/prefer-reduce-type-parameter': 'error', '@typescript-eslint/prefer-return-this-type': 'error', + '@typescript-eslint/related-getter-setter-pairs': 'error', 'require-await': 'off', '@typescript-eslint/require-await': 'error', '@typescript-eslint/restrict-plus-operands': [