-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
chore: use eslint-plugin-eslint-plugin flat configs #17204
Changes from all commits
3a30c6e
ee52efc
cc9019a
21a6225
3f8b593
39f5786
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 |
---|---|---|
|
@@ -27,10 +27,12 @@ | |
|
||
const path = require("path"); | ||
const internalPlugin = require("eslint-plugin-internal-rules"); | ||
const eslintPlugin = require("eslint-plugin-eslint-plugin"); | ||
const eslintPluginRulesRecommendedConfig = require("eslint-plugin-eslint-plugin/configs/rules-recommended"); | ||
const eslintPluginTestsRecommendedConfig = require("eslint-plugin-eslint-plugin/configs/tests-recommended"); | ||
const { FlatCompat } = require("@eslint/eslintrc"); | ||
const js = require("./packages/js"); | ||
const globals = require("globals"); | ||
const merge = require("lodash.merge"); | ||
|
||
//----------------------------------------------------------------------------- | ||
// Helpers | ||
|
@@ -99,8 +101,7 @@ module.exports = [ | |
}, | ||
{ | ||
plugins: { | ||
"internal-rules": internalPlugin, | ||
"eslint-plugin": eslintPlugin | ||
"internal-rules": internalPlugin | ||
}, | ||
languageOptions: { | ||
ecmaVersion: "latest" | ||
|
@@ -129,33 +130,31 @@ module.exports = [ | |
{ | ||
files: ["lib/rules/*", "tools/internal-rules/*"], | ||
ignores: ["**/index.js"], | ||
rules: { | ||
...eslintPlugin.configs["rules-recommended"].rules, | ||
"eslint-plugin/no-missing-message-ids": "error", | ||
"eslint-plugin/no-unused-message-ids": "error", | ||
"eslint-plugin/prefer-message-ids": "error", | ||
Comment on lines
-134
to
-136
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. These three rules are now already in the rules recommended config, so I removed them from here. |
||
"eslint-plugin/prefer-placeholders": "error", | ||
"eslint-plugin/prefer-replace-text": "error", | ||
"eslint-plugin/report-message-format": ["error", "[^a-z].*\\.$"], | ||
"eslint-plugin/require-meta-docs-description": ["error", { pattern: "^(Enforce|Require|Disallow) .+[^. ]$" }], | ||
"internal-rules/no-invalid-meta": "error" | ||
} | ||
...merge({}, eslintPluginRulesRecommendedConfig, { | ||
rules: { | ||
"eslint-plugin/prefer-placeholders": "error", | ||
"eslint-plugin/prefer-replace-text": "error", | ||
"eslint-plugin/report-message-format": ["error", "[^a-z].*\\.$"], | ||
"eslint-plugin/require-meta-docs-description": ["error", { pattern: "^(Enforce|Require|Disallow) .+[^. ]$" }], | ||
"internal-rules/no-invalid-meta": "error" | ||
} | ||
}) | ||
}, | ||
{ | ||
files: ["lib/rules/*"], | ||
ignores: ["index.js"], | ||
ignores: ["**/index.js"], | ||
rules: { | ||
"eslint-plugin/require-meta-docs-url": ["error", { pattern: "https://eslint.org/docs/latest/rules/{{name}}" }] | ||
} | ||
}, | ||
{ | ||
files: ["tests/lib/rules/*", "tests/tools/internal-rules/*"], | ||
rules: { | ||
...eslintPlugin.configs["tests-recommended"].rules, | ||
"eslint-plugin/prefer-output-null": "error", | ||
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. This rule is now already in the tests recommended config, so I removed it from here. |
||
"eslint-plugin/test-case-property-ordering": "error", | ||
"eslint-plugin/test-case-shorthand-strings": "error" | ||
} | ||
...merge({}, eslintPluginTestsRecommendedConfig, { | ||
rules: { | ||
"eslint-plugin/test-case-property-ordering": "error", | ||
"eslint-plugin/test-case-shorthand-strings": "error" | ||
} | ||
}) | ||
}, | ||
{ | ||
files: ["tests/**/*.js"], | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -188,7 +188,6 @@ module.exports = { | |
}] | ||
}, | ||
fixable: "code", | ||
// eslint-disable-next-line eslint-plugin/require-meta-has-suggestions -- Does not detect conditional suggestions | ||
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. This false positive has been fixed in the new version of |
||
hasSuggestions: true, | ||
messages: { | ||
assignment: "Assignment (=) can be replaced with operator assignment ({{operator}}).", | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -212,7 +212,6 @@ module.exports = { | |
context.report({ | ||
node, | ||
messageId: "unexpected", | ||
data: { identifier: node.name }, | ||
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. This was caught by the new version of |
||
fix(fixer) { | ||
const linesBetween = sourceCode.getText().slice(lastToken.range[1], nextToken.range[0]).split(astUtils.LINEBREAK_MATCHER); | ||
|
||
|
@@ -231,7 +230,6 @@ module.exports = { | |
context.report({ | ||
node, | ||
messageId: "expected", | ||
data: { identifier: node.name }, | ||
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. This was caught by the new version of eslint-plugin/no-unused-placeholders. |
||
fix(fixer) { | ||
if ((noNextLineToken ? getLastCommentLineOfBlock(nextLineNum) : lastToken.loc.end.line) === nextToken.loc.start.line) { | ||
return fixer.insertTextBefore(nextToken, "\n\n"); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
module.exports = { | ||
rules: { | ||
quotes: ["error", "single"] | ||
} | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
module.exports = [{ | ||
rules: { | ||
quotes: ["error", "single"] | ||
} | ||
}]; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
var a = 'b'; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
var a = 'b'; |
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.
Can you destructure these configs from
require('eslint-plugin-eslint-plugin')
? That will be more future-proof than reaching directly into specific files in the event that the plugin more-strictly defines its Node API.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.
I believe this is the only way to use flat configs from
eslint-plugin-eslint-plugin
at the moment.https://github.com/eslint-community/eslint-plugin-eslint-plugin#eslintconfigjs-requires-eslintv8230
Configs in
require('eslint-plugin-eslint-plugin').configs
are in the eslintrc format.@aladdin-add is that correct?
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.
(not blocking) @aladdin-add shouldn't we export the flat configs too? Reaching into specific files is fragile especially if we were to move things around.
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.
they are exported. The default predefined configs are eslintrc, and to not affect the use of eslintrc, I added a new directory to place the flat configs: https://github.com/eslint-community/eslint-plugin-eslint-plugin/tree/main/configs. Any better suggestions?
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.
I'm going to merge this as it correctly uses the current interface of eslint-plugin-eslint-plugin. We can update the usage if and when the interface changes.
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.
So
require('eslint-plugin-eslint-plugin').configs
will always have to be in the eslintrc format for backwards compatibility, but flat configs have to be accessed by file path e.g.require("eslint-plugin-eslint-plugin/configs/rules-recommended")
? Is there a cleaner, long-term plan or recommendation for supporting both config formats (instead of just having to access the newer one by file path)?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.
I think we could discuss that in #17242.