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 new require-meta-default-options rule #502

Merged

Conversation

FloEdelmann
Copy link
Contributor

Fixes #499.

@bmish
Copy link
Member

bmish commented Nov 22, 2024

@JoshuaKGoldberg

Copy link
Contributor

@JoshuaKGoldberg JoshuaKGoldberg 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 for the ping & thanks for making this!

Left some questions that I'll defer to a maintainer.

docs/rules/require-meta-default-options.md Outdated Show resolved Hide resolved
`,
`
module.exports = {
meta: { schema: [{}, {}], defaultOptions: [1] },
Copy link
Contributor

Choose a reason for hiding this comment

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

[Non-actionable] 🤔 it does strike me as odd that linting would permit a schema with a noticeably different length than defaultOptions. In theory a rule could enforce they have the same length, and the same type of values... but that feels more like type checking.

So, not requesting changes here, just non-actionable musing & confirming 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Some rules accept a variable length of arguments, e.g. vue/no-restricted-v-on (but actually, the default value should be [] here, which is also forbidden by the rule, so maybe something needs to be done anyway).

Also, some rules have anyOf or oneOf in their root schema, with different number of properties in each case, e.g. @stylistic/function-call-spacing. Enforcing a strict number of default options is difficult here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've allowed array root schemas to have empty defaultOptions now: 235bb00

lib/rules/consistent-output.js Show resolved Hide resolved
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 13e625a into eslint-community:main Dec 18, 2024
7 checks passed
@FloEdelmann FloEdelmann deleted the require-meta-default-options branch December 18, 2024 08:48
@aladdin-add
Copy link
Contributor

it has been shipped in v6.4.0 🎉

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.

Rule suggestion: require-meta-default-options
4 participants