Skip to content

Commit

Permalink
fix(tasks/lint_rules): Refactor syncing typescript-eslint and eslint …
Browse files Browse the repository at this point in the history
…status (#4654)

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.
  • Loading branch information
leaysgur authored Aug 5, 2024
1 parent 06aec77 commit 1763597
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 18 deletions.
4 changes: 0 additions & 4 deletions tasks/lint_rules/src/eslint-rules.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
3 changes: 3 additions & 0 deletions tasks/lint_rules/src/main.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion tasks/lint_rules/src/markdown-renderer.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -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(", ")}.
`;

/**
Expand Down
38 changes: 25 additions & 13 deletions tasks/lint_rules/src/oxlint-rules.cjs
Original file line number Diff line number Diff line change
@@ -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(
Expand Down Expand Up @@ -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);
}
}
Expand All @@ -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
]);

/**
Expand Down Expand Up @@ -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;
}
};

0 comments on commit 1763597

Please sign in to comment.