Jul 10, 2023 · Apr 22, 2023 · Apr 22, 2023 · Apr 22, 2023 · May 1, 2023 · May 1, 2023
diff --git a/packages/eslint-plugin/package.json b/packages/eslint-plugin/package.json
    "@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": "*",
diff --git a/packages/eslint-plugin/src/rules/no-restricted-imports.ts b/packages/eslint-plugin/src/rules/no-restricted-imports.ts
 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,
 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',
      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 } {
    },
    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) {
diff --git a/packages/eslint-plugin/src/rules/parameter-properties.ts b/packages/eslint-plugin/src/rules/parameter-properties.ts
            items: {
              $ref: '#/items/0/$defs/modifier',
            },
            minItems: 1,
          },
          prefer: {
            type: 'string',
diff --git a/packages/eslint-plugin/tests/areOptionsValid.test.ts b/packages/eslint-plugin/tests/areOptionsValid.test.ts
 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);
  });
 });
diff --git a/packages/eslint-plugin/tests/areOptionsValid.ts b/packages/eslint-plugin/tests/areOptionsValid.ts
 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,
  };
 }
diff --git a/packages/eslint-plugin/tests/rules/array-type.test.ts b/packages/eslint-plugin/tests/rules/array-type.test.ts

 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',
    );
  });
 });

 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`);
    }
  });
 });
diff --git a/packages/eslint-plugin/tests/schema-snapshots/parameter-properties.shot b/packages/eslint-plugin/tests/schema-snapshots/parameter-properties.shot
Original file line number	Diff line number	Diff line change
Expand Up		@@ -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": "*",
Expand Down
Original file line number	Diff line number	Diff line change
		@@ -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,
Expand All		@@ -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 {
Copy link Member bradzacherApr 27, 2023 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. question: why did you switch back to basing off of the base rule's schema here? Any reason or was it just to go back to de-duping things? Copy link ContributorAuthor domdomeggMay 1, 2023 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. No strong reason: just to dedupe things / keep things in sync.
		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',
Expand All		@@ -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 } {
Expand DownExpand Up		@@ -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) {
Expand Down
Original file line number	Diff line number	Diff line change
Expand Up		@@ -60,7 +60,6 @@ export default util.createRule<Options, MessageIds>({
		items: {
		$ref: '#/items/0/$defs/modifier',
		},
		minItems: 1,
bradzacher marked this conversation as resolved. Show resolvedHide resolved
		},
		prefer: {
		type: 'string',
Expand Down
Original file line number	Diff line number	Diff line change
		@@ -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);
		});
		});
Original file line number	Diff line number	Diff line change
		@@ -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,
		};
		}
Original file line number	Diff line number	Diff line change
Expand Up		@@ -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',
Expand DownExpand Up		@@ -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`);
		}
		});
		});
Comment on lines +2161 to +2175 Copy link Member bradzacherApr 27, 2023 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. I wonder if we should do a rule-tester style API here to remove the boilerplate. For example something like this: ruleOptionsTester(rule,{valid:[ ...],invalid:[[{default:'simple-array'}],['array'],],}); Copy link ContributorAuthor domdomeggMay 1, 2023 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. Yeah, I think something like this would be neat if we end up doing a lot of these kinds of tests. For now I think it's simple enough that we can leave this to be done in future if necessary? JoshuaKGoldberg reacted with thumbs up emoji Copy link Member bradzacherJun 24, 2023• edited Loading Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. Note that there is eslint/rfcs#103 and the implementing PReslint/eslint#16823 This would allow us to include these tests in the rule like: tester.run('rule',rule,{valid:[ ...],invalid:[ ...],fatal:[{options:['simple-array'],error:{name:'SchemaValidationError',message:'Whatever the json schema error is',},},{options:['array'],error:{name:'SchemaValidationError',message:'Whatever the json schema error is',},},],} so definitely we don't need to bother with building a bespoke test framework!