feat(eslint-plugin): [prefer-readonly-parameter-types] add option treatMethodsAsReadonly

fix #1758

Author: RebeccaStevens

Diff for: 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;
 ```
 
 </details>
+
+### `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 for: 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<Options, MessageIds>({
           ignoreInferredTypes: {
             type: 'boolean',
           },
+          ...util.readonlynessOptionsSchema.properties,
         },
       },
     ],
@@ -45,10 +46,13 @@ export default util.createRule<Options, MessageIds>({
     {
       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<Options, MessageIds>({
 
           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 for: 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<ts.Type>,
 ): Readonlyness {
   function checkTypeArguments(arrayType: ts.TypeReference): Readonlyness {
@@ -35,7 +59,7 @@ function isTypeReadonlyArrayOrTuple(
     if (
       typeArguments.some(
         typeArg =>
-          isTypeReadonlyRecurser(checker, typeArg, seenTypes) ===
+          isTypeReadonlyRecurser(checker, typeArg, options, seenTypes) ===
           Readonlyness.Mutable,
       )
     ) {
@@ -71,6 +95,7 @@ function isTypeReadonlyArrayOrTuple(
 function isTypeReadonlyObject(
   checker: ts.TypeChecker,
   type: ts.Type,
+  options: ReadonlynessOptions,
   seenTypes: Set<ts.Type>,
 ): Readonlyness {
   function checkIndexSignature(kind: ts.IndexKind): Readonlyness {
@@ -88,7 +113,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 +148,7 @@ function isTypeReadonlyObject(
       }
 
       if (
-        isTypeReadonlyRecurser(checker, propertyType, seenTypes) ===
+        isTypeReadonlyRecurser(checker, propertyType, options, seenTypes) ===
         Readonlyness.Mutable
       ) {
         return Readonlyness.Mutable;
@@ -137,14 +173,15 @@ function isTypeReadonlyObject(
 function isTypeReadonlyRecurser(
   checker: ts.TypeChecker,
   type: ts.Type,
+  options: ReadonlynessOptions,
   seenTypes: Set<ts.Type>,
 ): Readonlyness.Readonly | Readonlyness.Mutable {
   seenTypes.add(type);
 
   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 +201,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 +229,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 for: 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<Foo>) {}
     `,
+    // 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<string>) {}
+        function bar(arg: ReadonlyMap<string, string>) {}
+      `,
+      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,
+        },
+      ],
+    },
   ],
 });