Skip to content
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

Add eslint-plugin-eslint-plugin and add URLs to all rules #88

Closed
wants to merge 7 commits into from

Conversation

ddzz
Copy link

@ddzz ddzz commented Sep 25, 2021

This PR adds eslint-plugin-eslint-plugin and its recommended rules to this plugin so that ESLint catches common issues found in ESLint plugins.

@ddzz ddzz changed the title Add URLs to all rules Add eslint-plugin-eslint-plugin and add URLs to all rules Sep 26, 2021
.eslintrc.json Outdated
],
"rules": {
"eslint-plugin/prefer-message-ids": "off",
"eslint-plugin/require-meta-docs-url": "error"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You'll want to add the rule format so that the correct rule URLs can be enforced and autofixed:

Suggested change
"eslint-plugin/require-meta-docs-url": "error"
"eslint-plugin/require-meta-docs-url": ["error", {
"pattern": "https://github.com/cypress-io/eslint-plugin-cypress/blob/master/docs/rules/{{name}}.md"
}],

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some rules don't use documentation from GitHub, so this pattern can't be used.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. If it's just one rule, then it would probably be good to still use the pattern globally, but just disable the rule for that specific edge case:

url: 'non-github-url' // eslint-disable-line eslint-plugin/require-meta-docs-url

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1/3rd of existing rules don't use GitHub docs. eslint-disables will just clutter up the rules.

.eslintrc.json Outdated Show resolved Hide resolved
@ddzz ddzz force-pushed the add-urls-to-rules branch from ec94286 to 2adbda4 Compare September 26, 2021 18:56
.eslintrc.json Outdated Show resolved Hide resolved
.eslintrc.json Outdated Show resolved Hide resolved
.eslintrc.json Outdated
],
"rules": {
"eslint-plugin/prefer-message-ids": "off",
"eslint-plugin/require-meta-type": "off",
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding a type to a rule is a trivial one-line change so would be good to add types instead of disabling this.

https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/blob/master/docs/rules/require-meta-type.md

@@ -29,6 +29,7 @@
"@cypress/eslint-plugin-json": "3.2.1",
"condition-circle": "2.0.2",
"eslint": "^5.7.0",
"eslint-plugin-eslint-plugin": "^3.6.1",
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"eslint-plugin-eslint-plugin": "^3.6.1",
"eslint-plugin-eslint-plugin": "^4.0.1",

Can you bump to v4 which was just released today?

https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/releases/tag/v4.0.0

@DamienCassou
Copy link

I've forked the project and welcome PRs: #100.

@ddzz ddzz closed this May 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants