From d9604f34d72b39969fb9c929a4658090b578c312 Mon Sep 17 00:00:00 2001 From: Luca Mollema Date: Fri, 7 Jun 2024 10:00:39 +0200 Subject: [PATCH 1/7] feat(rule-tester) Add initial implementation of FlatRuleTester Introduces a `FlatRuleTester` class which overhauls the API Why? Because we shouldn't be writing rules as JSON inside our (Java/Type)script And the new API prevents writing our tests as a giant function call which we pass a giant object We'll need to port all the rules over to the new tester eventually --- packages/rule-tester/package.json | 1 + packages/rule-tester/src/FlatRuleTester.ts | 581 ++++++++++++++++++ packages/rule-tester/src/index.ts | 1 + .../rule-tester/src/types/FlatRuleTester.ts | 113 ++++ .../rule-tester/tests/FlatRuleTester.test.ts | 111 ++++ packages/rule-tester/tests/fixtures/file.ts | 0 .../rule-tester/tests/fixtures/tsconfig.json | 4 + packages/rule-tester/tsconfig.json | 3 +- yarn.lock | 3 +- 9 files changed, 815 insertions(+), 2 deletions(-) create mode 100644 packages/rule-tester/src/FlatRuleTester.ts create mode 100644 packages/rule-tester/src/types/FlatRuleTester.ts create mode 100644 packages/rule-tester/tests/FlatRuleTester.test.ts create mode 100644 packages/rule-tester/tests/fixtures/file.ts create mode 100644 packages/rule-tester/tests/fixtures/tsconfig.json diff --git a/packages/rule-tester/package.json b/packages/rule-tester/package.json index 6c74f3a20791..0adb94f39580 100644 --- a/packages/rule-tester/package.json +++ b/packages/rule-tester/package.json @@ -52,6 +52,7 @@ "@typescript-eslint/utils": "7.12.0", "ajv": "^6.12.6", "json-stable-stringify-without-jsonify": "^1.0.1", + "lodash": "^4.6.2", "lodash.merge": "4.6.2", "semver": "^7.6.0" }, diff --git a/packages/rule-tester/src/FlatRuleTester.ts b/packages/rule-tester/src/FlatRuleTester.ts new file mode 100644 index 000000000000..b93f0a07ccc5 --- /dev/null +++ b/packages/rule-tester/src/FlatRuleTester.ts @@ -0,0 +1,581 @@ +import assert from 'node:assert'; +import { existsSync } from 'node:fs'; +import { join } from 'node:path'; + +import { getParserServices } from '@typescript-eslint/utils/eslint-utils'; +import type { + FlatConfig, + LooseRuleDefinition, + RuleModule, +} from '@typescript-eslint/utils/ts-eslint'; +import { Linter } from '@typescript-eslint/utils/ts-eslint'; +import { isEqual } from 'lodash'; + +import { TestFramework } from './TestFramework'; +import type { + ExpectedError, + ExpectedSuggestion, + FileUnawareConfig, + InvalidSample, + LegacyInvalidSample, + LegacyValidSample, + RuleTesterConfig, + Sample, + ValidSample, +} from './types/FlatRuleTester'; +import { sanitize } from './utils/validationHelpers'; + +// Hide methods publicly by default, for simplicity's sake +export const DATA = Symbol('DATA'); +export const CONSTRUCT = Symbol('CONSTRUCT'); + +// Needed for type exports +// eslint-disable-next-line @typescript-eslint/no-namespace +export declare namespace FlatRuleTester { + export type { + ExpectedError, + ExpectedSuggestion, + InvalidSample, + LegacyInvalidSample, + LegacyValidSample, + RuleTesterConfig, + Sample, + ValidSample, + FileUnawareConfig, + }; +} + +/** + * The new, flat rule tester + * + * **WARNING:** This is NOT a drop-in-replacement for RuleTester, minor code migration will be needed and major code migration will be needed for optimal performance (See {@linkcode FlatRuleConfig#fromObject}) + * + * List of current breaking changes: + * - Uses ESLint's flat config system + * - The data property of errors in invalid cases is unsupported (ESLint does not expose its interpolate method, it is not a permanent solution to copy the implementation) + * - Requires type-checking + * - Does not support string imports for parsers + */ +export class FlatRuleTester extends TestFramework { + #config: Readonly>; + #rules: FlatRuleConfig[]; + #linter: Linter; + #fixtureDir: string; + #hasTested: boolean; + + constructor(readonly config: Readonly>) { + super(); + this.#hasTested = false; + this.#config = config; + this.#rules = []; + this.#linter = new Linter({ + // @ts-expect-error: Broken types + configType: 'flat', + }); + + this.#fixtureDir = this.#config.fixtureRootDir; + assert.ok(existsSync(this.#fixtureDir), 'Fixture directory must exist'); + } + + /** + * Defines a new rule + * + * It is up to the caller to ensure no rules are duplicated + */ + rule( + name: string, + rule: RuleModule, + ): FlatRuleConfig { + const tester = FlatRuleConfig[CONSTRUCT](name, rule); + this.#rules.push(tester); + return tester; + } + + /** + * Run the defined tests. + * + * Must be called exactly once, after all tests have been defined. + * + * Explanation of why we need to do this below: + * ```md + * Some testing frameworks do not allow tests to be defined in the afterAll hook, this is a problem and prevents us from automatically running tests. + * Thus, for consistency, we always require the calling of the test() method + * ``` + */ + test(): void { + assert.ok(!this.#hasTested, 'You should only call the test() method once!'); + this.#hasTested = true; + + for (const ruleDef of this.#rules) { + const { rule, name, configurations } = ruleDef[DATA](); + + FlatRuleTester.describe(name, () => { + configurations + .map(c => c[DATA]()) + .forEach(data => { + const custom: FlatConfig.Config = { + languageOptions: { + parserOptions: { + tsconfigRootDir: this.#fixtureDir, + project: true, + }, + }, + plugins: { + assertions: { + meta: { + name: 'assertions', + }, + rules: { + assertions: { + meta: { + messages: { + assert: 'assert', + }, + schema: [], + type: 'problem', + }, + defaultOptions: [], + // eslint-disable-next-line @typescript-eslint/explicit-function-return-type + create(ctx) { + assert.doesNotThrow( + () => getParserServices(ctx), + 'Parser services must be available', + ); + + return {}; + }, + } satisfies RuleModule<'assert'> as LooseRuleDefinition, + }, + }, + rule: { + meta: { + name: 'rule', + }, + rules: { + rule, + }, + }, + }, + rules: { + 'rule/rule': ['error', ...data.configuration], + 'assertions/assertions': ['error'], + }, + files: ['**/*.*'], + }; + + FlatRuleTester.describe('valid', () => + data.valid.forEach(validCase => + FlatRuleTester.it(validCase.name, () => { + if (validCase.format) { + assert.ok(validCase.format in this.#config.extensions); + } + + const name = `/file.${validCase.format ?? 'ts'}`; + assert.ok( + existsSync(join(this.#fixtureDir, name)), + `Fixtures must exist for format: ${name}`, + ); + + const fullConfig: FlatConfig.ConfigArray = [ + custom, + { + ...this.#config.baseOptions, + files: ['**/*.*'], + }, + ]; + if (validCase.format) { + const formatConfig = + this.#config.extensions[validCase.format as F]; + fullConfig.splice(1, 0, { + ...formatConfig, + files: [`**/*.${validCase.format}`], + }); + } + + const results = this.#linter.verify( + validCase.code, + fullConfig, + join(this.#fixtureDir, name), + ); + + results.forEach(msg => { + throw new Error( + `Got error ${msg.message} (${msg.messageId}), expected nothing`, + ); + }); + }), + ), + ); + FlatRuleTester.describe('invalid', () => + data.invalid.forEach(invalidCase => + FlatRuleTester.it(sanitize(invalidCase.name), () => { + if (invalidCase.format) { + assert.ok( + invalidCase.format in this.#config.extensions, + `Undefined format: ${sanitize(invalidCase.format)}, should be in ${sanitize(Object.keys(this.#config.extensions).join(', '))}`, + ); + } + + const name = `/file.${invalidCase.format ?? 'ts'}`; + assert.ok( + existsSync(join(this.#fixtureDir, name)), + 'Fixtures must exist', + ); + + const fullConfig: FlatConfig.ConfigArray = [ + custom, + { + ...this.#config.baseOptions, + files: ['**/*.*'], + }, + ]; + if (invalidCase.format) { + const formatConfig = + this.#config.extensions[invalidCase.format as F]; + fullConfig.splice(1, 0, { + ...formatConfig, + files: [`**/*.${invalidCase.format}`], + }); + } + + const allMessages: Linter.LintMessage[] = []; + const results = this.#linter.verifyAndFix( + invalidCase.code, + fullConfig, + { + filename: join(this.#fixtureDir, name), + fix: true, + postprocess: messages => { + const e = messages.reduce((prev, cur) => [ + ...prev, + ...cur, + ]); + allMessages.push(...e); + return e; + }, + }, + ); + results.messages = allMessages; // ESLint bug ._. this fixes it + + if (typeof invalidCase.output === 'string') { + assert.strictEqual( + results.output, + invalidCase.output, + 'Output code and fix differ', + ); + } else { + assert.strictEqual( + results.output, + invalidCase.code, + 'Result output must be untouched', + ); + } + + assert.strictEqual( + results.messages.length, + invalidCase.errors.length, + 'Result messages must be the same length', + ); + + results.messages.forEach((message, idx) => { + assert.ok( + message.ruleId != null, + `Parser/linter error occured: ${message.message}`, + ); + + assert.ok( + message.ruleId === 'rule/rule', + `Errors must be from the defined rule, as opposed to typescript-eslint or eslint: ${message.ruleId}`, + ); + + const expectedMessage = invalidCase.errors[idx]; + + if (expectedMessage.column) { + assert.strictEqual( + expectedMessage.column, + message.column, + 'Error column must be the same', + ); + } + if (expectedMessage.endColumn) { + assert.strictEqual( + expectedMessage.endColumn, + message.endColumn, + 'Error end column must be the same', + ); + } + if (expectedMessage.line) { + assert.strictEqual( + expectedMessage.line, + message.line, + 'Error line must be the same', + ); + } + if (expectedMessage.endLine) { + assert.strictEqual( + expectedMessage.endLine, + message.endLine, + 'Error end line must be the same', + ); + } + if (expectedMessage.message) { + assert.strictEqual( + expectedMessage.message, + message.message, + 'Error message must be the same', + ); + } + if (expectedMessage.messageId) { + assert.strictEqual( + expectedMessage.messageId, + message.messageId, + 'Error message ID must be the same', + ); + } + if (expectedMessage.type) { + assert.strictEqual( + expectedMessage.type, + message.nodeType, + 'Error message ID must be the same', + ); + } + + // eslint-disable-next-line deprecation/deprecation + if (expectedMessage.data) { + // TODO: Implement a compatible version + if (!this.#config.hideLegacyConfigLeftovers) { + throw new Error( + 'The data property of invalid testcases is unimplemented and deprecated, use hideLegacyConfigLeftovers to hide this', + ); + } + } + + if (expectedMessage.suggestions !== undefined) { + assert.ok( + message.suggestions !== undefined, + 'Suggestions must not be undefined', + ); + + assert.strictEqual( + expectedMessage.suggestions.length, + message.suggestions.length, + 'Result suggestions must be the same length', + ); + message.suggestions.forEach((suggestion, idx) => { + const expectedSuggestion = + // @ts-expect-error: TS forgot that we did an assertion + expectedMessage.suggestions[idx]; + + if (expectedSuggestion.desc) { + assert.strictEqual( + expectedSuggestion.desc, + suggestion.desc, + 'Error message ID must be the same', + ); + } + if (expectedSuggestion.output) { + assert.strictEqual( + expectedSuggestion.output, + suggestion.fix, + 'Error message ID must be the same', + ); + } + if (expectedSuggestion.messageId) { + assert.strictEqual( + expectedSuggestion.messageId, + suggestion.messageId, + 'Error message ID must be the same', + ); + } + }); + } + }); + }), + ), + ); + }); + }); + + for (const [_, value] of Object.entries( + // No actual cast done, just removing the type argument + this.#config.extensions as RuleTesterConfig['extensions'], + )) { + // eslint-disable-next-line @typescript-eslint/consistent-type-imports + type parser = typeof import('@typescript-eslint/parser'); + if ( + value.languageOptions?.parser && + (value.languageOptions.parser as parser | { clearCaches: undefined }) + .clearCaches + ) { + (value.languageOptions.parser as parser).clearCaches(); + } + } + } + } +} + +export class FlatRuleConfig< + MessageIds extends string, + Options extends unknown[], +> { + #name: string; + #configurations: FlatRuleConfiguration[]; + #rule: RuleModule; + + #findOrMakeConfiguration( + configuration: Options, + ): FlatRuleConfiguration { + let found: FlatRuleConfiguration | undefined = + this.#configurations.find(val => + isEqual(val[DATA]().configuration, configuration), + ); + if (!found) { + found = new FlatRuleConfiguration(configuration); + this.#configurations.push(found); + } + + return found; + } + + private constructor(name: string, rule: RuleModule) { + this.#name = name; + this.#rule = rule; + this.#configurations = []; + } + + /** + * Defines a new configuration + * + * Use this over fromObject + */ + configuration( + ...configuration: Options + ): FlatRuleConfiguration { + const opt = new FlatRuleConfiguration(configuration); + this.#configurations.push(opt); + return opt; + } + + /** + * Defines a group of rules from an object of a similar schema to the RuleTester.run() method + * Used for API compatibility + * + * Do not use for new rules + */ + fromObject( + defaultConfiguration: Options, + obj: { + valid: (LegacyValidSample | string)[]; + invalid: LegacyInvalidSample[]; + format?: string; + }, + ): void { + const format = obj.format ?? 'ts'; + + obj.valid.forEach(it => { + if (typeof it === 'string') { + it = { + code: it, + name: it, + options: undefined, + }; + + const config = this.#findOrMakeConfiguration( + it.options ?? defaultConfiguration, + ); + + config.valid(it.name ?? it.code, it.code, format); + } + }); + + obj.invalid.forEach(it => { + const config = this.#findOrMakeConfiguration( + it.options ?? defaultConfiguration, + ); + + config.invalid( + it.name ?? it.code, + it.code, + it.output ?? null, + it.errors, + format, + ); + }); + } + + [DATA](): { + configurations: FlatRuleConfiguration[]; + rule: RuleModule; + name: string; + } { + return { + configurations: this.#configurations, + rule: this.#rule, + name: this.#name, + }; + } + + static [CONSTRUCT]( + name: string, + rule: RuleModule, + ): FlatRuleConfig { + return new FlatRuleConfig(name, rule); + } +} + +/** + * A singular rule configuration, used to define valid and invalid tests + */ +export class FlatRuleConfiguration< + MessageIds extends string, + Config extends unknown[], +> { + #validSamples: ValidSample[]; + #invalidSamples: InvalidSample[]; + #configuration: Config; + + constructor(configuration: Config) { + this.#validSamples = []; + this.#invalidSamples = []; + this.#configuration = configuration; + } + + // We return this to allow method chaining + valid(name: string, code: string, format?: string): this { + this.#validSamples.push({ + code, + name, + format, + }); + return this; + } + + invalid( + name: string, + code: string, + output: string | null, + errors: ExpectedError[], + format?: string, + ): this { + this.#invalidSamples.push({ + code, + name, + output, + errors, + format, + }); + return this; + } + + [DATA](): { + valid: ValidSample[]; + invalid: InvalidSample[]; + configuration: Config; + } { + return { + valid: this.#validSamples, + invalid: this.#invalidSamples, + configuration: this.#configuration, + }; + } +} diff --git a/packages/rule-tester/src/index.ts b/packages/rule-tester/src/index.ts index 6ea08fc5addb..ecef80c9238f 100644 --- a/packages/rule-tester/src/index.ts +++ b/packages/rule-tester/src/index.ts @@ -1,4 +1,5 @@ export { RuleTester } from './RuleTester'; +export { FlatRuleTester } from './FlatRuleTester'; export { noFormat } from './noFormat'; export type { InvalidTestCase, diff --git a/packages/rule-tester/src/types/FlatRuleTester.ts b/packages/rule-tester/src/types/FlatRuleTester.ts new file mode 100644 index 000000000000..27090d6a4481 --- /dev/null +++ b/packages/rule-tester/src/types/FlatRuleTester.ts @@ -0,0 +1,113 @@ +import type { + AST_NODE_TYPES, + AST_TOKEN_TYPES, +} from '@typescript-eslint/typescript-estree'; +import type { FlatConfig } from '@typescript-eslint/utils/ts-eslint'; + +export type FileUnawareConfig = Omit; + +export interface RuleTesterConfig { + baseOptions: FileUnawareConfig; + /** + * A list of extension-specific configurations + * + * You must add an extension-specific configuration for each extension you use (For safety reasons, else you could mistype "ts" like "ys" and it wouldn't yell at you) + */ + extensions: { + [K in Formats]: FileUnawareConfig; + }; + + /** + * The directory where fixtures (Virtual files that make TypeScript and eslint plugins happy by providing a virtual file) are located + * + * compilerServices are mandatory to prevent bugs + * + * Requires a fixtures directory as such: + * ```plaintext + * (directory pointed to by fixtureRootDir, must be absolute) + * |-file.ts + * |-file.{any extra extension you need} + * |-tsconfig.json + * ``` + * `tsconfig.json` must be: + * ```json + * { + * "compilerOptions": { ##SET ANY OPTIONS HERE## }, + * "include": ["./file.*", "./file.ts"] + * } + * ``` + * In addition, ensure to put your fixture directory INSIDE of the project directory, else type errors will occur + */ + fixtureRootDir: string; + + /** + * For future use, do not set. + */ + fixtureMode?: undefined; + + /** + * RuleTester compatibility mode, hides some errors, including: + * - The data property of invalid test cases is unimplemented and deprecated + */ + hideLegacyConfigLeftovers?: boolean; +} + +export interface ExpectedSuggestion { + desc?: string; + output?: string; + messageId?: MessageIds; +} + +export interface ExpectedError { + line?: number; + column?: number; + endLine?: number; + endColumn?: number; + message?: string; + messageId?: MessageIds; + suggestions?: ExpectedSuggestion[]; + type?: /* Trick to make it show up as a string */ + AST_NODE_TYPES | `${AST_NODE_TYPES}` | AST_TOKEN_TYPES | `${AST_TOKEN_TYPES}`; + /** + * Not implemented, here for parity, DO NOT USE + * + * @deprecated + */ + data?: unknown; +} + +export interface Sample { + name: string; + code: string; + format?: string; +} + +export type ValidSample = Sample; + +export interface InvalidSample extends Sample { + errors: ExpectedError[]; + output: string | null; +} + +/** + * Used for legacy (ESLint RuleTester) compatibility. + */ +export interface LegacyValidSample { + name?: string; + code: string; + options?: Options; +} + +/** + * Used for legacy (core ESLint RuleTester) compatibility, + */ +export interface LegacyInvalidSample< + MessageIds extends string, + Options extends unknown[], +> { + name?: string; + code: string; + errors: ExpectedError[]; + options?: Options; + output?: string | null; +} diff --git a/packages/rule-tester/tests/FlatRuleTester.test.ts b/packages/rule-tester/tests/FlatRuleTester.test.ts new file mode 100644 index 000000000000..11d7a4ec9689 --- /dev/null +++ b/packages/rule-tester/tests/FlatRuleTester.test.ts @@ -0,0 +1,111 @@ +/// +/* eslint-disable @typescript-eslint/explicit-function-return-type */ +import { join } from 'node:path'; + +import { AST_NODE_TYPES } from '@typescript-eslint/typescript-estree'; + +import { FlatRuleTester } from '../src/FlatRuleTester'; + +const tester = new FlatRuleTester({ + baseOptions: { + languageOptions: { + parser: require('@typescript-eslint/parser'), + parserOptions: { + ecmaFeatures: {}, + ecmaVersion: 2024, + }, + }, + }, + extensions: { + ts: {}, + }, + fixtureRootDir: join(__dirname, './fixtures'), +}); + +const myRule = tester.rule('my-rule', { + meta: { + messages: { + disallowedName: 'disallowed', + suggest: 'suggest', + }, + schema: [ + { + type: 'array', + items: { + type: 'string', + }, + }, + ], + type: 'problem', + hasSuggestions: true, + }, + create(ctx) { + return { + FunctionDeclaration(node) { + if ( + ctx.options.length === 1 && + node.id != null && + ctx.options[0].includes(node.id.name) + ) { + return; + } + + return ctx.report({ + node, + messageId: 'disallowedName', + suggest: [ + { + fix(fixer) { + return [fixer.remove(node)]; + }, + messageId: 'suggest', + }, + ], + }); + }, + }; + }, + defaultOptions: [['string']] as [string[]] | [], +}); + +const hello = myRule.configuration(['hello']); +hello.valid('Hello function', 'function hello() {}'); +hello.invalid('Not a hello function', 'function nothello() {}', null, [ + { + messageId: 'disallowedName', + suggestions: [ + { + messageId: 'suggest', + }, + ], + }, +]); + +const again = myRule.configuration(['again']); +again.valid('Again function', 'function again() {}'); +again.invalid('Not an again function', 'function notagain() {}', null, [ + { + messageId: 'disallowedName', + suggestions: [ + { + messageId: 'suggest', + }, + ], + }, +]); + +myRule.fromObject([], { + valid: [], + invalid: [ + { + code: `function test() {}`, + errors: [ + { + type: AST_NODE_TYPES.FunctionDeclaration, + }, + ], + }, + ], +}); + +tester.test(); diff --git a/packages/rule-tester/tests/fixtures/file.ts b/packages/rule-tester/tests/fixtures/file.ts new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/packages/rule-tester/tests/fixtures/tsconfig.json b/packages/rule-tester/tests/fixtures/tsconfig.json new file mode 100644 index 000000000000..0f2b5aeb649e --- /dev/null +++ b/packages/rule-tester/tests/fixtures/tsconfig.json @@ -0,0 +1,4 @@ +{ + "compilerOptions": {}, + "include": ["./file.ts"] +} diff --git a/packages/rule-tester/tsconfig.json b/packages/rule-tester/tsconfig.json index 9cea515ba6b2..6d58b792d5e2 100644 --- a/packages/rule-tester/tsconfig.json +++ b/packages/rule-tester/tsconfig.json @@ -4,5 +4,6 @@ "composite": false, "rootDir": "." }, - "include": ["src", "typings", "tests", "tools"] + "include": ["src", "typings", "tests", "tools"], + "exclude": ["tests/fixtures"] } diff --git a/yarn.lock b/yarn.lock index 0f88a624ba67..53ed20f5b03a 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5709,6 +5709,7 @@ __metadata: espree: ^10.0.1 esprima: ^4.0.1 json-stable-stringify-without-jsonify: ^1.0.1 + lodash: ^4.6.2 lodash.merge: 4.6.2 mocha: ^10.4.0 semver: ^7.6.0 @@ -13811,7 +13812,7 @@ __metadata: languageName: node linkType: hard -"lodash@npm:^4.17.20, lodash@npm:^4.17.21, lodash@npm:~4.17.15": +"lodash@npm:^4.17.20, lodash@npm:^4.17.21, lodash@npm:^4.6.2, lodash@npm:~4.17.15": version: 4.17.21 resolution: "lodash@npm:4.17.21" checksum: eb835a2e51d381e561e508ce932ea50a8e5a68f4ebdd771ea240d3048244a8d13658acbd502cd4829768c56f2e16bdd4340b9ea141297d472517b83868e677f7 From b5490ef32643e862e6f81aa243ba9738cb7503c1 Mon Sep 17 00:00:00 2001 From: Luca Mollema Date: Tue, 11 Jun 2024 09:21:10 +0200 Subject: [PATCH 2/7] fix(rule-tester) Fix many issues with FlatRuleTester --- .../eslint-plugin/tests/fixtures/file.tsx | 0 packages/rule-tester/src/FlatRuleTester.ts | 68 +++++++++++-------- .../rule-tester/src/types/FlatRuleTester.ts | 8 +-- 3 files changed, 42 insertions(+), 34 deletions(-) create mode 100644 packages/eslint-plugin/tests/fixtures/file.tsx diff --git a/packages/eslint-plugin/tests/fixtures/file.tsx b/packages/eslint-plugin/tests/fixtures/file.tsx new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/packages/rule-tester/src/FlatRuleTester.ts b/packages/rule-tester/src/FlatRuleTester.ts index b93f0a07ccc5..9527c2691f15 100644 --- a/packages/rule-tester/src/FlatRuleTester.ts +++ b/packages/rule-tester/src/FlatRuleTester.ts @@ -23,6 +23,8 @@ import type { Sample, ValidSample, } from './types/FlatRuleTester'; +import { interpolate } from './utils/interpolate'; +import * as SourceCodeFixer from './utils/SourceCodeFixer'; import { sanitize } from './utils/validationHelpers'; // Hide methods publicly by default, for simplicity's sake @@ -52,7 +54,6 @@ export declare namespace FlatRuleTester { * * List of current breaking changes: * - Uses ESLint's flat config system - * - The data property of errors in invalid cases is unsupported (ESLint does not expose its interpolate method, it is not a permanent solution to copy the implementation) * - Requires type-checking * - Does not support string imports for parsers */ @@ -166,11 +167,11 @@ export class FlatRuleTester extends TestFramework { FlatRuleTester.describe('valid', () => data.valid.forEach(validCase => FlatRuleTester.it(validCase.name, () => { - if (validCase.format) { - assert.ok(validCase.format in this.#config.extensions); + if (validCase.extension) { + assert.ok(validCase.extension in this.#config.extensions); } - const name = `/file.${validCase.format ?? 'ts'}`; + const name = `/file.${validCase.extension ?? 'ts'}`; assert.ok( existsSync(join(this.#fixtureDir, name)), `Fixtures must exist for format: ${name}`, @@ -183,12 +184,12 @@ export class FlatRuleTester extends TestFramework { files: ['**/*.*'], }, ]; - if (validCase.format) { + if (validCase.extension) { const formatConfig = - this.#config.extensions[validCase.format as F]; + this.#config.extensions[validCase.extension as F]; fullConfig.splice(1, 0, { ...formatConfig, - files: [`**/*.${validCase.format}`], + files: [`**/*.${validCase.extension}`], }); } @@ -209,14 +210,14 @@ export class FlatRuleTester extends TestFramework { FlatRuleTester.describe('invalid', () => data.invalid.forEach(invalidCase => FlatRuleTester.it(sanitize(invalidCase.name), () => { - if (invalidCase.format) { + if (invalidCase.extension) { assert.ok( - invalidCase.format in this.#config.extensions, - `Undefined format: ${sanitize(invalidCase.format)}, should be in ${sanitize(Object.keys(this.#config.extensions).join(', '))}`, + invalidCase.extension in this.#config.extensions, + `Undefined format: ${sanitize(invalidCase.extension)}, should be in ${sanitize(Object.keys(this.#config.extensions).join(', '))}`, ); } - const name = `/file.${invalidCase.format ?? 'ts'}`; + const name = `/file.${invalidCase.extension ?? 'ts'}`; assert.ok( existsSync(join(this.#fixtureDir, name)), 'Fixtures must exist', @@ -229,12 +230,12 @@ export class FlatRuleTester extends TestFramework { files: ['**/*.*'], }, ]; - if (invalidCase.format) { + if (invalidCase.extension) { const formatConfig = - this.#config.extensions[invalidCase.format as F]; + this.#config.extensions[invalidCase.extension as F]; fullConfig.splice(1, 0, { ...formatConfig, - files: [`**/*.${invalidCase.format}`], + files: [`**/*.${invalidCase.extension}`], }); } @@ -342,12 +343,20 @@ export class FlatRuleTester extends TestFramework { // eslint-disable-next-line deprecation/deprecation if (expectedMessage.data) { - // TODO: Implement a compatible version - if (!this.#config.hideLegacyConfigLeftovers) { - throw new Error( - 'The data property of invalid testcases is unimplemented and deprecated, use hideLegacyConfigLeftovers to hide this', - ); - } + this.#config; + assert.ok(message.messageId !== undefined); + + const hydrated = interpolate( + rule.meta.messages[message.messageId], + // eslint-disable-next-line deprecation/deprecation + expectedMessage.data as Record, + ); + + assert.strictEqual( + message.message, + hydrated, + 'Message must be equal to data-hydrated message', + ); } if (expectedMessage.suggestions !== undefined) { @@ -370,21 +379,23 @@ export class FlatRuleTester extends TestFramework { assert.strictEqual( expectedSuggestion.desc, suggestion.desc, - 'Error message ID must be the same', + 'Suggestion description must be the same', ); } if (expectedSuggestion.output) { assert.strictEqual( expectedSuggestion.output, - suggestion.fix, - 'Error message ID must be the same', + SourceCodeFixer.applyFixes(results.output, [ + suggestion, + ]).output, + 'Suggestion must be the same must be the same', ); } if (expectedSuggestion.messageId) { assert.strictEqual( expectedSuggestion.messageId, suggestion.messageId, - 'Error message ID must be the same', + 'Suggestion message ID must be the same', ); } }); @@ -463,7 +474,7 @@ export class FlatRuleConfig< * Do not use for new rules */ fromObject( - defaultConfiguration: Options, + defaultConfiguration: Options | null, obj: { valid: (LegacyValidSample | string)[]; invalid: LegacyInvalidSample[]; @@ -471,6 +482,9 @@ export class FlatRuleConfig< }, ): void { const format = obj.format ?? 'ts'; + if (defaultConfiguration == null) { + defaultConfiguration = this.#rule.defaultOptions; + } obj.valid.forEach(it => { if (typeof it === 'string') { @@ -545,7 +559,7 @@ export class FlatRuleConfiguration< this.#validSamples.push({ code, name, - format, + extension: format, }); return this; } @@ -562,7 +576,7 @@ export class FlatRuleConfiguration< name, output, errors, - format, + extension: format, }); return this; } diff --git a/packages/rule-tester/src/types/FlatRuleTester.ts b/packages/rule-tester/src/types/FlatRuleTester.ts index 27090d6a4481..512f6ae714ce 100644 --- a/packages/rule-tester/src/types/FlatRuleTester.ts +++ b/packages/rule-tester/src/types/FlatRuleTester.ts @@ -44,12 +44,6 @@ export interface RuleTesterConfig { * For future use, do not set. */ fixtureMode?: undefined; - - /** - * RuleTester compatibility mode, hides some errors, including: - * - The data property of invalid test cases is unimplemented and deprecated - */ - hideLegacyConfigLeftovers?: boolean; } export interface ExpectedSuggestion { @@ -79,7 +73,7 @@ export interface ExpectedError { export interface Sample { name: string; code: string; - format?: string; + extension?: string; } export type ValidSample = Sample; From 60c17d63d4acddc63b14f98b38549c32b31d9d7c Mon Sep 17 00:00:00 2001 From: Luca Mollema Date: Tue, 11 Jun 2024 09:43:55 +0200 Subject: [PATCH 3/7] fix(rule-tester) Add defaultExtension --- packages/rule-tester/src/FlatRuleTester.ts | 65 ++++++++++--------- .../rule-tester/src/types/FlatRuleTester.ts | 5 ++ 2 files changed, 39 insertions(+), 31 deletions(-) diff --git a/packages/rule-tester/src/FlatRuleTester.ts b/packages/rule-tester/src/FlatRuleTester.ts index 9527c2691f15..d79963aa852f 100644 --- a/packages/rule-tester/src/FlatRuleTester.ts +++ b/packages/rule-tester/src/FlatRuleTester.ts @@ -167,14 +167,18 @@ export class FlatRuleTester extends TestFramework { FlatRuleTester.describe('valid', () => data.valid.forEach(validCase => FlatRuleTester.it(validCase.name, () => { - if (validCase.extension) { - assert.ok(validCase.extension in this.#config.extensions); - } + const extension: F = (validCase.extension ?? + this.#config.defaultExtension ?? + 'ts') as F; + assert.ok( + Object.hasOwn(this.#config.extensions, extension), + `Undefined format: ${sanitize(extension)}, should be in ${sanitize(Object.keys(this.#config.extensions).join(', '))}`, + ); - const name = `/file.${validCase.extension ?? 'ts'}`; + const name = `/file.${extension}`; assert.ok( existsSync(join(this.#fixtureDir, name)), - `Fixtures must exist for format: ${name}`, + `Fixtures must exist for format: ${sanitize(name)}`, ); const fullConfig: FlatConfig.ConfigArray = [ @@ -184,14 +188,15 @@ export class FlatRuleTester extends TestFramework { files: ['**/*.*'], }, ]; - if (validCase.extension) { - const formatConfig = - this.#config.extensions[validCase.extension as F]; - fullConfig.splice(1, 0, { - ...formatConfig, - files: [`**/*.${validCase.extension}`], - }); - } + + const formatConfig = this.#config.extensions[extension]; + + assert.notStrictEqual(formatConfig, undefined); + + fullConfig.splice(1, 0, { + ...formatConfig, + files: [`**/*.${extension}`], + }); const results = this.#linter.verify( validCase.code, @@ -201,7 +206,7 @@ export class FlatRuleTester extends TestFramework { results.forEach(msg => { throw new Error( - `Got error ${msg.message} (${msg.messageId}), expected nothing`, + `Got error ${sanitize(msg.message)} (${msg.messageId ? sanitize(msg.messageId) : '(unknown message ID)'}), expected nothing`, ); }); }), @@ -210,14 +215,15 @@ export class FlatRuleTester extends TestFramework { FlatRuleTester.describe('invalid', () => data.invalid.forEach(invalidCase => FlatRuleTester.it(sanitize(invalidCase.name), () => { - if (invalidCase.extension) { - assert.ok( - invalidCase.extension in this.#config.extensions, - `Undefined format: ${sanitize(invalidCase.extension)}, should be in ${sanitize(Object.keys(this.#config.extensions).join(', '))}`, - ); - } + const extension: F = (invalidCase.extension ?? + this.#config.defaultExtension ?? + 'ts') as F; + assert.ok( + Object.hasOwn(this.#config.extensions, extension), + `Undefined format: ${sanitize(extension)}, should be in ${sanitize(Object.keys(this.#config.extensions).join(', '))}`, + ); - const name = `/file.${invalidCase.extension ?? 'ts'}`; + const name = `/file.${extension}`; assert.ok( existsSync(join(this.#fixtureDir, name)), 'Fixtures must exist', @@ -230,14 +236,11 @@ export class FlatRuleTester extends TestFramework { files: ['**/*.*'], }, ]; - if (invalidCase.extension) { - const formatConfig = - this.#config.extensions[invalidCase.extension as F]; - fullConfig.splice(1, 0, { - ...formatConfig, - files: [`**/*.${invalidCase.extension}`], - }); - } + const formatConfig = this.#config.extensions[extension]; + fullConfig.splice(1, 0, { + ...formatConfig, + files: [`**/*.${invalidCase.extension}`], + }); const allMessages: Linter.LintMessage[] = []; const results = this.#linter.verifyAndFix( @@ -281,12 +284,12 @@ export class FlatRuleTester extends TestFramework { results.messages.forEach((message, idx) => { assert.ok( message.ruleId != null, - `Parser/linter error occured: ${message.message}`, + `Parser/linter error occured: ${sanitize(message.message)}`, ); assert.ok( message.ruleId === 'rule/rule', - `Errors must be from the defined rule, as opposed to typescript-eslint or eslint: ${message.ruleId}`, + `Errors must be from the defined rule, as opposed to typescript-eslint or eslint: ${sanitize(message.ruleId)}`, ); const expectedMessage = invalidCase.errors[idx]; diff --git a/packages/rule-tester/src/types/FlatRuleTester.ts b/packages/rule-tester/src/types/FlatRuleTester.ts index 512f6ae714ce..b26fbc5d1675 100644 --- a/packages/rule-tester/src/types/FlatRuleTester.ts +++ b/packages/rule-tester/src/types/FlatRuleTester.ts @@ -44,6 +44,11 @@ export interface RuleTesterConfig { * For future use, do not set. */ fixtureMode?: undefined; + + /** + * Default extension + */ + defaultExtension?: NoInfer; } export interface ExpectedSuggestion { From 8d9de4f18e41162596716ee17898941bc703d90f Mon Sep 17 00:00:00 2001 From: Luca Mollema Date: Tue, 11 Jun 2024 09:50:28 +0200 Subject: [PATCH 4/7] docs(rule-tester) Minor documentation update --- packages/rule-tester/src/FlatRuleTester.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/rule-tester/src/FlatRuleTester.ts b/packages/rule-tester/src/FlatRuleTester.ts index d79963aa852f..adfca0d8c707 100644 --- a/packages/rule-tester/src/FlatRuleTester.ts +++ b/packages/rule-tester/src/FlatRuleTester.ts @@ -56,6 +56,7 @@ export declare namespace FlatRuleTester { * - Uses ESLint's flat config system * - Requires type-checking * - Does not support string imports for parsers + * - Does not support modifying parserOptions on a per-case basis (For performance reasons) */ export class FlatRuleTester extends TestFramework { #config: Readonly>; From d1b5b9e6f88ced0d16a19b93a302abc4ed8fa576 Mon Sep 17 00:00:00 2001 From: Luca Mollema Date: Tue, 11 Jun 2024 09:59:52 +0200 Subject: [PATCH 5/7] docs(rule-tester): Document [DATA] methods in FlatRuleTester --- packages/rule-tester/src/FlatRuleTester.ts | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/packages/rule-tester/src/FlatRuleTester.ts b/packages/rule-tester/src/FlatRuleTester.ts index adfca0d8c707..d85490a82642 100644 --- a/packages/rule-tester/src/FlatRuleTester.ts +++ b/packages/rule-tester/src/FlatRuleTester.ts @@ -520,7 +520,10 @@ export class FlatRuleConfig< ); }); } - + /** + * Gets all configurations and the associated rule from a rule instance + * Is a symbol to prevent cluttering up available methods + */ [DATA](): { configurations: FlatRuleConfiguration[]; rule: RuleModule; @@ -585,6 +588,10 @@ export class FlatRuleConfiguration< return this; } + /** + * Gets all samples and configuration from a configuration instance + * Is a symbol to prevent cluttering up available methods + */ [DATA](): { valid: ValidSample[]; invalid: InvalidSample[]; From 99cf495b0a70a43e21bd5a7b1f14d89d20350ec5 Mon Sep 17 00:00:00 2001 From: Luca Mollema Date: Tue, 11 Jun 2024 10:34:13 +0200 Subject: [PATCH 6/7] fix(rule-tester): Use hasOwnProperty as opposed to hasOwn --- packages/rule-tester/src/FlatRuleTester.ts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/packages/rule-tester/src/FlatRuleTester.ts b/packages/rule-tester/src/FlatRuleTester.ts index d85490a82642..005ce00f5a3f 100644 --- a/packages/rule-tester/src/FlatRuleTester.ts +++ b/packages/rule-tester/src/FlatRuleTester.ts @@ -172,7 +172,10 @@ export class FlatRuleTester extends TestFramework { this.#config.defaultExtension ?? 'ts') as F; assert.ok( - Object.hasOwn(this.#config.extensions, extension), + Object.prototype.hasOwnProperty.call( + this.#config.extensions, + extension, + ), `Undefined format: ${sanitize(extension)}, should be in ${sanitize(Object.keys(this.#config.extensions).join(', '))}`, ); @@ -220,7 +223,10 @@ export class FlatRuleTester extends TestFramework { this.#config.defaultExtension ?? 'ts') as F; assert.ok( - Object.hasOwn(this.#config.extensions, extension), + Object.prototype.hasOwnProperty.call( + this.#config.extensions, + extension, + ), `Undefined format: ${sanitize(extension)}, should be in ${sanitize(Object.keys(this.#config.extensions).join(', '))}`, ); From 3a7ae98779d4dea715e19987120f7046a0d8a164 Mon Sep 17 00:00:00 2001 From: Luca Mollema Date: Tue, 11 Jun 2024 11:37:14 +0200 Subject: [PATCH 7/7] fix(rule-tester): Fix knip.ts to ignore fixtures in rule-tester --- knip.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/knip.ts b/knip.ts index 25de69c9596b..f3ba79ad6c30 100644 --- a/knip.ts +++ b/knip.ts @@ -58,6 +58,9 @@ export default { 'packages/parser': { ignore: ['tests/fixtures/**'], }, + 'packages/rule-tester': { + ignore: ['tests/fixtures/**'], + }, 'packages/scope-manager': { ignore: ['tests/fixtures/**'], },