From af4fa36148836938b7efb08bf1c72dfbb0af4295 Mon Sep 17 00:00:00 2001 From: Rebecca Stevens Date: Sun, 15 Aug 2021 21:59:37 +1200 Subject: [PATCH] feat(eslint-plugin): [prefer-readonly-parameter-types] add option treatMethodsAsReadonly fix #1758 --- .../rules/prefer-readonly-parameter-types.ts | 13 ++++- .../eslint-plugin/src/util/isTypeReadonly.ts | 54 ++++++++++++++++--- .../prefer-readonly-parameter-types.test.ts | 39 ++++++++++++++ 3 files changed, 96 insertions(+), 10 deletions(-) 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 a36020e93d3c..3f8ce970e615 100644 --- a/packages/eslint-plugin/src/rules/prefer-readonly-parameter-types.ts +++ b/packages/eslint-plugin/src/rules/prefer-readonly-parameter-types.ts @@ -8,6 +8,7 @@ type Options = [ { checkParameterProperties?: boolean; ignoreInferredTypes?: boolean; + treatMethodsAsReadonly?: boolean; }, ]; type MessageIds = 'shouldBeReadonly'; @@ -34,6 +35,9 @@ export default util.createRule({ ignoreInferredTypes: { type: 'boolean', }, + treatMethodsAsReadonly: { + type: 'boolean', + }, }, }, ], @@ -45,10 +49,13 @@ export default util.createRule({ { checkParameterProperties: true, ignoreInferredTypes: false, + treatMethodsAsReadonly: false, }, ], create(context, options) { - const [{ checkParameterProperties, ignoreInferredTypes }] = options; + const [ + { checkParameterProperties, ignoreInferredTypes, treatMethodsAsReadonly }, + ] = options; const { esTreeNodeToTSNodeMap, program } = util.getParserServices(context); const checker = program.getTypeChecker(); @@ -94,7 +101,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 b10f00006ec5..e51df43226e5 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,18 @@ const enum Readonlyness { Readonly = 3, } +export interface ReadonlynessOptions { + readonly treatMethodsAsReadonly: boolean; +} + +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 { @@ -35,7 +45,7 @@ function isTypeReadonlyArrayOrTuple( if ( typeArguments.some( typeArg => - isTypeReadonlyRecurser(checker, typeArg, seenTypes) === + isTypeReadonlyRecurser(checker, typeArg, options, seenTypes) === Readonlyness.Mutable, ) ) { @@ -71,6 +81,7 @@ function isTypeReadonlyArrayOrTuple( function isTypeReadonlyObject( checker: ts.TypeChecker, type: ts.Type, + options: ReadonlynessOptions, seenTypes: Set, ): Readonlyness { function checkIndexSignature(kind: ts.IndexKind): Readonlyness { @@ -88,7 +99,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; } } @@ -112,7 +134,7 @@ function isTypeReadonlyObject( } if ( - isTypeReadonlyRecurser(checker, propertyType, seenTypes) === + isTypeReadonlyRecurser(checker, propertyType, options, seenTypes) === Readonlyness.Mutable ) { return Readonlyness.Mutable; @@ -137,6 +159,7 @@ function isTypeReadonlyObject( function isTypeReadonlyRecurser( checker: ts.TypeChecker, type: ts.Type, + options: ReadonlynessOptions, seenTypes: Set, ): Readonlyness.Readonly | Readonlyness.Mutable { seenTypes.add(type); @@ -144,7 +167,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; @@ -164,12 +187,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 ) { @@ -182,9 +215,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 9278420a7647..bddfd61a6631 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,45 @@ ruleTester.run('prefer-readonly-parameter-types', rule, { } function foo(arg: Readonly) {} `, + // methods treated as readonly + { + 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 {