fix(eslint-plugin): fix schemas across several rules and add schema t…

…ests (#6947)

* fix(eslint-plugin): fix schemas across several rules and add schema tests

* Fix typecheck

* Fix lint

* Address review feedback

* Fix CI

* Revert accidental change

* Fix tests

---------

Co-authored-by: Josh Goldberg <git@joshuakgoldberg.com>

packages/eslint-plugin/package.json

@@ -76,6 +76,7 @@
     "@types/prettier": "*",
     "@typescript-eslint/rule-schema-to-typescript-types": "6.0.0",
     "@typescript-eslint/rule-tester": "6.0.0",
+    "ajv": "^6.12.6",
     "chalk": "^5.0.1",
     "cross-fetch": "*",
     "jest-specific-snapshot": "*",

packages/eslint-plugin/src/rules/no-restricted-imports.ts

@@ -1,5 +1,9 @@
 import type { TSESTree } from '@typescript-eslint/utils';
-import type { JSONSchema4 } from '@typescript-eslint/utils/json-schema';
+import type {
+  JSONSchema4AnyOfSchema,
+  JSONSchema4ArraySchema,
+  JSONSchema4ObjectSchema,
+} from '@typescript-eslint/utils/json-schema';
 import type {
   ArrayOfStringOrObject,
   ArrayOfStringOrObjectPatterns,
@@ -19,38 +23,96 @@ const baseRule = getESLintCoreRule('no-restricted-imports');
 export type Options = InferOptionsTypeFromRule<typeof baseRule>;
 export type MessageIds = InferMessageIdsTypeFromRule<typeof baseRule>;
 
-const arrayOfStringsOrObjects: JSONSchema4 = {
+// In some versions of eslint, the base rule has a completely incompatible schema
+// This helper function is to safely try to get parts of the schema. If it's not
+// possible, we'll fallback to less strict checks.
+const tryAccess = <T>(getter: () => T, fallback: T): T => {
+  try {
+    return getter();
+  } catch {
+    return fallback;
+  }
+};
+
+const baseSchema = baseRule.meta.schema as {
+  anyOf: [
+    unknown,
+    {
+      type: 'array';
+      items: [
+        {
+          type: 'object';
+          properties: {
+            paths: {
+              type: 'array';
+              items: {
+                anyOf: [
+                  { type: 'string' },
+                  {
+                    type: 'object';
+                    properties: JSONSchema4ObjectSchema['properties'];
+                    required: string[];
+                  },
+                ];
+              };
+            };
+            patterns: {
+              anyOf: [
+                { type: 'array'; items: { type: 'string' } },
+                {
+                  type: 'array';
+                  items: {
+                    type: 'object';
+                    properties: JSONSchema4ObjectSchema['properties'];
+                    required: string[];
+                  };
+                },
+              ];
+            };
+          };
+        },
+      ];
+    },
+  ];
+};
+
+const allowTypeImportsOptionSchema: JSONSchema4ObjectSchema['properties'] = {
+  allowTypeImports: {
+    type: 'boolean',
+    description: 'Disallow value imports, but allow type-only imports.',
+  },
+};
+
+const arrayOfStringsOrObjects: JSONSchema4ArraySchema = {
   type: 'array',
   items: {
     anyOf: [
       { type: 'string' },
       {
         type: 'object',
+        additionalProperties: false,
         properties: {
-          name: { type: 'string' },
-          message: {
-            type: 'string',
-            minLength: 1,
-          },
-          importNames: {
-            type: 'array',
-            items: {
-              type: 'string',
-            },
-          },
-          allowTypeImports: {
-            type: 'boolean',
-            description: 'Disallow value imports, but allow type-only imports.',
-          },
+          ...tryAccess(
+            () =>
+              baseSchema.anyOf[1].items[0].properties.paths.items.anyOf[1]
+                .properties,
+            undefined,
+          ),
+          ...allowTypeImportsOptionSchema,
         },
-        additionalProperties: false,
-        required: ['name'],
+        required: tryAccess(
+          () =>
+            baseSchema.anyOf[1].items[0].properties.paths.items.anyOf[1]
+              .required,
+          undefined,
+        ),
       },
     ],
   },
   uniqueItems: true,
 };
-const arrayOfStringsOrObjectPatterns: JSONSchema4 = {
+
+const arrayOfStringsOrObjectPatterns: JSONSchema4AnyOfSchema = {
   anyOf: [
     {
       type: 'array',
@@ -63,43 +125,48 @@ const arrayOfStringsOrObjectPatterns: JSONSchema4 = {
       type: 'array',
       items: {
         type: 'object',
+        additionalProperties: false,
         properties: {
-          importNames: {
-            type: 'array',
-            items: {
-              type: 'string',
-            },
-            minItems: 1,
-            uniqueItems: true,
-          },
-          group: {
-            type: 'array',
-            items: {
-              type: 'string',
-            },
-            minItems: 1,
-            uniqueItems: true,
-          },
-          message: {
-            type: 'string',
-            minLength: 1,
-          },
-          caseSensitive: {
-            type: 'boolean',
-          },
-          allowTypeImports: {
-            type: 'boolean',
-            description: 'Disallow value imports, but allow type-only imports.',
-          },
+          ...tryAccess(
+            () =>
+              baseSchema.anyOf[1].items[0].properties.patterns.anyOf[1].items
+                .properties,
+            undefined,
+          ),
+          ...allowTypeImportsOptionSchema,
         },
-        additionalProperties: false,
-        required: ['group'],
+        required: tryAccess(
+          () =>
+            baseSchema.anyOf[1].items[0].properties.patterns.anyOf[1].items
+              .required,
+          [],
+        ),
       },
       uniqueItems: true,
     },
   ],
 };
 
+const schema: JSONSchema4AnyOfSchema = {
+  anyOf: [
+    arrayOfStringsOrObjects,
+    {
+      type: 'array',
+      items: [
+        {
+          type: 'object',
+          properties: {
+            paths: arrayOfStringsOrObjects,
+            patterns: arrayOfStringsOrObjectPatterns,
+          },
+          additionalProperties: false,
+        },
+      ],
+      additionalItems: false,
+    },
+  ],
+};
+
 function isObjectOfPaths(
   obj: unknown,
 ): obj is { paths: ArrayOfStringOrObject } {
@@ -153,25 +220,7 @@ export default createRule<Options, MessageIds>({
     },
     messages: baseRule.meta.messages,
     fixable: baseRule.meta.fixable,
-    schema: {
-      anyOf: [
-        arrayOfStringsOrObjects,
-        {
-          type: 'array',
-          items: [
-            {
-              type: 'object',
-              properties: {
-                paths: arrayOfStringsOrObjects,
-                patterns: arrayOfStringsOrObjectPatterns,
-              },
-              additionalProperties: false,
-            },
-          ],
-          additionalItems: false,
-        },
-      ],
-    },
+    schema,
   },
   defaultOptions: [],
   create(context) {

packages/eslint-plugin/src/rules/parameter-properties.ts

@@ -60,7 +60,6 @@ export default util.createRule<Options, MessageIds>({
             items: {
               $ref: '#/items/0/$defs/modifier',
             },
-            minItems: 1,
           },
           prefer: {
             type: 'string',

packages/eslint-plugin/tests/areOptionsValid.test.ts

@@ -0,0 +1,32 @@
+import * as util from '../src/util';
+import { areOptionsValid } from './areOptionsValid';
+
+const exampleRule = util.createRule<['value-a' | 'value-b'], never>({
+  name: 'my-example-rule',
+  meta: {
+    type: 'layout',
+    docs: {
+      description: 'Detects something or other',
+    },
+    schema: [{ type: 'string', enum: ['value-a', 'value-b'] }],
+    messages: {},
+  },
+  defaultOptions: ['value-a'],
+  create() {
+    return {};
+  },
+});
+
+test('returns true for valid options', () => {
+  expect(areOptionsValid(exampleRule, ['value-a'])).toBe(true);
+});
+
+describe('returns false for invalid options', () => {
+  test('bad enum value', () => {
+    expect(areOptionsValid(exampleRule, ['value-c'])).toBe(false);
+  });
+
+  test('bad type', () => {
+    expect(areOptionsValid(exampleRule, [true])).toBe(false);
+  });
+});

packages/eslint-plugin/tests/areOptionsValid.ts

@@ -0,0 +1,44 @@
+import { TSUtils } from '@typescript-eslint/utils';
+import type { RuleModule } from '@typescript-eslint/utils/ts-eslint';
+import Ajv from 'ajv';
+import type { JSONSchema4 } from 'json-schema';
+
+const ajv = new Ajv({ async: false });
+
+export function areOptionsValid(
+  rule: RuleModule<string, readonly unknown[]>,
+  options: unknown,
+): boolean {
+  const normalizedSchema = normalizeSchema(rule.meta.schema);
+
+  const valid = ajv.validate(normalizedSchema, options);
+  if (typeof valid !== 'boolean') {
+    // Schema could not validate options synchronously. This is not allowed for ESLint rules.
+    return false;
+  }
+
+  return valid;
+}
+
+function normalizeSchema(
+  schema: JSONSchema4 | readonly JSONSchema4[],
+): JSONSchema4 {
+  if (!TSUtils.isArray(schema)) {
+    return schema;
+  }
+
+  if (schema.length === 0) {
+    return {
+      type: 'array',
+      minItems: 0,
+      maxItems: 0,
+    };
+  }
+
+  return {
+    type: 'array',
+    items: schema as JSONSchema4[],
+    minItems: 0,
+    maxItems: schema.length,
+  };
+}

packages/eslint-plugin/tests/rules/array-type.test.ts

@@ -4,6 +4,7 @@ import { TSESLint } from '@typescript-eslint/utils';
 
 import type { OptionString } from '../../src/rules/array-type';
 import rule from '../../src/rules/array-type';
+import { areOptionsValid } from '../areOptionsValid';
 
 const ruleTester = new RuleTester({
   parser: '@typescript-eslint/parser',
@@ -2156,3 +2157,19 @@ type BrokenArray = {
     );
   });
 });
+
+describe('schema validation', () => {
+  // https://github.com/typescript-eslint/typescript-eslint/issues/6852
+  test("array-type does not accept 'simple-array' option", () => {
+    if (areOptionsValid(rule, [{ default: 'simple-array' }])) {
+      throw new Error(`Options succeeded validation for bad options`);
+    }
+  });
+
+  // https://github.com/typescript-eslint/typescript-eslint/issues/6892
+  test('array-type does not accept non object option', () => {
+    if (areOptionsValid(rule, ['array'])) {
+      throw new Error(`Options succeeded validation for bad options`);
+    }
+  });
+});