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

feat: no-invalid-regexp support v flag #17404

Merged
merged 5 commits into from Jul 25, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
29 changes: 22 additions & 7 deletions lib/rules/no-invalid-regexp.js
Expand Up @@ -10,7 +10,7 @@

const RegExpValidator = require("@eslint-community/regexpp").RegExpValidator;
const validator = new RegExpValidator();
const validFlags = /[dgimsuy]/gu;
const validFlags = /[dgimsuvy]/gu;
const undefined1 = void 0;

//------------------------------------------------------------------------------
Expand Down Expand Up @@ -108,12 +108,14 @@ module.exports = {
/**
* Check syntax error in a given pattern.
* @param {string} pattern The RegExp pattern to validate.
* @param {boolean} uFlag The Unicode flag.
* @param {Object} flags The RegExp flags to validate.
* @param {boolean} [flags.unicode] The Unicode flag.
* @param {boolean} [flags.unicodeSets] The UnicodeSets flag.
* @returns {string|null} The syntax error.
*/
function validateRegExpPattern(pattern, uFlag) {
function validateRegExpPattern(pattern, flags) {
try {
validator.validatePattern(pattern, undefined1, undefined1, uFlag);
validator.validatePattern(pattern, undefined1, undefined1, flags);
return null;
} catch (err) {
return err.message;
Expand All @@ -131,10 +133,19 @@ module.exports = {
}
try {
validator.validateFlags(flags);
return null;
} catch {
return `Invalid flags supplied to RegExp constructor '${flags}'`;
}

/*
* `regexpp` checks the combination of `u` and `v` flags when parsing `Pattern` according to `ecma262`,
* but this rule may check only the flag when the pattern is unidentifiable, so check it here.
* https://tc39.es/ecma262/multipage/text-processing.html#sec-parsepattern
*/
if (flags.includes("u") && flags.includes("v")) {
return "Regex 'u' and 'v' flags cannot be used together";
}
return null;
}

return {
Expand Down Expand Up @@ -166,8 +177,12 @@ module.exports = {

// If flags are unknown, report the regex only if its pattern is invalid both with and without the "u" flag
flags === null
? validateRegExpPattern(pattern, true) && validateRegExpPattern(pattern, false)
: validateRegExpPattern(pattern, flags.includes("u"))
? (
validateRegExpPattern(pattern, { unicode: true, unicodeSets: false }) &&
validateRegExpPattern(pattern, { unicode: false, unicodeSets: true }) &&
mdjermanovic marked this conversation as resolved.
Show resolved Hide resolved
validateRegExpPattern(pattern, { unicode: false, unicodeSets: false })
)
: validateRegExpPattern(pattern, { unicode: flags.includes("u"), unicodeSets: flags.includes("v") })
);

if (message) {
Expand Down
2 changes: 1 addition & 1 deletion package.json
Expand Up @@ -61,7 +61,7 @@
"bugs": "https://github.com/eslint/eslint/issues/",
"dependencies": {
"@eslint-community/eslint-utils": "^4.2.0",
"@eslint-community/regexpp": "^4.4.0",
"@eslint-community/regexpp": "^4.6.0",
"@eslint/eslintrc": "^2.1.0",
"@eslint/js": "8.44.0",
"@humanwhocodes/config-array": "^0.11.10",
Expand Down
50 changes: 50 additions & 0 deletions tests/lib/rules/no-invalid-regexp.js
Expand Up @@ -81,6 +81,14 @@ ruleTester.run("no-invalid-regexp", rule, {
"new RegExp('\\\\p{Script=Vith}', 'u')",
"new RegExp('\\\\p{Script=Vithkuqi}', 'u')",

// ES2024
"new RegExp('[A--B]', 'v')",
"new RegExp('[A&&B]', 'v')",
"new RegExp('[A--[0-9]]', 'v')",
"new RegExp('[\\\\p{Basic_Emoji}--\\\\q{a|bc|def}]', 'v')",
"new RegExp('[A--B]', flags)", // valid only with `v` flag
"new RegExp('[[]\\\\u{0}*', flags)", // valid only with `u` flag

// allowConstructorFlags
{
code: "new RegExp('.', 'g')",
Expand Down Expand Up @@ -288,6 +296,48 @@ ruleTester.run("no-invalid-regexp", rule, {
data: { message: "Invalid flags supplied to RegExp constructor 'z'" },
type: "NewExpression"
}]
},

// ES2024
{
code: "new RegExp('[[]', 'v');",
errors: [{
messageId: "regexMessage",
data: { message: "Invalid regular expression: /[[]/u: Unterminated character class" },
type: "NewExpression"
}]
},
{
code: "new RegExp('.', 'uv');",
errors: [{
messageId: "regexMessage",
data: { message: "Regex 'u' and 'v' flags cannot be used together" },
type: "NewExpression"
}]
},
{
code: "new RegExp(pattern, 'uv');",
errors: [{
messageId: "regexMessage",
data: { message: "Regex 'u' and 'v' flags cannot be used together" },
type: "NewExpression"
}]
mdjermanovic marked this conversation as resolved.
Show resolved Hide resolved
},
{
code: "new RegExp('[A--B]' /* valid only with `v` flag */, 'u')",
errors: [{
messageId: "regexMessage",
data: { message: "Invalid regular expression: /[A--B]/u: Range out of order in character class" },
type: "NewExpression"
}]
},
{
code: "new RegExp('[[]\\\\u{0}*' /* valid only with `u` flag */, 'v')",
errors: [{
messageId: "regexMessage",
data: { message: "Invalid regular expression: /[[]\\u{0}*/u: Unterminated character class" },
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 noticed that the error message issued by regexpp was wrong. i will fix the error message.

Copy link
Member

Choose a reason for hiding this comment

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

Okay, waiting for the fix then. Otherwise, this LGTM.

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 fixed the error message.
1a775c4

type: "NewExpression"
}]
}
]
});