feat: add deprecation warnings for legacy API in RuleTester

[snitin315]

Prerequisites checklist

• I have read the contributing guidelines.

What is the purpose of this pull request? (put an "X" next to an item)

[ ] Documentation update
[ ] Bug fix (template)
[ ] New rule (template)
[ ] Changes an existing rule (template)
[ ] Add autofix to a rule
[ ] Add a CLI option
[x] Add something to the core
[ ] Other, please explain:

What changes did you make? (Give an overview)

Resolve #15761
Refers #14709

Prepares for upcoming breaking changes in ESLint v9 to disallow function-style rules and rules with options that are missing a schema.

Adds the deprecation warnings mentioned in the RFC (eslint/rfcs#85).

These deprecation warnings will help alert plugin authors that they need to update their rules. They are only visible when running rule tests and not to end-users who just use the rules.

Is there anything you'd like reviewers to focus on?

This lint rule can assist with the conversion: https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/blob/main/docs/rules/require-meta-schema.md");

I'm not sure if we should add the above line in the deprecation message. Two links in the log can be overwhelming. Maybe we can mention this somewhere in our docs here - https://eslint.org/docs/latest/developer-guide/working-with-rules

[netlify]

✅ Deploy Preview for docs-eslint canceled.

Name	Link
🔨 Latest commit	9bdb8fa
🔍 Latest deploy log	https://app.netlify.com/sites/docs-eslint/deploys/62db6dbc1666a9000833ee90

[mdjermanovic]

I checked all tests that are using function-style fixture rules, and it seems that only this one is specifically testing how RuleTester handles function-style rules, so I think we should convert all other fixture rules to object-style now. That will avoid logging deprecation warnings when we run npm test, and also ensure that RuleTester functionalities are tested with object-style rules, which seems more relevant than testing with legacy function-style rules.

Those fixure rules are:

• no-eval
• no-test-settings
• no-test-global
• no-test-filename
• no-invalid-args (we can also add meta.shema to this one).
• no-invalid-schema
• no-schema-violation
• modify-ast
• modify-ast-at-first
• modify-ast-at-last

[snitin315]

@mdjermanovic Thank you for investigating this, I'll update the PR with your suggestions.

[mdjermanovic]

only this one is specifically testing how RuleTester handles function-style rules

As for this one, I think we can just remove that test case + two test cases right before it; in total, we can now remove these 3 test cases.

These 3 test cases were added to check RuleTester-only errors we introduced in v7.6.0 (ecb2b73) as a notice about future Linter errors that were planned and eventually implemented in v8.0.0 (4a7aab7). Since these are now Linter errors, there's no need to test them in RuleTester.

[mdjermanovic]

ESLint currently allows schema as a top-level property in object-style rules, and it's treated the same as meta.schema.

// rule
module.exports = {

    // this will be treated as the options schema
    schema: {
        /* ... */
    },

    create(context) {
        /* ... */
    }
};

This is most likely an oversight. getRuleOptionsSchema gets schema from the given rule by rule.schema || rule.meta && rule.meta.schema. I believe rule.schema was only intended to get schema from function-style rules, but it works for object-style rules as well.

This PR does not use getRuleOptionsSchema (it gets the schema directly from rule.meta.schema) so it will log the warning as if the schema is not defined. I think this behavior is correct and that we want to drop support for top-level schema properties in favor of meta.schema, but this wasn't mentioned in the RFC. @eslint/eslint-tsc thoughts?

[nzakas]

I think this behavior is correct and that we want to drop support for top-level schema properties in favor of meta.schema, but this wasn't mentioned in the RFC.

I agree. 👍

[bmish]

Good catch, I'd very much like to drop support for top-level schema in this coming major release given that it wasn't intended nor documented for object-style rules.

[mdjermanovic]

LGTM, thanks!

[nzakas]

Should we also duplicate these changes in FlatRuleTester?

[bmish]

@nzakas good point, it seems like we have to duplicate these changes/tests in FlatRuleTester, otherwise we'll have divergence and these changes will be lost.

[mdjermanovic]

Should we also duplicate these changes in FlatRuleTester?

I think it isn't necessary since we are planning eslint/rfcs#85 for ESLint v9, and FlatRuleTester will not replace RuleTester before ESLint v9. When we implement eslint/rfcs#85 in linter/config, these deprecation warnings will become errors thrown by linter/config.

[bmish]

Thanks for your work on this!

[nzakas]

Fair enough.