diff --git a/packages/eslint-plugin/docs/rules/prefer-readonly-parameter-types.md b/packages/eslint-plugin/docs/rules/prefer-readonly-parameter-types.md index 34544456f09..3b6f1346d7c 100644 --- a/packages/eslint-plugin/docs/rules/prefer-readonly-parameter-types.md +++ b/packages/eslint-plugin/docs/rules/prefer-readonly-parameter-types.md @@ -130,6 +130,7 @@ interface Options { const defaultOptions: Options = { checkParameterProperties: true, ignoreInferredTypes: false, + treatMethodsAsReadonly: false, }; ``` @@ -214,3 +215,43 @@ export const acceptsCallback: AcceptsCallback; ``` + +### `treatMethodsAsReadonly` + +This option allows you to treat all mutable methods as though they were readonly. This may be desirable in when you are never reassigning methods. + +Examples of **incorrect** code for this rule with `{treatMethodsAsReadonly: false}`: + +```ts +type MyType = { + readonly prop: string; + method(): string; // note: this method is mutable +}; +function foo(arg: MyType) {} +``` + +Examples of **correct** code for this rule with `{treatMethodsAsReadonly: false}`: + +```ts +type MyType = Readonly<{ + prop: string; + method(): string; +}>; +function foo(arg: MyType) {} + +type MyOtherType = { + readonly prop: string; + readonly method: () => string; +}; +function bar(arg: MyOtherType) {} +``` + +Examples of **correct** code for this rule with `{treatMethodsAsReadonly: true}`: + +```ts +type MyType = { + readonly prop: string; + method(): string; // note: this method is mutable +}; +function foo(arg: MyType) {} +``` diff --git a/packages/eslint-plugin/src/rules/prefer-readonly-parameter-types.ts b/packages/eslint-plugin/src/rules/prefer-readonly-parameter-types.ts index a36020e93d3..fdf48395eae 100644 --- a/packages/eslint-plugin/src/rules/prefer-readonly-parameter-types.ts +++ b/packages/eslint-plugin/src/rules/prefer-readonly-parameter-types.ts @@ -8,7 +8,7 @@ type Options = [ { checkParameterProperties?: boolean; ignoreInferredTypes?: boolean; - }, + } & util.ReadonlynessOptions, ]; type MessageIds = 'shouldBeReadonly'; @@ -34,6 +34,7 @@ export default util.createRule({ ignoreInferredTypes: { type: 'boolean', }, + ...util.readonlynessOptionsSchema.properties, }, }, ], @@ -45,10 +46,13 @@ export default util.createRule({ { checkParameterProperties: true, ignoreInferredTypes: false, + ...util.readonlynessOptionsDefaults, }, ], create(context, options) { - const [{ checkParameterProperties, ignoreInferredTypes }] = options; + const [ + { checkParameterProperties, ignoreInferredTypes, treatMethodsAsReadonly }, + ] = options; const { esTreeNodeToTSNodeMap, program } = util.getParserServices(context); const checker = program.getTypeChecker(); @@ -94,7 +98,9 @@ export default util.createRule({ const tsNode = esTreeNodeToTSNodeMap.get(actualParam); const type = checker.getTypeAtLocation(tsNode); - const isReadOnly = util.isTypeReadonly(checker, type); + const isReadOnly = util.isTypeReadonly(checker, type, { + treatMethodsAsReadonly: treatMethodsAsReadonly!, + }); if (!isReadOnly) { context.report({ diff --git a/packages/eslint-plugin/src/util/isTypeReadonly.ts b/packages/eslint-plugin/src/util/isTypeReadonly.ts index 4a42c90c1e0..8b3efc27856 100644 --- a/packages/eslint-plugin/src/util/isTypeReadonly.ts +++ b/packages/eslint-plugin/src/util/isTypeReadonly.ts @@ -4,6 +4,7 @@ import { isUnionOrIntersectionType, unionTypeParts, isPropertyReadonlyInType, + isSymbolFlagSet, } from 'tsutils'; import * as ts from 'typescript'; import { getTypeOfPropertyOfType, nullThrows, NullThrowsReasons } from '.'; @@ -17,9 +18,32 @@ const enum Readonlyness { Readonly = 3, } +export interface ReadonlynessOptions { + readonly treatMethodsAsReadonly?: boolean; +} + +export const readonlynessOptionsSchema = { + type: 'object', + additionalProperties: false, + properties: { + treatMethodsAsReadonly: { + type: 'boolean', + }, + }, +}; + +export const readonlynessOptionsDefaults: ReadonlynessOptions = { + treatMethodsAsReadonly: false, +}; + +function hasSymbol(node: ts.Node): node is ts.Node & { symbol: ts.Symbol } { + return Object.prototype.hasOwnProperty.call(node, 'symbol'); +} + function isTypeReadonlyArrayOrTuple( checker: ts.TypeChecker, type: ts.Type, + options: ReadonlynessOptions, seenTypes: Set, ): Readonlyness { function checkTypeArguments(arrayType: ts.TypeReference): Readonlyness { @@ -40,7 +64,7 @@ function isTypeReadonlyArrayOrTuple( if ( typeArguments.some( typeArg => - isTypeReadonlyRecurser(checker, typeArg, seenTypes) === + isTypeReadonlyRecurser(checker, typeArg, options, seenTypes) === Readonlyness.Mutable, ) ) { @@ -76,6 +100,7 @@ function isTypeReadonlyArrayOrTuple( function isTypeReadonlyObject( checker: ts.TypeChecker, type: ts.Type, + options: ReadonlynessOptions, seenTypes: Set, ): Readonlyness { function checkIndexSignature(kind: ts.IndexKind): Readonlyness { @@ -93,7 +118,18 @@ function isTypeReadonlyObject( if (properties.length) { // ensure the properties are marked as readonly for (const property of properties) { - if (!isPropertyReadonlyInType(type, property.getEscapedName(), checker)) { + if ( + !( + isPropertyReadonlyInType(type, property.getEscapedName(), checker) || + (options.treatMethodsAsReadonly && + property.valueDeclaration !== undefined && + hasSymbol(property.valueDeclaration) && + isSymbolFlagSet( + property.valueDeclaration.symbol, + ts.SymbolFlags.Method, + )) + ) + ) { return Readonlyness.Mutable; } } @@ -117,7 +153,7 @@ function isTypeReadonlyObject( } if ( - isTypeReadonlyRecurser(checker, propertyType, seenTypes) === + isTypeReadonlyRecurser(checker, propertyType, options, seenTypes) === Readonlyness.Mutable ) { return Readonlyness.Mutable; @@ -142,6 +178,7 @@ function isTypeReadonlyObject( function isTypeReadonlyRecurser( checker: ts.TypeChecker, type: ts.Type, + options: ReadonlynessOptions, seenTypes: Set, ): Readonlyness.Readonly | Readonlyness.Mutable { seenTypes.add(type); @@ -149,7 +186,7 @@ function isTypeReadonlyRecurser( if (isUnionType(type)) { // all types in the union must be readonly const result = unionTypeParts(type).every(t => - isTypeReadonlyRecurser(checker, t, seenTypes), + isTypeReadonlyRecurser(checker, t, options, seenTypes), ); const readonlyness = result ? Readonlyness.Readonly : Readonlyness.Mutable; return readonlyness; @@ -169,12 +206,22 @@ function isTypeReadonlyRecurser( return Readonlyness.Readonly; } - const isReadonlyArray = isTypeReadonlyArrayOrTuple(checker, type, seenTypes); + const isReadonlyArray = isTypeReadonlyArrayOrTuple( + checker, + type, + options, + seenTypes, + ); if (isReadonlyArray !== Readonlyness.UnknownType) { return isReadonlyArray; } - const isReadonlyObject = isTypeReadonlyObject(checker, type, seenTypes); + const isReadonlyObject = isTypeReadonlyObject( + checker, + type, + options, + seenTypes, + ); /* istanbul ignore else */ if ( isReadonlyObject !== Readonlyness.UnknownType ) { @@ -187,9 +234,14 @@ function isTypeReadonlyRecurser( /** * Checks if the given type is readonly */ -function isTypeReadonly(checker: ts.TypeChecker, type: ts.Type): boolean { +function isTypeReadonly( + checker: ts.TypeChecker, + type: ts.Type, + options: ReadonlynessOptions, +): boolean { return ( - isTypeReadonlyRecurser(checker, type, new Set()) === Readonlyness.Readonly + isTypeReadonlyRecurser(checker, type, options, new Set()) === + Readonlyness.Readonly ); } diff --git a/packages/eslint-plugin/tests/rules/prefer-readonly-parameter-types.test.ts b/packages/eslint-plugin/tests/rules/prefer-readonly-parameter-types.test.ts index 9278420a764..0a6ee103807 100644 --- a/packages/eslint-plugin/tests/rules/prefer-readonly-parameter-types.test.ts +++ b/packages/eslint-plugin/tests/rules/prefer-readonly-parameter-types.test.ts @@ -154,6 +154,74 @@ ruleTester.run('prefer-readonly-parameter-types', rule, { } function foo(arg: Readonly) {} `, + // immutable methods + ` + type MyType = Readonly<{ + prop: string; + method(): string; + }>; + function foo(arg: MyType) {} + `, + ` + type MyType = { + readonly prop: string; + readonly method: () => string; + }; + function bar(arg: MyType) {} + `, + // methods treated as readonly + { + code: ` + type MyType = { + readonly prop: string; + method(): string; + }; + function foo(arg: MyType) {} + `, + options: [ + { + treatMethodsAsReadonly: true, + }, + ], + }, + { + code: ` + class Foo { + method() {} + } + function foo(arg: Foo) {} + `, + options: [ + { + treatMethodsAsReadonly: true, + }, + ], + }, + { + code: ` + interface Foo { + method(): void; + } + function foo(arg: Foo) {} + `, + options: [ + { + treatMethodsAsReadonly: true, + }, + ], + }, + // ReadonlySet and ReadonlyMap are seen as readonly when methods are treated as readonly + { + code: ` + function foo(arg: ReadonlySet) {} + function bar(arg: ReadonlyMap) {} + `, + options: [ + { + treatMethodsAsReadonly: true, + }, + ], + }, // parameter properties should work fine { @@ -715,5 +783,23 @@ ruleTester.run('prefer-readonly-parameter-types', rule, { }, ], }, + // Mutable methods. + { + code: ` + type MyType = { + readonly prop: string; + method(): string; + }; + function foo(arg: MyType) {} + `, + errors: [ + { + messageId: 'shouldBeReadonly', + line: 6, + column: 22, + endColumn: 33, + }, + ], + }, ], });