feat: nested array checks

typescript-eslint · Jan 25, 2020 · c5f4881 · c5f4881
1 parent 57997d0
commit c5f4881
Show file tree

Hide file tree

Showing 2 changed files with 155 additions and 27 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
@@ -38,25 +38,41 @@ export default util.createRule({
      * - false if the type is a mutable array or mutable tuple.
      */
     function isTypeReadonlyArrayOrTuple(type: ts.Type): boolean | null {
+      function checkTypeArguments(arrayType: ts.TypeReference): boolean {
+        const typeArguments = checker.getTypeArguments(arrayType);
+        if (typeArguments.length === 0) {
+          // this shouldn't happen in reality as:
+          // - tuples require at least 1 type argument
+          // - ReadonlyArray requires at least 1 type argument
+          return true;
+        }
+
+        // validate the element types are also readonly
+        if (typeArguments.some(typeArg => !isTypeReadonly(typeArg))) {
+          return false;
+        }
+        return true;
+      }
+
       if (checker.isArrayType(type)) {
         const symbol = util.nullThrows(
           type.getSymbol(),
           util.NullThrowsReasons.MissingToken('symbol', 'array type'),
         );
         const escapedName = symbol.getEscapedName();
-        if (escapedName === 'ReadonlyArray') {
-          return true;
-        }
-        if (escapedName === 'Array') {
+        if (escapedName === 'Array' && escapedName !== 'ReadonlyArray') {
           return false;
         }
+
+        return checkTypeArguments(type);
       }
 
       if (checker.isTupleType(type)) {
-        if (type.target.readonly) {
-          return true;
+        if (!type.target.readonly) {
+          return false;
         }
-        return false;
+
+        return checkTypeArguments(type);
       }
 
       return null;

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
@@ -1,5 +1,13 @@
 import rule from '../../src/rules/prefer-readonly-parameter-types';
 import { RuleTester, getFixturesRootDir } from '../RuleTester';
+import { TSESLint } from '@typescript-eslint/experimental-utils';
+import {
+  InferMessageIdsTypeFromRule,
+  InferOptionsTypeFromRule,
+} from '../../src/util';
+
+type MessageIds = InferMessageIdsTypeFromRule<typeof rule>;
+type Options = InferOptionsTypeFromRule<typeof rule>;
 
 const rootPath = getFixturesRootDir();
 
@@ -24,54 +32,158 @@ const primitives = [
   'null',
   'undefined',
 ];
+const arrays = [
+  'readonly string[]',
+  'Readonly<string[]>',
+  'ReadonlyArray<string>',
+  'readonly [string]',
+  'Readonly<[string]>',
+];
+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
+      readonly property: boolean
+    }
+    function foo(arg: Test) {}
+  `,
+  `
+    type Test = (() => void) & {
+      readonly property: boolean
+    };
+    function foo(arg: Test) {}
+  `,
+];
 
 ruleTester.run('prefer-readonly-parameter-types', rule, {
   valid: [
+    'function foo() {}',
+
     // primitives
     ...primitives.map(type => `function foo(arg: ${type}) {}`),
 
     // arrays
-    'function foo(arg: readonly string[]) {}',
-    'function foo(arg: Readonly<string[]>) {}',
-    'function foo(arg: ReadonlyArray<string>) {}',
-    'function foo(arg: ReadonlyArray<string> | ReadonlyArray<number>) {}',
+    ...arrays.map(type => `function foo(arg: ${type}) {}`),
+    // nested arrays
+    'function foo(arg: readonly (readonly string[])[]) {}',
+    'function foo(arg: Readonly<Readonly<string[]>[]>) {}',
+    'function foo(arg: ReadonlyArray<ReadonlyArray<string>>) {}',
 
     // functions
     'function foo(arg: () => void) {}',
-    `
-      interface ImmutablePropFunction {
-        (): void
-        readonly immutable: boolean
-      }
-      function foo(arg: ImmutablePropFunction) {}
-    `,
 
     // unions
     'function foo(arg: string | null) {}',
     'function foo(arg: string | ReadonlyArray<string>) {}',
     'function foo(arg: string | (() => void)) {}',
+    'function foo(arg: ReadonlyArray<string> | ReadonlyArray<number>) {}',
+
+    // objects
+
+    // weird intersections
+    ...weirdIntersections.map(code => code),
   ],
   invalid: [
+    // arrays
+    ...arrays.map<TSESLint.InvalidTestCase<MessageIds, Options>>(baseType => {
+      const type = baseType
+        .replace(/readonly /g, '')
+        .replace(/Readonly<(.+?)>/g, '$1')
+        .replace(/ReadonlyArray/g, 'Array');
+      return {
+        code: `function foo(arg: ${type}) {}`,
+        errors: [
+          {
+            messageId: 'shouldBeReadonly',
+            column: 14,
+            endColumn: 19 + type.length,
+          },
+        ],
+      };
+    }),
+    // nested arrays
     {
-      code: 'function foo(arg: string[]) {}',
+      code: 'function foo(arg: readonly (string[])[]) {}',
       errors: [
         {
           messageId: 'shouldBeReadonly',
-          data: {
-            type: 'Parameters',
-          },
+          column: 14,
+          endColumn: 40,
+        },
+      ],
+    },
+    {
+      code: 'function foo(arg: Readonly<string[][]>) {}',
+      errors: [
+        {
+          messageId: 'shouldBeReadonly',
+          column: 14,
+          endColumn: 39,
+        },
+      ],
+    },
+    {
+      code: 'function foo(arg: ReadonlyArray<Array<string>>) {}',
+      errors: [
+        {
+          messageId: 'shouldBeReadonly',
+          column: 14,
+          endColumn: 47,
         },
       ],
     },
+
+    // objects
+    // {
+    //   code: `
+    //     interface MutablePropFunction {
+    //       (): void
+    //       mutable: boolean
+    //     }
+    //     function foo(arg: MutablePropFunction) {}
+    //   `,
+    //   errors: [],
+    // },
+
+    // weird intersections
+    ...weirdIntersections.map<TSESLint.InvalidTestCase<MessageIds, Options>>(
+      baseCode => {
+        const code = baseCode.replace(/readonly /g, '');
+        return {
+          code,
+          errors: [{ messageId: 'shouldBeReadonly' }],
+        };
+      },
+    ),
+    {
+      code: `
+        interface Test extends Array<string> {
+          readonly property: boolean
+        }
+        function foo(arg: Test) {}
+      `,
+      errors: [{ messageId: 'shouldBeReadonly' }],
+    },
     {
       code: `
-        interface MutablePropFunction {
-          (): void
-          mutable: boolean
+        interface Test extends Array<string> {
+          property: boolean
         }
-        function foo(arg: MutablePropFunction) {}
+        function foo(arg: Test) {}
       `,
-      errors: [],
+      errors: [{ messageId: 'shouldBeReadonly' }],
     },
   ],
 });