eslint / rewrite

Monorepo for the new version of ESLint
Apache License 2.0
169 stars 11 forks source link

Bug: `@eslint/compat` `fixupPluginRules()` does not work with rule options #127

Closed ehmicky closed 1 month ago

ehmicky commented 1 month ago

Which packages are affected?

Environment

Node version: 23.1.0 npm version: 10.9.0 ESLint version: 9.13.0 Operating System: Ubuntu 24.04

What did you do?

// eslint.config.js
import {fixupPluginRules} from '@eslint/compat';
import fp from 'eslint-plugin-fp';

export default [
  {
    plugins: {
      fp: fixupPluginRules(fp),
    },
    rules: {
      'fp/no-mutating-methods': ['error', {allowedObjects: ['state']}]
    },
  },
];
// index.js
const state = [];
state.push(3);
$ eslint index.js

Oops! Something went wrong! :(

ESLint: 9.13.0

Error: Key "rules": Key "fp/no-mutating-methods":
    Value [{"allowedObjects":["state"]}] should NOT have more than 0 items.

    at RuleValidator.validate (/home/ether/Desktop/repos/eslint-rewrite-bug/node_modules/eslint/lib/config/rule-validator.js:170:27)
    at new Config (/home/ether/Desktop/repos/eslint-rewrite-bug/node_modules/eslint/lib/config/config.js:243:27)
    at [finalizeConfig] (/home/ether/Desktop/repos/eslint-rewrite-bug/node_modules/eslint/lib/config/flat-config-array.js:216:16)
    at FlatConfigArray.getConfigWithStatus (/home/ether/Desktop/repos/eslint-rewrite-bug/node_modules/@eslint/config-array/dist/cjs/index.cjs:1102:55)
    at FlatConfigArray.getConfig (/home/ether/Desktop/repos/eslint-rewrite-bug/node_modules/@eslint/config-array/dist/cjs/index.cjs:1120:15)
    at /home/ether/Desktop/repos/eslint-rewrite-bug/node_modules/eslint/lib/eslint/eslint.js:764:40
    at async Promise.all (index 0)
    at async ESLint.lintFiles (/home/ether/Desktop/repos/eslint-rewrite-bug/node_modules/eslint/lib/eslint/eslint.js:759:25)
    at async Object.execute (/home/ether/Desktop/repos/eslint-rewrite-bug/node_modules/eslint/lib/cli.js:498:23)
    at async main (/home/ether/Desktop/repos/eslint-rewrite-bug/node_modules/eslint/bin/eslint.js:158:22)

What did you expect to happen?

The fixupPluginRules() from @eslint/compat should fix the fp/no-mutating-methods rule.

What actually happened?

It crashed it instead.

Link to Minimal Reproducible Example

https://github.com/ehmicky/eslint-rewrite-bug

Participation

Additional comments

eslint-plugin-fp is unmaintained, so I cannot expect an upgrade to ESLint 9, and need to rely on @eslint/compat instead.

However, their rule seems to be correct: https://github.com/jfmengels/eslint-plugin-fp/blob/205861e874a4db8caedfbe2cc8af599d8d45b475/rules/no-mutating-methods.js#L70-L80. The above configuration worked fine with ESLint 8.

Please note that:

mdjermanovic commented 1 month ago

The problem is that fp/no-mutating-methods specifies schema as a top-level property instead of a property of the meta object:

https://github.com/jfmengels/eslint-plugin-fp/blob/205861e874a4db8caedfbe2cc8af599d8d45b475/rules/no-mutating-methods.js#L82-L92

Top-level schema property in object-style rules was never officially supported, I believe, but was working in ESLint v8 by chance because it was picked up by code that was supposed to handle schema property of function-style rules.

I think we could update @eslint/compat to copy top-level schema into meta.schema. @nzakas what do you think?

nzakas commented 1 month ago

I think we could update @eslint/compat to copy top-level schema into meta.schema. @nzakas what do you think?

That seems like the easiest solution to me. :+1:

mdjermanovic commented 1 month ago

I can work on this.

ehmicky commented 1 month ago

Thanks @mdjermanovic!

I was having the same problem with eslint-plugin-filenames, and it turns out it also incorrectly sets module.exports.schema.

I am not familiar with old plugins, but eslint-plugin-filenames seems quite different:

nzakas commented 1 month ago

@ehmicky yes, that was the original format of rules that we deprecated and removed in v9.