-
-
Notifications
You must be signed in to change notification settings - Fork 31
feat: Add new rule no-meta-replaced-by
#518
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
Changes from 4 commits
8d0b6d3
4d3a319
83f0008
672b690
5141e3a
3f8083e
e95948b
94b6791
43ce974
316269a
cb74f04
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,63 @@ | ||
# Disallow rules to use a `meta.replacedBy` property (`eslint-plugin/no-meta-replaced-by`) | ||
|
||
💼 This rule is enabled in the ✅ `recommended` [config](https://github.com/eslint-community/eslint-plugin-eslint-plugin#presets). | ||
|
||
<!-- end auto-generated rule header --> | ||
|
||
ESLint v9.21.0 introduces a new `DeprecatedInfo` type to describe whether a rule is deprecated and how it can be replaced. The legacy format used the meta property `replacedBy` which should be defined inside `deprecated` instead of at the top level. | ||
error-four-o-four marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
||
Examples of a correct usage can be found at [array-bracket-newline](https://github.com/eslint/eslint/blob/4112fd09531092e9651e9981205bcd603dc56acf/lib/rules/array-bracket-newline.js#L18-L38) and [typescript-eslint/no-empty-interface](https://github.com/typescript-eslint/typescript-eslint/blob/af94f163a1d6447a84c5571fff5e38e4c700edb9/packages/eslint-plugin/src/rules/no-empty-interface.ts#L19-L30) | ||
error-four-o-four marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
||
## Rule Details | ||
|
||
This rule disallows the `replacedBy` property in rules' `meta`. | ||
error-four-o-four marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
||
Examples of **incorrect** code for this rule: | ||
|
||
```js | ||
/* eslint eslint-plugin/no-meta-replaced-by: error */ | ||
|
||
module.exports = { | ||
meta: { | ||
deprecated: true, | ||
replacedBy: [], | ||
error-four-o-four marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
}, | ||
create() {}, | ||
}; | ||
``` | ||
|
||
Examples of **correct** code for this rule: | ||
|
||
```js | ||
/* eslint eslint-plugin/no-meta-replaced-by: error */ | ||
|
||
module.exports = { | ||
meta: { | ||
deprecated: { | ||
message: 'The new rule adds more functionality', | ||
replacedBy: [ | ||
{ | ||
rule: { | ||
name: 'the-new-rule', | ||
}, | ||
}, | ||
], | ||
}, | ||
}, | ||
create() {}, | ||
}; | ||
|
||
module.exports = { | ||
meta: {}, | ||
create() {}, | ||
}; | ||
``` | ||
|
||
## When Not To Use It | ||
|
||
If you do not plan to provide rule's documentation in website, you can turn off this rule. | ||
|
||
error-four-o-four marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
## Further Reading | ||
|
||
- [ESLint docs: `DeprecatedInfo`](https://eslint.org/docs/latest/extend/rule-deprecation#-deprecatedinfo-type) | ||
- [RFC introducing `DeprecatedInfo` type](https://github.com/eslint/rfcs/blob/main/designs/2023-rule-options-defaults/README.md) | ||
error-four-o-four marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,63 @@ | ||
/** | ||
* @fileoverview Disallows the usage of `meta.replacedBy` property | ||
*/ | ||
|
||
'use strict'; | ||
|
||
const utils = require('../utils'); | ||
|
||
// ------------------------------------------------------------------------------ | ||
// Rule Definition | ||
// ------------------------------------------------------------------------------ | ||
|
||
/** @type {import('eslint').Rule.RuleModule} */ | ||
module.exports = { | ||
meta: { | ||
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. We could also add an autofixer to this rule to move 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. Yes, thought so too. But I'd have to get down to the nitty-gritty of eslint and ASTs. The autofixer should be added in a follow-up. 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. Sounds good. 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. Another fix in a follow-up might be to remove 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'm also thinking about the neccessity and possibilities so that the autofixer is able to add the property If this is not the desired behaviour of the autofixer it would simply convert the items of the old If it is the desired behaviour one would have to determine and compare the prefix of the plugin. This could be achieved by either providing an option in 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 think we can leave it. Doesn't hurt to continue to support the older rule properties. 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. Regarding the autofixer, it would be nice if the autofixer can work without any configuration. But if there's an option that can be provided to the rule to improve the autofixer further, I'm open to that as an added bonus. |
||
type: 'problem', | ||
docs: { | ||
description: 'disallow rules to use a `meta.replacedBy` property', | ||
error-four-o-four marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
category: 'Rules', | ||
recommended: true, | ||
error-four-o-four marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
url: 'https://github.com/eslint-community/eslint-plugin-eslint-plugin/tree/HEAD/docs/rules/no-meta-replaced-by.md', | ||
}, | ||
schema: [], | ||
messages: { | ||
useNewFormat: | ||
'Use `meta.deprecated.replacedBy` instead of `meta.replacedBy`', | ||
}, | ||
}, | ||
create(context) { | ||
const sourceCode = utils.getSourceCode(context); | ||
const ruleInfo = utils.getRuleInfo(sourceCode); | ||
|
||
if (!ruleInfo) { | ||
return {}; | ||
} | ||
|
||
return { | ||
Program() { | ||
const metaNode = ruleInfo.meta; | ||
|
||
if (!metaNode) { | ||
return; | ||
} | ||
|
||
const replacedByNode = utils | ||
.evaluateObjectProperties(metaNode, sourceCode.scopeManager) | ||
.find( | ||
(p) => | ||
p.type === 'Property' && utils.getKeyName(p) === 'replacedBy', | ||
); | ||
|
||
if (!replacedByNode) { | ||
return; | ||
} | ||
|
||
context.report({ | ||
node: replacedByNode, | ||
messageId: 'useNewFormat', | ||
}); | ||
}, | ||
}; | ||
}, | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,132 @@ | ||
/** | ||
* @fileoverview require rules to implement a `meta.type` property | ||
* @author 唯然<[email protected]> | ||
*/ | ||
|
||
'use strict'; | ||
|
||
// ------------------------------------------------------------------------------ | ||
// Requirements | ||
// ------------------------------------------------------------------------------ | ||
|
||
const rule = require('../../../lib/rules/no-meta-replaced-by'); | ||
const RuleTester = require('../eslint-rule-tester').RuleTester; | ||
|
||
// ------------------------------------------------------------------------------ | ||
// Tests | ||
// ------------------------------------------------------------------------------ | ||
|
||
const valid = [ | ||
error-four-o-four marked this conversation as resolved.
Show resolved
Hide resolved
|
||
'module.exports = {};', | ||
` | ||
module.exports = { | ||
create(context) {}, | ||
}; | ||
`, | ||
` | ||
module.exports = { | ||
meta: {}, | ||
create(context) {}, | ||
}; | ||
`, | ||
{ | ||
code: ` | ||
module.exports = { | ||
meta: { | ||
deprecated: { | ||
replacedBy: [ | ||
{ | ||
rule: { | ||
name: 'foo', | ||
}, | ||
}, | ||
], | ||
}, | ||
}, | ||
create(context) {}, | ||
}; | ||
`, | ||
errors: 0, | ||
}, | ||
]; | ||
|
||
const invalid = [ | ||
{ | ||
code: ` | ||
module.exports = { | ||
meta: { | ||
replacedBy: [], | ||
}, | ||
create(context) {}, | ||
}; | ||
`, | ||
errors: [ | ||
{ | ||
messageId: 'useNewFormat', | ||
line: 4, | ||
endLine: 4, | ||
}, | ||
], | ||
}, | ||
{ | ||
code: ` | ||
const meta = { | ||
replacedBy: null, | ||
}; | ||
|
||
module.exports = { | ||
meta, | ||
create(context) {}, | ||
}; | ||
`, | ||
errors: [ | ||
{ | ||
messageId: 'useNewFormat', | ||
line: 3, | ||
endLine: 3, | ||
}, | ||
], | ||
}, | ||
{ | ||
code: ` | ||
const spread = { | ||
replacedBy: null, | ||
}; | ||
|
||
module.exports = { | ||
meta: { | ||
...spread, | ||
}, | ||
create(context) {}, | ||
}; | ||
`, | ||
errors: [{ messageId: 'useNewFormat' }], | ||
}, | ||
]; | ||
|
||
const testToESM = (test) => { | ||
if (typeof test === 'string') { | ||
return test.replace('module.exports =', 'export default'); | ||
} | ||
|
||
const code = test.code.replace('module.exports =', 'export default'); | ||
|
||
return { | ||
...test, | ||
code, | ||
}; | ||
}; | ||
|
||
new RuleTester({ | ||
languageOptions: { sourceType: 'commonjs' }, | ||
}).run('no-meta-replaced-by', rule, { | ||
valid, | ||
invalid, | ||
}); | ||
|
||
new RuleTester({ | ||
languageOptions: { sourceType: 'module' }, | ||
}).run('no-meta-replaced-by', rule, { | ||
valid: valid.map(testToESM), | ||
invalid: invalid.map(testToESM), | ||
}); |
Uh oh!
There was an error while loading. Please reload this page.
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.
Regarding when not to use it, I don't think we necessarily need to include this section. In fact, anyone can use this rule regardless of ESLint version. As far as I know, ESLint itself does not validate nor use the
meta.deprecated
property. So anybody can use the object format formeta.deprecated
.Note that if someone is using TypeScript to define their rules, they would want to make sure they have up-to-date rule types (which I believe come from @types/eslint or typescript-eslint). And rule authors would want to make sure they are using up-to-date versions of any third-party tooling like eslint-doc-generator that use this property.
OR, you could include the section and just state the context I've mentioned here.