feat: add meta.defaultOptions #17656

JoshuaKGoldberg · 2023-10-16T16:50:19Z

Prerequisites checklist

I have read the contributing guidelines.

What is the purpose of this pull request? (put an "X" next to an item)

[ ] Documentation update
[ ] Bug fix (template)
[ ] New rule (template)
[ ] Changes an existing rule (template)
[ ] Add autofix to a rule
[ ] Add a CLI option
[x] Add something to the core
[ ] Other, please explain:

What changes did you make? (Give an overview)

Augments linting to factor in a new optional meta.defaultOptions. If a rule describes default options this way, any user-provided options are merged on top of the default options.

These defaults are applied by a new deepMergeArrays utility in:

ConfigValidator
Linter in two places:
- For configs: by augmenting the existing getRuleOptions helper
- For inline comments in flat config: with a new getRuleOptionsInline helper
  - Inline comments in legacy configs go through RuleValidator instead
RuleValidator

Is there anything you'd like reviewers to focus on?

eslint-github-bot · 2023-10-16T16:50:25Z

Hi @JoshuaKGoldberg!, thanks for the Pull Request

The pull request title isn't properly formatted. We ask that you update the pull request title to match this format, as we use it to generate changelogs and automate releases.

The commit message tag wasn't recognized. Did you mean "docs", "fix", or "feat"?
There should be a space following the initial tag and colon, for example 'feat: Message'.
The first letter of the tag should be in lowercase

To Fix: You can fix this problem by clicking 'Edit' next to the pull request title at the top of this page.

Read more about contributing to ESLint here

netlify · 2023-10-16T16:50:26Z

✅ Deploy Preview for docs-eslint ready!

Name	Link
🔨 Latest commit	`6657263`
🔍 Latest deploy log	https://app.netlify.com/sites/docs-eslint/deploys/65ff19183446850008e3746f
😎 Deploy Preview	https://deploy-preview-17656--docs-eslint.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

eslint-github-bot · 2023-10-19T03:29:26Z

Hi @JoshuaKGoldberg!, thanks for the Pull Request

The pull request title isn't properly formatted. We ask that you update the pull request title to match this format, as we use it to generate changelogs and automate releases.

The commit message tag wasn't recognized. Did you mean "docs", "fix", or "feat"?
There should be a space following the initial tag and colon, for example 'feat: Message'.
The first letter of the tag should be in lowercase

To Fix: You can fix this problem by clicking 'Edit' next to the pull request title at the top of this page.

Read more about contributing to ESLint here

eslint-github-bot · 2023-10-26T10:06:49Z

Hi @JoshuaKGoldberg!, thanks for the Pull Request

The pull request title isn't properly formatted. We ask that you update the pull request title to match this format, as we use it to generate changelogs and automate releases.

The commit message tag wasn't recognized. Did you mean "docs", "fix", or "feat"?
There should be a space following the initial tag and colon, for example 'feat: Message'.
The first letter of the tag should be in lowercase

To Fix: You can fix this problem by clicking 'Edit' next to the pull request title at the top of this page.

Read more about contributing to ESLint here

github-actions · 2023-11-05T22:31:18Z

Hi everyone, it looks like we lost track of this pull request. Please review and see what the next steps are. This pull request will auto-close in 7 days without an update.

tests/lib/linter/deep-merge.js

github-actions · 2023-11-16T22:31:28Z

Hi everyone, it looks like we lost track of this pull request. Please review and see what the next steps are. This pull request will auto-close in 7 days without an update.

Rec0iL99 · 2023-11-17T13:42:31Z

Not stale. @JoshuaKGoldberg there's a review comment for you to look into. Thanks.

eslint-github-bot · 2023-11-21T02:42:30Z

Hi @JoshuaKGoldberg!, thanks for the Pull Request

The pull request title isn't properly formatted. We ask that you update the pull request title to match this format, as we use it to generate changelogs and automate releases.

The commit message tag wasn't recognized. Did you mean "docs", "fix", or "feat"?
There should be a space following the initial tag and colon, for example 'feat: Message'.
The first letter of the tag should be in lowercase

To Fix: You can fix this problem by clicking 'Edit' next to the pull request title at the top of this page.

Read more about contributing to ESLint here

eslint-github-bot · 2023-11-21T02:58:00Z

Hi @JoshuaKGoldberg!, thanks for the Pull Request

