feat: object checks

packages/eslint-plugin/src/util/isTypeReadonly/index.ts → packages/eslint-plugin/src/util/isTypeReadonly.ts

@@ -3,10 +3,10 @@ import {
   isUnionType,
   isUnionOrIntersectionType,
   unionTypeParts,
+  isPropertyReadonlyInType,
 } from 'tsutils';
 import * as ts from 'typescript';
-import { nullThrows, NullThrowsReasons } from '..';
-import { isReadonlySymbol } from './isReadonlySymbol';
+import { nullThrows, NullThrowsReasons } from '.';
 
 /**
  * Returns:
@@ -79,20 +79,32 @@ function isTypeReadonlyObject(
 
   const properties = type.getProperties();
   if (properties.length) {
+    // ensure the properties are marked as readonly
     for (const property of properties) {
-      // eslint-disable-next-line @typescript-eslint/ban-ts-ignore
-      // @ts-ignore
-      const x = checker.isReadonlySymbol(property);
-      const y = isReadonlySymbol(property);
-      if (x !== y) {
-        throw new Error('FUCK');
-      }
-      if (!isReadonlySymbol(property)) {
+      if (!isPropertyReadonlyInType(
+        type,
+        property.getEscapedName(),
+        checker,
+      )) {
         return false;
       }
     }
 
     // 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 = nullThrows(
+        checker.getTypeOfPropertyOfType(type, property.getName()),
+        NullThrowsReasons.MissingToken(`property "${property.name}"`, 'type'),
+      );
+      if (!isTypeReadonly(checker, propertyType)) {
+        return false;
+      }
+    }
   }
 
   const isStringIndexSigReadonly = checkIndexSignature(ts.IndexKind.String);

packages/eslint-plugin/tests/rules/prefer-readonly-parameter-types.test.ts

@@ -39,19 +39,12 @@ const arrays = [
   'readonly [string]',
   'Readonly<[string]>',
 ];
+const objects = [
+  '{ foo: "" }',
+  '{ foo: readonly string[] }',
+  '{ foo(): void }',
+];
 const weirdIntersections = [
-  `
-    interface Test extends ReadonlyArray<string> {
-      readonly property: boolean
-    }
-    function foo(arg: Test) {}
-  `,
-  `
-    type Test = (readonly string[]) & {
-      readonly property: boolean
-    };
-    function foo(arg: Test) {}
-  `,
   `
     interface Test {
       (): void
@@ -65,26 +58,10 @@ const weirdIntersections = [
     };
     function foo(arg: Test) {}
   `,
-  `
-    type Test = string & number;
-    function foo(arg: Test) {}
-  `,
 ];
 
 ruleTester.run('prefer-readonly-parameter-types', rule, {
   valid: [
-    `
-      type Test = (readonly string[]) & {
-        property: boolean
-      };
-      function foo(arg: Test) {}
-    `,
-    `
-      interface Test extends ReadonlyArray<string> {
-        property: boolean
-      }
-      function foo(arg: Test) {}
-    `,
     'function foo(arg: { readonly a: string }) {}',
     'function foo() {}',
 
@@ -108,9 +85,35 @@ ruleTester.run('prefer-readonly-parameter-types', rule, {
     'function foo(arg: ReadonlyArray<string> | ReadonlyArray<number>) {}',
 
     // objects
+    ...objects.map(type => `function foo(arg: Readonly<${type}>) {}`),
+    `
+      function foo(arg: {
+        readonly foo: {
+          readonly bar: string
+        }
+      }) {}
+    `,
 
     // weird other cases
     ...weirdIntersections.map(code => code),
+    `
+      interface Test extends ReadonlyArray<string> {
+        readonly property: boolean
+      }
+      function foo(arg: Readonly<Test>) {}
+    `,
+    `
+      type Test = (readonly string[]) & {
+        readonly property: boolean
+      };
+      function foo(arg: Readonly<Test>) {}
+    `,
+    `
+      type Test = string & number;
+      function foo(arg: Test) {}
+    `,
+
+    // declaration merging
     `
       class Foo {
         readonly bang = 1;
@@ -123,6 +126,7 @@ ruleTester.run('prefer-readonly-parameter-types', rule, {
       }
       function foo(arg: Foo) {}
     `,
+    // method made readonly via Readonly<T>
     `
       class Foo {
         method() {}
@@ -181,16 +185,36 @@ ruleTester.run('prefer-readonly-parameter-types', rule, {
     },
 
     // objects
-    // {
-    //   code: `
-    //     interface MutablePropFunction {
-    //       (): void
-    //       mutable: boolean
-    //     }
-    //     function foo(arg: MutablePropFunction) {}
-    //   `,
-    //   errors: [],
-    // },
+    ...objects.map<TSESLint.InvalidTestCase<MessageIds, Options>>(type => {
+      return {
+        code: `function foo(arg: ${type}) {}`,
+        errors: [
+          {
+            messageId: 'shouldBeReadonly',
+            column: 14,
+            endColumn: 19 + type.length,
+          },
+        ],
+      };
+    }),
+    {
+      code: `
+        function foo(arg: {
+          readonly foo: {
+            bar: string
+          }
+        }) {}
+      `,
+      errors: [
+        {
+          messageId: 'shouldBeReadonly',
+          line: 2,
+          column: 22,
+          endLine: 6,
+          endColumn: 10,
+        },
+      ],
+    },
 
     // weird intersections
     ...weirdIntersections.map<TSESLint.InvalidTestCase<MessageIds, Options>>(

packages/eslint-plugin/typings/typescript.d.ts

@@ -6,16 +6,21 @@ declare module 'typescript' {
 
     /**
      * @returns `true` if the given type is an array type:
-     * - Array<foo>
-     * - ReadonlyArray<foo>
-     * - foo[]
-     * - readonly foo[]
+     * - `Array<foo>`
+     * - `ReadonlyArray<foo>`
+     * - `foo[]`
+     * - `readonly foo[]`
      */
     isArrayType(type: Type): type is TypeReference;
     /**
      * @returns `true` if the given type is a tuple type:
-     * - [foo]
+     * - `[foo]`
+     * - `readonly [foo]`
      */
     isTupleType(type: Type): type is TupleTypeReference;
+    /**
+     * Return the type of the given property in the given type, or undefined if no such property exists
+     */
+    getTypeOfPropertyOfType(type: Type, propertyName: string): Type | undefined;
   }
 }