-
-
Notifications
You must be signed in to change notification settings - Fork 86
feat: report malformed rule deprecation metadata #147
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,116 @@ | ||
| - Repo: eslint/eslint | ||
| - Start Date: 2026-05-03 | ||
| - RFC PR: https://github.com/eslint/rfcs/pull/147 | ||
| - Authors: xbinaryx | ||
|
|
||
| # Report Malformed Rule Deprecation Metadata | ||
|
|
||
| ## Summary | ||
|
|
||
| `RuleTester` should validate the shape of `meta.deprecated` on the rule under test and throw an assertion error when the value does not match the documented `boolean | DeprecatedInfo` contract. | ||
|
|
||
| ## Motivation | ||
|
|
||
| Rule deprecation metadata is used by ESLint to populate `usedDeprecatedRules` and by integrations to explain replacement rules. Today, malformed `meta.deprecated` values can pass through rule tests unnoticed, which allows invalid metadata to reach users and downstream tools. `RuleTester` already catches common rule authoring mistakes such as invalid option schemas, missing `meta.fixable`, and missing `meta.hasSuggestions`; deprecation metadata should receive the same early feedback. | ||
|
|
||
| ## Detailed Design | ||
|
|
||
| When `RuleTester#run()` is called, `RuleTester` will validate `rule.meta.deprecated` at the very beginning of the execution, before any valid or invalid test cases are run. This will be implemented as part of the existing `assertRule()` validation function, which runs early in the process. If `meta.deprecated` is omitted or set to `undefined`, no validation error is reported. | ||
|
|
||
| The accepted values are: | ||
|
xbinaryx marked this conversation as resolved.
Outdated
|
||
|
|
||
| ```js | ||
| meta: { | ||
| deprecated: true | ||
| } | ||
|
|
||
| meta: { | ||
| deprecated: false | ||
| } | ||
|
|
||
| meta: { | ||
| deprecated: { | ||
| message: "Use eslint-plugin-example/example instead.", | ||
| url: "https://example.com/deprecations/example", | ||
| deprecatedSince: "9.0.0", | ||
| availableUntil: "10.0.0", | ||
| replacedBy: [ | ||
| { | ||
| message: "Replacement rule.", | ||
| url: "https://example.com/replacements/example", | ||
| plugin: { | ||
| name: "eslint-plugin-example", | ||
| url: "https://example.com/plugin" | ||
| }, | ||
| rule: { | ||
| name: "example", | ||
| url: "https://example.com/rules/example" | ||
| } | ||
| } | ||
| ] | ||
| } | ||
| } | ||
| ``` | ||
|
|
||
| The object form must match `DeprecatedInfo`: | ||
|
|
||
| - `message`, `url`, and `deprecatedSince` must be strings when present. | ||
| - `availableUntil` must be a string or `null` when present. | ||
|
Comment on lines
+57
to
+58
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For semver-format strings, perhaps we could also check that they don't start with "v"? It should also be possible to check if the string is valid semver, but I'm not sure if we would want to add a dependency for this purpose. |
||
| - `replacedBy` must be an array when present. | ||
| - No other top-level `DeprecatedInfo` properties are allowed. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there any reason to disallow unknown properties when ESLint does not use them.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But TypeScript also allows overwriting the types of external packages which can be used to extend the type, which would then not be possible for the rule tester.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because this section is meant to be used by ESLint itself, I'm fine with disallowing unknown keys. There are plenty of other places inside of |
||
|
|
||
| Each `replacedBy` entry must be a `ReplacedByInfo` object: | ||
|
|
||
| - `message` and `url` must be strings when present. | ||
| - `plugin` and `rule` must be `ExternalSpecifier` objects when present. | ||
| - No other `ReplacedByInfo` properties are allowed. | ||
|
|
||
| Each `ExternalSpecifier` object may contain: | ||
|
|
||
| - `name`, which must be a string when present. | ||
| - `url`, which must be a string when present. | ||
| - No other `ExternalSpecifier` properties are allowed. | ||
|
|
||
| Malformed values cause `RuleTester` to throw before any valid or invalid test case is executed. Error messages should include the invalid metadata path, such as: | ||
|
|
||
| ```text | ||
| Rule's `meta.deprecated` must be a boolean or object. | ||
| Rule's `meta.deprecated.replacedBy` must be an array. | ||
| Rule's `meta.deprecated.replacedBy[0].plugin.name` must be a string. | ||
| ``` | ||
|
|
||
|
xbinaryx marked this conversation as resolved.
|
||
| ## Documentation | ||
|
|
||
| No updates to the existing documentation are required, as the [Rule Deprecation](https://eslint.org/docs/latest/extend/rule-deprecation) page already defines the accepted runtime shape for rule deprecation metadata. However, because this is a breaking change for rule tests, it will be noted in the migration guide for the next major release. | ||
|
|
||
| ## Drawbacks | ||
|
|
||
| This is a breaking change for rule packages whose tests currently pass with malformed deprecation metadata. Some projects may also be storing custom, nonstandard fields under `meta.deprecated`; those projects will need to remove those fields or move them elsewhere. | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Worth noting that we've decided breaking changes to
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't recall us deciding that, can you provide a link (to a TSC meeting where we discussed this?). As far as I'm aware, all breaking changes to |
||
|
|
||
| ## Backwards Compatibility Analysis | ||
|
|
||
| Existing rules with valid deprecation metadata are unaffected. Rules with malformed metadata will start failing in `RuleTester`. This disruption is intentional so rule authors discover metadata problems before publishing. | ||
|
|
||
| ## Alternatives | ||
|
|
||
| One alternative is to validate only the top-level `meta.deprecated` type and ignore nested fields. That would catch the most severe malformed values, but it would still allow invalid replacement metadata to reach `usedDeprecatedRules`. | ||
|
|
||
| Another alternative is to rely on TypeScript definitions. That helps TypeScript rule authors, but many ESLint rules are written in JavaScript and can still publish malformed metadata. | ||
|
|
||
| ## Open Questions | ||
|
|
||
| None at this time. | ||
|
|
||
| ## Help Needed | ||
|
|
||
| No additional help is needed for the initial implementation. | ||
|
|
||
| ## Frequently Asked Questions | ||
|
|
||
| ### Does this affect end users running ESLint? | ||
|
|
||
| No. This proposal changes `RuleTester`, so it affects rule authors and maintainers running rule tests. | ||
|
|
||
| ## Related Discussions | ||
|
|
||
| eslint/eslint#20603 | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be more specific.