From 17635973b6c73407143cdeec5fc2ba39dfcfff11 Mon Sep 17 00:00:00 2001 From: Yuji Sugiura <6259812+leaysgur@users.noreply.github.com> Date: Mon, 5 Aug 2024 14:21:52 +0900 Subject: [PATCH] fix(tasks/lint_rules): Refactor syncing typescript-eslint and eslint status (#4654) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #4085 This issue seemed to have been addressed in #3779 , but partially reverted in #3813 ? 🤔 Since I wasn't aware of these changes, I've just checked the current implementation through the review requests in #4611 and refactored as the original author. --- tasks/lint_rules/src/eslint-rules.cjs | 4 --- tasks/lint_rules/src/main.cjs | 3 ++ tasks/lint_rules/src/markdown-renderer.cjs | 2 +- tasks/lint_rules/src/oxlint-rules.cjs | 38 ++++++++++++++-------- 4 files changed, 29 insertions(+), 18 deletions(-) diff --git a/tasks/lint_rules/src/eslint-rules.cjs b/tasks/lint_rules/src/eslint-rules.cjs index 0ee0be1c51d59..8068285be17c9 100644 --- a/tasks/lint_rules/src/eslint-rules.cjs +++ b/tasks/lint_rules/src/eslint-rules.cjs @@ -263,7 +263,3 @@ exports.loadTargetPluginRules = (linter) => { loadPluginReactPerfRules(linter); loadPluginNextRules(linter); }; - -// some typescript rules are some extension of the basic eslint rules -// we need them later to map them for both -exports.pluginTypeScriptRulesNames = Object.keys(pluginTypeScriptAllRules); \ No newline at end of file diff --git a/tasks/lint_rules/src/main.cjs b/tasks/lint_rules/src/main.cjs index 33c414824208c..0475ae9150c8f 100644 --- a/tasks/lint_rules/src/main.cjs +++ b/tasks/lint_rules/src/main.cjs @@ -8,6 +8,8 @@ const { createRuleEntries, updateNotSupportedStatus, updateImplementedStatus, + overrideTypeScriptPluginStatusWithEslintPluginStatus: + syncTypeScriptPluginStatusWithEslintPluginStatus, } = require("./oxlint-rules.cjs"); const { renderMarkdown } = require("./markdown-renderer.cjs"); const { updateGitHubIssue } = require("./result-reporter.cjs"); @@ -59,6 +61,7 @@ Plugins: ${Array.from(ALL_TARGET_PLUGINS.keys()).join(", ")} const ruleEntries = createRuleEntries(linter.getRules()); await updateImplementedStatus(ruleEntries); updateNotSupportedStatus(ruleEntries); + syncTypeScriptPluginStatusWithEslintPluginStatus(ruleEntries); // // Render list and update if necessary diff --git a/tasks/lint_rules/src/markdown-renderer.cjs b/tasks/lint_rules/src/markdown-renderer.cjs index b070470a5ca75..e29b18318765c 100644 --- a/tasks/lint_rules/src/markdown-renderer.cjs +++ b/tasks/lint_rules/src/markdown-renderer.cjs @@ -9,7 +9,7 @@ const renderIntroduction = ({ npm }) => ` > This comment is maintained by CI. Do not edit this comment directly. > To update comment template, see https://github.com/oxc-project/oxc/tree/main/tasks/lint_rules -This is tracking issue for ${npm.map(n => "`" + n + "`").join(", ")}. +This is tracking issue for ${npm.map((n) => "`" + n + "`").join(", ")}. `; /** diff --git a/tasks/lint_rules/src/oxlint-rules.cjs b/tasks/lint_rules/src/oxlint-rules.cjs index 829b57ab8efa5..046bbb2358a18 100644 --- a/tasks/lint_rules/src/oxlint-rules.cjs +++ b/tasks/lint_rules/src/oxlint-rules.cjs @@ -1,6 +1,5 @@ const { resolve } = require("node:path"); const { readFile } = require("node:fs/promises"); -const { pluginTypeScriptRulesNames } = require("./eslint-rules.cjs"); const readAllImplementedRuleNames = async () => { const rulesFile = await readFile( @@ -35,17 +34,6 @@ const readAllImplementedRuleNames = async () => { // Ignore no reference rules if (prefixedName.startsWith("oxc/")) continue; - // some tyescript rules are extensions of eslint core rules - if (prefixedName.startsWith("eslint/")) { - const ruleName = prefixedName.replace('eslint/', ''); - - // there is an alias, so we add it with this in mind. - if (pluginTypeScriptRulesNames.includes(ruleName)) { - rules.add(`typescript/${ruleName}`); - continue; - } - } - rules.add(prefixedName); } } @@ -57,7 +45,8 @@ const NOT_SUPPORTED_RULE_NAMES = new Set([ "eslint/no-dupe-args", // superseded by strict mode "eslint/no-octal", // superseded by strict mode "eslint/no-with", // superseded by strict mode - "import/no-unresolved" // Will always contain false positives due to module resolution complexity + "eslint/no-new-symbol", // Deprecated as of ESLint v9, but for a while disable manually + "import/no-unresolved", // Will always contain false positives due to module resolution complexity ]); /** @@ -115,3 +104,26 @@ exports.updateNotSupportedStatus = (ruleEntries) => { if (rule) rule.isNotSupported = true; } }; + +/** + * Some typescript-eslint rules are re-implemented version of eslint rules. + * e.g. no-array-constructor, max-params, etc... + * Since oxlint supports these rules under eslint/* and it also supports TS, + * we should override these to make implementation status up-to-date. + * + * @param {RuleEntries} ruleEntries + */ +exports.overrideTypeScriptPluginStatusWithEslintPluginStatus = ( + ruleEntries, +) => { + for (const [name, rule] of ruleEntries) { + if (!name.startsWith("typescript/")) continue; + + // This assumes that if the same name found, it implements the same rule. + const eslintRule = ruleEntries.get(name.replace("typescript/", "eslint/")); + if (!eslintRule) continue; + + rule.isImplemented = eslintRule.isImplemented; + rule.isNotSupported = eslintRule.isNotSupported; + } +};