From 27960a3a9ba40aff067d9c732c26ddbba479e933 Mon Sep 17 00:00:00 2001 From: "Alexander S." Date: Fri, 8 Nov 2024 17:39:50 +0100 Subject: [PATCH] fix: put `react-hooks` plugin rules into own scope and not under `react` (#236) closes #233 --- scripts/constants.ts | 7 +++++++ scripts/traverse-rules.ts | 21 ++++++++++++++++++--- src/__snapshots__/configs.spec.ts.snap | 6 +++--- src/build-from-oxlint-config.spec.ts | 4 ++-- src/build-from-oxlint-config.ts | 17 ++++++++++++++++- src/generated/configs-by-scope.ts | 6 ++++++ src/generated/rules-by-category.ts | 2 +- src/generated/rules-by-scope.ts | 6 +++++- 8 files changed, 58 insertions(+), 11 deletions(-) diff --git a/scripts/constants.ts b/scripts/constants.ts index 926c5dd..ad40951 100644 --- a/scripts/constants.ts +++ b/scripts/constants.ts @@ -65,6 +65,13 @@ export const typescriptRulesExtendEslintRules = [ 'space-infix-ops', ]; +// All rules from `eslint-plugin-react-hooks` +// Since oxlint supports these rules under react/*, we need to remap them. +export const reactHookRulesInsideReactScope = [ + 'rules-of-hooks', + 'exhaustive-deps', +]; + export function convertScope(scope: string) { return Reflect.has(scopeMaps, scope) ? scopeMaps[scope as 'eslint'] diff --git a/scripts/traverse-rules.ts b/scripts/traverse-rules.ts index a5132b1..1ada495 100644 --- a/scripts/traverse-rules.ts +++ b/scripts/traverse-rules.ts @@ -3,6 +3,7 @@ import path from 'node:path'; import { ignoreScope, prefixScope, + reactHookRulesInsideReactScope, SPARSE_CLONE_DIRECTORY, TARGET_DIRECTORY, typescriptRulesExtendEslintRules, @@ -70,13 +71,27 @@ async function processFile( let match = blockRegex.exec(content); // 'ok' way to get the scope, depends on the directory structure - const scope = getFolderNameUnderRules(filePath); + let scope = getFolderNameUnderRules(filePath); const shouldIgnoreRule = ignoreScope.has(scope); // when the file is called `mod.rs` we want to use the parent directory name as the rule name // Note that this is fairly brittle, as relying on the directory structure can be risky - let effectiveRuleName = `${prefixScope(scope)}${getFileNameWithoutExtension(filePath, currentDirectory)}`; - effectiveRuleName = effectiveRuleName.replaceAll('_', '-'); + const ruleNameWithoutScope = getFileNameWithoutExtension( + filePath, + currentDirectory + ).replaceAll('_', '-'); + + // All rules from `eslint-plugin-react-hooks` + // Since oxlint supports these rules under react/*, we need to remap them. + if ( + scope === 'react' && + reactHookRulesInsideReactScope.includes(ruleNameWithoutScope) + ) { + scope = 'react_hooks'; + } + + const effectiveRuleName = + `${prefixScope(scope)}${ruleNameWithoutScope}`.replaceAll('_', '-'); // add the rule to the skipped array and continue to see if there's a match regardless if (shouldIgnoreRule) { diff --git a/src/__snapshots__/configs.spec.ts.snap b/src/__snapshots__/configs.spec.ts.snap index 58b0daf..9d97956 100644 --- a/src/__snapshots__/configs.spec.ts.snap +++ b/src/__snapshots__/configs.spec.ts.snap @@ -881,6 +881,9 @@ exports[`contains all the oxlint rules 1`] = ` "radix": [ 0, ], + "react-hooks/rules-of-hooks": [ + 0, + ], "react-perf/jsx-no-jsx-as-prop": [ 0, ], @@ -971,9 +974,6 @@ exports[`contains all the oxlint rules 1`] = ` "react/require-render-return": [ 0, ], - "react/rules-of-hooks": [ - 0, - ], "react/self-closing-comp": [ 0, ], diff --git a/src/build-from-oxlint-config.spec.ts b/src/build-from-oxlint-config.spec.ts index 4742d8d..f7918d6 100644 --- a/src/build-from-oxlint-config.spec.ts +++ b/src/build-from-oxlint-config.spec.ts @@ -134,7 +134,7 @@ describe('buildFromOxlintConfig', () => { 'react_perf/jsx-no-new-array-as-prop': 'warn', 'nextjs/no-img-element': 'warn', 'jsx_a11y/alt-text': 'warn', - // 'react/rules-of-hooks': 'warn', -- ToDo oxc-project/eslint-plugin-oxlint#233 + 'react/rules-of-hooks': 'warn', // 'deepscan/xxx': 'warn', }, }); @@ -148,7 +148,7 @@ describe('buildFromOxlintConfig', () => { ); expect('@next/next/no-img-element' in configs[0].rules!).toBe(true); expect('jsx-a11y/alt-text' in configs[0].rules!).toBe(true); - // expect('react-hooks/rules-of-hooks' in rules[0].rules!).toBe(true); + expect('react-hooks/rules-of-hooks' in configs[0].rules!).toBe(true); }); it('detects rules without plugin name', () => { diff --git a/src/build-from-oxlint-config.ts b/src/build-from-oxlint-config.ts index 58ae5d9..7c45ea1 100644 --- a/src/build-from-oxlint-config.ts +++ b/src/build-from-oxlint-config.ts @@ -18,6 +18,13 @@ const aliasPluginNames: Record = { jsx_a11y: 'jsx-a11y', }; +// All rules from `eslint-plugin-react-hooks` +// Since oxlint supports these rules under react/*, we need to remap them. +export const reactHookRulesInsideReactScope = [ + 'rules-of-hooks', + 'exhaustive-deps', +]; + const allRulesObjects = Object.values(configByCategory).map( (config) => config.rules ); @@ -139,9 +146,17 @@ const getEsLintRuleName = (rule: string): string | undefined => { const ruleName = match[2]; // map to the right eslint plugin - const esPluginName = + let esPluginName = pluginName in aliasPluginNames ? aliasPluginNames[pluginName] : pluginName; + // special case for eslint-plugin-react-hooks + if ( + esPluginName === 'react' && + reactHookRulesInsideReactScope.includes(ruleName) + ) { + esPluginName = 'react-hooks'; + } + // extra check for eslint const expectedRule = esPluginName === '' ? ruleName : `${esPluginName}/${ruleName}`; diff --git a/src/generated/configs-by-scope.ts b/src/generated/configs-by-scope.ts index 821d288..6d172b0 100644 --- a/src/generated/configs-by-scope.ts +++ b/src/generated/configs-by-scope.ts @@ -52,6 +52,11 @@ const reactConfig = { rules: rules.reactRules, }; +const reactHooksConfig = { + name: 'oxlint/react-hooks', + rules: rules.reactHooksRules, +}; + const reactPerfConfig = { name: 'oxlint/react-perf', rules: rules.reactPerfRules, @@ -83,6 +88,7 @@ const configByScope = { 'flat/node': nodeConfig, 'flat/promise': promiseConfig, 'flat/react': reactConfig, + 'flat/react-hooks': reactHooksConfig, 'flat/react-perf': reactPerfConfig, 'flat/tree-shaking': treeShakingConfig, 'flat/unicorn': unicornConfig, diff --git a/src/generated/rules-by-category.ts b/src/generated/rules-by-category.ts index 58774da..4d1f3b2 100644 --- a/src/generated/rules-by-category.ts +++ b/src/generated/rules-by-category.ts @@ -87,7 +87,7 @@ const nurseryRules = { 'import/no-unused-modules': 'off', 'promise/no-return-in-finally': 'off', 'react/require-render-return': 'off', - 'react/rules-of-hooks': 'off', + 'react-hooks/rules-of-hooks': 'off', 'tree-shaking/no-side-effects-in-initialization': 'off', '@typescript-eslint/consistent-type-imports': 'off', } as const; diff --git a/src/generated/rules-by-scope.ts b/src/generated/rules-by-scope.ts index e8a9579..b9dc977 100644 --- a/src/generated/rules-by-scope.ts +++ b/src/generated/rules-by-scope.ts @@ -356,12 +356,15 @@ const reactRules = { 'react/prefer-es6-class': 'off', 'react/react-in-jsx-scope': 'off', 'react/require-render-return': 'off', - 'react/rules-of-hooks': 'off', 'react/self-closing-comp': 'off', 'react/style-prop-object': 'off', 'react/void-dom-elements-no-children': 'off', } as const; +const reactHooksRules = { + 'react-hooks/rules-of-hooks': 'off', +} as const; + const reactPerfRules = { 'react-perf/jsx-no-jsx-as-prop': 'off', 'react-perf/jsx-no-new-array-as-prop': 'off', @@ -485,6 +488,7 @@ export { nodeRules, promiseRules, reactRules, + reactHooksRules, reactPerfRules, treeShakingRules, unicornRules,