fix(type-utils): check IndexSignature internals when checking isTypeR…

…eadonly fix #3714
typescript-eslint · Jan 9, 2022 · bea1b72 · bea1b72
1 parent 2a23d8e
commit bea1b72
Show file tree

Hide file tree

Showing 2 changed files with 67 additions and 55 deletions.
diff --git a/packages/type-utils/src/isTypeReadonly.ts b/packages/type-utils/src/isTypeReadonly.ts
@@ -107,62 +107,76 @@ function isTypeReadonlyObject(
   function checkIndexSignature(kind: ts.IndexKind): Readonlyness {
     const indexInfo = checker.getIndexInfoOfType(type, kind);
     if (indexInfo) {
-      return indexInfo.isReadonly
-        ? Readonlyness.Readonly
-        : Readonlyness.Mutable;
+      if (!indexInfo.isReadonly) {
+        return Readonlyness.Mutable;
+      }
+
+      return checkProperties(indexInfo.type);
     }
 
     return Readonlyness.UnknownType;
   }
 
-  const properties = type.getProperties();
-  if (properties.length) {
-    // ensure the properties are marked as readonly
-    for (const property of properties) {
-      if (
-        !(
-          isPropertyReadonlyInType(type, property.getEscapedName(), checker) ||
-          (options.treatMethodsAsReadonly &&
-            property.valueDeclaration !== undefined &&
-            hasSymbol(property.valueDeclaration) &&
-            isSymbolFlagSet(
-              property.valueDeclaration.symbol,
-              ts.SymbolFlags.Method,
-            ))
-        )
-      ) {
-        return Readonlyness.Mutable;
+  function checkProperties(type: ts.Type): Readonlyness {
+    const properties = type.getProperties();
+    if (properties.length) {
+      // ensure the properties are marked as readonly
+      for (const property of properties) {
+        if (
+          !(
+            isPropertyReadonlyInType(
+              type,
+              property.getEscapedName(),
+              checker,
+            ) ||
+            (options.treatMethodsAsReadonly &&
+              property.valueDeclaration !== undefined &&
+              hasSymbol(property.valueDeclaration) &&
+              isSymbolFlagSet(
+                property.valueDeclaration.symbol,
+                ts.SymbolFlags.Method,
+              ))
+          )
+        ) {
+          return Readonlyness.Mutable;
+        }
       }
-    }
 
-    // all properties were readonly
-    // now ensure that all of the values are readonly also.
-
-    // do this after checking property readonly-ness as a perf optimization,
-    // as we might be able to bail out early due to a mutable property before
-    // doing this deep, potentially expensive check.
-    for (const property of properties) {
-      const propertyType = ESLintUtils.nullThrows(
-        getTypeOfPropertyOfType(checker, type, property),
-        ESLintUtils.NullThrowsReasons.MissingToken(
-          `property "${property.name}"`,
-          'type',
-        ),
-      );
-
-      // handle recursive types.
-      // we only need this simple check, because a mutable recursive type will break via the above prop readonly check
-      if (seenTypes.has(propertyType)) {
-        continue;
-      }
-
-      if (
-        isTypeReadonlyRecurser(checker, propertyType, options, seenTypes) ===
-        Readonlyness.Mutable
-      ) {
-        return Readonlyness.Mutable;
+      // all properties were readonly
+      // now ensure that all of the values are readonly also.
+
+      // do this after checking property readonly-ness as a perf optimization,
+      // as we might be able to bail out early due to a mutable property before
+      // doing this deep, potentially expensive check.
+      for (const property of properties) {
+        const propertyType = ESLintUtils.nullThrows(
+          getTypeOfPropertyOfType(checker, type, property),
+          ESLintUtils.NullThrowsReasons.MissingToken(
+            `property "${property.name}"`,
+            'type',
+          ),
+        );
+
+        // handle recursive types.
+        // we only need this simple check, because a mutable recursive type will break via the above prop readonly check
+        if (seenTypes.has(propertyType)) {
+          continue;
+        }
+
+        if (
+          isTypeReadonlyRecurser(checker, propertyType, options, seenTypes) ===
+          Readonlyness.Mutable
+        ) {
+          return Readonlyness.Mutable;
+        }
       }
     }
+    return Readonlyness.UnknownType;
+  }
+
+  const isProperties = checkProperties(type);
+  if (isProperties === Readonlyness.Mutable) {
+    return isProperties;
   }
 
   const isStringIndexSigReadonly = checkIndexSignature(ts.IndexKind.String);

diff --git a/packages/type-utils/tests/isTypeReadonly.test.ts b/packages/type-utils/tests/isTypeReadonly.test.ts
@@ -229,15 +229,13 @@ describe('isTypeReadonly', () => {
       });
 
       describe('is not readonly', () => {
-        describe('default options', () => {
-          it('fails with a mutable PropertySignature inside a readonly IndexSignature', () => {
-            const { type, checker } = getType(
-              `type Test = { readonly [key: string]: { foo: string[]; }; };`,
-            );
+        it('fails with a mutable PropertySignature inside a readonly IndexSignature', () => {
+          const { type, checker } = getType(
+            `type Test = { readonly [key: string]: { foo: string[]; }; };`,
+          );
 
-            const result = isTypeReadonly(checker, type);
-            expect(result).toBe(false);
-          });
+          const result = isTypeReadonly(checker, type);
+          expect(result).toBe(false);
         });
       });
     });