fix!: correct camelcase rule schema for allow option

[eMerzh]

Prerequisites checklist

• I have read the contributing guidelines.

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

The Json schema of the camelCase rule validate that the first element is a string instead of validating that it's an array of string

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

What changes did you make? (Give an overview)

Correct the Json Schema

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

Nope

[eslint-github-bot]

Hi @eMerzh!, 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'.

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]

✅ Deploy Preview for docs-eslint ready!

Name	Link
🔨 Latest commit	3b65d7c
🔍 Latest deploy log	https://app.netlify.com/sites/docs-eslint/deploys/660a7745ae2f020008dac466
😎 Deploy Preview	https://deploy-preview-18232--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.

[mdjermanovic]

Good catch!

Current schema actually allows arrays with multiple elements, as in this test case:

eslint/tests/lib/rules/camelcase.js

Lines 328 to 331 in dadc5bf

	{
	code: "ignored_foo = 0; ignored_bar = 1;",
	options: [{ allow: ["ignored_foo", "ignored_bar"] }]
	},

But it only validates that the first element is a string, while the others can be anything, as in this example:

/* eslint camelcase: ["error", { allow: ["a_b", 5, "c_d"] }] */ // ok configuration

let a_b; // ok
let c_d; // ok
let e_f; // error here

Playground demo

[mdjermanovic]

Per our semver policies, this is a breaking change (Part of the public API is removed or changed in an incompatible way > Rule schemas).

@eslint/eslint-tsc Since we've already released v9 rc, looks like we'll have to leave this for ESLint v10?

[eMerzh]

@mdjermanovic i understand your position but if i understand, this shouldn't break your CI as normally you could "only" have "one string" in the array... thus this technically allows you do to more

Also it's only in the validation side, not on the lint itself

[mdjermanovic]

@mdjermanovic i understand your position but if i understand, this shouldn't break your CI as normally you could "only" have "one string" in the array... thus this technically allows you do to more

The current schema already allows multiple strings in the array (demo). items: [{ type: "string" }] means that the first element must be a string, but it doesn't disallow additional elements.

After this change, a configuration like ["error", { allow: ["a_b", "c_d", 5] }] would break CI because 5 is not a string. Currently, this configuration works (demo). That 5 is currently a no-op, but it doesn't cause validation errors.

[Shinigami92]

This will fix eslint-types/eslint-define-config#278 semi-automatically (after the generate rule-types script is executed)

[fasttime]

This looks like a bug that should be fixed. I'm noting that the current logic allows arbitrary elements in the allow array after the first string without any kind of validation. If one of the elements it's a Symbol, or an object that can't be formatted, or even an invalid regex pattern like "foo)", the rule will crash.

/*eslint camelcase: ["error", {allow: ["foo", {toString: null}]}]*/

function bar_baz() {
}

Playground link

We could look into how to improve option validation for this rule in a non-breaking way, but that wouldn't be the end of the job, so probably it's best to schedule the proper fix for v10 if it can't be done earlier.

[Tanujkanti4441]

What i also noticed that if the allow: [""] option has an empty string then rule doesn't report the camelcase variables name - playground and also rule doesn't check whether the string has an underscore or not, should we validate the option this way?

[nzakas]

@mdjermanovic If we are categorizing this as a bug fix, then I'm okay with including in the next RC. RCs (and betas) really are designed to catch bugs. As long as we're not adding anything new, I think it's appropriate to include in v9.

[aladdin-add]

👍 to include it in v9, but we need:

1. add a few tests.
2. add it in the migration guide: https://github.com/eslint/eslint/blob/main/docs/src/use/migrate-to-9.0.0.md

[mdjermanovic]

@mdjermanovic If we are categorizing this as a bug fix, then I'm okay with including in the next RC. RCs (and betas) really are designed to catch bugs. As long as we're not adding anything new, I think it's appropriate to include in v9.

Sounds good to me, marking this as accepted.

[eMerzh]

i was thinking about adding those in tests/lib/rules/camelcase.js but it is really the place? 🤔

also i was thinking of a test with 2-3 allows and 1 or 2 with numbers or objects in options, did you think about something else?

[mdjermanovic]

RuleTester currently doesn't support testing invalid options. (it will when we implement https://github.com/eslint/rfcs/tree/main/designs/2023-test-rule-errors).

I think it would be fine to just add a couple more tests with valid options (allow with multiple string elements).

[mdjermanovic]

This code would be valid without options, maybe you intended to use some other variable name?

[aladdin-add]

LGTM, thanks! Would like another review before merging.

[mdjermanovic]

LGTM, thanks! Since this is a breaking change, it requires another approval from @eslint/eslint-tsc before merging.

[nzakas]

LGTM. Thanks!