no-invalid-regexp: Make allowConstructorFlags option case sensitive

[trosos]

Hi,

The allowConstructorFlags option of the no-invalid-regexp rule is case insensitive.
I believe that the option should be treated as case sensitive (because regexp flags are case sensitive).

What rule do you want to change?

no-invalid-regexp

What change to do you want to make?

Generate more warnings

How do you think the change should be implemented?

A new default behavior

Example code

const ESLint = require("eslint");
console.log(new ESLint.Linter().verify(
  'RegExp(".", "A");',
  { rules: { "no-invalid-regexp": [1,
    { allowConstructorFlags: ["a"] }
  ] } }
));

What does the rule currently do for this code?

The rule treats the option as case insensitive, and reports no issues.

What will the rule do after it's changed?

The rule should IMO report invalid flag.

Participation

• I am willing to submit a pull request to implement this change.

Additional comments

No response

[mdjermanovic]

I'm not sure why this option was made to be case-insensitive. There's no mention of this in #5352 or #5249.

By looking at the changes from #5352, it seems intentional as the regex that processes the option has the i flag, and there was this test: { code: "new RegExp('.', 'u')", options: [{ allowConstructorFlags: ["U"] }]}. The test was later accidentally removed in 672deb0.

I agree that this option should be case-sensitive, so that allowConstructorFlags: ["a"] does not allow RegExp(".", "A"), and that allowConstructorFlags: ["A"] does not allow RegExp(".", "a"). This change might be problematic though, as it might require some users to change their eslint configs and could be said to fall under "Major release > Part of the public API is removed or changed in an incompatible way > Rule schemas" in our semver policy.

Since this was apparently intentional, I'd like more input from the team on whether we should make this change (any ideas why this option is case-insensitive?) and whether we can treat it as a semver-minor bug fix.

[github-actions]

Oops! It looks like we lost track of this issue. What do we want to do here? This issue will auto-close in 7 days without an update.

[snitin315]

I'm 👍🏻 on this proposal as I also feel it should be case-sensitive. This would be a breaking change definitely so it should fall under the next major release.

[github-actions]

Oops! It looks like we lost track of this issue. What do we want to do here? This issue will auto-close in 7 days without an update.

[snitin315]

Marking this as accepted and adding it to the v9 board for tracking.

[mdjermanovic]

In no-constructor-return, instead of schema: {} it should be schema: [].

For example, "no-constructor-return": ["error", "foo"] should cause a validation error.

Edit: please disregard, I updated wrong issue.

[JoshuaKGoldberg]

#17533 is ready for review. Marked as draft for now as it's a breaking change.