From 25230bd02f96b9576e4036a0c9df5caec6287192 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Tue, 30 Jul 2024 13:21:16 -0400 Subject: [PATCH 1/6] fix(rule-tester): provide Linter a cwd in its constructor --- packages/rule-tester/src/RuleTester.ts | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/packages/rule-tester/src/RuleTester.ts b/packages/rule-tester/src/RuleTester.ts index 07c95b83c23e..0a22bbf4c725 100644 --- a/packages/rule-tester/src/RuleTester.ts +++ b/packages/rule-tester/src/RuleTester.ts @@ -167,7 +167,7 @@ function getUnsubstitutedMessagePlaceholders( export class RuleTester extends TestFramework { readonly #testerConfig: TesterConfigWithDefaults; readonly #rules: Record = {}; - readonly #linter: Linter = new Linter({ configType: 'flat' }); + readonly #linter: Linter; /** * Creates a new instance of RuleTester. @@ -175,6 +175,11 @@ export class RuleTester extends TestFramework { constructor(testerConfig?: RuleTesterConfig) { super(); + this.#linter = new Linter({ + configType: 'flat', + cwd: testerConfig?.languageOptions?.parserOptions?.tsconfigRootDir, + }); + /** * The configuration to use for this tester. Combination of the tester * configuration and the default configuration. @@ -707,16 +712,7 @@ export class RuleTester extends TestFramework { }, }, }); - messages = this.#linter.verify( - code, - // ESLint uses an internal FlatConfigArray that extends @humanwhocodes/config-array. - // Linter uses a typeof getConfig === "function" check. - // We mock out that check here to force it not to use Linter's cwd as basePath. - Object.assign([], { - getConfig: () => actualConfig, - }), - filename, - ); + messages = this.#linter.verify(code, actualConfig, filename); } finally { SourceCode.prototype.applyInlineConfig = applyInlineConfig; SourceCode.prototype.applyLanguageOptions = applyLanguageOptions; From a3b6a9279d48908433203dcacd3e903b5e03db36 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Tue, 30 Jul 2024 13:36:07 -0400 Subject: [PATCH 2/6] chore: use private testerConfig --- packages/rule-tester/src/RuleTester.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/rule-tester/src/RuleTester.ts b/packages/rule-tester/src/RuleTester.ts index 0a22bbf4c725..a4a834b06fb4 100644 --- a/packages/rule-tester/src/RuleTester.ts +++ b/packages/rule-tester/src/RuleTester.ts @@ -175,11 +175,6 @@ export class RuleTester extends TestFramework { constructor(testerConfig?: RuleTesterConfig) { super(); - this.#linter = new Linter({ - configType: 'flat', - cwd: testerConfig?.languageOptions?.parserOptions?.tsconfigRootDir, - }); - /** * The configuration to use for this tester. Combination of the tester * configuration and the default configuration. @@ -188,6 +183,11 @@ export class RuleTester extends TestFramework { rules: { [`${RULE_TESTER_PLUGIN_PREFIX}validate-ast`]: 'error' }, }); + this.#linter = new Linter({ + configType: 'flat', + cwd: this.#testerConfig.languageOptions.parserOptions?.tsconfigRootDir, + }); + // make sure that the parser doesn't hold onto file handles between tests // on linux (i.e. our CI env), there can be very a limited number of watch handles available const constructor = this.constructor as typeof RuleTester; From 0060a7493fb7d5f8ee66af52056d44b803b4bb31 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Tue, 30 Jul 2024 14:18:38 -0400 Subject: [PATCH 3/6] fix internal test relativity --- .../rules/no-relative-paths-to-internal-packages.test.ts | 8 +++++++- packages/rule-tester/src/RuleTester.ts | 1 + 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/packages/eslint-plugin-internal/tests/rules/no-relative-paths-to-internal-packages.test.ts b/packages/eslint-plugin-internal/tests/rules/no-relative-paths-to-internal-packages.test.ts index 0f48ae9a2c45..3f29b69e50e0 100644 --- a/packages/eslint-plugin-internal/tests/rules/no-relative-paths-to-internal-packages.test.ts +++ b/packages/eslint-plugin-internal/tests/rules/no-relative-paths-to-internal-packages.test.ts @@ -5,7 +5,13 @@ import rule, { PACKAGES_DIR, } from '../../src/rules/no-relative-paths-to-internal-packages'; -const ruleTester = new RuleTester(); +const ruleTester = new RuleTester({ + languageOptions: { + parserOptions: { + tsconfigRootDir: PACKAGES_DIR, + }, + }, +}); ruleTester.run('no-relative-paths-to-internal-packages', rule, { valid: [ diff --git a/packages/rule-tester/src/RuleTester.ts b/packages/rule-tester/src/RuleTester.ts index a4a834b06fb4..4e010d7e865e 100644 --- a/packages/rule-tester/src/RuleTester.ts +++ b/packages/rule-tester/src/RuleTester.ts @@ -913,6 +913,7 @@ export class RuleTester extends TestFramework { const hasMessageOfThisRule = messages.some(m => m.ruleId === ruleName); + // console.log({ messages }); for (let i = 0, l = item.errors.length; i < l; i++) { const error = item.errors[i]; const message = messages[i]; From bd4607d9cc27870e63babe4bfbfa455a6043176b Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Tue, 30 Jul 2024 14:49:57 -0400 Subject: [PATCH 4/6] fix: always normalize test name --- packages/rule-tester/src/RuleTester.ts | 40 ++++++++++++++------------ 1 file changed, 21 insertions(+), 19 deletions(-) diff --git a/packages/rule-tester/src/RuleTester.ts b/packages/rule-tester/src/RuleTester.ts index 4e010d7e865e..ab429606bc8b 100644 --- a/packages/rule-tester/src/RuleTester.ts +++ b/packages/rule-tester/src/RuleTester.ts @@ -287,14 +287,19 @@ export class RuleTester extends TestFramework { Hugely helps with the string-based valid test cases as it means they don't need to be made objects! */ - const getFilename = (testOptions?: ParserOptions): string => { + const getFilename = ( + originalFilename: string | undefined, + testOptions: ParserOptions | undefined, + ): string => { const resolvedOptions = deepMerge( this.#testerConfig.languageOptions.parserOptions, testOptions, ) as ParserOptions; - const filename = resolvedOptions.ecmaFeatures?.jsx - ? this.#testerConfig.defaultFilenames.tsx - : this.#testerConfig.defaultFilenames.ts; + const filename = + originalFilename ?? + (resolvedOptions.ecmaFeatures?.jsx + ? this.#testerConfig.defaultFilenames.tsx + : this.#testerConfig.defaultFilenames.ts); if (resolvedOptions.project) { return path.join( resolvedOptions.tsconfigRootDir ?? process.cwd(), @@ -314,22 +319,19 @@ export class RuleTester extends TestFramework { if (languageOptions.parser === parser) { throw new Error(DUPLICATE_PARSER_ERROR_MESSAGE); } - if (!test.filename) { - return { - ...test, - filename: getFilename(languageOptions.parserOptions), - languageOptions: { - ...languageOptions, - parserOptions: { - // Re-running simulates --fix mode, which implies an isolated program - // (i.e. parseAndGenerateServicesCalls[test.filename] > 1). - disallowAutomaticSingleRunInference: true, - ...languageOptions.parserOptions, - }, + return { + ...test, + filename: getFilename(test.filename, languageOptions.parserOptions), + languageOptions: { + ...languageOptions, + parserOptions: { + // Re-running simulates --fix mode, which implies an isolated program + // (i.e. parseAndGenerateServicesCalls[test.filename] > 1). + disallowAutomaticSingleRunInference: true, + ...languageOptions.parserOptions, }, - }; - } - return test; + }, + }; }; const normalizedTests = { From 60075d2489f76f28ff931b66f2575fb2d2b3dd7c Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Tue, 30 Jul 2024 14:52:00 -0400 Subject: [PATCH 5/6] Why did we even want to test espree in the first place --- .../tests/rules/no-use-before-define.test.ts | 24 ------------------- 1 file changed, 24 deletions(-) diff --git a/packages/eslint-plugin/tests/rules/no-use-before-define.test.ts b/packages/eslint-plugin/tests/rules/no-use-before-define.test.ts index be7cd2ac7730..c1e0f29bbb31 100644 --- a/packages/eslint-plugin/tests/rules/no-use-before-define.test.ts +++ b/packages/eslint-plugin/tests/rules/no-use-before-define.test.ts @@ -722,30 +722,6 @@ function a() {} }, { code: ` -'use strict'; -a(); -{ - function a() {} -} - `, - languageOptions: { - // eslint-disable-next-line @typescript-eslint/no-require-imports - parser: require('espree'), - parserOptions: { - ecmaVersion: 6, - sourceType: 'script', - }, - }, - errors: [ - { - messageId: 'noUseBeforeDefine', - data: { name: 'a' }, - type: AST_NODE_TYPES.Identifier, - }, - ], - }, - { - code: ` a(); try { throw new Error(); From 09c739c8ae2ced6f93fa2981a4b7131114396336 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Tue, 30 Jul 2024 15:01:29 -0400 Subject: [PATCH 6/6] remove unnecessary espree --- packages/eslint-plugin/package.json | 1 - yarn.lock | 1 - 2 files changed, 2 deletions(-) diff --git a/packages/eslint-plugin/package.json b/packages/eslint-plugin/package.json index 4d876425df5b..239e8c1c3cf4 100644 --- a/packages/eslint-plugin/package.json +++ b/packages/eslint-plugin/package.json @@ -80,7 +80,6 @@ "cross-env": "^7.0.3", "cross-fetch": "*", "eslint": "*", - "espree": "^10.0.1", "jest": "29.7.0", "jest-specific-snapshot": "^8.0.0", "json-schema": "*", diff --git a/yarn.lock b/yarn.lock index 2cb664a08646..a0d80f9b5b44 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5640,7 +5640,6 @@ __metadata: cross-env: ^7.0.3 cross-fetch: "*" eslint: "*" - espree: ^10.0.1 graphemer: ^1.4.0 ignore: ^5.3.1 jest: 29.7.0