The pull request title isn't properly formatted. We ask that you update the pull request title to match this format, as we use it to generate changelogs and automate releases.

The commit message tag wasn't recognized. Did you mean "docs", "fix", or "feat"?
There should be a space following the initial tag and colon, for example 'feat: Message'.
The first letter of the tag should be in lowercase

To Fix: You can fix this problem by clicking 'Edit' next to the pull request title at the top of this page.

Read more about contributing to ESLint here

eslint-github-bot · 2023-11-21T03:58:25Z

Hi @JoshuaKGoldberg!, thanks for the Pull Request

The pull request title isn't properly formatted. We ask that you update the pull request title to match this format, as we use it to generate changelogs and automate releases.

The commit message tag wasn't recognized. Did you mean "docs", "fix", or "feat"?
There should be a space following the initial tag and colon, for example 'feat: Message'.
The first letter of the tag should be in lowercase

To Fix: You can fix this problem by clicking 'Edit' next to the pull request title at the top of this page.

Read more about contributing to ESLint here

eslint-github-bot · 2023-11-21T04:09:19Z

Hi @JoshuaKGoldberg!, thanks for the Pull Request

The pull request title isn't properly formatted. We ask that you update the pull request title to match this format, as we use it to generate changelogs and automate releases.

The commit message tag wasn't recognized. Did you mean "docs", "fix", or "feat"?
There should be a space following the initial tag and colon, for example 'feat: Message'.
The first letter of the tag should be in lowercase

To Fix: You can fix this problem by clicking 'Edit' next to the pull request title at the top of this page.

Read more about contributing to ESLint here

eslint-github-bot · 2023-11-21T04:10:46Z

Hi @JoshuaKGoldberg!, thanks for the Pull Request

The pull request title isn't properly formatted. We ask that you update the pull request title to match this format, as we use it to generate changelogs and automate releases.

The commit message tag wasn't recognized. Did you mean "docs", "fix", or "feat"?
There should be a space following the initial tag and colon, for example 'feat: Message'.
The first letter of the tag should be in lowercase

To Fix: You can fix this problem by clicking 'Edit' next to the pull request title at the top of this page.

Read more about contributing to ESLint here

eslint-github-bot · 2023-11-21T05:22:23Z

Hi @JoshuaKGoldberg!, thanks for the Pull Request

The pull request title isn't properly formatted. We ask that you update the pull request title to match this format, as we use it to generate changelogs and automate releases.

The commit message tag wasn't recognized. Did you mean "docs", "fix", or "feat"?
There should be a space following the initial tag and colon, for example 'feat: Message'.
The first letter of the tag should be in lowercase

To Fix: You can fix this problem by clicking 'Edit' next to the pull request title at the top of this page.

Read more about contributing to ESLint here

github-actions · 2023-12-01T22:31:21Z

Hi everyone, it looks like we lost track of this pull request. Please review and see what the next steps are. This pull request will auto-close in 7 days without an update.

JoshuaKGoldberg · 2024-02-16T00:07:03Z

tests/lib/linter/linter.js

@@ -6739,6 +6762,52 @@ var a = "test2";
 });
 });

+ describe("options", () => {
+ it("rules should apply meta.defaultOptions on top of schema defaults", () => {


Note that this is different from main. I asked about the code intent: #17656 (comment)

github-actions · 2024-02-29T22:31:30Z

Hi everyone, it looks like we lost track of this pull request. Please review and see what the next steps are. This pull request will auto-close in 7 days without an update.

Rec0iL99 · 2024-03-01T04:56:05Z

Waiting for @mdjermanovic to review here.

@JoshuaKGoldberg there's a conflict to resolve.

github-actions · 2024-03-22T22:31:15Z

Hi everyone, it looks like we lost track of this pull request. Please review and see what the next steps are. This pull request will auto-close in 7 days without an update.

JoshuaKGoldberg · 2024-03-23T18:11:18Z

I believe it's waiting on re-review from @mdjermanovic?

There were a few merge conflicts introduced since the last merge from main. I resolved them just now.

mdjermanovic · 2024-03-29T17:36:45Z

Sorry for the delay. A problem is that eslintrc will still be supported in ESLint v9, and rules should work the same regardless of which config system is used.

In flat config, default options are merged before validation. I think this generally makes more sense than validating provided options first and then merging defaults. Ajv also applies schema defaults before validating options. In eslintrc mode, however, as currently implemented, default options are merged after validation (right before running rules).

This means that the same configuration for a rule could be valid in one config system but invalid in the other.

For example, if this is a rule:

{
    meta: {
        defaultOptions: [{
            foo: 42
        }],
        schema: [{
            type: "object",
            maxProperties: 2 // allows one or two in addition to "foo"?
        }]
    },
    create() {
        return {};
    }
}

configuration ["error", { bar: 6, baz: 7 }] would be valid in .eslintrc.js file, but invalid in eslint.config.js file (Value {"foo":42,"bar":6,"baz":7} should NOT have more than 2 properties).

I think we should address this, but there are two possible ways:

Change eslintrc to apply defaults before validating options.
Change flat config to apply defaults after validating options.

I'd be in favor of 1), although it probably requires changes in @eslint/eslintrc package. 2) seems easier to implement at the moment, but it's unlikely that we'd ever change this behavior later, so if we think that merging defaults should be before the validation, it's probably better to do 1). Thoughts?

mdjermanovic · 2024-03-29T17:42:38Z

lib/config/rule-validator.js

 if (validateRule) {

- validateRule(ruleOptions.slice(1));
+ const slicedOptions = ruleOptions.slice(1);
+ const mergedOptions = deepMergeArrays(rule.meta?.defaultOptions, slicedOptions);


Defaults options wouldn't be applied if the rule has schema: false?

mdjermanovic · 2024-03-29T17:53:36Z

lib/linter/linter.js

- let ruleOptions = Array.isArray(ruleValue) ? ruleValue : [ruleValue];
-
- assertIsRuleSeverity(ruleId, ruleOptions[0]);
+ let ruleOptions = getRuleOptionsInline(ruleId, ruleValue, rule.meta?.defaultOptions);


Merging default options at this point would cause a bug when the configuration comment just overrides severity (https://eslint.org/docs/next/use/migrate-to-9.0.0#eslint-comment-options) because for the rest of the code below it would look like the comment had options specified.

We could let ruleValidator.validate() below merge default options, and then get the final rule config from the config that was passed to it?

github-actions · 2024-04-08T22:31:41Z

Hi everyone, it looks like we lost track of this pull request. Please review and see what the next steps are. This pull request will auto-close in 7 days without an update.

Tanujkanti4441 · 2024-04-09T07:52:52Z

Hi @JoshuaKGoldberg, some reviews are there you can checkout.

JoshuaKGoldberg · 2024-04-10T19:07:08Z

I'd be in favor of 1), although it probably requires changes in @eslint/eslintrc package.

I'm in favor of that as well. Is there some process I should follow?

nzakas · 2024-04-11T20:04:30Z

@JoshuaKGoldberg nothing special, just go ahead and open a PR on eslint/eslintrc and mention this PR in it.

JoshuaKGoldberg · 2024-04-26T14:18:32Z

I still plan on continuing this work, but likely won't be able to for at least the month of May. ESLint v9 & typescript-eslint v8 are much higher priority for me at the moment.

If anybody else wants to tackle the eslint/eslintrc changes in the meantime, please do go ahead! That would be lovely 🙂. But if not, I should be able to later this summer.

[Reference] feat: add meta.defaultOptions

9b70e84

JoshuaKGoldberg mentioned this pull request Oct 19, 2023

feat: Support for meta.defaultOptions on rules eslint/rfcs#113

Merged

Removed optionsRaw

90504b7

computed-property-spacing: defaultOptions

7a61675

github-actions bot added the Stale label Nov 5, 2023

mdjermanovic reviewed Nov 6, 2023

View reviewed changes

tests/lib/linter/deep-merge.js Outdated Show resolved Hide resolved

github-actions bot removed the Stale label Nov 6, 2023

github-actions bot added the Stale label Nov 16, 2023

Rec0iL99 removed the Stale label Nov 17, 2023

Merge branch 'main' into rule-meta-default-options

13f6270

fix: handle object type mismatches in merging

e493311

Validate arrays in flat-config-array

1bb2568

Fix rule defaultOptions typos

629f691

Put back getRuleOptions as before

6276b71

Apply deep merging in config-validator and rule-validator

1f36e93

github-actions bot added the Stale label Dec 1, 2023

JoshuaKGoldberg commented Feb 16, 2024

View reviewed changes

github-actions bot added the Stale label Feb 29, 2024

Rec0iL99 removed the Stale label Mar 1, 2024

Merge branch 'main'

2b8659b

github-actions bot added rule Relates to ESLint's core rules core Relates to ESLint's core APIs and features labels Mar 5, 2024

JoshuaKGoldberg dismissed stale reviews from bmish and nzakas via 2b8659b March 12, 2024 12:05

github-actions bot added the Stale label Mar 22, 2024

Merge branch 'main'

6657263

github-actions bot removed the Stale label Mar 23, 2024

aladdin-add self-requested a review March 28, 2024 13:35

mdjermanovic reviewed Mar 29, 2024

View reviewed changes

github-actions bot added the Stale label Apr 8, 2024

Tanujkanti4441 added core Relates to ESLint's core APIs and features and removed core Relates to ESLint's core APIs and features Stale labels Apr 9, 2024

Tanujkanti4441 assigned JoshuaKGoldberg Apr 9, 2024

JoshuaKGoldberg marked this pull request as draft April 26, 2024 14:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add meta.defaultOptions #17656

feat: add meta.defaultOptions #17656

JoshuaKGoldberg commented Oct 16, 2023 •

edited

eslint-github-bot bot commented Oct 16, 2023

netlify bot commented Oct 16, 2023 •

edited

eslint-github-bot bot commented Oct 19, 2023

eslint-github-bot bot commented Oct 26, 2023

github-actions bot commented Nov 5, 2023

github-actions bot commented Nov 16, 2023

Rec0iL99 commented Nov 17, 2023

eslint-github-bot bot commented Nov 21, 2023

eslint-github-bot bot commented Nov 21, 2023

eslint-github-bot bot commented Nov 21, 2023

eslint-github-bot bot commented Nov 21, 2023

eslint-github-bot bot commented Nov 21, 2023

eslint-github-bot bot commented Nov 21, 2023

github-actions bot commented Dec 1, 2023

JoshuaKGoldberg Feb 16, 2024

github-actions bot commented Feb 29, 2024

Rec0iL99 commented Mar 1, 2024

github-actions bot commented Mar 22, 2024

JoshuaKGoldberg commented Mar 23, 2024

mdjermanovic commented Mar 29, 2024

mdjermanovic Mar 29, 2024

mdjermanovic Mar 29, 2024

github-actions bot commented Apr 8, 2024

Tanujkanti4441 commented Apr 9, 2024

JoshuaKGoldberg commented Apr 10, 2024

nzakas commented Apr 11, 2024

JoshuaKGoldberg commented Apr 26, 2024

feat: add meta.defaultOptions #17656

Are you sure you want to change the base?

feat: add meta.defaultOptions #17656

Conversation

JoshuaKGoldberg commented Oct 16, 2023 • edited

Prerequisites checklist

What is the purpose of this pull request? (put an "X" next to an item)

What changes did you make? (Give an overview)

Is there anything you'd like reviewers to focus on?

eslint-github-bot bot commented Oct 16, 2023

netlify bot commented Oct 16, 2023 • edited

✅ Deploy Preview for docs-eslint ready!

eslint-github-bot bot commented Oct 19, 2023

eslint-github-bot bot commented Oct 26, 2023

github-actions bot commented Nov 5, 2023

github-actions bot commented Nov 16, 2023

Rec0iL99 commented Nov 17, 2023

eslint-github-bot bot commented Nov 21, 2023

eslint-github-bot bot commented Nov 21, 2023

eslint-github-bot bot commented Nov 21, 2023

eslint-github-bot bot commented Nov 21, 2023

eslint-github-bot bot commented Nov 21, 2023

eslint-github-bot bot commented Nov 21, 2023

github-actions bot commented Dec 1, 2023

JoshuaKGoldberg Feb 16, 2024

Choose a reason for hiding this comment

github-actions bot commented Feb 29, 2024

Rec0iL99 commented Mar 1, 2024

github-actions bot commented Mar 22, 2024

JoshuaKGoldberg commented Mar 23, 2024

mdjermanovic commented Mar 29, 2024

mdjermanovic Mar 29, 2024

Choose a reason for hiding this comment

mdjermanovic Mar 29, 2024

Choose a reason for hiding this comment

github-actions bot commented Apr 8, 2024

Tanujkanti4441 commented Apr 9, 2024

JoshuaKGoldberg commented Apr 10, 2024

nzakas commented Apr 11, 2024

JoshuaKGoldberg commented Apr 26, 2024

JoshuaKGoldberg commented Oct 16, 2023 •

edited

netlify bot commented Oct 16, 2023 •

edited