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

…atMethodsAsReadonly fix #1758
typescript-eslint · Aug 15, 2021 · af4fa36 · af4fa36
1 parent db78642
commit af4fa36
Show file tree

Hide file tree

Showing 3 changed files with 96 additions and 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
@@ -8,6 +8,7 @@ type Options = [
   {
     checkParameterProperties?: boolean;
     ignoreInferredTypes?: boolean;
+    treatMethodsAsReadonly?: boolean;
   },
 ];
 type MessageIds = 'shouldBeReadonly';
@@ -34,6 +35,9 @@ export default util.createRule<Options, MessageIds>({
           ignoreInferredTypes: {
             type: 'boolean',
           },
+          treatMethodsAsReadonly: {
+            type: 'boolean',
+          },
         },
       },
     ],
@@ -45,10 +49,13 @@ export default util.createRule<Options, MessageIds>({
     {
       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<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 --git 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<ts.Type>,
 ): 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<ts.Type>,
 ): 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,14 +159,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 +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
@@ -154,6 +154,45 @@ ruleTester.run('prefer-readonly-parameter-types', rule, {
       }
       function foo(arg: Readonly<Foo>) {}
     `,
+    // 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<string>) {}
+        function bar(arg: ReadonlyMap<string, string>) {}
+      `,
+      options: [
+        {
+          treatMethodsAsReadonly: true,
+        },
+      ],
+    },
 
     // parameter properties should work fine
     {