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

New: Add new rule require-meta-has-suggestions #105

Merged

Conversation

bmish
Copy link
Member

@bmish bmish commented Apr 7, 2021

Suggestable ESLint rules should have a meta.hasSuggestions property to indicate that they provide suggestions. This new rule enforces that the meta.hasSuggestions property is correctly enabled when a rule provides suggestions, and not enabled when a rule does not provide suggestions.

The change to require suggestable rules to have meta.hasSuggestions has been accepted and mentioned in the blog post for the upcoming ESLint 8 breaking changes. So we should adopt this change now to help plugin authors ensure they are compatible with ESLint 8 as soon as possible. The old property meta.docs.suggestion was unused anyway.

https://eslint.org/blog/2021/06/whats-coming-in-eslint-8.0.0#rules-with-suggestions-now-require-the-metahassuggestions-property

We should make this a recommended rule in the next major release.

This is very similar to the existing eslint-plugin/require-meta-fixable rule enforcing the correct presence of the meta.fixable property.

Copy link
Contributor

@aladdin-add aladdin-add left a comment

Choose a reason for hiding this comment

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

just a suggestion, but otherwise lgtm, thanks!

contextIdentifiers = utils.getContextIdentifiers(context, node);
},
CallExpression (node) {
if (
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

Will look into this.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure if it's convenient to use selectors here since we have a variable contextIdentifiers in the condition.

@bmish
Copy link
Member Author

bmish commented Apr 9, 2021

I have moved this PR to draft status pending the outcome of this issue eslint/eslint#14312 to discuss the future of the meta.docs.suggestion property.

@bmish bmish changed the title Add new rule require-meta-docs-suggestion Add new rule require-meta-has-suggestions Jun 12, 2021
Suggestable ESLint rules should have a `meta.hasSuggestions` property to indicate that they provide [suggestions](https://eslint.org/docs/developer-guide/working-with-rules#providing-suggestions). This new rule enforces that the `meta.hasSuggestions` property is correctly enabled when a rule provides suggestions, and not enabled when a rule does not provide suggestions.

The change to require suggestable rules to have `meta.hasSuggestions` has been accepted and mentioned in the blog post for the upcoming ESLint 8 breaking changes. So we should adopt this change now to help plugin authors ensure they are compatible with ESLint 8 as soon as possible. The old property `meta.docs.suggestion` was unused anyway.

https://eslint.org/blog/2021/06/whats-coming-in-eslint-8.0.0#rules-with-suggestions-now-require-the-metahassuggestions-property

This is very similar to the existing [eslint-plugin/require-meta-fixable](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/blob/master/docs/rules/require-meta-fixable.md) rule enforcing the correct presence of the `meta.fixable` property.
@bmish bmish force-pushed the require-meta-docs-suggestion branch from 971aa26 to 54cf3d3 Compare June 12, 2021 18:21
@bmish bmish marked this pull request as ready for review June 12, 2021 18:24
@bmish
Copy link
Member Author

bmish commented Jun 12, 2021

Now that the meta.hasSuggestions property has been accepted as part of ESLint 8, I have updated the rule to target that property, and we can merge/release this anytime.

@aladdin-add aladdin-add changed the title Add new rule require-meta-has-suggestions New: Add new rule require-meta-has-suggestions Jun 13, 2021
Copy link
Contributor

@aladdin-add aladdin-add left a comment

Choose a reason for hiding this comment

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

LGTM, thanks! 💯

@aladdin-add aladdin-add merged commit ff0ae38 into eslint-community:master Jun 13, 2021
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.

None yet

3 participants