linter: how to solve the problem for rules, which have same name but are inside different plugins

[Sysix]

Follow up from #7242
We did make a great step in the right direction, but there are still problems.
I want to summarize it for you and discuss how to solve it forever 🤞.

I do not provide a solution. This is a discussion to discuss each problem
and in a perfect world, create issues/prs for it.

There is probably more stuff that I did not cover. Feel free to extend the problems.

To make it a bit more simple, here is a legend for the different categories:

• 🟥 Problems with oxlint
• 🟦 Problems with oxlint documentation
• 🟫 Problems with eslint-plugin-oxlint
• 🔲 Problems with @oxlint/migrate

Preword

We have 4 different rule-cases, some are split into separate files, and some are remapped by oxlint:

• Plugin B is the same rule as Plugin A
  • Typescript rules that are copied from eslint but support ts syntax
• Plugin B extends the rule from Plugin A
  • Typescript rules which has more options than the eslint version + ts syntax
• Plugin B changes the behavior of the rule of Plugin A
  • no-nested-ternary (unicorn allows one deep level, eslint does not)
• Plugin B copied the rule from Plugin A
  • vitest / jest

Each combination will create its own kind of problem. Some we have (almost) completed.

🟦 JS Parsing for keeping all rules up to date

The smallest one first:
We need to keep track of all the special rules:
https://github.com/oxc-project/oxc/blob/main/tasks/lint_rules/src/oxlint-rules.mjs#L147-L218

cargo -p oxlint --rules will currently not work, see next part

🟥🟦🟫🔲 cargo -p oxc_linter --rules

This command or oxlint .... can't be used because the remapped rules are not displayed:

oxc/crates/oxc_linter/src/utils/mod.rs

Lines 21 to 100 in 85fbe8c

	/// List of Jest rules that have Vitest equivalents.
	const VITEST_COMPATIBLE_JEST_RULES: phf::Set<&'static str> = phf::phf_set! {
	"consistent-test-it",
	"expect-expect",
	"max-expects",
	"max-nested-describe",
	"no-alias-methods",
	"no-commented-out-tests",
	"no-conditional-expect",
	"no-conditional-in-test",
	"no-disabled-tests",
	"no-duplicate-hooks",
	"no-focused-tests",
	"no-hooks",
	"no-identical-title",
	"no-interpolation-in-snapshots",
	"no-restricted-jest-methods",
	"no-restricted-matchers",
	"no-standalone-expect",
	"no-test-prefixes",
	"no-test-return-statement",
	"prefer-comparison-matcher",
	"prefer-each",
	"prefer-equality-matcher",
	"prefer-expect-resolves",
	"prefer-hooks-in-order",
	"prefer-hooks-on-top",
	"prefer-lowercase-title",
	"prefer-mock-promise-shorthand",
	"prefer-strict-equal",
	"prefer-to-have-length",
	"prefer-todo",
	"require-to-throw-message",
	"require-top-level-describe",
	"valid-describe-callback",
	"valid-expect",
	};
	// List of Eslint rules that have Typescript equivalents.
	const TYPESCRIPT_COMPATIBLE_ESLINT_RULES: phf::Set<&'static str> = phf::phf_set! {
	"class-methods-use-this",
	"default-param-last",
	"init-declarations",
	"max-params",
	"no-array-constructor",
	"no-dupe-class-members",
	"no-empty-function",
	"no-invalid-this",
	"no-loop-func",
	"no-loss-of-precision",
	"no-magic-numbers",
	"no-redeclare",
	"no-restricted-imports",
	"no-shadow",
	"no-unused-expressions",
	"no-unused-vars",
	"no-use-before-define",
	"no-useless-constructor",
	// these rules are equivalents, but not supported
	// "block-spacing",
	// "brace-style",
	// "comma-dangle",
	// "comma-spacing",
	// "func-call-spacing",
	// "indent",
	// "key-spacing",
	// "keyword-spacing",
	// "lines-around-comment",
	// "lines-between-class-members",
	// "no-extra-parens",
	// "no-extra-semi",
	// "object-curly-spacing",
	// "padding-line-between-statements",
	// "quotes",
	// "semi",
	// "space-before-blocks",
	// "space-before-function-paren",
	// "space-infix-ops",
	};

🟥🟦

As a user I expect to see my lovely vitest rules, which are copies of jest, too.
I need to know that oxlint is remapping some rules and keep track of them.

Oxlint will not count them as separate rules. More details in the next block (integration tests)

🟫🔲

Not only users, but programs needs to keep track of the rules too.
Both plugins have the same mapping:
https://github.com/oxc-project/eslint-plugin-oxlint/blob/78910e2ce4fbdd7592c92e8c3e0791ab1ca09e3f/src/constants.ts#L18-L83
https://github.com/oxc-project/oxlint-migrate/blob/e7992ae2dcebe0218abd562feb0e70e834355b4b/src/constants.ts#L18-L41

This is needed to generate a JS Array / Object:
https://github.com/oxc-project/eslint-plugin-oxlint/blob/78910e2ce4fbdd7592c92e8c3e0791ab1ca09e3f/scripts/traverse-rules.ts#L57-L91
https://github.com/oxc-project/oxlint-migrate/blob/e7992ae2dcebe0218abd562feb0e70e834355b4b/scripts/traverse-rules.ts#L56-L98

For eslint-plugin-oxlint we need to work around with the integration tests here:
https://github.com/oxc-project/eslint-plugin-oxlint/blob/78910e2ce4fbdd7592c92e8c3e0791ab1ca09e3f/src/build-from-oxlint-config.spec.ts#L149-L183

🟥 Diagnostic Messages

Because we are remapping the rules, the diagnosticmessages are displaying with the "main" plugin name.
We can not be sure what plugin should be displayed. See more info here: #8223

🟥 (v1.1) Reporting invalid rules

When we are checking rules if they are invalid, we need to keep in mind that the remapped rules exists.
#4146

🟥🟫🔲 Disabling / Enabling rules

When disabling @typescript/xy should be eslint/xy be disabled too?
When disabling vitest/xy should be vitest/xy be disabled too?

🟥

Problem with eslint and unicorn rule: #9402

Is there a difference between: {rules: { "@typescript/xy": "error", "eslint/xy": "off"}} and
{rules: { "eslint/xy": "off", "@typescript/xy": "error"}} ?

Is there a difference between: {rules: { "@typescript/xy": "off", "eslint/xy": "error"}} and
{rules: { "eslint/xy": "off", "@typescript/xy": "error"}} ?

How do we handle{rules: { "jest/xy": "error", "vitest/xy": "off"}}?

🟫

The question above did keep me from implementing this Issue:
oxc-project/eslint-plugin-oxlint#228

🔲

The migration tool wants to keep the config file as small as possible.
There is an implementation only for ts/eslint rules only at the moment. See oxc-project/oxlint-migrate#7
I can include it for jest/vitest too, but that will confuse people who do not know they are the same.

🟥🟫🔲 Overrides Rules

see "disabling / enabling" rules for the base problems.
This will also include for overrides in .oxlintc..json config

🟥🟫🔲 Extending Config

see "disabling / enabling" rules for the base problems.
This will also include for extends in .oxlintc..json config, which is currently in work: #9217

🟥🟫🔲 Nested Configs

see "disabling / enabling" rules for the base problems.
This will also include for nested configurations, which are currently in under a feature flag: #9152

EDIT v1.1

[Sysix]

Lets try to tackle it down in smaller pieces @camc314 @camchenry @shulaoda
First the biggest problem, I think the other problems will be solved when we know how to handle this one:

🟥🟫🔲 Disabling / Enabling rules

When disabling @typescript/xy should be eslint/xy be disabled too?
When disabling vitest/xy should be vitest/xy be disabled too?
🟥
Problem with eslint and unicorn rule: #9402
Is there a difference between: {rules: { "@typescript/xy": "error", "eslint/xy": "off"}} and
{rules: { "eslint/xy": "off", "@typescript/xy": "error"}} ?
Is there a difference between: {rules: { "@typescript/xy": "off", "eslint/xy": "error"}} and
{rules: { "eslint/xy": "off", "@typescript/xy": "error"}} ?
How do we handle{rules: { "jest/xy": "error", "vitest/xy": "off"}}?

My preffered way:

• Disable eslint rules too when typescript rule is disabled
• typescript rule should override eslint rule with config
• Not disable jest rules when disabling vitest rule
• jest and vitest rule configuration should be separated
• other plugins have the files separated already

Problems on the way:

• how to we split jest and vitest
  • it is the only way (for me) to have separate configuration in .oxlintrc.json
• Overriding typescript rule config with eslint rule config (in .oxlintrc.json) could lead to some missconfiguration
• maybe some more?

[shulaoda]

I believe that for two identical rules coming from different plugin names, we should prioritize using the deny/warn configuration. For two different rules from different plugin names, we should enable configurations for each of them separately, again prioritizing deny/warn. This is also what we are currently doing.

My question is: Why did you decide to split Jest and Vitest? Most of their rules are the same, and I think it’s unlikely that someone would use both Vitest and Jest in the same project. Could you provide an example of a use case based on the scenarios I described above?

[Sysix]

My question is: Why did you decide to split Jest and Vitest? Most of their rules are the same, and I think it’s unlikely that someone would use both Vitest and Jest in the same project.

This is mostly personal reason. Here some "bullet" points

Projects are using both until migration is done

Working on a company project where we migrated from jest to vitest.
Glad we had not many tests but it took us some weeks to finish.

We had no eslint-plugin-jest enabled but used eslint-plugin-vitest and fixed the errors on the way.
I expect that some other will change their lint setup too when migrating.

Because of 🟥 Diagnostic Messages

Using vitest it is wierd to have still eslint-plugin-jest in the diagnostic messages.
See #8223

Confusion / Optimization problems for @oxlint/migrate

@oxlint/migrate is trying to optimize the generated .oxlintrc.json file to have small rules.
Having jest/xy and vitest/xy enabled can be optimized away. Currently this is how it works with typescript and eslint.

Step 1: Rename the rule
We only want to have one rule name because of step 2:
https://github.com/oxc-project/oxlint-migrate/blob/39811fb3fd1b5797cccddca360f7620732ca58a8/src/plugins_rules.ts#L184-L212

Step 2: Check for duplicate rules from overrides
https://github.com/oxc-project/oxlint-migrate/blob/39811fb3fd1b5797cccddca360f7620732ca58a8/src/plugins_rules.ts#L161-L182

Doing the same for vitest to jest feels wrong. The user has loaded their eslint.config.* the vitest rules and we are outputting jest rules.
I can accept this wierd behavior for typescript rules
because oxlint supports typescript out of the box (and enables the rules via eslint + default category)

[Sysix]

Looks like I am the only one who is crying about it --> Closing